[FIXED] Allow TLS InsecureSkipVerify again

This has an effect only on connections created by the server,
so routes and gateways (explicit and implicit).
Make sure that an explicit warning is printed if the insecure
property is set, but otherwise allow it.

Resolves #1062

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

server/gateway.go

@@ -46,6 +46,9 @@ var (
 	gatewaySolicitDelay          = int64(defaultSolicitGatewaysDelay)
 )
 
+// Warning when user configures gateway TLS insecure
+const gatewayTLSInsecureWarning = "TLS certificate chain and hostname of solicited gateways will not be verified, do not use in production!"
+
 // SetGatewaysSolicitDelay sets the initial delay before gateways
 // connections are initiated.
 // Used by tests.
@@ -231,9 +234,6 @@ func validateGatewayOptions(o *Options) error {
 	if o.Gateway.Port == 0 {
 		return fmt.Errorf("gateway %q has no port specified (select -1 for random port)", o.Gateway.Name)
 	}
-	if o.Gateway.TLSConfig != nil && o.Gateway.TLSConfig.InsecureSkipVerify {
-		return fmt.Errorf("tls InsecureSkipVerify not supported for gateway")
-	}
 	for i, g := range o.Gateway.Gateways {
 		if g.Name == "" {
 			return fmt.Errorf("gateway in the list %d has no name", i)
@@ -241,9 +241,6 @@ func validateGatewayOptions(o *Options) error {
 		if len(g.URLs) == 0 {
 			return fmt.Errorf("gateway %q has no URL", g.Name)
 		}
-		if g.TLSConfig != nil && g.TLSConfig.InsecureSkipVerify {
-			return fmt.Errorf("tls InsecureSkipVerify not supported for remote gateway %q", g.Name)
-		}
 	}
 	return nil
 }
@@ -434,6 +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
+				}
+			}
+		}
+		if warn {
+			s.Warnf(gatewayTLSInsecureWarning)
+		}
+	}
 	s.mu.Unlock()
 
 	// Let them know we are up

server/route.go

@@ -97,7 +97,7 @@ const (
 const sendRouteSubsInGoRoutineThreshold = 1024 * 1024 // 1MB
 
 // Warning when user configures cluster TLS insecure
-const clusterTLSInsecureWarning = "TLS Hostname verification disabled, system will not verify identity of the solicited route"
+const clusterTLSInsecureWarning = "TLS certificate chain and hostname of solicited routes will not be verified, do not use in production!"
 
 // Can be changed for tests
 var routeConnectDelay = DEFAULT_ROUTE_CONNECT

server/server.go

@@ -309,11 +309,6 @@ func NewServer(opts *Options) (*Server, error) {
 }
 
 func validateOptions(o *Options) error {
-	// For now, InsecureSkipVerify is supported only for Cluster.
-	// So fail if it was set to client and or gateways.
-	if o.TLSConfig != nil && o.TLSConfig.InsecureSkipVerify {
-		return fmt.Errorf("tls InsecureSkipVerify not supported for client connections")
-	}
 	// Check that the trust configuration is correct.
 	if err := validateTrustedOperators(o); err != nil {
 		return err

server/server_test.go

@@ -1088,16 +1088,31 @@ func TestClientWriteLoopStall(t *testing.T) {
 	}
 }
 
-func TestInsecureSkipVerifyNotSupportedForClientAndGateways(t *testing.T) {
-	checkServerFails := func(t *testing.T, o *Options) {
+func TestInsecureSkipVerifyWarning(t *testing.T) {
+	checkServerFails := func(t *testing.T, o *Options, expectedWarn string) {
 		t.Helper()
 		s, err := NewServer(o)
-		if s != nil {
-			s.Shutdown()
+		if err != nil {
+			t.Fatalf("Error on new server: %v", err)
 		}
-		if err == nil || !strings.Contains(err.Error(), "not supported") {
-			t.Fatalf("Expected error about not supported feature, got %v", err)
+		l := &captureWarnLogger{warn: make(chan string, 1)}
+		s.SetLogger(l, false, false)
+		wg := sync.WaitGroup{}
+		wg.Add(1)
+		go func() {
+			s.Start()
+			wg.Done()
+		}()
+		select {
+		case w := <-l.warn:
+			if !strings.Contains(w, expectedWarn) {
+				t.Fatalf("Expected warning %q, got %q", expectedWarn, w)
+			}
+		case <-time.After(2 * time.Second):
+			t.Fatalf("Did not get any warning")
 		}
+		s.Shutdown()
+		wg.Wait()
 	}
 
 	tc := &TLSConfigOpts{}
@@ -1111,20 +1126,22 @@ func TestInsecureSkipVerifyNotSupportedForClientAndGateways(t *testing.T) {
 	if err != nil {
 		t.Fatalf("Error generating tls config: %v", err)
 	}
-	o.TLSConfig = config
-	checkServerFails(t, o)
+	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()
 
-	// Remove the setting
-	o.TLSConfig.InsecureSkipVerify = false
+	// Remove the route setting
+	o.Cluster.Port = 0
+	o.Cluster.TLSConfig = nil
 	// Configure GW
 	o.Gateway.Name = "A"
 	o.Gateway.Host = "127.0.0.1"
 	o.Gateway.Port = -1
 	o.Gateway.TLSConfig = gwConfig
-	checkServerFails(t, o)
+	checkServerFails(t, o, gatewayTLSInsecureWarning)
 
 	// Get a config for the remote gateway
 	rgwConfig := gwConfig.Clone()
@@ -1139,7 +1156,7 @@ func TestInsecureSkipVerifyNotSupportedForClientAndGateways(t *testing.T) {
 			TLSConfig: rgwConfig,
 		},
 	}
-	checkServerFails(t, o)
+	checkServerFails(t, o, gatewayTLSInsecureWarning)
 }
 
 func TestConnectErrorReports(t *testing.T) {

test/cluster_tls_test.go

@@ -89,7 +89,7 @@ type captureClusterTLSInsecureLogger struct {
 
 func (c *captureClusterTLSInsecureLogger) Warnf(format string, v ...interface{}) {
 	msg := fmt.Sprintf(format, v...)
-	if strings.Contains(msg, "verification disabled") {
+	if strings.Contains(msg, "solicited routes will not be verified") {
 		select {
 		case c.ch <- struct{}{}:
 		default: