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

IAM: Role Trust Relationships cannot be extended with multiple principals #27006

Open
ttais2017 opened this issue Sep 5, 2023 · 10 comments
Open
Labels
@aws-cdk/aws-iam Related to AWS Identity and Access Management documentation This is a problem with documentation. feature-request A feature should be added or improved. p1

Comments

@ttais2017
Copy link

ttais2017 commented Sep 5, 2023

Describe the bug

The Trust Relationships of a Role cannot be extended with multiple principals. As example I included this configuration (which is possible from AWS-console)

image

Expected Behavior

The Trust Relationships can contain multiple principals

Current Behavior

While Deployment I'm getting this errors:

" A PolicyStatement used in an identity-based policy cannot specify any IAM principals" and "A PolicyStatement used in an identity-based policy must specify at least one resource"

Reproduction Steps

see this code as example:

image

I tried also with this:

image

but I got this errors:

image

Possible Solution

No response

Additional Information/Context

No response

CDK CLI Version

aws-cli/2.13.12 Python/3.11.4 Windows/10 exe/AMD64 prompt/off

Framework Version

No response

Node.js Version

9.5.1

OS

Windows

Language

Java

Language Version

Java 17

Other information

No response

@ttais2017 ttais2017 added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Sep 5, 2023
@github-actions github-actions bot added the @aws-cdk/aws-iam Related to AWS Identity and Access Management label Sep 5, 2023
@khushail khushail self-assigned this Sep 5, 2023
@khushail khushail added investigating This issue is being investigated and/or work is in progress to resolve the issue. and removed needs-triage This issue or PR still needs to be triaged. labels Sep 5, 2023
@wz2b
Copy link

wz2b commented Sep 5, 2023

I have this same exact problem. I'm trying to create a function that will be called by cloudfront. According to this document you need to allow the role to be assumed by two principles. Currently, the assumedBy doesn't accept an array.

const fnRole = new iam.Role(this, "redirect-function-role-id", {
    roleName: "redirect-function-role",
    assumedBy: new iam.ServicePrincipal('edgelambda.amazonaws.com')
})

As a workaround, I thought I could just add another principal to the trust policy like this:

 fnRole.grantAssumeRole(new iam.ServicePrincipal('lambda.amazonaws.com'))

but the messed up thing is that just silently did nothing - no error, it just didn't work. The output of the two things above generated this:

  redirectfunctionroleid9F482E14:
    Type: AWS::IAM::Role
    Properties:
      AssumeRolePolicyDocument:
        Statement:
          - Action: sts:AssumeRole
            Effect: Allow
            Principal:
              Service: edgelambda.amazonaws.com
        Version: "2012-10-17"
      RoleName: redirect-function-role

I think that is related to #24507 - as mentioned in that ticket, this does work:

  role.assumeRolePolicy?.addStatements(new iam.PolicyStatement({
    actions: ['sts:AssumeRole'],
    principals: [new iam.ServicePrincipal('lambda.amazonaws.com')],
  }))

I kind of consider that to be a workaround in the sense that I think the .grantAssumeRole() should do the same thing - maybe the developers disagree. I think what'd be an even better approach would be to make assumedBy take either string or string[]

@peterwoodworth
Copy link
Contributor

grantAssumeRole doesn't add to the Role's trust policy - it grants the other principal permissions in their policy statement. Does addToPrincipalPolicy not work? https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.aws_iam.Role.html#addwbrtowbrprincipalwbrpolicystatement

@peterwoodworth peterwoodworth added the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. label Sep 5, 2023
@khushail khushail removed their assignment Sep 5, 2023
@khushail khushail removed the investigating This issue is being investigated and/or work is in progress to resolve the issue. label Sep 5, 2023
@ttais2017
Copy link
Author

I tried this:
image

but I get deploy errors:
image

@ttais2017
Copy link
Author

I dont still understand why this method does not work. @peterwoodworth: "grantAssumeRole doesn't add to the Role's trust polic" ?
image

@github-actions github-actions bot removed the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. label Sep 6, 2023
@wz2b
Copy link

wz2b commented Sep 6, 2023

I dont still understand why this method does not work. @peterwoodworth

I don't, either. The name pretty strongly implies that you're granting some principal authority to assume the role.

@peterwoodworth
Copy link
Contributor

From our README, try CompositePrincipal. This works for me

    const role = new iam.Role(this, 'Role', {
      assumedBy: new iam.CompositePrincipal(
        new iam.ServicePrincipal('lambda.amazonaws.com'),
        new iam.ServicePrincipal('apigateway.amazonaws.com'),
      ),
      managedPolicies: [
        iam.ManagedPolicy.fromAwsManagedPolicyName('AmazonSagemakerFullAccess'),
        iam.ManagedPolicy.fromAwsManagedPolicyName('AmazonS3FullAccess')
      ],
      inlinePolicies: {
        policy: new iam.PolicyDocument({
          statements: [
            new iam.PolicyStatement({
              actions: ['lakeformation:*'],
              resources: ['*'],
              effect: iam.Effect.ALLOW,
            })
          ]
        })
      }
    })

Regarding the confusion over grantAssumeRole, it states:

Grant permissions to the given principal to assume this role.

I'm not really sure how this can be made more clear in the docstring. It's stating that it's granting permission to the principal, it doesn't state anything about adding to its own trust policy. If you have any suggestions on how to make this more clear, let me know.

Aside from the docstring, we should at the least document a use case of grantAssumeRole(), addToPrincipalPolicy(), and assumeRolePolicy

@peterwoodworth peterwoodworth added p1 feature-request A feature should be added or improved. documentation This is a problem with documentation. and removed bug This issue is a bug. labels Sep 6, 2023
@wz2b
Copy link

wz2b commented Sep 7, 2023

I'm not really sure how this can be made more clear in the docstring.

I'm not sure, either. I have a feeling I'm missing something fundamental. As an experiment I tried doing this:

const fnRole = new iam.Role(this, "redirect-function-role-id", {
    roleName: "redirect-function-role",
    assumedBy: new iam.ServicePrincipal('edgelambda.amazonaws.com')
})

then did a cdk synth > file1.json. Then I added this:

 fnRole.grantAssumeRole(new iam.ServicePrincipal('lambda.amazonaws.com'))

then did cdk synth > file2.json. Then I diffed them, and they were the same:

  craproleid58ADBD94:
    Type: AWS::IAM::Role
    Properties:
      AssumeRolePolicyDocument:
        Statement:
          - Action: sts:AssumeRole
            Effect: Allow
            Principal:
              Service: edgelambda.amazonaws.com
        Version: "2012-10-17"
      RoleName: redirect-function-role

What I expected it to do was to add a second principal under the AssumeRolePolicyDocument. It didn't change the role at all, unless it somehow changed it outside of the role definition in the template.

@ttais2017
Copy link
Author

@peterwoodworth : Tnx for your suggestion. I tested that and it works also for me.

w.r.t. the Documentation, I thinks the confusion is related to the name of action "sts:AssumeRole" and then think the developer, the function grantAssumeRole should also be used for that. My suggestion add into the docu as "note" or "warning" . For adding trusted relationships pls use "CompositePrincipal" while creating the role" or something like that.

@wz2b
Copy link

wz2b commented Sep 8, 2023

I should note that if you use the other method, on the role, assumeRolePolicy?.addStatements that you have full access to the trust policy. This would let you do more than just adding a Composite Principal. There probably aren't a whole lot of reasons you would want to do this, but if you wanted to do something like add a condition to the trust policy that's how you could do it (in addition to adding multiple principals):

            "Condition": {
                "StringEqualsIfExists": {
                    "aws:SourceAccount": "<account-id>"
                }
            }

You might want to do that if you have weird subaccount or cross-account requirements shrug.

Your way is less typing.

@aperuru
Copy link

aperuru commented Oct 19, 2023

I ran into a similar problem, but I was able to solve this with fewer lines of code using spread operator.
The only difference is that I intended to add multiple account principals to the trusted relationship along with the service principal so that the permissions are not wide open.

    const accountsToAllowAssumedBy = ['000000000000', '111111111111'];

    const role = new iam.Role(this, 'Role', {
      roleName: 'AssumeRoleTest',
      assumedBy: new iam.CompositePrincipal(
        new iam.ServicePrincipal('ecs-tasks.amazonaws.com'),
        ...accountsToAllowAssumedBy.map((account) => new iam.AccountPrincipal(account)), //Spread Operator (...)
      ),
    });

Output:

"Role1ABCC5F0": {
   "Type": "AWS::IAM::Role",
   "Properties": {
    "RoleName": "AssumeRoleTest",
    "AssumeRolePolicyDocument": {
     "Statement": [
      {
       "Action": "sts:AssumeRole",
       "Effect": "Allow",
       "Principal": {
        "AWS": [
         "arn:aws:iam::000000000000:root",
         "arn:aws:iam::111111111111:root"
        ],
        "Service": "ecs-tasks.amazonaws.com"
       }
      }
     ],
    },
   },
  },

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-iam Related to AWS Identity and Access Management documentation This is a problem with documentation. feature-request A feature should be added or improved. p1
Projects
None yet
Development

No branches or pull requests

5 participants