(aws-cloudtrail): isOrganizationTrail attaches insufficient permissions to bucket

[danilobuerger]

Describe the bug

When using a Trail with isOrganizationTrail: true, the bucket policy contains insufficient permissions and the stack creation fails with:

Invalid request provided: Incorrect S3 bucket policy is detected for bucket: ...

The Trail attaches the following permissions:

aws-cdk/packages/@aws-cdk/aws-cloudtrail/lib/cloudtrail.ts

Lines 226 to 241 in c425e8c

	this.s3bucket.addToResourcePolicy(new iam.PolicyStatement({
	resources: [this.s3bucket.bucketArn],
	actions: ['s3:GetBucketAcl'],
	principals: [cloudTrailPrincipal],
	}));
	this.s3bucket.addToResourcePolicy(new iam.PolicyStatement({
	resources: [this.s3bucket.arnForObjects(
	`${props.s3KeyPrefix ? `${props.s3KeyPrefix}/` : ''}AWSLogs/${Stack.of(this).account}/*`,
	)],
	actions: ['s3:PutObject'],
	principals: [cloudTrailPrincipal],
	conditions: {
	StringEquals: { 's3:x-amz-acl': 'bucket-owner-full-control' },
	},
	}));

However, it is missing:

"Action": "s3:PutObject",
"Resource": "arn:aws:s3:::myOrganizationBucket/AWSLogs/o-exampleorgid/*"

as described here: https://docs.aws.amazon.com/awscloudtrail/latest/userguide/create-s3-bucket-policy-for-cloudtrail.html#org-trail-bucket-policy

Expected Behavior

It creates an organization trail.

Current Behavior

It fails with

Invalid request provided: Incorrect S3 bucket policy is detected for bucket: ...

Reproduction Steps

const bucket = new Bucket(this, "Bucket");
new Trail(this, 'Trail', { bucket, isOrganizationTrail: true });

Possible Solution

Adding the permission as outlined above.

Additional Information/Context

No response

CDK CLI Version

2.43.1

Framework Version

No response

Node.js Version

18.9.0

OS

macOS

Language

Typescript

Language Version

No response

Other information

No response

[peterwoodworth]

Thanks for reporting this @danilobuerger,

I can reproduce this just with this snippet

    new Trail(this, 'Trail', {
      isOrganizationTrail: true
    });

@daschaa did you ever run into this error? It's suspicious to me that the integration test you submitted with this PR works if I can't get it to deploy now here

[daschaa]

@peterwoodworth That's very strange. The integration tests should have failed in the pull request.
I'm sorry that I forgot to adjust the permissions!

The challenge that we will now face is, that we have to pass/retrieve the belonging organization id to construct the correct permission statement. @peterwoodworth Afaik there is no way to get the organization id from the props that are passed when instantiating a trail. Do you know if this is possible?

[danilobuerger]

@daschaa a few ideas:

1. pass the organization id as a prop
2. use a custom resource to fetch it: https://docs.aws.amazon.com/organizations/latest/APIReference/API_DescribeOrganization.html
3. Tighten the policy via aws:SourceArn instead, but allowing a wider s3 prefix:

"Principal": {
  "Service": [
    "cloudtrail.amazonaws.com"
  ]
},
"Action": "s3:PutObject",
"Resource": "arn:aws:s3:::myOrganizationBucket/AWSLogs/*",
"Condition": {
  "StringEquals": {
    "s3:x-amz-acl": "bucket-owner-full-control",
    "aws:SourceArn": "arn:aws:cloudtrail:region:111111111111:trail/trailName"
  }
}

[daschaa]

@danilobuerger Thanks for the good hints. ❤️
I like the idea to pass in a prop with the organization id. The challenge will be then to bind the flag isOrganizationTrail to the mandatory property organizationId. It's doable, but maybe not that clean.
An option would be to introduce a OrganizationTrail, which inherits all the stuff from a Trail but has a mandatory parameter organizationId. Then the boolean flag is no longer needed and from a user experience it is clear why an organization id is needed.

At the first glance the third option you mentioned would maybe be the easiest to implement and it would not introduce big changes to the current implementation. But I'm not sure if that would violate the principle of least privilege.

The second option is not a possibility from my point of view, as it may be that the lambda for the custom resource would have to run in a VPC. I know this from my experience and therefore I would not prefer this option.

@danilobuerger What is the best option in your opinion?

[sionelt]

I recently ran into the same issue and had to attach the missing organization policy to the bucket to make it work.

I hope I'm not putting in, but I do like the suggestion to have a new construct OrganizationTrail that extends the Trail, a much better design than the current isOrganizationTrail boolean. And requiring organizationId to narrow down the policy makes for better security.

[beniusij]

This bug still seems to have been active for a year since first reported. Are there any plans to fix this? Also, documentation in AWS seems to be inaccurate as the policies attached to the bucket are a bit different from the ones in the documentation. Namely, the documentation does not specify using organisation id for the "write" bucket policy's conditions section. Any plans to have that updated?

To be frank, I'm still facing this problem despite these suggestions so any assistance with the workaround would be massively appreciated!

[paulhcsun]

Hey @beniusij, a fix is in the works but for now I've found this comment which explains the workaround of adding in the missing policy in more detail from a duplicate issue: #11602 (comment).

[github-actions]

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.