From 5009d775a14bfde3ba286c746083d4f9d23e2cd6 Mon Sep 17 00:00:00 2001 From: Steven Clark Date: Mon, 18 Mar 2024 18:12:37 -0400 Subject: [PATCH] Support OCSP responses without NextUpdate field set (#25912) * Support OCSP responses without a NextUpdate value set - Validate that the ThisUpdate value is properly prior to our current time and if NextUpdate is set that, ThisUpdate is before NextUpdate. - If we don't have a value for NextUpdate just compare against ThisUpdate. * Add ocsp_this_update_max_ttl support to cert auth - Allow configuring a maximum TTL of the OCSP response based on the ThisUpdate time like OpenSSL does - Add test to validate that we don't cache OCSP responses with no NextUpdate * Add cl * Add missing ` in docs * Rename ocsp_this_update_max_ttl to ocsp_this_update_max_age * Missed a few TTL references * Fix error message --- builtin/credential/cert/path_certs.go | 25 ++- builtin/credential/cert/path_login.go | 3 +- changelog/25912.txt | 3 + sdk/helper/ocsp/client.go | 78 +++++--- sdk/helper/ocsp/ocsp_test.go | 260 ++++++++++++++++++++++--- website/content/api-docs/auth/cert.mdx | 5 +- 6 files changed, 319 insertions(+), 55 deletions(-) create mode 100644 changelog/25912.txt diff --git a/builtin/credential/cert/path_certs.go b/builtin/credential/cert/path_certs.go index 6ddc5ec7829..2779b5179d0 100644 --- a/builtin/credential/cert/path_certs.go +++ b/builtin/credential/cert/path_certs.go @@ -10,6 +10,7 @@ import ( "strings" "time" + "github.com/hashicorp/go-secure-stdlib/parseutil" "github.com/hashicorp/go-sockaddr" "github.com/hashicorp/vault/sdk/framework" @@ -91,6 +92,11 @@ from the AuthorityInformationAccess extension on the certificate being inspected Default: false, Description: "If set to true, rather than accepting the first successful OCSP response, query all servers and consider the certificate valid only if all servers agree.", }, + "ocsp_this_update_max_age": { + Type: framework.TypeDurationSecond, + Default: 0, + Description: "If greater than 0, specifies the maximum age of an OCSP thisUpdate field to avoid accepting old responses without a nextUpdate field.", + }, "allowed_names": { Type: framework.TypeCommaStringSlice, Description: `A comma-separated list of names. @@ -309,6 +315,7 @@ func (b *backend) pathCertRead(ctx context.Context, req *logical.Request, d *fra "ocsp_servers_override": cert.OcspServersOverride, "ocsp_fail_open": cert.OcspFailOpen, "ocsp_query_all_servers": cert.OcspQueryAllServers, + "ocsp_this_update_max_age": int64(cert.OcspThisUpdateMaxAge.Seconds()), } cert.PopulateTokenData(data) @@ -367,6 +374,13 @@ func (b *backend) pathCertWrite(ctx context.Context, req *logical.Request, d *fr if ocspQueryAll, ok := d.GetOk("ocsp_query_all_servers"); ok { cert.OcspQueryAllServers = ocspQueryAll.(bool) } + if ocspThisUpdateMaxAge, ok := d.GetOk("ocsp_this_update_max_age"); ok { + maxAgeDuration, err := parseutil.ParseDurationSecond(ocspThisUpdateMaxAge) + if err != nil { + return nil, fmt.Errorf("failed to parse ocsp_this_update_max_age: %w", err) + } + cert.OcspThisUpdateMaxAge = maxAgeDuration + } if displayNameRaw, ok := d.GetOk("display_name"); ok { cert.DisplayName = displayNameRaw.(string) } @@ -512,11 +526,12 @@ type CertEntry struct { AllowedMetadataExtensions []string BoundCIDRs []*sockaddr.SockAddrMarshaler - OcspCaCertificates string - OcspEnabled bool - OcspServersOverride []string - OcspFailOpen bool - OcspQueryAllServers bool + OcspCaCertificates string + OcspEnabled bool + OcspServersOverride []string + OcspFailOpen bool + OcspQueryAllServers bool + OcspThisUpdateMaxAge time.Duration } const pathCertHelpSyn = ` diff --git a/builtin/credential/cert/path_login.go b/builtin/credential/cert/path_login.go index 18783c0fdf9..361fd1dc3c8 100644 --- a/builtin/credential/cert/path_login.go +++ b/builtin/credential/cert/path_login.go @@ -25,7 +25,7 @@ import ( "github.com/hashicorp/errwrap" "github.com/hashicorp/go-multierror" - glob "github.com/ryanuber/go-glob" + "github.com/ryanuber/go-glob" ) // ParsedCert is a certificate that has been configured as trusted @@ -663,6 +663,7 @@ func (b *backend) loadTrustedCerts(ctx context.Context, storage logical.Storage, conf.OcspFailureMode = ocsp.FailOpenFalse } conf.QueryAllServers = conf.QueryAllServers || entry.OcspQueryAllServers + conf.OcspThisUpdateMaxAge = entry.OcspThisUpdateMaxAge } } diff --git a/changelog/25912.txt b/changelog/25912.txt new file mode 100644 index 00000000000..fdb419c8f46 --- /dev/null +++ b/changelog/25912.txt @@ -0,0 +1,3 @@ +```release-note:improvement +auth/cert: Allow validation with OCSP responses with no NextUpdate time +``` diff --git a/sdk/helper/ocsp/client.go b/sdk/helper/ocsp/client.go index 9c1375c4f5b..5bb82e1f81e 100644 --- a/sdk/helper/ocsp/client.go +++ b/sdk/helper/ocsp/client.go @@ -162,9 +162,27 @@ func (c *Client) getHashAlgorithmFromOID(target pkix.AlgorithmIdentifier) crypto return crypto.SHA1 } -// isInValidityRange checks the validity -func isInValidityRange(currTime, nextUpdate time.Time) bool { - return !nextUpdate.IsZero() && !currTime.After(nextUpdate) +// isInValidityRange checks the validity times of the OCSP response making sure +// that thisUpdate and nextUpdate values are bounded within currTime +func isInValidityRange(currTime time.Time, ocspRes *ocsp.Response) bool { + thisUpdate := ocspRes.ThisUpdate + + // If the thisUpdate value in the OCSP response wasn't set or greater than current time fail this check + if thisUpdate.IsZero() || thisUpdate.After(currTime) { + return false + } + + nextUpdate := ocspRes.NextUpdate + if nextUpdate.IsZero() { + // We don't have a nextUpdate field set, assume we are okay. + return true + } + + if currTime.After(nextUpdate) || thisUpdate.After(nextUpdate) { + return false + } + + return true } func extractCertIDKeyFromRequest(ocspReq []byte) (*certIDKey, *ocspStatus) { @@ -209,7 +227,7 @@ func (c *Client) encodeCertIDKey(certIDKeyBase64 string) (*certIDKey, error) { }, nil } -func (c *Client) checkOCSPResponseCache(encodedCertID *certIDKey, subject, issuer *x509.Certificate) (*ocspStatus, error) { +func (c *Client) checkOCSPResponseCache(encodedCertID *certIDKey, subject, issuer *x509.Certificate, config *VerifyConfig) (*ocspStatus, error) { c.ocspResponseCacheLock.RLock() var cacheValue *ocspCachedResponse v, ok := c.ocspResponseCache.Get(*encodedCertID) @@ -218,7 +236,7 @@ func (c *Client) checkOCSPResponseCache(encodedCertID *certIDKey, subject, issue } c.ocspResponseCacheLock.RUnlock() - status, err := c.extractOCSPCacheResponseValue(cacheValue, subject, issuer) + status, err := c.extractOCSPCacheResponseValue(cacheValue, subject, issuer, config) if err != nil { return nil, err } @@ -235,18 +253,25 @@ func (c *Client) deleteOCSPCache(encodedCertID *certIDKey) { c.ocspResponseCacheLock.Unlock() } -func validateOCSP(ocspRes *ocsp.Response) (*ocspStatus, error) { +func validateOCSP(conf *VerifyConfig, ocspRes *ocsp.Response) (*ocspStatus, error) { curTime := time.Now() if ocspRes == nil { return nil, errors.New("OCSP Response is nil") } - if !isInValidityRange(curTime, ocspRes.NextUpdate) { + if !isInValidityRange(curTime, ocspRes) { return &ocspStatus{ code: ocspInvalidValidity, err: fmt.Errorf("invalid validity: producedAt: %v, thisUpdate: %v, nextUpdate: %v", ocspRes.ProducedAt, ocspRes.ThisUpdate, ocspRes.NextUpdate), }, nil } + + if conf.OcspThisUpdateMaxAge > 0 && curTime.Sub(ocspRes.ThisUpdate) > conf.OcspThisUpdateMaxAge { + return &ocspStatus{ + code: ocspInvalidValidity, + err: fmt.Errorf("invalid validity: thisUpdate: %v is greater than max age: %s", ocspRes.ThisUpdate, conf.OcspThisUpdateMaxAge), + }, nil + } return returnOCSPStatus(ocspRes), nil } @@ -450,7 +475,7 @@ func (c *Client) retryOCSP( // GetRevocationStatus checks the certificate revocation status for subject using issuer certificate. func (c *Client) GetRevocationStatus(ctx context.Context, subject, issuer *x509.Certificate, conf *VerifyConfig) (*ocspStatus, error) { - status, ocspReq, encodedCertID, err := c.validateWithCache(subject, issuer) + status, ocspReq, encodedCertID, err := c.validateWithCache(subject, issuer, conf) if err != nil { return nil, err } @@ -510,7 +535,7 @@ func (c *Client) GetRevocationStatus(ctx context.Context, subject, issuer *x509. return nil } - ret, err := validateOCSP(ocspRes) + ret, err := validateOCSP(conf, ocspRes) if err != nil { errors[i] = err return err @@ -582,6 +607,12 @@ func (c *Client) GetRevocationStatus(ctx context.Context, subject, issuer *x509. if !isValidOCSPStatus(ret.code) { return ret, nil } + + if ocspRes.NextUpdate.IsZero() { + // We should not cache values with no NextUpdate values + return ret, nil + } + v := ocspCachedResponse{ status: ret.code, time: float64(time.Now().UTC().Unix()), @@ -602,11 +633,12 @@ func isValidOCSPStatus(status ocspStatusCode) bool { } type VerifyConfig struct { - OcspEnabled bool - ExtraCas []*x509.Certificate - OcspServersOverride []string - OcspFailureMode FailOpenMode - QueryAllServers bool + OcspEnabled bool + ExtraCas []*x509.Certificate + OcspServersOverride []string + OcspFailureMode FailOpenMode + QueryAllServers bool + OcspThisUpdateMaxAge time.Duration } // VerifyLeafCertificate verifies just the subject against it's direct issuer @@ -692,12 +724,12 @@ func (c *Client) canEarlyExitForOCSP(results []*ocspStatus, chainSize int, conf return nil } -func (c *Client) validateWithCacheForAllCertificates(verifiedChains []*x509.Certificate) (bool, error) { +func (c *Client) validateWithCacheForAllCertificates(verifiedChains []*x509.Certificate, config *VerifyConfig) (bool, error) { n := len(verifiedChains) - 1 for j := 0; j < n; j++ { subject := verifiedChains[j] issuer := verifiedChains[j+1] - status, _, _, err := c.validateWithCache(subject, issuer) + status, _, _, err := c.validateWithCache(subject, issuer, config) if err != nil { return false, err } @@ -708,7 +740,7 @@ func (c *Client) validateWithCacheForAllCertificates(verifiedChains []*x509.Cert return true, nil } -func (c *Client) validateWithCache(subject, issuer *x509.Certificate) (*ocspStatus, []byte, *certIDKey, error) { +func (c *Client) validateWithCache(subject, issuer *x509.Certificate, config *VerifyConfig) (*ocspStatus, []byte, *certIDKey, error) { ocspReq, err := ocsp.CreateRequest(subject, issuer, &ocsp.RequestOptions{}) if err != nil { return nil, nil, nil, fmt.Errorf("failed to create OCSP request from the certificates: %v", err) @@ -717,7 +749,7 @@ func (c *Client) validateWithCache(subject, issuer *x509.Certificate) (*ocspStat if ocspS.code != ocspSuccess { return nil, nil, nil, fmt.Errorf("failed to extract CertID from OCSP Request: %v", err) } - status, err := c.checkOCSPResponseCache(encodedCertID, subject, issuer) + status, err := c.checkOCSPResponseCache(encodedCertID, subject, issuer, config) if err != nil { return nil, nil, nil, err } @@ -725,7 +757,7 @@ func (c *Client) validateWithCache(subject, issuer *x509.Certificate) (*ocspStat } func (c *Client) GetAllRevocationStatus(ctx context.Context, verifiedChains []*x509.Certificate, conf *VerifyConfig) ([]*ocspStatus, error) { - _, err := c.validateWithCacheForAllCertificates(verifiedChains) + _, err := c.validateWithCacheForAllCertificates(verifiedChains, conf) if err != nil { return nil, err } @@ -750,11 +782,11 @@ func (c *Client) verifyPeerCertificateSerial(conf *VerifyConfig) func(_ [][]byte } } -func (c *Client) extractOCSPCacheResponseValueWithoutSubject(cacheValue ocspCachedResponse) (*ocspStatus, error) { - return c.extractOCSPCacheResponseValue(&cacheValue, nil, nil) +func (c *Client) extractOCSPCacheResponseValueWithoutSubject(cacheValue ocspCachedResponse, conf *VerifyConfig) (*ocspStatus, error) { + return c.extractOCSPCacheResponseValue(&cacheValue, nil, nil, conf) } -func (c *Client) extractOCSPCacheResponseValue(cacheValue *ocspCachedResponse, subject, issuer *x509.Certificate) (*ocspStatus, error) { +func (c *Client) extractOCSPCacheResponseValue(cacheValue *ocspCachedResponse, subject, issuer *x509.Certificate, conf *VerifyConfig) (*ocspStatus, error) { subjectName := "Unknown" if subject != nil { subjectName = subject.Subject.CommonName @@ -778,7 +810,7 @@ func (c *Client) extractOCSPCacheResponseValue(cacheValue *ocspCachedResponse, s sdkOcspStatus := internalStatusCodeToSDK(cacheValue.status) - return validateOCSP(&ocsp.Response{ + return validateOCSP(conf, &ocsp.Response{ ProducedAt: time.Unix(int64(cacheValue.producedAt), 0).UTC(), ThisUpdate: time.Unix(int64(cacheValue.thisUpdate), 0).UTC(), NextUpdate: time.Unix(int64(cacheValue.nextUpdate), 0).UTC(), diff --git a/sdk/helper/ocsp/ocsp_test.go b/sdk/helper/ocsp/ocsp_test.go index 677dfa85aa5..9aa1d7c9f8b 100644 --- a/sdk/helper/ocsp/ocsp_test.go +++ b/sdk/helper/ocsp/ocsp_test.go @@ -21,6 +21,7 @@ import ( "net/http" "net/http/httptest" "net/url" + "sync/atomic" "testing" "time" @@ -168,6 +169,7 @@ func TestUnitEncodeCertIDGood(t *testing.T) { } func TestUnitCheckOCSPResponseCache(t *testing.T) { + conf := &VerifyConfig{OcspEnabled: true} c := New(testLogFactory, 10) dummyKey0 := certIDKey{ NameHash: "dummy0", @@ -183,7 +185,7 @@ func TestUnitCheckOCSPResponseCache(t *testing.T) { c.ocspResponseCache.Add(dummyKey0, &ocspCachedResponse{time: currentTime}) subject := &x509.Certificate{} issuer := &x509.Certificate{} - ost, err := c.checkOCSPResponseCache(&dummyKey, subject, issuer) + ost, err := c.checkOCSPResponseCache(&dummyKey, subject, issuer, conf) if err != nil { t.Fatal(err) } @@ -192,7 +194,7 @@ func TestUnitCheckOCSPResponseCache(t *testing.T) { } // old timestamp c.ocspResponseCache.Add(dummyKey, &ocspCachedResponse{time: float64(1395054952)}) - ost, err = c.checkOCSPResponseCache(&dummyKey, subject, issuer) + ost, err = c.checkOCSPResponseCache(&dummyKey, subject, issuer, conf) if err != nil { t.Fatal(err) } @@ -202,31 +204,230 @@ func TestUnitCheckOCSPResponseCache(t *testing.T) { // invalid validity c.ocspResponseCache.Add(dummyKey, &ocspCachedResponse{time: float64(currentTime - 1000)}) - ost, err = c.checkOCSPResponseCache(&dummyKey, subject, nil) + ost, err = c.checkOCSPResponseCache(&dummyKey, subject, nil, conf) if err == nil && isValidOCSPStatus(ost.code) { t.Fatalf("should have failed.") } } -func TestUnitExpiredOCSPResponse(t *testing.T) { +// TestUnitValidOCSPResponse validates various combinations of acceptable OCSP responses +func TestUnitValidOCSPResponse(t *testing.T) { rootCaKey, rootCa, leafCert := createCaLeafCerts(t) - expiredOcspResponse := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + type tests struct { + name string + ocspRes ocsp.Response + expectedStatus ocspStatusCode + } + + now := time.Now() + ctx := context.Background() + + tt := []tests{ + { + name: "normal", + ocspRes: ocsp.Response{ + SerialNumber: leafCert.SerialNumber, + ThisUpdate: now.Add(-1 * time.Hour), + NextUpdate: now.Add(30 * time.Minute), + Status: ocsp.Good, + }, + expectedStatus: ocspStatusGood, + }, + { + name: "no-next-update", + ocspRes: ocsp.Response{ + SerialNumber: leafCert.SerialNumber, + ThisUpdate: now.Add(-1 * time.Hour), + Status: ocsp.Good, + }, + expectedStatus: ocspStatusGood, + }, + { + name: "revoked-update", + ocspRes: ocsp.Response{ + SerialNumber: leafCert.SerialNumber, + ThisUpdate: now.Add(-1 * time.Hour), + Status: ocsp.Revoked, + }, + expectedStatus: ocspStatusRevoked, + }, + { + name: "revoked-update-with-next-update", + ocspRes: ocsp.Response{ + SerialNumber: leafCert.SerialNumber, + ThisUpdate: now.Add(-1 * time.Hour), + NextUpdate: now.Add(1 * time.Hour), + Status: ocsp.Revoked, + }, + expectedStatus: ocspStatusRevoked, + }, + } + for _, tc := range tt { + for _, maxAge := range []time.Duration{time.Duration(0), time.Duration(2 * time.Hour)} { + t.Run(tc.name+"-max-age-"+maxAge.String(), func(t *testing.T) { + ocspHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + response := buildOcspResponse(t, rootCa, rootCaKey, tc.ocspRes) + _, _ = w.Write(response) + }) + ts := httptest.NewServer(ocspHandler) + defer ts.Close() + + logFactory := func() hclog.Logger { + return hclog.NewNullLogger() + } + client := New(logFactory, 100) + config := &VerifyConfig{ + OcspEnabled: true, + OcspServersOverride: []string{ts.URL}, + OcspFailureMode: FailOpenFalse, + QueryAllServers: false, + OcspThisUpdateMaxAge: maxAge, + } + + status, err := client.GetRevocationStatus(ctx, leafCert, rootCa, config) + require.NoError(t, err, "ocsp response should have been considered valid") + require.NoError(t, status.err, "ocsp status should not contain an error") + require.Equal(t, &ocspStatus{code: tc.expectedStatus}, status) + }) + } + } +} + +// TestUnitBadOCSPResponses verifies that we fail properly on a bunch of different +// OCSP response conditions +func TestUnitBadOCSPResponses(t *testing.T) { + rootCaKey, rootCa, leafCert := createCaLeafCerts(t) + + type tests struct { + name string + ocspRes ocsp.Response + maxAge time.Duration + errContains string + } + + now := time.Now() + ctx := context.Background() + + tt := []tests{ + { + name: "expired-next-update", + ocspRes: ocsp.Response{ + SerialNumber: leafCert.SerialNumber, + ThisUpdate: now.Add(-1 * time.Hour), + NextUpdate: now.Add(-30 * time.Minute), + Status: ocsp.Good, + }, + errContains: "invalid validity", + }, + { + name: "this-update-in-future", + ocspRes: ocsp.Response{ + SerialNumber: leafCert.SerialNumber, + ThisUpdate: now.Add(1 * time.Hour), + NextUpdate: now.Add(2 * time.Hour), + Status: ocsp.Good, + }, + errContains: "invalid validity", + }, + { + name: "next-update-before-this-update", + ocspRes: ocsp.Response{ + SerialNumber: leafCert.SerialNumber, + ThisUpdate: now.Add(-1 * time.Hour), + NextUpdate: now.Add(-2 * time.Hour), + Status: ocsp.Good, + }, + errContains: "invalid validity", + }, + { + name: "missing-this-update", + ocspRes: ocsp.Response{ + SerialNumber: leafCert.SerialNumber, + NextUpdate: now.Add(2 * time.Hour), + Status: ocsp.Good, + }, + errContains: "invalid validity", + }, + { + name: "unknown-status", + ocspRes: ocsp.Response{ + SerialNumber: leafCert.SerialNumber, + ThisUpdate: now.Add(-1 * time.Hour), + NextUpdate: now.Add(30 * time.Minute), + Status: ocsp.Unknown, + }, + errContains: "OCSP status unknown", + }, + { + name: "over-max-age", + ocspRes: ocsp.Response{ + SerialNumber: leafCert.SerialNumber, + ThisUpdate: now.Add(-1 * time.Hour), + NextUpdate: now.Add(30 * time.Minute), + Status: ocsp.Good, + }, + maxAge: 10 * time.Minute, + errContains: "is greater than max age", + }, + } + for _, tc := range tt { + t.Run(tc.name, func(t *testing.T) { + ocspHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + response := buildOcspResponse(t, rootCa, rootCaKey, tc.ocspRes) + _, _ = w.Write(response) + }) + ts := httptest.NewServer(ocspHandler) + defer ts.Close() + + logFactory := func() hclog.Logger { + return hclog.NewNullLogger() + } + client := New(logFactory, 100) + + config := &VerifyConfig{ + OcspEnabled: true, + OcspServersOverride: []string{ts.URL}, + OcspFailureMode: FailOpenFalse, + QueryAllServers: false, + OcspThisUpdateMaxAge: tc.maxAge, + } + + status, err := client.GetRevocationStatus(ctx, leafCert, rootCa, config) + if err == nil && status == nil || (status != nil && status.err == nil) { + t.Fatalf("expected an error got none") + } + if err != nil { + require.ErrorContains(t, err, tc.errContains, + "Expected error got response: %v, %v", status, err) + } + if status != nil && status.err != nil { + require.ErrorContains(t, status.err, tc.errContains, + "Expected error got response: %v, %v", status, err) + } + }) + } +} + +// TestUnitZeroNextUpdateAreNotCached verifies that we are not caching the responses +// with no NextUpdate field set as according to RFC6960 4.2.2.1 +// "If nextUpdate is not set, the responder is indicating that newer +// revocation information is available all the time." +func TestUnitZeroNextUpdateAreNotCached(t *testing.T) { + rootCaKey, rootCa, leafCert := createCaLeafCerts(t) + numQueries := &atomic.Uint32{} + ocspHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + numQueries.Add(1) now := time.Now() ocspRes := ocsp.Response{ - SerialNumber: big.NewInt(2), + SerialNumber: leafCert.SerialNumber, ThisUpdate: now.Add(-1 * time.Hour), - NextUpdate: now.Add(-30 * time.Minute), Status: ocsp.Good, } - response, err := ocsp.CreateResponse(rootCa, rootCa, ocspRes, rootCaKey) - if err != nil { - _, _ = w.Write(ocsp.InternalErrorErrorResponse) - t.Fatalf("failed generating OCSP response: %v", err) - } + response := buildOcspResponse(t, rootCa, rootCaKey, ocspRes) _, _ = w.Write(response) }) - ts := httptest.NewServer(expiredOcspResponse) + ts := httptest.NewServer(ocspHandler) defer ts.Close() logFactory := func() hclog.Logger { @@ -234,18 +435,26 @@ func TestUnitExpiredOCSPResponse(t *testing.T) { } client := New(logFactory, 100) - ctx := context.Background() - config := &VerifyConfig{ OcspEnabled: true, OcspServersOverride: []string{ts.URL}, - OcspFailureMode: FailOpenFalse, - QueryAllServers: false, } - status, err := client.GetRevocationStatus(ctx, leafCert, rootCa, config) - require.ErrorContains(t, err, "invalid validity", - "Expected error got response: %v, %v", status, err) + _, err := client.GetRevocationStatus(context.Background(), leafCert, rootCa, config) + require.NoError(t, err, "Failed fetching revocation status") + + _, err = client.GetRevocationStatus(context.Background(), leafCert, rootCa, config) + require.NoError(t, err, "Failed fetching revocation status second time") + + require.Equal(t, uint32(2), numQueries.Load()) +} + +func buildOcspResponse(t *testing.T, ca *x509.Certificate, caKey *ecdsa.PrivateKey, ocspRes ocsp.Response) []byte { + response, err := ocsp.CreateResponse(ca, ca, ocspRes, caKey) + if err != nil { + t.Fatalf("failed generating OCSP response: %v", err) + } + return response } func createCaLeafCerts(t *testing.T) (*ecdsa.PrivateKey, *x509.Certificate, *x509.Certificate) { @@ -291,8 +500,9 @@ func createCaLeafCerts(t *testing.T) (*ecdsa.PrivateKey, *x509.Certificate, *x50 } func TestUnitValidateOCSP(t *testing.T) { + conf := &VerifyConfig{OcspEnabled: true} ocspRes := &ocsp.Response{} - ost, err := validateOCSP(ocspRes) + ost, err := validateOCSP(conf, ocspRes) if err == nil && isValidOCSPStatus(ost.code) { t.Fatalf("should have failed.") } @@ -301,7 +511,7 @@ func TestUnitValidateOCSP(t *testing.T) { ocspRes.ThisUpdate = currentTime.Add(-2 * time.Hour) ocspRes.NextUpdate = currentTime.Add(2 * time.Hour) ocspRes.Status = ocsp.Revoked - ost, err = validateOCSP(ocspRes) + ost, err = validateOCSP(conf, ocspRes) if err != nil { t.Fatal(err) } @@ -310,7 +520,7 @@ func TestUnitValidateOCSP(t *testing.T) { t.Fatalf("should have failed. expected: %v, got: %v", ocspStatusRevoked, ost.code) } ocspRes.Status = ocsp.Good - ost, err = validateOCSP(ocspRes) + ost, err = validateOCSP(conf, ocspRes) if err != nil { t.Fatal(err) } @@ -319,7 +529,7 @@ func TestUnitValidateOCSP(t *testing.T) { t.Fatalf("should have success. expected: %v, got: %v", ocspStatusGood, ost.code) } ocspRes.Status = ocsp.Unknown - ost, err = validateOCSP(ocspRes) + ost, err = validateOCSP(conf, ocspRes) if err != nil { t.Fatal(err) } @@ -327,7 +537,7 @@ func TestUnitValidateOCSP(t *testing.T) { t.Fatalf("should have failed. expected: %v, got: %v", ocspStatusUnknown, ost.code) } ocspRes.Status = ocsp.ServerFailed - ost, err = validateOCSP(ocspRes) + ost, err = validateOCSP(conf, ocspRes) if err != nil { t.Fatal(err) } diff --git a/website/content/api-docs/auth/cert.mdx b/website/content/api-docs/auth/cert.mdx index c9fdeef5cd7..864d175744b 100644 --- a/website/content/api-docs/auth/cert.mdx +++ b/website/content/api-docs/auth/cert.mdx @@ -77,6 +77,9 @@ Sets a CA cert and associated parameters in a role name. - `ocsp_fail_open` `(bool: false)` - If true and an OCSP response cannot be fetched or is of an unknown status, the login will proceed as if the certificate has not been revoked. +- `ocsp_this_update_max_age` `(integer:0 or string: "")` - If greater than 0, specifies + the maximum age of an OCSP thisUpdate field. This avoids accepting old responses + without a nextUpdate field. - `ocsp_query_all_servers` `(bool: false)` - If set to true, rather than accepting the first successful OCSP response, query all servers and consider the certificate valid only if all servers agree. @@ -360,7 +363,7 @@ Configuration options for the method. `allowed_metadata_extensions` will be stored in the alias - `ocsp_cache_size` `(int: 100)` - The size of the OCSP response LRU cache. Note that this cache is used for all configured certificates. -- `role_cache_size` `(int: 200)` - The size of the role cache. Use `-1` to disable +- `role_cache_size` `(int: 200)` - The size of the role cache. Use `-1` to disable role caching. ### Sample payload