From 0a72993d80b913d91fbefb38a5fca220015b1f63 Mon Sep 17 00:00:00 2001 From: Ivan Kozlovic Date: Fri, 12 Jul 2019 17:04:59 -0600 Subject: [PATCH] Add warning for TLS insecure setting on LeafNodes Also fix for #1071 in that we need to check remote gateways TLS config even if main gateway section is not configured with TLS. Related to #1071 Signed-off-by: Ivan Kozlovic --- server/gateway.go | 25 +++++++++-------- server/leafnode.go | 22 +++++++++++++++ server/server_test.go | 64 ++++++++++++++++++++++++++++--------------- 3 files changed, 77 insertions(+), 34 deletions(-) diff --git a/server/gateway.go b/server/gateway.go index a12f14f9..8cc4775b 100644 --- a/server/gateway.go +++ b/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() diff --git a/server/leafnode.go b/server/leafnode.go index d11fba37..1064dff2 100644 --- a/server/leafnode.go +++ b/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 diff --git a/server/server_test.go b/server/server_test.go index ae0a71b6..d859f3a7 100644 --- a/server/server_test.go +++ b/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) {