backport of commit e19ce98 (#17854) (#17853)

Co-authored-by: Alexander Scheel <[email protected]>

Co-authored-by: Alexander Scheel <[email protected]>

builtin/logical/pki/config_util.go

@@ -47,9 +47,8 @@ func updateDefaultIssuerId(ctx context.Context, s logical.Storage, id issuerID)
 	}
 
 	if config.DefaultIssuerId != id {
-		return setIssuersConfig(ctx, s, &issuerConfigEntry{
-			DefaultIssuerId: id,
-		})
+		config.DefaultIssuerId = id
+		return setIssuersConfig(ctx, s, config)
 	}
 
 	return nil

builtin/logical/pki/integation_test.go

@@ -292,6 +292,109 @@ func TestIntegration_SetSignedWithBackwardsPemBundles(t *testing.T) {
 	require.False(t, resp.IsError(), "got an error issuing a leaf cert from int ca: %#v", resp)
 }
 
+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{
@@ -106,11 +112,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) {
@@ -123,6 +134,7 @@ func (b *backend) pathCAIssuersWrite(ctx context.Context, req *logical.Request,
 		return logical.ErrorResponse("Cannot update defaults until migration has completed"), nil
 	}
 
+	// 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
@@ -131,26 +143,41 @@ func (b *backend) pathCAIssuersWrite(ctx context.Context, req *logical.Request,
 	parsedIssuer, err := resolveIssuerReference(ctx, req.Storage, newDefault)
 	if err != nil {
 		return logical.ErrorResponse("Error resolving issuer reference: " + err.Error()), nil
-	}
-
-	response := &logical.Response{
-		Data: map[string]interface{}{
-			"default": parsedIssuer,
-		},
+		return nil, errutil.UserError{Err: "Error resolving issuer reference: " + err.Error()}
 	}
 
 	entry, err := fetchIssuerById(ctx, req.Storage, parsedIssuer)
 	if err != nil {
 		return logical.ErrorResponse("Unable to fetch issuer: " + err.Error()), nil
 	}
 
+	// 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)
+	}
+
+	// Update the config
+	config, err := getIssuersConfig(ctx, req.Storage)
+	if err != 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 = updateDefaultIssuerId(ctx, req.Storage, parsedIssuer)
+	err = setIssuersConfig(ctx, req.Storage, config)
 	if err != nil {
 		return logical.ErrorResponse("Error updating issuer configuration: " + err.Error()), nil
 	}

builtin/logical/pki/path_manage_issuers.go

@@ -244,6 +244,27 @@ func (b *backend) pathImportIssuers(ctx context.Context, req *logical.Request, d
 		if err != nil {
 			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 := getIssuersConfig(ctx, req.Storage)
+		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 := updateDefaultIssuerId(ctx, req.Storage, 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

@@ -253,6 +253,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 := getIssuersConfig(ctx, req.Storage)
+	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 := updateDefaultIssuerId(ctx, req.Storage, myIssuer.ID); err != nil {
+			resp.AddWarning("Unable to update this new root as the default issuer: " + err.Error())
+		}
+	}
+
 	return resp, nil
 }
 

builtin/logical/pki/storage.go

@@ -157,7 +157,12 @@ type keyConfigEntry struct {
 }
 
 type issuerConfigEntry struct {
-	DefaultIssuerId issuerID `json:"default" structs:"default" mapstructure:"default"`
+	// This new fetchedDefault field allows us to detect if the default
+	// issuer was modified, in turn dispatching the timestamp updater
+	// if necessary.
+	fetchedDefault             issuerID `json:"-"`
+	DefaultIssuerId            issuerID `json:"default"`
+	DefaultFollowsLatestIssuer bool     `json:"default_follows_latest_issuer"`
 }
 
 func listKeys(ctx context.Context, s logical.Storage) ([]keyID, error) {
@@ -515,6 +520,9 @@ func deleteIssuer(ctx context.Context, s logical.Storage, id issuerID) (bool, er
 	wasDefault := false
 	if config.DefaultIssuerId == id {
 		wasDefault = true
+		// Overwrite the fetched default issuer as we're going to remove this
+		// entry.
+		config.fetchedDefault = issuerID("")
 		config.DefaultIssuerId = issuerID("")
 		if err := setIssuersConfig(ctx, s, config); err != nil {
 			return wasDefault, err
@@ -734,7 +742,16 @@ func setIssuersConfig(ctx context.Context, s logical.Storage, config *issuerConf
 		return err
 	}
 
-	return s.Put(ctx, json)
+	if err := s.Put(ctx, json); err != nil {
+		return err
+	}
+
+	// n.b.: on 1.12+ we have If-Modified-Since support which requires
+	// comparing fetchedDefault and DefaultIssuerId. As this is on 1.11,
+	// we don't have that but for compatibility, we've left the field and
+	// rest of the logic.
+
+	return nil
 }
 
 func getIssuersConfig(ctx context.Context, s logical.Storage) (*issuerConfigEntry, error) {
@@ -749,6 +766,7 @@ func getIssuersConfig(ctx context.Context, s logical.Storage) (*issuerConfigEntr
 			return nil, errutil.InternalError{Err: fmt.Sprintf("unable to decode issuer configuration: %v", err)}
 		}
 	}
+	issuerConfig.fetchedDefault = issuerConfig.DefaultIssuerId
 
 	return issuerConfig, nil
 }

builtin/logical/pki/storage_migrations_test.go

@@ -124,7 +124,7 @@ func Test_migrateStorageOnlyKey(t *testing.T) {
 
 	issuersConfig, err := getIssuersConfig(ctx, s)
 	require.NoError(t, err)
-	require.Equal(t, &issuerConfigEntry{}, issuersConfig)
+	require.Equal(t, issuerID(""), issuersConfig.DefaultIssuerId)
 
 	// Make sure if we attempt to re-run the migration nothing happens...
 	err = migrateStorage(ctx, b, s)
@@ -214,7 +214,7 @@ func Test_migrateStorageSimpleBundle(t *testing.T) {
 
 	issuersConfig, err := getIssuersConfig(ctx, s)
 	require.NoError(t, err)
-	require.Equal(t, &issuerConfigEntry{DefaultIssuerId: issuerId}, issuersConfig)
+	require.Equal(t, issuerId, issuersConfig.DefaultIssuerId)
 
 	// Make sure if we attempt to re-run the migration nothing happens...
 	err = migrateStorage(ctx, b, s)

builtin/logical/pki/storage_test.go

@@ -16,6 +16,14 @@ var ctx = context.Background()
 func Test_ConfigsRoundTrip(t *testing.T) {
 	_, s := createBackendWithStorage(t)
 
+	// Create an empty key, issuer for testing.
+	key := keyEntry{ID: genKeyId()}
+	err := writeKey(ctx, s, key)
+	require.NoError(t, err)
+	issuer := &issuerEntry{ID: genIssuerId()}
+	err = writeIssuer(ctx, s, issuer)
+	require.NoError(t, err)
+
 	// Verify we handle nothing stored properly
 	keyConfigEmpty, err := getKeysConfig(ctx, s)
 	require.NoError(t, err)
@@ -27,10 +35,10 @@ func Test_ConfigsRoundTrip(t *testing.T) {
 
 	// Now attempt to store and reload properly
 	origKeyConfig := &keyConfigEntry{
-		DefaultKeyId: genKeyId(),
+		DefaultKeyId: key.ID,
 	}
 	origIssuerConfig := &issuerConfigEntry{
-		DefaultIssuerId: genIssuerId(),
+		DefaultIssuerId: issuer.ID,
 	}
 
 	err = setKeysConfig(ctx, s, origKeyConfig)
@@ -44,7 +52,7 @@ func Test_ConfigsRoundTrip(t *testing.T) {
 
 	issuerConfig, err := getIssuersConfig(ctx, s)
 	require.NoError(t, err)
-	require.Equal(t, origIssuerConfig, issuerConfig)
+	require.Equal(t, origIssuerConfig.DefaultIssuerId, issuerConfig.DefaultIssuerId)
 }
 
 func Test_IssuerRoundTrip(t *testing.T) {

builtin/logical/pki/test_helpers.go

@@ -291,3 +291,9 @@ func CBList(b *backend, s logical.Storage, path string) (*logical.Response, erro
 func CBDelete(b *backend, s logical.Storage, path string) (*logical.Response, error) {
 	return CBReq(b, s, logical.DeleteOperation, path, make(map[string]interface{}))
 }
+
+func requireSuccessNonNilResponse(t *testing.T, resp *logical.Response, err error, msgAndArgs ...interface{}) {
+	require.NoError(t, err, msgAndArgs...)
+	require.False(t, resp.IsError(), msgAndArgs...)
+	require.NotNil(t, resp, msgAndArgs...)
+}

changelog/17824.txt

@@ -0,0 +1,3 @@
+```release-note:improvement
+secrets/pki: Allow issuer creation, import to change default issuer via `default_follows_latest_issuer`.
+```