allquixotic commented 2 years ago

Describe the feature

Setting up an event rule (AWS::Events::Rule) requires a tremendous amount of infrastructure beneath it, all configured in very precise and not very well documented ways, to be able to react to arbitrary API calls logged to CloudTrail.

CDK makes it much easier than CloudFormation, but it could still be easier. Specifically, I'd like the ability for the CloudTrail and/or Events modules of CDK to automatically configure the KeyPolicy of the AWS::KMS::Key that you create for encrypting the CloudTrail log bucket. This is really tricky to get right on your own.

Use Case

Here is my current code in Python:

mykey = kms.Key(self, "LogKey", alias="logkey")
trail = cloudtrail.Trail(self, "myCloudTrail", send_to_cloud_watch_logs=True, encryption_key=mykey)
rule = events.Rule(self, "rule", event_pattern=events.EventPattern(
  source=["aws.organizations"],
  detail_type=["AWS API Call via CloudTrail"],
  detail={
    "serviceEventDetails": {
      "createAccountStatus": {
        "state": ["SUCCEEDED"]
      }
    },
    "eventName": ["CreateAccountResult"]
  }
), targets= [
  targets.EventBus(events.eventBus.from_event_bus_arn(self, id="TargetEventBus", event_bus_arn="arn:aws:events:us-east-1:111111111111:event-bus/TargetEventBus"))])

Even with all this code, expressing the insanely simple idea of "run a Lambda function in a member account when a new account is created in the AWS Organization" is still not working. It fails when trying to create the AWS::CloudTrail::Trail with:

Resource handler returned message: "Invalid request provided: Insufficient permissions to access S3 bucket blahblah or KMS key arn:aws:kms:us-east-1:111111111111:key/(guid). (Service: CloudTrail, Status Code: 400, Request ID: guid, ...

And I haven't even shown the code for creating the event bus and Lambda function in the member account!

It's not the CDK's fault that the infrastructure for such a seemingly simple use case requires a degree in rocket science to successfully stand up, but CDK can certainly make this easier.

It seems the culprit of my code is either:

The bucket, which automatically gets created by the CloudTrail CDK construct, doesn't have a sufficiently open bucket policy for CloudTrail itself to be able to access it (this seems unlikely); or:
The bucket is fine, but the KMS Key's KeyPolicy was not adjusted by the subsequent code to ensure that CloudTrail and EventBridge can use the key as needed.

I've been studying the pure CloudFormation way to do all this for hours, and I still don't understand exactly what the KeyPolicy and BucketPolicy are supposed to be for this seemingly simple use case that can be expressed in one sentence. I wish CDK would just take care of it for me in a construct.

Proposed Solution

Enhance existing constructs, or create a new construct, to handle the design pattern here, and chop off all the boilerplate, so that the developer only has to concern themselves with the correct EventPattern and Targets for the EventBridge rule, and can completely abstract away the PhD-level machinations of permissions on the S3 bucket and KMS key to make this idea a reality.

Other Information

No response

Acknowledgements

[ ] I may be able to implement this feature request
[ ] This feature might incur a breaking change

CDK version used

2.24.1

Environment details (OS name and version, etc.)

RHEL 8 with Python 3.9

rix0rrr commented 2 years ago

I agree that the Trail construct could have helped you more here. Of course, today changing it to add that policy would be a risky change that would require a feature flag.

In the mean time, I would suggest adding a key policy yourself, stating that anyone allowed to write to that S3 bucket can encrypt using KMS:

key.grantEncrypt(new kms.ViaServicePrincipal('s3.amazonaws.com', new iam.AnyPrincipal()));

https://docs.aws.amazon.com/cdk/api/v1/docs/@aws-cdk_aws-kms.ViaServicePrincipal.html

Either that or:

key.grantEncrypt(new iam.ServicePrincipal('cloudtrail.amazonaws.com'));

timothy-cloudopsguy commented 1 year ago

FWIW, neither of the grants mentioned by @rix0rrr worked for me.

