From 040258dc415af48100968eba52b445a875b132fc Mon Sep 17 00:00:00 2001 From: Waldemar Quevedo Date: Thu, 14 Oct 2021 14:42:48 -0700 Subject: [PATCH] Fix for #2628 #2629 issues Signed-off-by: Waldemar Quevedo --- server/ocsp.go | 20 ++- test/leafnode_test.go | 2 +- test/ocsp_test.go | 356 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 369 insertions(+), 9 deletions(-) diff --git a/server/ocsp.go b/server/ocsp.go index 56b664be..59fd4871 100644 --- a/server/ocsp.go +++ b/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) diff --git a/test/leafnode_test.go b/test/leafnode_test.go index e0221bc2..30b4e3a4 100644 --- a/test/leafnode_test.go +++ b/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 diff --git a/test/ocsp_test.go b/test/ocsp_test.go index f26898e5..b887b35f 100644 --- a/test/ocsp_test.go +++ b/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) + } +}