Optional automatic default issuer selection (#17824)

* Correctly preserve other issuer config params

When setting a new default issuer, our helper function would overwrite
other parameters in the issuer configuration entry. However, up until
now, there were none.

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>

* Add new parameter to allow default to follow new

This parameter will allow operators to have the default issuer
automatically update when a new root is generated or a single issuer
with a key (potentially with others lacking key) is imported.

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>

* Storage migration tests fail on new members

These internal members shouldn't be tested by the storage migration
code, and so should be elided from the test results.

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>

* Follow new issuer on root generation, import

This updates the two places where issuers can be created (outside of
legacy CA bundle migration which already sets the default) to follow
newly created issuers when the config is set.

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>

* Add changelog entry

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>

* Add documentation

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>

* Add test for new default-following behavior

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>

builtin/logical/pki/config_util.go

@@ -46,57 +46,59 @@ func (sc *storageContext) updateDefaultIssuerId(id issuerID) error {
 	}
 
 	if config.DefaultIssuerId != id {
-		oldDefault := config.DefaultIssuerId
-		newDefault := id
-		now := time.Now().UTC()
+		config.DefaultIssuerId = id
+		return sc.setIssuersConfig(config)
+	}
 
-		err := sc.setIssuersConfig(&issuerConfigEntry{
-			DefaultIssuerId: newDefault,
-		})
-		if err != nil {
-			return err
-		}
+	return nil
+}
+
+func (sc *storageContext) changeDefaultIssuerTimestamps(oldDefault issuerID, newDefault issuerID) error {
+	if newDefault == oldDefault {
+		return nil
+	}
 
-		// When the default issuer changes, we need to modify four
-		// pieces of information:
-		//
-		// 1. The old default issuer's modification time, as it no
-		//    longer works for the /cert/ca path.
-		// 2. The new default issuer's modification time, as it now
-		//    works for the /cert/ca path.
-		// 3. & 4. Both issuer's CRLs, as they behave the same, under
-		//    the /cert/crl path!
-		for _, thisId := range []issuerID{oldDefault, newDefault} {
-			if len(thisId) == 0 {
-				continue
-			}
-
-			// 1 & 2 above.
-			issuer, err := sc.fetchIssuerById(thisId)
-			if err != nil {
-				return fmt.Errorf("unable to update issuer (%v)'s modification time: error fetching issuer: %v", thisId, err)
-			}
-
-			issuer.LastModified = now
-			err = sc.writeIssuer(issuer)
-			if err != nil {
-				return fmt.Errorf("unable to update issuer (%v)'s modification time: error persisting issuer: %v", thisId, err)
-			}
+	now := time.Now().UTC()
+
+	// When the default issuer changes, we need to modify four
+	// pieces of information:
+	//
+	// 1. The old default issuer's modification time, as it no
+	//    longer works for the /cert/ca path.
+	// 2. The new default issuer's modification time, as it now
+	//    works for the /cert/ca path.
+	// 3. & 4. Both issuer's CRLs, as they behave the same, under
+	//    the /cert/crl path!
+	for _, thisId := range []issuerID{oldDefault, newDefault} {
+		if len(thisId) == 0 {
+			continue
 		}
 
-		// Fetch and update the localCRLConfigEntry (3&4).
-		cfg, err := sc.getLocalCRLConfig()
+		// 1 & 2 above.
+		issuer, err := sc.fetchIssuerById(thisId)
 		if err != nil {
-			return fmt.Errorf("unable to update local CRL config's modification time: error fetching local CRL config: %v", err)
+			return fmt.Errorf("unable to update issuer (%v)'s modification time: error fetching issuer: %v", thisId, err)
 		}
 
-		cfg.LastModified = now
-		cfg.DeltaLastModified = now
-		err = sc.setLocalCRLConfig(cfg)
+		issuer.LastModified = now
+		err = sc.writeIssuer(issuer)
 		if err != nil {
-			return fmt.Errorf("unable to update local CRL config's modification time: error persisting local CRL config: %v", err)
+			return fmt.Errorf("unable to update issuer (%v)'s modification time: error persisting issuer: %v", thisId, err)
 		}
 	}
 
+	// Fetch and update the localCRLConfigEntry (3&4).
+	cfg, err := sc.getLocalCRLConfig()
+	if err != nil {
+		return fmt.Errorf("unable to update local CRL config's modification time: error fetching local CRL config: %v", err)
+	}
+
+	cfg.LastModified = now
+	cfg.DeltaLastModified = now
+	err = sc.setLocalCRLConfig(cfg)
+	if err != nil {
+		return fmt.Errorf("unable to update local CRL config's modification time: error persisting local CRL config: %v", err)
+	}
+
 	return nil
 }

builtin/logical/pki/integation_test.go

@@ -357,6 +357,109 @@ func TestIntegration_CSRGeneration(t *testing.T) {
 	}
 }
 
+func TestIntegration_AutoIssuer(t *testing.T) {
+	t.Parallel()
+	b, s := createBackendWithStorage(t)
+
+	// Generate two roots. The first should become default under the existing
+	// behavior; when we update the config and generate a second, it should
+	// take over as default. Deleting the first and re-importing it will make
+	// it default again, and then disabling the option and removing and
+	// reimporting the second and creating a new root won't affect it again.
+	resp, err := CBWrite(b, s, "root/generate/internal", map[string]interface{}{
+		"common_name": "Root X1",
+		"issuer_name": "root-1",
+		"key_type":    "ec",
+	})
+	requireSuccessNonNilResponse(t, resp, err)
+	issuerIdOne := resp.Data["issuer_id"]
+	require.NotEmpty(t, issuerIdOne)
+	certOne := resp.Data["certificate"]
+	require.NotEmpty(t, certOne)
+
+	resp, err = CBRead(b, s, "config/issuers")
+	requireSuccessNonNilResponse(t, resp, err)
+	require.Equal(t, issuerIdOne, resp.Data["default"])
+
+	// Enable the new config option.
+	_, err = CBWrite(b, s, "config/issuers", map[string]interface{}{
+		"default":                       issuerIdOne,
+		"default_follows_latest_issuer": true,
+	})
+	require.NoError(t, err)
+
+	// Now generate the second root; it should become default.
+	resp, err = CBWrite(b, s, "root/generate/internal", map[string]interface{}{
+		"common_name": "Root X2",
+		"issuer_name": "root-2",
+		"key_type":    "ec",
+	})
+	requireSuccessNonNilResponse(t, resp, err)
+	issuerIdTwo := resp.Data["issuer_id"]
+	require.NotEmpty(t, issuerIdTwo)
+	certTwo := resp.Data["certificate"]
+	require.NotEmpty(t, certTwo)
+
+	resp, err = CBRead(b, s, "config/issuers")
+	requireSuccessNonNilResponse(t, resp, err)
+	require.Equal(t, issuerIdTwo, resp.Data["default"])
+
+	// Deleting the first shouldn't affect the default issuer.
+	_, err = CBDelete(b, s, "issuer/root-1")
+	require.NoError(t, err)
+	resp, err = CBRead(b, s, "config/issuers")
+	requireSuccessNonNilResponse(t, resp, err)
+	require.Equal(t, issuerIdTwo, resp.Data["default"])
+
+	// But reimporting it should update it to the new issuer's value.
+	resp, err = CBWrite(b, s, "issuers/import/bundle", map[string]interface{}{
+		"pem_bundle": certOne,
+	})
+	requireSuccessNonNilResponse(t, resp, err)
+	issuerIdOneReimported := issuerID(resp.Data["imported_issuers"].([]string)[0])
+
+	resp, err = CBRead(b, s, "config/issuers")
+	requireSuccessNonNilResponse(t, resp, err)
+	require.Equal(t, issuerIdOneReimported, resp.Data["default"])
+
+	// Now update the config to disable this option again.
+	_, err = CBWrite(b, s, "config/issuers", map[string]interface{}{
+		"default":                       issuerIdOneReimported,
+		"default_follows_latest_issuer": false,
+	})
+	require.NoError(t, err)
+
+	// Generating a new root shouldn't update the default.
+	resp, err = CBWrite(b, s, "root/generate/internal", map[string]interface{}{
+		"common_name": "Root X3",
+		"issuer_name": "root-3",
+		"key_type":    "ec",
+	})
+	requireSuccessNonNilResponse(t, resp, err)
+	issuerIdThree := resp.Data["issuer_id"]
+	require.NotEmpty(t, issuerIdThree)
+
+	resp, err = CBRead(b, s, "config/issuers")
+	requireSuccessNonNilResponse(t, resp, err)
+	require.Equal(t, issuerIdOneReimported, resp.Data["default"])
+
+	// Deleting and re-importing root 2 should also not affect it.
+	_, err = CBDelete(b, s, "issuer/root-2")
+	require.NoError(t, err)
+	resp, err = CBRead(b, s, "config/issuers")
+	requireSuccessNonNilResponse(t, resp, err)
+	require.Equal(t, issuerIdOneReimported, resp.Data["default"])
+
+	resp, err = CBWrite(b, s, "issuers/import/bundle", map[string]interface{}{
+		"pem_bundle": certTwo,
+	})
+	requireSuccessNonNilResponse(t, resp, err)
+	require.Equal(t, 1, len(resp.Data["imported_issuers"].([]string)))
+	resp, err = CBRead(b, s, "config/issuers")
+	requireSuccessNonNilResponse(t, resp, err)
+	require.Equal(t, issuerIdOneReimported, resp.Data["default"])
+}
+
 func genTestRootCa(t *testing.T, b *backend, s logical.Storage) (issuerID, keyID) {
 	return genTestRootCaWithIssuerName(t, b, s, "")
 }

