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.

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

CloudTrail Trail with Prefix doesn't set correct bucket policy #6741

Closed
ralovely opened this issue Mar 16, 2020 · 0 comments · Fixed by #7053
Closed

CloudTrail Trail with Prefix doesn't set correct bucket policy #6741

ralovely opened this issue Mar 16, 2020 · 0 comments · Fixed by #7053
Assignees
Labels
@aws-cdk/aws-cloudtrail Related to AWS CloudTrail bug This issue is a bug. effort/small Small work item – less than a day of effort good first issue Related to contributions. See CONTRIBUTING.md in-progress This issue is being actively worked on. p2

Comments

@ralovely
Copy link

When creating a CloudTrail trail via the Trail construct, adding an s3KeyPrefix will result in CloudFormation failing with Error Code: InsufficientS3BucketPolicyException.

Reproduction Steps

Add this to a stack:

const trail = new cloudtrail.Trail(this, 'MyTrail', {
  s3KeyPrefix: 'blah'
});

then cdk deploy.

Error Log

8/9 | 10:37:27 pm | CREATE_FAILED        | AWS::CloudTrail::Trail | CloudTrail (CloudTrailA62D711D) Incorrect S3 bucket policy is detected for bucket: cloudtrailstack-cloudtrailbucketc68b2c3b-pawnspu0dxgs (Service: AWSCloudTrail; Status Code: 400; Error Code: InsufficientS3BucketPolicyException; Request ID: a6ff0cd7-301e-42c7-b3d4-b8c48f38cde3)
  new Trail (~/ct-test/node_modules/@aws-cdk/aws-cloudtrail/lib/index.js:74:23)
  \_ new CloudtrailStack (~/ct-test/lib/cloudtrail-stack.js:24:19)
  \_ Object.<anonymous> (~/ct-test/bin/ct-test.js:7:1)
  \_ Module._compile (internal/modules/cjs/loader.js:1121:30)
  \_ Object.Module._extensions..js (internal/modules/cjs/loader.js:1160:10)
  \_ Module.load (internal/modules/cjs/loader.js:976:32)
  \_ Function.Module._load (internal/modules/cjs/loader.js:884:14)
  \_ Function.executeUserEntryPoint [as runMain] (internal/modules/run_main.js:67:12)
  \_ internal/main/run_main_module.js:17:47

Environment

  • CLI Version : 1.27.0
  • Framework Version: 1.27.0
  • OS : MacOS Catalina
  • Language : Javascript

Other

The snippet above will create a CloudFormation template with this policy:

PolicyDocument:
  Statement:
    - Action: s3:GetBucketAcl
      Effect: Allow
      Principal:
        Service: cloudtrail.amazonaws.com
      Resource:
        Fn::GetAtt:
          - MyTrailS3F1F68733
          - Arn
    - Action: s3:PutObject
      Condition:
        StringEquals:
          s3:x-amz-acl: bucket-owner-full-control
      Effect: Allow
      Principal:
        Service: cloudtrail.amazonaws.com
      Resource:
        Fn::Join:
          - ""
          - - Fn::GetAtt:
                - MyTrailS3F1F68733
                - Arn
            - /AWSLogs/
            - Ref: AWS::AccountId
            - /*

The policy doesn't include the prefix in the resource statements.

This is confirmed in the source code where there is no mention of props.s3KeyPrefix:

resources: [this.s3bucket.arnForObjects(`AWSLogs/${Stack.of(this).account}/*`)],

When created via the console, a trail with a prefix results in a policy like:

{
    "Version": "2012-10-17",
    "Statement": [
        {
            "Sid": "AWSCloudTrailAclCheck20150319",
            "Effect": "Allow",
            "Principal": {
                "Service": "cloudtrail.amazonaws.com"
            },
            "Action": "s3:GetBucketAcl",
            "Resource": "arn:aws:s3:::mytrailbucket"
        },
        {
            "Sid": "AWSCloudTrailWrite20150319",
            "Effect": "Allow",
            "Principal": {
                "Service": "cloudtrail.amazonaws.com"
            },
            "Action": "s3:PutObject",
            "Resource": "arn:aws:s3:::mytrailbucket/blah/AWSLogs/111111111111/*",
            "Condition": {
                "StringEquals": {
                    "s3:x-amz-acl": "bucket-owner-full-control"
                }
            }
        }
    ]
}

Thank you.


This is 🐛 Bug Report

@ralovely ralovely added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Mar 16, 2020
@SomayaB SomayaB added the @aws-cdk/aws-cloudtrail Related to AWS CloudTrail label Mar 16, 2020
@rix0rrr rix0rrr added p2 effort/small Small work item – less than a day of effort good first issue Related to contributions. See CONTRIBUTING.md labels Mar 17, 2020
RafalWilinski added a commit to RafalWilinski/aws-cdk that referenced this issue Mar 28, 2020
@SomayaB SomayaB added in-progress This issue is being actively worked on. and removed needs-triage This issue or PR still needs to be triaged. labels Mar 30, 2020
@mergify mergify bot closed this as completed in #7053 Mar 31, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-cloudtrail Related to AWS CloudTrail bug This issue is a bug. effort/small Small work item – less than a day of effort good first issue Related to contributions. See CONTRIBUTING.md in-progress This issue is being actively worked on. p2
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants