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

Approle local secret IDs #4427

Merged
merged 23 commits into from
Apr 24, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
953c7fb
local secret IDs
vishalnayak Apr 23, 2018
f8055c8
add prefix to LocalStorage
vishalnayak Apr 23, 2018
4ee66b5
fix path regex and role storage
vishalnayak Apr 23, 2018
52efa5e
Fix the tidy operation to consider both local and non-local secretID …
vishalnayak Apr 23, 2018
3d7e704
segregate local and non-local accessor entries
vishalnayak Apr 23, 2018
184dac8
Upgrade secret ID prefix and fix tests
vishalnayak Apr 23, 2018
b929187
naming changes
vishalnayak Apr 23, 2018
b4f6b6f
update docs
vishalnayak Apr 23, 2018
20c7f20
error on enable_local_secret_ids update after role creation
vishalnayak Apr 23, 2018
83aabbb
Add enable_local_secret_ids to role read response
vishalnayak Apr 24, 2018
f39f405
Add immutability test
vishalnayak Apr 24, 2018
42e95d4
Add tests
vishalnayak Apr 24, 2018
a7814f3
Merge branch 'master-oss' into approle-local-secretid
vishalnayak Apr 24, 2018
0962457
Fix api path for reading the field
vishalnayak Apr 24, 2018
33256ab
Add field read test
vishalnayak Apr 24, 2018
3f92d9c
remove unneeded setting of secret ID prefix
vishalnayak Apr 24, 2018
417b004
fix typo
vishalnayak Apr 24, 2018
419e70c
refactor to be able to defer lock.Unlock()
vishalnayak Apr 24, 2018
4222df3
Merge branch 'master-oss' into approle-local-secretid
vishalnayak Apr 24, 2018
3c49d7b
remove unneeded comments
vishalnayak Apr 24, 2018
a030db2
s/enable_local_secret_ids/local_secret_ids
vishalnayak Apr 24, 2018
cce5861
Merge branch 'master-oss' into approle-local-secretid
vishalnayak Apr 24, 2018
7837f11
Merge branch 'master' into approle-local-secretid
vishalnayak Apr 24, 2018
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
11 changes: 11 additions & 0 deletions builtin/credential/approle/backend.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,13 @@ import (
"github.com/hashicorp/vault/logical/framework"
)

const (
secretIDPrefix = "secret_id/"
secretIDLocalPrefix = "secret_id_local/"
secretIDAccessorPrefix = "accessor/"
secretIDAccessorLocalPrefix = "accessor_local/"
)

type backend struct {
*framework.Backend

Expand Down Expand Up @@ -90,6 +97,10 @@ func Backend(conf *logical.BackendConfig) (*backend, error) {
Unauthenticated: []string{
"login",
},
LocalStorage: []string{
secretIDLocalPrefix,
secretIDAccessorLocalPrefix,
},
},
Paths: framework.PathAppend(
rolePaths(b),
Expand Down
105 changes: 92 additions & 13 deletions builtin/credential/approle/path_role.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,10 @@ type roleStorageEntry struct {
// LowerCaseRoleName enforces the lower casing of role names for all the
// roles that get created since this field was introduced.
LowerCaseRoleName bool `json:"lower_case_role_name" mapstructure:"lower_case_role_name" structs:"lower_case_role_name"`

// SecretIDPrefix is the storage prefix for persisting secret IDs. This
// differs based on whether the secret IDs are cluster local or not.
SecretIDPrefix string `json:"secret_id_prefix" mapstructure:"secret_id_prefix" structs:"secret_id_prefix"`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Feels like this should be a bool, or an iota, rather than storing the prefix over and over.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I see why you did this.

}

// roleIDStorageEntry represents the reverse mapping from RoleID to Role
Expand Down Expand Up @@ -163,6 +167,11 @@ TTL will be set to the value of this parameter.`,
Type: framework.TypeString,
Description: "Identifier of the role. Defaults to a UUID.",
},
"local_secret_ids": &framework.FieldSchema{
Type: framework.TypeBool,
Description: `If set, the secret IDs generated using this role will be cluster local. This
can only be set during role creation and once set, it can't be reset later.`,
},
},
ExistenceCheck: b.pathRoleExistenceCheck,
Callbacks: map[logical.Operation]framework.OperationFunc{
Expand All @@ -174,6 +183,20 @@ TTL will be set to the value of this parameter.`,
HelpSynopsis: strings.TrimSpace(roleHelp["role"][0]),
HelpDescription: strings.TrimSpace(roleHelp["role"][1]),
},
&framework.Path{
Pattern: "role/" + framework.GenericNameRegex("role_name") + "/local-secret-ids$",
Fields: map[string]*framework.FieldSchema{
"role_name": &framework.FieldSchema{
Type: framework.TypeString,
Description: "Name of the role.",
},
},
Callbacks: map[logical.Operation]framework.OperationFunc{
logical.ReadOperation: b.pathRoleLocalSecretIDsRead,
},
HelpSynopsis: strings.TrimSpace(roleHelp["role-local-secret-ids"][0]),
HelpDescription: strings.TrimSpace(roleHelp["role-local-secret-ids"][1]),
},
&framework.Path{
Pattern: "role/" + framework.GenericNameRegex("role_name") + "/policies$",
Fields: map[string]*framework.FieldSchema{
Expand Down Expand Up @@ -585,7 +608,7 @@ func (b *backend) pathRoleSecretIDList(ctx context.Context, req *logical.Request

// Listing works one level at a time. Get the first level of data
// which could then be used to get the actual SecretID storage entries.
secretIDHMACs, err := req.Storage.List(ctx, fmt.Sprintf("secret_id/%s/", roleNameHMAC))
secretIDHMACs, err := req.Storage.List(ctx, fmt.Sprintf("%s%s/", role.SecretIDPrefix, roleNameHMAC))
if err != nil {
return nil, err
}
Expand All @@ -598,7 +621,7 @@ func (b *backend) pathRoleSecretIDList(ctx context.Context, req *logical.Request
}

// Prepare the full index of the SecretIDs.
entryIndex := fmt.Sprintf("secret_id/%s/%s", roleNameHMAC, secretIDHMAC)
entryIndex := fmt.Sprintf("%s%s/%s", role.SecretIDPrefix, roleNameHMAC, secretIDHMAC)

// SecretID locks are not indexed by SecretIDs itself.
// This is because SecretIDs are not stored in plaintext
Expand Down Expand Up @@ -731,6 +754,11 @@ func (b *backend) roleEntry(ctx context.Context, s logical.Storage, roleName str
needsUpgrade = true
}

if role.SecretIDPrefix == "" {
role.SecretIDPrefix = secretIDPrefix
needsUpgrade = true
}

if needsUpgrade && (b.System().LocalMount() || !b.System().ReplicationState().HasState(consts.ReplicationPerformanceSecondary)) {
entry, err := logical.StorageEntryJSON("role/"+strings.ToLower(roleName), &role)
if err != nil {
Expand Down Expand Up @@ -776,7 +804,20 @@ func (b *backend) pathRoleCreateUpdate(ctx context.Context, req *logical.Request
LowerCaseRoleName: true,
}
} else if role == nil {
return logical.ErrorResponse(fmt.Sprintf("invalid role name")), nil
return logical.ErrorResponse(fmt.Sprintf("role name %q doesn't exist", roleName)), nil
}

localSecretIDsRaw, ok := data.GetOk("local_secret_ids")
if ok {
switch {
case req.Operation == logical.CreateOperation:
localSecretIDs := localSecretIDsRaw.(bool)
if localSecretIDs {
role.SecretIDPrefix = secretIDLocalPrefix
}
default:
return logical.ErrorResponse("local_secret_ids can only be modified during role creation"), nil
}
}

previousRoleID := role.RoleID
Expand Down Expand Up @@ -916,6 +957,11 @@ func (b *backend) pathRoleRead(ctx context.Context, req *logical.Request, data *
"token_max_ttl": role.TokenMaxTTL / time.Second,
"token_num_uses": role.TokenNumUses,
"token_ttl": role.TokenTTL / time.Second,
"local_secret_ids": false,
}

if role.SecretIDPrefix == secretIDLocalPrefix {
respData["local_secret_ids"] = true
}

resp := &logical.Response{
Expand Down Expand Up @@ -985,7 +1031,7 @@ func (b *backend) pathRoleDelete(ctx context.Context, req *logical.Request, data
}

// Just before the role is deleted, remove all the SecretIDs issued as part of the role.
if err = b.flushRoleSecrets(ctx, req.Storage, roleName, role.HMACKey); err != nil {
if err = b.flushRoleSecrets(ctx, req.Storage, roleName, role.HMACKey, role.SecretIDPrefix); err != nil {
return nil, errwrap.Wrapf(fmt.Sprintf("failed to invalidate the secrets belonging to role %q: {{err}}", roleName), err)
}

Expand Down Expand Up @@ -1044,7 +1090,7 @@ func (b *backend) pathRoleSecretIDLookupUpdate(ctx context.Context, req *logical
}

// Create the index at which the secret_id would've been stored
entryIndex := fmt.Sprintf("secret_id/%s/%s", roleNameHMAC, secretIDHMAC)
entryIndex := fmt.Sprintf("%s%s/%s", role.SecretIDPrefix, roleNameHMAC, secretIDHMAC)

return b.secretIDCommon(ctx, req.Storage, entryIndex, secretIDHMAC)
}
Expand Down Expand Up @@ -1119,7 +1165,7 @@ func (b *backend) pathRoleSecretIDDestroyUpdateDelete(ctx context.Context, req *
return nil, errwrap.Wrapf("failed to create HMAC of role_name: {{err}}", err)
}

entryIndex := fmt.Sprintf("secret_id/%s/%s", roleNameHMAC, secretIDHMAC)
entryIndex := fmt.Sprintf("%s%s/%s", role.SecretIDPrefix, roleNameHMAC, secretIDHMAC)

lock := b.secretIDLock(secretIDHMAC)
lock.Lock()
Expand All @@ -1135,7 +1181,7 @@ func (b *backend) pathRoleSecretIDDestroyUpdateDelete(ctx context.Context, req *
}

// Delete the accessor of the SecretID first
if err := b.deleteSecretIDAccessorEntry(ctx, req.Storage, result.SecretIDAccessor); err != nil {
if err := b.deleteSecretIDAccessorEntry(ctx, req.Storage, result.SecretIDAccessor, role.SecretIDPrefix); err != nil {
return nil, err
}

Expand Down Expand Up @@ -1176,7 +1222,7 @@ func (b *backend) pathRoleSecretIDAccessorLookupUpdate(ctx context.Context, req
return nil, fmt.Errorf("role %q does not exist", roleName)
}

accessorEntry, err := b.secretIDAccessorEntry(ctx, req.Storage, secretIDAccessor)
accessorEntry, err := b.secretIDAccessorEntry(ctx, req.Storage, secretIDAccessor, role.SecretIDPrefix)
if err != nil {
return nil, err
}
Expand All @@ -1189,7 +1235,7 @@ func (b *backend) pathRoleSecretIDAccessorLookupUpdate(ctx context.Context, req
return nil, errwrap.Wrapf("failed to create HMAC of role_name: {{err}}", err)
}

entryIndex := fmt.Sprintf("secret_id/%s/%s", roleNameHMAC, accessorEntry.SecretIDHMAC)
entryIndex := fmt.Sprintf("%s%s/%s", role.SecretIDPrefix, roleNameHMAC, accessorEntry.SecretIDHMAC)

return b.secretIDCommon(ctx, req.Storage, entryIndex, accessorEntry.SecretIDHMAC)
}
Expand Down Expand Up @@ -1217,7 +1263,7 @@ func (b *backend) pathRoleSecretIDAccessorDestroyUpdateDelete(ctx context.Contex
return nil, fmt.Errorf("role %q does not exist", roleName)
}

accessorEntry, err := b.secretIDAccessorEntry(ctx, req.Storage, secretIDAccessor)
accessorEntry, err := b.secretIDAccessorEntry(ctx, req.Storage, secretIDAccessor, role.SecretIDPrefix)
if err != nil {
return nil, err
}
Expand All @@ -1230,14 +1276,14 @@ func (b *backend) pathRoleSecretIDAccessorDestroyUpdateDelete(ctx context.Contex
return nil, errwrap.Wrapf("failed to create HMAC of role_name: {{err}}", err)
}

entryIndex := fmt.Sprintf("secret_id/%s/%s", roleNameHMAC, accessorEntry.SecretIDHMAC)
entryIndex := fmt.Sprintf("%s%s/%s", role.SecretIDPrefix, roleNameHMAC, accessorEntry.SecretIDHMAC)

lock := b.secretIDLock(accessorEntry.SecretIDHMAC)
lock.Lock()
defer lock.Unlock()

// Delete the accessor of the SecretID first
if err := b.deleteSecretIDAccessorEntry(ctx, req.Storage, secretIDAccessor); err != nil {
if err := b.deleteSecretIDAccessorEntry(ctx, req.Storage, secretIDAccessor, role.SecretIDPrefix); err != nil {
return nil, err
}

Expand Down Expand Up @@ -1404,6 +1450,33 @@ func (b *backend) pathRoleBindSecretIDDelete(ctx context.Context, req *logical.R
return nil, b.setRoleEntry(ctx, req.Storage, roleName, role, "")
}

func (b *backend) pathRoleLocalSecretIDsRead(ctx context.Context, req *logical.Request, data *framework.FieldData) (*logical.Response, error) {
roleName := data.Get("role_name").(string)
if roleName == "" {
return logical.ErrorResponse("missing role_name"), nil
}

lock := b.roleLock(roleName)
lock.RLock()
defer lock.RUnlock()

if role, err := b.roleEntry(ctx, req.Storage, strings.ToLower(roleName)); err != nil {
return nil, err
} else if role == nil {
return nil, nil
} else {
localSecretIDs := false
if role.SecretIDPrefix == secretIDLocalPrefix {
localSecretIDs = true
}
return &logical.Response{
Data: map[string]interface{}{
"local_secret_ids": localSecretIDs,
},
}, nil
}
}

func (b *backend) pathRolePoliciesUpdate(ctx context.Context, req *logical.Request, data *framework.FieldData) (*logical.Response, error) {
roleName := data.Get("role_name").(string)
if roleName == "" {
Expand Down Expand Up @@ -2047,7 +2120,7 @@ func (b *backend) handleRoleSecretIDCommon(ctx context.Context, req *logical.Req
roleName = strings.ToLower(roleName)
}

if secretIDStorage, err = b.registerSecretIDEntry(ctx, req.Storage, roleName, secretID, role.HMACKey, secretIDStorage); err != nil {
if secretIDStorage, err = b.registerSecretIDEntry(ctx, req.Storage, roleName, secretID, role.HMACKey, role.SecretIDPrefix, secretIDStorage); err != nil {
return nil, errwrap.Wrapf("failed to store secret_id: {{err}}", err)
}

Expand Down Expand Up @@ -2265,4 +2338,10 @@ duration specified by this value. The renewal duration will
be fixed. If the Period in the role is modified, the token
will pick up the new value during its next renewal.`,
},
"role-local-secret-ids": {
"Enables cluster local secret IDs",
`If set, the secret IDs generated using this role will be cluster local.
This can only be set during role creation and once set, it can't be
reset later.`,
},
}
Loading