From 60499e274942d7c053aae32361661d372aef16f6 Mon Sep 17 00:00:00 2001 From: Waldemar Quevedo Date: Wed, 9 Jun 2021 23:38:55 -0700 Subject: [PATCH] ocsp: add more config options to customize ocsp stapling Signed-off-by: Waldemar Quevedo --- server/opts.go | 46 ++++- server/reload.go | 11 ++ test/ocsp_test.go | 472 +++++++++++++++++++++++++++++++++++++++++++++- 3 files changed, 523 insertions(+), 6 deletions(-) diff --git a/server/opts.go b/server/opts.go index a29e1109..191880b6 100644 --- a/server/opts.go +++ b/server/opts.go @@ -877,12 +877,48 @@ func (o *Options) processConfigFileLine(k string, v interface{}, errors *[]error // and certs path for OCSP Stapling monitoring. o.tlsConfigOpts = tc case "ocsp": - switch v.(type) { + switch vv := v.(type) { case bool: - // Default is Auto which honors Must Staple status request - // but does not shutdown the server in case it is revoked, - // letting the client choose whether to trust or not the server. - o.OCSPConfig = &OCSPConfig{Mode: OCSPModeAuto} + if vv { + // Default is Auto which honors Must Staple status request + // but does not shutdown the server in case it is revoked, + // letting the client choose whether to trust or not the server. + o.OCSPConfig = &OCSPConfig{Mode: OCSPModeAuto} + } else { + o.OCSPConfig = &OCSPConfig{Mode: OCSPModeNever} + } + case map[string]interface{}: + ocsp := &OCSPConfig{Mode: OCSPModeAuto} + + for kk, kv := range vv { + _, v = unwrapValue(kv, &tk) + switch kk { + case "mode": + mode := v.(string) + switch { + case strings.EqualFold(mode, "always"): + ocsp.Mode = OCSPModeAlways + case strings.EqualFold(mode, "must"): + ocsp.Mode = OCSPModeMust + case strings.EqualFold(mode, "never"): + ocsp.Mode = OCSPModeNever + case strings.EqualFold(mode, "auto"): + ocsp.Mode = OCSPModeAuto + default: + *errors = append(*errors, &configErr{tk, fmt.Sprintf("error parsing ocsp config: unsupported ocsp mode %T", mode)}) + } + case "urls": + urls := v.([]string) + ocsp.OverrideURLs = urls + case "url": + url := v.(string) + ocsp.OverrideURLs = []string{url} + default: + *errors = append(*errors, &configErr{tk, fmt.Sprintf("error parsing ocsp config: unsupported field %T", kk)}) + return + } + } + o.OCSPConfig = ocsp default: *errors = append(*errors, &configErr{tk, fmt.Sprintf("error parsing ocsp config: unsupported type %T", v)}) return diff --git a/server/reload.go b/server/reload.go index f6b12bb7..18aa7b01 100644 --- a/server/reload.go +++ b/server/reload.go @@ -582,6 +582,15 @@ func (jso jetStreamOption) IsJetStreamChange() bool { return true } +type ocspOption struct { + noopOption + newValue *OCSPConfig +} + +func (a *ocspOption) Apply(s *Server) { + s.Noticef("Reloaded: OCSP") +} + // connectErrorReports implements the option interface for the `connect_error_reports` // setting. type connectErrorReports struct { @@ -1213,6 +1222,8 @@ func (s *Server) diffOptions(newOpts *Options) ([]option, error) { return nil, fmt.Errorf("config reload not supported for %s: old=%v, new=%v", field.Name, oldValue, newValue) } + case "ocspconfig": + diffOpts = append(diffOpts, &ocspOption{newValue: newValue.(*OCSPConfig)}) default: // TODO(ik): Implement String() on those options to have a nice print. // %v is difficult to figure what's what, %+v print private fields and diff --git a/test/ocsp_test.go b/test/ocsp_test.go index bc34bae2..b042b1a4 100644 --- a/test/ocsp_test.go +++ b/test/ocsp_test.go @@ -964,7 +964,7 @@ func TestOCSPReloadRotateTLSCertEnableMustStaple(t *testing.T) { nc.Close() } -func TestOCSPClusterReload(t *testing.T) { +func TestOCSPCluster(t *testing.T) { const ( caCert = "configs/certs/ocsp/ca-cert.pem" caKey = "configs/certs/ocsp/ca-key.pem" @@ -1782,6 +1782,476 @@ func TestOCSPGateway(t *testing.T) { } } +func TestOCSPCustomConfig(t *testing.T) { + const ( + caCert = "configs/certs/ocsp/ca-cert.pem" + caKey = "configs/certs/ocsp/ca-key.pem" + serverCert = "configs/certs/ocsp/server-cert.pem" + serverKey = "configs/certs/ocsp/server-key.pem" + ) + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + ocspr := newOCSPResponder(t, caCert, caKey) + ocspURL := fmt.Sprintf("http://%s", ocspr.Addr) + defer ocspr.Shutdown(ctx) + + var ( + errExpectedNoStaple = fmt.Errorf("expected no staple") + errMissingStaple = fmt.Errorf("missing OCSP Staple from server") + ) + + for _, test := range []struct { + name string + config string + opts []nats.Option + err error + rerr error + configure func() + }{ + { + "OCSP Stapling in auto mode makes server fail to boot if status is revoked", + ` + port: -1 + + ocsp { + mode: auto + } + + tls { + cert_file: "configs/certs/ocsp/server-cert.pem" + key_file: "configs/certs/ocsp/server-key.pem" + ca_file: "configs/certs/ocsp/ca-cert.pem" + timeout: 5 + } + `, + []nats.Option{ + nats.Secure(&tls.Config{ + VerifyConnection: func(s tls.ConnectionState) error { + if s.OCSPResponse != nil { + return errExpectedNoStaple + } + return nil + }, + }), + nats.ClientCert("./configs/certs/ocsp/client-cert.pem", "./configs/certs/ocsp/client-key.pem"), + nats.RootCAs(caCert), + nats.ErrorHandler(noOpErrHandler), + }, + nil, + nil, + func() { setOCSPStatus(t, ocspURL, serverCert, ocsp.Revoked) }, + }, + { + "OCSP Stapling must staple ignored if disabled with ocsp: false", + ` + port: -1 + + ocsp: false + + 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 + } + `, + []nats.Option{ + nats.Secure(&tls.Config{ + VerifyConnection: func(s tls.ConnectionState) error { + if s.OCSPResponse != nil { + return errExpectedNoStaple + } + return nil + }, + }), + nats.ClientCert("./configs/certs/ocsp/client-cert.pem", "./configs/certs/ocsp/client-key.pem"), + nats.RootCAs(caCert), + nats.ErrorHandler(noOpErrHandler), + }, + nil, + nil, + func() { + setOCSPStatus(t, ocspURL, "configs/certs/ocsp/server-status-request-url-01-cert.pem", ocsp.Good) + }, + }, + { + "OCSP Stapling must staple ignored if disabled with ocsp mode never", + ` + port: -1 + + ocsp: { mode: never } + + 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 + } + `, + []nats.Option{ + nats.Secure(&tls.Config{ + VerifyConnection: func(s tls.ConnectionState) error { + if s.OCSPResponse != nil { + return errExpectedNoStaple + } + return nil + }, + }), + nats.ClientCert("./configs/certs/ocsp/client-cert.pem", "./configs/certs/ocsp/client-key.pem"), + nats.RootCAs(caCert), + nats.ErrorHandler(noOpErrHandler), + }, + nil, + nil, + func() { + setOCSPStatus(t, ocspURL, "configs/certs/ocsp/server-status-request-url-01-cert.pem", ocsp.Good) + }, + }, + { + "OCSP Stapling in always mode fetches a staple even if cert does not have one", + ` + port: -1 + + ocsp { + mode: always + url: "http://127.0.0.1:8888" + } + + tls { + cert_file: "configs/certs/ocsp/server-cert.pem" + key_file: "configs/certs/ocsp/server-key.pem" + ca_file: "configs/certs/ocsp/ca-cert.pem" + timeout: 5 + } + `, + []nats.Option{ + nats.Secure(&tls.Config{ + VerifyConnection: func(s tls.ConnectionState) error { + if s.OCSPResponse == nil { + return errMissingStaple + } + return nil + }, + }), + nats.ClientCert("./configs/certs/ocsp/client-cert.pem", "./configs/certs/ocsp/client-key.pem"), + nats.RootCAs(caCert), + nats.ErrorHandler(noOpErrHandler), + }, + nil, + nil, + func() { setOCSPStatus(t, ocspURL, serverCert, ocsp.Good) }, + }, + { + "OCSP Stapling in must staple mode does not fetch staple if there is no must staple flag", + ` + port: -1 + + ocsp { + mode: must + url: "http://127.0.0.1:8888" + } + + tls { + cert_file: "configs/certs/ocsp/server-cert.pem" + key_file: "configs/certs/ocsp/server-key.pem" + ca_file: "configs/certs/ocsp/ca-cert.pem" + timeout: 5 + } + `, + []nats.Option{ + nats.Secure(&tls.Config{ + VerifyConnection: func(s tls.ConnectionState) error { + if s.OCSPResponse != nil { + return errExpectedNoStaple + } + return nil + }, + }), + nats.ClientCert("./configs/certs/ocsp/client-cert.pem", "./configs/certs/ocsp/client-key.pem"), + nats.RootCAs(caCert), + nats.ErrorHandler(noOpErrHandler), + }, + nil, + nil, + func() { setOCSPStatus(t, ocspURL, serverCert, ocsp.Good) }, + }, + { + "OCSP Stapling in must staple mode fetches staple if there is a must staple flag", + ` + port: -1 + + ocsp { + mode: must + url: "http://127.0.0.1:8888" + } + + 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 + } + `, + []nats.Option{ + nats.Secure(&tls.Config{ + VerifyConnection: func(s tls.ConnectionState) error { + if s.OCSPResponse == nil { + return errMissingStaple + } + return nil + }, + }), + nats.ClientCert("./configs/certs/ocsp/client-cert.pem", "./configs/certs/ocsp/client-key.pem"), + nats.RootCAs(caCert), + nats.ErrorHandler(noOpErrHandler), + }, + nil, + nil, + func() { + setOCSPStatus(t, ocspURL, "configs/certs/ocsp/server-status-request-url-01-cert.pem", ocsp.Good) + }, + }, + } { + t.Run(test.name, func(t *testing.T) { + test.configure() + content := test.config + conf := createConfFile(t, []byte(content)) + defer removeFile(t, conf) + s, opts := RunServerWithConfig(conf) + defer s.Shutdown() + + nc, err := nats.Connect(fmt.Sprintf("tls://localhost:%d", opts.Port), test.opts...) + if test.err == nil && err != nil { + t.Errorf("Expected to connect, got %v", err) + } else if test.err != nil && err == nil { + t.Errorf("Expected error on connect") + } else if test.err != nil && err != nil { + // Error on connect was expected + if test.err.Error() != err.Error() { + t.Errorf("Expected error %s, got: %s", test.err, err) + } + return + } + defer nc.Close() + + nc.Subscribe("ping", func(m *nats.Msg) { + m.Respond([]byte("pong")) + }) + nc.Flush() + + _, err = nc.Request("ping", []byte("ping"), 250*time.Millisecond) + if test.rerr != nil && err == nil { + t.Errorf("Expected error getting response") + } else if test.rerr == nil && err != nil { + t.Errorf("Expected response") + } + }) + } +} + +func TestOCSPCustomConfigReloadDisable(t *testing.T) { + const ( + caCert = "configs/certs/ocsp/ca-cert.pem" + caKey = "configs/certs/ocsp/ca-key.pem" + serverCert = "configs/certs/ocsp/server-cert.pem" + updatedServerCert = "configs/certs/ocsp/server-status-request-url-01-cert.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, serverCert, ocsp.Good) + setOCSPStatus(t, addr, updatedServerCert, ocsp.Good) + + // Start with server without OCSP Stapling MustStaple + content := ` + port: -1 + + ocsp: { mode: always, url: "http://127.0.0.1:8888" } + + tls { + cert_file: "configs/certs/ocsp/server-cert.pem" + key_file: "configs/certs/ocsp/server-key.pem" + ca_file: "configs/certs/ocsp/ca-cert.pem" + timeout: 5 + } + ` + conf := createConfFile(t, []byte(content)) + defer removeFile(t, conf) + s, opts := RunServerWithConfig(conf) + defer s.Shutdown() + + nc, err := nats.Connect(fmt.Sprintf("tls://localhost:%d", opts.Port), + nats.Secure(&tls.Config{ + VerifyConnection: func(s tls.ConnectionState) error { + if s.OCSPResponse == nil { + return fmt.Errorf("missing OCSP Staple!") + } + return nil + }, + }), + nats.RootCAs(caCert), + nats.ErrorHandler(noOpErrHandler), + ) + if err != nil { + t.Fatal(err) + } + sub, err := nc.SubscribeSync("foo") + if err != nil { + t.Fatal(err) + } + nc.Publish("foo", []byte("hello world")) + nc.Flush() + + _, err = sub.NextMsg(1 * time.Second) + if err != nil { + t.Fatal(err) + } + nc.Close() + + // Change and disable OCSP Stapling. + content = ` + port: -1 + + ocsp: { mode: never } + + 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 + } + ` + if err := ioutil.WriteFile(conf, []byte(content), 0666); err != nil { + t.Fatalf("Error writing config: %v", err) + } + if err := s.Reload(); err != nil { + t.Fatal(err) + } + + // The new certificate has must staple but OCSP Stapling is disabled. + time.Sleep(2 * time.Second) + + nc, err = nats.Connect(fmt.Sprintf("tls://localhost:%d", opts.Port), + nats.Secure(&tls.Config{ + VerifyConnection: func(s tls.ConnectionState) error { + if s.OCSPResponse != nil { + return fmt.Errorf("unexpected OCSP Staple!") + } + return nil + }, + }), + nats.RootCAs(caCert), + nats.ErrorHandler(noOpErrHandler), + ) + if err != nil { + t.Fatal(err) + } + nc.Close() +} + +func TestOCSPCustomConfigReloadEnable(t *testing.T) { + const ( + caCert = "configs/certs/ocsp/ca-cert.pem" + caKey = "configs/certs/ocsp/ca-key.pem" + serverCert = "configs/certs/ocsp/server-cert.pem" + updatedServerCert = "configs/certs/ocsp/server-status-request-url-01-cert.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, serverCert, ocsp.Good) + setOCSPStatus(t, addr, updatedServerCert, ocsp.Good) + + // Start with server without OCSP Stapling MustStaple + content := ` + port: -1 + + ocsp: { mode: never, url: "http://127.0.0.1:8888" } + + 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 + } + ` + conf := createConfFile(t, []byte(content)) + defer removeFile(t, conf) + s, opts := RunServerWithConfig(conf) + defer s.Shutdown() + + nc, err := nats.Connect(fmt.Sprintf("tls://localhost:%d", opts.Port), + nats.Secure(&tls.Config{ + VerifyConnection: func(s tls.ConnectionState) error { + if s.OCSPResponse != nil { + return fmt.Errorf("unexpected OCSP Staple!") + } + return nil + }, + }), + nats.RootCAs(caCert), + nats.ErrorHandler(noOpErrHandler), + ) + if err != nil { + t.Fatal(err) + } + sub, err := nc.SubscribeSync("foo") + if err != nil { + t.Fatal(err) + } + nc.Publish("foo", []byte("hello world")) + nc.Flush() + + _, err = sub.NextMsg(1 * time.Second) + if err != nil { + t.Fatal(err) + } + nc.Close() + + // Change and disable OCSP Stapling. + content = ` + port: -1 + + ocsp: { mode: always, url: "http://127.0.0.1:8888" } + + 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 + } + ` + if err := ioutil.WriteFile(conf, []byte(content), 0666); err != nil { + t.Fatalf("Error writing config: %v", err) + } + if err := s.Reload(); err != nil { + t.Fatal(err) + } + time.Sleep(2 * time.Second) + + nc, err = nats.Connect(fmt.Sprintf("tls://localhost:%d", opts.Port), + nats.Secure(&tls.Config{ + VerifyConnection: func(s tls.ConnectionState) error { + if s.OCSPResponse == nil { + return fmt.Errorf("missing OCSP Staple!") + } + return nil + }, + }), + nats.RootCAs(caCert), + nats.ErrorHandler(noOpErrHandler), + ) + if err != nil { + t.Fatal(err) + } + nc.Close() +} + func newOCSPResponder(t *testing.T, issuerCertPEM, issuerKeyPEM string) *http.Server { t.Helper() var mu sync.Mutex