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.

Sign up for GitHub

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

Jump to bottom

[ecs] Over-broad IAM permissions in ECS Cluster ASG #9492

Closed
ddneilson opened this issue Aug 6, 2020 · 0 comments · Fixed by #9493
Closed

[ecs] Over-broad IAM permissions in ECS Cluster ASG #9492

ddneilson opened this issue Aug 6, 2020 · 0 comments · Fixed by #9493
Assignees
@uttarasridhar
Labels
@aws-cdk/aws-ecs Related to Amazon Elastic Container bug This issue is a bug. needs-triage This issue or PR still needs to be triaged.

Comments

@ddneilson
Copy link
Contributor

The AutoScalingGroup/capacity that is created for an ECS Cluster is created with overly broad IAM permissions. Specifically, the policy statement found here has overly broad '*' resources.

Reproduction Steps

Create any ECS cluster with capacity.

What did you expect to happen?

IAM Permissions that adhere to the principle of least privilege.

What actually happened?

Broad permissions

Environment

  • CLI Version : 1.52.0
  • Framework Version: 1.52.0
  • Node.js Version: any
  • OS : any
  • Language (Version): all

Other

Some specifics.

  • ecs:CreateCluster is not needed at all; the cluster already exists.
  • ecs:DeregisterContainerInstance, ecs:RegisterContainerInstance, and ecs:Submit* can and should be scoped-down to the specific cluster arn that the ASG will be servicing.
  • logs:CreateLogStream and logs:PutLogEvents appear to be unnecessary; the task will have appropriate permissions for writing to logs.

Also, I have a PR prepared to fix this and will be posting it shortly.

This is 🐛 Bug Report
@ddneilson ddneilson added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Aug 6, 2020
@ddneilson ddneilson mentioned this issue Aug 6, 2020
Merged
@SomayaB SomayaB added the @aws-cdk/aws-ecs Related to Amazon Elastic Container label Aug 6, 2020
@mergify mergify bot closed this as completed in #9493 Aug 7, 2020
mergify bot pushed a commit that referenced this issue Aug 7, 2020
@ddneilson
fix(ecs): Scope-down IAM permissions on Cluster ASG (#9493)
1670289 
This fixes #9492 by down-scoping some IAM permissions granted to the ASG that is created for an ECS cluster, and removing some unneccessary permissions.

### Testing

This was tested by deploying a simple app that was basically the sample from the ECS module readme, and verifying that: (a) the cluster is operational (i.e. tasks are running), and (b) those tasks are able to write to logs.

The essentials of the app are:
```ts
const app = new cdk.App();

const env = {
    account: process.env.CDK_DEFAULT_ACCOUNT,
    region: process.env.CDK_DEFAULT_REGION
}

const stack = new cdk.Stack(app, 'Testing', { env });
const vpc = new ec2.Vpc(stack, 'Vpc');

// Create an ECS cluster
const cluster = new ecs.Cluster(stack, 'Cluster', {
  vpc,
});

// Add capacity to it
cluster.addCapacity('DefaultAutoScalingGroupCapacity', {
  instanceType: new ec2.InstanceType("t2.xlarge"),
  desiredCapacity: 2,
});

const taskDefinition = new ecs.Ec2TaskDefinition(stack, 'TaskDef');

taskDefinition.addContainer('DefaultContainer', {
  image: ecs.ContainerImage.fromRegistry("amazon/amazon-ecs-sample"),
  memoryLimitMiB: 512,
  logging: ecs.LogDriver.awsLogs({
    logGroup: new logs.LogGroup(stack, 'LogGroup', {
      logGroupName: '/test-group/',
      removalPolicy: cdk.RemovalPolicy.DESTROY,
      retention: logs.RetentionDays.ONE_DAY,
    }),
    streamPrefix: 'testing-',
  }),
});

// Instantiate an Amazon ECS Service
const ecsService = new ecs.Ec2Service(stack, 'Service', {
  cluster,
  taskDefinition,
  desiredCount: 2,
});
```

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
eladb pushed a commit that referenced this issue Aug 10, 2020
@ddneilson
fix(ecs): Scope-down IAM permissions on Cluster ASG (#9493)
59f7713 
This fixes #9492 by down-scoping some IAM permissions granted to the ASG that is created for an ECS cluster, and removing some unneccessary permissions.

### Testing

This was tested by deploying a simple app that was basically the sample from the ECS module readme, and verifying that: (a) the cluster is operational (i.e. tasks are running), and (b) those tasks are able to write to logs.

The essentials of the app are:
```ts
const app = new cdk.App();

const env = {
    account: process.env.CDK_DEFAULT_ACCOUNT,
    region: process.env.CDK_DEFAULT_REGION
}

const stack = new cdk.Stack(app, 'Testing', { env });
const vpc = new ec2.Vpc(stack, 'Vpc');

// Create an ECS cluster
const cluster = new ecs.Cluster(stack, 'Cluster', {
  vpc,
});

// Add capacity to it
cluster.addCapacity('DefaultAutoScalingGroupCapacity', {
  instanceType: new ec2.InstanceType("t2.xlarge"),
  desiredCapacity: 2,
});

const taskDefinition = new ecs.Ec2TaskDefinition(stack, 'TaskDef');

taskDefinition.addContainer('DefaultContainer', {
  image: ecs.ContainerImage.fromRegistry("amazon/amazon-ecs-sample"),
  memoryLimitMiB: 512,
  logging: ecs.LogDriver.awsLogs({
    logGroup: new logs.LogGroup(stack, 'LogGroup', {
      logGroupName: '/test-group/',
      removalPolicy: cdk.RemovalPolicy.DESTROY,
      retention: logs.RetentionDays.ONE_DAY,
    }),
    streamPrefix: 'testing-',
  }),
});

// Instantiate an Amazon ECS Service
const ecsService = new ecs.Ec2Service(stack, 'Service', {
  cluster,
  taskDefinition,
  desiredCount: 2,
});
```

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@ianzylstra ianzylstra mentioned this issue Jan 5, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@uttarasridhar uttarasridhar

Labels
@aws-cdk/aws-ecs Related to Amazon Elastic Container bug This issue is a bug. needs-triage This issue or PR still needs to be triaged.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix(ecs): Scope-down IAM permissions on Cluster ASG ddneilson/aws-cdk
3 participants
@uttarasridhar @SomayaB @ddneilson