builtin/logical/pki/path_config_ca.go

@@ -4,6 +4,7 @@ import (
 	"context"
 
 	"github.com/hashicorp/vault/sdk/framework"
+	"github.com/hashicorp/vault/sdk/helper/errutil"
 	"github.com/hashicorp/vault/sdk/logical"
 )
 
@@ -52,6 +53,11 @@ func pathConfigIssuers(b *backend) *framework.Path {
 				Type:        framework.TypeString,
 				Description: `Reference (name or identifier) to the default issuer.`,
 			},
+			"default_follows_latest_issuer": {
+				Type:        framework.TypeBool,
+				Description: `Whether the default issuer should automatically follow the latest generated or imported issuer. Defaults to false.`,
+				Default:     false,
+			},
 		},
 
 		Operations: map[logical.Operation]framework.OperationHandler{
@@ -107,11 +113,16 @@ func (b *backend) pathCAIssuersRead(ctx context.Context, req *logical.Request, _
 		return logical.ErrorResponse("Error loading issuers configuration: " + err.Error()), nil
 	}
 
+	return b.formatCAIssuerConfigRead(config), nil
+}
+
+func (b *backend) formatCAIssuerConfigRead(config *issuerConfigEntry) *logical.Response {
 	return &logical.Response{
 		Data: map[string]interface{}{
-			defaultRef: config.DefaultIssuerId,
+			defaultRef:                      config.DefaultIssuerId,
+			"default_follows_latest_issuer": config.DefaultFollowsLatestIssuer,
 		},
-	}, nil
+	}
 }
 
 func (b *backend) pathCAIssuersWrite(ctx context.Context, req *logical.Request, data *framework.FieldData) (*logical.Response, error) {
@@ -124,36 +135,50 @@ func (b *backend) pathCAIssuersWrite(ctx context.Context, req *logical.Request,
 		return logical.ErrorResponse("Cannot update defaults until migration has completed"), nil
 	}
 
+	sc := b.makeStorageContext(ctx, req.Storage)
+
+	// Validate the new default reference.
 	newDefault := data.Get(defaultRef).(string)
 	if len(newDefault) == 0 || newDefault == defaultRef {
 		return logical.ErrorResponse("Invalid issuer specification; must be non-empty and can't be 'default'."), nil
 	}
-
-	sc := b.makeStorageContext(ctx, req.Storage)
 	parsedIssuer, err := sc.resolveIssuerReference(newDefault)
 	if err != nil {
 		return logical.ErrorResponse("Error resolving issuer reference: " + err.Error()), nil
+		return nil, errutil.UserError{Err: "Error resolving issuer reference: " + err.Error()}
+	}
+	entry, err := sc.fetchIssuerById(parsedIssuer)
+	if err != nil {
+		return logical.ErrorResponse("Unable to fetch issuer: " + err.Error()), nil
 	}
 
-	response := &logical.Response{
-		Data: map[string]interface{}{
-			"default": parsedIssuer,
-		},
+	// Get the other new parameters. This doesn't exist on the /root/replace
+	// variant of this call.
+	var followIssuer bool
+	followIssuersRaw, followOk := data.GetOk("default_follows_latest_issuer")
+	if followOk {
+		followIssuer = followIssuersRaw.(bool)
 	}
 
-	entry, err := sc.fetchIssuerById(parsedIssuer)
+	// Update the config
+	config, err := sc.getIssuersConfig()
 	if err != nil {
-		return logical.ErrorResponse("Unable to fetch issuer: " + err.Error()), nil
+		return logical.ErrorResponse("Unable to fetch existing issuers configuration: " + err.Error()), nil
+	}
+	config.DefaultIssuerId = parsedIssuer
+	if followOk {
+		config.DefaultFollowsLatestIssuer = followIssuer
 	}
 
+	// Add our warning if necessary.
+	response := b.formatCAIssuerConfigRead(config)
 	if len(entry.KeyID) == 0 {
 		msg := "This selected default issuer has no key associated with it. Some operations like issuing certificates and signing CRLs will be unavailable with the requested default issuer until a key is imported or the default issuer is changed."
 		response.AddWarning(msg)
 		b.Logger().Error(msg)
 	}
 
-	err = sc.updateDefaultIssuerId(parsedIssuer)
-	if err != nil {
+	if err := sc.setIssuersConfig(config); err != nil {
 		return logical.ErrorResponse("Error updating issuer configuration: " + err.Error()), nil
 	}
 

builtin/logical/pki/path_manage_issuers.go

@@ -258,6 +258,27 @@ func (b *backend) pathImportIssuers(ctx context.Context, req *logical.Request, d
 
 			return nil, err
 		}
+
+		var issuersWithKeys []string
+		for _, issuer := range createdIssuers {
+			if issuerKeyMap[issuer] != "" {
+				issuersWithKeys = append(issuersWithKeys, issuer)
+			}
+		}
+
+		// Check whether we need to update our default issuer configuration.
+		config, err := sc.getIssuersConfig()
+		if err != nil {
+			response.AddWarning("Unable to fetch default issuers configuration to update default issuer if necessary: " + err.Error())
+		} else if config.DefaultFollowsLatestIssuer {
+			if len(issuersWithKeys) == 1 {
+				if err := sc.updateDefaultIssuerId(issuerID(issuersWithKeys[0])); err != nil {
+					response.AddWarning("Unable to update this new root as the default issuer: " + err.Error())
+				}
+			} else if len(issuersWithKeys) > 1 {
+				response.AddWarning("Default issuer left unchanged: could not select new issuer automatically as multiple imported issuers had key material in Vault.")
+			}
+		}
 	}
 
 	// While we're here, check if we should warn about a bad default key. We

builtin/logical/pki/path_root.go

@@ -279,6 +279,16 @@ func (b *backend) pathCAGenerateRoot(ctx context.Context, req *logical.Request,
 		resp.AddWarning("Max path length of the generated certificate is zero. This certificate cannot be used to issue intermediate CA certificates.")
 	}
 
+	// Check whether we need to update our default issuer configuration.
+	config, err := sc.getIssuersConfig()
+	if err != nil {
+		resp.AddWarning("Unable to fetch default issuers configuration to update default issuer if necessary: " + err.Error())
+	} else if config.DefaultFollowsLatestIssuer {
+		if err := sc.updateDefaultIssuerId(myIssuer.ID); err != nil {
+			resp.AddWarning("Unable to update this new root as the default issuer: " + err.Error())
+		}
+	}
+
 	resp = addWarnings(resp, warnings)
 
 	return resp, nil