-
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
aws_ecs: overly broad permissions granted by enableExecuteCommand #31397
Comments
Hi @sandfox , thanks for reaching out. The issue is reproducible with the shared code sample. When the synthesized template has this DefaultPolicy - "FargateIssueTaskDefinitionv2ExecutionRoleDefaultPolicy1F84A7F4": {
"Type": "AWS::IAM::Policy",
"Properties": {
"PolicyDocument": {
"Statement": [
{
"Action": [
"logs:CreateLogStream",
"logs:PutLogEvents"
],
"Effect": "Allow",
"Resource": {
"Fn::GetAtt": [
"FargateIssueTaskDefinitionv2FargateIssueContainerv2LogGroup5E0366CD",
"Arn"
]
}
}
],
"Version": "2012-10-17"
},
.......
"FargateIssueTaskDefinitionv2TaskRoleDefaultPolicyE18491F5": {
"Type": "AWS::IAM::Policy",
"Properties": {
"PolicyDocument": {
"Statement": [
{
"Action": [
"logs:CreateLogStream",
"logs:DescribeLogGroups",
"logs:DescribeLogStreams",
"logs:PutLogEvents",
"ssmmessages:CreateControlChannel",
"ssmmessages:CreateDataChannel",
"ssmmessages:OpenControlChannel",
"ssmmessages:OpenDataChannel"
],
"Effect": "Allow",
"Resource": "*"
}
],
"Version": "2012-10-17"
},
"PolicyName": "FargateIssueTaskDefinitionv2TaskRoleDefaultPolicyE18491F5",
"Roles": [
{
"Ref": "FargateIssueTaskDefinitionv2TaskRoleBF6E7591"
}
]
}, and when its changed without any log configuration, still the policy has these statements - "FargateIssueTaskDefinitionv2TaskRoleDefaultPolicyE18491F5": {
"Type": "AWS::IAM::Policy",
"Properties": {
"PolicyDocument": {
"Statement": [
{
"Action": [
"logs:CreateLogStream",
"logs:DescribeLogGroups",
"logs:DescribeLogStreams",
"logs:PutLogEvents",
"ssmmessages:CreateControlChannel",
"ssmmessages:CreateDataChannel",
"ssmmessages:OpenControlChannel",
"ssmmessages:OpenDataChannel"
],
"Effect": "Allow",
"Resource": "*"
}
],
"Version": "2012-10-17"
},
"PolicyName": "FargateIssueTaskDefinitionv2TaskRoleDefaultPolicyE18491F5",
"Roles": [
{
"Ref": "FargateIssueTaskDefinitionv2TaskRoleBF6E7591"
}
] Looking at the cdk code , this block is getting executed if executeCommandLogging is anything but NONE - which might need to be updated with another check for log config. As per Docs, when the logging is not configured, output should not be logged. So this seems to be an issue. |
I think we should fix this line
when logging is undefined, at this moment, it would default to Maybe we should just leave const logging = this.cluster.executeCommandConfiguration?.logging or const logging = this.cluster.executeCommandConfiguration?.logging ?? ExecuteCommandLogging.NONE |
Thanks Pahud and Shailja for the deep dive. Also thanks to @sandfox for reporting the issue. I think this is a valid I pushed a PR to fix it but instead of using the suggested solution by Pahud, I chose to wrap a feature flag inside the method |
Comments on closed issues and PRs are hard for our team to see. |
1 similar comment
Comments on closed issues and PRs are hard for our team to see. |
Describe the bug
If a FargateService has
enableExecuteCommand: true
and the ECS cluster it runs on hasexecuteCommandConfiguration.logging
set to anything butecs.ExecuteCommandLogging.NONE
then the CDK automatically grants the underlying TaskDefinition overly broad cloudwatch logs permissions regardless of need. If the logging configuration has no cloudwatch logs config set then it allowsCreateLogStream
,DescribeLogStreams
,PutLogEvents
onresource: ["*"]
https://github.com/aws/aws-cdk/blob/af9e6bae94c0c303364c2c4f2033eb3823fb59c9/packages/aws-cdk-lib/aws-ecs/lib/base/base-service.ts#L754C1-L756C8
aws-cdk/packages/aws-cdk-lib/aws-ecs/lib/base/base-service.ts
Lines 1035 to 1052 in af9e6ba
As best I understand it, the CDK is automatically granting cloudwatch logs permissions if any kind
executeCommandConfiguration.logging
is enabled, even if there is no configuration set to send to logs to cloudwatch. I'm not aware of any reason why these permissions need to be automatically granted if there is no config to send logs to cloudwatch. It seems to me that these permissions should at least be behind some kind of "is cloudwatch logging enabled" check, and potentially not even be needed unless logging is set toecs.ExecuteCommandLogging.OVERRIDE
(based on my understanding of https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-ecs-cluster-executecommandconfiguration.html)
The current behaviour feels bad from a security point of view and does indeed trigger various security tooling to complain about overly broad write permissions.
e.g checkov
Regression Issue
Last Known Working CDK Version
No response
Expected Behavior
No extra cloudwatch logs permissions to be added to the Task Role
Current Behavior
lots of cloudwatch logs permissions get added to the task role.
Reproduction Steps
The text was updated successfully, but these errors were encountered: