-
Notifications
You must be signed in to change notification settings - Fork 4k
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
chore(bootstrap): prevent confused deputy #28342
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons. If you believe this pull request should receive an exemption, please comment and provide a justification.
A comment requesting an exemption should contain the text Exemption Request
. Additionally, if clarification is needed add Clarification Request
to a comment.
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.
I'm not necessarily against this change, but two things I'd like to clear up before we merge this:
|
We have deployed this change in our own infrastructure and CDK continues to operate flawlessly. Unlike #27764, this change makes things less permissive, so it can't open a security flaw for users with a legacy setup.
We recently paid for a penetration test and security audit of our AWS infrastructure and they flagged this cdk role as a potential risk of confused deputy. I don't know exact steps to reproduce, but the general idea is that this role trusts the cloudformation service and it might be possible to trick that service into assuming the cdk role in another account. This additional condition ensures that is never possible. I suspect that an actual exploit for this doesn't exist out there in the wild right now, but making this change adds defence in depth. |
I'm pretty sure this is not possible for 2 reasons:
For both of these reasons, this is not exploitable, nor is it ever likely to be.
|
@scarytom, looks like our stance is that we don't have an exploitable issue here. Unless you have a rebuttal, I will close this PR in a bit. |
This role is susceptible to cross-account confused deputy. Adding a condition prevents this.
see https://docs.aws.amazon.com/transcribe/latest/dg/security-iam-confused-deputy.html
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license