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.56k stars 3.87k forks source link

[aws-cloudtrail] Incorrect S3 bucket policy is detected for bucket #11602

Closed jaystary closed 2 years ago

jaystary commented 3 years ago

I'm trying to use a custom S3 bucket to store cloudtrail logs. This works, if I let cloudtrail create the bucket by not defining it explicitly, but when I define it explicitly I run into this issue:

Incorrect S3 bucket policy is detected for bucket: aaa-cloudtrail-prod (Service: AWSCloudTrail; Status Code: 400; Error Code : InsufficientS3BucketPolicyException; Request ID: 12c3d322-3443-4c23-822f-1d0c70235a84; Proxy: null)

This shows the diff between the automated created S3 Bucket generated permissions (in red) vs the ones I attached via CDK:

cloudtrail_issue

Reproduction Steps

s3bucket_ct.add_to_resource_policy(
                permission=iam.PolicyStatement(
                    principals=[
                        iam.ServicePrincipal('cloudtrail.amazonaws.com')
                    ],
                    actions=[
                        "s3:PutObject"
                    ],
                    resources=[
                        s3bucket_ct.arn_for_objects('AWSLogs/{}/*'.format(self.node.try_get_context('account')))
                    ],
                    conditions={
                        "StringEquals": {"s3:x-amz-acl": "bucket-owner-full-control"}
                    }
                )
            )

            s3bucket_ct.add_to_resource_policy(
                permission=iam.PolicyStatement(
                    principals=[
                        iam.ServicePrincipal('cloudtrail.amazonaws.com')
                    ],
                    actions=[
                        "s3:GetBucketAcl"
                    ],
                    resources=[
                        s3bucket_ct.bucket_arn
                    ]
                )
            )

ibuck_ct = s3bucket_ct.from_bucket_arn(self, id=f"aaa-BUCKET-ARN-CT-{repo}", bucket_arn = s3bucket_ct.bucket_arn)

trail = cloudtrail.Trail(self, f"aaa-CLOUDTRAIL-{repo}",
                enable_file_validation=True,
                include_global_service_events=False,
                is_multi_region_trail=False,
                management_events=cloudtrail.ReadWriteType.WRITE_ONLY,
                bucket=ibuck_ct
                )

What did you expect to happen?

Create CloudTrail with a custom S3 bucket that has permission to receive events just as it is created with the autogenerated S3 bucket

What actually happened?

InsufficientS3BucketPolicyException

Environment

This is :bug: Bug Report

rupertevill commented 3 years ago

@jaystary as a potential work around you may define the custom S3 bucket used for Cloudtrail logs in an initial stack, along with the calls to add_to_resource_policy.

Then this bucket may be passed to the stack containing the Cloudtrail definition 👍

Far from optimal but worked for me 🙂

mhoc commented 3 years ago

I'm not sure if this will specifically address the issue you're seeing, but: We ran into the same thing, and it seems like it had something to do with adding bucket policies in addition to the ones cloudtrail/cdk would automatically attach.

Specifically: with enforceSSL: true on the bucket configuration, it wouldn't work. This flag simply instructs the CDK to add another Deny clause to the bucket policy on requests that don't happen over HTTPs. Its unclear whether this is because Cloudtrail actually doesn't/can't deliver logs over SSL, or if its a more surface level issue where whatever part of AWS which is validating those bucket policies runs into that Deny clause and improperly fails validation, but regardless: removing that and ensuring the bucket policy was exclusively what the cloudtrail cdk attached worked for us without having to resort to using the S3 bucket new cloudtrail.Trail(...) would create when no bucket is provided.

const bucket = new s3.Bucket(this, "Bucket", {
    blockPublicAccess: s3.BlockPublicAccess.BLOCK_ALL,
    bucketName: "MyBucket",
    encryption: s3.BucketEncryption.KMS_MANAGED,
});
const trail = new cloudtrail.Trail(this, 'Trail', {
    bucket,
    isMultiRegionTrail: true,
    s3KeyPrefix: 'cloudtrail',
    trailName: 'MyTrail',  
 });
github-actions[bot] commented 2 years ago

This issue has not received any attention in 1 year. If you want to keep this issue open, please leave a comment below and auto-close will be canceled.

fractionalJoe commented 1 year ago

I see this issue is closed but this took me a while so sharing for any other lost souls. For me it wasn't an issue with enforceSSL, I was able to add that without issue. It was that I had isOrganizationTrail set. It generated the two standard policy statements on the bucket but not the third one needed for Organizations. I added the code below and was able to successfully deploy. Also, I notice that the two policies that were added added didn't include the "AWS:SourceArn" condition that's generated when adding through the console. Not a dealbreaker for me, but this would allow other Trails to write to this bucket.

const trailName = "trail-name";
const stack = Stack.of(this);
logBucket.addToResourcePolicy(
      new PolicyStatement({
        sid: "AWSCloudTrailOrgWrite",
        effect: Effect.ALLOW,
        principals: [new ServicePrincipal("cloudtrail.amazonaws.com")],
        actions: ["s3:PutObject"],
        resources: [`${logBucket.bucketArn}/AWSLogs/###AWSOrgID###/*`],
        conditions: {
          StringEquals: {
            "s3:x-amz-acl": "bucket-owner-full-control",
            "AWS:SourceArn": `arn:aws:cloudtrail:${stack.region}:${stack.account}:trail/${trailName}`,
          },
        },
      }),
    );

Replace ###AWSOrgID### with your AWS Organization ID. Note that if you try to use trail.trailArn for the AWS:SourceArn, you'll get a circular reference error so you need to build this manually.