From 8b2ca02dd661b74502ac73c14e87acd54146107c Mon Sep 17 00:00:00 2001 From: Shikha Aggarwal Date: Wed, 6 Mar 2024 11:10:11 -0800 Subject: [PATCH 1/6] fix: add validation for ALB access log bucket when KMS key is provided --- .../lib/alb/application-load-balancer.ts | 64 ++++++++++++++++++- .../test/alb/load-balancer.test.ts | 23 ++++++- 2 files changed, 84 insertions(+), 3 deletions(-) diff --git a/packages/aws-cdk-lib/aws-elasticloadbalancingv2/lib/alb/application-load-balancer.ts b/packages/aws-cdk-lib/aws-elasticloadbalancingv2/lib/alb/application-load-balancer.ts index 4ddb300b9b92f..bb5ac6ac32e91 100644 --- a/packages/aws-cdk-lib/aws-elasticloadbalancingv2/lib/alb/application-load-balancer.ts +++ b/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,65 @@ 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 */ diff --git a/packages/aws-cdk-lib/aws-elasticloadbalancingv2/test/alb/load-balancer.test.ts b/packages/aws-cdk-lib/aws-elasticloadbalancingv2/test/alb/load-balancer.test.ts index b100a161ab50e..db67a324c9641 100644 --- a/packages/aws-cdk-lib/aws-elasticloadbalancingv2/test/alb/load-balancer.test.ts +++ b/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 }; } @@ -403,6 +409,19 @@ describe('tests', () => { }); }); + test('bucket with KMS throws validation error', () => { + //GIVEN + const { 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(); From bd0ce63ad7b33cba695de1dda26158e7e1b32da7 Mon Sep 17 00:00:00 2001 From: Shikha Aggarwal Date: Thu, 7 Mar 2024 14:16:42 -0800 Subject: [PATCH 2/6] fix: add validation for ALB access log bucket when KMS key is provided --- .../lib/alb/application-load-balancer.ts | 1 + .../aws-elasticloadbalancingv2/test/alb/load-balancer.test.ts | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/aws-cdk-lib/aws-elasticloadbalancingv2/lib/alb/application-load-balancer.ts b/packages/aws-cdk-lib/aws-elasticloadbalancingv2/lib/alb/application-load-balancer.ts index bb5ac6ac32e91..8f1d0ea5a82ac 100644 --- a/packages/aws-cdk-lib/aws-elasticloadbalancingv2/lib/alb/application-load-balancer.ts +++ b/packages/aws-cdk-lib/aws-elasticloadbalancingv2/lib/alb/application-load-balancer.ts @@ -213,6 +213,7 @@ export class ApplicationLoadBalancer extends BaseLoadBalancer implements IApplic lb.addDependency(bucketPolicy); } } + /** * Add a security group to this load balancer */ diff --git a/packages/aws-cdk-lib/aws-elasticloadbalancingv2/test/alb/load-balancer.test.ts b/packages/aws-cdk-lib/aws-elasticloadbalancingv2/test/alb/load-balancer.test.ts index db67a324c9641..301bccb3efd9a 100644 --- a/packages/aws-cdk-lib/aws-elasticloadbalancingv2/test/alb/load-balancer.test.ts +++ b/packages/aws-cdk-lib/aws-elasticloadbalancingv2/test/alb/load-balancer.test.ts @@ -411,7 +411,7 @@ describe('tests', () => { test('bucket with KMS throws validation error', () => { //GIVEN - const { bucket, lb } = loggingSetup(true); + const { stack, bucket, lb } = loggingSetup(true); // WHEN const logAccessLogFunctionTest = () => lb.logAccessLogs(bucket); From aae2c2688d3a6089e8614e1ddd4bbaef3583d396 Mon Sep 17 00:00:00 2001 From: Shikha Aggarwal Date: Thu, 7 Mar 2024 15:14:08 -0800 Subject: [PATCH 3/6] fix: add validation for ALB access log bucket when KMS key is provided --- .../test/alb/load-balancer.test.ts | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/packages/aws-cdk-lib/aws-elasticloadbalancingv2/test/alb/load-balancer.test.ts b/packages/aws-cdk-lib/aws-elasticloadbalancingv2/test/alb/load-balancer.test.ts index 301bccb3efd9a..71bdb9a82f940 100644 --- a/packages/aws-cdk-lib/aws-elasticloadbalancingv2/test/alb/load-balancer.test.ts +++ b/packages/aws-cdk-lib/aws-elasticloadbalancingv2/test/alb/load-balancer.test.ts @@ -277,7 +277,7 @@ describe('tests', () => { }, { Key: 'access_logs.s3.bucket', - Value: { Ref: 'AccessLoggingBucketA6D88F29' }, + Value: { Ref: 'AccessLogBucketDA470295' }, }, { Key: 'access_logs.s3.prefix', @@ -297,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'], }); }); @@ -319,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' }, '/*']], }, }, @@ -328,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' } }, @@ -338,7 +338,7 @@ describe('tests', () => { Effect: 'Allow', Principal: { Service: 'delivery.logs.amazonaws.com' }, Resource: { - 'Fn::GetAtt': ['AccessLoggingBucketA6D88F29', 'Arn'], + 'Fn::GetAtt': ['AccessLogBucketDA470295', 'Arn'], }, }, ], @@ -363,7 +363,7 @@ describe('tests', () => { }, { Key: 'access_logs.s3.bucket', - Value: { Ref: 'AccessLoggingBucketA6D88F29' }, + Value: { Ref: 'AccessLogBucketDA470295' }, }, { Key: 'access_logs.s3.prefix', @@ -382,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' }, '/*']], }, }, @@ -391,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' } }, @@ -401,7 +401,7 @@ describe('tests', () => { Effect: 'Allow', Principal: { Service: 'delivery.logs.amazonaws.com' }, Resource: { - 'Fn::GetAtt': ['AccessLoggingBucketA6D88F29', 'Arn'], + 'Fn::GetAtt': ['AccessLogBucketDA470295', 'Arn'], }, }, ], From 69d863bd758265867c82157cdf58f551e1202b4f Mon Sep 17 00:00:00 2001 From: Shikha Aggarwal Date: Wed, 20 Mar 2024 18:47:50 -0700 Subject: [PATCH 4/6] modifying read me --- .../aws-elasticloadbalancingv2/README.md | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/packages/aws-cdk-lib/aws-elasticloadbalancingv2/README.md b/packages/aws-cdk-lib/aws-elasticloadbalancingv2/README.md index cc7d200abdf83..f4eab0e8253d2 100644 --- a/packages/aws-cdk-lib/aws-elasticloadbalancingv2/README.md +++ b/packages/aws-cdk-lib/aws-elasticloadbalancingv2/README.md @@ -225,6 +225,23 @@ const lb = new elbv2.ApplicationLoadBalancer(this, 'LB', { For more information, see [Load balancer attributes](https://docs.aws.amazon.com/elasticloadbalancing/latest/application/application-load-balancers.html#load-balancer-attributes) +### Setting up Access Log Bucket on Application Load Balancer + +The only server-side encryption option that's supported is Amazon S3-managed keys (SSE-S3). For more information +Documentation: https://docs.aws.amazon.com/elasticloadbalancing/latest/application/enable-access-logging.html + +```ts + +declare const vpc: ec2.vpc; + +declare const bucket: s3.bucket = new s3.bucket(this, 'ALBAccessLogsBucket',{ + encryption: s3.BucketEncryption.S3_MANAGED,}); + +const lb = new elbv2.ApplicationLoadBalancer(this, 'LB', { vpc }); +lb.logAccessLogs(bucket); + +``` + ## Defining a Network Load Balancer Network Load Balancers are defined in a similar way to Application Load From 645abf90aa73279e479ee4875ce86af2dffa6ed1 Mon Sep 17 00:00:00 2001 From: Shikha Aggarwal Date: Thu, 21 Mar 2024 14:37:06 -0700 Subject: [PATCH 5/6] correcting code in ReadMe --- packages/aws-cdk-lib/aws-elasticloadbalancingv2/README.md | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/aws-cdk-lib/aws-elasticloadbalancingv2/README.md b/packages/aws-cdk-lib/aws-elasticloadbalancingv2/README.md index f4eab0e8253d2..a3139c52608ff 100644 --- a/packages/aws-cdk-lib/aws-elasticloadbalancingv2/README.md +++ b/packages/aws-cdk-lib/aws-elasticloadbalancingv2/README.md @@ -232,10 +232,11 @@ Documentation: https://docs.aws.amazon.com/elasticloadbalancing/latest/applicati ```ts -declare const vpc: ec2.vpc; +declare const vpc: ec2.Vpc; -declare const bucket: s3.bucket = new s3.bucket(this, 'ALBAccessLogsBucket',{ - encryption: s3.BucketEncryption.S3_MANAGED,}); +const bucket = new s3.Bucket(this, 'ALBAccessLogsBucket',{ + encryption: s3.BucketEncryption.S3_MANAGED, + }); const lb = new elbv2.ApplicationLoadBalancer(this, 'LB', { vpc }); lb.logAccessLogs(bucket); From 3d3cdc76b756c686eb374e997e5de6ef81f50da8 Mon Sep 17 00:00:00 2001 From: Shikha Aggarwal Date: Tue, 26 Mar 2024 13:39:04 -0700 Subject: [PATCH 6/6] fix readme error by adding s3 fixture --- .../rosetta/aws_elasticloadbalancingv2/default.ts-fixture | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/aws-cdk-lib/rosetta/aws_elasticloadbalancingv2/default.ts-fixture b/packages/aws-cdk-lib/rosetta/aws_elasticloadbalancingv2/default.ts-fixture index 9f9c0bb37b6be..48fd3869082f0 100644 --- a/packages/aws-cdk-lib/rosetta/aws_elasticloadbalancingv2/default.ts-fixture +++ b/packages/aws-cdk-lib/rosetta/aws_elasticloadbalancingv2/default.ts-fixture @@ -5,6 +5,7 @@ import * as elbv2 from 'aws-cdk-lib/aws-elasticloadbalancingv2'; import * as ec2 from 'aws-cdk-lib/aws-ec2'; import * as autoscaling from 'aws-cdk-lib/aws-autoscaling'; import * as cloudwatch from 'aws-cdk-lib/aws-cloudwatch'; +import * as s3 from 'aws-cdk-lib/aws-s3'; class Fixture extends Stack { constructor(scope: Construct, id: string) {