diff --git a/builtin/logical/pki/config_util.go b/builtin/logical/pki/config_util.go index f775b9d58..84170d761 100644 --- a/builtin/logical/pki/config_util.go +++ b/builtin/logical/pki/config_util.go @@ -46,56 +46,58 @@ func (sc *storageContext) updateDefaultIssuerId(id issuerID) error { } if config.DefaultIssuerId != id { - oldDefault := config.DefaultIssuerId - newDefault := id - now := time.Now().UTC() - - err := sc.setIssuersConfig(&issuerConfigEntry{ - DefaultIssuerId: newDefault, - }) - if err != nil { - return err - } - - // 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) - } - } - - // 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) - } + config.DefaultIssuerId = id + return sc.setIssuersConfig(config) + } + + return nil +} + +func (sc *storageContext) changeDefaultIssuerTimestamps(oldDefault issuerID, newDefault issuerID) error { + if newDefault == oldDefault { + return nil + } + + 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 + } + + // 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) + } + } + + // 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 diff --git a/builtin/logical/pki/integation_test.go b/builtin/logical/pki/integation_test.go index a655662ec..d71909981 100644 --- a/builtin/logical/pki/integation_test.go +++ b/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, "") } diff --git a/builtin/logical/pki/path_config_ca.go b/builtin/logical/pki/path_config_ca.go index dd2010e90..f7a770645 100644 --- a/builtin/logical/pki/path_config_ca.go +++ b/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()} } - - response := &logical.Response{ - Data: map[string]interface{}{ - "default": parsedIssuer, - }, - } - entry, err := sc.fetchIssuerById(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 := sc.getIssuersConfig() + 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 = sc.updateDefaultIssuerId(parsedIssuer) - if err != nil { + if err := sc.setIssuersConfig(config); err != nil { return logical.ErrorResponse("Error updating issuer configuration: " + err.Error()), nil } diff --git a/builtin/logical/pki/path_manage_issuers.go b/builtin/logical/pki/path_manage_issuers.go index ef5ada9b6..712c80f7d 100644 --- a/builtin/logical/pki/path_manage_issuers.go +++ b/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 diff --git a/builtin/logical/pki/path_root.go b/builtin/logical/pki/path_root.go index 18be9dcac..5f9ab837e 100644 --- a/builtin/logical/pki/path_root.go +++ b/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 diff --git a/builtin/logical/pki/storage.go b/builtin/logical/pki/storage.go index e0abd6ed2..1b27b921e 100644 --- a/builtin/logical/pki/storage.go +++ b/builtin/logical/pki/storage.go @@ -187,7 +187,12 @@ type keyConfigEntry struct { } type issuerConfigEntry struct { - DefaultIssuerId issuerID `json:"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"` } type storageContext struct { @@ -658,6 +663,9 @@ func (sc *storageContext) deleteIssuer(id issuerID) (bool, error) { 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 := sc.setIssuersConfig(config); err != nil { return wasDefault, err @@ -912,7 +920,15 @@ func (sc *storageContext) setIssuersConfig(config *issuerConfigEntry) error { return err } - return sc.Storage.Put(sc.Context, json) + if err := sc.Storage.Put(sc.Context, json); err != nil { + return err + } + + if err := sc.changeDefaultIssuerTimestamps(config.fetchedDefault, config.DefaultIssuerId); err != nil { + return err + } + + return nil } func (sc *storageContext) getIssuersConfig() (*issuerConfigEntry, error) { @@ -927,6 +943,7 @@ func (sc *storageContext) getIssuersConfig() (*issuerConfigEntry, error) { return nil, errutil.InternalError{Err: fmt.Sprintf("unable to decode issuer configuration: %v", err)} } } + issuerConfig.fetchedDefault = issuerConfig.DefaultIssuerId return issuerConfig, nil } diff --git a/builtin/logical/pki/storage_migrations_test.go b/builtin/logical/pki/storage_migrations_test.go index 170f2dfc0..49217bcdf 100644 --- a/builtin/logical/pki/storage_migrations_test.go +++ b/builtin/logical/pki/storage_migrations_test.go @@ -127,7 +127,7 @@ func Test_migrateStorageOnlyKey(t *testing.T) { issuersConfig, err := sc.getIssuersConfig() 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) @@ -220,7 +220,7 @@ func Test_migrateStorageSimpleBundle(t *testing.T) { issuersConfig, err := sc.getIssuersConfig() 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) diff --git a/builtin/logical/pki/storage_test.go b/builtin/logical/pki/storage_test.go index 2f9f99c28..17700576f 100644 --- a/builtin/logical/pki/storage_test.go +++ b/builtin/logical/pki/storage_test.go @@ -18,6 +18,14 @@ func Test_ConfigsRoundTrip(t *testing.T) { b, s := createBackendWithStorage(t) sc := b.makeStorageContext(ctx, s) + // Create an empty key, issuer for testing. + key := keyEntry{ID: genKeyId()} + err := sc.writeKey(key) + require.NoError(t, err) + issuer := &issuerEntry{ID: genIssuerId()} + err = sc.writeIssuer(issuer) + require.NoError(t, err) + // Verify we handle nothing stored properly keyConfigEmpty, err := sc.getKeysConfig() require.NoError(t, err) @@ -29,10 +37,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 = sc.setKeysConfig(origKeyConfig) @@ -46,7 +54,7 @@ func Test_ConfigsRoundTrip(t *testing.T) { issuerConfig, err := sc.getIssuersConfig() require.NoError(t, err) - require.Equal(t, origIssuerConfig, issuerConfig) + require.Equal(t, origIssuerConfig.DefaultIssuerId, issuerConfig.DefaultIssuerId) } func Test_IssuerRoundTrip(t *testing.T) { diff --git a/changelog/17824.txt b/changelog/17824.txt new file mode 100644 index 000000000..0d3fbb6ca --- /dev/null +++ b/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`. +``` diff --git a/website/content/api-docs/secret/pki.mdx b/website/content/api-docs/secret/pki.mdx index 91fff00bf..e937bbd08 100644 --- a/website/content/api-docs/secret/pki.mdx +++ b/website/content/api-docs/secret/pki.mdx @@ -2962,7 +2962,8 @@ $ curl \ ```json { "data": { - "default": "3dc79a5a-7a6c-70e2-1123-94b88557ba12" + "default": "3dc79a5a-7a6c-70e2-1123-94b88557ba12", + "default_follows_latest_issuer": "false" } } ``` @@ -2982,6 +2983,27 @@ This endpoint allows setting the value of the default issuer. either a name or an ID). When no value is specified and the path is `/pki/root/replace`, the default value of `"next"` will be used. +- `default_follows_latest_issuer` `(bool: false)` - Specifies whether a + root creation or an issuer import operation updates the default issuer + to the newly added issuer. + + While the new multi-issuer functionality of 1.11 was backwards compatible + on a per-API basis, some applications relied explicitly on unsafe behavior + across multiple APIs that we addressed. For instance, calling + `/intermediate/generate/:type` would silently remove any (potentially + in-use!) key material and generate new private keys. While our response to + this endpoint is backwards compatible (returning a new key and safely + preserving old keys), some applications implicitly relied on this behavior. + This new option is meant to provide compatibility across API calls to these + callers: the newly created issuer (once _imported_ -- not on intermediate + generation) will become the default and it will look (to anyone strictly + using old APIs) that it is the only issuer in the mount. However, it is + encouraged for applications to update to the newer, safer semantics + associated with [multi-issuer rotation](/docs/secrets/pki/rotation-primitives). + +~> Note: When an import creates more than one new issuer with key material + known to this mount, no default update will occur. + #### Sample Payload ```json