fix: add validation for ALB access log bucket when KMS key is provided

packages/aws-cdk-lib/aws-elasticloadbalancingv2/lib/alb/application-load-balancer.ts

@@ -3,8 +3,11 @@ import { ApplicationListener, BaseApplicationListenerProps } from './application
 import { ListenerAction } from './application-listener-action';
 import * as cloudwatch from '../../../aws-cloudwatch';
 import * as ec2 from '../../../aws-ec2';
+import { PolicyStatement } from '../../../aws-iam/lib/policy-statement';
+import { ServicePrincipal } from '../../../aws-iam/lib/principals';
+import * as s3 from '../../../aws-s3';
 import * as cxschema from '../../../cloud-assembly-schema';
-import { Duration, Lazy, Names, Resource } from '../../../core';
+import { CfnResource, Duration, Lazy, Names, Resource, Stack } from '../../../core';
 import * as cxapi from '../../../cx-api';
 import { ApplicationELBMetrics } from '../elasticloadbalancingv2-canned-metrics.generated';
 import { BaseLoadBalancer, BaseLoadBalancerLookupOptions, BaseLoadBalancerProps, ILoadBalancerV2 } from '../shared/base-load-balancer';
@@ -151,6 +154,66 @@ export class ApplicationLoadBalancer extends BaseLoadBalancer implements IApplic
     });
   }
 
+  /**
+   * Enable access logging for this load balancer.
+   *
+   * A region must be specified on the stack containing the load balancer; you cannot enable logging on
+   * environment-agnostic stacks. See https://docs.aws.amazon.com/cdk/latest/guide/environments.html
+   */
+  public logAccessLogs(bucket: s3.IBucket, prefix?: string) {
+
+    /**
+    * KMS key encryption is not supported on Access Log bucket for ALB, the bucket must use Amazon S3-managed keys (SSE-S3).
+    * See https://docs.aws.amazon.com/elasticloadbalancing/latest/application/enable-access-logging.html#bucket-permissions-troubleshooting
+    */
+
+    if (bucket.encryptionKey) {
+      throw new Error('Encryption key detected. Bucket encryption using KMS keys is unsupported');
+    }
+
+    prefix = prefix || '';
+    this.setAttribute('access_logs.s3.enabled', 'true');
+    this.setAttribute('access_logs.s3.bucket', bucket.bucketName.toString());
+    this.setAttribute('access_logs.s3.prefix', prefix);
+
+    const logsDeliveryServicePrincipal = new ServicePrincipal('delivery.logs.amazonaws.com');
+    bucket.addToResourcePolicy(new PolicyStatement({
+      actions: ['s3:PutObject'],
+      principals: [this.resourcePolicyPrincipal()],
+      resources: [
+        bucket.arnForObjects(`${prefix ? prefix + '/' : ''}AWSLogs/${Stack.of(this).account}/*`),
+      ],
+    }));
+    bucket.addToResourcePolicy(
+      new PolicyStatement({
+        actions: ['s3:PutObject'],
+        principals: [logsDeliveryServicePrincipal],
+        resources: [
+          bucket.arnForObjects(`${prefix ? prefix + '/' : ''}AWSLogs/${this.env.account}/*`),
+        ],
+        conditions: {
+          StringEquals: { 's3:x-amz-acl': 'bucket-owner-full-control' },
+        },
+      }),
+    );
+    bucket.addToResourcePolicy(
+      new PolicyStatement({
+        actions: ['s3:GetBucketAcl'],
+        principals: [logsDeliveryServicePrincipal],
+        resources: [bucket.bucketArn],
+      }),
+    );
+
+    // make sure the bucket's policy is created before the ALB (see https://github.com/aws/aws-cdk/issues/1633)
+    // at the L1 level to avoid creating a circular dependency (see https://github.com/aws/aws-cdk/issues/27528
+    // and https://github.com/aws/aws-cdk/issues/27928)
+    const lb = this.node.defaultChild;
+    const bucketPolicy = bucket.policy?.node.defaultChild;
+    if (lb && bucketPolicy && CfnResource.isCfnResource(lb) && CfnResource.isCfnResource(bucketPolicy)) {
+      lb.addDependency(bucketPolicy);
+    }
+  }
+
   /**
    * Add a security group to this load balancer
    */

packages/aws-cdk-lib/aws-elasticloadbalancingv2/test/alb/load-balancer.test.ts

@@ -2,6 +2,7 @@ import { Construct } from 'constructs';
 import { Match, Template } from '../../../assertions';
 import { Metric } from '../../../aws-cloudwatch';
 import * as ec2 from '../../../aws-ec2';
+import { Key } from '../../../aws-kms';
 import * as s3 from '../../../aws-s3';
 import * as cdk from '../../../core';
 import * as elbv2 from '../../lib';
@@ -246,11 +247,16 @@ describe('tests', () => {
       }
     }
 
