From 2d181e1c273ca1d4016cda158cd258c18acb7601 Mon Sep 17 00:00:00 2001
From: Ivan Kozlovic <ivan@synadia.com>
Date: Mon, 7 Nov 2022 17:26:17 -0700
Subject: [PATCH] [FIXED] Routing: TLS connections to discovered server may
 fail

The server was not setting "server name" in the TLS configuration
for route connections, which may lead to failed (re)connect if
the certificate does not allow for the IP and the URL did not
have the hostname, which would happen with gossip protocol.

Signed-off-by: Ivan Kozlovic <ivan@synadia.com>
---
 server/reload.go      |  2 ++
 server/route.go       | 25 ++++++++++++++++++-
 server/routes_test.go | 57 +++++++++++++++++++++++++++++++++++++++++++
 server/server.go      |  1 +
 4 files changed, 84 insertions(+), 1 deletion(-)

diff --git a/server/reload.go b/server/reload.go
index aff64ccd..7058417f 100644
--- a/server/reload.go
+++ b/server/reload.go
@@ -413,7 +413,9 @@ func (r *routesOption) Apply(server *Server) {
 	}
 
 	// Add routes.
+	server.mu.Lock()
 	server.solicitRoutes(r.add)
+	server.mu.Unlock()
 
 	server.Noticef("Reloaded: cluster routes")
 }
diff --git a/server/route.go b/server/route.go
index 0ed56a48..c37bf682 100644
--- a/server/route.go
+++ b/server/route.go
@@ -1305,6 +1305,7 @@ func (s *Server) createRoute(conn net.Conn, rURL *url.URL) *client {
 	authRequired := s.routeInfo.AuthRequired
 	tlsRequired := s.routeInfo.TLSRequired
 	clusterName := s.info.Cluster
+	tlsName := s.routeTLSName
 	s.mu.Unlock()
 
 	// Grab lock
@@ -1329,8 +1330,13 @@ func (s *Server) createRoute(conn net.Conn, rURL *url.URL) *client {
 			tlsConfig = tlsConfig.Clone()
 		}
 		// Perform (server or client side) TLS handshake.
-		if _, err := c.doTLSHandshake("route", didSolicit, rURL, tlsConfig, _EMPTY_, opts.Cluster.TLSTimeout, opts.Cluster.TLSPinnedCerts); err != nil {
+		if resetTLSName, err := c.doTLSHandshake("route", didSolicit, rURL, tlsConfig, tlsName, opts.Cluster.TLSTimeout, opts.Cluster.TLSPinnedCerts); err != nil {
 			c.mu.Unlock()
+			if resetTLSName {
+				s.mu.Lock()
+				s.routeTLSName = _EMPTY_
+				s.mu.Unlock()
+			}
 			return nil
 		}
 	}
@@ -1897,7 +1903,24 @@ func (c *client) isSolicitedRoute() bool {
 	return c.kind == ROUTER && c.route != nil && c.route.didSolicit
 }
 
+// Save the first hostname found in route URLs. This will be used in gossip mode
+// when trying to create a TLS connection by setting the tlsConfig.ServerName.
+// Lock is held on entry
+func (s *Server) saveRouteTLSName() {
+	var tlsName string
+	for _, u := range s.getOpts().Routes {
+		if tlsName == _EMPTY_ && net.ParseIP(u.Hostname()) == nil {
+			tlsName = u.Hostname()
+		}
+	}
+	s.routeTLSName = tlsName
+}
+
+// Start connection process to provided routes. Each route connection will
+// be started in a dedicated go routine.
+// Lock is held on entry
 func (s *Server) solicitRoutes(routes []*url.URL) {
+	s.saveRouteTLSName()
 	for _, r := range routes {
 		route := r
 		s.startGoRoutine(func() { s.connectToRoute(route, true, true) })
diff --git a/server/routes_test.go b/server/routes_test.go
index ed3405ed..1c9c205e 100644
--- a/server/routes_test.go
+++ b/server/routes_test.go
@@ -1670,3 +1670,60 @@ func TestRouteSolicitedReconnectsEvenIfImplicit(t *testing.T) {
 		// OK
 	}
 }
+
+func TestRouteSaveTLSName(t *testing.T) {
+	c1Conf := createConfFile(t, []byte(`
+		port: -1
+		cluster {
+			name: "abc"
+			port: -1
+			tls {
+				cert_file: '../test/configs/certs/server-noip.pem'
+				key_file: '../test/configs/certs/server-key-noip.pem'
+				ca_file: '../test/configs/certs/ca.pem'
+			}
+		}
+	`))
+	defer removeFile(t, c1Conf)
+	s1, o1 := RunServerWithConfig(c1Conf)
+	defer s1.Shutdown()
+
+	c2And3Conf := createConfFile(t, []byte(fmt.Sprintf(`
+		port: -1
+		cluster {
+			name: "abc"
+			port: -1
+			routes: ["nats://localhost:%d"]
+			tls {
+				cert_file: '../test/configs/certs/server-noip.pem'
+				key_file: '../test/configs/certs/server-key-noip.pem'
+				ca_file: '../test/configs/certs/ca.pem'
+			}
+		}
+	`, o1.Cluster.Port)))
+	defer removeFile(t, c2And3Conf)
+	s2, _ := RunServerWithConfig(c2And3Conf)
+	defer s2.Shutdown()
+
+	checkClusterFormed(t, s1, s2)
+
+	s3, _ := RunServerWithConfig(c2And3Conf)
+	defer s3.Shutdown()
+
+	checkClusterFormed(t, s1, s2, s3)
+
+	// Do a reload of s2 and close the route connections and make sure it
+	// reconnects properly.
+	err := s2.Reload()
+	require_NoError(t, err)
+
+	s2.mu.RLock()
+	for _, r := range s2.routes {
+		r.mu.Lock()
+		r.nc.Close()
+		r.mu.Unlock()
+	}
+	s2.mu.RUnlock()
+
+	checkClusterFormed(t, s1, s2, s3)
+}
diff --git a/server/server.go b/server/server.go
index 607958a9..c6e18980 100644
--- a/server/server.go
+++ b/server/server.go
@@ -152,6 +152,7 @@ type Server struct {
 	routeInfoJSON       []byte
 	routeResolver       netResolver
 	routesToSelf        map[string]struct{}
+	routeTLSName        string
 	leafNodeListener    net.Listener
 	leafNodeListenerErr error
 	leafNodeInfo        Info