From 2cc24499bf62b6dd48ae9bd265c38a6847f75a3f Mon Sep 17 00:00:00 2001 From: shikha372 Date: Tue, 26 Mar 2024 21:09:43 -0700 Subject: [PATCH] fix: add validation for ALB access log bucket when KMS key is provided (#29382) ### Issue # (if applicable) Closes #22031. ### Reason for this change Adds a validation with correct error indicating ALB Access log bucket does not support KMS encryption ### Description of changes Currently access logs bucket encryption with KMS is not supported in case of ALB but while deploying it throws an error indicating the failure with bucket permissions. This validation introduces an upfront check to throw an error if `bucket.encryptionKey `is defined. Documentation: https://docs.aws.amazon.com/elasticloadbalancing/latest/application/enable-access-logging.html ### Description of how you validated changes Added unit tests for validation. ### Checklist - [ ] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md) ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- .../aws-elasticloadbalancingv2/README.md | 18 +++++ .../lib/alb/application-load-balancer.ts | 65 ++++++++++++++++++- .../test/alb/load-balancer.test.ts | 41 ++++++++---- .../default.ts-fixture | 1 + 4 files changed, 113 insertions(+), 12 deletions(-) diff --git a/packages/aws-cdk-lib/aws-elasticloadbalancingv2/README.md b/packages/aws-cdk-lib/aws-elasticloadbalancingv2/README.md index 7d3494daedbe3..401eb132b4840 100644 --- a/packages/aws-cdk-lib/aws-elasticloadbalancingv2/README.md +++ b/packages/aws-cdk-lib/aws-elasticloadbalancingv2/README.md @@ -234,6 +234,24 @@ 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; + +const 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 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 bb6f2cfbefb8d..4d7bbd7c38c4a 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'; @@ -170,6 +173,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 */ 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 0bc9f98a775c0..b9fd54bd7ce95 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'; @@ -284,11 +285,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 }; } @@ -309,7 +315,7 @@ describe('tests', () => { }, { Key: 'access_logs.s3.bucket', - Value: { Ref: 'AccessLoggingBucketA6D88F29' }, + Value: { Ref: 'AccessLogBucketDA470295' }, }, { Key: 'access_logs.s3.prefix', @@ -329,7 +335,7 @@ describe('tests', () => { // THEN // verify the ALB depends on the bucket policy Template.fromStack(stack).hasResource('AWS::ElasticLoadBalancingV2::LoadBalancer', { - DependsOn: ['AccessLoggingBucketPolicy700D7CC6'], + DependsOn: ['AccessLogBucketPolicyF52D2D01'], }); }); @@ -351,7 +357,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' }, '/*']], }, }, @@ -360,7 +366,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' } }, @@ -370,7 +376,7 @@ describe('tests', () => { Effect: 'Allow', Principal: { Service: 'delivery.logs.amazonaws.com' }, Resource: { - 'Fn::GetAtt': ['AccessLoggingBucketA6D88F29', 'Arn'], + 'Fn::GetAtt': ['AccessLogBucketDA470295', 'Arn'], }, }, ], @@ -395,7 +401,7 @@ describe('tests', () => { }, { Key: 'access_logs.s3.bucket', - Value: { Ref: 'AccessLoggingBucketA6D88F29' }, + Value: { Ref: 'AccessLogBucketDA470295' }, }, { Key: 'access_logs.s3.prefix', @@ -414,7 +420,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' }, '/*']], }, }, @@ -423,7 +429,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' } }, @@ -433,7 +439,7 @@ describe('tests', () => { Effect: 'Allow', Principal: { Service: 'delivery.logs.amazonaws.com' }, Resource: { - 'Fn::GetAtt': ['AccessLoggingBucketA6D88F29', 'Arn'], + 'Fn::GetAtt': ['AccessLogBucketDA470295', 'Arn'], }, }, ], @@ -441,6 +447,19 @@ describe('tests', () => { }); }); + 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(); 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) {