Skip to content

Commit

Permalink
auth/aws: Allow lists in binds (#3907)
Browse files Browse the repository at this point in the history 
* auth/aws: Allow lists in binds

In the aws auth method, allow a number of binds to take in lists
instead of a single string value. The intended semantic is that, for
each bind type set, clients must match at least one of each of the bind
types set in order to authenticate.
  • Loading branch information
@joelthompson @jefferai
joelthompson authored and jefferai committed Mar 2, 2018
1 parent 4d419aa commit 8a115c7
Show file tree
Hide file tree
Showing 6 changed files with 391 additions and 236 deletions.
33 changes: 18 additions & 15 deletions builtin/credential/aws/backend_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1084,7 +1084,7 @@ func TestBackendAcc_LoginWithInstanceIdentityDocAndWhitelistIdentity(t *testing.
"auth_type": "ec2",
"policies": "root",
"max_ttl": "120s",
"bound_ami_id": "wrong_ami_id",
"bound_ami_id": []string{"wrong_ami_id", "wrong_ami_id2"},
"bound_account_id": accountID,
"bound_iam_role_arn": iamARN,
}
Expand All @@ -1108,10 +1108,10 @@ func TestBackendAcc_LoginWithInstanceIdentityDocAndWhitelistIdentity(t *testing.
t.Fatalf("bad: expected error response: resp:%#v\nerr:%v", resp, err)
}

// Place the correct AMI ID, but make the AccountID wrong
// Place the correct AMI ID in one of the values, but make the AccountID wrong
roleReq.Operation = logical.UpdateOperation
data["bound_ami_id"] = amiID
data["bound_account_id"] = "wrong-account-id"
data["bound_ami_id"] = []string{"wrong_ami_id_1", amiID, "wrong_ami_id_2"}
data["bound_account_id"] = []string{"wrong-account-id", "wrong-account-id-2"}
resp, err = b.HandleRequest(context.Background(), roleReq)
if err != nil || (resp != nil && resp.IsError()) {
t.Fatalf("bad: failed to create role: resp:%#v\nerr:%v", resp, err)
Expand All @@ -1123,9 +1123,9 @@ func TestBackendAcc_LoginWithInstanceIdentityDocAndWhitelistIdentity(t *testing.
t.Fatalf("bad: expected error response: resp:%#v\nerr:%v", resp, err)
}

// Place the correct AccountID, but make the wrong IAMRoleARN
data["bound_account_id"] = accountID
data["bound_iam_role_arn"] = "wrong_iam_role_arn"
// Place the correct AccountID in one of the values, but make the wrong IAMRoleARN
data["bound_account_id"] = []string{"wrong-account-id-1", accountID, "wrong-account-id-2"}
data["bound_iam_role_arn"] = []string{"wrong_iam_role_arn", "wrong_iam_role_arn_2"}
resp, err = b.HandleRequest(context.Background(), roleReq)
if err != nil || (resp != nil && resp.IsError()) {
t.Fatalf("bad: failed to create role: resp:%#v\nerr:%v", resp, err)
Expand All @@ -1137,8 +1137,8 @@ func TestBackendAcc_LoginWithInstanceIdentityDocAndWhitelistIdentity(t *testing.
t.Fatalf("bad: expected error response: resp:%#v\nerr:%v", resp, err)
}

// place the correct IAM role ARN
data["bound_iam_role_arn"] = iamARN
// place a correct IAM role ARN
data["bound_iam_role_arn"] = []string{"wrong_iam_role_arn_1", iamARN, "wrong_iam_role_arn_2"}
resp, err = b.HandleRequest(context.Background(), roleReq)
if err != nil || (resp != nil && resp.IsError()) {
t.Fatalf("bad: failed to create role: resp:%#v\nerr:%v", resp, err)
Expand Down Expand Up @@ -1456,7 +1456,7 @@ func TestBackendAcc_LoginWithCallerIdentity(t *testing.T) {

// configuring the valid role we'll be able to login to
roleData := map[string]interface{}{
"bound_iam_principal_arn": entity.canonicalArn(),
"bound_iam_principal_arn": []string{entity.canonicalArn(), "arn:aws:iam::123456789012:role/FakeRoleArn1*"}, // Fake ARN MUST be wildcard terminated because we're resolving unique IDs, and the wildcard termination prevents unique ID resolution
"policies": "root",
"auth_type": iamAuthType,
}
Expand Down Expand Up @@ -1489,16 +1489,19 @@ func TestBackendAcc_LoginWithCallerIdentity(t *testing.T) {
}

fakeArn := "arn:aws:iam::123456789012:role/somePath/FakeRole"
fakeArn2 := "arn:aws:iam::123456789012:role/somePath/FakeRole2"
fakeArnResolverCount := 0
fakeArnResolver := func(ctx context.Context, s logical.Storage, arn string) (string, error) {
if arn == fakeArn {
return fmt.Sprintf("FakeUniqueIdFor%s", fakeArn), nil
if strings.HasPrefix(arn, fakeArn) {
fakeArnResolverCount++
return fmt.Sprintf("FakeUniqueIdFor%s%d", arn, fakeArnResolverCount), nil
}
return b.resolveArnToRealUniqueId(context.Background(), s, arn)
}
b.resolveArnToUniqueIDFunc = fakeArnResolver

// now we're creating the invalid role we won't be able to login to
roleData["bound_iam_principal_arn"] = fakeArn
roleData["bound_iam_principal_arn"] = []string{fakeArn, fakeArn2}
roleRequest.Path = "role/" + testInvalidRoleName
resp, err = b.HandleRequest(context.Background(), roleRequest)
if err != nil || (resp != nil && resp.IsError()) {
Expand Down Expand Up @@ -1630,11 +1633,11 @@ func TestBackendAcc_LoginWithCallerIdentity(t *testing.T) {
wildcardRoleName := "valid_wildcard"
wildcardEntity := *entity
wildcardEntity.FriendlyName = "*"
roleData["bound_iam_principal_arn"] = wildcardEntity.canonicalArn()
roleData["bound_iam_principal_arn"] = []string{wildcardEntity.canonicalArn(), "arn:aws:iam::123456789012:role/DoesNotExist/Vault_Fake_Role*"}
roleRequest.Path = "role/" + wildcardRoleName
resp, err = b.HandleRequest(context.Background(), roleRequest)
if err != nil || (resp != nil && resp.IsError()) {
t.Fatalf("bad: failed to create wildcard role: resp:%#v\nerr:%v", resp, err)
t.Fatalf("bad: failed to create wildcard roles: resp:%#v\nerr:%v", resp, err)
}

loginData["role"] = wildcardRoleName
Expand Down
103 changes: 66 additions & 37 deletions builtin/credential/aws/path_login.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -386,7 +386,7 @@ func (b *backend) verifyInstanceMeetsRoleRequirements(ctx context.Context,

// Verify that the AccountID of the instance trying to login matches the
// AccountID specified as a constraint on role
if roleEntry.BoundAccountID != "" && identityDoc.AccountID != roleEntry.BoundAccountID {
if len(roleEntry.BoundAccountIDs) > 0 && !strutil.StrListContains(roleEntry.BoundAccountIDs, identityDoc.AccountID) {
return fmt.Errorf("account ID %q does not belong to role %q", identityDoc.AccountID, roleName), nil
}

Expand All @@ -399,54 +399,61 @@ func (b *backend) verifyInstanceMeetsRoleRequirements(ctx context.Context,
// already calling the API to validate the Instance ID anyway, so it shouldn't
// matter. The benefit is that we have the exact same code whether auth_type
// is ec2 or iam.
if roleEntry.BoundAmiID != "" {
if len(roleEntry.BoundAmiIDs) > 0 {
if instance.ImageId == nil {
return nil, fmt.Errorf("AMI ID in the instance description is nil")
}
if roleEntry.BoundAmiID != *instance.ImageId {
if !strutil.StrListContains(roleEntry.BoundAmiIDs, *instance.ImageId) {
return fmt.Errorf("AMI ID %q does not belong to role %q", instance.ImageId, roleName), nil
}
}

// Validate the SubnetID if corresponding bound was set on the role
if roleEntry.BoundSubnetID != "" {
if len(roleEntry.BoundSubnetIDs) > 0 {
if instance.SubnetId == nil {
return nil, fmt.Errorf("subnet ID in the instance description is nil")
}
if roleEntry.BoundSubnetID != *instance.SubnetId {
if !strutil.StrListContains(roleEntry.BoundSubnetIDs, *instance.SubnetId) {
return fmt.Errorf("subnet ID %q does not satisfy the constraint on role %q", *instance.SubnetId, roleName), nil
}
}

// Validate the VpcID if corresponding bound was set on the role
if roleEntry.BoundVpcID != "" {
if len(roleEntry.BoundVpcIDs) > 0 {
if instance.VpcId == nil {
return nil, fmt.Errorf("VPC ID in the instance description is nil")
}
if roleEntry.BoundVpcID != *instance.VpcId {
if !strutil.StrListContains(roleEntry.BoundVpcIDs, *instance.VpcId) {
return fmt.Errorf("VPC ID %q does not satisfy the constraint on role %q", *instance.VpcId, roleName), nil
}
}

// Check if the IAM instance profile ARN of the instance trying to
// login, matches the IAM instance profile ARN specified as a constraint
// on the role
if roleEntry.BoundIamInstanceProfileARN != "" {
if len(roleEntry.BoundIamInstanceProfileARNs) > 0 {
if instance.IamInstanceProfile == nil {
return nil, fmt.Errorf("IAM instance profile in the instance description is nil")
}
if instance.IamInstanceProfile.Arn == nil {
return nil, fmt.Errorf("IAM instance profile ARN in the instance description is nil")
}
iamInstanceProfileARN := *instance.IamInstanceProfile.Arn
if !strings.HasPrefix(iamInstanceProfileARN, roleEntry.BoundIamInstanceProfileARN) {
matchesInstanceProfile := false
for _, boundInstanceProfileARN := range roleEntry.BoundIamInstanceProfileARNs {
if strings.HasPrefix(iamInstanceProfileARN, boundInstanceProfileARN) {
matchesInstanceProfile = true
break
}
}
if !matchesInstanceProfile {
return fmt.Errorf("IAM instance profile ARN %q does not satisfy the constraint role %q", iamInstanceProfileARN, roleName), nil
}
}

// Check if the IAM role ARN of the instance trying to login, matches
// the IAM role ARN specified as a constraint on the role.
if roleEntry.BoundIamRoleARN != "" {
if len(roleEntry.BoundIamRoleARNs) > 0 {
if instance.IamInstanceProfile == nil {
return nil, fmt.Errorf("IAM instance profile in the instance description is nil")
}
Expand Down Expand Up @@ -484,7 +491,14 @@ func (b *backend) verifyInstanceMeetsRoleRequirements(ctx context.Context,
return nil, fmt.Errorf("IAM role ARN could not be fetched")
}

if !strings.HasPrefix(iamRoleARN, roleEntry.BoundIamRoleARN) {
matchesInstanceRoleARN := false
for _, boundIamRoleARN := range roleEntry.BoundIamRoleARNs {
if strings.HasPrefix(iamRoleARN, boundIamRoleARN) {
matchesInstanceRoleARN = true
break
}
}
if !matchesInstanceRoleARN {
return fmt.Errorf("IAM role ARN %q does not satisfy the constraint role %q", iamRoleARN, roleName), nil
}
}
Expand Down Expand Up @@ -588,7 +602,7 @@ func (b *backend) pathLoginUpdateEc2(ctx context.Context, req *logical.Request,

// Verify that the `Region` of the instance trying to login matches the
// `Region` specified as a constraint on role
if roleEntry.BoundRegion != "" && identityDocParsed.Region != roleEntry.BoundRegion {
if len(roleEntry.BoundRegions) > 0 && !strutil.StrListContains(roleEntry.BoundRegions, identityDocParsed.Region) {
return logical.ErrorResponse(fmt.Sprintf("Region %q does not satisfy the constraint on role %q", identityDocParsed.Region, roleName)), nil
}

Expand Down Expand Up @@ -939,17 +953,20 @@ func (b *backend) pathLoginRenewIam(ctx context.Context, req *logical.Request, d
// read the role directly to know what the bind is. It's a relatively small amount of leakage, in
// some fairly corner cases, and in the most likely error case (role has been changed to a new ARN),
// the error message is identical.
if roleEntry.BoundIamPrincipalARN != "" {
if len(roleEntry.BoundIamPrincipalARNs) > 0 {
// We might not get here if all bindings were on the inferred entity, which we've already validated
// above
// As with logins, there are three ways to pass this check:
// 1: clientUserId is in roleEntry.BoundIamPrincipalIDs (entries in roleEntry.BoundIamPrincipalIDs
// implies that roleEntry.ResolveAWSUniqueIDs is true)
// 2: roleEntry.ResolveAWSUniqueIDs is false and canonical_arn is in roleEntry.BoundIamPrincipalARNs
// 3: Full ARN matches one of the wildcard globs in roleEntry.BoundIamPrincipalARNs
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 ARN %q", canonicalArn)
}
} else if strings.HasSuffix(roleEntry.BoundIamPrincipalARN, "*") {
switch {
case ok && strutil.StrListContains(roleEntry.BoundIamPrincipalIDs, clientUserId): // check 1 passed
case !roleEntry.ResolveAWSUniqueIDs && strutil.StrListContains(roleEntry.BoundIamPrincipalARNs, canonicalArn): // check 2 passed
default:
// check 3 is a bit more complex, so we do it last
fullArn := b.getCachedUserId(clientUserId)
if fullArn == "" {
entity, err := parseIamArn(canonicalArn)
Expand All @@ -967,11 +984,16 @@ func (b *backend) pathLoginRenewIam(ctx context.Context, req *logical.Request, d
b.setCachedUserId(clientUserId, fullArn)
}
}
if !strutil.GlobbedStringsMatch(roleEntry.BoundIamPrincipalARN, fullArn) {
matchedWildcardBind := false
for _, principalARN := range roleEntry.BoundIamPrincipalARNs {
if strings.HasSuffix(principalARN, "*") && strutil.GlobbedStringsMatch(principalARN, fullArn) {
matchedWildcardBind = true
break
}
}
if !matchedWildcardBind {
return nil, fmt.Errorf("role no longer bound to ARN %q", canonicalArn)
}
} else if roleEntry.BoundIamPrincipalARN != canonicalArn {
return nil, fmt.Errorf("role no longer bound to ARN %q", canonicalArn)
}
}

Expand Down Expand Up @@ -1189,15 +1211,19 @@ func (b *backend) pathLoginUpdateIam(ctx context.Context, req *logical.Request,

// The role creation should ensure that either we're inferring this is an EC2 instance
// or that we're binding an ARN
// The only way BoundIamPrincipalID could get set is if BoundIamPrincipalARN was also set and
// resolving to internal IDs was turned on, which can't be turned off. So, there should be no
// way for this to be set and not match BoundIamPrincipalARN
if roleEntry.BoundIamPrincipalID != "" {
if callerUniqueId != roleEntry.BoundIamPrincipalID {
return logical.ErrorResponse(fmt.Sprintf("expected IAM %s %s to resolve to unique AWS ID %q but got %q instead", entity.Type, entity.FriendlyName, roleEntry.BoundIamPrincipalID, callerUniqueId)), nil
}
} else if roleEntry.BoundIamPrincipalARN != "" {
if strings.HasSuffix(roleEntry.BoundIamPrincipalARN, "*") {
if len(roleEntry.BoundIamPrincipalARNs) > 0 {
// As with renews, there are three ways to pass this check:
// 1: callerUniqueId is in roleEntry.BoundIamPrincipalIDs (entries in roleEntry.BoundIamPrincipalIDs
// implies that roleEntry.ResolveAWSUniqueIDs is true)
// 2: roleEntry.ResolveAWSUniqueIDs is false and entity.canonicalArn() is in roleEntry.BoundIamPrincipalARNs
// 3: Full ARN matches one of the wildcard globs in roleEntry.BoundIamPrincipalARNs
// Need to be able to handle pathological configurations such as roleEntry.BoundIamPrincipalARNs looking something like:
// arn:aw:iam::123456789012:{user/UserName,user/path/*,role/RoleName,role/path/*}
switch {
case strutil.StrListContains(roleEntry.BoundIamPrincipalIDs, callerUniqueId): // check 1 passed
case !roleEntry.ResolveAWSUniqueIDs && strutil.StrListContains(roleEntry.BoundIamPrincipalARNs, entity.canonicalArn()): // check 2 passed
default:
// evaluate check 3
fullArn := b.getCachedUserId(callerUniqueId)
if fullArn == "" {
fullArn, err = b.fullArn(ctx, entity, req.Storage)
Expand All @@ -1209,13 +1235,16 @@ func (b *backend) pathLoginUpdateIam(ctx context.Context, req *logical.Request,
}
b.setCachedUserId(callerUniqueId, fullArn)
}
if !strutil.GlobbedStringsMatch(roleEntry.BoundIamPrincipalARN, fullArn) {
// Note: Intentionally giving the exact same error message as a few lines below. Otherwise, we might leak information
// about whether the bound IAM principal ARN is a wildcard or not, and what that wildcard is.
matchedWildcardBind := false
for _, principalARN := range roleEntry.BoundIamPrincipalARNs {
if strings.HasSuffix(principalARN, "*") && strutil.GlobbedStringsMatch(principalARN, fullArn) {
matchedWildcardBind = true
break
}
}
if !matchedWildcardBind {
return logical.ErrorResponse(fmt.Sprintf("IAM Principal %q does not belong to the role %q", callerID.Arn, roleName)), nil
}
} else if roleEntry.BoundIamPrincipalARN != entity.canonicalArn() {
return logical.ErrorResponse(fmt.Sprintf("IAM Principal %q does not belong to the role %q", callerID.Arn, roleName)), nil
}
}

Expand Down
Loading

0 comments on commit 8a115c7

Please sign in to comment.