Merge pull request #1264 from nats-io/fix_tlssavename

[FIXED] Reset of tlsName only for x509.HostnameError

server/gateway.go

@@ -17,6 +17,7 @@ import (
 	"bytes"
 	"crypto/sha256"
 	"crypto/tls"
+	"crypto/x509"
 	"encoding/json"
 	"fmt"
 	"math/rand"
@@ -744,20 +745,21 @@ func (s *Server) createGateway(cfg *gatewayCfg, url *url.URL, conn net.Conn) {
 
 	// Check for TLS
 	if tlsRequired {
+		var host string
 		var timeout float64
 		// If we solicited, we will act like the client, otherwise the server.
 		if solicit {
 			c.Debugf("Starting TLS gateway client handshake")
 			cfg.RLock()
 			tlsName := cfg.tlsName
-			tlsConfig := cfg.TLSConfig
+			tlsConfig := cfg.TLSConfig.Clone()
 			timeout = cfg.TLSTimeout
 			cfg.RUnlock()
 			if tlsConfig.ServerName == "" {
 				// If the given url is a hostname, use this hostname for the
 				// ServerName. If it is an IP, use the cfg's tlsName. If none
 				// is available, resort to current IP.
-				host := url.Hostname()
+				host = url.Hostname()
 				if tlsName != "" && net.ParseIP(host) != nil {
 					host = tlsName
 				}
@@ -779,6 +781,17 @@ func (s *Server) createGateway(cfg *gatewayCfg, url *url.URL, conn net.Conn) {
 
 		c.mu.Unlock()
 		if err := conn.Handshake(); err != nil {
+			if solicit {
+				// Based on type of error, possibly clear the saved tlsName
+				// See: https://github.com/nats-io/nats-server/issues/1256
+				if _, ok := err.(x509.HostnameError); ok {
+					cfg.Lock()
+					if host == cfg.tlsName {
+						cfg.tlsName = ""
+					}
+					cfg.Unlock()
+				}
+			}
 			c.Errorf("TLS gateway handshake error: %v", err)
 			c.sendErr("Secure Connection - TLS Required")
 			c.closeConnection(TLSHandshakeError)

server/leafnode.go

@@ -17,6 +17,7 @@ import (
 	"bufio"
 	"bytes"
 	"crypto/tls"
+	"crypto/x509"
 	"encoding/base64"
 	"encoding/json"
 	"fmt"
@@ -638,30 +639,30 @@ func (s *Server) createLeafNode(conn net.Conn, remote *leafNodeCfg) *client {
 		}
 
 		// Do TLS here as needed.
-		tlsRequired := c.leaf.remote.TLS || c.leaf.remote.TLSConfig != nil
+		tlsRequired := remote.TLS || remote.TLSConfig != nil
 		if tlsRequired {
 			c.Debugf("Starting TLS leafnode client handshake")
 			// Specify the ServerName we are expecting.
 			var tlsConfig *tls.Config
-			if c.leaf.remote.TLSConfig != nil {
-				tlsConfig = c.leaf.remote.TLSConfig.Clone()
+			if remote.TLSConfig != nil {
+				tlsConfig = remote.TLSConfig.Clone()
 			} else {
 				tlsConfig = &tls.Config{MinVersion: tls.VersionTLS12}
 			}
 
-			url := c.leaf.remote.getCurrentURL()
-			host, _, _ := net.SplitHostPort(url.Host)
-			// We need to check if this host is an IP. If so, we probably
-			// had this advertised to us and should use the configured host
-			// name for the TLS server name.
-			if net.ParseIP(host) != nil {
-				if c.leaf.remote.tlsName != "" {
-					host = c.leaf.remote.tlsName
-				} else {
-					host, _, _ = net.SplitHostPort(c.leaf.remote.curURL.Host)
+			var host string
+			// If ServerName was given to us from the option, use that, always.
+			if tlsConfig.ServerName == "" {
+				url := remote.getCurrentURL()
+				host = url.Hostname()
+				// We need to check if this host is an IP. If so, we probably
+				// had this advertised to us and should use the configured host
+				// name for the TLS server name.
+				if remote.tlsName != "" && net.ParseIP(host) != nil {
+					host = remote.tlsName
 				}
+				tlsConfig.ServerName = host
 			}
-			tlsConfig.ServerName = host
 
 			c.nc = tls.Client(c.nc, tlsConfig)
 
@@ -669,10 +670,10 @@ func (s *Server) createLeafNode(conn net.Conn, remote *leafNodeCfg) *client {
 
 			// Setup the timeout
 			var wait time.Duration
-			if c.leaf.remote.TLSTimeout == 0 {
+			if remote.TLSTimeout == 0 {
 				wait = TLS_TIMEOUT
 			} else {
-				wait = secondsToDuration(c.leaf.remote.TLSTimeout)
+				wait = secondsToDuration(remote.TLSTimeout)
 			}
 			time.AfterFunc(wait, func() { tlsTimeout(c, conn) })
 			conn.SetReadDeadline(time.Now().Add(wait))
@@ -680,17 +681,21 @@ func (s *Server) createLeafNode(conn net.Conn, remote *leafNodeCfg) *client {
 			// Force handshake
 			c.mu.Unlock()
 			if err = conn.Handshake(); err != nil {
+				if solicited {
+					// If we overrode and used the saved tlsName but that failed
+					// we will clear that here. This is for the case that another server
+					// does not have the same tlsName, maybe only IPs.
+					// https://github.com/nats-io/nats-server/issues/1256
+					if _, ok := err.(x509.HostnameError); ok {
+						remote.Lock()
+						if host == remote.tlsName {
+							remote.tlsName = ""
+						}
+						remote.Unlock()
+					}
+				}
 				c.Errorf("TLS handshake error: %v", err)
 				c.closeConnection(TLSHandshakeError)
-				// If we overrode and used the saved tlsName but that failed
-				// we will clear that here. This is for the case that another server
-				// does not have the same tlsName, maybe only IPs.
-				// https://github.com/nats-io/nats-server/issues/1256
-				c.mu.Lock()
-				if host == c.leaf.remote.tlsName {
-					c.leaf.remote.tlsName = ""
-				}
-				c.mu.Unlock()
 				return nil
 			}
 			// Reset the read deadline

test/gateway_test.go

@@ -16,8 +16,10 @@ package test
 import (
 	"bufio"
 	"bytes"
+	"crypto/tls"
 	"fmt"
 	"net"
+	"net/url"
 	"regexp"
 	"testing"
 	"time"
@@ -571,3 +573,92 @@ func TestGatewaySystemConnectionAllowedToPublishOnGWPrefix(t *testing.T) {
 		}
 	}
 }
+
+func TestGatewayTLSMixedIPAndDNS(t *testing.T) {
+	server.SetGatewaysSolicitDelay(5 * time.Millisecond)
+	defer server.ResetGatewaysSolicitDelay()
+
+	confA1 := createConfFile(t, []byte(`
+		listen: 127.0.0.1:-1
+		gateway {
+			name: "A"
+			listen: "127.0.0.1:-1"
+			tls {
+				cert_file: "./configs/certs/server-iponly.pem"
+				key_file:  "./configs/certs/server-key-iponly.pem"
+				ca_file:   "./configs/certs/ca.pem"
+				timeout: 2
+			}
+		}
+		cluster {
+			listen: "127.0.0.1:-1"
+		}
+	`))
+	srvA1, optsA1 := RunServerWithConfig(confA1)
+	defer srvA1.Shutdown()
+
+	confA2Template := `
+		listen: 127.0.0.1:-1
+		gateway {
+			name: "A"
+			listen: "localhost:-1"
+			tls {
+				cert_file: "./configs/certs/server-cert.pem"
+				key_file:  "./configs/certs/server-key.pem"
+				ca_file:   "./configs/certs/ca.pem"
+				timeout: 2
+			}
+		}
+		cluster {
+			listen: "127.0.0.1:-1"
+			routes [
+				"nats://%s:%d"
+			]
+		}
+	`
+	confA2 := createConfFile(t, []byte(fmt.Sprintf(confA2Template,
+		optsA1.Cluster.Host, optsA1.Cluster.Port)))
+	srvA2, optsA2 := RunServerWithConfig(confA2)
+	defer srvA2.Shutdown()
+
+	checkClusterFormed(t, srvA1, srvA2)
+
+	// Create a GW connection to cluster "A". Don't use the helper since we need verification etc.
+	o := DefaultTestOptions
+	o.Port = -1
+	o.Gateway.Name = "B"
+	o.Gateway.Host = "127.0.0.1"
+	o.Gateway.Port = -1
+
+	tc := &server.TLSConfigOpts{}
+	tc.CertFile = "./configs/certs/server-cert.pem"
+	tc.KeyFile = "./configs/certs/server-key.pem"
+	tc.CaFile = "./configs/certs/ca.pem"
+	tc.Timeout = 2.0
+	tlsConfig, err := server.GenTLSConfig(tc)
+	if err != nil {
+		t.Fatalf("Error generating TLS config: %v", err)
+	}
+	tlsConfig.ClientAuth = tls.RequireAndVerifyClientCert
+	tlsConfig.RootCAs = tlsConfig.ClientCAs
+
+	o.Gateway.TLSConfig = tlsConfig.Clone()
+
+	rurl, _ := url.Parse(fmt.Sprintf("nats://%s:%d", optsA2.Gateway.Host, optsA2.Gateway.Port))
+	remote := &server.RemoteGatewayOpts{Name: "A", URLs: []*url.URL{rurl}}
+	remote.TLSConfig = tlsConfig.Clone()
+	o.Gateway.Gateways = []*server.RemoteGatewayOpts{remote}
+
+	srvB := RunServer(&o)
+	defer srvB.Shutdown()
+
+	waitForOutboundGateways(t, srvB, 1, 10*time.Second)
+	waitForOutboundGateways(t, srvA1, 1, 10*time.Second)
+	waitForOutboundGateways(t, srvA2, 1, 10*time.Second)
+
+	// Now kill off srvA2 and force serverB to connect to srvA1.
+	srvA2.Shutdown()
+
+	// Make sure this works.
+	waitForOutboundGateways(t, srvB, 1, 10*time.Second)
+}

test/leafnode_test.go

@@ -3082,8 +3082,6 @@ func TestClusterTLSMixedIPAndDNS(t *testing.T) {
 		t.Fatalf("Failed to parse root certificate from %q", "./configs/certs/ca.pem")
 	}
 	remote.TLSConfig.RootCAs = pool
-	host, _, _ := net.SplitHostPort(optsB.LeafNode.Host)
-	remote.TLSConfig.ServerName = host
 	o.LeafNode.Remotes = []*server.RemoteLeafOpts{remote}
 	sl, _ := RunServer(&o), &o
 	defer sl.Shutdown()