From 9480573cf71ee309f3684769a5b5160400a9600c Mon Sep 17 00:00:00 2001 From: Kit Haines Date: Fri, 5 May 2023 11:14:41 -0400 Subject: [PATCH] Structure of ACME Tidy (#20494) * Structure of ACME Tidy. * The tidy endpoints/call information. * Counts for status plumbing. * Update typo calls, add information saving date of account creation. * Missed some field locations. * Set-up of Tidy command. * Proper tidy function; lock to work with * Remove order safety buffer. * Missed a field. * Read lock for account creation; Write lock for tidy (account deletion) * Type issues fixed. * fix range operator. * Fix path_tidy read. * Add fields to auto-tidy config. * Add (and standardize) Tidy Config Response * Test pass, consistent fields * Changes from PR-Reviews. * Update test to updated default due to PR-Review. --- builtin/logical/pki/acme_state.go | 3 + builtin/logical/pki/backend.go | 4 +- builtin/logical/pki/backend_test.go | 6 + builtin/logical/pki/fields.go | 25 ++ builtin/logical/pki/path_acme_account.go | 91 ++++++ builtin/logical/pki/path_acme_order.go | 61 ++++ builtin/logical/pki/path_tidy.go | 378 +++++++++++++++++------ builtin/logical/pki/path_tidy_test.go | 1 + 8 files changed, 479 insertions(+), 90 deletions(-) diff --git a/builtin/logical/pki/acme_state.go b/builtin/logical/pki/acme_state.go index 248078feab8a..957f7a30fb18 100644 --- a/builtin/logical/pki/acme_state.go +++ b/builtin/logical/pki/acme_state.go @@ -217,6 +217,8 @@ type acmeAccount struct { TermsOfServiceAgreed bool `json:"termsOfServiceAgreed"` Jwk []byte `json:"jwk"` AcmeDirectory string `json:"acme-directory"` + AccountCreatedDate time.Time `json:"account_created_date"` + AccountRevokedDate time.Time `json:"account_revoked_date"` } type acmeOrder struct { @@ -288,6 +290,7 @@ func (a *acmeState) CreateAccount(ac *acmeContext, c *jwsCtx, contact []string, Jwk: c.Jwk, Status: StatusValid, AcmeDirectory: ac.acmeDirectory, + AccountCreatedDate: time.Now(), } json, err := logical.StorageEntryJSON(acmeAccountPrefix+c.Kid, acct) if err != nil { diff --git a/builtin/logical/pki/backend.go b/builtin/logical/pki/backend.go index 23eae009928f..7869cd9b08f5 100644 --- a/builtin/logical/pki/backend.go +++ b/builtin/logical/pki/backend.go @@ -345,7 +345,9 @@ type backend struct { issuersLock sync.RWMutex // Context around ACME operations - acmeState *acmeState + acmeState *acmeState + acmeAccountLock sync.RWMutex // (Write) Locked on Tidy, (Read) Locked on Account Creation + // TODO: Stress test this - eg. creating an order while an account is being revoked } type roleOperation func(ctx context.Context, req *logical.Request, data *framework.FieldData, role *roleEntry) (*logical.Response, error) diff --git a/builtin/logical/pki/backend_test.go b/builtin/logical/pki/backend_test.go index d275a2e2d8cb..acabd723ab21 100644 --- a/builtin/logical/pki/backend_test.go +++ b/builtin/logical/pki/backend_test.go @@ -4030,6 +4030,12 @@ func TestBackend_RevokePlusTidy_Intermediate(t *testing.T) { "revocation_queue_deleted_count": json.Number("0"), "cross_revoked_cert_deleted_count": json.Number("0"), "internal_backend_uuid": backendUUID, + "tidy_acme": false, + "acme_account_safety_buffer": json.Number("2592000"), + "acme_orders_deleted_count": json.Number("0"), + "acme_account_revoked_count": json.Number("0"), + "acme_account_deleted_count": json.Number("0"), + "total_acme_account_count": json.Number("0"), } // Let's copy the times from the response so that we can use deep.Equal() timeStarted, ok := tidyStatus.Data["time_started"] diff --git a/builtin/logical/pki/fields.go b/builtin/logical/pki/fields.go index 4a2c486b995a..a8c793c365e2 100644 --- a/builtin/logical/pki/fields.go +++ b/builtin/logical/pki/fields.go @@ -491,6 +491,23 @@ this removes ALL issuers within the mount (and is thus not desirable in most operational scenarios).`, } + fields["tidy_acme"] = &framework.FieldSchema{ + Type: framework.TypeBool, + Description: `Set to true to enable tidying ACME accounts, +orders and authorizations. ACME orders are tidied (deleted) +safety_buffer after the certificate associated with them expires, +or after the order and relevant authorizations have expired if no +certificate was produced. Authorizations are tidied with the +corresponding order. + +When a valid ACME Account is at least acme_account_safety_buffer +old, and has no remaining orders associated with it, the account is +marked as revoked. After another acme_account_safety_buffer has +passed from the revocation or deactivation date, a revoked or +deactivated ACME account is deleted.`, + Default: false, + } + fields["safety_buffer"] = &framework.FieldSchema{ Type: framework.TypeDurationSecond, Description: `The amount of extra time that must have passed @@ -509,6 +526,14 @@ Defaults to 8760 hours (1 year).`, Default: int(defaultTidyConfig.IssuerSafetyBuffer / time.Second), // TypeDurationSecond currently requires defaults to be int } + fields["acme_account_safety_buffer"] = &framework.FieldSchema{ + Type: framework.TypeDurationSecond, + Description: `The amount of time that must pass after creation +that an account with no orders is marked revoked, and the amount of time +after being marked revoked or dea`, + Default: int(defaultTidyConfig.AcmeAccountSafetyBuffer / time.Second), // TypeDurationSecond currently requires defaults to be int + } + fields["pause_duration"] = &framework.FieldSchema{ Type: framework.TypeString, Description: `The amount of time to wait between processing diff --git a/builtin/logical/pki/path_acme_account.go b/builtin/logical/pki/path_acme_account.go index eda5e520b191..c4a6b2b655d1 100644 --- a/builtin/logical/pki/path_acme_account.go +++ b/builtin/logical/pki/path_acme_account.go @@ -7,6 +7,7 @@ import ( "fmt" "net/http" "strings" + "time" "github.com/hashicorp/go-secure-stdlib/strutil" @@ -236,6 +237,8 @@ func (b *backend) acmeNewAccountCreateHandler(acmeCtx *acmeContext, userCtx *jws // return nil, fmt.Errorf("terms of service not agreed to: %w", ErrUserActionRequired) //} + b.acmeAccountLock.RLock() // Prevents Account Creation and Tidy Interfering + defer b.acmeAccountLock.RUnlock() accountByKid, err := b.acmeState.CreateAccount(acmeCtx, userCtx, contact, termsOfServiceAgreed) if err != nil { return nil, fmt.Errorf("failed to create account: %w", err) @@ -282,6 +285,7 @@ func (b *backend) acmeNewAccountUpdateHandler(acmeCtx *acmeContext, userCtx *jws // TODO: This should cancel any ongoing operations (do not revoke certs), // perhaps we should delete this account here? account.Status = StatusDeactivated + account.AccountRevokedDate = time.Now() } if shouldUpdate { @@ -294,3 +298,90 @@ func (b *backend) acmeNewAccountUpdateHandler(acmeCtx *acmeContext, userCtx *jws resp := formatAccountResponse(acmeCtx, account) 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, acmeThumbprintPrefix+keyThumbprint) + if err != nil { + return fmt.Errorf("error retrieving thumbprint entry %v, unable to find corresponding account entry: %w", keyThumbprint, err) + } + if thumbprintEntry == nil { + return fmt.Errorf("empty thumbprint entry %v, unable to find corresponding account entry", keyThumbprint) + } + + var thumbprint acmeThumbprint + err = thumbprintEntry.DecodeJSON(&thumbprint) + if err != nil { + return fmt.Errorf("unable to decode thumbprint entry %v to find account entry: %w", keyThumbprint, err) + } + + if len(thumbprint.Kid) == 0 { + return fmt.Errorf("unable to find account entry: empty kid within thumbprint entry: %s", keyThumbprint) + } + + // Now Get the Account: + accountEntry, err := ac.sc.Storage.Get(ac.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, acmeThumbprintPrefix+keyThumbprint) + if err != nil { + return err + } + b.tidyStatusIncDeletedAcmeAccountCount() + return nil + } + + var account acmeAccount + err = accountEntry.DecodeJSON(&account) + if err != nil { + return err + } + + // Tidy Orders On the Account + orderIds, err := as.ListOrderIds(ac, thumbprint.Kid) + if err != nil { + return err + } + allOrdersTidied := true + for _, orderId := range orderIds { + wasTidied, err := b.acmeTidyOrder(ac, thumbprint.Kid, acmeAccountPrefix+thumbprint.Kid+"/orders/"+orderId, ac.sc, certTidyBuffer) + if err != nil { + return err + } + if !wasTidied { + allOrdersTidied = false + } + } + + if allOrdersTidied && time.Now().After(account.AccountCreatedDate.Add(accountTidyBuffer)) { + // Tidy this account + // If it is Revoked or Deactivated: + if (account.Status == StatusRevoked || account.Status == StatusDeactivated) && time.Now().After(account.AccountRevokedDate.Add(accountTidyBuffer)) { + // We Delete the Account Associated with this Thumbprint: + err = ac.sc.Storage.Delete(ac.sc.Context, 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, acmeThumbprintPrefix+keyThumbprint) + if err != nil { + return err + } + b.tidyStatusIncDeletedAcmeAccountCount() + } else if account.Status == StatusValid { + // Revoke This Account + account.AccountRevokedDate = time.Now() + account.Status = StatusRevoked + err := as.UpdateAccount(ac, &account) + if err != nil { + return err + } + b.tidyStatusIncRevAcmeAccountCount() + } + } + + return nil +} diff --git a/builtin/logical/pki/path_acme_order.go b/builtin/logical/pki/path_acme_order.go index dd415e5b15d2..a11de104c018 100644 --- a/builtin/logical/pki/path_acme_order.go +++ b/builtin/logical/pki/path_acme_order.go @@ -884,3 +884,64 @@ func parseOrderIdentifiers(data map[string]interface{}) ([]*ACMEIdentifier, erro return identifiers, nil } + +func (b *backend) acmeTidyOrder(ac *acmeContext, accountId string, orderPath string, sc *storageContext, certTidyBuffer time.Duration) (wasTidied bool, err error) { + // First we get the order; note that the orderPath includes the account + // It's only accessed at acme/orders/ with the account context + // It's saved at acme//orders/ + entry, err := ac.sc.Storage.Get(ac.sc.Context, orderPath) + if err != nil { + return false, fmt.Errorf("error loading order: %w", err) + } + if entry == nil { + return false, fmt.Errorf("order does not exist: %w", ErrMalformed) + } + var order acmeOrder + err = entry.DecodeJSON(&order) + if err != nil { + return false, fmt.Errorf("error decoding order: %w", err) + } + + // Determine whether we should tidy this order + shouldTidy := false + // It is faster to check certificate information on the order entry rather than fetch the cert entry to parse: + if !order.CertificateExpiry.IsZero() { + // This implies that a certificate exists + // When a certificate exists, we want to expire and tidy the order when we tidy the certificate: + if time.Now().After(order.CertificateExpiry.Add(certTidyBuffer)) { // It's time to clean + shouldTidy = true + } + } else { + // This implies that no certificate exists + // In this case, we want to expire the order after it has expired (+ some safety buffer) + if time.Now().After(order.Expires) { + shouldTidy = true + } + } + if shouldTidy == false { + return shouldTidy, nil + } + + // Tidy this Order + // That includes any certificate acme//orders/orderPath/cert + // That also includes any related authorizations: acme//authorizations/ + + // First Authorizations + for _, authorizationId := range order.AuthorizationIds { + err = ac.sc.Storage.Delete(ac.sc.Context, getAuthorizationPath(accountId, authorizationId)) + if err != nil { + return false, err + } + } + + // Normal Tidy will Take Care of the Certificate + + // And Finally, the order: + err = ac.sc.Storage.Delete(ac.sc.Context, orderPath) + if err != nil { + return false, err + } + b.tidyStatusIncDelAcmeOrderCount() + + return true, nil +} diff --git a/builtin/logical/pki/path_tidy.go b/builtin/logical/pki/path_tidy.go index 0b35b4b3d1b2..e7ed2ef46612 100644 --- a/builtin/logical/pki/path_tidy.go +++ b/builtin/logical/pki/path_tidy.go @@ -35,9 +35,10 @@ const ( type tidyStatus struct { // Parameters used to initiate the operation - safetyBuffer int - issuerSafetyBuffer int - revQueueSafetyBuffer int + safetyBuffer int + issuerSafetyBuffer int + revQueueSafetyBuffer int + acmeAccountSafetyBuffer int tidyCertStore bool tidyRevokedCerts bool @@ -46,6 +47,7 @@ type tidyStatus struct { tidyBackupBundle bool tidyRevocationQueue bool tidyCrossRevokedCerts bool + tidyAcme bool pauseDuration string // Status @@ -62,42 +64,51 @@ type tidyStatus struct { missingIssuerCertCount uint revQueueDeletedCount uint crossRevokedDeletedCount uint + + acmeAccountsCount uint + acmeAccountsRevokedCount uint + acmeAccountsDeletedCount uint + acmeOrdersDeletedCount uint } type tidyConfig struct { - Enabled bool `json:"enabled"` - Interval time.Duration `json:"interval_duration"` - CertStore bool `json:"tidy_cert_store"` - RevokedCerts bool `json:"tidy_revoked_certs"` - IssuerAssocs bool `json:"tidy_revoked_cert_issuer_associations"` - ExpiredIssuers bool `json:"tidy_expired_issuers"` - BackupBundle bool `json:"tidy_move_legacy_ca_bundle"` - SafetyBuffer time.Duration `json:"safety_buffer"` - IssuerSafetyBuffer time.Duration `json:"issuer_safety_buffer"` - PauseDuration time.Duration `json:"pause_duration"` - MaintainCount bool `json:"maintain_stored_certificate_counts"` - PublishMetrics bool `json:"publish_stored_certificate_count_metrics"` - RevocationQueue bool `json:"tidy_revocation_queue"` - QueueSafetyBuffer time.Duration `json:"revocation_queue_safety_buffer"` - CrossRevokedCerts bool `json:"tidy_cross_cluster_revoked_certs"` + Enabled bool `json:"enabled"` + Interval time.Duration `json:"interval_duration"` + CertStore bool `json:"tidy_cert_store"` + RevokedCerts bool `json:"tidy_revoked_certs"` + IssuerAssocs bool `json:"tidy_revoked_cert_issuer_associations"` + ExpiredIssuers bool `json:"tidy_expired_issuers"` + BackupBundle bool `json:"tidy_move_legacy_ca_bundle"` + TidyAcme bool `json:"tidy_acme"` + SafetyBuffer time.Duration `json:"safety_buffer"` + IssuerSafetyBuffer time.Duration `json:"issuer_safety_buffer"` + AcmeAccountSafetyBuffer time.Duration `json:"acme_account_safety_buffer"` + PauseDuration time.Duration `json:"pause_duration"` + MaintainCount bool `json:"maintain_stored_certificate_counts"` + PublishMetrics bool `json:"publish_stored_certificate_count_metrics"` + RevocationQueue bool `json:"tidy_revocation_queue"` + QueueSafetyBuffer time.Duration `json:"revocation_queue_safety_buffer"` + CrossRevokedCerts bool `json:"tidy_cross_cluster_revoked_certs"` } var defaultTidyConfig = tidyConfig{ - Enabled: false, - Interval: 12 * time.Hour, - CertStore: false, - RevokedCerts: false, - IssuerAssocs: false, - ExpiredIssuers: false, - BackupBundle: false, - SafetyBuffer: 72 * time.Hour, - IssuerSafetyBuffer: 365 * 24 * time.Hour, - PauseDuration: 0 * time.Second, - MaintainCount: false, - PublishMetrics: false, - RevocationQueue: false, - QueueSafetyBuffer: 48 * time.Hour, - CrossRevokedCerts: false, + Enabled: false, + Interval: 12 * time.Hour, + CertStore: false, + RevokedCerts: false, + IssuerAssocs: false, + ExpiredIssuers: false, + BackupBundle: false, + TidyAcme: false, + SafetyBuffer: 72 * time.Hour, + IssuerSafetyBuffer: 365 * 24 * time.Hour, + AcmeAccountSafetyBuffer: 30 * 24 * time.Hour, + PauseDuration: 0 * time.Second, + MaintainCount: false, + PublishMetrics: false, + RevocationQueue: false, + QueueSafetyBuffer: 48 * time.Hour, + CrossRevokedCerts: false, } func pathTidy(b *backend) *framework.Path { @@ -174,6 +185,16 @@ func pathTidyCancel(b *backend) *framework.Path { Description: `Tidy revoked certificate issuer associations`, Required: false, }, + "tidy_acme": { + Type: framework.TypeBool, + Description: `Tidy Unused Acme Accounts, and Orders`, + Required: false, + }, + "acme_account_safety_buffer": { + Type: framework.TypeInt, + Description: `Safety buffer after creation after which accounts lacking orders are revoked`, + Required: false, + }, "tidy_expired_issuers": { Type: framework.TypeBool, Description: `Tidy expired issuers`, @@ -262,6 +283,26 @@ func pathTidyCancel(b *backend) *framework.Path { Type: framework.TypeString, Required: false, }, + "total_acme_account_count": { + Type: framework.TypeInt, + Description: `Total number of acme accounts iterated over`, + Required: false, + }, + "acme_account_deleted_count": { + Type: framework.TypeInt, + Description: `The number of revoked acme accounts removed`, + Required: false, + }, + "acme_account_revoked_count": { + Type: framework.TypeInt, + Description: `The number of unused acme accounts revoked`, + Required: false, + }, + "acme_orders_deleted_count": { + Type: framework.TypeInt, + Description: `The number of expired, unused acme orders removed`, + Required: false, + }, }, }}, }, @@ -305,6 +346,11 @@ func pathTidyStatus(b *backend) *framework.Path { Description: `Revocation queue safety buffer`, Required: true, }, + "acme_account_safety_buffer": { + Type: framework.TypeInt, + Description: `Safety buffer after creation after which accounts lacking orders are revoked`, + Required: false, + }, "tidy_cert_store": { Type: framework.TypeBool, Description: `Tidy certificate store`, @@ -330,6 +376,11 @@ func pathTidyStatus(b *backend) *framework.Path { Description: ``, Required: false, }, + "tidy_acme": { + Type: framework.TypeBool, + Description: `Tidy Unused Acme Accounts, and Orders`, + Required: true, + }, "pause_duration": { Type: framework.TypeString, Description: `Duration to pause between tidying certificates`, @@ -410,6 +461,26 @@ func pathTidyStatus(b *backend) *framework.Path { Type: framework.TypeString, Required: true, }, + "total_acme_account_count": { + Type: framework.TypeInt, + Description: `Total number of acme accounts iterated over`, + Required: false, + }, + "acme_account_deleted_count": { + Type: framework.TypeInt, + Description: `The number of revoked acme accounts removed`, + Required: false, + }, + "acme_account_revoked_count": { + Type: framework.TypeInt, + Description: `The number of unused acme accounts revoked`, + Required: false, + }, + "acme_orders_deleted_count": { + Type: framework.TypeInt, + Description: `The number of expired, unused acme orders removed`, + Required: false, + }, }, }}, }, @@ -478,6 +549,11 @@ func pathConfigAutoTidy(b *backend) *framework.Path { Description: `Specifies whether tidy expired issuers`, Required: true, }, + "tidy_acme": { + Type: framework.TypeBool, + Description: `Tidy Unused Acme Accounts, and Orders`, + Required: true, + }, "safety_buffer": { Type: framework.TypeInt, Description: `Safety buffer time duration`, @@ -488,6 +564,11 @@ func pathConfigAutoTidy(b *backend) *framework.Path { Description: `Issuer safety buffer`, Required: true, }, + "acme_account_safety_buffer": { + Type: framework.TypeInt, + Description: `Safety buffer after creation after which accounts lacking orders are revoked`, + Required: false, + }, "pause_duration": { Type: framework.TypeString, Description: `Duration to pause between tidying certificates`, @@ -561,6 +642,11 @@ func pathConfigAutoTidy(b *backend) *framework.Path { Description: `Specifies whether tidy expired issuers`, Required: true, }, + "tidy_acme": { + Type: framework.TypeBool, + Description: `Tidy Unused Acme Accounts, and Orders`, + Required: true, + }, "safety_buffer": { Type: framework.TypeInt, Description: `Safety buffer time duration`, @@ -571,6 +657,11 @@ func pathConfigAutoTidy(b *backend) *framework.Path { Description: `Issuer safety buffer`, Required: true, }, + "acme_account_safety_buffer": { + Type: framework.TypeInt, + Description: `Safety buffer after creation after which accounts lacking orders are revoked`, + Required: true, + }, "pause_duration": { Type: framework.TypeString, Description: `Duration to pause between tidying certificates`, @@ -592,6 +683,14 @@ func pathConfigAutoTidy(b *backend) *framework.Path { Type: framework.TypeDurationSecond, Required: true, }, + "publish_stored_certificate_count_metrics": { + Type: framework.TypeBool, + Required: true, + }, + "maintain_stored_certificate_counts": { + Type: framework.TypeBool, + Required: true, + }, }, }}, }, @@ -618,6 +717,8 @@ func (b *backend) pathTidyWrite(ctx context.Context, req *logical.Request, d *fr tidyRevocationQueue := d.Get("tidy_revocation_queue").(bool) queueSafetyBuffer := d.Get("revocation_queue_safety_buffer").(int) tidyCrossRevokedCerts := d.Get("tidy_cross_cluster_revoked_certs").(bool) + tidyAcme := d.Get("tidy_acme").(bool) + acmeAccountSafetyBuffer := d.Get("acme_account_safety_buffer").(int) if safetyBuffer < 1 { return logical.ErrorResponse("safety_buffer must be greater than zero"), nil @@ -631,6 +732,10 @@ func (b *backend) pathTidyWrite(ctx context.Context, req *logical.Request, d *fr return logical.ErrorResponse("revocation_queue_safety_buffer must be greater than zero"), nil } + if acmeAccountSafetyBuffer < 1 { + return logical.ErrorResponse("acme_account_safety_buffer must be greater than zero"), nil + } + if pauseDurationStr != "" { var err error pauseDuration, err = time.ParseDuration(pauseDurationStr) @@ -646,22 +751,25 @@ func (b *backend) pathTidyWrite(ctx context.Context, req *logical.Request, d *fr bufferDuration := time.Duration(safetyBuffer) * time.Second issuerBufferDuration := time.Duration(issuerSafetyBuffer) * time.Second queueSafetyBufferDuration := time.Duration(queueSafetyBuffer) * time.Second + acmeAccountSafetyBufferDuration := time.Duration(acmeAccountSafetyBuffer) * time.Second // Manual run with constructed configuration. config := &tidyConfig{ - Enabled: true, - Interval: 0 * time.Second, - CertStore: tidyCertStore, - RevokedCerts: tidyRevokedCerts, - IssuerAssocs: tidyRevokedAssocs, - ExpiredIssuers: tidyExpiredIssuers, - BackupBundle: tidyBackupBundle, - SafetyBuffer: bufferDuration, - IssuerSafetyBuffer: issuerBufferDuration, - PauseDuration: pauseDuration, - RevocationQueue: tidyRevocationQueue, - QueueSafetyBuffer: queueSafetyBufferDuration, - CrossRevokedCerts: tidyCrossRevokedCerts, + Enabled: true, + Interval: 0 * time.Second, + CertStore: tidyCertStore, + RevokedCerts: tidyRevokedCerts, + IssuerAssocs: tidyRevokedAssocs, + ExpiredIssuers: tidyExpiredIssuers, + BackupBundle: tidyBackupBundle, + SafetyBuffer: bufferDuration, + IssuerSafetyBuffer: issuerBufferDuration, + PauseDuration: pauseDuration, + RevocationQueue: tidyRevocationQueue, + QueueSafetyBuffer: queueSafetyBufferDuration, + CrossRevokedCerts: tidyCrossRevokedCerts, + TidyAcme: tidyAcme, + AcmeAccountSafetyBuffer: acmeAccountSafetyBufferDuration, } if !atomic.CompareAndSwapUint32(b.tidyCASGuard, 0, 1) { @@ -777,6 +885,17 @@ func (b *backend) startTidyOperation(req *logical.Request, config *tidyConfig) { } } + // Check for cancel before continuing. + if atomic.CompareAndSwapUint32(b.tidyCancelCAS, 1, 0) { + return tidyCancelledError + } + + if config.TidyAcme { + if err := b.doTidyAcme(ctx, req, logger, config); err != nil { + return err + } + } + return nil } @@ -1381,6 +1500,53 @@ func (b *backend) doTidyCrossRevocationStore(ctx context.Context, req *logical.R return nil } +func (b *backend) doTidyAcme(ctx context.Context, req *logical.Request, logger hclog.Logger, config *tidyConfig) error { + b.acmeAccountLock.Lock() + defer b.acmeAccountLock.Unlock() + + sc := b.makeStorageContext(ctx, req.Storage) + list, err := sc.Storage.List(ctx, acmeThumbprintPrefix) + if err != nil { + return err + } + + b.tidyStatusLock.Lock() + b.tidyStatus.acmeAccountsCount = uint(len(list)) + b.tidyStatusLock.Unlock() + + baseUrl, _, err := getAcmeBaseUrl(sc, req.Path) + if err != nil { + return err + } + + acmeCtx := &acmeContext{ + baseUrl: baseUrl, + sc: sc, + } + + for _, thumbprint := range list { + err := b.tidyAcmeAccountByThumbprint(b.acmeState, acmeCtx, thumbprint, config.SafetyBuffer, config.AcmeAccountSafetyBuffer) + if err != nil { + logger.Warn("error tidying account %v: %v", thumbprint, err.Error()) + } + + // Check for cancel before continuing. + if atomic.CompareAndSwapUint32(b.tidyCancelCAS, 1, 0) { + return tidyCancelledError + } + + // Check for pause duration to reduce resource consumption. + if config.PauseDuration > (0 * time.Second) { + b.acmeAccountLock.Unlock() // Correct the Lock + time.Sleep(config.PauseDuration) + b.acmeAccountLock.Lock() + } + + } + + return nil +} + func (b *backend) pathTidyCancelWrite(ctx context.Context, req *logical.Request, d *framework.FieldData) (*logical.Response, error) { if atomic.LoadUint32(b.tidyCASGuard) == 0 { resp := &logical.Response{} @@ -1419,6 +1585,7 @@ func (b *backend) pathTidyStatusRead(_ context.Context, _ *logical.Request, _ *f "tidy_move_legacy_ca_bundle": nil, "tidy_revocation_queue": nil, "tidy_cross_cluster_revoked_certs": nil, + "tidy_acme": nil, "pause_duration": nil, "state": "Inactive", "error": nil, @@ -1433,6 +1600,11 @@ func (b *backend) pathTidyStatusRead(_ context.Context, _ *logical.Request, _ *f "internal_backend_uuid": nil, "revocation_queue_deleted_count": nil, "cross_revoked_cert_deleted_count": nil, + "total_acme_account_count": nil, + "acme_account_deleted_count": nil, + "acme_account_revoked_count": nil, + "acme_orders_deleted_count": nil, + "acme_account_safety_buffer": nil, }, } @@ -1463,6 +1635,7 @@ func (b *backend) pathTidyStatusRead(_ context.Context, _ *logical.Request, _ *f resp.Data["tidy_move_legacy_ca_bundle"] = b.tidyStatus.tidyBackupBundle resp.Data["tidy_revocation_queue"] = b.tidyStatus.tidyRevocationQueue resp.Data["tidy_cross_cluster_revoked_certs"] = b.tidyStatus.tidyCrossRevokedCerts + resp.Data["tidy_acme"] = b.tidyStatus.tidyAcme resp.Data["pause_duration"] = b.tidyStatus.pauseDuration resp.Data["time_started"] = b.tidyStatus.timeStarted resp.Data["message"] = b.tidyStatus.message @@ -1473,6 +1646,11 @@ func (b *backend) pathTidyStatusRead(_ context.Context, _ *logical.Request, _ *f resp.Data["cross_revoked_cert_deleted_count"] = b.tidyStatus.crossRevokedDeletedCount resp.Data["revocation_queue_safety_buffer"] = b.tidyStatus.revQueueSafetyBuffer resp.Data["last_auto_tidy_finished"] = b.lastTidy + resp.Data["total_acme_account_count"] = b.tidyStatus.acmeAccountsCount + resp.Data["acme_account_deleted_count"] = b.tidyStatus.acmeAccountsDeletedCount + resp.Data["acme_account_revoked_count"] = b.tidyStatus.acmeAccountsRevokedCount + resp.Data["acme_orders_deleted_count"] = b.tidyStatus.acmeOrdersDeletedCount + resp.Data["acme_account_safety_buffer"] = b.tidyStatus.acmeAccountSafetyBuffer switch b.tidyStatus.state { case tidyStatusStarted: @@ -1505,23 +1683,7 @@ func (b *backend) pathConfigAutoTidyRead(ctx context.Context, req *logical.Reque } return &logical.Response{ - Data: map[string]interface{}{ - "enabled": config.Enabled, - "interval_duration": int(config.Interval / time.Second), - "tidy_cert_store": config.CertStore, - "tidy_revoked_certs": config.RevokedCerts, - "tidy_revoked_cert_issuer_associations": config.IssuerAssocs, - "tidy_expired_issuers": config.ExpiredIssuers, - "safety_buffer": int(config.SafetyBuffer / time.Second), - "issuer_safety_buffer": int(config.IssuerSafetyBuffer / time.Second), - "pause_duration": config.PauseDuration.String(), - "publish_stored_certificate_count_metrics": config.PublishMetrics, - "maintain_stored_certificate_counts": config.MaintainCount, - "tidy_move_legacy_ca_bundle": config.BackupBundle, - "tidy_revocation_queue": config.RevocationQueue, - "revocation_queue_safety_buffer": int(config.QueueSafetyBuffer / time.Second), - "tidy_cross_cluster_revoked_certs": config.CrossRevokedCerts, - }, + Data: getTidyConfigData(*config), }, nil } @@ -1623,21 +1785,7 @@ func (b *backend) pathConfigAutoTidyWrite(ctx context.Context, req *logical.Requ } return &logical.Response{ - Data: map[string]interface{}{ - "enabled": config.Enabled, - "interval_duration": int(config.Interval / time.Second), - "tidy_cert_store": config.CertStore, - "tidy_revoked_certs": config.RevokedCerts, - "tidy_revoked_cert_issuer_associations": config.IssuerAssocs, - "tidy_expired_issuers": config.ExpiredIssuers, - "tidy_move_legacy_ca_bundle": config.BackupBundle, - "safety_buffer": int(config.SafetyBuffer / time.Second), - "issuer_safety_buffer": int(config.IssuerSafetyBuffer / time.Second), - "pause_duration": config.PauseDuration.String(), - "tidy_revocation_queue": config.RevocationQueue, - "revocation_queue_safety_buffer": int(config.QueueSafetyBuffer / time.Second), - "tidy_cross_cluster_revoked_certs": config.CrossRevokedCerts, - }, + Data: getTidyConfigData(*config), }, nil } @@ -1646,17 +1794,19 @@ func (b *backend) tidyStatusStart(config *tidyConfig) { defer b.tidyStatusLock.Unlock() b.tidyStatus = &tidyStatus{ - safetyBuffer: int(config.SafetyBuffer / time.Second), - issuerSafetyBuffer: int(config.IssuerSafetyBuffer / time.Second), - revQueueSafetyBuffer: int(config.QueueSafetyBuffer / time.Second), - tidyCertStore: config.CertStore, - tidyRevokedCerts: config.RevokedCerts, - tidyRevokedAssocs: config.IssuerAssocs, - tidyExpiredIssuers: config.ExpiredIssuers, - tidyBackupBundle: config.BackupBundle, - tidyRevocationQueue: config.RevocationQueue, - tidyCrossRevokedCerts: config.CrossRevokedCerts, - pauseDuration: config.PauseDuration.String(), + safetyBuffer: int(config.SafetyBuffer / time.Second), + issuerSafetyBuffer: int(config.IssuerSafetyBuffer / time.Second), + revQueueSafetyBuffer: int(config.QueueSafetyBuffer / time.Second), + acmeAccountSafetyBuffer: int(config.AcmeAccountSafetyBuffer / time.Second), + tidyCertStore: config.CertStore, + tidyRevokedCerts: config.RevokedCerts, + tidyRevokedAssocs: config.IssuerAssocs, + tidyExpiredIssuers: config.ExpiredIssuers, + tidyBackupBundle: config.BackupBundle, + tidyRevocationQueue: config.RevocationQueue, + tidyCrossRevokedCerts: config.CrossRevokedCerts, + tidyAcme: config.TidyAcme, + pauseDuration: config.PauseDuration.String(), state: tidyStatusStarted, timeStarted: time.Now(), @@ -1737,6 +1887,27 @@ func (b *backend) tidyStatusIncCrossRevCertCount() { b.tidyStatus.crossRevokedDeletedCount++ } +func (b *backend) tidyStatusIncRevAcmeAccountCount() { + b.tidyStatusLock.Lock() + defer b.tidyStatusLock.Unlock() + + b.tidyStatus.acmeAccountsRevokedCount++ +} + +func (b *backend) tidyStatusIncDeletedAcmeAccountCount() { + b.tidyStatusLock.Lock() + defer b.tidyStatusLock.Unlock() + + b.tidyStatus.acmeAccountsDeletedCount++ +} + +func (b *backend) tidyStatusIncDelAcmeOrderCount() { + b.tidyStatusLock.Lock() + defer b.tidyStatusLock.Unlock() + + b.tidyStatus.acmeOrdersDeletedCount++ +} + const pathTidyHelpSyn = ` Tidy up the backend by removing expired certificates, revocation information, or both. @@ -1806,6 +1977,12 @@ The result includes the following fields: * 'tidy_cross_cluster_revoked_certs': the value of this parameter when initiating the tidy operation * 'cross_revoked_cert_deleted_count': the number of cross-cluster revoked certificate entries deleted * 'revocation_queue_safety_buffer': the value of this parameter when initiating the tidy operation +* 'tidy_acme': the value of this parameter when initiating the tidy operation +* 'acme_account_safety_buffer': the value of this parameter when initiating the tidy operation +* 'total_acme_account_count': the total number of acme accounts in the list to be iterated over +* 'acme_account_deleted_count': the number of revoked acme accounts deleted during the operation +* 'acme_account_revoked_count': the number of acme accounts revoked during the operation +* 'acme_orders_deleted_count': the number of acme orders deleted during the operation ` const pathConfigAutoTidySyn = ` @@ -1821,3 +1998,26 @@ controls the frequency of auto-tidy execution). Once enabled, a tidy operation will be kicked off automatically, as if it were executed with the posted configuration. ` + +func getTidyConfigData(config tidyConfig) map[string]interface{} { + return map[string]interface{}{ + // This map is in the same order as tidyConfig to ensure that all fields are accounted for + "enabled": config.Enabled, + "interval_duration": int(config.Interval / time.Second), + "tidy_cert_store": config.CertStore, + "tidy_revoked_certs": config.RevokedCerts, + "tidy_revoked_cert_issuer_associations": config.IssuerAssocs, + "tidy_expired_issuers": config.ExpiredIssuers, + "tidy_move_legacy_ca_bundle": config.BackupBundle, + "tidy_acme": config.TidyAcme, + "safety_buffer": int(config.SafetyBuffer / time.Second), + "issuer_safety_buffer": int(config.IssuerSafetyBuffer / time.Second), + "acme_account_safety_buffer": int(config.AcmeAccountSafetyBuffer / time.Second), + "pause_duration": config.PauseDuration.String(), + "publish_stored_certificate_count_metrics": config.PublishMetrics, + "maintain_stored_certificate_counts": config.MaintainCount, + "tidy_revocation_queue": config.RevocationQueue, + "revocation_queue_safety_buffer": int(config.QueueSafetyBuffer / time.Second), + "tidy_cross_cluster_revoked_certs": config.CrossRevokedCerts, + } +} diff --git a/builtin/logical/pki/path_tidy_test.go b/builtin/logical/pki/path_tidy_test.go index d24555bddca2..c2e6c55e7f79 100644 --- a/builtin/logical/pki/path_tidy_test.go +++ b/builtin/logical/pki/path_tidy_test.go @@ -408,6 +408,7 @@ func TestTidyIssuerConfig(t *testing.T) { defaultConfigMap["safety_buffer"] = int(time.Duration(defaultConfigMap["safety_buffer"].(float64)) / time.Second) defaultConfigMap["pause_duration"] = time.Duration(defaultConfigMap["pause_duration"].(float64)).String() defaultConfigMap["revocation_queue_safety_buffer"] = int(time.Duration(defaultConfigMap["revocation_queue_safety_buffer"].(float64)) / time.Second) + defaultConfigMap["acme_account_safety_buffer"] = int(time.Duration(defaultConfigMap["acme_account_safety_buffer"].(float64)) / time.Second) require.Equal(t, defaultConfigMap, resp.Data)