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

(cli): does not properly set externalId when trying to assume lookupRole #22535

Closed
peterwoodworth opened this issue Oct 17, 2022 · 5 comments · Fixed by #31089
Closed

(cli): does not properly set externalId when trying to assume lookupRole #22535

peterwoodworth opened this issue Oct 17, 2022 · 5 comments · Fixed by #31089
Assignees
Labels
bug This issue is a bug. effort/small Small work item – less than a day of effort p1 package/tools Related to AWS CDK Tools or CLI

Comments

@peterwoodworth
Copy link
Contributor

peterwoodworth commented Oct 17, 2022

Describe the bug

The lookup role doesn't seem to be assumable through CDK when requiring an external id. All other roles (deploy, file publishing, image publishing) properly set the external id, however the lookup role isn't working.

Expected Behavior

I expect the lookup role to be assumable when requiring an external ID

Current Behavior

Throws error:

Could not assume role in target account using current credentials (which are for account 181354823827) 
User: arn:aws:iam::181354823827:user/test is not authorized to perform: sts:AssumeRole 
on resource: arn:aws:iam::676158502875:role/cdk-hnb659fds-lookup-role-676158502875-eu-west-1 . 
Please make sure that this role exists in the account. If it doesn't exist, (re)-bootstrap 
the environment with the right '--trust', using the latest version of the CDK CLI.

Reproduction Steps

Bootstrap an environment in account A with cdk bootstrap aws://123456789012/eu-west-1 --trust 111122223333 --cloudformation-execution-policies arn:aws:iam::aws:policy/AdministratorAccess --trust-for-lookup 111122223333. Then modify the trust policy on the four roles described above to require an external id when account B is assuming the role. (Note: --trust-for-lookup is redundant if --trust is also specified for the same account and doesn't fix the issue whether you use it or not)

In the CDK app, pass in these external ids to the stack to be deployed like so

new ExternalIdStack(app, 'ExternalIdTestingStack', {
  synthesizer: new DefaultStackSynthesizer({
    lookupRoleExternalId: 'myexternalid',
    deployRoleExternalId: 'myexternalid',
    fileAssetPublishingExternalId: 'myexternalid',
    imageAssetPublishingExternalId: 'myexternalid'
  }),
  env: {
    region: 'eu-west-1', account: testAccountId
  }
})

Inside the stack, you can test the various roles by commenting out each section. Both the image role and file role succeed deployment, indicating that the image role, file role, and deploy role all can be assumed through CDK when an external ID is required. However even with the exact same trust policy as the others, the lookup role never succeeds.

    // Test Lookup Role
    Vpc.fromLookup(this, 'Vpc', {
      vpcName: 'MyVpc'
    });

    // Test File Role
    // new BucketDeployment(this, 'BucketDeployment', {
    //   sources: [Source.asset('./assets')],
    //   destinationBucket: new Bucket(this, 'DestinationBucket', {
    //     autoDeleteObjects: true,
    //     removalPolicy: RemovalPolicy.DESTROY
    //   }),
    // });

    // Test Image Role
    // const td = new TaskDefinition(this, 'TaskDefinition', {
    //   compatibility: Compatibility.EC2_AND_FARGATE,
    //   cpu: '1024',
    //   memoryMiB: '2048'
    // });
    // td.addContainer('MyContainer', {
    //   image: ContainerImage.fromAsset('./docker'),
    //   memoryLimitMiB: 2048
    // })

Possible Solution

On a quick look the code seemed to pass this properly all the say to the Sdk provider. Though I'm not sure what else would be interfering with this

Additional Information/Context

No response

CDK CLI Version

2.46.0

Framework Version

No response

Node.js Version

16

OS

Mac

Language

Typescript

Language Version

No response

Other information

This was originally reported in a SIM ticket, I was able to reproduce and investigated the other roles as well

@peterwoodworth peterwoodworth added bug This issue is a bug. p1 effort/small Small work item – less than a day of effort package/tools Related to AWS CDK Tools or CLI labels Oct 17, 2022
@ddhanak
Copy link

ddhanak commented Nov 2, 2022

I just faced this issue, is there a workaround?

edit: I manually edited one of the role to allow the sts:AssumeRole on the lookup-role that is created by the bootstrap command in the destination account to make it work but this seems to be quite a tedious workaround.

