From ff5ff849fd5d76eaad8e5cbc0f49b7806e20e596 Mon Sep 17 00:00:00 2001 From: Gabriel Santos Date: Mon, 29 Aug 2022 20:28:47 +0100 Subject: [PATCH] PKI - Honor header If-Modified-Since if present (#16249) * honor header if-modified-since if present * pathGetIssuerCRL first version * check if modified since for CA endpoints * fix date comparison for CA endpoints * suggested changes and refactoring * add writeIssuer to updateDefaultIssuerId and fix error * Move methods out of storage.go into util.go For the most part, these take a SC as param, but aren't directly storage relevant operations. Move them out of storage.go as a result. Signed-off-by: Alexander Scheel * Use UTC timezone for storage Signed-off-by: Alexander Scheel * Rework path_fetch for better if-modified-since handling Signed-off-by: Alexander Scheel * Invalidate all issuers, CRLs on default write When the default is updated, access under earlier timestamps will not work as we're unclear if the timestamp is for this issuer or a previous issuer. Thus, we need to invalidate the CRL and both issuers involved (previous, next) by updating their LastModifiedTimes. Signed-off-by: Alexander Scheel * Add tests for If-Modified-Since Signed-off-by: Alexander Scheel * Correctly invalidate default issuer changes When the default issuer changes, we'll have to mark the invalidation on PR secondary clusters, so they know to update their CRL mapping as well. The swapped issuers will have an updated modification time (which will eventually replicate down and thus be correct), but the CRL modification time is cluster-local information and thus won't be replicated. Signed-off-by: Alexander Scheel * make fmt Signed-off-by: Alexander Scheel * Refactor sendNotModifiedResponseIfNecessary Signed-off-by: Alexander Scheel * Add changelog entry Signed-off-by: Alexander Scheel * Add documentation on if-modified-since Signed-off-by: Alexander Scheel Signed-off-by: Alexander Scheel Co-authored-by: Alexander Scheel --- builtin/logical/pki/backend.go | 8 + builtin/logical/pki/backend_test.go | 252 ++++++++++++++++++++++ builtin/logical/pki/config_util.go | 52 ++++- builtin/logical/pki/crl_util.go | 35 +++ builtin/logical/pki/path_fetch.go | 32 ++- builtin/logical/pki/path_fetch_issuers.go | 30 ++- builtin/logical/pki/storage.go | 8 + builtin/logical/pki/util.go | 95 +++++++- changelog/16249.txt | 3 + website/content/api-docs/secret/pki.mdx | 10 + 10 files changed, 507 insertions(+), 18 deletions(-) create mode 100644 changelog/16249.txt diff --git a/builtin/logical/pki/backend.go b/builtin/logical/pki/backend.go index dac0b224b..7e0f0797b 100644 --- a/builtin/logical/pki/backend.go +++ b/builtin/logical/pki/backend.go @@ -394,6 +394,8 @@ func (b *backend) invalidate(ctx context.Context, key string) { case key == "config/crl": // We may need to reload our OCSP status flag b.crlBuilder.markConfigDirty() + case key == storageIssuerConfig: + b.crlBuilder.invalidateCRLBuildTime() } } @@ -428,6 +430,12 @@ func (b *backend) periodicFunc(ctx context.Context, request *logical.Request) er return err } + // Check if the CRL was invalidated due to issuer swap and update + // accordingly. + if err := b.crlBuilder.flushCRLBuildTimeInvalidation(sc); err != nil { + return err + } + // All good! return nil } diff --git a/builtin/logical/pki/backend_test.go b/builtin/logical/pki/backend_test.go index 92aa1758e..625c63efa 100644 --- a/builtin/logical/pki/backend_test.go +++ b/builtin/logical/pki/backend_test.go @@ -5091,6 +5091,258 @@ TgM7RZnmEjNdeaa4M52o7VY= require.NotContains(t, resp.Data["usage"], "crl-signing") } +func TestBackend_IfModifiedSinceHeaders(t *testing.T) { + t.Parallel() + coreConfig := &vault.CoreConfig{ + LogicalBackends: map[string]logical.Factory{ + "pki": Factory, + }, + } + cluster := vault.NewTestCluster(t, coreConfig, &vault.TestClusterOptions{ + HandlerFunc: vaulthttp.Handler, + }) + cluster.Start() + defer cluster.Cleanup() + client := cluster.Cores[0].Client + + // Mount PKI. + err := client.Sys().Mount("pki", &api.MountInput{ + Type: "pki", + Config: api.MountConfigInput{ + DefaultLeaseTTL: "16h", + MaxLeaseTTL: "60h", + // Required to allow the header to be passed through. + PassthroughRequestHeaders: []string{"if-modified-since"}, + }, + }) + require.NoError(t, err) + + // Get a time before CA generation. Subtract two seconds to ensure + // the value in the seconds field is different than the time the CA + // is actually generated at. + beforeOldCAGeneration := time.Now().Add(-2 * time.Second) + + // Generate an internal CA. This one is the default. + resp, err := client.Logical().Write("pki/root/generate/internal", map[string]interface{}{ + "ttl": "40h", + "common_name": "Root X1", + "key_type": "ec", + "issuer_name": "old-root", + }) + require.NoError(t, err) + require.NotNil(t, resp) + require.NotNil(t, resp.Data) + require.NotEmpty(t, resp.Data["certificate"]) + + // CA is generated, but give a grace window. + afterOldCAGeneration := time.Now().Add(2 * time.Second) + + // When you _save_ headers, client returns a copy. But when you go to + // reset them, it doesn't create a new copy (and instead directly + // assigns). This means we have to continually refresh our view of the + // last headers, otherwise the headers added after the last set operation + // leak into this copy... Yuck! + lastHeaders := client.Headers() + for _, path := range []string{"pki/cert/ca", "pki/cert/crl", "pki/issuer/default/json", "pki/issuer/old-root/json", "pki/issuer/old-root/crl"} { + t.Logf("path: %v", path) + field := "certificate" + if strings.HasPrefix(path, "pki/issuer") && strings.HasSuffix(path, "/crl") { + field = "crl" + } + + // Reading the CA should work, without a header. + resp, err := client.Logical().Read(path) + require.NoError(t, err) + require.NotNil(t, resp) + require.NotNil(t, resp.Data) + require.NotEmpty(t, resp.Data[field]) + + // Ensure that the CA is returned correctly if we give it the old time. + client.AddHeader("If-Modified-Since", beforeOldCAGeneration.Format(time.RFC1123)) + resp, err = client.Logical().Read(path) + require.NoError(t, err) + require.NotNil(t, resp) + require.NotNil(t, resp.Data) + require.NotEmpty(t, resp.Data[field]) + client.SetHeaders(lastHeaders) + lastHeaders = client.Headers() + + // Ensure that the CA is elided if we give it the present time (plus a + // grace window). + client.AddHeader("If-Modified-Since", afterOldCAGeneration.Format(time.RFC1123)) + t.Logf("headers: %v", client.Headers()) + resp, err = client.Logical().Read(path) + require.NoError(t, err) + require.Nil(t, resp) + client.SetHeaders(lastHeaders) + lastHeaders = client.Headers() + } + + // Wait three seconds. This ensures we have adequate grace period + // to distinguish the two cases, even with grace periods. + time.Sleep(3 * time.Second) + + // Generating a second root. This one isn't the default. + beforeNewCAGeneration := time.Now().Add(-2 * time.Second) + + // Generate an internal CA. This one is the default. + _, err = client.Logical().Write("pki/root/generate/internal", map[string]interface{}{ + "ttl": "40h", + "common_name": "Root X1", + "key_type": "ec", + "issuer_name": "new-root", + }) + require.NoError(t, err) + + // As above. + afterNewCAGeneration := time.Now().Add(2 * time.Second) + + // New root isn't the default, so it has fewer paths. + for _, path := range []string{"pki/issuer/new-root/json", "pki/issuer/new-root/crl"} { + t.Logf("path: %v", path) + field := "certificate" + if strings.HasPrefix(path, "pki/issuer") && strings.HasSuffix(path, "/crl") { + field = "crl" + } + + // Reading the CA should work, without a header. + resp, err := client.Logical().Read(path) + require.NoError(t, err) + require.NotNil(t, resp) + require.NotNil(t, resp.Data) + require.NotEmpty(t, resp.Data[field]) + + // Ensure that the CA is returned correctly if we give it the old time. + client.AddHeader("If-Modified-Since", beforeNewCAGeneration.Format(time.RFC1123)) + resp, err = client.Logical().Read(path) + require.NoError(t, err) + require.NotNil(t, resp) + require.NotNil(t, resp.Data) + require.NotEmpty(t, resp.Data[field]) + client.SetHeaders(lastHeaders) + lastHeaders = client.Headers() + + // Ensure that the CA is elided if we give it the present time (plus a + // grace window). + client.AddHeader("If-Modified-Since", afterNewCAGeneration.Format(time.RFC1123)) + t.Logf("headers: %v", client.Headers()) + resp, err = client.Logical().Read(path) + require.NoError(t, err) + require.Nil(t, resp) + client.SetHeaders(lastHeaders) + lastHeaders = client.Headers() + } + + // Wait three seconds. This ensures we have adequate grace period + // to distinguish the two cases, even with grace periods. + time.Sleep(3 * time.Second) + + // Now swap the default issuers around. + _, err = client.Logical().Write("pki/config/issuers", map[string]interface{}{ + "default": "new-root", + }) + require.NoError(t, err) + + // Reading both with the last modified date should return new values. + for _, path := range []string{"pki/cert/ca", "pki/cert/crl", "pki/issuer/default/json", "pki/issuer/old-root/json", "pki/issuer/new-root/json", "pki/issuer/old-root/crl", "pki/issuer/new-root/crl"} { + t.Logf("path: %v", path) + field := "certificate" + if strings.HasPrefix(path, "pki/issuer") && strings.HasSuffix(path, "/crl") { + field = "crl" + } + + // Ensure that the CA is returned correctly if we give it the old time. + client.AddHeader("If-Modified-Since", afterOldCAGeneration.Format(time.RFC1123)) + resp, err = client.Logical().Read(path) + require.NoError(t, err) + require.NotNil(t, resp) + require.NotNil(t, resp.Data) + require.NotEmpty(t, resp.Data[field]) + client.SetHeaders(lastHeaders) + lastHeaders = client.Headers() + + // Ensure that the CA is returned correctly if we give it the old time. + client.AddHeader("If-Modified-Since", afterNewCAGeneration.Format(time.RFC1123)) + resp, err = client.Logical().Read(path) + require.NoError(t, err) + require.NotNil(t, resp) + require.NotNil(t, resp.Data) + require.NotEmpty(t, resp.Data[field]) + client.SetHeaders(lastHeaders) + lastHeaders = client.Headers() + } + + // Wait for things to settle, record the present time, and wait for the + // clock to definitely tick over again. + time.Sleep(2 * time.Second) + preRevocationTimestamp := time.Now() + time.Sleep(2 * time.Second) + + // The above tests should say everything is cached. + for _, path := range []string{"pki/cert/ca", "pki/cert/crl", "pki/issuer/default/json", "pki/issuer/old-root/json", "pki/issuer/new-root/json", "pki/issuer/old-root/crl", "pki/issuer/new-root/crl"} { + t.Logf("path: %v", path) + + // Ensure that the CA is returned correctly if we give it the new time. + client.AddHeader("If-Modified-Since", preRevocationTimestamp.Format(time.RFC1123)) + resp, err = client.Logical().Read(path) + require.NoError(t, err) + require.Nil(t, resp) + client.SetHeaders(lastHeaders) + lastHeaders = client.Headers() + } + + // We could generate some leaves and verify the revocation updates the + // CRL. But, revoking the issuer behaves the same, so let's do that + // instead. + _, err = client.Logical().Write("pki/issuer/old-root/revoke", map[string]interface{}{}) + require.NoError(t, err) + + // CA should still be valid. + for _, path := range []string{"pki/cert/ca", "pki/issuer/default/json", "pki/issuer/old-root/json", "pki/issuer/new-root/json"} { + t.Logf("path: %v", path) + + // Ensure that the CA is returned correctly if we give it the old time. + client.AddHeader("If-Modified-Since", preRevocationTimestamp.Format(time.RFC1123)) + resp, err = client.Logical().Read(path) + require.NoError(t, err) + require.Nil(t, resp) + client.SetHeaders(lastHeaders) + lastHeaders = client.Headers() + } + + // CRL should be invalidated + for _, path := range []string{"pki/cert/crl", "pki/issuer/old-root/crl", "pki/issuer/new-root/crl"} { + t.Logf("path: %v", path) + field := "certificate" + if strings.HasPrefix(path, "pki/issuer") && strings.HasSuffix(path, "/crl") { + field = "crl" + } + + client.AddHeader("If-Modified-Since", preRevocationTimestamp.Format(time.RFC1123)) + resp, err = client.Logical().Read(path) + require.NoError(t, err) + require.NotNil(t, resp) + require.NotNil(t, resp.Data) + require.NotEmpty(t, resp.Data[field]) + client.SetHeaders(lastHeaders) + lastHeaders = client.Headers() + } + + // Finally, if we send some time in the future, everything should be cached again! + futureTime := time.Now().Add(30 * time.Second) + for _, path := range []string{"pki/cert/ca", "pki/cert/crl", "pki/issuer/default/json", "pki/issuer/old-root/json", "pki/issuer/new-root/json", "pki/issuer/old-root/crl", "pki/issuer/new-root/crl"} { + t.Logf("path: %v", path) + + // Ensure that the CA is returned correctly if we give it the new time. + client.AddHeader("If-Modified-Since", futureTime.Format(time.RFC1123)) + resp, err = client.Logical().Read(path) + require.NoError(t, err) + require.Nil(t, resp) + client.SetHeaders(lastHeaders) + lastHeaders = client.Headers() + } +} + var ( initTest sync.Once rsaCAKey string diff --git a/builtin/logical/pki/config_util.go b/builtin/logical/pki/config_util.go index b49eafee7..4558e246c 100644 --- a/builtin/logical/pki/config_util.go +++ b/builtin/logical/pki/config_util.go @@ -1,7 +1,9 @@ package pki import ( + "fmt" "strings" + "time" ) func (sc *storageContext) isDefaultKeySet() (bool, error) { @@ -44,9 +46,55 @@ func (sc *storageContext) updateDefaultIssuerId(id issuerID) error { } if config.DefaultIssuerId != id { - return sc.setIssuersConfig(&issuerConfigEntry{ - DefaultIssuerId: id, + oldDefault := config.DefaultIssuerId + newDefault := id + now := time.Now().UTC() + + err := sc.setIssuersConfig(&issuerConfigEntry{ + DefaultIssuerId: newDefault, }) + if err != nil { + return err + } + + // When the default issuer changes, we need to modify four + // pieces of information: + // + // 1. The old default issuer's modification time, as it no + // longer works for the /cert/ca path. + // 2. The new default issuer's modification time, as it now + // works for the /cert/ca path. + // 3. & 4. Both issuer's CRLs, as they behave the same, under + // the /cert/crl path! + for _, thisId := range []issuerID{oldDefault, newDefault} { + if len(thisId) == 0 { + continue + } + + // 1 & 2 above. + issuer, err := sc.fetchIssuerById(thisId) + if err != nil { + return fmt.Errorf("unable to update issuer (%v)'s modification time: error fetching issuer: %v", thisId, err) + } + + issuer.LastModified = now + err = sc.writeIssuer(issuer) + if err != nil { + return fmt.Errorf("unable to update issuer (%v)'s modification time: error persisting issuer: %v", thisId, err) + } + } + + // Fetch and update the localCRLConfigEntry (3&4). + cfg, err := sc.getLocalCRLConfig() + if err != nil { + return fmt.Errorf("unable to update local CRL config's modification time: error fetching local CRL config: %v", err) + } + + cfg.LastModified = now + err = sc.setLocalCRLConfig(cfg) + if err != nil { + return fmt.Errorf("unable to update local CRL config's modification time: error persisting local CRL config: %v", err) + } } return nil diff --git a/builtin/logical/pki/crl_util.go b/builtin/logical/pki/crl_util.go index aadf3556b..abc17ba84 100644 --- a/builtin/logical/pki/crl_util.go +++ b/builtin/logical/pki/crl_util.go @@ -75,6 +75,10 @@ type crlBuilder struct { _config sync.RWMutex dirty *atomic2.Bool config crlConfig + + // Whether to invalidate our LastModifiedTime due to write on the + // global issuance config. + invalidate *atomic2.Bool } const ( @@ -91,6 +95,7 @@ func newCRLBuilder() *crlBuilder { lastDeltaRebuildCheck: time.Now(), dirty: atomic2.NewBool(true), config: defaultCrlConfig, + invalidate: atomic2.NewBool(false), } } @@ -207,6 +212,33 @@ func (cb *crlBuilder) checkForAutoRebuild(sc *storageContext) error { return nil } +// Mark the internal LastModifiedTime tracker invalid. +func (cb *crlBuilder) invalidateCRLBuildTime() { + cb.invalidate.Store(true) +} + +// Update the config to mark the modified CRL. See note in +// updateDefaultIssuerId about why this is necessary. +func (cb *crlBuilder) flushCRLBuildTimeInvalidation(sc *storageContext) error { + if cb.invalidate.CAS(true, false) { + // Flush out our invalidation. + cfg, err := sc.getLocalCRLConfig() + if err != nil { + cb.invalidate.Store(true) + return fmt.Errorf("unable to update local CRL config's modification time: error fetching: %v", err) + } + + cfg.LastModified = time.Now().UTC() + err = sc.setLocalCRLConfig(cfg) + if err != nil { + cb.invalidate.Store(true) + return fmt.Errorf("unable to update local CRL config's modification time: error persisting: %v", err) + } + } + + return nil +} + // rebuildIfForced is to be called by readers or periodic functions that might need to trigger // a refresh of the CRL before the read occurs. func (cb *crlBuilder) rebuildIfForced(ctx context.Context, b *backend, request *logical.Request) error { @@ -898,6 +930,9 @@ func buildAnyCRLs(sc *storageContext, forceNew bool, isDelta bool) error { lastCompleteNumber = crlNumber - 1 } + // Update `LastModified` + crlConfig.LastModified = time.Now().UTC() + // Lastly, build the CRL. nextUpdate, err := buildCRL(sc, globalCRLConfig, forceNew, representative, revokedCerts, crlIdentifier, crlNumber, isDelta, lastCompleteNumber) if err != nil { diff --git a/builtin/logical/pki/path_fetch.go b/builtin/logical/pki/path_fetch.go index b73bae196..871eb6ae5 100644 --- a/builtin/logical/pki/path_fetch.go +++ b/builtin/logical/pki/path_fetch.go @@ -159,6 +159,7 @@ func (b *backend) pathFetchRead(ctx context.Context, req *logical.Request, data response = &logical.Response{ Data: map[string]interface{}{}, } + sc := b.makeStorageContext(ctx, req.Storage) // Some of these need to return raw and some non-raw; // this is basically handled by setting contentType or not. @@ -166,19 +167,34 @@ func (b *backend) pathFetchRead(ctx context.Context, req *logical.Request, data // paths still need to return raw output. switch { - case req.Path == "ca" || req.Path == "ca/pem": + case req.Path == "ca" || req.Path == "ca/pem" || req.Path == "cert/ca" || req.Path == "cert/ca/raw" || req.Path == "cert/ca/raw/pem": + ret, err := sendNotModifiedResponseIfNecessary(&IfModifiedSinceHelper{req: req, issuerRef: defaultRef}, sc, response) + if err != nil || ret { + retErr = err + goto reply + } + serial = "ca" contentType = "application/pkix-cert" - if req.Path == "ca/pem" { + if req.Path == "ca/pem" || req.Path == "cert/ca/raw/pem" { pemType = "CERTIFICATE" contentType = "application/pem-certificate-chain" + } else if req.Path == "cert/ca" { + pemType = "CERTIFICATE" + contentType = "" } case req.Path == "ca_chain" || req.Path == "cert/ca_chain": serial = "ca_chain" if req.Path == "ca_chain" { contentType = "application/pkix-cert" } - case req.Path == "crl" || req.Path == "crl/pem" || req.Path == "crl/delta" || req.Path == "crl/delta/pem": + case req.Path == "crl" || req.Path == "crl/pem" || req.Path == "crl/delta" || req.Path == "crl/delta/pem" || req.Path == "cert/crl" || req.Path == "cert/crl/raw" || req.Path == "cert/crl/raw/pem": + ret, err := sendNotModifiedResponseIfNecessary(&IfModifiedSinceHelper{req: req}, sc, response) + if err != nil || ret { + retErr = err + goto reply + } + serial = legacyCRLPath if req.Path == "crl/delta" || req.Path == "crl/delta/pem" { serial = deltaCRLPath @@ -187,13 +203,10 @@ func (b *backend) pathFetchRead(ctx context.Context, req *logical.Request, data if req.Path == "crl/pem" || req.Path == "crl/delta/pem" { pemType = "X509 CRL" contentType = "application/x-pem-file" + } else if req.Path == "cert/crl" { + pemType = "X509 CRL" + contentType = "" } - case req.Path == "cert/crl" || req.Path == "cert/delta-crl": - serial = legacyCRLPath - if req.Path == "cert/delta-crl" { - serial = deltaCRLPath - } - pemType = "X509 CRL" case strings.HasSuffix(req.Path, "/pem") || strings.HasSuffix(req.Path, "/raw"): serial = data.Get("serial").(string) contentType = "application/pkix-cert" @@ -212,7 +225,6 @@ func (b *backend) pathFetchRead(ctx context.Context, req *logical.Request, data // Prefer fetchCAInfo to fetchCertBySerial for CA certificates. if serial == "ca_chain" || serial == "ca" { - sc := b.makeStorageContext(ctx, req.Storage) caInfo, err := sc.fetchCAInfo(defaultRef, ReadOnlyUsage) if err != nil { switch err.(type) { diff --git a/builtin/logical/pki/path_fetch_issuers.go b/builtin/logical/pki/path_fetch_issuers.go index a9c1e196e..0b0cb28d8 100644 --- a/builtin/logical/pki/path_fetch_issuers.go +++ b/builtin/logical/pki/path_fetch_issuers.go @@ -346,6 +346,7 @@ func (b *backend) pathUpdateIssuer(ctx context.Context, req *logical.Request, da if newName != issuer.Name { oldName = issuer.Name issuer.Name = newName + issuer.LastModified = time.Now().UTC() modified = true } @@ -532,6 +533,7 @@ func (b *backend) pathPatchIssuer(ctx context.Context, req *logical.Request, dat if newName != issuer.Name { oldName = issuer.Name issuer.Name = newName + issuer.LastModified = time.Now().UTC() modified = true } } @@ -743,9 +745,20 @@ func (b *backend) pathGetRawIssuer(ctx context.Context, req *logical.Request, da return nil, err } - certificate := []byte(issuer.Certificate) - var contentType string + var certificate []byte + + response := &logical.Response{} + ret, err := sendNotModifiedResponseIfNecessary(&IfModifiedSinceHelper{req: req, issuerRef: ref}, sc, response) + if err != nil { + return nil, err + } + if ret { + return response, nil + } + + certificate = []byte(issuer.Certificate) + if strings.HasSuffix(req.Path, "/pem") { contentType = "application/pem-certificate-chain" } else if strings.HasSuffix(req.Path, "/der") { @@ -913,7 +926,18 @@ func (b *backend) pathGetIssuerCRL(ctx context.Context, req *logical.Request, da return nil, err } + var certificate []byte + var contentType string + sc := b.makeStorageContext(ctx, req.Storage) + response := &logical.Response{} + ret, err := sendNotModifiedResponseIfNecessary(&IfModifiedSinceHelper{req: req}, sc, response) + if err != nil { + return nil, err + } + if ret { + return response, nil + } crlPath, err := sc.resolveIssuerCRLPath(issuerName) if err != nil { return nil, err @@ -928,12 +952,10 @@ func (b *backend) pathGetIssuerCRL(ctx context.Context, req *logical.Request, da return nil, err } - var certificate []byte if crlEntry != nil && len(crlEntry.Value) > 0 { certificate = []byte(crlEntry.Value) } - var contentType string if strings.HasSuffix(req.Path, "/der") { contentType = "application/pkix-crl" } else if strings.HasSuffix(req.Path, "/pem") { diff --git a/builtin/logical/pki/storage.go b/builtin/logical/pki/storage.go index 6bf5316e4..d2a5dc9df 100644 --- a/builtin/logical/pki/storage.go +++ b/builtin/logical/pki/storage.go @@ -35,6 +35,9 @@ const ( maxRolesToFindOnIssuerChange = 10 latestIssuerVersion = 1 + + headerIfModifiedSince = "If-Modified-Since" + headerLastModified = "Last-Modified" ) type keyID string @@ -157,6 +160,7 @@ type issuerEntry struct { RevocationTime int64 `json:"revocation_time"` RevocationTimeUTC time.Time `json:"revocation_time_utc"` AIAURIs *certutil.URLEntries `json:"aia_uris,omitempty"` + LastModified time.Time `json:"last_modified"` Version uint `json:"version"` } @@ -165,6 +169,7 @@ type localCRLConfigEntry struct { CRLNumberMap map[crlID]int64 `json:"crl_number_map"` LastCompleteNumberMap map[crlID]int64 `json:"last_complete_number_map"` CRLExpirationMap map[crlID]time.Time `json:"crl_expiration_map"` + LastModified time.Time `json:"last_modified"` } type keyConfigEntry struct { @@ -622,6 +627,9 @@ func (sc *storageContext) upgradeIssuerIfRequired(issuer *issuerEntry) *issuerEn func (sc *storageContext) writeIssuer(issuer *issuerEntry) error { issuerId := issuer.ID + if issuer.LastModified.IsZero() { + issuer.LastModified = time.Now().UTC() + } json, err := logical.StorageEntryJSON(issuerPrefix+issuerId.String(), issuer) if err != nil { diff --git a/builtin/logical/pki/util.go b/builtin/logical/pki/util.go index bde344b9a..6822df260 100644 --- a/builtin/logical/pki/util.go +++ b/builtin/logical/pki/util.go @@ -5,14 +5,16 @@ import ( "crypto/x509" "fmt" "math/big" + "net/http" "regexp" "strings" - - "github.com/hashicorp/vault/sdk/helper/certutil" + "time" "github.com/hashicorp/vault/sdk/framework" + "github.com/hashicorp/vault/sdk/helper/certutil" "github.com/hashicorp/vault/sdk/helper/errutil" + "github.com/hashicorp/vault/sdk/logical" ) const ( @@ -227,3 +229,92 @@ func isStringArrayDifferent(a, b []string) bool { return false } + +func hasHeader(header string, req *logical.Request) bool { + var hasHeader bool + headerValue := req.Headers[header] + if len(headerValue) > 0 { + hasHeader = true + } + + return hasHeader +} + +func parseIfNotModifiedSince(req *logical.Request) (time.Time, error) { + var headerTimeValue time.Time + headerValue := req.Headers[headerIfModifiedSince] + + headerTimeValue, err := time.Parse(time.RFC1123, headerValue[0]) + if err != nil { + return headerTimeValue, fmt.Errorf("failed to parse given value for '%s' header: %v", headerIfModifiedSince, err) + } + + return headerTimeValue, nil +} + +type IfModifiedSinceHelper struct { + req *logical.Request + issuerRef issuerID +} + +func sendNotModifiedResponseIfNecessary(helper *IfModifiedSinceHelper, sc *storageContext, resp *logical.Response) (bool, error) { + responseHeaders := map[string][]string{} + if !hasHeader(headerIfModifiedSince, helper.req) { + return false, nil + } + + before, err := sc.isIfModifiedSinceBeforeLastModified(helper, responseHeaders) + if err != nil { + return false, err + } + + if !before { + return false, nil + } + + // Fill response + resp.Data = map[string]interface{}{ + logical.HTTPContentType: "", + logical.HTTPStatusCode: 304, + } + resp.Headers = responseHeaders + + return true, nil +} + +func (sc *storageContext) isIfModifiedSinceBeforeLastModified(helper *IfModifiedSinceHelper, responseHeaders map[string][]string) (bool, error) { + var before bool + var err error + var lastModified time.Time + ifModifiedSince, err := parseIfNotModifiedSince(helper.req) + if err != nil { + return before, err + } + + if helper.issuerRef == "" { + crlConfig, err := sc.getLocalCRLConfig() + if err != nil { + return before, err + } + lastModified = crlConfig.LastModified + } else { + issuerId, err := sc.resolveIssuerReference(string(helper.issuerRef)) + if err != nil { + return before, err + } + + issuer, err := sc.fetchIssuerById(issuerId) + if err != nil { + return before, err + } + + lastModified = issuer.LastModified + } + + if !lastModified.IsZero() && lastModified.Before(ifModifiedSince) { + before = true + responseHeaders[headerLastModified] = []string{lastModified.Format(http.TimeFormat)} + } + + return before, nil +} diff --git a/changelog/16249.txt b/changelog/16249.txt new file mode 100644 index 000000000..f84977d04 --- /dev/null +++ b/changelog/16249.txt @@ -0,0 +1,3 @@ +```release-note:improvement +secrets/pki: Honor If-Modified-Since header on CA, CRL fetch; requires passthrough_request_headers modification on the mount point. +``` diff --git a/website/content/api-docs/secret/pki.mdx b/website/content/api-docs/secret/pki.mdx index 771c15c15..4fcd29f3c 100644 --- a/website/content/api-docs/secret/pki.mdx +++ b/website/content/api-docs/secret/pki.mdx @@ -1018,6 +1018,11 @@ endpoint. These are unauthenticated endpoints. +~> Note: this endpoint accepts the `If-Modified-Since` header, to respond with + 304 Not Modified when the requested resource has not changed. This header + needs to be allowed on the PKI mount by tuning the `passthrough_request_headers` + option. + | Method | Path | Issuer | Format | | :----- | :----------------------------- | :-------- |:----------------------------------------------------------------------------------| | `GET` | `/pki/cert/ca` | `default` | JSON | @@ -1119,6 +1124,11 @@ These are unauthenticated endpoints. ~> **Note**: As of Vault 1.11.0, these endpoints now serve a [version 2](https://datatracker.ietf.org/doc/html/rfc5280#section-5.1.2.1) CRL response. +~> Note: this endpoint accepts the `If-Modified-Since` header, to respond with + 304 Not Modified when the requested resource has not changed. This header + needs to be allowed on the PKI mount by tuning the `passthrough_request_headers` + option. + | Method | Path | Issuer | Format | Type | | :----- | :-------------------------------------- | :-------- | :-------------------------------------------------------------------------------- | :------- | | `GET` | `/pki/cert/crl` | `default` | JSON | Complete |