Skip to content

Commit

Permalink
Look up proper AWS account ID on aws token renew (#3012)
Browse files Browse the repository at this point in the history
Also properly handle renewing tokens when bound_iam_principal_arn has a
path component.

Fixes #2990
  • Loading branch information
joelthompson authored and jefferai committed Jul 17, 2017
1 parent af7c2ab commit 284b346
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 23 deletions.
5 changes: 4 additions & 1 deletion builtin/credential/aws/backend_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1428,7 +1428,7 @@ func TestBackendAcc_LoginWithCallerIdentity(t *testing.T) {
t.Fatalf("bad: failed to create role; resp:%#v\nerr:%v", resp, err)
}

fakeArn := "arn:aws:iam::123456789012:role/FakeRole"
fakeArn := "arn:aws:iam::123456789012:role/somePath/FakeRole"
fakeArnResolver := func(s logical.Storage, arn string) (string, error) {
if arn == fakeArn {
return fmt.Sprintf("FakeUniqueIdFor%s", fakeArn), nil
Expand Down Expand Up @@ -1535,6 +1535,9 @@ func TestBackendAcc_LoginWithCallerIdentity(t *testing.T) {
renewReq.Auth.LeaseOptions = resp.Auth.LeaseOptions
renewReq.Auth.Policies = resp.Auth.Policies
renewReq.Auth.IssueTime = time.Now()
// dump a fake ARN into the metadata to ensure that we ONLY look
// at the unique ID that has been generated
renewReq.Auth.Metadata["canonical_arn"] = "fake_arn"
// ensure we can renew
resp, err = b.pathLoginRenew(renewReq, empty_login_fd)
if err != nil {
Expand Down
36 changes: 14 additions & 22 deletions builtin/credential/aws/path_login.go
Original file line number Diff line number Diff line change
Expand Up @@ -918,7 +918,7 @@ func (b *backend) pathLoginRenewIam(
if !ok {
return nil, fmt.Errorf("no inferred AWS region in auth metadata")
}
_, err := b.validateInstance(req.Storage, instanceID, instanceRegion, req.Auth.Metadata["accountID"])
_, err := b.validateInstance(req.Storage, instanceID, instanceRegion, req.Auth.Metadata["account_id"])
if err != nil {
return nil, fmt.Errorf("failed to verify instance ID %q: %v", instanceID, err)
}
Expand All @@ -927,30 +927,22 @@ func (b *backend) pathLoginRenewIam(
}
}

// The role may have been specified with only bindings on the inferred entity. The
// creation/modification of roles ensures that there is always at least one valid binding, and
// those bindings are either with a bound_iam_principal_arn or bindings on the inferred enttity.
// If the bound_iam_principal_arn is set to "", then that means all the bindings were set on the
// inferred entity type and already checked, so we don't need to check bound_iam_principal_arn.
if roleEntry.BoundIamPrincipalARN != "" && roleEntry.BoundIamPrincipalARN != canonicalArn {
return nil, fmt.Errorf("role no longer bound to arn %q", canonicalArn)
}
// Need to hanndle the case where an auth token was generated before we put client_user_id in the metadata
// Basic goal here is:
// 1. If no client_user_id metadata exists, then skip the check (it might be nice to fill it in later, but
// could be complicated)
// 2. If role is not bound to an ID, that means that checking the unique ID has been disabled, so skip the
// check
// 3. Otherwise, ensure that the stored client_user_id matches the bound IAM principal ID. If an IAM user
// or role is deleted and recreated, then existing clients will NOT be able to renew and they'll need
// to reauthenticate to Vault with updated IAM credentials
originalUserId, ok := req.Auth.Metadata["client_user_id"]
if ok && roleEntry.BoundIamPrincipalID != "" && roleEntry.BoundIamPrincipalID != req.Auth.Metadata["client_user_id"] {
return nil, fmt.Errorf("role no longer bound to ID %q", originalUserId)
if roleEntry.BoundIamPrincipalARN != "" {
// We might not get here if all bindings were on the inferred entity, which we've already validated
// above
clientUserId, ok := req.Auth.Metadata["client_user_id"]
if ok && roleEntry.BoundIamPrincipalID != "" {
// Resolving unique IDs is enabled and the auth metadata contains the unique ID, so checking the
// unique ID is authoritative at this stage
if roleEntry.BoundIamPrincipalID != clientUserId {
return nil, fmt.Errorf("role no longer bound to ID %q", clientUserId)
}
} else if roleEntry.BoundIamPrincipalARN != canonicalArn {
return nil, fmt.Errorf("role no longer bound to arn %q", canonicalArn)
}
}

return framework.LeaseExtend(roleEntry.TTL, roleEntry.MaxTTL, b.System())(req, data)

}

func (b *backend) pathLoginRenewEc2(
Expand Down

0 comments on commit 284b346

Please sign in to comment.