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

Added role-option max_sts_ttl to cap TTL for AWS STS credentials. #5500

Merged
merged 3 commits into from
Oct 20, 2018
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
4 changes: 4 additions & 0 deletions builtin/logical/aws/backend_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -652,6 +652,7 @@ func testAccStepReadPolicy(t *testing.T, name string, value string) logicaltest.
"policy_document": value,
"credential_types": []string{iamUserCred, federationTokenCred},
"default_sts_ttl": int64(0),
"max_sts_ttl": int64(0),
}
if !reflect.DeepEqual(resp.Data, expected) {
return fmt.Errorf("bad: got: %#v\nexpected: %#v", resp.Data, expected)
Expand Down Expand Up @@ -749,6 +750,7 @@ func TestBackend_iamUserManagedInlinePolicies(t *testing.T) {
"credential_types": []string{iamUserCred},
"role_arns": []string(nil),
"default_sts_ttl": int64(0),
"max_sts_ttl": int64(0),
}
logicaltest.Test(t, logicaltest.TestCase{
AcceptanceTest: true,
Expand Down Expand Up @@ -828,6 +830,7 @@ func TestBackend_RoleDefaultSTSTTL(t *testing.T) {
"role_arns": []string{fmt.Sprintf("arn:aws:iam::%s:role/%s", awsAccountID, roleName)},
"credential_type": assumedRoleCred,
"default_sts_ttl": minAwsAssumeRoleDuration,
"max_sts_ttl": minAwsAssumeRoleDuration,
}
logicaltest.Test(t, logicaltest.TestCase{
AcceptanceTest: true,
Expand Down Expand Up @@ -883,6 +886,7 @@ func testAccStepReadArnPolicy(t *testing.T, name string, value string) logicalte
"policy_document": "",
"credential_types": []string{iamUserCred},
"default_sts_ttl": int64(0),
"max_sts_ttl": int64(0),
}
if !reflect.DeepEqual(resp.Data, expected) {
return fmt.Errorf("bad: got: %#v\nexpected: %#v", resp.Data, expected)
Expand Down
23 changes: 23 additions & 0 deletions builtin/logical/aws/path_roles.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,11 @@ GetFederationToken API call, acting as a filter on permissions available.`,
Description: fmt.Sprintf("Default TTL for %s and %s credential types when no TTL is explicitly requested with the credentials", assumedRoleCred, federationTokenCred),
},

"max_sts_ttl": &framework.FieldSchema{
Type: framework.TypeDurationSecond,
Description: fmt.Sprintf("Max allowed TTL for %s and %s credential types", assumedRoleCred, federationTokenCred),
},

"arn": &framework.FieldSchema{
Type: framework.TypeString,
Description: `Deprecated; use role_arns or policy_arns instead. ARN Reference to a managed policy
Expand Down Expand Up @@ -222,6 +227,22 @@ func (b *backend) pathRolesWrite(ctx context.Context, req *logical.Request, d *f
roleEntry.DefaultSTSTTL = time.Duration(defaultSTSTTLRaw.(int)) * time.Second
}

if maxSTSTTLRaw, ok := d.GetOk("max_sts_ttl"); ok {
if legacyRole != "" {
return logical.ErrorResponse("cannot supply deprecated role or policy parameters with max_sts_ttl"), nil
}
if !strutil.StrListContains(roleEntry.CredentialTypes, assumedRoleCred) && !strutil.StrListContains(roleEntry.CredentialTypes, federationTokenCred) {
return logical.ErrorResponse(fmt.Sprintf("max_sts_ttl parameter only valid for %s and %s credential types", assumedRoleCred, federationTokenCred)), nil
}

maxSTSTTL := time.Duration(maxSTSTTLRaw.(int)) * time.Second
if roleEntry.DefaultSTSTTL > 0 && roleEntry.DefaultSTSTTL > maxSTSTTL {
Copy link
Contributor

Choose a reason for hiding this comment

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

Two things:

First,m I think you should also test if maxSTSTL > 0. The reason is that, let's say a user has a max_sts_ttl of 3600 and a default_sts_ttl of 1200. Then, the user wants to remove the max_sts_ttl. That could be done by explicitly setting max_sts_ttl to 0, but then it would trip this check.

Second, you should probably move this check to after this if block. Imagine a user first sets max_sts_ttl to 1200, then in a second request, sets default_sts_ttl to 1800. That should probably also fail out, but I don't think it would.

return logical.ErrorResponse(`"default_sts_ttl" value must be less than "max_sts_ttl" value`), nil
}

roleEntry.MaxSTSTTL = maxSTSTTL
}

if legacyRole != "" {
roleEntry = upgradeLegacyPolicyEntry(legacyRole)
if roleEntry.InvalidData != "" {
Expand Down Expand Up @@ -402,6 +423,7 @@ type awsRoleEntry struct {
ProhibitFlexibleCredPath bool `json:"prohibit_flexible_cred_path,omitempty"` // Disallow accessing STS credentials via the creds path and vice verse
Version int `json:"version"` // Version number of the role format
DefaultSTSTTL time.Duration `json:"default_sts_ttl"` // Default TTL for STS credentials
MaxSTSTTL time.Duration `json:"max_sts_ttl"` // Max allowed TTL for STS credentials
}

func (r *awsRoleEntry) toResponseData() map[string]interface{} {
Expand All @@ -411,6 +433,7 @@ func (r *awsRoleEntry) toResponseData() map[string]interface{} {
"role_arns": r.RoleArns,
"policy_document": r.PolicyDocument,
"default_sts_ttl": int64(r.DefaultSTSTTL.Seconds()),
"max_sts_ttl": int64(r.MaxSTSTTL.Seconds()),
}
if r.InvalidData != "" {
respData["invalid_data"] = r.InvalidData
Expand Down
1 change: 1 addition & 0 deletions builtin/logical/aws/path_roles_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ func TestBackend_PathListRoles(t *testing.T) {
"role_arns": []string{"arn:aws:iam::123456789012:role/path/RoleName"},
"credential_type": assumedRoleCred,
"default_sts_ttl": 3600,
"max_sts_ttl": 3600,
}

roleReq := &logical.Request{
Expand Down
12 changes: 12 additions & 0 deletions builtin/logical/aws/path_user.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,18 @@ func (b *backend) pathCredsRead(ctx context.Context, req *logical.Request, d *fr
default:
ttl = int64(d.Get("ttl").(int))
}

var maxTTL int64
if role.MaxSTSTTL > 0 {
maxTTL = int64(role.MaxSTSTTL.Seconds())
} else {
maxTTL = int64(b.System().MaxLeaseTTL().Seconds())
}

if ttl > maxTTL {
ttl = maxTTL
}

roleArn := d.Get("role_arn").(string)

var credentialType string
Expand Down
4 changes: 4 additions & 0 deletions website/source/api/secret/aws/index.html.md
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,10 @@ updated with the new attributes.
on the role, then this default TTL will be used. Valid only when
`credential_type` is one of `assumed_role` or `federation_token`.

- `max_sts_ttl` `(string)` - The max allowed TTL for STS credentials (credentials
TTL are capped to `max_sts_ttl`). Valid only when `credential_type` is one of
`assumed_role` or `federation_token`.

Legacy parameters:

These parameters are supported for backwards compatibility only. They cannot be
Expand Down