Schedule rebuilding PKI CRLs on active nodes only

 - Address an issue that we were scheduling the rebuilding of a CRL on standby
   nodes, which would not be able to write to storage.
 - Fix an issue with standby nodes not correctly determining that a migration previously
   occurred.

builtin/logical/pki/backend.go

@@ -281,6 +281,9 @@ func (b *backend) metricsWrap(callType string, roleMode int, ofunc roleOperation
 
 // initialize is used to perform a possible PKI storage migration if needed
 func (b *backend) initialize(ctx context.Context, _ *logical.InitializationRequest) error {
+	// Load up our current pki storage state, no matter the host type we are on.
+	b.updatePkiStorageVersion(ctx)
+
 	// Early exit if not a primary cluster or performance secondary with a local mount.
 	if b.System().ReplicationState().HasState(consts.ReplicationDRSecondary|consts.ReplicationPerformanceStandby) ||
 		(!b.System().LocalMount() && b.System().ReplicationState().HasState(consts.ReplicationPerformanceSecondary)) {
@@ -288,7 +291,7 @@ func (b *backend) initialize(ctx context.Context, _ *logical.InitializationReque
 		return nil
 	}
 
-	if err := migrateStorage(ctx, b.crlBuilder, b.storage, b.Logger()); err != nil {
+	if err := migrateStorage(ctx, b, b.storage); err != nil {
 		b.Logger().Error("Error during migration of PKI mount: " + err.Error())
 		return err
 	}
@@ -325,13 +328,15 @@ func (b *backend) invalidate(ctx context.Context, key string) {
 		// This is for a secondary cluster to pick up that the migration has completed
 		// and reset its compatibility mode and rebuild the CRL locally.
 		b.updatePkiStorageVersion(ctx)
-		b.crlBuilder.requestRebuild()
+		b.crlBuilder.requestRebuildOnActiveNode(b)
 	case strings.HasPrefix(key, issuerPrefix):
 		// If an issuer has changed on the primary, we need to schedule an update of our CRL,
 		// the primary cluster would have done it already, but the CRL is cluster specific so
 		// force a rebuild of ours.
 		if !b.useLegacyBundleCaStorage() {
-			b.crlBuilder.requestRebuild()
+			b.crlBuilder.requestRebuildOnActiveNode(b)
+		} else {
+			b.Logger().Debug("Ignoring invalidation updates for issuer as the PKI migration has yet to complete.")
 		}
 	}
 }

builtin/logical/pki/ca_util.go

@@ -18,7 +18,7 @@ import (
 	"github.com/hashicorp/vault/sdk/logical"
 )
 
-func (b *backend) getGenerationParams(ctx context.Context, data *framework.FieldData, mountPoint string) (exported bool, format string, role *roleEntry, errorResp *logical.Response) {
+func (b *backend) getGenerationParams(ctx context.Context, storage logical.Storage, data *framework.FieldData, mountPoint string) (exported bool, format string, role *roleEntry, errorResp *logical.Response) {
 	exportedStr := data.Get("exported").(string)
 	switch exportedStr {
 	case "exported":
@@ -39,7 +39,7 @@ func (b *backend) getGenerationParams(ctx context.Context, data *framework.Field
 		return
 	}
 
-	keyType, keyBits, err := getKeyTypeAndBitsForRole(ctx, b, data, mountPoint)
+	keyType, keyBits, err := getKeyTypeAndBitsForRole(ctx, b, storage, data, mountPoint)
 	if err != nil {
 		errorResp = logical.ErrorResponse(err.Error())
 		return
@@ -114,7 +114,7 @@ func parseCABundle(ctx context.Context, b *backend, req *logical.Request, bundle
 	return bundle.ToParsedCertBundle()
 }
 
-func getKeyTypeAndBitsForRole(ctx context.Context, b *backend, data *framework.FieldData, mountPoint string) (string, int, error) {
+func getKeyTypeAndBitsForRole(ctx context.Context, b *backend, storage logical.Storage, data *framework.FieldData, mountPoint string) (string, int, error) {
 	exportedStr := data.Get("exported").(string)
 	var keyType string
 	var keyBits int
@@ -146,7 +146,7 @@ func getKeyTypeAndBitsForRole(ctx context.Context, b *backend, data *framework.F
 	}
 
 	if existingKeyRequestedFromFieldData(data) {
-		existingPubKey, err := getExistingPublicKey(ctx, b.storage, data)
+		existingPubKey, err := getExistingPublicKey(ctx, storage, data)
 		if err != nil {
 			return "", 0, errors.New("failed to lookup public key from existing key: " + err.Error())
 		}

builtin/logical/pki/cert_util.go

@@ -139,6 +139,7 @@ func fetchCAInfo(ctx context.Context, b *backend, req *logical.Request, issuerRe
 func fetchCertBundle(ctx context.Context, b *backend, s logical.Storage, issuerRef string) (*issuerEntry, *certutil.CertBundle, error) {
 	if b.useLegacyBundleCaStorage() {
 		// We have not completed the migration so attempt to load the bundle from the legacy location
+		b.Logger().Info("Using legacy CA bundle")
 		return getLegacyCertBundle(ctx, s)
 	}
 

builtin/logical/pki/crl_test.go

@@ -133,7 +133,7 @@ func TestBackend_Secondary_CRL_Rebuilding(t *testing.T) {
 	b, s := createBackendWithStorage(t)
 
 	// Write out the issuer/key to storage without going through the api call as replication would.
-	bundle := genCertBundle(t, b)
+	bundle := genCertBundle(t, b, s)
 	issuer, _, err := writeCaBundle(ctx, s, bundle, "", "")
 	require.NoError(t, err)
 

builtin/logical/pki/crl_util.go

@@ -14,6 +14,8 @@ import (
 	"sync/atomic"
 	"time"
 
+	"github.com/hashicorp/vault/sdk/helper/consts"
+
 	"github.com/hashicorp/vault/sdk/helper/certutil"
 	"github.com/hashicorp/vault/sdk/helper/errutil"
 	"github.com/hashicorp/vault/sdk/logical"
@@ -58,8 +60,16 @@ func (cb *crlBuilder) rebuild(ctx context.Context, b *backend, request *logical.
 	return cb._doRebuild(ctx, b, request, forceNew, _ignoreForceFlag)
 }
 
-// requestRebuild will schedule a rebuild of the CRL from the next reader or writer.
-func (cb *crlBuilder) requestRebuild() {
+// requestRebuildOnActiveNode will schedule a rebuild of the CRL from the next read or write api call assuming we are the active node of a cluster
+func (cb *crlBuilder) requestRebuildOnActiveNode(b *backend) {
+	// Only schedule us on active nodes, ignoring secondary nodes, the active can/should rebuild the CRL.
+	if b.System().ReplicationState().HasState(consts.ReplicationPerformanceStandby) ||
+		b.System().ReplicationState().HasState(consts.ReplicationDRSecondary) {
+		b.Logger().Debug("Ignoring request to schedule a CRL rebuild, not on active node.")
+		return
+	}
+
+	b.Logger().Info("Scheduling PKI CRL rebuild.")
 	cb.m.Lock()
 	defer cb.m.Unlock()
 	atomic.StoreUint32(&cb.forceRebuild, 1)

builtin/logical/pki/path_intermediate.go

@@ -58,7 +58,7 @@ func (b *backend) pathGenerateIntermediate(ctx context.Context, req *logical.Req
 		data.Raw["exported"] = "existing"
 	}
 
-	exported, format, role, errorResp := b.getGenerationParams(ctx, data, req.MountPoint)
+	exported, format, role, errorResp := b.getGenerationParams(ctx, req.Storage, data, req.MountPoint)
 	if errorResp != nil {
 		return errorResp, nil
 	}

builtin/logical/pki/path_root.go

@@ -91,7 +91,7 @@ func (b *backend) pathCAGenerateRoot(ctx context.Context, req *logical.Request,
 		return logical.ErrorResponse("Can not create root CA until migration has completed"), nil
 	}
 
-	exported, format, role, errorResp := b.getGenerationParams(ctx, data, req.MountPoint)
+	exported, format, role, errorResp := b.getGenerationParams(ctx, req.Storage, data, req.MountPoint)
 	if errorResp != nil {
 		return errorResp, nil
 	}

builtin/logical/pki/storage_migrations.go

@@ -6,7 +6,6 @@ import (
 	"encoding/hex"
 	"time"
 
-	log "github.com/hashicorp/go-hclog"
 	"github.com/hashicorp/vault/sdk/helper/certutil"
 	"github.com/hashicorp/vault/sdk/logical"
 )
@@ -64,7 +63,7 @@ func getMigrationInfo(ctx context.Context, s logical.Storage) (migrationInfo, er
 	return migrationInfo, nil
 }
 
-func migrateStorage(ctx context.Context, cb *crlBuilder, s logical.Storage, logger log.Logger) error {
+func migrateStorage(ctx context.Context, b *backend, s logical.Storage) error {
 	migrationInfo, err := getMigrationInfo(ctx, s)
 	if err != nil {
 		return err
@@ -72,23 +71,25 @@ func migrateStorage(ctx context.Context, cb *crlBuilder, s logical.Storage, logg
 
 	if !migrationInfo.isRequired {
 		// No migration was deemed to be required.
-		logger.Debug("existing migration found and was considered valid, skipping migration.")
+		b.Logger().Debug("existing migration found and was considered valid, skipping migration.")
 		return nil
 	}
 
-	logger.Info("performing PKI migration to new keys/issuers layout")
+	b.Logger().Info("performing PKI migration to new keys/issuers layout")
 	if migrationInfo.legacyBundle != nil {
 		anIssuer, aKey, err := writeCaBundle(ctx, s, migrationInfo.legacyBundle, "current", "current")
 		if err != nil {
 			return err
 		}
-		logger.Debug("Migration generated the following ids and set them as defaults",
+		b.Logger().Debug("Migration generated the following ids and set them as defaults",
 			"issuer id", anIssuer.ID, "key id", aKey.ID)
 	} else {
-		logger.Debug("No legacy CA certs found, no migration required.")
+		b.Logger().Debug("No legacy CA certs found, no migration required.")
 	}
 
-	cb.requestRebuild()
+	// Since we do not have all the mount information available we must schedule
+	// the CRL to be rebuilt at a later time.
+	b.crlBuilder.requestRebuildOnActiveNode(b)
 
 	// We always want to write out this log entry as the secondary clusters leverage this path to wake up
 	// if they were upgraded prior to the primary cluster's migration occurred.
@@ -101,7 +102,7 @@ func migrateStorage(ctx context.Context, cb *crlBuilder, s logical.Storage, logg
 		return err
 	}
 
-	logger.Info("successfully completed migration to new keys/issuers layout")
+	b.Logger().Info("successfully completed migration to new keys/issuers layout")
 	return nil
 }
 

builtin/logical/pki/storage_migrations_test.go

@@ -64,7 +64,7 @@ func Test_migrateStorageSimpleBundle(t *testing.T) {
 	b.pkiStorageVersion.Store(0)
 	require.True(t, b.useLegacyBundleCaStorage(), "pre migration we should have been told to use legacy storage.")
 
-	bundle := genCertBundle(t, b)
+	bundle := genCertBundle(t, b, s)
 	json, err := logical.StorageEntryJSON(legacyCertBundlePath, bundle)
 	require.NoError(t, err)
 	err = s.Put(ctx, json)
@@ -128,7 +128,7 @@ func Test_migrateStorageSimpleBundle(t *testing.T) {
 	require.Equal(t, &issuerConfigEntry{DefaultIssuerId: issuerId}, issuersConfig)
 
 	// Make sure if we attempt to re-run the migration nothing happens...
-	err = migrateStorage(ctx, b.crlBuilder, s, b.Logger())
+	err = migrateStorage(ctx, b, s)
 	require.NoError(t, err)
 	logEntry2, err := getLegacyBundleMigrationLog(ctx, s)
 	require.NoError(t, err)

builtin/logical/pki/storage_test.go

@@ -50,8 +50,8 @@ func Test_ConfigsRoundTrip(t *testing.T) {
 
 func Test_IssuerRoundTrip(t *testing.T) {
 	b, s := createBackendWithStorage(t)
-	issuer1, key1 := genIssuerAndKey(t, b)
-	issuer2, key2 := genIssuerAndKey(t, b)
+	issuer1, key1 := genIssuerAndKey(t, b, s)
+	issuer2, key2 := genIssuerAndKey(t, b, s)
 
 	// We get an error when issuer id not found
 	_, err := fetchIssuerById(ctx, s, issuer1.ID)
@@ -94,8 +94,8 @@ func Test_IssuerRoundTrip(t *testing.T) {
 
 func Test_KeysIssuerImport(t *testing.T) {
 	b, s := createBackendWithStorage(t)
-	issuer1, key1 := genIssuerAndKey(t, b)
-	issuer2, key2 := genIssuerAndKey(t, b)
+	issuer1, key1 := genIssuerAndKey(t, b, s)
+	issuer2, key2 := genIssuerAndKey(t, b, s)
 
 	// Key 1 before Issuer 1; Issuer 2 before Key 2.
 	// Remove KeyIDs from non-written entities before beginning.
@@ -158,8 +158,8 @@ func Test_KeysIssuerImport(t *testing.T) {
 	require.Equal(t, "", key2Ref.Name)
 }
 
-func genIssuerAndKey(t *testing.T, b *backend) (issuerEntry, keyEntry) {
-	certBundle := genCertBundle(t, b)
+func genIssuerAndKey(t *testing.T, b *backend, s logical.Storage) (issuerEntry, keyEntry) {
+	certBundle := genCertBundle(t, b, s)
 
 	keyId := genKeyId()
 
@@ -182,7 +182,7 @@ func genIssuerAndKey(t *testing.T, b *backend) (issuerEntry, keyEntry) {
 	return pkiIssuer, pkiKey
 }
 
-func genCertBundle(t *testing.T, b *backend) *certutil.CertBundle {
+func genCertBundle(t *testing.T, b *backend, s logical.Storage) *certutil.CertBundle {
 	// Pretty gross just to generate a cert bundle, but
 	fields := addCACommonFields(map[string]*framework.FieldSchema{})
 	fields = addCAKeyGenerationFields(fields)
@@ -195,14 +195,14 @@ func genCertBundle(t *testing.T, b *backend) *certutil.CertBundle {
 			"ttl":      3600,
 		},
 	}
-	_, _, role, respErr := b.getGenerationParams(ctx, apiData, "/pki")
+	_, _, role, respErr := b.getGenerationParams(ctx, s, apiData, "/pki")
 	require.Nil(t, respErr)
 
 	input := &inputBundle{
 		req: &logical.Request{
 			Operation: logical.UpdateOperation,
 			Path:      "issue/testrole",
-			Storage:   b.storage,
+			Storage:   s,
 		},
 		apiData: apiData,
 		role:    role,