From b70e2c2fc2c92ce983dc3b52d8082513cd6206ae Mon Sep 17 00:00:00 2001 From: Tyler Treat Date: Wed, 7 Jun 2017 12:25:56 -0500 Subject: [PATCH 1/6] Implement config reload support for TLS Allows reloading TLS config. This includes enabling/disabling TLS, rotating keys/certs, enabling/disabling client verification, etc. --- server/configs/reload/reload.conf | 8 ++++++++ server/reload.go | 17 +++++++++++++++++ server/reload_test.go | 22 +++++++++++++++------- 3 files changed, 40 insertions(+), 7 deletions(-) diff --git a/server/configs/reload/reload.conf b/server/configs/reload/reload.conf index 770455bca17..3be276091f0 100644 --- a/server/configs/reload/reload.conf +++ b/server/configs/reload/reload.conf @@ -2,3 +2,11 @@ debug: true # enable on reload trace: true # enable on reload logtime: false + +# Enable TLS on reload +tls { + cert_file: "../test/configs/certs/server-cert.pem" + key_file: "../test/configs/certs/server-key.pem" + ca_file: "../test/configs/certs/ca.pem" + verify: true +} diff --git a/server/reload.go b/server/reload.go index c44b5036a31..c037fbbdbe7 100644 --- a/server/reload.go +++ b/server/reload.go @@ -3,6 +3,7 @@ package server import ( + "crypto/tls" "errors" "fmt" "reflect" @@ -41,6 +42,20 @@ func (d *debugOption) Apply(server *Server) { server.Noticef("Reloaded: debug = %v", d.newValue) } +// tlsOption implements the option interface for the `tls` setting. +type tlsOption struct { + newValue *tls.Config +} + +// Apply the tls change. +func (t *tlsOption) Apply(server *Server) { + tlsRequired := t.newValue != nil + server.info.TLSRequired = tlsRequired + server.info.TLSVerify = tlsRequired && t.newValue.ClientAuth == tls.RequireAndVerifyClientCert + server.generateServerInfoJSON() + server.Noticef("Reloaded: tls") +} + // Reload reads the current configuration file and applies any supported // changes. This returns an error if the server was not started with a config // file or an option which doesn't support hot-swapping was changed. @@ -99,6 +114,8 @@ func (s *Server) diffOptions(newOpts *Options) ([]option, error) { diffOpts = append(diffOpts, &traceOption{newValue.(bool)}) case "debug": diffOpts = append(diffOpts, &debugOption{newValue.(bool)}) + case "tlsconfig": + diffOpts = append(diffOpts, &tlsOption{newValue.(*tls.Config)}) default: // Bail out if attempting to reload any unsupported options. return nil, fmt.Errorf("Config reload not supported for %s", field.Name) diff --git a/server/reload_test.go b/server/reload_test.go index 33f21866e91..8a958dfb462 100644 --- a/server/reload_test.go +++ b/server/reload_test.go @@ -193,12 +193,20 @@ func TestConfigReload(t *testing.T) { } // Ensure config changed. - var updatedGolden *Options = &Options{} - *updatedGolden = *golden - updatedGolden.Trace = true - updatedGolden.Debug = true - if !reflect.DeepEqual(updatedGolden, server.getOpts()) { - t.Fatalf("Options are incorrect.\nexpected: %+v\ngot: %+v", - updatedGolden, server.getOpts()) + updated := server.getOpts() + if !updated.Trace { + t.Fatal("Expected Trace to be true") + } + if !updated.Debug { + t.Fatal("Expected Debug to be true") + } + if updated.TLSConfig == nil { + t.Fatal("Expected TLSConfig to be non-nil") + } + if !server.info.TLSRequired { + t.Fatal("Expected TLSRequired to be true") + } + if !server.info.TLSVerify { + t.Fatal("Expected TLSVerify to be true") } } From b7211f6dc87a509c7d2d79af73d22370c9e15b72 Mon Sep 17 00:00:00 2001 From: Tyler Treat Date: Wed, 7 Jun 2017 14:58:30 -0500 Subject: [PATCH 2/6] Improve readability of TLSVerify assignment --- server/reload.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/server/reload.go b/server/reload.go index c037fbbdbe7..684f0b47fc0 100644 --- a/server/reload.go +++ b/server/reload.go @@ -51,7 +51,9 @@ type tlsOption struct { func (t *tlsOption) Apply(server *Server) { tlsRequired := t.newValue != nil server.info.TLSRequired = tlsRequired - server.info.TLSVerify = tlsRequired && t.newValue.ClientAuth == tls.RequireAndVerifyClientCert + if tlsRequired { + server.info.TLSVerify = (t.newValue.ClientAuth == tls.RequireAndVerifyClientCert) + } server.generateServerInfoJSON() server.Noticef("Reloaded: tls") } From d172ca580190169443774a6350e3295064a1eea0 Mon Sep 17 00:00:00 2001 From: Tyler Treat Date: Wed, 7 Jun 2017 16:34:39 -0500 Subject: [PATCH 3/6] Add unit tests around TLS config reload --- server/configs/basic.conf | 1 + server/configs/certs/cert.new.pem | 19 ++++ server/configs/certs/key.new.pem | 27 +++++ server/configs/tls_verify_test.conf | 11 ++ server/reload.go | 4 +- server/reload_test.go | 171 ++++++++++++++++++++++++++++ 6 files changed, 232 insertions(+), 1 deletion(-) create mode 100644 server/configs/basic.conf create mode 100644 server/configs/certs/cert.new.pem create mode 100644 server/configs/certs/key.new.pem create mode 100644 server/configs/tls_verify_test.conf diff --git a/server/configs/basic.conf b/server/configs/basic.conf new file mode 100644 index 00000000000..f826ee21756 --- /dev/null +++ b/server/configs/basic.conf @@ -0,0 +1 @@ +listen: localhost:4443 diff --git a/server/configs/certs/cert.new.pem b/server/configs/certs/cert.new.pem new file mode 100644 index 00000000000..ccf5db7ec13 --- /dev/null +++ b/server/configs/certs/cert.new.pem @@ -0,0 +1,19 @@ +-----BEGIN CERTIFICATE----- +MIIDHjCCAgagAwIBAgIQIzutzCXGYFnGCOLeORrJdDANBgkqhkiG9w0BAQsFADAS +MRAwDgYDVQQKEwdBY21lIENvMB4XDTE3MDYwNzIxMDExMVoXDTE4MDYwNzIxMDEx +MVowEjEQMA4GA1UEChMHQWNtZSBDbzCCASIwDQYJKoZIhvcNAQEBBQADggEPADCC +AQoCggEBAL6+3Ab4M5gYDfACnOm5xLSMwEgjFyeFE4HRgWbC+672nm8CKqyPZLkp +ZIkS05ugpMXsOPY7JoTWgHL3L/wE0G2igh5jro88uwH64QzDEPmcS8yWqI1ypJq9 +2+86aWxpLYoi549qZW9Vj/IDgPEG72IlanQ1rSCnSCeeF++bFNMSxfPPFiA+KJKL +PIdP5MPxkNGAZ1YAlJSr+vhwQTYxJ5HbGiXMpkmLwIwSadGKAohZg9i7NMiKQBU5 +2N/+6U+qtEmW7JyF1/Bkzbuf9wRGN471BXXomm/2GcSlD2PDmyOERvCnAYnYc2BV +QG0/S8YIGa+nOm460tZfuH6KWVCyG98CAwEAAaNwMG4wDgYDVR0PAQH/BAQDAgKk +MB0GA1UdJQQWMBQGCCsGAQUFBwMBBggrBgEFBQcDAjAPBgNVHRMBAf8EBTADAQH/ +MCwGA1UdEQQlMCOCCWxvY2FsaG9zdIEWdHlsZXIudHJlYXRAYXBjZXJhLmNvbTAN +BgkqhkiG9w0BAQsFAAOCAQEALFfqxV5BWDOILqh+dEtPBGdNyN9CYLLtmibr3eeS +PvF6jHC0ik2u5nBdqSaI3QtwaF65KXtLdaq9N2UvzehWpcW1Wy0PtKB5QIN/hR7I +aRYEejz+IEnduoK6XKJHR7RO7I9ipI9CdvmKJHB8ogGt7a42lIRQMbIeU+68ceiU +WfoHXPiJyib9uw/ewEgg/J+jnbDMJARnjR76P942iHrg/elUNLBBxarxwGj3tAHF +M2ER7mVj+8Y98Pgw82avX6oxhiDM4N6UbyYhuxjoda4Av4dNb7MABHDhdIgSRI1Y +a6pSdajhUg7Dj51IKPkOZ4d7KT3IPeTbJCI/S+tHNeWlLA== +-----END CERTIFICATE----- diff --git a/server/configs/certs/key.new.pem b/server/configs/certs/key.new.pem new file mode 100644 index 00000000000..40aaccf7d0c --- /dev/null +++ b/server/configs/certs/key.new.pem @@ -0,0 +1,27 @@ +-----BEGIN RSA PRIVATE KEY----- +MIIEowIBAAKCAQEAvr7cBvgzmBgN8AKc6bnEtIzASCMXJ4UTgdGBZsL7rvaebwIq +rI9kuSlkiRLTm6Ckxew49jsmhNaAcvcv/ATQbaKCHmOujzy7AfrhDMMQ+ZxLzJao +jXKkmr3b7zppbGktiiLnj2plb1WP8gOA8QbvYiVqdDWtIKdIJ54X75sU0xLF888W +ID4okos8h0/kw/GQ0YBnVgCUlKv6+HBBNjEnkdsaJcymSYvAjBJp0YoCiFmD2Ls0 +yIpAFTnY3/7pT6q0SZbsnIXX8GTNu5/3BEY3jvUFdeiab/YZxKUPY8ObI4RG8KcB +idhzYFVAbT9LxggZr6c6bjrS1l+4fopZULIb3wIDAQABAoIBACr89LWVZntWoH2A ++UArn8tZFVSso+FCOp09TD6Onw5VgmteP6PYRUj9rSy/U3V1hO0eSdAkkI/Lj/NZ +BjV0GE09HLogmQyrETJnCiVIKSE4OlUHd0E5nyNIurJ1paDLK3pAV5OY1Pd8fw55 +/6tSdszVxeIe3r/HM5nKJXbYqp7O7rbyI+d9Q5dBlP1cRPHrjpTPVv16MjFhxIPB +R+C0n61CTRjRJRUN9xEwSe7iOy0ynCACtGZpv+srLWx4yj8QBg7+blm+9C67jcBs +bny0VQ344DYHiVNr/cpPezn2LlHWFpCuy+7sVTq9aAZ8k/UDPxOe9eHN93O4WCru +GAa77iECgYEA3inEhpzCLsXvIUxkf2Q4wWBVBs3VjcVcdPw3baCWlwqeV6iCJHTw +WbLQfec7m3kbDZiZvhJtQyqfNndSLwxC9Nb3s2dKhLuzX6AiN84+Z5M8CGor7iFM +pDdDV5igh9kyrMr3iR01/bB2hb5YQPzoAuV9136vP143mMG4HdMaGncCgYEA28wb +ODvhj0y+1heS/swcOG+Bw+/Cwqp/SxkREoBYc7lLNXQQ5VmQM/DOXTbgyo6+u8/n ++3VbT0OjmjMLYodynPkl1qcVTIVc38YbxKsdPU+c30DHU4xyUjBalcMjw4ayqCNx +epzz62PohrASFpQr1r+oVnpPsCSDkmMI4d4Z+9kCgYAlEmMw80eT9oOI0u6SM28l +FaYalI5mMeDTxKKbMIjwe10g04Wj/797uFMCL2vK7dKN2kENbpW894fJ1u9n2mvx +301GKp5Mt+Wet2H+XfQb5H3ICa969SOM44vhOh7PjHbgTp4vyygPRTsB5lljvtAY +a6MsKn+j21z7qJfIoklg0QKBgQCdm6RBFJ9PdEa7mjfrwUzTIxI3/+r2T+/rV9Qo +IiRLBylo8QtUin6e4CP6L2nNlcIrRpAgfiy1j9j2r3eQdXO4H+gEHddmAZNxWst6 +oQDcgAQLCpZj0KgBS28JSN6STDo72v56X6WAuyl3uzWdPy6YVOJO8HHH6sb151Ht +NKgJMQKBgDV4QWEXYIUDBU3EU8+rTDaJVSOXIO/XwyWKX4Lx1TT1R26IB6oROV1E +RhEKI45iMWz6NAlqniud3Zk973LKyJ2JpV3NaxqrpG7l02HWJUrKutcFNzfGYthz +QciEF7Tsr/VSSaMKDFVLgRWwDjsIzmnRaypX7O59C+ao5KiAQfFQ +-----END RSA PRIVATE KEY----- diff --git a/server/configs/tls_verify_test.conf b/server/configs/tls_verify_test.conf new file mode 100644 index 00000000000..cb7c07f6800 --- /dev/null +++ b/server/configs/tls_verify_test.conf @@ -0,0 +1,11 @@ + +# Simple TLS config file + +listen: localhost:4443 + +tls { + cert_file: "./configs/certs/cert.new.pem" + key_file: "./configs/certs/key.new.pem" + ca_file: "./configs/certs/cert.new.pem" + verify: true +} diff --git a/server/reload.go b/server/reload.go index 684f0b47fc0..0923d6f9175 100644 --- a/server/reload.go +++ b/server/reload.go @@ -51,11 +51,13 @@ type tlsOption struct { func (t *tlsOption) Apply(server *Server) { tlsRequired := t.newValue != nil server.info.TLSRequired = tlsRequired + message := "disabled" if tlsRequired { server.info.TLSVerify = (t.newValue.ClientAuth == tls.RequireAndVerifyClientCert) + message = "enabled" } server.generateServerInfoJSON() - server.Noticef("Reloaded: tls") + server.Noticef("Reloaded: tls = %s", message) } // Reload reads the current configuration file and applies any supported diff --git a/server/reload_test.go b/server/reload_test.go index 8a958dfb462..c2f916640b9 100644 --- a/server/reload_test.go +++ b/server/reload_test.go @@ -3,11 +3,14 @@ package server import ( + "fmt" "os" "path/filepath" "reflect" "testing" "time" + + "github.com/nats-io/go-nats" ) // Ensure Reload returns an error when attempting to reload a server that did @@ -210,3 +213,171 @@ func TestConfigReload(t *testing.T) { t.Fatal("Expected TLSVerify to be true") } } + +// Ensure Reload supports TLS config changes. Test this by starting a server +// with TLS enabled, connect to it to verify, reload config using a different +// key pair and client verification enabled, ensure reconnect fails, then +// ensure reconnect succeeds when the client provides a cert. +func TestConfigReloadRotateTLS(t *testing.T) { + dir, err := os.Getwd() + if err != nil { + t.Fatalf("Error getting working directory: %v", err) + } + config := filepath.Join(dir, "tmp.conf") + + if err := os.Symlink("./configs/tls_test.conf", config); err != nil { + t.Fatalf("Error creating symlink: %v", err) + } + defer os.Remove(config) + + opts, err := ProcessConfigFile(config) + if err != nil { + t.Fatalf("Error processing config file: %v", err) + } + + server := RunServer(opts) + defer server.Shutdown() + + // Ensure we can connect as a sanity check. + addr := fmt.Sprintf("nats://%s:%d", opts.Host, opts.Port) + nc, err := nats.Connect(addr, nats.Secure()) + if err != nil { + t.Fatalf("Error creating client: %v", err) + } + nc.Close() + + // Rotate cert and enable client verification. + if err := os.Remove(config); err != nil { + t.Fatalf("Error deleting symlink: %v", err) + } + if err := os.Symlink("./configs/tls_verify_test.conf", config); err != nil { + t.Fatalf("Error creating symlink: %v", err) + } + if err := server.Reload(); err != nil { + t.Fatalf("Error reloading config: %v", err) + } + + // Ensure connecting fails. + if _, err := nats.Connect(addr, nats.Secure()); err == nil { + t.Fatal("Expected connect to fail") + } + + // Ensure connecting succeeds when client presents cert. + cert := nats.ClientCert("./configs/certs/cert.new.pem", "./configs/certs/key.new.pem") + nc, err = nats.Connect(addr, cert, nats.RootCAs("./configs/certs/cert.new.pem")) + if err != nil { + t.Fatalf("Error creating client: %v", err) + } + nc.Close() +} + +// Ensure Reload supports enabling TLS. Test this by starting a server without +// TLS enabled, connect to it to verify, reload config with TLS enabled, ensure +// reconnect fails, then ensure reconnect succeeds when using secure. +func TestConfigReloadEnableTLS(t *testing.T) { + dir, err := os.Getwd() + if err != nil { + t.Fatalf("Error getting working directory: %v", err) + } + config := filepath.Join(dir, "tmp.conf") + + if err := os.Symlink("./configs/basic.conf", config); err != nil { + t.Fatalf("Error creating symlink: %v", err) + } + defer os.Remove(config) + + opts, err := ProcessConfigFile(config) + if err != nil { + t.Fatalf("Error processing config file: %v", err) + } + + server := RunServer(opts) + defer server.Shutdown() + + // Ensure we can connect as a sanity check. + addr := fmt.Sprintf("nats://%s:%d", opts.Host, opts.Port) + nc, err := nats.Connect(addr) + if err != nil { + t.Fatalf("Error creating client: %v", err) + } + nc.Close() + + // Enable TLS. + if err := os.Remove(config); err != nil { + t.Fatalf("Error deleting symlink: %v", err) + } + if err := os.Symlink("./configs/tls_test.conf", config); err != nil { + t.Fatalf("Error creating symlink: %v", err) + } + if err := server.Reload(); err != nil { + t.Fatalf("Error reloading config: %v", err) + } + + // Ensure connecting fails. + if _, err := nats.Connect(addr); err == nil { + t.Fatal("Expected connect to fail") + } + + // Ensure connecting succeeds when using secure. + nc, err = nats.Connect(addr, nats.Secure()) + if err != nil { + t.Fatalf("Error creating client: %v", err) + } + nc.Close() +} + +// Ensure Reload supports disabling TLS. Test this by starting a server with +// TLS enabled, connect to it to verify, reload config with TLS disabled, +// ensure reconnect fails, then ensure reconnect succeeds when connecting +// without secure. +func TestConfigReloadDisableTLS(t *testing.T) { + dir, err := os.Getwd() + if err != nil { + t.Fatalf("Error getting working directory: %v", err) + } + config := filepath.Join(dir, "tmp.conf") + + if err := os.Symlink("./configs/tls_test.conf", config); err != nil { + t.Fatalf("Error creating symlink: %v", err) + } + defer os.Remove(config) + + opts, err := ProcessConfigFile(config) + if err != nil { + t.Fatalf("Error processing config file: %v", err) + } + + server := RunServer(opts) + defer server.Shutdown() + + // Ensure we can connect as a sanity check. + addr := fmt.Sprintf("nats://%s:%d", opts.Host, opts.Port) + nc, err := nats.Connect(addr, nats.Secure()) + if err != nil { + t.Fatalf("Error creating client: %v", err) + } + nc.Close() + + // Disable TLS. + if err := os.Remove(config); err != nil { + t.Fatalf("Error deleting symlink: %v", err) + } + if err := os.Symlink("./configs/basic.conf", config); err != nil { + t.Fatalf("Error creating symlink: %v", err) + } + if err := server.Reload(); err != nil { + t.Fatalf("Error reloading config: %v", err) + } + + // Ensure connecting fails. + if _, err := nats.Connect(addr, nats.Secure()); err == nil { + t.Fatal("Expected connect to fail") + } + + // Ensure connecting succeeds when not using secure. + nc, err = nats.Connect(addr) + if err != nil { + t.Fatalf("Error creating client: %v", err) + } + nc.Close() +} From 30db4be9653af67df7450e70244d1be6844df5bd Mon Sep 17 00:00:00 2001 From: Tyler Treat Date: Wed, 7 Jun 2017 17:03:33 -0500 Subject: [PATCH 4/6] Ensure old connection still works in TLS reload test --- server/reload_test.go | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/server/reload_test.go b/server/reload_test.go index c2f916640b9..d2a183c96c0 100644 --- a/server/reload_test.go +++ b/server/reload_test.go @@ -244,7 +244,12 @@ func TestConfigReloadRotateTLS(t *testing.T) { if err != nil { t.Fatalf("Error creating client: %v", err) } - nc.Close() + defer nc.Close() + sub, err := nc.SubscribeSync("foo") + if err != nil { + t.Fatalf("Error subscribing: %v", err) + } + defer sub.Unsubscribe() // Rotate cert and enable client verification. if err := os.Remove(config); err != nil { @@ -264,11 +269,23 @@ func TestConfigReloadRotateTLS(t *testing.T) { // Ensure connecting succeeds when client presents cert. cert := nats.ClientCert("./configs/certs/cert.new.pem", "./configs/certs/key.new.pem") - nc, err = nats.Connect(addr, cert, nats.RootCAs("./configs/certs/cert.new.pem")) + conn, err := nats.Connect(addr, cert, nats.RootCAs("./configs/certs/cert.new.pem")) if err != nil { t.Fatalf("Error creating client: %v", err) } - nc.Close() + conn.Close() + + // Ensure the original connection can still publish/receive. + if err := nc.Publish("foo", []byte("hello")); err != nil { + t.Fatalf("Error publishing: %v", err) + } + msg, err := sub.NextMsg(time.Second) + if err != nil { + t.Fatalf("Error receiving msg: %v", err) + } + if string(msg.Data) != "hello" { + t.Fatalf("Msg is incorrect.\nexpected: %+v\ngot: %+v", []byte("hello"), msg.Data) + } } // Ensure Reload supports enabling TLS. Test this by starting a server without From 648b7367c062e1df34a3f366c87ed36e9f0b203c Mon Sep 17 00:00:00 2001 From: Tyler Treat Date: Wed, 7 Jun 2017 17:40:17 -0500 Subject: [PATCH 5/6] Fix data race --- server/parser.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/parser.go b/server/parser.go index 7811680cae3..b624e84cca0 100644 --- a/server/parser.go +++ b/server/parser.go @@ -86,7 +86,7 @@ func (c *client) parse(buf []byte) error { var b byte mcl := MAX_CONTROL_LINE_SIZE - if c.srv != nil && c.srv.opts != nil { + if c.srv != nil && c.srv.getOpts() != nil { mcl = c.srv.getOpts().MaxControlLine } From 4c33177bd381fe105f279c8d14083b9260e67c22 Mon Sep 17 00:00:00 2001 From: Tyler Treat Date: Wed, 7 Jun 2017 18:05:28 -0500 Subject: [PATCH 6/6] Bump up TLS timeout in test configs --- server/configs/tls_test.conf | 1 + server/configs/tls_verify_test.conf | 1 + server/reload.go | 3 +++ 3 files changed, 5 insertions(+) diff --git a/server/configs/tls_test.conf b/server/configs/tls_test.conf index c944f74e042..253e4249584 100644 --- a/server/configs/tls_test.conf +++ b/server/configs/tls_test.conf @@ -6,4 +6,5 @@ listen: localhost:4443 tls { cert_file: "./configs/certs/server.pem" key_file: "./configs/certs/key.pem" + timeout: 2 } diff --git a/server/configs/tls_verify_test.conf b/server/configs/tls_verify_test.conf index cb7c07f6800..205a608f9bc 100644 --- a/server/configs/tls_verify_test.conf +++ b/server/configs/tls_verify_test.conf @@ -8,4 +8,5 @@ tls { key_file: "./configs/certs/key.new.pem" ca_file: "./configs/certs/cert.new.pem" verify: true + timeout: 2 } diff --git a/server/reload.go b/server/reload.go index 0923d6f9175..f65adee3798 100644 --- a/server/reload.go +++ b/server/reload.go @@ -120,6 +120,9 @@ func (s *Server) diffOptions(newOpts *Options) ([]option, error) { diffOpts = append(diffOpts, &debugOption{newValue.(bool)}) case "tlsconfig": diffOpts = append(diffOpts, &tlsOption{newValue.(*tls.Config)}) + case "tlstimeout": + // TLSTimeout change is picked up when Options is swapped. + continue default: // Bail out if attempting to reload any unsupported options. return nil, fmt.Errorf("Config reload not supported for %s", field.Name)