aws / aws-cdk

The AWS Cloud Development Kit is a framework for defining cloud infrastructure in code
https://aws.amazon.com/cdk
Apache License 2.0
11.51k stars 3.85k forks source link

(core): LegacyStackSynthesizer not working in CDKv2 #25185

Open adriantaut opened 1 year ago

adriantaut commented 1 year ago

Describe the bug

Hello CDK team,

We have a bunch of sensitive CF Actions(ec2:CreateVpc, ec2:CreateRoute, ec2:CreateInternetGateway, etc) within our AWS Accounts that are restricted for normal users through SCPs. This is because there is a central team in the company that manages the network infrastructure. This team can assume the so called infra-admin IAM Role in each account whenever CDK stacks containing the VPCs need to be deployed. A snippet of the SCP can be found below:

    {
      "Sid": "ResourceStarDenials",
      "Effect": "Deny",
      "Action": [
        "ec2:CreateVpc",
        "ec2:ModifyVpnConnection",
        "ec2:AttachInternetGateway",
        "ec2:DeregisterInstanceEventNotificationAttributes",
        "ec2:CreateTransitGateway",
        "ec2:DeleteRouteTable",
        "ec2:AssociateRouteTable",
        "ec2:CreateTransitGatewayPeeringAttachment",
        "ec2:DeregisterTransitGatewayMulticastGroupSources",
        "ec2:CreateInternetGateway",
        "ec2:ModifyVpnTunnelCertificate",
        "ec2:CreateTransitGatewayMulticastDomain",
        "ec2:CreateVpnGateway",
        "ec2:AssociateTransitGatewayRouteTable",
        "ec2:CreateVpcPeeringConnection",
        "ec2:EnableVpcClassicLink",
        "ec2:DeleteNetworkAcl",
        "ec2:DisableTransitGatewayRouteTablePropagation",
        "ec2:CreateClientVpnRoute",
        "ec2:DeregisterTransitGatewayMulticastGroupMembers",
        "ec2:CreateRouteTable",
        "ec2:AssociateTransitGatewayMulticastDomain",
        "ec2:CreateClientVpnEndpoint",
        "ec2:ModifyVpnTunnelOptions",
        "ec2:ModifyVpcTenancy",
        "ec2:DeleteVpc",
        "ec2:CreateSubnet",
        "ec2:DeleteTransitGateway",
        "ec2:CreateVpnConnection",
        "ec2:CreateRoute",
        "ec2:DeleteRoute",
        "ec2:DisassociateRouteTable",
        "ec2:ReplaceRoute",
        "ec2:ReplaceRouteTableAssociation"
      ],
      "Resource": "*",
      "Condition": {
        "StringNotLike": {
          "aws:PrincipalArn": [
            "arn:aws:iam::*:role/infra-admin",
            "arn:aws:iam::*:role/AWSControlTowerExecution"
          ]
        }
      }
    }

We used to run cdk deploy commands by assuming this infra-admin IAM Role in each of our AWS account. The cdk deploy command was executed with the assumed role.

Now we are migrated to CDK 2.0 and we noticed cdk deploy uses the cdk-hnb659fds-deploy-role-<account_no>-<region> IAM Role for actual deployment of the CDK Stacks. We've tried using the LegacyStackSynthesizer() that is supposed to use the CLI credentials, but still getting permission denied with the following encoded message:

{
   "allowed":false,
   "explicitDeny":true,
   "matchedStatements":{
      "items":[
         {
            "statementId":"ResourceStarDenials",
            "effect":"DENY",
            "principals":{
               "items":[
                  {
                     "value":"AROA5TMLYOHVR7ML6OZLT"
                  }
               ]
            },
            "principalGroups":{
               "items":[

               ]
            },
            "actions":{
               "items":[
                  {
                     "value":"ec2:CreateVpc"
                  },
                  {
                     "value":"ec2:ModifyVpnConnection"
                  },
                  {
                     "value":"ec2:AttachInternetGateway"
                  },
                  {
                     "value":"ec2:DeregisterInstanceEventNotificationAttributes"
                  },
                  {
                     "value":"ec2:CreateTransitGateway"
                  },
                  {
                     "value":"ec2:DeleteRouteTable"
                  },
                  {
                     "value":"ec2:AssociateRouteTable"
                  },
                  {
                     "value":"ec2:CreateTransitGatewayPeeringAttachment"
                  },
                  {
                     "value":"ec2:DeregisterTransitGatewayMulticastGroupSources"
                  },
                  {
                     "value":"ec2:CreateInternetGateway"
                  },
                  {
                     "value":"ec2:ModifyVpnTunnelCertificate"
                  },
                  {
                     "value":"ec2:CreateTransitGatewayMulticastDomain"
                  },
                  {
                     "value":"ec2:CreateVpnGateway"
                  },
                  {
                     "value":"ec2:AssociateTransitGatewayRouteTable"
                  },
                  {
                     "value":"ec2:CreateVpcPeeringConnection"
                  },
                  {
                     "value":"ec2:EnableVpcClassicLink"
                  },
                  {
                     "value":"ec2:DeleteNetworkAcl"
                  },
                  {
                     "value":"ec2:DisableTransitGatewayRouteTablePropagation"
                  },
                  {
                     "value":"ec2:CreateClientVpnRoute"
                  },
                  {
                     "value":"ec2:DeregisterTransitGatewayMulticastGroupMembers"
                  },
                  {
                     "value":"ec2:CreateRouteTable"
                  },
                  {
                     "value":"ec2:AssociateTransitGatewayMulticastDomain"
                  },
                  {
                     "value":"ec2:CreateClientVpnEndpoint"
                  },
                  {
                     "value":"ec2:ModifyVpnTunnelOptions"
                  },
                  {
                     "value":"ec2:ModifyVpcTenancy"
                  },
                  {
                     "value":"ec2:DeleteVpc"
                  },
                  {
                     "value":"ec2:CreateSubnet"
                  },
                  {
                     "value":"ec2:DeleteTransitGateway"
                  },
                  {
                     "value":"ec2:CreateVpnConnection"
                  },
                  {
                     "value":"ec2:CreateRoute"
                  },
                  {
                     "value":"ec2:DeleteRoute"
                  },
                  {
                     "value":"ec2:DisassociateRouteTable"
                  },
                  {
                     "value":"ec2:ReplaceRoute"
                  },
                  {
                     "value":"ec2:ReplaceRouteTableAssociation"
                  }
               ]
            },
            "resources":{
               "items":[
                  {
                     "value":"*"
                  }
               ]
            },
            "conditions":{
               "items":[
                  {
                     "key":"aws:PrincipalArn",
                     "values":{
                        "items":[
                           {
                              "value":"arn:aws:iam::*:role/infra-admin"
                           },
                           {
                              "value":"arn:aws:iam::*:role/AWSControlTowerExecution"
                           }
                        ]
                     }
                  }
               ]
            }
         }
      ]
   },
   "failures":{
      "items":[

      ]
   },
   "context":{
      "principal":{
         "id":"AROA5TMLYOHVR7ML6OZLT:AWSCloudFormation",
         "arn":"arn:aws:sts::<account-number>:assumed-role/cdk-hnb659fds-cfn-exec-role-<account-number>-<region>/AWSCloudFormation"
      },
      "action":"ec2:CreateVpc",
      "resource":"arn:aws:ec2:eu-west-1:<account-number>:vpc/*",
      "conditions":{
         "items":[
            {
               "key":"aws:Region",
               "values":{
                  "items":[
                     {
                        "value":"eu-west-1"
                     }
                  ]
               }
            },
            {
               "key":"aws:Service",
               "values":{
                  "items":[
                     {
                        "value":"ec2"
                     }
                  ]
               }
            },
            {
               "key":"aws:Resource",
               "values":{
                  "items":[
                     {
                        "value":"vpc/*"
                     }
                  ]
               }
            },
            {
               "key":"aws:Type",
               "values":{
                  "items":[
                     {
                        "value":"vpc"
                     }
                  ]
               }
            },
            {
               "key":"aws:Account",
               "values":{
                  "items":[
                     {
                        "value":"<account-number>"
                     }
                  ]
               }
            },
            {
               "key":"ec2:VpcID",
               "values":{
                  "items":[
                     {
                        "value":"*"
                     }
                  ]
               }
            },
            {
               "key":"aws:ARN",
               "values":{
                  "items":[
                     {
                        "value":"arn:aws:ec2:eu-west-1:<account-number>:vpc/*"
                     }
                  ]
               }
            }
         ]
      }
   }
}

