From a7b6f3490f59cfeef8b63fe12ceec31279388221 Mon Sep 17 00:00:00 2001 From: Rachel Culpepper <84159930+rculpepper@users.noreply.github.com> Date: Thu, 9 May 2024 15:12:23 -0500 Subject: [PATCH] Add cert metadata fields for tidy (#26867) * add cert metadata fields for tidy * fix import * add missing fields to schema * add new fields to expected value * change error --- builtin/logical/pki/backend_test.go | 2 + builtin/logical/pki/fields.go | 5 ++ builtin/logical/pki/metadata_oss.go | 5 ++ builtin/logical/pki/path_tidy.go | 75 ++++++++++++++++++++++++++++- 4 files changed, 86 insertions(+), 1 deletion(-) diff --git a/builtin/logical/pki/backend_test.go b/builtin/logical/pki/backend_test.go index c46ce0c72e..bec839b2c2 100644 --- a/builtin/logical/pki/backend_test.go +++ b/builtin/logical/pki/backend_test.go @@ -4108,6 +4108,7 @@ func TestBackend_RevokePlusTidy_Intermediate(t *testing.T) { "tidy_move_legacy_ca_bundle": false, "tidy_revocation_queue": false, "tidy_cross_cluster_revoked_certs": false, + "tidy_cert_metadata": false, "pause_duration": "0s", "state": "Finished", "error": nil, @@ -4129,6 +4130,7 @@ func TestBackend_RevokePlusTidy_Intermediate(t *testing.T) { "acme_account_revoked_count": json.Number("0"), "acme_account_deleted_count": json.Number("0"), "total_acme_account_count": json.Number("0"), + "cert_metadata_deleted_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 ed0cf59cd9..ba4edf7101 100644 --- a/builtin/logical/pki/fields.go +++ b/builtin/logical/pki/fields.go @@ -579,6 +579,11 @@ the cross-cluster revoked certificate store. Only runs on the active primary node.`, } + fields["tidy_cert_metadata"] = &framework.FieldSchema{ + Type: framework.TypeBool, + Description: `Set to true to enable tidying up certificate metadata`, + } + return fields } diff --git a/builtin/logical/pki/metadata_oss.go b/builtin/logical/pki/metadata_oss.go index 4475f8436c..ab23b00498 100644 --- a/builtin/logical/pki/metadata_oss.go +++ b/builtin/logical/pki/metadata_oss.go @@ -11,6 +11,7 @@ import ( "errors" "math/big" + "github.com/hashicorp/go-hclog" "github.com/hashicorp/vault/builtin/logical/pki/issuing" "github.com/hashicorp/vault/sdk/logical" ) @@ -24,3 +25,7 @@ func storeMetadata(ctx context.Context, storage logical.Storage, issuerId issuin func GetCertificateMetadata(ctx context.Context, storage logical.Storage, serialNumber *big.Int) (*CertificateMetadata, error) { return nil, ErrMetadataIsEntOnly } + +func (b *backend) doTidyCertMetadata(ctx context.Context, req *logical.Request, logger hclog.Logger, config *tidyConfig) error { + return ErrMetadataIsEntOnly +} diff --git a/builtin/logical/pki/path_tidy.go b/builtin/logical/pki/path_tidy.go index 191f27fea7..98564d8ca8 100644 --- a/builtin/logical/pki/path_tidy.go +++ b/builtin/logical/pki/path_tidy.go @@ -16,6 +16,7 @@ import ( "github.com/hashicorp/go-hclog" "github.com/hashicorp/go-secure-stdlib/parseutil" "github.com/hashicorp/vault/builtin/logical/pki/issuing" + "github.com/hashicorp/vault/helper/constants" "github.com/hashicorp/vault/sdk/framework" "github.com/hashicorp/vault/sdk/helper/consts" "github.com/hashicorp/vault/sdk/logical" @@ -50,6 +51,7 @@ type tidyStatus struct { tidyRevocationQueue bool tidyCrossRevokedCerts bool tidyAcme bool + tidyCertMetadata bool pauseDuration string // Status @@ -66,6 +68,7 @@ type tidyStatus struct { missingIssuerCertCount uint revQueueDeletedCount uint crossRevokedDeletedCount uint + certMetadataDeletedCount uint acmeAccountsCount uint acmeAccountsRevokedCount uint @@ -87,6 +90,7 @@ type tidyConfig struct { RevocationQueue bool `json:"tidy_revocation_queue"` CrossRevokedCerts bool `json:"tidy_cross_cluster_revoked_certs"` TidyAcme bool `json:"tidy_acme"` + CertMetadata bool `json:"tidy_cert_metadata"` // Safety Buffers SafetyBuffer time.Duration `json:"safety_buffer"` @@ -101,7 +105,7 @@ type tidyConfig struct { } func (tc *tidyConfig) IsAnyTidyEnabled() bool { - return tc.CertStore || tc.RevokedCerts || tc.IssuerAssocs || tc.ExpiredIssuers || tc.BackupBundle || tc.TidyAcme || tc.CrossRevokedCerts || tc.RevocationQueue + return tc.CertStore || tc.RevokedCerts || tc.IssuerAssocs || tc.ExpiredIssuers || tc.BackupBundle || tc.TidyAcme || tc.CrossRevokedCerts || tc.RevocationQueue || tc.CertMetadata } func (tc *tidyConfig) AnyTidyConfig() string { @@ -126,6 +130,7 @@ var defaultTidyConfig = tidyConfig{ RevocationQueue: false, QueueSafetyBuffer: 48 * time.Hour, CrossRevokedCerts: false, + CertMetadata: false, } func pathTidy(b *backend) *framework.Path { @@ -217,6 +222,11 @@ func pathTidyCancel(b *backend) *framework.Path { Description: `Tidy expired issuers`, Required: false, }, + "tidy_cert_metadata": { + Type: framework.TypeBool, + Description: `Tidy cert metadata`, + Required: false, + }, "pause_duration": { Type: framework.TypeString, Description: `Duration to pause between tidying certificates`, @@ -321,6 +331,11 @@ func pathTidyCancel(b *backend) *framework.Path { Description: `The number of expired, unused acme orders removed`, Required: false, }, + "cert_metadata_deleted_count": { + Type: framework.TypeInt, + Description: `The number of metadata entries removed`, + Required: false, + }, }, }}, }, @@ -399,6 +414,11 @@ func pathTidyStatus(b *backend) *framework.Path { Description: `Tidy Unused Acme Accounts, and Orders`, Required: true, }, + "tidy_cert_metadata": { + Type: framework.TypeBool, + Description: `Tidy cert metadata`, + Required: true, + }, "pause_duration": { Type: framework.TypeString, Description: `Duration to pause between tidying certificates`, @@ -499,6 +519,11 @@ func pathTidyStatus(b *backend) *framework.Path { Description: `The number of expired, unused acme orders removed`, Required: false, }, + "cert_metadata_deleted_count": { + Type: framework.TypeInt, + Description: `The number of metadata entries removed`, + Required: false, + }, }, }}, }, @@ -587,6 +612,11 @@ available on the tidy-status endpoint.`, Description: `Tidy Unused Acme Accounts, and Orders`, Required: true, }, + "tidy_cert_metadata": { + Type: framework.TypeBool, + Description: `Tidy cert metadata`, + Required: true, + }, "safety_buffer": { Type: framework.TypeInt, Description: `Safety buffer time duration`, @@ -680,6 +710,11 @@ available on the tidy-status endpoint.`, Description: `Tidy Unused Acme Accounts, and Orders`, Required: true, }, + "tidy_cert_metadata": { + Type: framework.TypeBool, + Description: `Tidy cert metadata`, + Required: true, + }, "safety_buffer": { Type: framework.TypeInt, Description: `Safety buffer time duration`, @@ -753,6 +788,7 @@ func (b *backend) pathTidyWrite(ctx context.Context, req *logical.Request, d *fr tidyCrossRevokedCerts := d.Get("tidy_cross_cluster_revoked_certs").(bool) tidyAcme := d.Get("tidy_acme").(bool) acmeAccountSafetyBuffer := d.Get("acme_account_safety_buffer").(int) + tidyCertMetadata := d.Get("tidy_cert_metadata").(bool) if safetyBuffer < 1 { return logical.ErrorResponse("safety_buffer must be greater than zero"), nil @@ -782,6 +818,10 @@ func (b *backend) pathTidyWrite(ctx context.Context, req *logical.Request, d *fr } } + if tidyCertMetadata && !constants.IsEnterprise { + return logical.ErrorResponse("certificate metadata is only supported on Vault Enterprise"), nil + } + bufferDuration := time.Duration(safetyBuffer) * time.Second issuerBufferDuration := time.Duration(issuerSafetyBuffer) * time.Second queueSafetyBufferDuration := time.Duration(queueSafetyBuffer) * time.Second @@ -804,6 +844,7 @@ func (b *backend) pathTidyWrite(ctx context.Context, req *logical.Request, d *fr CrossRevokedCerts: tidyCrossRevokedCerts, TidyAcme: tidyAcme, AcmeAccountSafetyBuffer: acmeAccountSafetyBufferDuration, + CertMetadata: tidyCertMetadata, } if !atomic.CompareAndSwapUint32(b.tidyCASGuard, 0, 1) { @@ -930,6 +971,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.CertMetadata { + if err := b.doTidyCertMetadata(ctx, req, logger, config); err != nil { + return err + } + } + return nil } @@ -1647,6 +1699,7 @@ func (b *backend) pathTidyStatusRead(_ context.Context, _ *logical.Request, _ *f "tidy_revocation_queue": nil, "tidy_cross_cluster_revoked_certs": nil, "tidy_acme": nil, + "tidy_cert_metadata": nil, "pause_duration": nil, "state": "Inactive", "error": nil, @@ -1666,6 +1719,7 @@ func (b *backend) pathTidyStatusRead(_ context.Context, _ *logical.Request, _ *f "acme_account_revoked_count": nil, "acme_orders_deleted_count": nil, "acme_account_safety_buffer": nil, + "cert_metadata_deleted_count": nil, }, } @@ -1699,6 +1753,7 @@ func (b *backend) pathTidyStatusRead(_ context.Context, _ *logical.Request, _ *f 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["tidy_cert_metadata"] = b.tidyStatus.tidyCertMetadata resp.Data["pause_duration"] = b.tidyStatus.pauseDuration resp.Data["time_started"] = b.tidyStatus.timeStarted resp.Data["message"] = b.tidyStatus.message @@ -1714,6 +1769,7 @@ func (b *backend) pathTidyStatusRead(_ context.Context, _ *logical.Request, _ *f 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 + resp.Data["cert_metadata_deleted_count"] = b.tidyStatus.certMetadataDeletedCount switch b.tidyStatus.state { case tidyStatusStarted: @@ -1839,6 +1895,14 @@ func (b *backend) pathConfigAutoTidyWrite(ctx context.Context, req *logical.Requ } } + if tidyCertMetadataRaw, ok := d.GetOk("tidy_cert_metadata"); ok { + config.CertMetadata = tidyCertMetadataRaw.(bool) + + if config.CertMetadata && !constants.IsEnterprise { + return logical.ErrorResponse("certificate metadata is only supported on Vault Enterprise"), 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 } @@ -1881,6 +1945,7 @@ func (b *backend) tidyStatusStart(config *tidyConfig) { tidyRevocationQueue: config.RevocationQueue, tidyCrossRevokedCerts: config.CrossRevokedCerts, tidyAcme: config.TidyAcme, + tidyCertMetadata: config.CertMetadata, pauseDuration: config.PauseDuration.String(), state: tidyStatusStarted, @@ -1983,6 +2048,13 @@ func (b *backend) tidyStatusIncDelAcmeOrderCount() { b.tidyStatus.acmeOrdersDeletedCount++ } +func (b *backend) tidyStatusIncCertMetadataCount() { + b.tidyStatusLock.Lock() + defer b.tidyStatusLock.Unlock() + + b.tidyStatus.certMetadataDeletedCount++ +} + const pathTidyHelpSyn = ` Tidy up the backend by removing expired certificates, revocation information, or both. @@ -2094,5 +2166,6 @@ func getTidyConfigData(config tidyConfig) map[string]interface{} { "tidy_revocation_queue": config.RevocationQueue, "revocation_queue_safety_buffer": int(config.QueueSafetyBuffer / time.Second), "tidy_cross_cluster_revoked_certs": config.CrossRevokedCerts, + "tidy_cert_metadata": config.CertMetadata, } }