From d349f5b0a7f65df091cff60cbf4c865e0374abff Mon Sep 17 00:00:00 2001 From: Joel Thompson Date: Thu, 15 Mar 2018 10:19:28 -0600 Subject: [PATCH] auth/aws: Allow binding by EC2 instance IDs (#3816) * auth/aws: Allow binding by EC2 instance IDs This allows specifying a list of EC2 instance IDs that are allowed to bind to the role. To keep style formatting with the other bindings, this is still called bound_ec2_instance_id rather than bound_ec2_instance_ids as I intend to convert the other bindings to accept lists as well (where it makes sense) and keeping them with singular names would be the easiest for backwards compatibility. Partially fixes #3797 --- builtin/credential/aws/backend_test.go | 34 +++++++++++++++++---- builtin/credential/aws/path_login.go | 5 ++++ builtin/credential/aws/path_role.go | 36 ++++++++++++++++++----- builtin/credential/aws/path_role_test.go | 2 ++ website/source/api/auth/aws/index.html.md | 5 ++++ 5 files changed, 68 insertions(+), 14 deletions(-) diff --git a/builtin/credential/aws/backend_test.go b/builtin/credential/aws/backend_test.go index 2e8f0eb3e9d7..84b667a4a362 100644 --- a/builtin/credential/aws/backend_test.go +++ b/builtin/credential/aws/backend_test.go @@ -1070,6 +1070,11 @@ func TestBackendAcc_LoginWithInstanceIdentityDocAndWhitelistIdentity(t *testing. "nonce": "vault-client-nonce", } + parsedIdentityDoc, err := b.parseIdentityDocument(context.Background(), storage, pkcs7) + if err != nil { + t.Fatal(err) + } + // Perform the login operation with a AMI ID that is not matching // the bound on the role. loginRequest := &logical.Request{ @@ -1081,12 +1086,13 @@ func TestBackendAcc_LoginWithInstanceIdentityDocAndWhitelistIdentity(t *testing. // Place the wrong AMI ID in the role data. data := map[string]interface{}{ - "auth_type": "ec2", - "policies": "root", - "max_ttl": "120s", - "bound_ami_id": []string{"wrong_ami_id", "wrong_ami_id2"}, - "bound_account_id": accountID, - "bound_iam_role_arn": iamARN, + "auth_type": "ec2", + "policies": "root", + "max_ttl": "120s", + "bound_ami_id": []string{"wrong_ami_id", "wrong_ami_id2"}, + "bound_account_id": accountID, + "bound_iam_role_arn": iamARN, + "bound_ec2_instance_id": []string{parsedIdentityDoc.InstanceID, "i-1234567"}, } roleReq := &logical.Request{ @@ -1139,6 +1145,19 @@ func TestBackendAcc_LoginWithInstanceIdentityDocAndWhitelistIdentity(t *testing. // place a correct IAM role ARN data["bound_iam_role_arn"] = []string{"wrong_iam_role_arn_1", iamARN, "wrong_iam_role_arn_2"} + data["bound_ec2_instance_id"] = "i-1234567" + 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) + } + // Attempt to login and expect a fail because instance ID is wrong + resp, err = b.HandleRequest(context.Background(), loginRequest) + if err != nil || resp == nil || (resp != nil && !resp.IsError()) { + t.Fatalf("bad: expected error response: resp:%#v\nerr:%v", resp, err) + } + + // place a correct EC2 Instance ID + data["bound_ec2_instance_id"] = []string{parsedIdentityDoc.InstanceID, "i-1234567"} 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) @@ -1170,6 +1189,9 @@ func TestBackendAcc_LoginWithInstanceIdentityDocAndWhitelistIdentity(t *testing. if instanceID == "" { t.Fatalf("instance ID not present in the response object") } + if instanceID != parsedIdentityDoc.InstanceID { + t.Fatalf("instance ID in response (%q) did not match instance ID from identity document (%q)", instanceID, parsedIdentityDoc.InstanceID) + } _, ok := resp.Auth.Metadata["nonce"] if ok { diff --git a/builtin/credential/aws/path_login.go b/builtin/credential/aws/path_login.go index 4a58f23cb2de..3a59d34e07ae 100644 --- a/builtin/credential/aws/path_login.go +++ b/builtin/credential/aws/path_login.go @@ -384,6 +384,11 @@ func (b *backend) verifyInstanceMeetsRoleRequirements(ctx context.Context, return nil, fmt.Errorf("nil identityDoc") } + // Verify that the instance ID matches one of the ones set by the role + if len(roleEntry.BoundEc2InstanceIDs) > 0 && !strutil.StrListContains(roleEntry.BoundEc2InstanceIDs, *instance.InstanceId) { + return fmt.Errorf("instance ID %q does not belong to the role %q", *instance.InstanceId, roleName), nil + } + // Verify that the AccountID of the instance trying to login matches the // AccountID specified as a constraint on role if len(roleEntry.BoundAccountIDs) > 0 && !strutil.StrListContains(roleEntry.BoundAccountIDs, identityDoc.AccountID) { diff --git a/builtin/credential/aws/path_role.go b/builtin/credential/aws/path_role.go index 2a1d463dc7ec..d1586024d10a 100644 --- a/builtin/credential/aws/path_role.go +++ b/builtin/credential/aws/path_role.go @@ -71,6 +71,13 @@ with an IAM instance profile ARN which has a prefix that matches one of the values specified by this parameter. The value is prefix-matched (as though it were a glob ending in '*'). This is only applicable when auth_type is ec2 or inferred_entity_type is ec2_instance.`, + }, + "bound_ec2_instance_id": { + Type: framework.TypeCommaStringSlice, + Description: `If set, defines a constraint on the EC2 instances to have one of the +given instance IDs. Can be a list or comma-separated string of EC2 instance +IDs. This is only applicable when auth_type is ec2 or inferred_entity_type is +ec2_instance.`, }, "resolve_aws_unique_ids": { Type: framework.TypeBool, @@ -548,6 +555,10 @@ func (b *backend) pathRoleCreateUpdate(ctx context.Context, req *logical.Request roleEntry.BoundIamInstanceProfileARNs = boundIamInstanceProfileARNRaw.([]string) } + if boundEc2InstanceIDRaw, ok := data.GetOk("bound_ec2_instance_id"); ok { + roleEntry.BoundEc2InstanceIDs = boundEc2InstanceIDRaw.([]string) + } + if boundIamPrincipalARNRaw, ok := data.GetOk("bound_iam_principal_arn"); ok { principalARNs := boundIamPrincipalARNRaw.([]string) roleEntry.BoundIamPrincipalARNs = principalARNs @@ -621,56 +632,63 @@ func (b *backend) pathRoleCreateUpdate(ctx context.Context, req *logical.Request if len(roleEntry.BoundAccountIDs) > 0 { if !allowEc2Binds { - return logical.ErrorResponse(fmt.Sprintf("specified bound_account_id but not allowing ec2 auth_type or inferring %s", ec2EntityType)), nil + return logical.ErrorResponse(fmt.Sprintf("specified bound_account_id but not specifying ec2 auth_type or inferring %s", ec2EntityType)), nil } numBinds++ } if len(roleEntry.BoundRegions) > 0 { if roleEntry.AuthType != ec2AuthType { - return logical.ErrorResponse("specified bound_region but not allowing ec2 auth_type"), nil + return logical.ErrorResponse("specified bound_region but not specifying ec2 auth_type"), nil } numBinds++ } if len(roleEntry.BoundAmiIDs) > 0 { if !allowEc2Binds { - return logical.ErrorResponse(fmt.Sprintf("specified bound_ami_id but not allowing ec2 auth_type or inferring %s", ec2EntityType)), nil + return logical.ErrorResponse(fmt.Sprintf("specified bound_ami_id but not specifying ec2 auth_type or inferring %s", ec2EntityType)), nil } numBinds++ } if len(roleEntry.BoundIamInstanceProfileARNs) > 0 { if !allowEc2Binds { - return logical.ErrorResponse(fmt.Sprintf("specified bound_iam_instance_profile_arn but not allowing ec2 auth_type or inferring %s", ec2EntityType)), nil + return logical.ErrorResponse(fmt.Sprintf("specified bound_iam_instance_profile_arn but not specifying ec2 auth_type or inferring %s", ec2EntityType)), nil + } + numBinds++ + } + + if len(roleEntry.BoundEc2InstanceIDs) > 0 { + if !allowEc2Binds { + return logical.ErrorResponse(fmt.Sprintf("specified bound_ec2_instance_id but not specifying ec2 auth_type or inferring %s", ec2EntityType)), nil } numBinds++ } if len(roleEntry.BoundIamRoleARNs) > 0 { if !allowEc2Binds { - return logical.ErrorResponse(fmt.Sprintf("specified bound_iam_role_arn but not allowing ec2 auth_type or inferring %s", ec2EntityType)), nil + return logical.ErrorResponse(fmt.Sprintf("specified bound_iam_role_arn but not specifying ec2 auth_type or inferring %s", ec2EntityType)), nil } numBinds++ } if len(roleEntry.BoundIamPrincipalARNs) > 0 { if roleEntry.AuthType != iamAuthType { - return logical.ErrorResponse("specified bound_iam_principal_arn but not allowing iam auth_type"), nil + return logical.ErrorResponse("specified bound_iam_principal_arn but not specifying iam auth_type"), nil } numBinds++ } if len(roleEntry.BoundVpcIDs) > 0 { if !allowEc2Binds { - return logical.ErrorResponse(fmt.Sprintf("specified bound_vpc_id but not allowing ec2 auth_type or inferring %s", ec2EntityType)), nil + return logical.ErrorResponse(fmt.Sprintf("specified bound_vpc_id but not specifying ec2 auth_type or inferring %s", ec2EntityType)), nil } numBinds++ } if len(roleEntry.BoundSubnetIDs) > 0 { if !allowEc2Binds { - return logical.ErrorResponse(fmt.Sprintf("specified bound_subnet_id but not allowing ec2 auth_type or inferring %s", ec2EntityType)), nil + return logical.ErrorResponse(fmt.Sprintf("specified bound_subnet_id but not specifying ec2 auth_type or inferring %s", ec2EntityType)), nil } numBinds++ } @@ -794,6 +812,7 @@ type awsRoleEntry struct { AuthType string `json:"auth_type" ` BoundAmiIDs []string `json:"bound_ami_id_list"` BoundAccountIDs []string `json:"bound_account_id_list"` + BoundEc2InstanceIDs []string `json:"bound_ec2_instance_id_list"` BoundIamPrincipalARNs []string `json:"bound_iam_principal_arn_list"` BoundIamPrincipalIDs []string `json:"bound_iam_principal_id_list"` BoundIamRoleARNs []string `json:"bound_iam_role_arn_list"` @@ -830,6 +849,7 @@ func (r *awsRoleEntry) ToResponseData() map[string]interface{} { "auth_type": r.AuthType, "bound_ami_id": r.BoundAmiIDs, "bound_account_id": r.BoundAccountIDs, + "bound_ec2_instance_id": r.BoundEc2InstanceIDs, "bound_iam_principal_arn": r.BoundIamPrincipalARNs, "bound_iam_principal_id": r.BoundIamPrincipalIDs, "bound_iam_role_arn": r.BoundIamRoleARNs, diff --git a/builtin/credential/aws/path_role_test.go b/builtin/credential/aws/path_role_test.go index 18f6a3e40aad..efed15e2db62 100644 --- a/builtin/credential/aws/path_role_test.go +++ b/builtin/credential/aws/path_role_test.go @@ -570,6 +570,7 @@ func TestAwsEc2_RoleCrud(t *testing.T) { "bound_iam_instance_profile_arn": "arn:aws:iam::123456789012:instance-profile/MyInstanceProfile", "bound_subnet_id": "testsubnetid", "bound_vpc_id": "testvpcid", + "bound_ec2_instance_id": "i-12345678901234567,i-76543210987654321", "role_tag": "testtag", "resolve_aws_unique_ids": false, "allow_instance_migration": true, @@ -600,6 +601,7 @@ func TestAwsEc2_RoleCrud(t *testing.T) { "bound_ami_id": []string{"testamiid"}, "bound_account_id": []string{"testaccountid"}, "bound_region": []string{"testregion"}, + "bound_ec2_instance_id": []string{"i-12345678901234567", "i-76543210987654321"}, "bound_iam_principal_arn": []string{}, "bound_iam_principal_id": []string{}, "bound_iam_role_arn": []string{"arn:aws:iam::123456789012:role/MyRole"}, diff --git a/website/source/api/auth/aws/index.html.md b/website/source/api/auth/aws/index.html.md index 23a63b1bae56..16e01c3d0df0 100644 --- a/website/source/api/auth/aws/index.html.md +++ b/website/source/api/auth/aws/index.html.md @@ -588,6 +588,10 @@ list in order to satisfy that constraint. prefix-matched (as though it were a glob ending in `*`). This constraint is checked by the ec2 auth method as well as the iam auth method only when inferring an ec2 instance. This is a comma-separated string or a JSON array. +- `bound_ec2_instance_id` `(list: [])` - If set, defines a constraint on the + EC2 instances to have one of these instance IDs. This constraint is checked by + the ec2 auth method as well as the iam auth method only when inferring an ec2 + instance. This is a comma-separated string or a JSON array. - `role_tag` `(string: "")` - If set, enables the role tags for this role. The value set for this field should be the 'key' of the tag on the EC2 instance. The 'value' of the tag should be generated using `role//tag` endpoint. @@ -681,6 +685,7 @@ list in order to satisfy that constraint. ```json { "bound_ami_id": ["ami-fce36987"], + "bound_ec2_instance_id": ["i-12345678901234567"], "role_tag": "", "policies": [ "default",