diff --git a/builtin/logical/pki/backend_test.go b/builtin/logical/pki/backend_test.go index 4cc576b75c78..d271e7ed844a 100644 --- a/builtin/logical/pki/backend_test.go +++ b/builtin/logical/pki/backend_test.go @@ -2789,6 +2789,118 @@ func setCerts() { rsaCACert = strings.TrimSpace(string(pem.EncodeToMemory(caCertPEMBlock))) } +func TestBackend_RevokePlusTidy_Intermediate(t *testing.T) { + // Enable PKI secret engine + 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() + cores := cluster.Cores + vault.TestWaitActive(t, cores[0].Core) + client := cores[0].Client + + var err error + + // Mount /pki as a root CA + err = client.Sys().Mount("pki", &api.MountInput{ + Type: "pki", + Config: api.MountConfigInput{ + DefaultLeaseTTL: "16h", + MaxLeaseTTL: "32h", + }, + }) + if err != nil { + t.Fatal(err) + } + + // Set the cluster's certificate as the root CA in /pki + pemBundleRootCA := string(cluster.CACertPEM) + string(cluster.CAKeyPEM) + _, err = client.Logical().Write("pki/config/ca", map[string]interface{}{ + "pem_bundle": pemBundleRootCA, + }) + if err != nil { + t.Fatal(err) + } + + // Mount /pki2 to operate as an intermediate CA + err = client.Sys().Mount("pki2", &api.MountInput{ + Type: "pki", + Config: api.MountConfigInput{ + DefaultLeaseTTL: "16h", + MaxLeaseTTL: "32h", + }, + }) + if err != nil { + t.Fatal(err) + } + + // Create a CSR for the intermediate CA + secret, err := client.Logical().Write("pki2/intermediate/generate/internal", nil) + if err != nil { + t.Fatal(err) + } + intermediateCSR := secret.Data["csr"].(string) + + // Sign the intermediate CSR using /pki + secret, err = client.Logical().Write("pki/root/sign-intermediate", map[string]interface{}{ + "permitted_dns_domains": ".myvault.com", + "csr": intermediateCSR, + }) + if err != nil { + t.Fatal(err) + } + intermediateCertSerial := secret.Data["serial_number"].(string) + intermediateCASerialColon := strings.ReplaceAll(strings.ToLower(intermediateCertSerial), ":", "-") + + // Get the intermediate cert after signing + secret, err = client.Logical().Read("pki/cert/" + intermediateCASerialColon) + if err != nil { + t.Fatal(err) + } + if secret == nil || len(secret.Data) == 0 || len(secret.Data["certificate"].(string)) == 0 { + t.Fatal("expected certificate information from read operation") + } + + // Issue a revoke on on /pki + _, err = client.Logical().Write("pki/revoke", map[string]interface{}{ + "serial_number": intermediateCertSerial, + }) + + // Revoke adds a fixed 2s buffer, so we sleep for a bit longer to ensure + // the revocation time is past the current time. + time.Sleep(3 * time.Second) + + // Issue a tidy on /pki + _, err = client.Logical().Write("pki/tidy", map[string]interface{}{ + "tidy_cert_store": true, + "tidy_revoked_certs": true, + "safety_buffer": "1s", + }) + if err != nil { + t.Fatal(err) + } + + // Sleep a bit to make sure we're past the safety buffer + time.Sleep(2 * time.Second) + + // Attempt to read the intermediate cert after revoke + tidy, and ensure + // that it's no longer present + secret, err = client.Logical().Read("pki/cert/" + intermediateCASerialColon) + if err != nil { + t.Fatal(err) + } + if secret != nil { + t.Fatalf("expected empty response data, got: %#v", secret.Data) + } + +} + var ( initTest sync.Once rsaCAKey string diff --git a/builtin/logical/pki/path_tidy.go b/builtin/logical/pki/path_tidy.go index a98c14534ef0..2a03984402ae 100644 --- a/builtin/logical/pki/path_tidy.go +++ b/builtin/logical/pki/path_tidy.go @@ -178,7 +178,13 @@ func (b *backend) pathTidyWrite(ctx context.Context, req *logical.Request, d *fr return errwrap.Wrapf(fmt.Sprintf("unable to parse stored revoked certificate with serial %q: {{err}}", serial), err) } - if time.Now().After(revokedCert.NotAfter.Add(bufferDuration)) { + // Remove the matched certificate entries from revoked/ and + // cert/ paths. We compare against both the NotAfter time + // within the cert itself and the time from the revocation + // entry, and perform tidy if either one tells us that the + // certificate has already been revoked. + now := time.Now() + if now.After(revokedCert.NotAfter.Add(bufferDuration)) || now.After(revInfo.RevocationTimeUTC.Add(bufferDuration)) { if err := req.Storage.Delete(ctx, "revoked/"+serial); err != nil { return errwrap.Wrapf(fmt.Sprintf("error deleting serial %q from revoked list: {{err}}", serial), err) }