You can find above the Principal that gets access denied being:

   "context":{
      "principal":{
         "id":"AROA5TMLYOHVR7ML6OZLT:AWSCloudFormation",
         "arn":"arn:aws:sts::<account-number>:assumed-role/cdk-hnb659fds-cfn-exec-role-<account-number>-<region>/AWSCloudFormation"
      },

PS: Allowing the cdk-hnb659fds-deploy-role-<account_no>-<region> role to execute these ChangeSets from SCP is not an option, but just assuming the infra-admin IAM Role during CDK deployments.

Expected Behavior

cdk deploy command is able to execute the ChangeSet using a user-specified IAM Role.

Current Behavior

Apparently none of the LegacyStackSynthesizer and CliCredentialsStackSynthesizer are working in a way that would allow using a desired IAM Role instead of the default cdk-hnb*** roles.

Reproduction Steps

Explained in the bug description.

Possible Solution

No response

Additional Information/Context

No response

CDK CLI Version

2.75.0

Framework Version

2.75.0

Node.js Version

v18.4.0

OS

MacOS

Language

Typescript

Language Version

No response

Other information

No response

pahud commented 1 year ago

Hi

I believe you will need to define your custom role ARN in DefaultStackSynthesizerProps.

Check out the related discussion here.

adriantaut commented 1 year ago

Hello @pahud , many thanks for your reply, yes, I've tried this path as well by doing

export class TribeGenericStack extends cdk.Stack {
  constructor(scope: Construct, props: TribeGenericStackProps, id?: string) {

    const infraAdminRoleArn = `arn:aws:iam::<account-number>:role/infra-admin`

    super(scope, id ?? `CloudOpsInfraStack`, {
      env: { account: <account-number>, region },
      synthesizer: new cdk.DefaultStackSynthesizer({
        lookupRoleArn: infraAdminRoleArn,
        deployRoleArn: infraAdminRoleArn,
        fileAssetPublishingRoleArn: infraAdminRoleArn,
        cloudFormationExecutionRole: infraAdminRoleArn,
      }),
      terminationProtection: true,
    });

...........

but because we have that single infra-admin role that is used both as deployRoleArn and cloudFormationExecutionRole, we need to set up a trust policy on the role like below:

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

and by doing this we basically allow anyone in the account that assumes the cloudFormationExecutionRole through cloudformation.amazonaws.com to make use of the role, which we can't afford unfortunately as it breaks exactly what we've tried to achieve.

pahud commented 1 month ago

Having a single infraAdminRoleArn for the 4 functional roles might not be a good idea and that's why CDK is designed to seperate them in the synthesizer.

In your case, let's assume the central team is at account 1111 and infra-admin is the iam role of 1111.

Now you are deploying account 2222 from the infra-admin role from 1111.

What you need to do:

  1. You still need to cdk bootstrap account 1111 by having --trust on 2222. This essentially allows 2222 to assume the 4 roles on 1111
  2. Now when you cdk deploy resources in account 1111 using the infra-admin role from 2222, what's happening is the infra-admin role would assume the 4 roles on 1111 for different operations like context lookup, assets uploading or cfn deploying.

I think this might be something you expect. Thoughts?

github-actions[bot] commented 1 month ago

This issue has not received a response in a while. If you want to keep this issue open, please leave a comment below and auto-close will be canceled.

adriantaut commented 3 weeks ago

@pahud I get the point of the 4 cdk roles for their different purposes and it totally make sense to let CDK deployments to use the proper roles for each of the actions.

The thing is, if we go with the 4 cdk roles that are created as part of the bootstrap, there is no way to differentiate who can use this and for what purposes; in this specific case, as an example, we want to ensure that only infra-admin IAM Role can create VPCs and other IAM Principles cannot (this is something we restricted via SCPs).

Now the thing is:

PS: infra-admin IAM Role exist in every AWS Account in our organization and can be assumed from the management account by a very small set of people.

pahud commented 3 weeks ago

Hi @adriantaut

I just reopened this issue as you are still having concerns.

In AWS, IAM role is the best practice as the principal for service operations. On cdk bootstrap, it creates 5 IAM roles for your account and if you look at the trust policies of the 5 IAM roles, they essentially allow the account root principal or cloudformation service principal to assume by default.

trust policy that allows account root principal to assume:

{
    "Version": "2008-10-17",
    "Statement": [
        {
            "Effect": "Allow",
            "Principal": {
                "AWS": "arn:aws:iam::<AWS_ACCOUNT_ID>:root"
            },
            "Action": "sts:AssumeRole"
        }
    ]
}

trust policy that allows cloudformation service to assume:

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

Trust policies that allows account root principal means "any iam principal within your account that is explicitly granted the sts:AssumeRole action on the target role arn would be eligible to assume for specific operations CDK requires when cdk deploy. The assuming principal could be IAM user or existing IAM role like infra-admin, as long as they have the sts:AssumeRole action you explicitly grant to them. If you don't do that, they won't be able to assume those roles and cdk deploy would immediately return permission denied.

The role assumption and operation process should be like this:

User(authenticated from IAM identity center) --> Assume infra-admin --> Assume the 4 roles --> invoke SDK calls -> pass the cfn-exec-role to cloudformation service -> cloudformation creates the resources on behalf of you

If we let CDK doing it's job, no matter cdk deploy is invoked by infra-admin IAM Role or by a regular user, it will always appear that cdk-hnb659fds-deploy-role-- IAM Role attempts to create the VPC. This will fail because of SCPs blocking the action

From CloudTrail's perspective, I believe you would see the log that someone assuming infra-admin assuming cdk-hnb659fds-deploy-role-* is invoking a specific API call like cloudformation:CreateStack that attempts to create the cloudformation stack or changeset and after that, your current identity would iam:PassRole the cdk-hnb659fds-cfn-exec-role-* role to the cloudformation service to allow cloudformation service assume this cfn-exec-role to create VPC on behave of you. So technically it's not deploy-role or infra-admin who creates the VPC, it's cloudformation service assuming the cfn-exec-role that creates the VPC.

we cannot afford whitelisting cdk-hnb659fds-deploy-role-- in SCPs instead of infra-admin for obvious reasons; this will allow anyone that can run CDK deployments to create VPCs

Again, this won't allow anyone that can run CDK deployments to directly create VPCs via SDK calls or AWS CLI.

Yes, "anyone" can run cdk deploy but they would immediately get permission denied if the admin didn't grant them the sts:AssumeRole permissions on the 4 roles.

What cdk deploy happens behind the scene is the principal( or "anyone" in your context) has to first assume to those 4 roles. If you didn't grant them the sts:AssumeRole permissions on those 4 roles, they won't be able to assume the 4 roles and they will get immediate permission deny when they run cdk deploy.

I know your concern. I think you have two options to address that:

Option 1 - update the trust policy of the 4 roles mentioned above. Instead of trusting the account root principal, just trust the infra-admin role like this:

{
    "Version": "2008-10-17",
    "Statement": [
        {
            "Effect": "Allow",
            "Principal": {
                 "AWS": "arn:aws:iam::<account_id>:role/infra-admin"
            },
            "Action": "sts:AssumeRole"
        }
    ]
}

Now, even "anyone" in your account is having "sts:AssumeRole" on the 4 target roles, they won't be able to do that because the trust policy only allows infra-admin to assume the 4 roles.

In terms of cfn-exec-role because it only trust cloudformation service, your responsibility is NOT allow "anyone" in your account to have "iam:PassRole" permission to pass cfn-exec-role to cloudformation. And you can even scope down the trust policy like this:

{
    "Version": "2012-10-17",
    "Statement": [
        {
            "Effect": "Allow",
            "Principal": {
                "Service": "cloudformation.amazonaws.com"
            },
            "Action": "sts:AssumeRole",
            "Condition": {
                "StringEquals": {
                    "aws:PrincipalArn": "arn:aws:iam::<account_id>:role/infra-admin"
                }
            }
        }
    ]
}

This essentially means only infra-admin can pass the cfn-exec-role to cloudformation for final deployment.

I guess this would address your concern without tearing down the 5 roles.

Option 2 - replace the roles

You probably can replace the 5 IAM roles with the infra-admin role. CDK allows you to customize the bootstrapping so it should be technically possible. But this could make the infra-admin role having the highest privileges and this means "anyone" who can assume this role would have the highest privileges. You probably need to think about it if this is really something you want. But I believe the trust policy of infra-admin should have been scoped so that probably won't be a concern.

I would suggest to reach out to your AWS account solutions architect or account manager to discuss your use cases with your account team as they would have better insight about your use cases and workloads so they can give you the best advices.

I hope this clarifies and addresses your concern.