Merge pull request #1073 from nats-io/add_warn_for_leafnode_tls_insecure

Add warning for TLS insecure setting on LeafNodes

server/gateway.go

@@ -431,20 +431,21 @@ func (s *Server) gatewayAcceptLoop(ch chan struct{}) {
 	}
 	// Setup state that can enable shutdown
 	s.gatewayListener = l
-	// Warn if insecure is configured in gateway{} or any remoteGateway{}
-	if tlsReq {
-		warn := opts.Gateway.TLSConfig.InsecureSkipVerify
-		if !warn {
-			for _, g := range opts.Gateway.Gateways {
-				if g.TLSConfig != nil && g.TLSConfig.InsecureSkipVerify {
-					warn = true
-					break
-				}
+
+	// Warn if insecure is configured in the main Gateway configuration
+	// or any of the RemoteGateway's. This means that we need to check
+	// remotes even if TLS would not be configured for the accept.
+	warn := tlsReq && opts.Gateway.TLSConfig.InsecureSkipVerify
+	if !warn {
+		for _, g := range opts.Gateway.Gateways {
+			if g.TLSConfig != nil && g.TLSConfig.InsecureSkipVerify {
+				warn = true
+				break
 			}
 		}
-		if warn {
-			s.Warnf(gatewayTLSInsecureWarning)
-		}
+	}
+	if warn {
+		s.Warnf(gatewayTLSInsecureWarning)
 	}
 	s.mu.Unlock()
 

server/leafnode.go

@@ -36,6 +36,9 @@ import (
 	"github.com/nats-io/nkeys"
 )
 
+// Warning when user configures leafnode TLS insecure
+const leafnodeTLSInsecureWarning = "TLS certificate chain and hostname of solicited leafnodes will not be verified. DO NOT USE IN PRODUCTION!"
+
 type leaf struct {
 	// Used to suppress sub and unsub interest. Same as routes but our audience
 	// here is tied to this leaf node. This will hold all subscriptions except this
@@ -293,6 +296,25 @@ func (s *Server) leafNodeAcceptLoop(ch chan struct{}) {
 
 	// Setup state that can enable shutdown
 	s.leafNodeListener = l
+
+	// As of now, a server that does not have remotes configured would
+	// never solicit a connection, so we should not have to warn if
+	// InsecureSkipVerify is set in main LeafNodes config (since
+	// this TLS setting matters only when soliciting a connection).
+	// Still, warn if insecure is set in any of LeafNode block.
+	// We need to check remotes, even if tls is not required on accept.
+	warn := tlsRequired && opts.LeafNode.TLSConfig.InsecureSkipVerify
+	if !warn {
+		for _, r := range opts.LeafNode.Remotes {
+			if r.TLSConfig != nil && r.TLSConfig.InsecureSkipVerify {
+				warn = true
+				break
+			}
+		}
+	}
+	if warn {
+		s.Warnf(leafnodeTLSInsecureWarning)
+	}
 	s.mu.Unlock()
 
 	// Let them know we are up

server/server_test.go

@@ -1089,7 +1089,7 @@ func TestClientWriteLoopStall(t *testing.T) {
 }
 
 func TestInsecureSkipVerifyWarning(t *testing.T) {
-	checkServerFails := func(t *testing.T, o *Options, expectedWarn string) {
+	checkWarnReported := func(t *testing.T, o *Options, expectedWarn string) {
 		t.Helper()
 		s, err := NewServer(o)
 		if err != nil {
@@ -1109,7 +1109,7 @@ func TestInsecureSkipVerifyWarning(t *testing.T) {
 				t.Fatalf("Expected warning %q, got %q", expectedWarn, w)
 			}
 		case <-time.After(2 * time.Second):
-			t.Fatalf("Did not get any warning")
+			t.Fatalf("Did not get warning %q", expectedWarn)
 		}
 		s.Shutdown()
 		wg.Wait()
@@ -1120,43 +1120,63 @@ func TestInsecureSkipVerifyWarning(t *testing.T) {
 	tc.KeyFile = "../test/configs/certs/server-key.pem"
 	tc.CaFile = "../test/configs/certs/ca.pem"
 	tc.Insecure = true
-
-	o := DefaultOptions()
 	config, err := GenTLSConfig(tc)
 	if err != nil {
 		t.Fatalf("Error generating tls config: %v", err)
 	}
-	o.Cluster.Port = -1
-	o.Cluster.TLSConfig = config
-	checkServerFails(t, o, clusterTLSInsecureWarning)
 
-	// Get a clone that we will use for the Gateway TLS setting
-	gwConfig := config.Clone()
+	o := DefaultOptions()
+	o.Cluster.Port = -1
+	o.Cluster.TLSConfig = config.Clone()
+	checkWarnReported(t, o, clusterTLSInsecureWarning)
 
 	// Remove the route setting
 	o.Cluster.Port = 0
 	o.Cluster.TLSConfig = nil
-	// Configure GW
+
+	// Configure LeafNode with no TLS in the main block first, but only with remotes.
+	o.LeafNode.Port = -1
+	rurl, _ := url.Parse("nats://127.0.0.1:1234")
+	o.LeafNode.Remotes = []*RemoteLeafOpts{
+		{
+			URLs:      []*url.URL{rurl},
+			TLSConfig: config.Clone(),
+		},
+	}
+	checkWarnReported(t, o, leafnodeTLSInsecureWarning)
+
+	// Now add to main block.
+	o.LeafNode.TLSConfig = config.Clone()
+	checkWarnReported(t, o, leafnodeTLSInsecureWarning)
+
+	// Now remove remote and check warning still reported
+	o.LeafNode.Remotes = nil
+	checkWarnReported(t, o, leafnodeTLSInsecureWarning)
+
+	// Remove the LN setting
+	o.LeafNode.Port = 0
+	o.LeafNode.TLSConfig = nil
+
+	// Configure GW with no TLS in main block first, but only with remotes
 	o.Gateway.Name = "A"
 	o.Gateway.Host = "127.0.0.1"
 	o.Gateway.Port = -1
-	o.Gateway.TLSConfig = gwConfig
-	checkServerFails(t, o, gatewayTLSInsecureWarning)
-
-	// Get a config for the remote gateway
-	rgwConfig := gwConfig.Clone()
-	gurl, _ := url.Parse("nats://127.0.0.1:1234")
-
-	// Remove the insecure for the main gateway config
-	o.Gateway.TLSConfig.InsecureSkipVerify = false
 	o.Gateway.Gateways = []*RemoteGatewayOpts{
 		{
 			Name:      "B",
-			URLs:      []*url.URL{gurl},
-			TLSConfig: rgwConfig,
+			URLs:      []*url.URL{rurl},
+			TLSConfig: config.Clone(),
 		},
 	}
-	checkServerFails(t, o, gatewayTLSInsecureWarning)
+	checkWarnReported(t, o, gatewayTLSInsecureWarning)
+
+	// Now add to main block.
+	o.Gateway.TLSConfig = config.Clone()
+	checkWarnReported(t, o, gatewayTLSInsecureWarning)
+
+	// Now remove remote and check warning still reported
+	o.Gateway.Gateways = nil
+	checkWarnReported(t, o, gatewayTLSInsecureWarning)
 }
 
 func TestConnectErrorReports(t *testing.T) {