search

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.71k stars 3.94k forks source link

Privilege escalation in codepipeline ecs deploy action #8303

Open winjer opened 4 years ago

winjer commented 4 years ago

The deploy action for ECS in a codepipeline attaches an iam:PassRole action to the codepipeline role which allows it to pass any role to ec2 and ecs-tasks:

https://github.com/aws/aws-cdk/blob/986e2814e072f0334d8470a2d60ea73dcceadfe4/packages/%40aws-cdk/aws-codepipeline-actions/lib/ecs/deploy-action.ts#L80-L91

This opens a potential privilege escalation.

The resources section of the statement should instead reference the relevant role for the ECS deployment I believe.

skinny85 commented 4 years ago

Hello @winjer ,

thanks for opening the issue. We discussed this with the ECS team, and unfortunately there is no way to scope down this permission, as the execution role for the task is not part of the CodePipeline definition.

This is in line with our Shared Responsibility Model for security, and the account boundary expectation:

https://docs.aws.amazon.com/cdk/latest/guide/security.html

Hope this clears things up!

Thanks, Adam

github-actions[bot] commented 4 years 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.

winjer commented 4 years ago

@skinny85 Sorry, I think this still needs resolving, and I think the shared responsibility thing is a red herring. I accept that it is my responsibility as a user to configure that role correctly, but I am running CDK code to do precisely that ;)

I am guessing you do not have access to the task role in all circumstances, and so this is hard, but that doesn't make it undesirable.

If I get the chance I will try and patch this, but I do believe this to be a real issue.

mattlorimor commented 2 years ago

I am 99% sure I just don't have the whole context or am misunderstanding, but is there a reason the condition check is a StringEqualsIfExists instead of a StringEquals? I know this doesn't address the * resources issue, but why is this policy expected to still allow iam:PassRole even when the iam:PassedToService key is not present (which is what StringEqualsIfExists would allow for).

This seems to imply that iam:PassRole is intended to be granted against all resources if the iam:PassedToService key is not present OR, if iam:PassedToService is present, it is one of the two defined service values.

mattlorimor commented 2 years ago

^^^ @ericzbeard - Is my question re: StringEqualsIfExists even relevant, here?