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): a composite principal is not efficiently synthesized for trust relationship #23765

Open
xiongbo-sjtu opened this issue Jan 20, 2023 · 12 comments
Labels
@aws-cdk/aws-iam Related to AWS Identity and Access Management bug This issue is a bug. effort/medium Medium work item – several days of effort p1

Comments

@xiongbo-sjtu
Copy link

Describe the bug

For our use case, we'd like to create a role in account X, and allow other AWS accounts to assume that role to read data from a bucket in account X.

Using typescript to provision the role in CDK, however, we have run into a character overflow issue with the trust policy under the trust relationships tab of the IAM role. Below is what we are told by AWS Support.

You need to increase the AWS Identity and Access Management (IAM) Role trust policy length in the US East (Northern Virginia). Your current limit is 2048, please note that 4096 is a hard limit and it cannot be increased further.

This bug is similar to #18774, which has been fixed. Apparently, the fix works for bucket policy and KMS key policy, but NOT for trust policy.

Expected Behavior

{
    "Version": "2012-10-17",
    "Statement": [
        {
            "Effect": "Allow",
            "Principal": {
                "AWS": [
                    "arn:aws:iam::123456789012:root",
                    "arn:aws:iam::234567890123:root",
                    "arn:aws:iam::345678901234:root"
               ]
            },
            "Action": "sts:AssumeRole"
        }
    ]
}

Current Behavior

{
    "Version": "2012-10-17",
    "Statement": [
        {
            "Effect": "Allow",
            "Principal": {
                "AWS": "arn:aws:iam::123456789012:root"
            },
            "Action": "sts:AssumeRole"
        },
        {
            "Effect": "Allow",
            "Principal": {
                "AWS": "arn:aws:iam::234567890123:root"
            },
            "Action": "sts:AssumeRole"
        },
        {
            "Effect": "Allow",
            "Principal": {
                "AWS": "arn:aws:iam::345678901234:root"
            },
            "Action": "sts:AssumeRole"
        }
    ]
}

Reproduction Steps

private createCrossAccountRole(consumers: iam.CompositePrincipal, bucketName: string) {
    const roleName = `ExternalReadOnlyAccessRole-${bucketName}`;
    new iam.Role(this, roleName, {
        roleName,
        assumedBy: consumers,
        inlinePolicies: {
            readOnlyAccess: new iam.PolicyDocument({
                assignSids: true,
                statements: [
                    new iam.PolicyStatement({
                        effect: iam.Effect.ALLOW,
                        actions: [
                            "s3:Get*",
                            "s3:List*",
                            "s3-object-lambda:Get*",
                            "s3-object-lambda:List*"
                        ],
                        resources: [
                            `arn:aws:s3:::${bucketName}`,
                            `arn:aws:s3:::${bucketName}/*`
                        ]
                    })
                ]
            })
        }
    });
}
const readOnlyAccounts = ['123456789012', '234567890123', '345678901234']
const principals = readOnlyAccounts.map((accountId: number | string) =>
    new iam.ArnPrincipal(`arn:aws:iam::${accountId}:root`)
);
const consumers = new iam.CompositePrincipal(...principals);
this.createCrossAccountRole(consumers, bucketName);

Possible Solution

No response

Additional Information/Context

No response

CDK CLI Version

2.60.0

Framework Version

No response

Node.js Version

v19.1.0

OS

mac 12.6.2

Language

Typescript

Language Version

typescript 4.9.4

Other information

No response

@xiongbo-sjtu xiongbo-sjtu added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Jan 20, 2023
@github-actions github-actions bot added the @aws-cdk/aws-iam Related to AWS Identity and Access Management label Jan 20, 2023
@fredski-github
Copy link

I have the same issue when using CompositePrincipal. I had to work around using CfnRole.

this code, generates non-optimized trust policy:

        securitylake_role = aws_iam.Role(
            self,
            "AmazonSecurityLakeMetaStoreManager",
            role_name="AmazonSecurityLakeMetaStoreManager",
            managed_policies=[securitylake_role_policy],
            assumed_by=aws_iam.CompositePrincipal(
                aws_iam.ServicePrincipal("lambda.amazonaws.com"),
                aws_iam.ServicePrincipal("securitylake.amazonaws.com")
            )
        )

{
    "Version": "2012-10-17",
    "Statement": [
        {
            "Effect": "Allow",
            "Principal": {
                "Service": "lambda.amazonaws.com"
            },
            "Action": "sts:AssumeRole"
        },
        {
            "Effect": "Allow",
            "Principal": {
                "Service": "securitylake.amazonaws.com"
            },
            "Action": "sts:AssumeRole"
        }
    ]
}

