Remove certificates from store if tidying revoked certificates (#5231)

This will cause them to be removed even if they have not expired yet,
whereas before it would simply leave them in the store until they were
expired, but remove from revocation info.

builtin/logical/pki/path_tidy.go

@@ -23,17 +23,12 @@ func pathTidy(b *backend) *framework.Path {
 the certificate store`,
 			},
 
-			"tidy_revocation_list": &framework.FieldSchema{
-				Type: framework.TypeBool,
-				Description: `Set to true to enable tidying up
-the revocation list`,
-			},
-
 			"tidy_revoked_certs": &framework.FieldSchema{
 				Type: framework.TypeBool,
 				Description: `Set to true to expire all revoked
-certificates, even if their duration has not yet passed. This will cause these
-certificates to be removed from the CRL the next time the CRL is generated.`,
+certificates, even if their duration has not yet passed, removing
+them both from the CRL and from storage. The CRL will be rotated
+if this causes any values to be removed.`,
 			},
 
 			"safety_buffer": &framework.FieldSchema{
@@ -58,7 +53,6 @@ Defaults to 72 hours.`,
 func (b *backend) pathTidyWrite(ctx context.Context, req *logical.Request, d *framework.FieldData) (*logical.Response, error) {
 	safetyBuffer := d.Get("safety_buffer").(int)
 	tidyCertStore := d.Get("tidy_cert_store").(bool)
-	tidyRevocationList := d.Get("tidy_revocation_list").(bool)
 	tidyRevokedCerts := d.Get("tidy_revoked_certs").(bool)
 
 	if safetyBuffer < 1 {
@@ -127,7 +121,7 @@ func (b *backend) pathTidyWrite(ctx context.Context, req *logical.Request, d *fr
 				}
 			}
 
-			if tidyRevocationList {
+			if tidyRevokedCerts {
 				b.revokeStorageLock.Lock()
 				defer b.revokeStorageLock.Unlock()
 
@@ -169,10 +163,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 tidyRevokedCerts || time.Now().After(revokedCert.NotAfter.Add(bufferDuration)) {
+					if time.Now().After(revokedCert.NotAfter.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)
 						}
+						if err := req.Storage.Delete(ctx, "certs/"+serial); err != nil {
+							return errwrap.Wrapf(fmt.Sprintf("error deleting serial %q from store when tidying revoked: {{err}}", serial), err)
+						}
 						tidiedRevoked = true
 					}
 				}

website/source/api/secret/pki/index.html.md

@@ -1539,12 +1539,10 @@ expiration time.
 - `tidy_cert_store` `(bool: false)` Specifies whether to tidy up the certificate
   store.
 
-- `tidy_revocation_list` `(bool: false)` Specifies whether to tidy up the
-  revocation list (CRL).
-
 - `tidy_revoked_certs` `(bool: false)` Set to true to expire all revoked
-  certificates, even if their duration has not yet passed. This will cause these
-  certificates to be removed from the CRL the next time the CRL is generated.
+  certificates, even if their duration has not yet passed, removing them both
+  from the CRL and from storage. The CRL will be rotated if this causes any
+  values to be removed.
 
 - `safety_buffer` `(string: "")` Specifies  A duration (given as an integer
   number of seconds or a string; defaults to `72h`) used as a safety buffer to