@danieljandey
Copy link

danieljandey commented Sep 22, 2023

Also facing this same issue. Weird that it’s only occurring for the lookup role.

@sumupitchayan
Copy link
Contributor

Reproduction Steps

Bootstrap an environment in account A with cdk bootstrap aws://123456789012/eu-west-1 --trust 111122223333 --cloudformation-execution-policies arn:aws:iam::aws:policy/AdministratorAccess --trust-for-lookup 111122223333. Then modify the trust policy on the four roles described above to require an external id when account B is assuming the role. (Note: --trust-for-lookup is redundant if --trust is also specified for the same account and doesn't fix the issue whether you use it or not)

How are you modifying the trust policy exactly? Are you modifying the bootstrap template or using a custom bootstrap template?

Copy link

Comments on closed issues and PRs are hard for our team to see.
If you need help, please open a new issue that references this one.

1 similar comment
Copy link

Comments on closed issues and PRs are hard for our team to see.
If you need help, please open a new issue that references this one.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 23, 2024
michelle-wangg pushed a commit to michelle-wangg/aws-cdk that referenced this issue Sep 23, 2024
Allow passing [all STS options](https://docs.aws.amazon.com/STS/latest/APIReference/API_AssumeRole.html#API_AssumeRole_RequestParameters) to assume role configuration for various CDK roles. 

> The following PR description focuses on Session Tags because it was originally the only option we wanted to add support to. After some thought, we decided to allow all available STS options via a transparent pass-through.

### Prerequisites

- cdklabs/cloud-assembly-schema#33
- cdklabs/cdk-assets#40

### Issue # (if applicable)

Closes aws#26157 
Fixes aws#22535

### Reason for this change

Enabling [ABAC](https://docs.aws.amazon.com/IAM/latest/UserGuide/introduction_attribute-based-access-control.html) via STS session tags. From the [AWS docs](https://docs.aws.amazon.com/IAM/latest/UserGuide/id_session-tags.html):


> _“Session Tags are key-value pair attributes that you pass when you assume an IAM role or federate a user in AWS STS. You do this by making an AWS CLI or AWS API request through AWS STS or through your identity provider (IdP). When you use AWS STS to request temporary security credentials, you generate a session. Sessions expire and have [credentials](https://docs.aws.amazon.com/STS/latest/APIReference/API_Credentials.html), such as an access key pair and a session token. When you use the session credentials to make a subsequent request, the [request context](https://docs.aws.amazon.com/IAM/latest/UserGuide/reference_policies_elements_condition.html#AccessPolicyLanguage_RequestContext) includes the [aws:PrincipalTag](https://docs.aws.amazon.com/IAM/latest/UserGuide/reference_policies_condition-keys.html#condition-keys-principaltag) context key. You can use the aws:PrincipalTag key in the Condition element of your policies to allow or deny access based on those tags”_

### Description of changes

The CDK creates [4 IAM roles](https://docs.aws.amazon.com/cdk/v2/guide/bootstrapping-env.html#bootstrapping-env-roles) during bootstrap. It then assumes these roles at various phases of the deployment.

- [DeploymentActionRole](https://github.com/aws/aws-cdk/blob/v2.154.1/packages/aws-cdk/lib/api/bootstrap/bootstrap-template.yaml#L429): Assumed when invoking CloudFormation operations such as _Deploy_ and _DescribeStackEvents_.
- [FilePublishingRole](https://github.com/aws/aws-cdk/blob/v2.154.1/packages/aws-cdk/lib/api/bootstrap/bootstrap-template.yaml#L275): Assumed when file assets are uploaded to the bootstrap bucket.
- [ImagePublishingRole](https://github.com/aws/aws-cdk/blob/v2.154.1/packages/aws-cdk/lib/api/bootstrap/bootstrap-template.yaml#L298): Assumed when docker images are published to the bootstrap ECR registry.
- [LookupRole](https://github.com/aws/aws-cdk/blob/v2.154.1/packages/aws-cdk/lib/api/bootstrap/bootstrap-template.yaml#L321): Assumed while performing context lookups.

Each of these roles should be assumable with their own specific session tags, as they server different purposes.

> Note: The [CloudFormationExecutionRole](https://github.com/aws/aws-cdk/blob/v2.154.1/packages/aws-cdk/lib/api/bootstrap/bootstrap-template.yaml#L536) is also created during bootstrap, but the CLI never assumes it, therefore it doesn't need session tags support.

Session tags for each role will be configurable via synthesizer properties (similarly to how `externalId` is [exposed](aws#15604)) both for the `DefaultStackSynthesizer`, and for a custom synthesizer extending the `StackSynthesizer` class. The new properties will propagate down and will eventually be written to the cloud assembly.

#### `+ manifest.json`

```json
{
  "version": "36.0.0",
  "artifacts": {
    "MyStack.assets": {
      "type": "cdk:asset-manifest",
      "properties": {
        "file": "SeshTagsManifestStack.assets.json",
        "requiresBootstrapStackVersion": 6,
        "bootstrapStackVersionSsmParameter": "/cdk-bootstrap/hnb659fds/version"
      }
    },
    "MyStack": {
      "type": "aws:cloudformation:stack",
      "environment": "aws://unknown-account/unknown-region",
      "properties": {
        "assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-deploy-role-${AWS::AccountId}-${AWS::Region}",
          "assumeRoleAdditionalOptions": {
            "Tags": < deployRoleSessionTags > 
          }
        "lookupRole": {
          "arn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-lookup-role-${AWS::AccountId}-${AWS::Region}",
          "requiresBootstrapStackVersion": 8,
          "bootstrapStackVersionSsmParameter": "/cdk-bootstrap/hnb659fds/version"
          "assumeRoleAdditionalOptions": {
            "Tags": < lookupRoleSessionTags > 
          }
        }
      },
```

#### `+ assets.json`

```json
{
  "version": "36.0.0",
  "files": {
    "9ebfd704f02f99b2711998e6435822b0dbed6e80dcac7e75f339fe894861ac20": {
      "source": {
        "path": "mystack.template.json",
        "packaging": "file"
      },
      "destinations": {
        "current_account-current_region": {
          "assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-file-publishing-role-${AWS::AccountId}-${AWS::Region}"
          "assumeRoleAdditionalOptions": {
             "Tags": < fileAssetPublishingRoleSessionTags >
           }
        }
      }
    }
  },
  "dockerImages": {
    "dockerHash": {
      "source": {
        "directory": "."
      },
      "destinations": {
        "current_account-current_region": {
          "assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-image-publishing-role-${AWS::AccountId}-${AWS::Region}"
          "assumeRoleAdditionalOptions": {
            "Tags": < imageAssetPublishingRoleSessionTags >
          }
        }
      }
    }
  }
}
```

### Description of how you validated changes

- CLI integration tests.
- CLI and framework unit tests.

### Checklist
- [X] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md)

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
HBobertz pushed a commit that referenced this issue Sep 24, 2024
Allow passing [all STS options](https://docs.aws.amazon.com/STS/latest/APIReference/API_AssumeRole.html#API_AssumeRole_RequestParameters) to assume role configuration for various CDK roles. 

> The following PR description focuses on Session Tags because it was originally the only option we wanted to add support to. After some thought, we decided to allow all available STS options via a transparent pass-through.

### Prerequisites

- cdklabs/cloud-assembly-schema#33
- cdklabs/cdk-assets#40

### Issue # (if applicable)

Closes #26157 
Fixes #22535

### Reason for this change

Enabling [ABAC](https://docs.aws.amazon.com/IAM/latest/UserGuide/introduction_attribute-based-access-control.html) via STS session tags. From the [AWS docs](https://docs.aws.amazon.com/IAM/latest/UserGuide/id_session-tags.html):


> _“Session Tags are key-value pair attributes that you pass when you assume an IAM role or federate a user in AWS STS. You do this by making an AWS CLI or AWS API request through AWS STS or through your identity provider (IdP). When you use AWS STS to request temporary security credentials, you generate a session. Sessions expire and have [credentials](https://docs.aws.amazon.com/STS/latest/APIReference/API_Credentials.html), such as an access key pair and a session token. When you use the session credentials to make a subsequent request, the [request context](https://docs.aws.amazon.com/IAM/latest/UserGuide/reference_policies_elements_condition.html#AccessPolicyLanguage_RequestContext) includes the [aws:PrincipalTag](https://docs.aws.amazon.com/IAM/latest/UserGuide/reference_policies_condition-keys.html#condition-keys-principaltag) context key. You can use the aws:PrincipalTag key in the Condition element of your policies to allow or deny access based on those tags”_

### Description of changes

The CDK creates [4 IAM roles](https://docs.aws.amazon.com/cdk/v2/guide/bootstrapping-env.html#bootstrapping-env-roles) during bootstrap. It then assumes these roles at various phases of the deployment.

- [DeploymentActionRole](https://github.com/aws/aws-cdk/blob/v2.154.1/packages/aws-cdk/lib/api/bootstrap/bootstrap-template.yaml#L429): Assumed when invoking CloudFormation operations such as _Deploy_ and _DescribeStackEvents_.
- [FilePublishingRole](https://github.com/aws/aws-cdk/blob/v2.154.1/packages/aws-cdk/lib/api/bootstrap/bootstrap-template.yaml#L275): Assumed when file assets are uploaded to the bootstrap bucket.
- [ImagePublishingRole](https://github.com/aws/aws-cdk/blob/v2.154.1/packages/aws-cdk/lib/api/bootstrap/bootstrap-template.yaml#L298): Assumed when docker images are published to the bootstrap ECR registry.
- [LookupRole](https://github.com/aws/aws-cdk/blob/v2.154.1/packages/aws-cdk/lib/api/bootstrap/bootstrap-template.yaml#L321): Assumed while performing context lookups.

Each of these roles should be assumable with their own specific session tags, as they server different purposes.

> Note: The [CloudFormationExecutionRole](https://github.com/aws/aws-cdk/blob/v2.154.1/packages/aws-cdk/lib/api/bootstrap/bootstrap-template.yaml#L536) is also created during bootstrap, but the CLI never assumes it, therefore it doesn't need session tags support.

Session tags for each role will be configurable via synthesizer properties (similarly to how `externalId` is [exposed](#15604)) both for the `DefaultStackSynthesizer`, and for a custom synthesizer extending the `StackSynthesizer` class. The new properties will propagate down and will eventually be written to the cloud assembly.

#### `+ manifest.json`

```json
{
  "version": "36.0.0",
  "artifacts": {
    "MyStack.assets": {
      "type": "cdk:asset-manifest",
      "properties": {
        "file": "SeshTagsManifestStack.assets.json",
        "requiresBootstrapStackVersion": 6,
        "bootstrapStackVersionSsmParameter": "/cdk-bootstrap/hnb659fds/version"
      }
    },
    "MyStack": {
      "type": "aws:cloudformation:stack",
      "environment": "aws://unknown-account/unknown-region",
      "properties": {
        "assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-deploy-role-${AWS::AccountId}-${AWS::Region}",
          "assumeRoleAdditionalOptions": {
            "Tags": < deployRoleSessionTags > 
          }
        "lookupRole": {
          "arn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-lookup-role-${AWS::AccountId}-${AWS::Region}",
          "requiresBootstrapStackVersion": 8,
          "bootstrapStackVersionSsmParameter": "/cdk-bootstrap/hnb659fds/version"
          "assumeRoleAdditionalOptions": {
            "Tags": < lookupRoleSessionTags > 
          }
        }
      },
```

#### `+ assets.json`

```json
{
  "version": "36.0.0",
  "files": {
    "9ebfd704f02f99b2711998e6435822b0dbed6e80dcac7e75f339fe894861ac20": {
      "source": {
        "path": "mystack.template.json",
        "packaging": "file"
      },
      "destinations": {
        "current_account-current_region": {
          "assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-file-publishing-role-${AWS::AccountId}-${AWS::Region}"
          "assumeRoleAdditionalOptions": {
             "Tags": < fileAssetPublishingRoleSessionTags >
           }
        }
      }
    }
  },
  "dockerImages": {
    "dockerHash": {
      "source": {
        "directory": "."
      },
      "destinations": {
        "current_account-current_region": {
          "assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-image-publishing-role-${AWS::AccountId}-${AWS::Region}"
          "assumeRoleAdditionalOptions": {
            "Tags": < imageAssetPublishingRoleSessionTags >
          }
        }
      }
    }
  }
}
```

### Description of how you validated changes

- CLI integration tests.
- CLI and framework unit tests.

### Checklist
- [X] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md)

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug This issue is a bug. effort/small Small work item – less than a day of effort p1 package/tools Related to AWS CDK Tools or CLI
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants