Add ability to perform automatic tidy operations

This enables the PKI secrets engine to allow tidy to be started
periodically by the engine itself, avoiding the need for interaction.
This operation is disabled by default (to avoid load on clusters which
don't need tidy to be run) but can be enabled.

In particular, a default tidy configuration is written (via
/config/auto-tidy) which mirrors the options passed to /tidy. Two
additional parameters, enabled and interval, are accepted, allowing
auto-tidy to be enabled or disabled and controlling the interval
(between successful tidy runs) to attempt auto-tidy.

Notably, a manual execution of tidy will delay additional auto-tidy
operations. Status is reported via the existing /tidy-status endpoint.

Signed-off-by: Alexander Scheel <[email protected]>

builtin/logical/pki/backend.go

@@ -126,6 +126,7 @@ func Backend(conf *logical.BackendConfig) *backend {
 			pathRevokeWithKey(&b),
 			pathTidy(&b),
 			pathTidyStatus(&b),
+			pathConfigAutoTidy(&b),
 
 			// Issuer APIs
 			pathListIssuers(&b),
@@ -185,6 +186,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
 }
 
@@ -198,6 +202,7 @@ type backend struct {
 
 	tidyStatusLock sync.RWMutex
 	tidyStatus     *tidyStatus
+	lastTidy       time.Time
 
 	pkiStorageVersion atomic.Value
 	crlBuilder        *crlBuilder
@@ -394,18 +399,78 @@ 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 {
+		if err := b.crlBuilder.reloadConfigIfRequired(sc); err != nil {
+			return err
+		}
+
+		// 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
+		}
+
+		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
+	doAutoTidy := func() error {
+		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()
+		nextOp := b.lastTidy.Add(config.Interval)
+		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.lastTidy = now
+
+		// 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
 	}
 
-	// Then attempt to rebuild the CRLs if required.
-	if err := b.crlBuilder.rebuildIfForced(ctx, b, request); 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)
+	}
+
+	if crlErr != nil {
+		return fmt.Errorf("Error building CRLs:\n - %v\n", crlErr)
+	}
+
+	if tidyErr != nil {
+		return fmt.Errorf("Error running auto-tidy:\n - %v\n", tidyErr)
 	}
 
 	// All good!

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
+}

builtin/logical/pki/path_tidy.go

@@ -17,58 +17,33 @@ import (
 )
 
 type tidyConfig struct {
+	Enabled      bool          `json:"enabled"`
+	Interval     time.Duration `json:"tidy_interval"`
 	CertStore    bool          `json:"tidy_cert_store"`
 	RevokedCerts bool          `json:"tidy_revoked_certs"`
 	IssuerAssocs bool          `json:"tidy_revoked_cert_issuer_associations"`
 	SafetyBuffer time.Duration `json:"safety_buffer"`
 }
 
+var defaultTidyConfig = tidyConfig{
+	Enabled:      false,
+	Interval:     12 * time.Hour,
+	CertStore:    false,
+	RevokedCerts: false,
+	IssuerAssocs: false,
+	SafetyBuffer: 259200 * time.Second,
+}
+
 func pathTidy(b *backend) *framework.Path {
 	return &framework.Path{
 		Pattern: "tidy$",
-		Fields: map[string]*framework.FieldSchema{
-			"tidy_cert_store": {
-				Type: framework.TypeBool,
-				Description: `Set to true to enable tidying up
-the certificate store`,
-			},
-
-			"tidy_revocation_list": {
-				Type:        framework.TypeBool,
-				Description: `Deprecated; synonym for 'tidy_revoked_certs`,
-			},
-
-			"tidy_revoked_certs": {
-				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.`,
-			},
-
-			"tidy_revoked_cert_issuer_associations": {
-				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.`,
-			},
-
-			"safety_buffer": {
-				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
-			},
-		},
-
+		Fields:  addTidyFields(map[string]*framework.FieldSchema{}),
 		Operations: map[logical.Operation]framework.OperationHandler{
 			logical.UpdateOperation: &framework.PathOperation{
 				Callback:                  b.pathTidyWrite,
 				ForwardPerformanceStandby: true,
 			},
 		},
-
 		HelpSynopsis:    pathTidyHelpSyn,
 		HelpDescription: pathTidyHelpDesc,
 	}
@@ -88,6 +63,36 @@ func pathTidyStatus(b *backend) *framework.Path {
 	}
 }
 
+func pathConfigAutoTidy(b *backend) *framework.Path {
+	return &framework.Path{
+		Pattern: "config/auto-tidy",
+		Fields: addTidyFields(map[string]*framework.FieldSchema{
+			"enabled": {
+				Type:        framework.TypeBool,
+				Description: `Set to true to enable automatic tidy operations.`,
+			},
+			"interval": {
+				Type:        framework.TypeDurationSecond,
+				Description: `Interval at which to run an auto-tidy operation. This is the time between tidy invocations (after one finishes to the start of the next). Running a manual tidy will reset this duration.`,
+				Default:     43200, // 32h, but TypeDurationSecond currently requires the default to be an int.
+			},
+		}),
+		Operations: map[logical.Operation]framework.OperationHandler{
+			logical.ReadOperation: &framework.PathOperation{
+				Callback: b.pathConfigAutoTidyRead,
+			},
+			logical.UpdateOperation: &framework.PathOperation{
+				Callback: b.pathConfigAutoTidyWrite,
+				// Read more about why these flags are set in backend.go.
+				ForwardPerformanceStandby:   true,
+				ForwardPerformanceSecondary: true,
+			},
+		},
+		HelpSynopsis:    pathConfigAutoTidySyn,
+		HelpDescription: pathConfigAutoTidyDesc,
+	}
+}
+
 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)
