Skip to content

fix(bootstrap): disallow AssumeRole with ExternalId by default#811

Merged
rix0rrr merged 14 commits intomainfrom
huijbers/no-deputized-assumeroles
Aug 22, 2025
Merged

fix(bootstrap): disallow AssumeRole with ExternalId by default#811
rix0rrr merged 14 commits intomainfrom
huijbers/no-deputized-assumeroles

Conversation

@rix0rrr
Copy link
Copy Markdown
Contributor

@rix0rrr rix0rrr commented Aug 21, 2025

By default, CDK Bootstrap roles are not designed to be deputized.

(Deputized means that you give an external entity access to assume roles on your behalf. They will supply an ExternalId to avoid Confused Deputy attacks)

If a deputy system (i.e., a system that assumes IAM Roles on behalf of its tenants) is using CDK and its policies are not configured carefully, it can be tricked into assuming its own CDK roles.

Because CDK Roles are not intended to be used in this way, we are adding a default security control that will make this misconfiguration less likely: AssumeRole calls with ExternalIds will be denied by default.

What if I do want to use ExternalIds?

If you are currently passing ExternalIds in an AssumeRole call to CDK bootstrap roles inside your own trusted organization (expecting the ExternalId to be present but ignored), this protection can be disabled by calling:

$ cdk bootstrap --no-deny-external-id

If you want to give permissions for other organizations to assume your CDK bootstrap roles in a deputized way, customize the bootstrap template and add a proper ExternalId condition.


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

By default, CDK Bootstrap roles are not designed to be deputized.

(Deputized means that you give an external entity access to assume roles
on your behalf. They will supply an ExternalId to avoid [Confused Deputy
attacks](https://docs.aws.amazon.com/IAM/latest/UserGuide/confused-deputy.html))

If a deputy system (i.e., a system that assumes IAM Roles on behalf of
its tenants) is using CDK and its policies are not configured carefully,
it can be tricked into assuming its own CDK roles.

Because CDK Roles are not intended to be used in this way, we are adding
a default security control that will make this misconfiguration less
likely: AssumeRole calls with ExternalIds will be denied by default.

What if I do want to use ExternalIds?
-------------------------------------

If you are currently passing `ExternalId`s in an `AssumeRole` call to
CDK bootstrap roles *inside your own trusted organization* (expecting the
ExternalId to be present but ignored), this protection can be disabled
by calling:

```
cdk bootstrap --no-deny-external-id
```

If you want to give permissions for other organizations to assume your
CDK bootstrap roles in a deputized way, customize the bootstrap template
and add a proper `ExternalId` condition.
@rix0rrr rix0rrr requested a review from a team August 21, 2025 11:47
@github-actions github-actions Bot added the p2 label Aug 21, 2025
@aws-cdk-automation aws-cdk-automation requested a review from a team August 21, 2025 11:48
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Aug 21, 2025

Codecov Report

❌ Patch coverage is 0% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 81.54%. Comparing base (80d4d15) to head (cceb843).
⚠️ Report is 274 commits behind head on main.

Files with missing lines Patch % Lines
packages/aws-cdk/lib/cli/cli.ts 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #811      +/-   ##
==========================================
+ Coverage   80.91%   81.54%   +0.62%     
==========================================
  Files          63       63              
  Lines        8618     8619       +1     
  Branches     1035     1040       +5     
==========================================
+ Hits         6973     7028      +55     
+ Misses       1616     1562      -54     
  Partials       29       29              
Flag Coverage Δ
suite.unit 81.54% <0.00%> (+0.62%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@rix0rrr
Copy link
Copy Markdown
Contributor Author

rix0rrr commented Aug 22, 2025

What do you mean this is a breaking change? W.r.t existing customized templates you mean?

Signed-off-by: github-actions <[email protected]>
Comment on lines +236 to +241
if (params.denyExternalId !== undefined) {
if (!templateParameters.includes('DenyExternalId')) {
throw new ToolkitError('The selected bootstrap template does not accept the DenyExternalId parameter');
}
bootstrapTemplateParameters.DenyExternalId = `${params.denyExternalId}`;
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Q: Should this just be a warning?
Q: Should we apply this to EVERY paramter?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A1: Perhaps. But we're making a clear distinction between passing the value and not passing it. If you pass the value, I don't think we'd want to drop it with only a warning.

A2: Perhaps. Hasn't caused a problem so far, so I'm not going to be holier than the pope without any customer pressure.

@rix0rrr rix0rrr marked this pull request as ready for review August 22, 2025 09:11
@rix0rrr rix0rrr enabled auto-merge August 22, 2025 09:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants