Fix leaf client connection failing in ocsp setup

Signed-off-by: Waldemar Quevedo <wally@nats.io>

server/ocsp.go

@@ -435,7 +435,7 @@ func (srv *Server) NewOCSPMonitor(config *tlsConfigKind) (*tls.Config, *OCSPMoni
 
 		// Check whether need to verify staples from a client connection depending on the type.
 		switch kind {
-		case kindStringMap[ROUTER], kindStringMap[GATEWAY], kindStringMap[LEAF]:
+		case kindStringMap[ROUTER], kindStringMap[GATEWAY]:
 			tc.VerifyConnection = func(s tls.ConnectionState) error {
 				oresp := s.OCSPResponse
 				if oresp == nil {
@@ -558,10 +558,11 @@ func (s *Server) configureOCSP() []*tlsConfigKind {
 			tlsConfig: config,
 			tlsOpts:   opts,
 			apply: func(tc *tls.Config) {
-
 				// RequireAndVerifyClientCert is used to tell a client that it
 				// should send the client cert to the server.
-				tc.ClientAuth = tls.RequireAndVerifyClientCert
+				if opts.Verify {
+					tc.ClientAuth = tls.RequireAndVerifyClientCert
+				}
 				// GetClientCertificate is used by a client to send the client cert
 				// to a server. We're a server, so we must not set this.
 				tc.GetClientCertificate = nil

test/ocsp_test.go

@@ -1255,6 +1255,7 @@ func TestOCSPLeaf(t *testing.T) {
 	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/client-cert.pem", ocsp.Good)
 
 	// Store Dirs
 	storeDirA := t.TempDir()
@@ -1275,6 +1276,7 @@ func TestOCSPLeaf(t *testing.T) {
 			timeout: 5
 		}
 		store_dir: '%s'
+
 		leafnodes {
 			host: "127.0.0.1"
 			port: -1
@@ -1285,6 +1287,8 @@ func TestOCSPLeaf(t *testing.T) {
 				key_file: "configs/certs/ocsp/server-status-request-url-02-key.pem"
 				ca_file: "configs/certs/ocsp/ca-cert.pem"
 				timeout: 5
+				# Leaf connection must present certs.
+				verify: true
 			}
 		}
 	`
@@ -1293,7 +1297,8 @@ func TestOCSPLeaf(t *testing.T) {
 	srvA, optsA := RunServerWithConfig(sconfA)
 	defer srvA.Shutdown()
 
-	// LeafNode that has the original as a remote.
+	// LeafNode that has the original as a remote and running
+	// without OCSP Stapling for the leaf remote.
 	srvConfB := `
 		host: "127.0.0.1"
 		port: -1
@@ -1307,12 +1312,14 @@ func TestOCSPLeaf(t *testing.T) {
 			timeout: 5
 		}
 		store_dir: '%s'
+
 		leafnodes {
 			remotes: [ {
 				url: "tls://127.0.0.1:%d"
 				tls {
-					cert_file: "configs/certs/ocsp/server-status-request-url-04-cert.pem"
-					key_file: "configs/certs/ocsp/server-status-request-url-04-key.pem"
+					# Cert without OCSP Stapling enabled is able to connect.
+					cert_file: "configs/certs/ocsp/client-cert.pem"
+					key_file: "configs/certs/ocsp/client-key.pem"
 					ca_file: "configs/certs/ocsp/ca-cert.pem"
 					timeout: 5
 				}
@@ -1341,19 +1348,19 @@ func TestOCSPLeaf(t *testing.T) {
 		t.Fatal(err)
 	}
 	defer cA.Close()
-	// checkLeafNodeConnected(t, srvA)
+	checkLeafNodeConnected(t, srvA)
 
 	// Revoke the seed server cluster certificate, following servers will not be able to verify connection.
 	setOCSPStatus(t, addr, "configs/certs/ocsp/server-status-request-url-02-cert.pem", ocsp.Revoked)
 
-	// Original set of servers still can communicate to each other, even though the cert has been revoked.
-	// checkLeafNodeConnected(t, srvA)
+	// Original set of servers still can communicate to each other via leafnode, even though the staple
+	// for the leaf server has been revoked.
+	checkLeafNodeConnected(t, srvA)
 
-	// Wait for seed server to notice that its certificate has been revoked,
-	// so that new leafnodes can't connect to it.
+	// Wait for seed server to notice that its certificate has been revoked.
 	time.Sleep(6 * time.Second)
 
-	// Start another server against the seed server that has an invalid OCSP Staple
+	// Start another server against the seed server that has an revoked OCSP Staple.
 	srvConfC := `
 		host: "127.0.0.1"
 		port: -1
@@ -1417,7 +1424,8 @@ func TestOCSPLeaf(t *testing.T) {
 	}
 	defer cC.Close()
 
-	// There should be no connectivity between the clients due to the revoked staple.
+	// There should be connectivity between the clients even if there is a revoked staple
+	// from a leafnode connection.
 	_, err = cA.Subscribe("foo", func(m *nats.Msg) {
 		m.Respond(nil)
 	})
@@ -1432,13 +1440,13 @@ func TestOCSPLeaf(t *testing.T) {
 		t.Fatal(err)
 	}
 	cB.Flush()
-	resp, err := cC.Request("foo", nil, 2*time.Second)
-	if err == nil {
-		t.Errorf("Unexpected success, response: %+v", resp)
+	_, err = cC.Request("foo", nil, 2*time.Second)
+	if err != nil {
+		t.Errorf("Expected success, got: %+v", err)
 	}
-	resp, err = cC.Request("bar", nil, 2*time.Second)
-	if err == nil {
-		t.Errorf("Unexpected success, response: %+v", resp)
+	_, err = cC.Request("bar", nil, 2*time.Second)
+	if err != nil {
+		t.Errorf("Expected success, got: %+v", err)
 	}
 
 	// Switch the certs from the leafnode server to new ones that are not revoked,
@@ -1466,6 +1474,7 @@ func TestOCSPLeaf(t *testing.T) {
 				key_file: "configs/certs/ocsp/server-status-request-url-08-key.pem"
 				ca_file: "configs/certs/ocsp/ca-cert.pem"
 				timeout: 5
+				verify: true
 			}
 		}
 	`
@@ -1502,6 +1511,384 @@ func TestOCSPLeaf(t *testing.T) {
 	}
 }
 
+func TestOCSPLeafNoVerify(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/client-cert.pem", ocsp.Good)
+
+	// Store Dirs
+	storeDirA := t.TempDir()
+	storeDirB := t.TempDir()
+	storeDirC := t.TempDir()
+
+	// LeafNode server configuration
+	srvConfA := `
+		host: "127.0.0.1"
+		port: -1
+
+		server_name: "AAA"
+
+		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'
+
+		leafnodes {
+			host: "127.0.0.1"
+			port: -1
+			advertise: "127.0.0.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
+				# Leaf server does not require certs for clients.
+				verify: false
+			}
+		}
+	`
+	srvConfA = fmt.Sprintf(srvConfA, storeDirA)
+	sconfA := createConfFile(t, []byte(srvConfA))
+	srvA, optsA := RunServerWithConfig(sconfA)
+	defer srvA.Shutdown()
+
+	// LeafNode remote that will connect to A and will not present certs.
+	srvConfB := `
+		host: "127.0.0.1"
+		port: -1
+
+		server_name: "BBB"
+
+		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
+		}
+		store_dir: '%s'
+
+		leafnodes {
+			remotes: [ {
+				url: "tls://127.0.0.1:%d"
+				tls {
+					ca_file: "configs/certs/ocsp/ca-cert.pem"
+					timeout: 5
+				}
+			} ]
+		}
+	`
+	srvConfB = fmt.Sprintf(srvConfB, storeDirB, optsA.LeafNode.Port)
+	conf := createConfFile(t, []byte(srvConfB))
+	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()
+	checkLeafNodeConnected(t, srvA)
+
+	// Revoke the seed server cluster certificate, following servers will not be able to verify connection.
+	setOCSPStatus(t, addr, "configs/certs/ocsp/server-status-request-url-02-cert.pem", ocsp.Revoked)
+
+	// Original set of servers still can communicate to each other, even though the cert has been revoked.
+	checkLeafNodeConnected(t, srvA)
+
+	// Wait for seed server to notice that its certificate has been revoked.
+	time.Sleep(6 * time.Second)
+
+	// Start another server against the seed server that has an revoked OCSP Staple.
+	srvConfC := `
+		host: "127.0.0.1"
+		port: -1
+
+		server_name: "CCC"
+
+		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'
+		leafnodes {
+			remotes: [ {
+				url: "tls://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
+					timeout: 5
+				}
+			} ]
+		}
+	`
+	srvConfC = fmt.Sprintf(srvConfC, storeDirC, optsA.LeafNode.Port)
+	conf = createConfFile(t, []byte(srvConfC))
+	srvC, optsC := RunServerWithConfig(conf)
+	defer srvC.Shutdown()
+
+	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()
+	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()
+
+	// There should be connectivity between the clients even if there is a revoked staple
+	// from a leafnode connection.
+	_, err = cA.Subscribe("foo", func(m *nats.Msg) {
+		m.Respond(nil)
+	})
+	if err != nil {
+		t.Errorf("%v", err)
+	}
+	cA.Flush()
+	_, err = cB.Subscribe("bar", func(m *nats.Msg) {
+		m.Respond(nil)
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+	cB.Flush()
+	_, err = cC.Request("foo", nil, 2*time.Second)
+	if err != nil {
+		t.Errorf("Expected success, got: %+v", err)
+	}
+	_, err = cC.Request("bar", nil, 2*time.Second)
+	if err != nil {
+		t.Errorf("Expected success, got: %+v", err)
+	}
+
+	// Switch the certs from the leafnode server to new ones that are not revoked,
+	// this should restart OCSP Stapling for the leafnode server.
+	srvConfA = `
+		host: "127.0.0.1"
+		port: -1
+
+		server_name: "AAA"
+
+		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'
+		leafnodes {
+			host: "127.0.0.1"
+			port: -1
+			advertise: "127.0.0.1"
+
+			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
+			}
+		}
+	`
+	srvConfA = fmt.Sprintf(srvConfA, storeDirA)
+	if err := os.WriteFile(sconfA, []byte(srvConfA), 0666); err != nil {
+		t.Fatalf("Error writing config: %v", err)
+	}
+	if err := srvA.Reload(); err != nil {
+		t.Fatal(err)
+	}
+	time.Sleep(4 * time.Second)
+
+	// A <-> A
+	_, err = cA.Request("foo", nil, 2*time.Second)
+	if err != nil {
+		t.Errorf("%v", err)
+	}
+
+	// B <-> A
+	_, err = cB.Request("foo", nil, 2*time.Second)
+	if err != nil {
+		t.Errorf("%v", err)
+	}
+
+	// C <-> A
+	_, err = cC.Request("foo", nil, 2*time.Second)
+	if err != nil {
+		t.Errorf("%v", err)
+	}
+	// C <-> B via leafnode A
+	_, err = cC.Request("bar", nil, 2*time.Second)
+	if err != nil {
+		t.Errorf("%v", err)
+	}
+}
+
+func TestOCSPLeafVerifyLeafRemote(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/client-cert.pem", ocsp.Good)
+
+	// Store Dirs
+	storeDirA := t.TempDir()
+	storeDirB := t.TempDir()
+
+	// LeafNode server configuration
+	srvConfA := `
+		host: "127.0.0.1"
+		port: -1
+
+		server_name: "AAA"
+
+		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'
+
+		leafnodes {
+			host: "127.0.0.1"
+			port: -1
+			advertise: "127.0.0.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
+				verify: true
+			}
+		}
+	`
+	srvConfA = fmt.Sprintf(srvConfA, storeDirA)
+	sconfA := createConfFile(t, []byte(srvConfA))
+	srvA, optsA := RunServerWithConfig(sconfA)
+	defer srvA.Shutdown()
+
+	// LeafNode remote that will connect to A and will not present certs.
+	srvConfB := `
+		host: "127.0.0.1"
+		port: -1
+
+		server_name: "BBB"
+
+		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
+		}
+		store_dir: '%s'
+
+		leafnodes {
+			remotes: [ {
+				url: "tls://127.0.0.1:%d"
+				tls {
+					ca_file: "configs/certs/ocsp/ca-cert.pem"
+					timeout: 5
+				}
+			} ]
+		}
+	`
+	srvConfB = fmt.Sprintf(srvConfB, storeDirB, optsA.LeafNode.Port)
+	conf := createConfFile(t, []byte(srvConfB))
+	srvB, _ := 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()
+	checkLeafNodeConnections(t, srvA, 0)
+}
+
 func TestOCSPGateway(t *testing.T) {
 	const (
 		caCert = "configs/certs/ocsp/ca-cert.pem"