this code, generates the optimized trust policy:

        with open("policies/assume_role.json", "r") as f:
            assume_role = f.read()

        securitylake_role = aws_iam.CfnRole(
            self,
            "AmazonSecurityLakeMetaStoreManager",
            role_name="AmazonSecurityLakeMetaStoreManager",
            managed_policy_arns=[securitylake_role_policy.managed_policy_arn],
            assume_role_policy_document=json.loads(assume_role),
        )

        {
            "Version": "2012-10-17",
            "Statement": [
                {
                    "Sid": "AllowLambdaAndSecurityLake",
                    "Effect": "Allow",
                    "Principal": {
                        "Service": [
                            "lambda.amazonaws.com",
                            "securitylake.amazonaws.com"
                        ]
                    },
                    "Action": "sts:AssumeRole"
                }
            ]
        }

@fredski-github
Copy link

the main issue i had was with Amazon SecurityLake, which does not accept a non-optimized trust policy.

https://docs.aws.amazon.com/security-lake/latest/userguide/getting-started.html

It enforces optimized, when trying to create the SecurityLake, the API returns this error: errorCode "Failed to validate the role provided. Role's trust policy does not contain exactly one statement."

{
    "Version": "2012-10-17",
    "Statement": [
        {
            "Sid": "AllowLambdaAndSecurityLake",
            "Effect": "Allow",
            "Principal": {
                "Service": [
                    "lambda.amazonaws.com",
                    "securitylake.amazonaws.com"
                ]
            },
            "Action": "sts:AssumeRole"
        }
    ]
}

@fredski-github
Copy link

python --version
Python 3.10.1

npx cdk version
2.64.0 (build fb67c77)

@pahud pahud added p1 effort/medium Medium work item – several days of effort and removed needs-triage This issue or PR still needs to be triaged. labels Feb 22, 2023
@JavierLuna
Copy link

Same issue here.

@mrgrain
Copy link
Contributor

mrgrain commented Jul 12, 2023

Cross posting for anyone picking this up:

The logic for this should be implemented somewhere in aws-iam/lib/private/merge-statements.ts or related files, not when adding a principal.

@vormav
Copy link

vormav commented Jan 25, 2024

@longtv2222
Copy link
Contributor

Can confirm that this is working by having "@aws-cdk/aws-iam:minimizePolicies": true in cdk.json, can we close this issue?

@fredski-github
Copy link

did not work for me:

this CDK code:

 securitylake_role_minimized = aws_iam.Role(
      self,
      "AmazonSecurityLakeMetaStoreManagerMinimized",
      role_name="AmazonSecurityLakeMetaStoreManagerMinimized",
      managed_policies=[securitylake_role_policy],
      assumed_by=aws_iam.CompositePrincipal(
          aws_iam.ServicePrincipal("lambda.amazonaws.com"),
          aws_iam.ServicePrincipal("securitylake.amazonaws.com")
      )
  )

generated this Trust Policy:

AmazonSecurityLakeMetaStoreManagerMinimizedCEAD8CAA:
    Type: AWS::IAM::Role
    Properties:
      AssumeRolePolicyDocument:
        Statement:
          - Action: sts:AssumeRole
            Effect: Allow
            Principal:
              Service: lambda.amazonaws.com
          - Action: sts:AssumeRole
            Effect: Allow
            Principal:
              Service: securitylake.amazonaws.com
        Version: "2012-10-17"
      ManagedPolicyArns:
        - Ref: AmazonSecurityLakeMetaStoreManagerRolePolicy427385A7
      RoleName: AmazonSecurityLakeMetaStoreManagerMinimized

cdk.json

{
  "app": "python app.py",
  "@aws-cdk/aws-iam:minimizePolicies": true
}
npx cdk version                
2.124.0 (build 4b6724c)

Am I doing anything wrong?

@longtv2222
Copy link
Contributor

cdk.json needs to look like:

{
  "app": "python app.py",
  "context": {
    "@aws-cdk/aws-iam:minimizePolicies": true
  }
}

and it should be good!

@fredski-github
Copy link

many thanks, that worked. wondering why CDK did not complain about the syntax issue in cdk.json. also, this setting would work best if it was part of the IAM construct, or even the stack. is there a way to have that setting for a stack or stacks? i want to make sure when i create a policy, it is respecting the SIDs and not merging them, is that the expected functionality of @aws-cdk/aws-iam:minimizePolicies?

here is the minimized result just for the sake of completion.

  AmazonSecurityLakeMetaStoreManagerMinimizedCEAD8CAA:
    Type: AWS::IAM::Role
    Properties:
      AssumeRolePolicyDocument:
        Statement:
          - Action: sts:AssumeRole
            Effect: Allow
            Principal:
              Service:
                - lambda.amazonaws.com
                - securitylake.amazonaws.com
        Version: "2012-10-17"
      ManagedPolicyArns:
        - Ref: AmazonSecurityLakeMetaStoreManagerRolePolicy427385A7
      RoleName: AmazonSecurityLakeMetaStoreManagerMinimized

@longtv2222
Copy link
Contributor

longtv2222 commented Feb 2, 2024

If you want to set the context for an IAM construct or a stack in code, you can use construct.node.setContext()

Refer CDK context docs

@fredski-github
Copy link

thank you, i will try those. it does not prevent closure of this ticket, the current solution is viable.

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 bug This issue is a bug. effort/medium Medium work item – several days of effort p1
Projects
None yet
7 participants