[FIXED] Gateway: TLS configuration reload now applies to implicit remotes (#6886)

When a config reload is issued and a server has implicit remote gateways
(that is, gateways that were gossiped and not explicitly configured),
the TLS changes were applied only to explicit remotes.

With this change, the changes to the main `tls{}` block will be applied
to implicit remotes.

Credit to @song0071000 for the proposed changes in the code.

Resolves #6875

Signed-off-by: Ivan Kozlovic <ivan@synadia.com>

Author: neilalexander

server/gateway.go

@@ -424,38 +424,48 @@ func (s *Server) newGateway(opts *Options) error {
 func (g *srvGateway) updateRemotesTLSConfig(opts *Options) {
 	g.Lock()
 	defer g.Unlock()
-
-	for _, ro := range opts.Gateway.Gateways {
-		if ro.Name == g.name {
+	// Instead of going over opts.Gateway.Gateways, which would include only
+	// explicit remotes, we are going to go through g.remotes.
+	for name, cfg := range g.remotes {
+		if name == g.name {
 			continue
 		}
-		if cfg, ok := g.remotes[ro.Name]; ok {
-			cfg.Lock()
-			// If TLS config is in remote, use that one, otherwise,
-			// use the TLS config from the main block.
-			if ro.TLSConfig != nil {
-				cfg.TLSConfig = ro.TLSConfig.Clone()
-			} else if opts.Gateway.TLSConfig != nil {
-				cfg.TLSConfig = opts.Gateway.TLSConfig.Clone()
+		var ro *RemoteGatewayOpts
+		// We now need to go back and find the RemoteGatewayOpts but only if
+		// this remote is explicit (otherwise it won't be found).
+		if !cfg.isImplicit() {
+			for _, r := range opts.Gateway.Gateways {
+				if r.Name == name {
+					ro = r
+					break
+				}
 			}
-
-			// Ensure that OCSP callbacks are always setup after a reload if needed.
-			mustStaple := opts.OCSPConfig != nil && opts.OCSPConfig.Mode == OCSPModeAlways
-			if mustStaple && opts.Gateway.TLSConfig != nil {
-				clientCB := opts.Gateway.TLSConfig.GetClientCertificate
-				verifyCB := opts.Gateway.TLSConfig.VerifyConnection
-				if mustStaple && cfg.TLSConfig != nil {
-					if clientCB != nil && cfg.TLSConfig.GetClientCertificate == nil {
-						cfg.TLSConfig.GetClientCertificate = clientCB
-					}
-					if verifyCB != nil && cfg.TLSConfig.VerifyConnection == nil {
-						cfg.TLSConfig.VerifyConnection = verifyCB
-					}
+		}
+		cfg.Lock()
+		// If we have an `ro` (that means an explicitly defined remote gateway)
+		// and it has an explicit TLS config, use that one, otherwise (no explicit
+		// TLS config in the remote, or implicit remote), use the TLS config from
+		// the main block.
+		if ro != nil && ro.TLSConfig != nil {
+			cfg.TLSConfig = ro.TLSConfig.Clone()
+		} else if opts.Gateway.TLSConfig != nil {
+			cfg.TLSConfig = opts.Gateway.TLSConfig.Clone()
+		}
+		// Ensure that OCSP callbacks are always setup after a reload if needed.
+		mustStaple := opts.OCSPConfig != nil && opts.OCSPConfig.Mode == OCSPModeAlways
+		if mustStaple && opts.Gateway.TLSConfig != nil {
+			clientCB := opts.Gateway.TLSConfig.GetClientCertificate
+			verifyCB := opts.Gateway.TLSConfig.VerifyConnection
+			if mustStaple && cfg.TLSConfig != nil {
+				if clientCB != nil && cfg.TLSConfig.GetClientCertificate == nil {
+					cfg.TLSConfig.GetClientCertificate = clientCB
+				}
+				if verifyCB != nil && cfg.TLSConfig.VerifyConnection == nil {
+					cfg.TLSConfig.VerifyConnection = verifyCB
 				}
 			}
-
-			cfg.Unlock()
 		}
+		cfg.Unlock()
 	}
 }
 

server/gateway_test.go

@@ -6458,6 +6458,65 @@ func TestGatewayTLSConfigReloadForRemote(t *testing.T) {
 	waitForOutboundGateways(t, srvB, 1, time.Second)
 }
 
+func TestGatewayTLSConfigReloadForImplicitRemote(t *testing.T) {
+	SetGatewaysSolicitDelay(5 * time.Millisecond)
+	defer ResetGatewaysSolicitDelay()
+
+	template := `
+		listen: 127.0.0.1:-1
+		gateway {
+			name: "A"
+			listen: "127.0.0.1:-1"
+			tls {
+				cert_file: "../test/configs/certs/srva-cert.pem"
+				key_file:  "../test/configs/certs/srva-key.pem"
+				%s
+				verify: true
+			}
+		}
+	`
+	confA := createConfFile(t, fmt.Appendf(nil, template, `ca_file:   "../test/configs/certs/ca.pem"`))
+	srvA, optsA := RunServerWithConfig(confA)
+	defer srvA.Shutdown()
+
+	optsB := testGatewayOptionsFromToWithTLS(t, "B", "A", []string{fmt.Sprintf("nats://127.0.0.1:%d", optsA.Gateway.Port)})
+	srvB := runGatewayServer(optsB)
+	defer srvB.Shutdown()
+
+	waitForInboundGateways(t, srvA, 1, time.Second)
+	waitForOutboundGateways(t, srvA, 1, time.Second)
+	waitForInboundGateways(t, srvB, 1, time.Second)
+	waitForOutboundGateways(t, srvB, 1, time.Second)
+
+	// We will verify that the config reload of the tls{} block is applied to
+	// the implicit remote (from A to B) by removing the ca_file.
+	reloadUpdateConfig(t, srvA, confA, fmt.Sprintf(template, ""))
+
+	// Get the remote from A to B
+	cfg := srvA.getRemoteGateway("B")
+	require_NotNil(t, cfg)
+	cfg.Lock()
+	tc := cfg.TLSConfig
+	cfg.Unlock()
+	require_NotNil(t, tc)
+	// The CA should have been removed.
+	require_True(t, tc.ClientCAs == nil)
+
+	// Reset the connection attempts, since we are going to close the connection
+	// from A to B and make sure that connection keeps failing.
+	cfg.resetConnAttempts()
+
+	// Get the outbound connection and close it.
+	c := srvA.getOutboundGatewayConnection("B")
+	require_NotNil(t, c)
+	c.mu.Lock()
+	c.nc.Close()
+	c.mu.Unlock()
+
+	// Verify that we fail to connect from A to B now.
+	waitForGatewayFailedConnect(t, srvA, "B", true, time.Second)
+}
+
 func TestGatewayAuthDiscovered(t *testing.T) {
 	SetGatewaysSolicitDelay(5 * time.Millisecond)
 	defer ResetGatewaysSolicitDelay()