secret/aws: Pass policy ARNs to AssumedRole and FederationToken roles (#6789)

* secret/aws: Pass policy ARNs to AssumedRole and FederationToken roles

AWS now allows you to pass policy ARNs as well as, and in addition to,
policy documents for AssumeRole and GetFederationToken (see
https://aws.amazon.com/about-aws/whats-new/2019/05/session-permissions/).
Vault already collects policy ARNs for iam_user credential types; now it
will allow policy ARNs for assumed_role and federation_token credential
types and plumb them through to the appropriate AWS calls.

This brings along a minor breaking change. Vault roles of the
federation_token credential type are now required to have either a
policy_document or a policy_arns specified. This was implicit
previously; a missing policy_document would result in a validation error
from the AWS SDK when retrieving credentials. However, it would still
allow creating a role that didn't have a policy_document specified and
then later specifying it, after which retrieving the AWS credentials
would work. Similar workflows in which the Vault role didn't have a
policy_document specified for some period of time, such as deleting the
policy_document and then later adding it back, would also have worked
previously but will now be broken.

The reason for this breaking change is because a credential_type of
federation_token without either a policy_document or policy_arns
specified will return credentials that have equivalent permissions to
the credentials the Vault server itself is using. This is quite
dangerous (e.g., it could allow Vault clients access to retrieve
credentials that could modify Vault's underlying storage) and so should
be discouraged. This scenario is still possible when passing in an
appropriate policy_document or policy_arns parameter, but clients should
be explicitly aware of what they are doing and opt in to it by passing
in the appropriate role parameters.

* Error out on dangerous federation token retrieval

The AWS secrets role code now disallows creation of a dangerous role
configuration; however, pre-existing roles could have existed that would
trigger this now-dangerous code path, so also adding a check for this
configuration at credential retrieval time.

* Run makefmt

* Fix tests

* Fix comments/docs

builtin/logical/aws/backend_test.go

@@ -72,7 +72,7 @@ func TestBackend_basicSTS(t *testing.T) {
 		PreCheck: func() {
 			testAccPreCheck(t)
 			createUser(t, userName, accessKey)
-			createRole(t, roleName, awsAccountID)
+			createRole(t, roleName, awsAccountID, []string{ec2PolicyArn})
 			// Sleep sometime because AWS is eventually consistent
 			// Both the createUser and createRole depend on this
 			log.Println("[WARN] Sleeping for 10 seconds waiting for AWS...")
@@ -90,7 +90,10 @@ func TestBackend_basicSTS(t *testing.T) {
 			testAccStepRead(t, "sts", "test2", []credentialTestFunc{describeInstancesTest}),
 		},
 		Teardown: func() error {
-			return teardown(accessKey, roleName, userName)
+			if err := deleteTestRole(roleName); err != nil {
+				return err
+			}
+			return deleteTestUser(accessKey, userName)
 		},
 	})
 }
@@ -200,7 +203,7 @@ func getAccountID() (string, error) {
 	return *res.Account, nil
 }
 
-func createRole(t *testing.T, roleName, awsAccountID string) {
+func createRole(t *testing.T, roleName, awsAccountID string, policyARNs []string) {
 	const testRoleAssumePolicy = `{
       "Version": "2012-10-17",
       "Statement": [
@@ -234,14 +237,16 @@ func createRole(t *testing.T, roleName, awsAccountID string) {
 		t.Fatalf("AWS CreateRole failed: %v", err)
 	}
 
+	for _, policyARN := range policyARNs {
 		attachment := &iam.AttachRolePolicyInput{
-		PolicyArn: aws.String(ec2PolicyArn),
+			PolicyArn: aws.String(policyARN),
 			RoleName:  aws.String(roleName), // Required
 		}
 		_, err = svc.AttachRolePolicy(attachment)
 
 		if err != nil {
-		t.Fatalf("AWS CreateRole failed: %v", err)
+			t.Fatalf("AWS AttachRolePolicy failed: %v", err)
+		}
 	}
 }
 
@@ -332,14 +337,25 @@ func deleteTestRole(roleName string) error {
 	}
 	svc := iam.New(session.New(awsConfig))
 
-	attachment := &iam.DetachRolePolicyInput{
-		PolicyArn: aws.String(ec2PolicyArn),
+	listAttachmentsInput := &iam.ListAttachedRolePoliciesInput{
+		RoleName: aws.String(roleName),
+	}
+	detacher := func(result *iam.ListAttachedRolePoliciesOutput, lastPage bool) bool {
+		for _, policy := range result.AttachedPolicies {
+			detachInput := &iam.DetachRolePolicyInput{
+				PolicyArn: policy.PolicyArn,
 				RoleName:  aws.String(roleName), // Required
 			}
-	_, err := svc.DetachRolePolicy(attachment)
+			_, err := svc.DetachRolePolicy(detachInput)
+			if err != nil {
+				log.Printf("[WARN] AWS DetachRolePolicy failed for policy %s: %v", *policy.PolicyArn, err)
+			}
+		}
+		return true
+	}
+	err := svc.ListAttachedRolePoliciesPages(listAttachmentsInput, detacher)
 	if err != nil {
 		log.Printf("[WARN] AWS DetachRolePolicy failed: %v", err)
-		return err
 	}
 
 	params := &iam.DeleteRoleInput{
@@ -356,11 +372,7 @@ func deleteTestRole(roleName string) error {
 	return nil
 }
 
-func teardown(accessKey *awsAccessKey, roleName, userName string) error {
-
-	if err := deleteTestRole(roleName); err != nil {
-		return err
-	}
+func deleteTestUser(accessKey *awsAccessKey, userName string) error {
 	awsConfig := &aws.Config{
 		Region:     aws.String("us-east-1"),
 		HTTPClient: cleanhttp.DefaultClient(),
@@ -387,20 +399,20 @@ func teardown(accessKey *awsAccessKey, roleName, userName string) error {
 		return err
 	}
 
-	deleteUserPolicyInput := &iam.DeleteUserPolicyInput{
+	deleteTestUserPolicyInput := &iam.DeleteUserPolicyInput{
 		PolicyName: aws.String("SelfDestructionTimebomb"),
 		UserName:   aws.String(userName),
 	}
-	_, err = svc.DeleteUserPolicy(deleteUserPolicyInput)
+	_, err = svc.DeleteUserPolicy(deleteTestUserPolicyInput)
 	if err != nil {
 		log.Printf("[WARN] AWS DeleteUserPolicy failed: %v", err)
 		return err
 	}
-	deleteUserInput := &iam.DeleteUserInput{
+	deleteTestUserInput := &iam.DeleteUserInput{
 		UserName: aws.String(userName),
 	}
 	log.Printf("[INFO] AWS DeleteUser: %s", userName)
-	_, err = svc.DeleteUser(deleteUserInput)
+	_, err = svc.DeleteUser(deleteTestUserInput)
 	if err != nil {
 		log.Printf("[WARN] AWS DeleteUser failed: %v", err)
 		return err
@@ -704,6 +716,7 @@ const testDynamoPolicy = `{
 
 const ec2PolicyArn = "arn:aws:iam::aws:policy/AmazonEC2ReadOnlyAccess"
 const iamPolicyArn = "arn:aws:iam::aws:policy/IAMReadOnlyAccess"
+const dynamoPolicyArn = "arn:aws:iam::aws:policy/AmazonDynamoDBReadOnlyAccess"
 
 func testAccStepWriteRole(t *testing.T, name string, data map[string]interface{}) logicaltest.TestStep {
 	return logicaltest.TestStep{
@@ -825,7 +838,7 @@ func TestBackend_AssumedRoleWithPolicyDoc(t *testing.T) {
 		AcceptanceTest: true,
 		PreCheck: func() {
 			testAccPreCheck(t)
-			createRole(t, roleName, awsAccountID)
+			createRole(t, roleName, awsAccountID, []string{ec2PolicyArn})
 			// Sleep sometime because AWS is eventually consistent
 			log.Println("[WARN] Sleeping for 10 seconds waiting for AWS...")
 			time.Sleep(10 * time.Second)
@@ -843,6 +856,73 @@ func TestBackend_AssumedRoleWithPolicyDoc(t *testing.T) {
 	})
 }
 
+func TestBackend_AssumedRoleWithPolicyARN(t *testing.T) {
+	t.Parallel()
+	roleName := generateUniqueName(t.Name())
+
+	awsAccountID, err := getAccountID()
+	if err != nil {
+		t.Logf("Unable to retrive user via sts:GetCallerIdentity: %#v", err)
+		t.Skip("Could not determine AWS account ID from sts:GetCallerIdentity for acceptance tests, skipping")
+	}
+	roleData := map[string]interface{}{
+		"policy_arns":     iamPolicyArn,
+		"role_arns":       []string{fmt.Sprintf("arn:aws:iam::%s:role/%s", awsAccountID, roleName)},
+		"credential_type": assumedRoleCred,
+	}
+	logicaltest.Test(t, logicaltest.TestCase{
+		AcceptanceTest: true,
+		PreCheck: func() {
+			testAccPreCheck(t)
+			createRole(t, roleName, awsAccountID, []string{ec2PolicyArn, iamPolicyArn})
+			log.Printf("[WARN] Sleeping for 10 seconds waiting for AWS...")
+			time.Sleep(10 * time.Second)
+		},
+		LogicalBackend: getBackend(t),
+		Steps: []logicaltest.TestStep{
+			testAccStepConfig(t),
+			testAccStepWriteRole(t, "test", roleData),
+			testAccStepRead(t, "sts", "test", []credentialTestFunc{listIamUsersTest, describeAzsTestUnauthorized}),
+			testAccStepRead(t, "creds", "test", []credentialTestFunc{listIamUsersTest, describeAzsTestUnauthorized}),
+		},
+		Teardown: func() error {
+			return deleteTestRole(roleName)
+		},
+	})
+}
+
+func TestBackend_FederationTokenWithPolicyARN(t *testing.T) {
+	t.Parallel()
+	userName := generateUniqueName(t.Name())
+	accessKey := &awsAccessKey{}
+
+	roleData := map[string]interface{}{
+		"policy_arns":     dynamoPolicyArn,
+		"credential_type": federationTokenCred,
+	}
+	logicaltest.Test(t, logicaltest.TestCase{
+		AcceptanceTest: true,
+		PreCheck: func() {
+			testAccPreCheck(t)
+			createUser(t, userName, accessKey)
+			// Sleep sometime because AWS is eventually consistent
+			log.Println("[WARN] Sleeping for 10 seconds waiting for AWS...")
+			time.Sleep(10 * time.Second)
+		},
+		LogicalBackend: getBackend(t),
+		Steps: []logicaltest.TestStep{
+			testAccStepConfigWithCreds(t, accessKey),
+			testAccStepWriteRole(t, "test", roleData),
+			testAccStepRead(t, "sts", "test", []credentialTestFunc{listDynamoTablesTest, describeAzsTestUnauthorized}),
+			testAccStepRead(t, "creds", "test", []credentialTestFunc{listDynamoTablesTest, describeAzsTestUnauthorized}),
+		},
+		Teardown: func() error {
+			//return deleteTestUser(accessKey, userName)
+			return nil
+		},
+	})
+}
+
 func TestBackend_RoleDefaultSTSTTL(t *testing.T) {
 	t.Parallel()
 	roleName := generateUniqueName(t.Name())
@@ -862,7 +942,7 @@ func TestBackend_RoleDefaultSTSTTL(t *testing.T) {
 		AcceptanceTest: true,
 		PreCheck: func() {
 			testAccPreCheck(t)
-			createRole(t, roleName, awsAccountID)
+			createRole(t, roleName, awsAccountID, []string{ec2PolicyArn})
 			log.Println("[WARN] Sleeping for 10 seconds waiting for AWS...")
 			time.Sleep(10 * time.Second)
 		},

builtin/logical/aws/path_roles.go

@@ -62,7 +62,10 @@ func pathRoles(b *backend) *framework.Path {
 
 			"policy_arns": &framework.FieldSchema{
 				Type: framework.TypeCommaStringSlice,
-				Description: "ARNs of AWS policies to attach to IAM users. Only valid when credential_type is " + iamUserCred,
+				Description: fmt.Sprintf(`ARNs of AWS policies. Behavior varies by credential_type. When credential_type is
+%s, then it will attach the specified policies to the generated IAM user.
+When credential_type is %s or %s, the policies will be passed as the
+PolicyArns parameter, acting as a filter on permissions available.`, iamUserCred, assumedRoleCred, federationTokenCred),
 				DisplayAttrs: &framework.DisplayAttributes{
 					Name: "Policy ARNs",
 				},
@@ -501,9 +504,6 @@ func (r *awsRoleEntry) validate() error {
 	if len(r.RoleArns) > 0 && !strutil.StrListContains(r.CredentialTypes, assumedRoleCred) {
 		errors = multierror.Append(errors, fmt.Errorf("cannot supply role_arns when credential_type isn't %s", assumedRoleCred))
 	}
-	if len(r.PolicyArns) > 0 && !strutil.StrListContains(r.CredentialTypes, iamUserCred) {
-		errors = multierror.Append(errors, fmt.Errorf("cannot supply policy_arns when credential_type isn't %s", iamUserCred))
-	}
 
 	return errors.ErrorOrNil()
 }

builtin/logical/aws/path_roles_test.go

@@ -282,6 +282,7 @@ func TestRoleEntryValidationAssumedRoleCred(t *testing.T) {
 	roleEntry := awsRoleEntry{
 		CredentialTypes: []string{assumedRoleCred},
 		RoleArns:        []string{"arn:aws:iam::123456789012:role/SomeRole"},
+		PolicyArns:      []string{"arn:aws:iam::aws:policy/AdministratorAccess"},
 		PolicyDocument:  allowAllPolicyDocument,
 		DefaultSTSTTL:   2,
 		MaxSTSTTL:       3,
@@ -290,11 +291,6 @@ func TestRoleEntryValidationAssumedRoleCred(t *testing.T) {
 		t.Errorf("bad: valid roleEntry %#v failed validation: %v", roleEntry, err)
 	}
 
-	roleEntry.PolicyArns = []string{"arn:aws:iam::aws:policy/AdministratorAccess"}
-	if roleEntry.validate() == nil {
-		t.Errorf("bad: invalid roleEntry with unrecognized PolicyArns %#v passed validation", roleEntry)
-	}
-	roleEntry.PolicyArns = []string{}
 	roleEntry.MaxSTSTTL = 1
 	if roleEntry.validate() == nil {
 		t.Errorf("bad: invalid roleEntry with MaxSTSTTL < DefaultSTSTTL %#v passed validation", roleEntry)
@@ -311,6 +307,7 @@ func TestRoleEntryValidationFederationTokenCred(t *testing.T) {
 	roleEntry := awsRoleEntry{
 		CredentialTypes: []string{federationTokenCred},
 		PolicyDocument:  allowAllPolicyDocument,
+		PolicyArns:      []string{"arn:aws:iam::aws:policy/AdministratorAccess"},
 		DefaultSTSTTL:   2,
 		MaxSTSTTL:       3,
 	}

builtin/logical/aws/path_user.go

@@ -126,9 +126,9 @@ func (b *backend) pathCredsRead(ctx context.Context, req *logical.Request, d *fr
 		case !strutil.StrListContains(role.RoleArns, roleArn):
 			return logical.ErrorResponse(fmt.Sprintf("role_arn %q not in allowed role arns for Vault role %q", roleArn, roleName)), nil
 		}
-		return b.assumeRole(ctx, req.Storage, req.DisplayName, roleName, roleArn, role.PolicyDocument, ttl)
+		return b.assumeRole(ctx, req.Storage, req.DisplayName, roleName, roleArn, role.PolicyDocument, role.PolicyArns, ttl)
 	case federationTokenCred:
-		return b.secretTokenCreate(ctx, req.Storage, req.DisplayName, roleName, role.PolicyDocument, ttl)
+		return b.getFederationToken(ctx, req.Storage, req.DisplayName, roleName, role.PolicyDocument, role.PolicyArns, ttl)
 	default:
 		return logical.ErrorResponse(fmt.Sprintf("unknown credential_type: %q", credentialType)), nil
 	}

builtin/logical/aws/secret_access_keys.go

@@ -65,8 +65,8 @@ func genUsername(displayName, policyName, userType string) (ret string, warning
 	return
 }
 
-func (b *backend) secretTokenCreate(ctx context.Context, s logical.Storage,
-	displayName, policyName, policy string,
+func (b *backend) getFederationToken(ctx context.Context, s logical.Storage,
+	displayName, policyName, policy string, policyARNs []string,
 	lifeTimeInSeconds int64) (*logical.Response, error) {
 	stsClient, err := b.clientSTS(ctx, s)
 	if err != nil {
@@ -75,12 +75,26 @@ func (b *backend) secretTokenCreate(ctx context.Context, s logical.Storage,
 
 	username, usernameWarning := genUsername(displayName, policyName, "sts")
 
-	tokenResp, err := stsClient.GetFederationToken(
-		&sts.GetFederationTokenInput{
+	getTokenInput := &sts.GetFederationTokenInput{
 		Name:            aws.String(username),
-			Policy:          aws.String(policy),
 		DurationSeconds: &lifeTimeInSeconds,
-		})
+	}
+	if len(policy) > 0 {
+		getTokenInput.Policy = aws.String(policy)
+	}
+	if len(policyARNs) > 0 {
+		getTokenInput.PolicyArns = convertPolicyARNs(policyARNs)
+	}
+
+	// If neither a policy document nor policy ARNs are specified, then GetFederationToken will
+	// return credentials equivalent to that of the Vault server itself. We probably don't want
+	// that by default; the behavior can be explicitly opted in to by associating the Vault role
+	// with a policy ARN or document that allows the appropriate permissions.
+	if policy == "" && len(policyARNs) == 0 {
+		return logical.ErrorResponse(fmt.Sprintf("must specify at least one of policy_arns or policy_document with %s credential_type", federationTokenCred)), nil
+	}
+
+	tokenResp, err := stsClient.GetFederationToken(getTokenInput)
 
 	if err != nil {
 		return logical.ErrorResponse(fmt.Sprintf(
@@ -111,7 +125,7 @@ func (b *backend) secretTokenCreate(ctx context.Context, s logical.Storage,
 }
 
 func (b *backend) assumeRole(ctx context.Context, s logical.Storage,
-	displayName, roleName, roleArn, policy string,
+	displayName, roleName, roleArn, policy string, policyARNs []string,
 	lifeTimeInSeconds int64) (*logical.Response, error) {
 	stsClient, err := b.clientSTS(ctx, s)
 	if err != nil {
@@ -128,6 +142,9 @@ func (b *backend) assumeRole(ctx context.Context, s logical.Storage,
 	if policy != "" {
 		assumeRoleInput.SetPolicy(policy)
 	}
+	if len(policyARNs) > 0 {
+		assumeRoleInput.SetPolicyArns(convertPolicyARNs(policyARNs))
+	}
 	tokenResp, err := stsClient.AssumeRole(assumeRoleInput)
 
 	if err != nil {
@@ -334,3 +351,14 @@ func normalizeDisplayName(displayName string) string {
 	re := regexp.MustCompile("[^a-zA-Z0-9+=,.@_-]")
 	return re.ReplaceAllString(displayName, "_")
 }
+
+func convertPolicyARNs(policyARNs []string) []*sts.PolicyDescriptorType {
+	size := len(policyARNs)
+	retval := make([]*sts.PolicyDescriptorType, size, size)
+	for i, arn := range policyARNs {
+		retval[i] = &sts.PolicyDescriptorType{
+			Arn: aws.String(arn),
+		}
+	}
+	return retval
+}

website/source/api/secret/aws/index.html.md

@@ -209,9 +209,12 @@ updated with the new attributes.
   is allowed to assume. Required when `credential_type` is `assumed_role` and
   prohibited otherwise. This is a comma-separated string or JSON array.
 
-- `policy_arns` `(list: [])` – Specifies the ARNs of the AWS managed policies to
-  be attached to IAM users when they are requested. Valid only when
-  `credential_type` is `iam_user`. When `credential_type` is `iam_user`, at
+- `policy_arns` `(list: [])` – Specifies a list of AWS managed policy ARN. The
+  behavior depends on the credential type. With `iam_user`, the policies will
+  be attached to IAM users when they are requested. With `assumed_role` and
+  `federation_token`, the policy ARNs will act as a filter on what the
+  credentials can do, similar to `policy_document`.
+  When `credential_type` is `iam_user` or `federation_token`, at
   least one of `policy_arns` or `policy_document` must be specified. This is a
   comma-separated string or JSON array.
 
@@ -219,7 +222,7 @@ updated with the new attributes.
   behavior depends on the credential type. With `iam_user`, the policy document
   will be attached to the IAM user generated and augment the permissions the IAM
   user has. With `assumed_role` and `federation_token`, the policy document will
-  act as a filter on what the credentials can do.
+  act as a filter on what the credentials can do, similar to `policy_arns`.
 
 - `default_sts_ttl` `(string)` - The default TTL for STS credentials. When a TTL is not
   specified when STS credentials are requested, and a default TTL is specified

website/source/docs/secrets/aws/index.html.md

@@ -198,15 +198,15 @@ does not allow temporary credentials (such as those from an IAM instance
 profile) to be used.
 
 An STS federation token inherits a set of permissions that are the combination
-(intersection) of three sets of permissions:
+(intersection) of four sets of permissions:
 
 1. The permissions granted to the `aws/config/root` credentials
-2. The user inline policy configured for the `aws/role`
-3. An implicit deny policy on IAM or STS operations.
+2. The user inline policy configured in the Vault role
+3. The managed policy ARNs configured in the Vault role
+4. An implicit deny policy on IAM or STS operations.
 
-Roles with a `credential_type` of `federation_token` can only specify a
-`policy_document` in the Vault role. AWS does not support support managed
-policies.
+Roles with a `credential_type` of `federation_token` can specify both a
+`policy_document` and `policy_arns` parameter in the Vault role.
 
 The `aws/config/root` credentials require IAM permissions for
 `sts:GetFederationToken` and the permissions to delegate to the STS
@@ -322,13 +322,17 @@ the aws/root/config credentials to assume the role.
 When specifying a Vault role with a `credential_type` of `assumed_role`, you can
 specify more than one IAM role ARN. If you do so, Vault clients can select which
 role ARN they would like to assume when retrieving credentials from that role.
-You can further specify a `policy_document` which, if specified, acts as a
+
+Further, you can specify both a `policy_document` and `policy_arns` parameters;
+if specified, each acts as a
 filter on the IAM permissions granted to the assumed role. For an action to be
 allowed, it must be permitted by both the IAM policy on the AWS role that is
-assumed as well as the `policy_document` specified on the Vault role. (The
+assumed, the `policy_document` specified on the Vault role (if specified), and
+the managed policies specified by the `policy_arns` parameter. (The
 `policy_document` parameter is passed in as the `Policy` parameter to the
 [sts:AssumeRole](https://docs.aws.amazon.com/STS/latest/APIReference/API_AssumeRole.html)
-API call.)
+API call, while the `policy_arns` parameter is passed in as the `PolicyArns`
+parameter to the same call.)
 
 Note: When multiple `role_arns` are specified, clients requesting credentials
 can specify any of the role ARNs that are defined on the Vault role in order to