-    function loggingSetup(): { stack: cdk.Stack; bucket: s3.Bucket; lb: elbv2.ApplicationLoadBalancer } {
+    function loggingSetup(withEncryption: boolean = false ): { stack: cdk.Stack; bucket: s3.Bucket; lb: elbv2.ApplicationLoadBalancer } {
       const app = new cdk.App();
       const stack = new cdk.Stack(app, undefined, { env: { region: 'us-east-1' } });
       const vpc = new ec2.Vpc(stack, 'Stack');
-      const bucket = new s3.Bucket(stack, 'AccessLoggingBucket');
+      let bucketProps = {};
+      if (withEncryption) {
+        const kmsKey = new Key(stack, 'TestKMSKey');
+        bucketProps = { ...bucketProps, encryption: s3.BucketEncryption.KMS, encyptionKey: kmsKey };
+      }
+      const bucket = new s3.Bucket(stack, 'AccessLogBucket', { ...bucketProps });
       const lb = new elbv2.ApplicationLoadBalancer(stack, 'LB', { vpc });
       return { stack, bucket, lb };
     }
@@ -271,7 +277,7 @@ describe('tests', () => {
           },
           {
             Key: 'access_logs.s3.bucket',
-            Value: { Ref: 'AccessLoggingBucketA6D88F29' },
+            Value: { Ref: 'AccessLogBucketDA470295' },
           },
           {
             Key: 'access_logs.s3.prefix',
@@ -291,7 +297,7 @@ describe('tests', () => {
       // THEN
       // verify the ALB depends on the bucket policy
       Template.fromStack(stack).hasResource('AWS::ElasticLoadBalancingV2::LoadBalancer', {
-        DependsOn: ['AccessLoggingBucketPolicy700D7CC6'],
+        DependsOn: ['AccessLogBucketPolicyF52D2D01'],
       });
     });
 
@@ -313,7 +319,7 @@ describe('tests', () => {
               Effect: 'Allow',
               Principal: { AWS: { 'Fn::Join': ['', ['arn:', { Ref: 'AWS::Partition' }, ':iam::127311923021:root']] } },
               Resource: {
-                'Fn::Join': ['', [{ 'Fn::GetAtt': ['AccessLoggingBucketA6D88F29', 'Arn'] }, '/AWSLogs/',
+                'Fn::Join': ['', [{ 'Fn::GetAtt': ['AccessLogBucketDA470295', 'Arn'] }, '/AWSLogs/',
                   { Ref: 'AWS::AccountId' }, '/*']],
               },
             },
@@ -322,7 +328,7 @@ describe('tests', () => {
               Effect: 'Allow',
               Principal: { Service: 'delivery.logs.amazonaws.com' },
               Resource: {
-                'Fn::Join': ['', [{ 'Fn::GetAtt': ['AccessLoggingBucketA6D88F29', 'Arn'] }, '/AWSLogs/',
+                'Fn::Join': ['', [{ 'Fn::GetAtt': ['AccessLogBucketDA470295', 'Arn'] }, '/AWSLogs/',
                   { Ref: 'AWS::AccountId' }, '/*']],
               },
               Condition: { StringEquals: { 's3:x-amz-acl': 'bucket-owner-full-control' } },
@@ -332,7 +338,7 @@ describe('tests', () => {
               Effect: 'Allow',
               Principal: { Service: 'delivery.logs.amazonaws.com' },
               Resource: {
-                'Fn::GetAtt': ['AccessLoggingBucketA6D88F29', 'Arn'],
+                'Fn::GetAtt': ['AccessLogBucketDA470295', 'Arn'],
               },
             },
           ],
@@ -357,7 +363,7 @@ describe('tests', () => {
           },
           {
             Key: 'access_logs.s3.bucket',
-            Value: { Ref: 'AccessLoggingBucketA6D88F29' },
+            Value: { Ref: 'AccessLogBucketDA470295' },
           },
           {
             Key: 'access_logs.s3.prefix',
@@ -376,7 +382,7 @@ describe('tests', () => {
               Effect: 'Allow',
               Principal: { AWS: { 'Fn::Join': ['', ['arn:', { Ref: 'AWS::Partition' }, ':iam::127311923021:root']] } },
               Resource: {
-                'Fn::Join': ['', [{ 'Fn::GetAtt': ['AccessLoggingBucketA6D88F29', 'Arn'] }, '/prefix-of-access-logs/AWSLogs/',
+                'Fn::Join': ['', [{ 'Fn::GetAtt': ['AccessLogBucketDA470295', 'Arn'] }, '/prefix-of-access-logs/AWSLogs/',
                   { Ref: 'AWS::AccountId' }, '/*']],
               },
             },
@@ -385,7 +391,7 @@ describe('tests', () => {
               Effect: 'Allow',
               Principal: { Service: 'delivery.logs.amazonaws.com' },
               Resource: {
-                'Fn::Join': ['', [{ 'Fn::GetAtt': ['AccessLoggingBucketA6D88F29', 'Arn'] }, '/prefix-of-access-logs/AWSLogs/',
+                'Fn::Join': ['', [{ 'Fn::GetAtt': ['AccessLogBucketDA470295', 'Arn'] }, '/prefix-of-access-logs/AWSLogs/',
                   { Ref: 'AWS::AccountId' }, '/*']],
               },
               Condition: { StringEquals: { 's3:x-amz-acl': 'bucket-owner-full-control' } },
@@ -395,14 +401,27 @@ describe('tests', () => {
               Effect: 'Allow',
               Principal: { Service: 'delivery.logs.amazonaws.com' },
               Resource: {
-                'Fn::GetAtt': ['AccessLoggingBucketA6D88F29', 'Arn'],
+                'Fn::GetAtt': ['AccessLogBucketDA470295', 'Arn'],
               },
             },
           ],
         },
       });
     });
 
+    test('bucket with KMS throws validation error', () => {
+      //GIVEN
+      const { stack, bucket, lb } = loggingSetup(true);
+
+      // WHEN
+      const logAccessLogFunctionTest = () => lb.logAccessLogs(bucket);
+
+      // THEN
+      // verify failure in case the access log bucket is encrypted with KMS
+      expect(logAccessLogFunctionTest).toThrow('Encryption key detected. Bucket encryption using KMS keys is unsupported');
+
+    });
+
     test('access logging on imported bucket', () => {
       // GIVEN
       const { stack, lb } = loggingSetup();