Fix ACME tidy to not reference acmeContext (#21870)

* Fix ACME tidy to not reference acmeCtx acmeContext is useful for when we need to reference things with a ACME base URL, but everything used in tidy doesn't need this URL as it is not coming from an ACME request. Refactor tidy to remove references to acmeContext, including dependent functions in acme_state.go. Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com> * Remove spurious log message Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com> * Draft Tidy Acme Test with Backdate Storage + Backdate Sysxsx * Fixes to ACME tidy testing Co-authored-by: kitography <khaines@mit.edu> Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com> * Correctly set account kid to update account status Co-authored-by: kitography <khaines@mit.edu> Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com> * Add TestTidyAcmeWithSafetyBuffer Co-authored-by: kitography <khaines@mit.edu> Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com> * Add test for disabling tidy operation Co-authored-by: kitography <khaines@mit.edu> Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com> * Add acme_account_safety_buffer to auto-tidy config Resolve: #21872 Co-authored-by: kitography <khaines@mit.edu> Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com> * Add tests verifying tidy safety buffers Co-authored-by: kitography <khaines@mit.edu> Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com> * Add changelog entry Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com> * Add account status validations and order cleanup tests --------- Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com> Co-authored-by: kitography <khaines@mit.edu> Co-authored-by: Steve Clark <steven.clark@hashicorp.com>
2025-10-29 01:32:33 +00:00 · 2023-07-17 13:54:28 -05:00
parent d79190808e
commit 4ec5e22ade
7 changed files with 546 additions and 33 deletions
--- a/builtin/logical/pki/acme_challenges_test.go
+++ b/builtin/logical/pki/acme_challenges_test.go
@@ -279,7 +279,6 @@ func TestAcmeValidateTLSALPN01Challenge(t *testing.T) {

 		ctx, cancel := context.WithTimeout(context.Background(), 1*time.Minute)
 		defer func() {
-			t.Logf("[alpn-server] defer context cancel executing")
 			cancel()
 		}()

--- a/builtin/logical/pki/acme_state.go
+++ b/builtin/logical/pki/acme_state.go
@@ -277,13 +277,13 @@ func (a *acmeState) CreateAccount(ac *acmeContext, c *jwsCtx, contact []string,
 	return acct, nil
 }

-func (a *acmeState) UpdateAccount(ac *acmeContext, acct *acmeAccount) error {
+func (a *acmeState) UpdateAccount(sc *storageContext, acct *acmeAccount) error {
 	json, err := logical.StorageEntryJSON(acmeAccountPrefix+acct.KeyId, acct)
 	if err != nil {
 		return fmt.Errorf("error creating account entry: %w", err)
 	}

-	if err := ac.sc.Storage.Put(ac.sc.Context, json); err != nil {
+	if err := sc.Storage.Put(sc.Context, json); err != nil {
 		return fmt.Errorf("error writing account entry: %w", err)
 	}

@@ -539,10 +539,10 @@ func (a *acmeState) SaveOrder(ac *acmeContext, order *acmeOrder) error {
 	return nil
 }

-func (a *acmeState) ListOrderIds(ac *acmeContext, accountId string) ([]string, error) {
+func (a *acmeState) ListOrderIds(sc *storageContext, accountId string) ([]string, error) {
 	accountOrderPrefixPath := acmeAccountPrefix + accountId + "/orders/"

-	rawOrderIds, err := ac.sc.Storage.List(ac.sc.Context, accountOrderPrefixPath)
+	rawOrderIds, err := sc.Storage.List(sc.Context, accountOrderPrefixPath)
 	if err != nil {
 		return nil, fmt.Errorf("failed listing order ids for account %s: %w", accountId, err)
 	}
--- a/builtin/logical/pki/path_acme_account.go
+++ b/builtin/logical/pki/path_acme_account.go
@@ -356,7 +356,7 @@ func (b *backend) acmeNewAccountUpdateHandler(acmeCtx *acmeContext, userCtx *jws
 	}

 	if shouldUpdate {
-		err = b.acmeState.UpdateAccount(acmeCtx, account)
+		err = b.acmeState.UpdateAccount(acmeCtx.sc, account)
 		if err != nil {
 			return nil, fmt.Errorf("failed to update account: %w", err)
 		}
@@ -366,8 +366,8 @@ func (b *backend) acmeNewAccountUpdateHandler(acmeCtx *acmeContext, userCtx *jws
 	return resp, nil
 }

-func (b *backend) tidyAcmeAccountByThumbprint(as *acmeState, ac *acmeContext, keyThumbprint string, certTidyBuffer, accountTidyBuffer time.Duration) error {
-	thumbprintEntry, err := ac.sc.Storage.Get(ac.sc.Context, path.Join(acmeThumbprintPrefix, keyThumbprint))
+func (b *backend) tidyAcmeAccountByThumbprint(as *acmeState, sc *storageContext, keyThumbprint string, certTidyBuffer, accountTidyBuffer time.Duration) error {
+	thumbprintEntry, err := sc.Storage.Get(sc.Context, path.Join(acmeThumbprintPrefix, keyThumbprint))
 	if err != nil {
 		return fmt.Errorf("error retrieving thumbprint entry %v, unable to find corresponding account entry: %w", keyThumbprint, err)
 	}
@@ -386,13 +386,13 @@ func (b *backend) tidyAcmeAccountByThumbprint(as *acmeState, ac *acmeContext, ke
 	}

 	// Now Get the Account:
-	accountEntry, err := ac.sc.Storage.Get(ac.sc.Context, acmeAccountPrefix+thumbprint.Kid)
+	accountEntry, err := sc.Storage.Get(sc.Context, acmeAccountPrefix+thumbprint.Kid)
 	if err != nil {
 		return err
 	}
 	if accountEntry == nil {
 		// We delete the Thumbprint Associated with the Account, and we are done
-		err = ac.sc.Storage.Delete(ac.sc.Context, path.Join(acmeThumbprintPrefix, keyThumbprint))
+		err = sc.Storage.Delete(sc.Context, path.Join(acmeThumbprintPrefix, keyThumbprint))
 		if err != nil {
 			return err
 		}
@@ -405,16 +405,17 @@ func (b *backend) tidyAcmeAccountByThumbprint(as *acmeState, ac *acmeContext, ke
 	if err != nil {
 		return err
 	}
+	account.KeyId = thumbprint.Kid

 	// Tidy Orders On the Account
-	orderIds, err := as.ListOrderIds(ac, thumbprint.Kid)
+	orderIds, err := as.ListOrderIds(sc, thumbprint.Kid)
 	if err != nil {
 		return err
 	}
 	allOrdersTidied := true
 	maxCertExpiryUpdated := false
 	for _, orderId := range orderIds {
-		wasTidied, orderExpiry, err := b.acmeTidyOrder(ac, thumbprint.Kid, getOrderPath(thumbprint.Kid, orderId), certTidyBuffer)
+		wasTidied, orderExpiry, err := b.acmeTidyOrder(sc, thumbprint.Kid, getOrderPath(thumbprint.Kid, orderId), certTidyBuffer)
 		if err != nil {
 			return err
 		}
@@ -436,13 +437,13 @@ func (b *backend) tidyAcmeAccountByThumbprint(as *acmeState, ac *acmeContext, ke
 		// If it is Revoked or Deactivated:
 		if (account.Status == AccountStatusRevoked || account.Status == AccountStatusDeactivated) && now.After(account.AccountRevokedDate.Add(accountTidyBuffer)) {
 			// We Delete the Account Associated with this Thumbprint:
-			err = ac.sc.Storage.Delete(ac.sc.Context, path.Join(acmeAccountPrefix, thumbprint.Kid))
+			err = sc.Storage.Delete(sc.Context, path.Join(acmeAccountPrefix, thumbprint.Kid))
 			if err != nil {
 				return err
 			}

 			// Now we delete the Thumbprint Associated with the Account:
-			err = ac.sc.Storage.Delete(ac.sc.Context, path.Join(acmeThumbprintPrefix, keyThumbprint))
+			err = sc.Storage.Delete(sc.Context, path.Join(acmeThumbprintPrefix, keyThumbprint))
 			if err != nil {
 				return err
 			}
@@ -451,7 +452,7 @@ func (b *backend) tidyAcmeAccountByThumbprint(as *acmeState, ac *acmeContext, ke
 			// Revoke This Account
 			account.AccountRevokedDate = now
 			account.Status = AccountStatusRevoked
-			err := as.UpdateAccount(ac, &account)
+			err := as.UpdateAccount(sc, &account)
 			if err != nil {
 				return err
 			}
@@ -464,7 +465,7 @@ func (b *backend) tidyAcmeAccountByThumbprint(as *acmeState, ac *acmeContext, ke
 	// already written above.
 	if maxCertExpiryUpdated && account.Status == AccountStatusValid {
 		// Update our expiry time we previously setup.
-		err := as.UpdateAccount(ac, &account)
+		err := as.UpdateAccount(sc, &account)
 		if err != nil {
 			return err
 		}
--- a/builtin/logical/pki/path_acme_order.go
+++ b/builtin/logical/pki/path_acme_order.go
@@ -681,7 +681,7 @@ func (b *backend) acmeGetOrderHandler(ac *acmeContext, _ *logical.Request, field
 }

 func (b *backend) acmeListOrdersHandler(ac *acmeContext, _ *logical.Request, _ *framework.FieldData, uc *jwsCtx, _ map[string]interface{}, acct *acmeAccount) (*logical.Response, error) {
-	orderIds, err := b.acmeState.ListOrderIds(ac, acct.KeyId)
+	orderIds, err := b.acmeState.ListOrderIds(ac.sc, acct.KeyId)
 	if err != nil {
 		return nil, err
 	}
@@ -1020,11 +1020,11 @@ func parseOrderIdentifiers(data map[string]interface{}) ([]*ACMEIdentifier, erro
 	return identifiers, nil
 }

-func (b *backend) acmeTidyOrder(ac *acmeContext, accountId string, orderPath string, certTidyBuffer time.Duration) (bool, time.Time, error) {
+func (b *backend) acmeTidyOrder(sc *storageContext, accountId string, orderPath string, certTidyBuffer time.Duration) (bool, time.Time, error) {
 	// First we get the order; note that the orderPath includes the account
 	// It's only accessed at acme/orders/<order_id> with the account context
 	// It's saved at acme/<account_id>/orders/<orderId>
-	entry, err := ac.sc.Storage.Get(ac.sc.Context, orderPath)
+	entry, err := sc.Storage.Get(sc.Context, orderPath)
 	if err != nil {
 		return false, time.Time{}, fmt.Errorf("error loading order: %w", err)
 	}
@@ -1069,20 +1069,20 @@ func (b *backend) acmeTidyOrder(ac *acmeContext, accountId string, orderPath str

 	// First Authorizations
 	for _, authorizationId := range order.AuthorizationIds {
-		err = ac.sc.Storage.Delete(ac.sc.Context, getAuthorizationPath(accountId, authorizationId))
+		err = sc.Storage.Delete(sc.Context, getAuthorizationPath(accountId, authorizationId))
 		if err != nil {
 			return false, orderExpiry, err
 		}
 	}

 	// Normal Tidy will Take Care of the Certificate, we need to clean up the certificate to account tracker though
-	err = ac.sc.Storage.Delete(ac.sc.Context, getAcmeSerialToAccountTrackerPath(accountId, order.CertificateSerialNumber))
+	err = sc.Storage.Delete(sc.Context, getAcmeSerialToAccountTrackerPath(accountId, order.CertificateSerialNumber))
 	if err != nil {
 		return false, orderExpiry, err
 	}

 	// And Finally, the order:
-	err = ac.sc.Storage.Delete(ac.sc.Context, orderPath)
+	err = sc.Storage.Delete(sc.Context, orderPath)
 	if err != nil {
 		return false, orderExpiry, err
 	}
--- a/builtin/logical/pki/path_tidy.go
+++ b/builtin/logical/pki/path_tidy.go
@@ -1546,18 +1546,8 @@ func (b *backend) doTidyAcme(ctx context.Context, req *logical.Request, logger h
 	b.tidyStatus.acmeAccountsCount = uint(len(thumbprints))
 	b.tidyStatusLock.Unlock()

-	baseUrl, _, err := getAcmeBaseUrl(sc, req)
-	if err != nil {
-		return err
-	}
-
-	acmeCtx := &acmeContext{
-		baseUrl: baseUrl,
-		sc:      sc,
-	}
-
 	for _, thumbprint := range thumbprints {
-		err := b.tidyAcmeAccountByThumbprint(b.acmeState, acmeCtx, thumbprint, config.SafetyBuffer, config.AcmeAccountSafetyBuffer)
+		err := b.tidyAcmeAccountByThumbprint(b.acmeState, sc, thumbprint, config.SafetyBuffer, config.AcmeAccountSafetyBuffer)
 		if err != nil {
 			logger.Warn("error tidying account %v: %v", thumbprint, err.Error())
 		}
@@ -1838,6 +1828,13 @@ func (b *backend) pathConfigAutoTidyWrite(ctx context.Context, req *logical.Requ
 		config.TidyAcme = tidyAcmeRaw.(bool)
 	}

+	if acmeAccountSafetyBufferRaw, ok := d.GetOk("acme_account_safety_buffer"); ok {
+		config.AcmeAccountSafetyBuffer = time.Duration(acmeAccountSafetyBufferRaw.(int)) * time.Second
+		if config.AcmeAccountSafetyBuffer < 1*time.Second {
+			return logical.ErrorResponse(fmt.Sprintf("given acme_account_safety_buffer must be at least one second; got: %v", acmeAccountSafetyBufferRaw)), nil
+		}
+	}
+
 	if config.Enabled && !config.IsAnyTidyEnabled() {
 		return logical.ErrorResponse("Auto-tidy enabled but no tidy operations were requested. Enable at least one tidy operation to be run (" + config.AnyTidyConfig() + ")."), nil
 	}
--- a/builtin/logical/pki/path_tidy_test.go
+++ b/builtin/logical/pki/path_tidy_test.go
@@ -4,13 +4,24 @@
 package pki

 import (
+	"context"
+	"crypto/ecdsa"
+	"crypto/elliptic"
+	"crypto/rand"
+	"crypto/rsa"
+	"crypto/x509"
+	"encoding/base64"
 	"encoding/json"
 	"errors"
 	"fmt"
+	"path"
 	"strings"
 	"testing"
 	"time"

+	"github.com/hashicorp/vault/sdk/helper/jsonutil"
+	"golang.org/x/crypto/acme"
+
 	"github.com/hashicorp/vault/helper/testhelpers"
 	"github.com/hashicorp/vault/sdk/helper/testhelpers/schema"

@@ -29,8 +40,11 @@ func TestTidyConfigs(t *testing.T) {

 	var cfg tidyConfig
 	operations := strings.Split(cfg.AnyTidyConfig(), " / ")
+	require.Greater(t, len(operations), 1, "expected more than one operation")
 	t.Logf("Got tidy operations: %v", operations)

+	lastOp := operations[len(operations)-1]
+
 	for _, operation := range operations {
 		b, s := CreateBackendWithStorage(t)

@@ -44,6 +58,17 @@ func TestTidyConfigs(t *testing.T) {
 		requireSuccessNonNilResponse(t, resp, err, "expected to be able to read auto-tidy operation for operation "+operation)
 		require.True(t, resp.Data[operation].(bool), "expected operation to be enabled after reading auto-tidy config "+operation)

+		resp, err = CBWrite(b, s, "config/auto-tidy", map[string]interface{}{
+			"enabled": true,
+			operation: false,
+			lastOp:    true,
+		})
+		requireSuccessNonNilResponse(t, resp, err, "expected to be able to disable auto-tidy operation "+operation)
+
+		resp, err = CBRead(b, s, "config/auto-tidy")
+		requireSuccessNonNilResponse(t, resp, err, "expected to be able to read auto-tidy operation for operation "+operation)
+		require.False(t, resp.Data[operation].(bool), "expected operation to be disabled after reading auto-tidy config "+operation)
+
 		resp, err = CBWrite(b, s, "tidy", map[string]interface{}{
 			operation: true,
 		})
@@ -56,6 +81,93 @@ func TestTidyConfigs(t *testing.T) {
 				}
 			}
 		}
+
+		lastOp = operation
+	}
+
+	// pause_duration is tested elsewhere in other tests.
+	type configSafetyBufferValueStr struct {
+		Config       string
+		FirstValue   int
+		SecondValue  int
+		DefaultValue int
+	}
+	configSafetyBufferValues := []configSafetyBufferValueStr{
+		{
+			Config:       "safety_buffer",
+			FirstValue:   1,
+			SecondValue:  2,
+			DefaultValue: int(defaultTidyConfig.SafetyBuffer / time.Second),
+		},
+		{
+			Config:       "issuer_safety_buffer",
+			FirstValue:   1,
+			SecondValue:  2,
+			DefaultValue: int(defaultTidyConfig.IssuerSafetyBuffer / time.Second),
+		},
+		{
+			Config:       "acme_account_safety_buffer",
+			FirstValue:   1,
+			SecondValue:  2,
+			DefaultValue: int(defaultTidyConfig.AcmeAccountSafetyBuffer / time.Second),
+		},
+		{
+			Config:       "revocation_queue_safety_buffer",
+			FirstValue:   1,
+			SecondValue:  2,
+			DefaultValue: int(defaultTidyConfig.QueueSafetyBuffer / time.Second),
+		},
+	}
+
+	for _, flag := range configSafetyBufferValues {
+		b, s := CreateBackendWithStorage(t)
+
+		resp, err := CBRead(b, s, "config/auto-tidy")
+		requireSuccessNonNilResponse(t, resp, err, "expected to be able to read auto-tidy operation for flag "+flag.Config)
+		require.Equal(t, resp.Data[flag.Config].(int), flag.DefaultValue, "expected initial auto-tidy config to match default value for "+flag.Config)
+
+		resp, err = CBWrite(b, s, "config/auto-tidy", map[string]interface{}{
+			"enabled":         true,
+			"tidy_cert_store": true,
+			flag.Config:       flag.FirstValue,
+		})
+		requireSuccessNonNilResponse(t, resp, err, "expected to be able to set auto-tidy config option "+flag.Config)
+
+		resp, err = CBRead(b, s, "config/auto-tidy")
+		requireSuccessNonNilResponse(t, resp, err, "expected to be able to read auto-tidy operation for config "+flag.Config)
+		require.Equal(t, resp.Data[flag.Config].(int), flag.FirstValue, "expected value to be set after reading auto-tidy config "+flag.Config)
+
+		resp, err = CBWrite(b, s, "config/auto-tidy", map[string]interface{}{
+			"enabled":         true,
+			"tidy_cert_store": true,
+			flag.Config:       flag.SecondValue,
+		})
+		requireSuccessNonNilResponse(t, resp, err, "expected to be able to set auto-tidy config option "+flag.Config)
+
+		resp, err = CBRead(b, s, "config/auto-tidy")
+		requireSuccessNonNilResponse(t, resp, err, "expected to be able to read auto-tidy operation for config "+flag.Config)
+		require.Equal(t, resp.Data[flag.Config].(int), flag.SecondValue, "expected value to be set after reading auto-tidy config "+flag.Config)
+
+		resp, err = CBWrite(b, s, "tidy", map[string]interface{}{
+			"tidy_cert_store": true,
+			flag.Config:       flag.FirstValue,
+		})
+		t.Logf("tidy run results: resp=%v/err=%v", resp, err)
+		requireSuccessNonNilResponse(t, resp, err, "expected to be able to start tidy operation with "+flag.Config)
+		if len(resp.Warnings) > 0 {
+			for _, warning := range resp.Warnings {
+				if strings.Contains(warning, "unrecognized parameter") && strings.Contains(warning, flag.Config) {
+					t.Fatalf("warning '%v' claims parameter '%v' is unknown", warning, flag.Config)
+				}
+			}
+		}
+
+		time.Sleep(2 * time.Second)
+
+		resp, err = CBRead(b, s, "tidy-status")
+		requireSuccessNonNilResponse(t, resp, err, "expected to be able to start tidy operation with "+flag.Config)
+		t.Logf("got response: %v for config: %v", resp, flag.Config)
+		require.Equal(t, resp.Data[flag.Config].(int), flag.FirstValue, "expected flag to be set in tidy-status for config "+flag.Config)
 	}
 }

@@ -810,3 +922,401 @@ func TestCertStorageMetrics(t *testing.T) {
 		return nil
 	})
 }
+
+// This test uses the default safety buffer with backdating.
+func TestTidyAcmeWithBackdate(t *testing.T) {
+	t.Parallel()
+
+	cluster, client, _ := setupAcmeBackend(t)
+	defer cluster.Cleanup()
+	testCtx := context.Background()
+
+	// Grab the mount UUID for sys/raw invocations.
+	pkiMount := findStorageMountUuid(t, client, "pki")
+
+	// Register an Account, do nothing with it
+	baseAcmeURL := "/v1/pki/acme/"
+	accountKey, err := rsa.GenerateKey(rand.Reader, 2048)
+	require.NoError(t, err, "failed creating rsa key")
+
+	acmeClient := getAcmeClientForCluster(t, cluster, baseAcmeURL, accountKey)
+
+	// Create new account with order/cert
+	t.Logf("Testing register on %s", baseAcmeURL)
+	acct, err := acmeClient.Register(testCtx, &acme.Account{}, func(tosURL string) bool { return true })
+	t.Logf("got account URI: %v", acct.URI)
+	require.NoError(t, err, "failed registering account")
+	identifiers := []string{"*.localdomain"}
+	order, err := acmeClient.AuthorizeOrder(testCtx, []acme.AuthzID{
+		{Type: "dns", Value: identifiers[0]},
+	})
+	require.NoError(t, err, "failed creating order")
+
+	// HACK: Update authorization/challenge to completed as we can't really do it properly in this workflow test.
+	markAuthorizationSuccess(t, client, acmeClient, acct, order)
+
+	goodCr := &x509.CertificateRequest{DNSNames: []string{identifiers[0]}}
+	csrKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
+	require.NoError(t, err, "failed generated key for CSR")
+	csr, err := x509.CreateCertificateRequest(rand.Reader, goodCr, csrKey)
+	require.NoError(t, err, "failed generating csr")
+	certs, _, err := acmeClient.CreateOrderCert(testCtx, order.FinalizeURL, csr, true)
+	require.NoError(t, err, "order finalization failed")
+	require.GreaterOrEqual(t, len(certs), 1, "expected at least one cert in bundle")
+
+	acmeCert, err := x509.ParseCertificate(certs[0])
+	require.NoError(t, err, "failed parsing acme cert")
+
+	// -> Ensure we see it in storage. Since we don't have direct storage
+	// access, use sys/raw interface.
+	acmeThumbprintsPath := path.Join("sys/raw/logical", pkiMount, acmeThumbprintPrefix)
+	listResp, err := client.Logical().ListWithContext(testCtx, acmeThumbprintsPath)
+	require.NoError(t, err, "failed listing ACME thumbprints")
+	require.NotEmpty(t, listResp.Data["keys"], "expected non-empty list response")
+
+	// Run Tidy
+	_, err = client.Logical().Write("pki/tidy", map[string]interface{}{
+		"tidy_acme": true,
+	})
+	require.NoError(t, err)
+
+	// Wait for tidy to finish.
+	waitForTidyToFinish(t, client, "pki")
+
+	// Check that the Account is Still There, Still Valid.
+	account, err := acmeClient.GetReg(context.Background(), "" /* legacy unused param*/)
+	require.NoError(t, err, "received account looking up acme account")
+	require.Equal(t, acme.StatusValid, account.Status)
+
+	// Find the associated thumbprint
+	listResp, err = client.Logical().ListWithContext(testCtx, acmeThumbprintsPath)
+	require.NoError(t, err)
+	require.NotNil(t, listResp)
+	thumbprintEntries := listResp.Data["keys"].([]interface{})
+	require.Equal(t, len(thumbprintEntries), 1)
+	thumbprint := thumbprintEntries[0].(string)
+
+	// Let "Time Pass"; this is a HACK, this function sys-writes to overwrite the date on objects in storage
+	duration := time.Until(acmeCert.NotAfter) + 31*24*time.Hour
+	accountId := acmeClient.KID[strings.LastIndex(string(acmeClient.KID), "/")+1:]
+	orderId := order.URI[strings.LastIndex(order.URI, "/")+1:]
+	backDateAcmeOrderSys(t, testCtx, client, string(accountId), orderId, duration, pkiMount)
+
+	// Run Tidy -> clean up order
+	_, err = client.Logical().Write("pki/tidy", map[string]interface{}{
+		"tidy_acme": true,
+	})
+	require.NoError(t, err)
+
+	// Wait for tidy to finish.
+	tidyResp := waitForTidyToFinish(t, client, "pki")
+
+	require.Equal(t, tidyResp.Data["acme_orders_deleted_count"], json.Number("1"),
+		"expected to revoke a single ACME order: %v", tidyResp)
+	require.Equal(t, tidyResp.Data["acme_account_revoked_count"], json.Number("0"),
+		"no ACME account should have been revoked: %v", tidyResp)
+	require.Equal(t, tidyResp.Data["acme_account_deleted_count"], json.Number("0"),
+		"no ACME account should have been revoked: %v", tidyResp)
+
+	// Make sure our order is indeed deleted.
+	_, err = acmeClient.GetOrder(context.Background(), order.URI)
+	require.ErrorContains(t, err, "order does not exist")
+
+	// Check that the Account is Still There, Still Valid.
+	account, err = acmeClient.GetReg(context.Background(), "" /* legacy unused param*/)
+	require.NoError(t, err, "received account looking up acme account")
+	require.Equal(t, acme.StatusValid, account.Status)
+
+	// Now back date the account to make sure we revoke it
+	backDateAcmeAccountSys(t, testCtx, client, thumbprint, duration, pkiMount)
+
+	// Run Tidy -> mark account revoked
+	_, err = client.Logical().Write("pki/tidy", map[string]interface{}{
+		"tidy_acme": true,
+	})
+	require.NoError(t, err)
+
+	// Wait for tidy to finish.
+	tidyResp = waitForTidyToFinish(t, client, "pki")
+	require.Equal(t, tidyResp.Data["acme_orders_deleted_count"], json.Number("0"),
+		"no ACME orders should have been deleted: %v", tidyResp)
+	require.Equal(t, tidyResp.Data["acme_account_revoked_count"], json.Number("1"),
+		"expected to revoke a single ACME account: %v", tidyResp)
+	require.Equal(t, tidyResp.Data["acme_account_deleted_count"], json.Number("0"),
+		"no ACME account should have been revoked: %v", tidyResp)
+
+	// Lookup our account to make sure we get the appropriate revoked status
+	account, err = acmeClient.GetReg(context.Background(), "" /* legacy unused param*/)
+	require.NoError(t, err, "received account looking up acme account")
+	require.Equal(t, acme.StatusRevoked, account.Status)
+
+	// Let "Time Pass"; this is a HACK, this function sys-writes to overwrite the date on objects in storage
+	backDateAcmeAccountSys(t, testCtx, client, thumbprint, duration, pkiMount)
+
+	// Run Tidy -> remove account
+	_, err = client.Logical().Write("pki/tidy", map[string]interface{}{
+		"tidy_acme": true,
+	})
+	require.NoError(t, err)
+
+	// Wait for tidy to finish.
+	waitForTidyToFinish(t, client, "pki")
+
+	// Check Account No Longer Appears
+	listResp, err = client.Logical().ListWithContext(testCtx, acmeThumbprintsPath)
+	require.NoError(t, err)
+	if listResp != nil {
+		thumbprintEntries = listResp.Data["keys"].([]interface{})
+		require.Equal(t, 0, len(thumbprintEntries))
+	}
+
+	// Nor Under Account
+	_, acctKID := path.Split(acct.URI)
+	acctPath := path.Join("sys/raw/logical", pkiMount, acmeAccountPrefix, acctKID)
+	t.Logf("account path: %v", acctPath)
+	getResp, err := client.Logical().ReadWithContext(testCtx, acctPath)
+	require.NoError(t, err)
+	require.Nil(t, getResp)
+}
+
+// This test uses a smaller safety buffer.
+func TestTidyAcmeWithSafetyBuffer(t *testing.T) {
+	t.Parallel()
+
+	// This would still be way easier if I could do both sides
+	cluster, client, _ := setupAcmeBackend(t)
+	defer cluster.Cleanup()
+	testCtx := context.Background()
+
+	// Grab the mount UUID for sys/raw invocations.
+	pkiMount := findStorageMountUuid(t, client, "pki")
+
+	// Register an Account, do nothing with it
+	baseAcmeURL := "/v1/pki/acme/"
+	accountKey, err := rsa.GenerateKey(rand.Reader, 2048)
+	require.NoError(t, err, "failed creating rsa key")
+
+	acmeClient := getAcmeClientForCluster(t, cluster, baseAcmeURL, accountKey)
+
+	// Create new account
+	t.Logf("Testing register on %s", baseAcmeURL)
+	acct, err := acmeClient.Register(testCtx, &acme.Account{}, func(tosURL string) bool { return true })
+	t.Logf("got account URI: %v", acct.URI)
+	require.NoError(t, err, "failed registering account")
+
+	// -> Ensure we see it in storage. Since we don't have direct storage
+	// access, use sys/raw interface.
+	acmeThumbprintsPath := path.Join("sys/raw/logical", pkiMount, acmeThumbprintPrefix)
+	listResp, err := client.Logical().ListWithContext(testCtx, acmeThumbprintsPath)
+	require.NoError(t, err, "failed listing ACME thumbprints")
+	require.NotEmpty(t, listResp.Data["keys"], "expected non-empty list response")
+	thumbprintEntries := listResp.Data["keys"].([]interface{})
+	require.Equal(t, len(thumbprintEntries), 1)
+
+	// Wait for the account to expire.
+	time.Sleep(2 * time.Second)
+
+	// Run Tidy -> mark account revoked
+	_, err = client.Logical().Write("pki/tidy", map[string]interface{}{
+		"tidy_acme":                  true,
+		"acme_account_safety_buffer": "1s",
+	})
+	require.NoError(t, err)
+
+	// Wait for tidy to finish.
+	statusResp := waitForTidyToFinish(t, client, "pki")
+	require.Equal(t, statusResp.Data["acme_account_revoked_count"], json.Number("1"), "expected to revoke a single ACME account")
+
+	// Wait for the account to expire.
+	time.Sleep(2 * time.Second)
+
+	// Run Tidy -> remove account
+	_, err = client.Logical().Write("pki/tidy", map[string]interface{}{
+		"tidy_acme":                  true,
+		"acme_account_safety_buffer": "1s",
+	})
+	require.NoError(t, err)
+
+	// Wait for tidy to finish.
+	waitForTidyToFinish(t, client, "pki")
+
+	// Check Account No Longer Appears
+	listResp, err = client.Logical().ListWithContext(testCtx, acmeThumbprintsPath)
+	require.NoError(t, err)
+	if listResp != nil {
+		thumbprintEntries = listResp.Data["keys"].([]interface{})
+		require.Equal(t, 0, len(thumbprintEntries))
+	}
+
+	// Nor Under Account
+	_, acctKID := path.Split(acct.URI)
+	acctPath := path.Join("sys/raw/logical", pkiMount, acmeAccountPrefix, acctKID)
+	t.Logf("account path: %v", acctPath)
+	getResp, err := client.Logical().ReadWithContext(testCtx, acctPath)
+	require.NoError(t, err)
+	require.Nil(t, getResp)
+}
+
+// The sys tests refer to all of the tests using sys/raw/logical which work off of a client
+func backDateAcmeAccountSys(t *testing.T, testContext context.Context, client *api.Client, thumbprintString string, backdateAmount time.Duration, mount string) {
+	rawThumbprintPath := path.Join("sys/raw/logical/", mount, acmeThumbprintPrefix+thumbprintString)
+	thumbprintResp, err := client.Logical().ReadWithContext(testContext, rawThumbprintPath)
+	if err != nil {
+		t.Fatalf("unable to fetch thumbprint response at %v: %v", rawThumbprintPath, err)
+	}
+
+	var thumbprint acmeThumbprint
+	err = jsonutil.DecodeJSON([]byte(thumbprintResp.Data["value"].(string)), &thumbprint)
+	if err != nil {
+		t.Fatalf("unable to decode thumbprint response %v to find account entry: %v", thumbprintResp.Data, err)
+	}
+
+	accountPath := path.Join("sys/raw/logical", mount, acmeAccountPrefix+thumbprint.Kid)
+	accountResp, err := client.Logical().ReadWithContext(testContext, accountPath)
+	if err != nil {
+		t.Fatalf("unable to fetch account entry %v: %v", thumbprint.Kid, err)
+	}
+
+	var account acmeAccount
+	err = jsonutil.DecodeJSON([]byte(accountResp.Data["value"].(string)), &account)
+	if err != nil {
+		t.Fatalf("unable to decode acme account %v: %v", accountResp, err)
+	}
+
+	t.Logf("got account before update: %v", account)
+
+	account.AccountCreatedDate = backDate(account.AccountCreatedDate, backdateAmount)
+	account.MaxCertExpiry = backDate(account.MaxCertExpiry, backdateAmount)
+	account.AccountRevokedDate = backDate(account.AccountRevokedDate, backdateAmount)
+
+	t.Logf("got account after update: %v", account)
+
+	encodeJSON, err := jsonutil.EncodeJSON(account)
+	_, err = client.Logical().WriteWithContext(context.Background(), accountPath, map[string]interface{}{
+		"value":    base64.StdEncoding.EncodeToString(encodeJSON),
+		"encoding": "base64",
+	})
+	if err != nil {
+		t.Fatalf("error saving backdated account entry at %v: %v", accountPath, err)
+	}
+
+	ordersPath := path.Join("sys/raw/logical", mount, acmeAccountPrefix, thumbprint.Kid, "/orders/")
+	ordersRaw, err := client.Logical().ListWithContext(context.Background(), ordersPath)
+	require.NoError(t, err, "failed listing orders")
+
+	if ordersRaw == nil {
+		t.Logf("skipping backdating orders as there are none")
+		return
+	}
+
+	require.NotNil(t, ordersRaw, "got no response data")
+	require.NotNil(t, ordersRaw.Data, "got no response data")
+
+	orders := ordersRaw.Data
+
+	for _, orderId := range orders["keys"].([]interface{}) {
+		backDateAcmeOrderSys(t, testContext, client, thumbprint.Kid, orderId.(string), backdateAmount, mount)
+	}
+
+	// No need to change certificates entries here - no time is stored on AcmeCertEntry
+}
+
+func backDateAcmeOrderSys(t *testing.T, testContext context.Context, client *api.Client, accountKid string, orderId string, backdateAmount time.Duration, mount string) {
+	rawOrderPath := path.Join("sys/raw/logical/", mount, acmeAccountPrefix, accountKid, "orders", orderId)
+	orderResp, err := client.Logical().ReadWithContext(testContext, rawOrderPath)
+	if err != nil {
+		t.Fatalf("unable to fetch order entry %v on account %v at %v", orderId, accountKid, rawOrderPath)
+	}
+
+	var order *acmeOrder
+	err = jsonutil.DecodeJSON([]byte(orderResp.Data["value"].(string)), &order)
+	if err != nil {
+		t.Fatalf("error decoding order entry %v on account %v, %v produced: %v", orderId, accountKid, orderResp, err)
+	}
+
+	order.Expires = backDate(order.Expires, backdateAmount)
+	order.CertificateExpiry = backDate(order.CertificateExpiry, backdateAmount)
+
+	encodeJSON, err := jsonutil.EncodeJSON(order)
+	_, err = client.Logical().WriteWithContext(context.Background(), rawOrderPath, map[string]interface{}{
+		"value":    base64.StdEncoding.EncodeToString(encodeJSON),
+		"encoding": "base64",
+	})
+	if err != nil {
+		t.Fatalf("error saving backdated order entry %v on account %v : %v", orderId, accountKid, err)
+	}
+
+	for _, authId := range order.AuthorizationIds {
+		backDateAcmeAuthorizationSys(t, testContext, client, accountKid, authId, backdateAmount, mount)
+	}
+}
+
+func backDateAcmeAuthorizationSys(t *testing.T, testContext context.Context, client *api.Client, accountKid string, authId string, backdateAmount time.Duration, mount string) {
+	rawAuthPath := path.Join("sys/raw/logical/", mount, acmeAccountPrefix, accountKid, "/authorizations/", authId)
+
+	authResp, err := client.Logical().ReadWithContext(testContext, rawAuthPath)
+	if err != nil {
+		t.Fatalf("unable to fetch authorization %v : %v", rawAuthPath, err)
+	}
+
+	var auth *ACMEAuthorization
+	err = jsonutil.DecodeJSON([]byte(authResp.Data["value"].(string)), &auth)
+	if err != nil {
+		t.Fatalf("error decoding auth %v, auth entry %v produced %v", rawAuthPath, authResp, err)
+	}
+
+	expiry, err := auth.GetExpires()
+	if err != nil {
+		t.Fatalf("could not get expiry on %v: %v", rawAuthPath, err)
+	}
+	newExpiry := backDate(expiry, backdateAmount)
+	auth.Expires = time.Time.Format(newExpiry, time.RFC3339)
+
+	encodeJSON, err := jsonutil.EncodeJSON(auth)
+	_, err = client.Logical().WriteWithContext(context.Background(), rawAuthPath, map[string]interface{}{
+		"value":    base64.StdEncoding.EncodeToString(encodeJSON),
+		"encoding": "base64",
+	})
+	if err != nil {
+		t.Fatalf("error updating authorization date on %v: %v", rawAuthPath, err)
+	}
+}
+
+func backDate(original time.Time, change time.Duration) time.Time {
+	if original.IsZero() {
+		return original
+	}
+
+	zeroTime := time.Time{}
+
+	if original.Before(zeroTime.Add(change)) {
+		return zeroTime
+	}
+
+	return original.Add(-change)
+}
+
+func waitForTidyToFinish(t *testing.T, client *api.Client, mount string) *api.Secret {
+	var statusResp *api.Secret
+	testhelpers.RetryUntil(t, 5*time.Second, func() error {
+		var err error
+
+		tidyStatusPath := mount + "/tidy-status"
+		statusResp, err = client.Logical().Read(tidyStatusPath)
+		if err != nil {
+			return fmt.Errorf("failed reading path: %s: %w", tidyStatusPath, err)
+		}
+		if state, ok := statusResp.Data["state"]; !ok || state == "Running" {
+			return fmt.Errorf("tidy status state is still running")
+		}
+
+		if errorOccurred, ok := statusResp.Data["error"]; !ok || !(errorOccurred == nil || errorOccurred == "") {
+			return fmt.Errorf("tidy status returned an error: %s", errorOccurred)
+		}
+
+		return nil
+	})
+
+	t.Logf("got tidy status: %v", statusResp.Data)
+	return statusResp
+}
--- a/changelog/21870.txt
+++ b/changelog/21870.txt
@@ -0,0 +1,6 @@
+```release-note:bug
+secrets/pki: Fix bug with ACME tidy, 'unable to determine acme base folder path'.
+```
+```release-note:bug
+secrets/pki: Fix preserving acme_account_safety_buffer on config/auto-tidy.
+```