From 40c0f0315307328894e3040c8fe6678afe7ce5cd Mon Sep 17 00:00:00 2001 From: Ivan Kozlovic Date: Tue, 30 Nov 2021 10:12:54 -0700 Subject: [PATCH] [FIXED] Monitoring: tls configuration not updated on reload When creating the http server, we need to provide a TLS configuration. After a config reload, the new TLS config would not be reflected. We had the same issue with Websocket and was fixed with the use of tls.Config.GetConfigForClient API, which makes the TLS handshake to ask for a TLS config. That fix for websocket was simply not applied to the HTTPs monitoring case. I have also fixed some flappers due to the use of localhost instead of 127.0.0.1 (connections possibly would resolve to some IPv6 address that the server would not accept, etc..) Signed-off-by: Ivan Kozlovic --- server/events_test.go | 2 +- server/jetstream_cluster_test.go | 10 +++--- server/leafnode_test.go | 12 +++---- server/monitor_test.go | 55 ++++++++++++++++++++++++++++++++ server/server.go | 33 +++++++++++++++++++ server/websocket.go | 20 +----------- server/websocket_test.go | 4 +++ 7 files changed, 105 insertions(+), 31 deletions(-) diff --git a/server/events_test.go b/server/events_test.go index 384d284e..ff0855bd 100644 --- a/server/events_test.go +++ b/server/events_test.go @@ -2257,7 +2257,7 @@ func TestServerEventsFilteredByTag(t *testing.T) { listen: -1 no_advertise: true routes [ - nats-route://localhost:%d + nats-route://127.0.0.1:%d ] } system_account: SYS diff --git a/server/jetstream_cluster_test.go b/server/jetstream_cluster_test.go index 2777aeb1..d945d29f 100644 --- a/server/jetstream_cluster_test.go +++ b/server/jetstream_cluster_test.go @@ -8732,7 +8732,7 @@ func TestJetStreamClusterMirrorAndSourceCrossNonNeighboringDomain(t *testing.T) system_account = SYS no_auth_user: a1 leafnodes: { - listen: localhost:-1 + listen: 127.0.0.1:-1 } `, storeDir1))) s1, _ := RunServerWithConfig(conf1) @@ -8748,8 +8748,8 @@ func TestJetStreamClusterMirrorAndSourceCrossNonNeighboringDomain(t *testing.T) system_account = SYS no_auth_user: a1 leafnodes:{ - remotes:[{ url:nats://a1:a1@localhost:%d, account: A}, - { url:nats://s1:s1@localhost:%d, account: SYS}] + remotes:[{ url:nats://a1:a1@127.0.0.1:%d, account: A}, + { url:nats://s1:s1@127.0.0.1:%d, account: SYS}] } `, storeDir2, s1.opts.LeafNode.Port, s1.opts.LeafNode.Port))) s2, _ := RunServerWithConfig(conf2) @@ -8765,8 +8765,8 @@ func TestJetStreamClusterMirrorAndSourceCrossNonNeighboringDomain(t *testing.T) system_account = SYS no_auth_user: a1 leafnodes:{ - remotes:[{ url:nats://a1:a1@localhost:%d, account: A}, - { url:nats://s1:s1@localhost:%d, account: SYS}] + remotes:[{ url:nats://a1:a1@127.0.0.1:%d, account: A}, + { url:nats://s1:s1@127.0.0.1:%d, account: SYS}] } `, storeDir3, s1.opts.LeafNode.Port, s1.opts.LeafNode.Port))) s3, _ := RunServerWithConfig(conf3) diff --git a/server/leafnode_test.go b/server/leafnode_test.go index feb6dc5d..e7327699 100644 --- a/server/leafnode_test.go +++ b/server/leafnode_test.go @@ -3911,9 +3911,9 @@ resolver_preload: { %s: %s %s: %s } -listen: localhost:-1 +listen: 127.0.0.1:-1 leafnodes: { - listen: localhost:-1 + listen: 127.0.0.1:-1 } jetstream :{ domain: "cluster" @@ -3924,7 +3924,7 @@ jetstream :{ ` tmplL := ` -listen: localhost:-1 +listen: 127.0.0.1:-1 accounts :{ A:{ jetstream: enable, users:[ {user:a1,password:a1}]}, SYS:{ users:[ {user:s1,password:s1}]}, @@ -3937,8 +3937,8 @@ jetstream: { max_file: 50Mb } leafnodes:{ - remotes:[{ url:nats://localhost:%d, account: A, credentials: %s}, - { url:nats://localhost:%d, account: SYS, credentials: %s}] + remotes:[{ url:nats://127.0.0.1:%d, account: A, credentials: %s}, + { url:nats://127.0.0.1:%d, account: SYS, credentials: %s}] } ` @@ -3961,7 +3961,7 @@ leafnodes:{ ncA := natsConnect(t, sA.ClientURL(), nats.UserCredentials(unlimitedCreds)) defer ncA.Close() - ncL := natsConnect(t, fmt.Sprintf("nats://a1:a1@localhost:%d", sL.opts.Port)) + ncL := natsConnect(t, fmt.Sprintf("nats://a1:a1@127.0.0.1:%d", sL.opts.Port)) defer ncL.Close() test := func(subject string, cSub, cPub *nats.Conn, remoteServerForSub *Server, accName string, pass bool) { diff --git a/server/monitor_test.go b/server/monitor_test.go index ac63eeca..9b0fb508 100644 --- a/server/monitor_test.go +++ b/server/monitor_test.go @@ -4310,3 +4310,58 @@ func TestMonitorJszAccountReserves(t *testing.T) { reloadUpdateConfig(t, s, conf, fmt.Sprintf(tmplCfg, tmpDir, "jetstream: enabled")) test(4, 0, 0, false) } + +func TestMonitorReloadTLSConfig(t *testing.T) { + template := ` + listen: "127.0.0.1:-1" + https: "127.0.0.1:-1" + tls { + cert_file: '%s' + key_file: '%s' + ca_file: '../test/configs/certs/ca.pem' + } + ` + conf := createConfFile(t, []byte(fmt.Sprintf(template, + "../test/configs/certs/server-noip.pem", + "../test/configs/certs/server-key-noip.pem"))) + defer removeFile(t, conf) + + s, _ := RunServerWithConfig(conf) + defer s.Shutdown() + + addr := fmt.Sprintf("127.0.0.1:%d", s.MonitorAddr().Port) + c, err := net.Dial("tcp", addr) + if err != nil { + t.Fatalf("Error creating ws connection: %v", err) + } + defer c.Close() + + tc := &TLSConfigOpts{CaFile: "../test/configs/certs/ca.pem"} + tlsConfig, err := GenTLSConfig(tc) + if err != nil { + t.Fatalf("Error generating TLS config: %v", err) + } + tlsConfig.ServerName = "127.0.0.1" + tlsConfig.RootCAs = tlsConfig.ClientCAs + tlsConfig.ClientCAs = nil + c = tls.Client(c, tlsConfig.Clone()) + if err := c.(*tls.Conn).Handshake(); err == nil || !strings.Contains(err.Error(), "SAN") { + t.Fatalf("Unexpected error: %v", err) + } + c.Close() + + reloadUpdateConfig(t, s, conf, fmt.Sprintf(template, + "../test/configs/certs/server-cert.pem", + "../test/configs/certs/server-key.pem")) + + c, err = net.Dial("tcp", addr) + if err != nil { + t.Fatalf("Error creating ws connection: %v", err) + } + defer c.Close() + + c = tls.Client(c, tlsConfig.Clone()) + if err := c.(*tls.Conn).Handshake(); err != nil { + t.Fatalf("Error on TLS handshake: %v", err) + } +} diff --git a/server/server.go b/server/server.go index 51df6842..54ad808c 100644 --- a/server/server.go +++ b/server/server.go @@ -23,6 +23,7 @@ import ( "fmt" "io" "io/ioutil" + "log" "math/rand" "net" "net/http" @@ -2155,6 +2156,36 @@ func (s *Server) basePath(p string) string { return path.Join(s.httpBasePath, p) } +type captureHTTPServerLog struct { + s *Server + prefix string +} + +func (cl *captureHTTPServerLog) Write(p []byte) (int, error) { + var buf [128]byte + var b = buf[:0] + + b = append(b, []byte(cl.prefix)...) + offset := 0 + if bytes.HasPrefix(p, []byte("http:")) { + offset = 6 + } + b = append(b, p[offset:]...) + cl.s.Errorf(string(b)) + return len(p), nil +} + +// The TLS configuration is passed to the listener when the monitoring +// "server" is setup. That prevents TLS configuration updates on reload +// from being used. By setting this function in tls.Config.GetConfigForClient +// we instruct the TLS handshake to ask for the tls configuration to be +// used for a specific client. We don't care which client, we always use +// the same TLS configuration. +func (s *Server) getTLSConfig(_ *tls.ClientHelloInfo) (*tls.Config, error) { + opts := s.getOpts() + return opts.TLSConfig, nil +} + // Start the monitoring server func (s *Server) startMonitoring(secure bool) error { // Snapshot server options. @@ -2177,6 +2208,7 @@ func (s *Server) startMonitoring(secure bool) error { } hp = net.JoinHostPort(opts.HTTPHost, strconv.Itoa(port)) config := opts.TLSConfig.Clone() + config.GetConfigForClient = s.getTLSConfig config.ClientAuth = tls.NoClientCert httpListener, err = tls.Listen("tcp", hp, config) @@ -2227,6 +2259,7 @@ func (s *Server) startMonitoring(secure bool) error { Addr: hp, Handler: mux, MaxHeaderBytes: 1 << 20, + ErrorLog: log.New(&captureHTTPServerLog{s, "monitoring: "}, _EMPTY_, 0), } s.mu.Lock() if s.shutdown { diff --git a/server/websocket.go b/server/websocket.go index b16edf0a..3dbcd554 100644 --- a/server/websocket.go +++ b/server/websocket.go @@ -1112,7 +1112,7 @@ func (s *Server) startWebsocketServer() { Addr: hp, Handler: mux, ReadTimeout: o.HandshakeTimeout, - ErrorLog: log.New(&wsCaptureHTTPServerLog{s}, _EMPTY_, 0), + ErrorLog: log.New(&captureHTTPServerLog{s, "websocket: "}, _EMPTY_, 0), } s.websocket.server = hs s.websocket.listener = hl @@ -1239,24 +1239,6 @@ func (s *Server) createWSClient(conn net.Conn, ws *websocket) *client { return c } -type wsCaptureHTTPServerLog struct { - s *Server -} - -func (cl *wsCaptureHTTPServerLog) Write(p []byte) (int, error) { - var buf [128]byte - var b = buf[:0] - - copy(b, []byte("websocket :")) - offset := 0 - if bytes.HasPrefix(p, []byte("http:")) { - offset = 6 - } - b = append(b, p[offset:]...) - cl.s.Errorf(string(b)) - return len(p), nil -} - func (c *client) wsCollapsePtoNB() (net.Buffers, int64) { var nb net.Buffers var mfs int diff --git a/server/websocket_test.go b/server/websocket_test.go index 8c0c2d04..82eeebb1 100644 --- a/server/websocket_test.go +++ b/server/websocket_test.go @@ -2366,6 +2366,10 @@ func TestWSHandshakeTimeout(t *testing.T) { // Check that server logs error select { case e := <-logger.errCh: + // Check that log starts with "websocket: " + if !strings.HasPrefix(e, "websocket: ") { + t.Fatalf("Wrong log line start: %s", e) + } if !strings.Contains(e, "timeout") { t.Fatalf("Unexpected error: %v", e) }