From f8914788f555f024dd5abd8a77f9af591a239bca Mon Sep 17 00:00:00 2001 From: Waldemar Quevedo Date: Tue, 14 Mar 2023 14:06:46 -0700 Subject: [PATCH] Fix leaf client connection failing in ocsp setup Signed-off-by: Waldemar Quevedo --- server/ocsp.go | 7 +- test/ocsp_test.go | 419 ++++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 407 insertions(+), 19 deletions(-) diff --git a/server/ocsp.go b/server/ocsp.go index fa8b2113..db27d214 100644 --- a/server/ocsp.go +++ b/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 diff --git a/test/ocsp_test.go b/test/ocsp_test.go index 7cf30e64..1725a6bf 100644 --- a/test/ocsp_test.go +++ b/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"