Add ability to perform automatic PKI tidy operations

builtin/logical/pki/backend.go

@@ -132,6 +132,7 @@ func Backend(conf *logical.BackendConfig) *backend {
 			pathRevokeWithKey(&b),
 			pathTidy(&b),
 			pathTidyStatus(&b),
+			pathConfigAutoTidy(&b),
 
 			// Issuer APIs
 			pathListIssuers(&b),
@@ -191,6 +192,9 @@ func Backend(conf *logical.BackendConfig) *backend {
 
 	b.crlBuilder = newCRLBuilder()
 
+	// Delay the first tidy until after we've started up.
+	b.lastTidy = time.Now()
+
 	return &b
 }
 
@@ -204,6 +208,7 @@ type backend struct {
 
 	tidyStatusLock sync.RWMutex
 	tidyStatus     *tidyStatus
+	lastTidy       time.Time
 
 	pkiStorageVersion atomic.Value
 	crlBuilder        *crlBuilder
@@ -400,34 +405,105 @@ func (b *backend) invalidate(ctx context.Context, key string) {
 }
 
 func (b *backend) periodicFunc(ctx context.Context, request *logical.Request) error {
-	// First attempt to reload the CRL configuration.
 	sc := b.makeStorageContext(ctx, request.Storage)
-	if err := b.crlBuilder.reloadConfigIfRequired(sc); err != nil {
-		return err
+
+	doCRL := func() error {
+		// First attempt to reload the CRL configuration.
+		if err := b.crlBuilder.reloadConfigIfRequired(sc); err != nil {
+			return err
+		}
+
+		// As we're (below) modifying the backing storage, we need to ensure
+		// we're not on a standby/secondary node.
+		if b.System().ReplicationState().HasState(consts.ReplicationPerformanceStandby) ||
+			b.System().ReplicationState().HasState(consts.ReplicationDRSecondary) {
+			return nil
+		}
+
+		// Check if we're set to auto rebuild and a CRL is set to expire.
+		if err := b.crlBuilder.checkForAutoRebuild(sc); err != nil {
+			return err
+		}
+
+		// Then attempt to rebuild the CRLs if required.
+		if err := b.crlBuilder.rebuildIfForced(ctx, b, request); err != nil {
+			return err
+		}
+
+		// If a delta CRL was rebuilt above as part of the complete CRL rebuild,
+		// this will be a no-op. However, if we do need to rebuild delta CRLs,
+		// this would cause us to do so.
+		if err := b.crlBuilder.rebuildDeltaCRLsIfForced(sc); err != nil {
+			return err
+		}
+
+		return nil
 	}
 
-	// As we're (below) modifying the backing storage, we need to ensure
-	// we're not on a standby/secondary node.
-	if b.System().ReplicationState().HasState(consts.ReplicationPerformanceStandby) ||
-		b.System().ReplicationState().HasState(consts.ReplicationDRSecondary) {
+	doAutoTidy := func() error {
+		// As we're (below) modifying the backing storage, we need to ensure
+		// we're not on a standby/secondary node.
+		if b.System().ReplicationState().HasState(consts.ReplicationPerformanceStandby) ||
+			b.System().ReplicationState().HasState(consts.ReplicationDRSecondary) {
+			return nil
+		}
+
+		config, err := sc.getAutoTidyConfig()
+		if err != nil {
+			return err
+		}
+
+		if !config.Enabled || config.Interval <= 0*time.Second {
+			return nil
+		}
+
+		// Check if we should run another tidy...
+		now := time.Now()
+		b.tidyStatusLock.RLock()
+		nextOp := b.lastTidy.Add(config.Interval)
+		b.tidyStatusLock.RUnlock()
+		if now.Before(nextOp) {
+			return nil
+		}
+
+		// Ensure a tidy isn't already running... If it is, we'll trigger
+		// again when the running one finishes.
+		if !atomic.CompareAndSwapUint32(b.tidyCASGuard, 0, 1) {
+			return nil
+		}
+
+		// Prevent ourselves from starting another tidy operation while
+		// this one is still running. This operation runs in the background
+		// and has a separate error reporting mechanism.
+		b.tidyStatusLock.Lock()
+		b.lastTidy = now
+		b.tidyStatusLock.Unlock()
+
+		// Because the request from the parent storage will be cleared at
+		// some point (and potentially reused) -- due to tidy executing in
+		// a background goroutine -- we need to copy the storage entry off
+		// of the backend instead.
+		backendReq := &logical.Request{
+			Storage: b.storage,
+		}
+
+		b.startTidyOperation(backendReq, config)
 		return nil
 	}
 
-	// Check if we're set to auto rebuild and a CRL is set to expire.
-	if err := b.crlBuilder.checkForAutoRebuild(sc); err != nil {
-		return err
+	crlErr := doCRL()
+	tidyErr := doAutoTidy()
+
+	if crlErr != nil && tidyErr != nil {
+		return fmt.Errorf("Error building CRLs:\n - %v\n\nError running auto-tidy:\n - %v\n", crlErr, tidyErr)
 	}
 
-	// Then attempt to rebuild the CRLs if required.
-	if err := b.crlBuilder.rebuildIfForced(ctx, b, request); err != nil {
-		return err
+	if crlErr != nil {
+		return fmt.Errorf("Error building CRLs:\n - %v\n", crlErr)
 	}
 
-	// If a delta CRL was rebuilt above as part of the complete CRL rebuild,
-	// this will be a no-op. However, if we do need to rebuild delta CRLs,
-	// this would cause us to do so.
-	if err := b.crlBuilder.rebuildDeltaCRLsIfForced(sc); err != nil {
-		return err
+	if tidyErr != nil {
+		return fmt.Errorf("Error running auto-tidy:\n - %v\n", tidyErr)
 	}
 
 	// Check if the CRL was invalidated due to issuer swap and update

builtin/logical/pki/crl_test.go

@@ -876,14 +876,11 @@ func TestAutoRebuild(t *testing.T) {
 		// storage without barrier encryption.
 		EnableRaw: true,
 	}
-	oldPeriod := vault.SetRollbackPeriodForTesting(newPeriod)
-	cluster := vault.NewTestCluster(t, coreConfig, &vault.TestClusterOptions{
+	cluster := vault.CreateTestClusterWithRollbackPeriod(t, newPeriod, coreConfig, &vault.TestClusterOptions{
 		HandlerFunc: vaulthttp.Handler,
 	})
-	cluster.Start()
 	defer cluster.Cleanup()
 	client := cluster.Cores[0].Client
-	vault.SetRollbackPeriodForTesting(oldPeriod)
 
 	// Mount PKI
 	err := client.Sys().Mount("pki", &api.MountInput{

builtin/logical/pki/fields.go

@@ -416,3 +416,41 @@ to the key.`,
 	}
 	return fields
 }
+
+func addTidyFields(fields map[string]*framework.FieldSchema) map[string]*framework.FieldSchema {
+	fields["tidy_cert_store"] = &framework.FieldSchema{
+		Type: framework.TypeBool,
+		Description: `Set to true to enable tidying up
+the certificate store`,
+	}
+
+	fields["tidy_revocation_list"] = &framework.FieldSchema{
+		Type:        framework.TypeBool,
+		Description: `Deprecated; synonym for 'tidy_revoked_certs`,
+	}
+
+	fields["tidy_revoked_certs"] = &framework.FieldSchema{
+		Type: framework.TypeBool,
+		Description: `Set to true to expire all revoked
+and expired certificates, removing them both from the CRL and from storage. The
+CRL will be rotated if this causes any values to be removed.`,
+	}
+
+	fields["tidy_revoked_cert_issuer_associations"] = &framework.FieldSchema{
+		Type: framework.TypeBool,
+		Description: `Set to true to validate issuer associations
+on revocation entries. This helps increase the performance of CRL building
+and OCSP responses.`,
+	}
+
+	fields["safety_buffer"] = &framework.FieldSchema{
+		Type: framework.TypeDurationSecond,
+		Description: `The amount of extra time that must have passed
+beyond certificate expiration before it is removed
+from the backend storage and/or revocation list.
+Defaults to 72 hours.`,
+		Default: 259200, // 72h, but TypeDurationSecond currently requires defaults to be int
+	}
+
+	return fields
+}