Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Backport of Optional automatic default issuer selection into release/1.11.x #17853

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions builtin/logical/pki/config_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
103 changes: 103 additions & 0 deletions builtin/logical/pki/integation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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, "")
}
Expand Down
45 changes: 36 additions & 9 deletions builtin/logical/pki/path_config_ca.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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) {
Expand All @@ -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
Expand All @@ -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
}
Expand Down
21 changes: 21 additions & 0 deletions builtin/logical/pki/path_manage_issuers.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
10 changes: 10 additions & 0 deletions builtin/logical/pki/path_root.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
22 changes: 20 additions & 2 deletions builtin/logical/pki/storage.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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) {
Expand All @@ -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
}
Expand Down
4 changes: 2 additions & 2 deletions builtin/logical/pki/storage_migrations_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
14 changes: 11 additions & 3 deletions builtin/logical/pki/storage_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
Expand All @@ -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) {
Expand Down
6 changes: 6 additions & 0 deletions builtin/logical/pki/test_helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -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...)
}
3 changes: 3 additions & 0 deletions changelog/17824.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:improvement
secrets/pki: Allow issuer creation, import to change default issuer via `default_follows_latest_issuer`.
```
Loading