pulumi / pulumi-awsx

AWS infrastructure best practices in component form!
https://www.pulumi.com/docs/guides/crosswalk/aws/
Apache License 2.0
219 stars 104 forks source link

TaskDefinition creates an inappropriate IAM Role by default #569

Open lukemundy-vgw opened 4 years ago

lukemundy-vgw commented 4 years ago

If a role isn't passed to awsx.ecs.TaskDefinition (and by extension also awsx.ecs.FargateTaskDefinition) a default role is created with two AWS-managed policies that allow the task some pretty broad and dangerous permissions.

The most egregious example is probably the blanket allow for iam:PassRole with no restrictions on which roles can be passed. This action is particularly dangerous as it can be used for privilege escalation, especially when combined with actions like iam:ListRoles (for enumerating available roles), iam:ListRolePolicies (listing policies attached to roles) and iam:GetRolePolicy (viewing details of policies attached to roles).

There are a number of other broad permissions that are slightly less concerning but still problematic to be part of a "default" policy, for example dynamodb:* which allows read/write/delete access to any data stored in DynamoDB, cloudformation:CreateStack and cloudformation:DeleteStack meaning tasks are now able to create their own resources etc

I understand that the intention of the AWSX resources is to "... provide component wrappers around many core AWS 'raw' resources to make them easier and more convenient to use" however I believe having this kind of default policy is actually quite dangerous and harmful and promotes bad security habits.

I'd like to propose that by default, no role should be created at all for a TaskDefinition unless one has been passed to it. I'd be happy to put together a PR that makes this change but wanted to discuss it first to ensure I don't waste my time.

What do you think?

lukemundy-vgw commented 3 years ago

Recently AWS have announced the deprecation of the two managed policies that are used by default by this resource and replacing them with more secure alternatives.

After the deprecation date the policies will continue to work for any existing resources but they will not be able to be attached to new resources, meaning any new deployments of awsx.ecs.FargateTaskDefinition after Jan 25th next year will fail.

leezen commented 3 years ago

We recently changed the defaults in the latest version of this package to use AmazonECSFullAccess (which corresponds to AmazonECS_FullAccess along with LambdaFullAccess (which corresponds to AWSLambda_FullAccess)

https://github.com/pulumi/pulumi-awsx/pull/631 https://github.com/pulumi/pulumi-awsx/pull/624

It's worth noting that LambdaFullAccess still uses iam:PassRole but only for within Lambda. Similarly, the new ECS policy has iam:PassRole as well, but again, scoped to a specific set of services (ECS, EC2, and Autoscaling). I think the two sets of policies strike a good balance between providing useful defaults while also not opening everything up (e.g. there's no dynamo:* or s3:*). If after reviewing the two sets of policies you still feel differently, please comment and I'll be happy to re-open this issue.

lukemundy-vgw commented 3 years ago

Hey @leezen, I can see the policy deprecation issues have indeed been fixed but I still think that these policies aren't really appropriate as defaults and the main concern I've tried to communicate in this issue hasn't really been addressed by those changes.

This particular role that is being defined is what all tasks based on these Task Definition resources will assume on boot up and retain for the life of that task. This means any code running in these tasks will, by default, have access to all the permissions provided by this policy. As of today, the following permissions are still part of either of the new AmazonECS_FullAccess or Lambda_FullAccess policies:

These permissions allow trivial creation (or deletion) of resources and even with the tighter restrictions on iam:PassRole it's still possible for a task with these permissions to trivially perform privilege escalation. For example compromised code could create a lambda function with arbitrary code and pass it the Administrator managed policy.

In addition to the above, there's two other primary reasons I think these aren't appropriate defaults for awsx.ecs.TaskDefinition:

Just to reiterate, I'd be happy to contribute fixes for this myself if we can agree on an approach.