Here's what I did in python CDK to make it work for creating CloudTrail using CMK. I had to make the S3 bucket and the Log group on my own...

            #
            # Create a cloudwatch log group and CMK for it
            #
            self.cloudtrail_kms_key = _kms.Key(
                self, 
                id="cloudtrail-kms-Key",
                enable_key_rotation=True,
                alias="infra-security-cloudtrail-logs",
                description="Key to encrypt CloudTrail"
            )
            self.cloudtrail_kms_key.grant_encrypt_decrypt(_iam.ServicePrincipal(f"logs.{self.region}.amazonaws.com"))
            self.cloudtrail_kms_key.grant_encrypt_decrypt(_iam.ServicePrincipal(f"cloudtrail.amazonaws.com"))
            self.cloudtrail_kms_key.grant_encrypt(_kms.ViaServicePrincipal('s3.amazonaws.com', _iam.AnyPrincipal()));

            #
            # Create a log group for CloudTrail
            #
            self.cloudtrail_log_group = _logs.LogGroup(
                self,
                id='cloudtrail_log_group',
                encryption_key=self.cloudtrail_kms_key,
                retention=_logs.RetentionDays.ONE_YEAR,
                removal_policy=RemovalPolicy.RETAIN
            )
            self.cloudtrail_log_group.node.add_dependency(self.cloudtrail_kms_key)

            #
            # Put the log group name into an SSM Parameter, so any stack or lambda can find it
            #
            self.cloudtrail_log_group_ssm = _ssm.StringParameter(
                self,
                id='cloudtrail_log_group_ssm',
                string_value=self.cloudtrail_log_group.log_group_name,
                parameter_name="/infra/security/cloudtrail/log_group_name"
            )

            #
            # Create an S3 bucket to use for cloudtrail. Trail can do this, but if you specify a CMK,
            # it does NOT automagically add the CMK to the bucket or allow the bucket to encrypt
            #
            self.cloudtrail_s3 = _s3.Bucket(
                self,
                id="cloudtrail-s3",
                block_public_access=_s3.BlockPublicAccess.BLOCK_ALL,
                bucket_key_enabled=True,
                encryption=_s3.BucketEncryption.KMS,
                encryption_key=self.cloudtrail_kms_key,
                enforce_ssl=True,
                removal_policy=RemovalPolicy.RETAIN,
                versioned=True
            )
            self.cloudtrail_s3.grant_read_write(_iam.ServicePrincipal(f"cloudtrail.amazonaws.com"))

            # 
            # Create a cloudtrail for mgmt and point it to the cloudwatch log group
            # NOTE: this costs extra on top of S3 but it's needed for getting metrics alerting for compliance / CIS framework / AWS Partner Network
            self.cloudtrail = _cloudtrail.Trail(
                self,
                id="cloudtrail",
                bucket=self.cloudtrail_s3,
                encryption_key=self.cloudtrail_kms_key,
                enable_file_validation=True,
                include_global_service_events=True,
                is_multi_region_trail=True,
                is_organization_trail=True,
                management_events=_cloudtrail.ReadWriteType.ALL,
                cloud_watch_log_group=self.cloudtrail_log_group,
                send_to_cloud_watch_logs=True,
                cloud_watch_logs_retention=_logs.RetentionDays.ONE_YEAR,
            )

NOTE:

Before deploying this to the root/organization account, you must run this command in the root/org account to enable CloudTrail as a trusted service in the organization. This will enable enforcing the auto creation of a CloudTrail in each account and pointing them back to the org account.

aws organizations enable-aws-service-access --service-principal cloudtrail.amazonaws.com

Alternatively, you could log into the console and go here to enable it:

https://us-east-1.console.aws.amazon.com/organizations/v2/home/services

Reference: https://dev.to/aws-heroes/centralising-audit-compliance-and-incident-detection-11fi

beniusij commented 1 year ago

@allquixotic are you still having this issue? I've recently learned that encrypt, decrypt and kms:DescribeKey permissions need to be granted to cloudtrail.amazonaws.com service principal before passing that key to trail construct. Just keep in mind that other roles who should have access to view those logs, will need to have decrypt permission granted as well.

allquixotic commented 1 year ago

This is still an issue, yes. Looks like there are low-level workarounds using L1/escape hatches, but was hoping that the high level construct could take care of these encryption details.

beniusij commented 1 year ago

Apologies, but I didn't catch whether my suggestion helped. If you haven't, could you give it a try and let us know? I'm pretty confident in that as it has helped me.

tobi2736 commented 8 months ago

I am facing the same problem. I simply want to create a multi-region CloudTrail in a single AWS account using CDK. As soon as KMS encryption is enabled the failures are thrown. I don't want to build everything manually like timothy-cloudopsguy did. The solution for me was to add the following code to my Python project: my_kms_key.grant_encrypt_decrypt(iam.ServicePrincipal(f"logs.{self.region}.amazonaws.com")) my_kms_key.grant_encrypt_decrypt(iam.ServicePrincipal(f"cloudtrail.amazonaws.com")) my_kms_key.grant_encrypt(kms.ViaServicePrincipal('s3.amazonaws.com', iam.AnyPrincipal())) In my opinion, this needs to be fixed as a matter of priority, because an encrypted CloudTrail is really standard...

aws / aws-cdk

(cloudtrail): Encryption key doesn't help set up policies correctly #20344