Merge pull request #2632 from nats-io/ocsp-tls-check

Fix for #2628 #2629 issues

server/ocsp.go

@@ -341,7 +341,10 @@ func (srv *Server) NewOCSPMonitor(config *tlsConfigKind) (*tls.Config, *OCSPMoni
 
 	// NOTE: Currently OCSP Stapling is enabled only for the first certificate found.
 	var mon *OCSPMonitor
-	for _, cert := range tc.Certificates {
+	for _, currentCert := range tc.Certificates {
+		// Create local copy since this will be used in the GetCertificate callback.
+		cert := currentCert
+
 		// This is normally non-nil, but can still be nil here when in tests
 		// or in some embedded scenarios.
 		if cert.Leaf == nil {
@@ -547,9 +550,11 @@ func (s *Server) configureOCSP() []*tlsConfigKind {
 		}
 		configs = append(configs, o)
 	}
-	for i, remote := range sopts.LeafNode.Remotes {
-		opts := remote.tlsConfigOpts
+	for _, remote := range sopts.LeafNode.Remotes {
 		if config := remote.TLSConfig; config != nil {
+			// Use a copy of the remote here since will be used
+			// in the apply func callback below.
+			r, opts := remote, remote.tlsConfigOpts
 			o := &tlsConfigKind{
 				kind:      kindStringMap[LEAF],
 				tlsConfig: config,
@@ -558,8 +563,7 @@ func (s *Server) configureOCSP() []*tlsConfigKind {
 					// GetCertificate is used by a server to send the server cert to a
 					// client. We're a client, so we must not set this.
 					tc.GetCertificate = nil
-
-					sopts.LeafNode.Remotes[i].TLSConfig = tc
+					r.TLSConfig = tc
 				},
 			}
 			configs = append(configs, o)
@@ -575,15 +579,15 @@ func (s *Server) configureOCSP() []*tlsConfigKind {
 		}
 		configs = append(configs, o)
 	}
-	for i, remote := range sopts.Gateway.Gateways {
-		opts := remote.tlsConfigOpts
+	for _, remote := range sopts.Gateway.Gateways {
 		if config := remote.TLSConfig; config != nil {
+			gw, opts := remote, remote.tlsConfigOpts
 			o := &tlsConfigKind{
 				kind:      kindStringMap[GATEWAY],
 				tlsConfig: config,
 				tlsOpts:   opts,
 				apply: func(tc *tls.Config) {
-					sopts.Gateway.Gateways[i].TLSConfig = tc
+					gw.TLSConfig = tc
 				},
 			}
 			configs = append(configs, o)

test/leafnode_test.go

@@ -675,7 +675,7 @@ func waitForOutboundGateways(t *testing.T, s *server.Server, expected int, timeo
 	}
 	checkFor(t, timeout, 15*time.Millisecond, func() error {
 		if n := s.NumOutboundGateways(); n != expected {
-			return fmt.Errorf("Expected %v outbound gateway(s), got %v (ulimt -n too low?)",
+			return fmt.Errorf("Expected %v outbound gateway(s), got %v (ulimit -n too low?)",
 				expected, n)
 		}
 		return nil

test/ocsp_test.go

@@ -2467,3 +2467,359 @@ func TestOCSPTLSConfigNoLeafSet(t *testing.T) {
 	s := RunServer(&o)
 	s.Shutdown()
 }
+
+func TestOCSPSuperCluster(t *testing.T) {
+	const (
+		caCert = "configs/certs/ocsp/ca-cert.pem"
+		caKey  = "configs/certs/ocsp/ca-key.pem"
+	)
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+	ocspr := newOCSPResponder(t, caCert, caKey)
+	defer ocspr.Shutdown(ctx)
+	addr := fmt.Sprintf("http://%s", ocspr.Addr)
+	setOCSPStatus(t, addr, "configs/certs/ocsp/server-status-request-url-01-cert.pem", ocsp.Good)
+	setOCSPStatus(t, addr, "configs/certs/ocsp/server-status-request-url-02-cert.pem", ocsp.Good)
+	setOCSPStatus(t, addr, "configs/certs/ocsp/server-status-request-url-03-cert.pem", ocsp.Good)
+	setOCSPStatus(t, addr, "configs/certs/ocsp/server-status-request-url-04-cert.pem", ocsp.Good)
+	setOCSPStatus(t, addr, "configs/certs/ocsp/server-status-request-url-05-cert.pem", ocsp.Good)
+	setOCSPStatus(t, addr, "configs/certs/ocsp/server-status-request-url-06-cert.pem", ocsp.Good)
+	setOCSPStatus(t, addr, "configs/certs/ocsp/server-status-request-url-07-cert.pem", ocsp.Good)
+	setOCSPStatus(t, addr, "configs/certs/ocsp/server-status-request-url-08-cert.pem", ocsp.Good)
+	setOCSPStatus(t, addr, "configs/certs/ocsp/server-cert.pem", ocsp.Good)
+
+	// Store Dirs
+	storeDirA := createDir(t, "_ocspA")
+	defer removeDir(t, storeDirA)
+	storeDirB := createDir(t, "_ocspB")
+	defer removeDir(t, storeDirB)
+	storeDirC := createDir(t, "_ocspC")
+	defer removeDir(t, storeDirC)
+	storeDirD := createDir(t, "_ocspD")
+	defer removeDir(t, storeDirD)
+
+	// Gateway server configuration
+	srvConfA := `
+		host: "127.0.0.1"
+		port: -1
+
+		server_name: "A"
+
+		ocsp { mode: "always" }
+
+		tls {
+			cert_file: "configs/certs/ocsp/server-status-request-url-01-cert.pem"
+			key_file: "configs/certs/ocsp/server-status-request-url-01-key.pem"
+			ca_file: "configs/certs/ocsp/ca-cert.pem"
+			timeout: 5
+		}
+		store_dir: "%s"
+
+		cluster {
+			name: A
+			host: "127.0.0.1"
+			advertise: 127.0.0.1
+			port: -1
+
+			tls {
+				cert_file: "configs/certs/ocsp/server-status-request-url-02-cert.pem"
+				key_file: "configs/certs/ocsp/server-status-request-url-02-key.pem"
+				ca_file: "configs/certs/ocsp/ca-cert.pem"
+				timeout: 5
+			}
+		}
+
+		gateway {
+			name: A
+			host: "127.0.0.1"
+			port: -1
+			advertise: "127.0.0.1"
+
+			tls {
+				cert_file: "configs/certs/ocsp/server-status-request-url-03-cert.pem"
+				key_file: "configs/certs/ocsp/server-status-request-url-03-key.pem"
+				ca_file: "configs/certs/ocsp/ca-cert.pem"
+				timeout: 5
+				verify: true
+			}
+		}
+	`
+	srvConfA = fmt.Sprintf(srvConfA, storeDirA)
+	sconfA := createConfFile(t, []byte(srvConfA))
+	defer removeFile(t, sconfA)
+	srvA, optsA := RunServerWithConfig(sconfA)
+	defer srvA.Shutdown()
+
+	// Server that has the original as a cluster.
+	srvConfB := `
+		host: "127.0.0.1"
+		port: -1
+
+		server_name: "B"
+
+		ocsp { mode: "always" }
+
+		tls {
+			cert_file: "configs/certs/ocsp/server-status-request-url-01-cert.pem"
+			key_file: "configs/certs/ocsp/server-status-request-url-01-key.pem"
+			ca_file: "configs/certs/ocsp/ca-cert.pem"
+			timeout: 5
+		}
+		store_dir: "%s"
+
+		cluster {
+			name: A
+			host: "127.0.0.1"
+			advertise: 127.0.0.1
+			port: -1
+
+			routes: [ nats://127.0.0.1:%d ]
+
+			tls {
+				cert_file: "configs/certs/ocsp/server-status-request-url-02-cert.pem"
+				key_file: "configs/certs/ocsp/server-status-request-url-02-key.pem"
+				ca_file: "configs/certs/ocsp/ca-cert.pem"
+				timeout: 5
+			}
+		}
+
+		gateway {
+			name: A
+			host: "127.0.0.1"
+			advertise: "127.0.0.1"
+			port: -1
+
+			tls {
+				cert_file: "configs/certs/ocsp/server-status-request-url-03-cert.pem"
+				key_file: "configs/certs/ocsp/server-status-request-url-03-key.pem"
+				ca_file: "configs/certs/ocsp/ca-cert.pem"
+				timeout: 5
+				verify: true
+			}
+		}
+	`
+	srvConfB = fmt.Sprintf(srvConfB, storeDirB, optsA.Cluster.Port)
+	conf := createConfFile(t, []byte(srvConfB))
+	defer removeFile(t, conf)
+	srvB, optsB := RunServerWithConfig(conf)
+	defer srvB.Shutdown()
+
+	// Client connects to server A.
+	cA, err := nats.Connect(fmt.Sprintf("tls://127.0.0.1:%d", optsA.Port),
+		nats.Secure(&tls.Config{
+			VerifyConnection: func(s tls.ConnectionState) error {
+				if s.OCSPResponse == nil {
+					return fmt.Errorf("missing OCSP Staple from server")
+				}
+				return nil
+			},
+		}),
+		nats.RootCAs(caCert),
+		nats.ErrorHandler(noOpErrHandler),
+	)
+	if err != nil {
+		t.Fatal(err)
+
+	}
+	defer cA.Close()
+
+	// Start another server that will make connect as a gateway to cluster A.
+	srvConfC := `
+		host: "127.0.0.1"
+		port: -1
+
+		server_name: "C"
+
+		ocsp { mode: "always" }
+
+		tls {
+			cert_file: "configs/certs/ocsp/server-status-request-url-05-cert.pem"
+			key_file: "configs/certs/ocsp/server-status-request-url-05-key.pem"
+			ca_file: "configs/certs/ocsp/ca-cert.pem"
+			timeout: 5
+		}
+		store_dir: "%s"
+		gateway {
+			name: C
+			host: "127.0.0.1"
+			advertise: "127.0.0.1"
+			port: -1
+			gateways: [{
+				name: "A", 
+				urls: ["nats://127.0.0.1:%d"]
+				tls {
+					cert_file: "configs/certs/ocsp/server-status-request-url-06-cert.pem"
+					key_file: "configs/certs/ocsp/server-status-request-url-06-key.pem"
+					ca_file: "configs/certs/ocsp/ca-cert.pem"
+					timeout: 5
+				}
+			}]
+			tls {
+				cert_file: "configs/certs/ocsp/server-status-request-url-06-cert.pem"
+				key_file: "configs/certs/ocsp/server-status-request-url-06-key.pem"
+				ca_file: "configs/certs/ocsp/ca-cert.pem"
+				timeout: 5
+				verify: true
+			}
+		}
+	`
+	srvConfC = fmt.Sprintf(srvConfC, storeDirC, optsA.Gateway.Port)
+	conf = createConfFile(t, []byte(srvConfC))
+	defer removeFile(t, conf)
+	srvC, optsC := RunServerWithConfig(conf)
+	defer srvC.Shutdown()
+
+	// Check that server is connected to any server from the other cluster.
+	checkClusterFormed(t, srvA, srvB)
+	waitForOutboundGateways(t, srvC, 1, 5*time.Second)
+
+	// Start one more server that will become another gateway.
+	srvConfD := `
+		host: "127.0.0.1"
+		port: -1
+
+		server_name: "D"
+
+		ocsp { mode: "auto", url: "%s" }
+
+		tls {
+			cert_file: "configs/certs/ocsp/server-status-request-url-07-cert.pem"
+			key_file: "configs/certs/ocsp/server-status-request-url-07-key.pem"
+			ca_file: "configs/certs/ocsp/ca-cert.pem"
+			timeout: 5
+		}
+		store_dir: "%s"
+		gateway {
+			name: D
+			host: "127.0.0.1"
+			advertise: "127.0.0.1"
+			port: -1
+			gateways: [{
+				name: "A", 
+				urls: ["nats://127.0.0.1:%d"]
+				tls {
+					cert_file: "configs/certs/ocsp/server-status-request-url-08-cert.pem"
+					key_file: "configs/certs/ocsp/server-status-request-url-08-key.pem"
+					ca_file: "configs/certs/ocsp/ca-cert.pem"
+					timeout: 5
+				}},
+				{
+				name: "C", 
+				urls: ["nats://127.0.0.1:%d"]
+
+				####################################################################
+				## TEST NOTE: This cert does not have an OCSP Staple intentionally##
+				####################################################################
+				tls {
+					ca_file: "configs/certs/ocsp/ca-cert.pem"
+					cert_file: "configs/certs/ocsp/server-cert.pem"
+					key_file: "configs/certs/ocsp/server-key.pem"
+					timeout: 5
+				}}
+			]
+			tls {
+				cert_file: "configs/certs/ocsp/server-status-request-url-08-cert.pem"
+				key_file: "configs/certs/ocsp/server-status-request-url-08-key.pem"
+				ca_file: "configs/certs/ocsp/ca-cert.pem"
+				timeout: 5
+				verify: true
+			}
+		}
+	`
+	srvConfD = fmt.Sprintf(srvConfD, addr, storeDirD, optsA.Gateway.Port, optsC.Gateway.Port)
+	conf = createConfFile(t, []byte(srvConfD))
+	defer removeFile(t, conf)
+	srvD, _ := RunServerWithConfig(conf)
+	defer srvD.Shutdown()
+
+	// There should be a single gateway here because one of the gateway connections does not have a OCSP staple.
+	waitForOutboundGateways(t, srvD, 1, 10*time.Second)
+
+	// Connect to cluster A using server B.
+	cB, err := nats.Connect(fmt.Sprintf("tls://127.0.0.1:%d", optsB.Port),
+		nats.Secure(&tls.Config{
+			VerifyConnection: func(s tls.ConnectionState) error {
+				if s.OCSPResponse == nil {
+					return fmt.Errorf("missing OCSP Staple from server")
+				}
+				return nil
+			},
+		}),
+		nats.RootCAs(caCert),
+		nats.ErrorHandler(noOpErrHandler),
+	)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer cB.Close()
+
+	// Connects to cluster C using server C.
+	cC, err := nats.Connect(fmt.Sprintf("tls://127.0.0.1:%d", optsC.Port),
+		nats.Secure(&tls.Config{
+			VerifyConnection: func(s tls.ConnectionState) error {
+				if s.OCSPResponse == nil {
+					return fmt.Errorf("missing OCSP Staple from server")
+				}
+				return nil
+			},
+		}),
+		nats.RootCAs(caCert),
+		nats.ErrorHandler(noOpErrHandler),
+	)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer cC.Close()
+
+	_, err = cA.Subscribe("foo", func(m *nats.Msg) {
+		m.Respond([]byte("From Server A"))
+	})
+	if err != nil {
+		t.Errorf("%v", err)
+	}
+	cA.Flush()
+
+	_, err = cB.Subscribe("bar", func(m *nats.Msg) {
+		m.Respond([]byte("From Server B"))
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+	cB.Flush()
+
+	// Confirm that a message from server C can flow back to server A via gateway..
+	var (
+		resp *nats.Msg
+		lerr error
+	)
+	for i := 0; i < 10; i++ {
+		resp, lerr = cC.Request("foo", nil, 500*time.Millisecond)
+		if lerr != nil {
+			continue
+		}
+		got := string(resp.Data)
+		expected := "From Server A"
+		if got != expected {
+			t.Fatalf("Expected %v, got: %v", expected, got)
+		}
+
+		// Make request to B
+		resp, lerr = cC.Request("bar", nil, 500*time.Millisecond)
+		if lerr != nil {
+			continue
+		}
+		got = string(resp.Data)
+		expected = "From Server B"
+		if got != expected {
+			t.Errorf("Expected %v, got: %v", expected, got)
+		}
+		lerr = nil
+		break
+	}
+	if lerr != nil {
+		t.Errorf("Unexpected error: %v", lerr)
+	}
+	if n := srvD.NumOutboundGateways(); n > 1 {
+		t.Errorf("Expected single gateway, got: %v", n)
+	}
+}