@@ -100,7 +105,10 @@ func (b *backend) pathTidyWrite(ctx context.Context, req *logical.Request, d *fr
 
 	bufferDuration := time.Duration(safetyBuffer) * time.Second
 
+	// Manual run with constructed configuration.
 	config := &tidyConfig{
+		Enabled:      true,
+		Interval:     0 * time.Second,
 		CertStore:    tidyCertStore,
 		RevokedCerts: tidyRevokedCerts,
 		IssuerAssocs: tidyRevokedAssocs,
@@ -119,6 +127,11 @@ func (b *backend) pathTidyWrite(ctx context.Context, req *logical.Request, d *fr
 		Storage: req.Storage,
 	}
 
+	// Mark the last tidy operation as relatively recent, to ensure we don't
+	// try to trigger the periodic function.
+	b.lastTidy = time.Now()
+
+	// Kick off the actual tidy.
 	b.startTidyOperation(req, config)
 
 	resp := &logical.Response{}
@@ -163,6 +176,11 @@ func (b *backend) startTidyOperation(req *logical.Request, config *tidyConfig) {
 			b.tidyStatusStop(err)
 		} else {
 			b.tidyStatusStop(nil)
+
+			// Since the tidy operation finished without an error, we don't
+			// really want to start another tidy right away (if the interval
+			// is too short). So mark the last tidy as now.
+			b.lastTidy = time.Now()
 		}
 	}()
 }
@@ -411,6 +429,65 @@ func (b *backend) pathTidyStatusRead(_ context.Context, _ *logical.Request, _ *f
 	return resp, nil
 }
 
+func (b *backend) pathConfigAutoTidyRead(ctx context.Context, req *logical.Request, data *framework.FieldData) (*logical.Response, error) {
+	sc := b.makeStorageContext(ctx, req.Storage)
+	config, err := sc.getAutoTidyConfig()
+	if err != nil {
+		return nil, err
+	}
+
+	return &logical.Response{
+		Data: map[string]interface{}{
+			"enabled":                               config.Enabled,
+			"interval":                              int(config.Interval / time.Second),
+			"tidy_cert_store":                       config.CertStore,
+			"tidy_revoked_certs":                    config.RevokedCerts,
+			"tidy_revoked_cert_issuer_associations": config.IssuerAssocs,
+			"safety_buffer":                         int(config.SafetyBuffer / time.Second),
+		},
+	}, nil
+}
+
+func (b *backend) pathConfigAutoTidyWrite(ctx context.Context, req *logical.Request, d *framework.FieldData) (*logical.Response, error) {
+	sc := b.makeStorageContext(ctx, req.Storage)
+	config, err := sc.getAutoTidyConfig()
+	if err != nil {
+		return nil, err
+	}
+
+	if enabledRaw, ok := d.GetOk("enabled"); ok {
+		config.Enabled = enabledRaw.(bool)
+	}
+
+	if intervalRaw, ok := d.GetOk("interval"); ok {
+		config.Interval = time.Duration(intervalRaw.(int)) * time.Second
+		if config.Interval < 0 {
+			return logical.ErrorResponse(fmt.Sprintf("given interval must be greater than or equal to zero seconds; got: %v", intervalRaw)), nil
+		}
+	}
+
+	if certStoreRaw, ok := d.GetOk("tidy_cert_store"); ok {
+		config.CertStore = certStoreRaw.(bool)
+	}
+
+	if revokedCertsRaw, ok := d.GetOk("tidy_revoked_certs"); ok {
+		config.RevokedCerts = revokedCertsRaw.(bool)
+	}
+
+	if issuerAssocRaw, ok := d.GetOk("tidy_revoked_cert_issuer_associations"); ok {
+		config.IssuerAssocs = issuerAssocRaw.(bool)
+	}
+
+	if safetyBufferRaw, ok := d.GetOk("safety_buffer"); ok {
+		config.SafetyBuffer = time.Duration(safetyBufferRaw.(int)) * time.Second
+		if config.SafetyBuffer < 1*time.Second {
+			return logical.ErrorResponse(fmt.Sprintf("given safety_buffer must be greater than zero seconds; got: %v", safetyBufferRaw)), nil
+		}
+	}
+
+	return nil, sc.writeAutoTidyConfig(config)
+}
+
 func (b *backend) tidyStatusStart(config *tidyConfig) {
 	b.tidyStatusLock.Lock()
 	defer b.tidyStatusLock.Unlock()
@@ -529,3 +606,7 @@ The result includes the following fields:
 * 'revoked_cert_deleted_count': The number of revoked certificate entries deleted
 * 'missing_issuer_cert_count': The number of revoked certificates which were missing a valid issuer reference
 `
+
+const pathConfigAutoTidySyn = ``
+
+const pathConfigAutoTidyDesc = ``