Skip to content

Commit

Permalink
fix: add validation for ALB access log bucket when KMS key is provided (
Browse files Browse the repository at this point in the history 
#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*
  • Loading branch information
@shikha372
shikha372 committed Mar 27, 2024
1 parent 9a34664 commit 2cc2449
Show file tree
Hide file tree
Showing 4 changed files with 113 additions and 12 deletions.
18 changes: 18 additions & 0 deletions packages/aws-cdk-lib/aws-elasticloadbalancingv2/README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
65 changes: 64 additions & 1 deletion packages/aws-cdk-lib/aws-elasticloadbalancingv2/lib/alb/application-load-balancer.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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
*/
Expand Down
41 changes: 30 additions & 11 deletions packages/aws-cdk-lib/aws-elasticloadbalancingv2/test/alb/load-balancer.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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 };
}
Expand All @@ -309,7 +315,7 @@ describe('tests', () => {
},
{
Key: 'access_logs.s3.bucket',
Value: { Ref: 'AccessLoggingBucketA6D88F29' },
Value: { Ref: 'AccessLogBucketDA470295' },
},
{
Key: 'access_logs.s3.prefix',
Expand All @@ -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'],
});
});

Expand All @@ -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' }, '/*']],
},
},
Expand All @@ -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' } },
Expand All @@ -370,7 +376,7 @@ describe('tests', () => {
Effect: 'Allow',
Principal: { Service: 'delivery.logs.amazonaws.com' },
Resource: {
'Fn::GetAtt': ['AccessLoggingBucketA6D88F29', 'Arn'],
'Fn::GetAtt': ['AccessLogBucketDA470295', 'Arn'],
},
},
],
Expand All @@ -395,7 +401,7 @@ describe('tests', () => {
},
{
Key: 'access_logs.s3.bucket',
Value: { Ref: 'AccessLoggingBucketA6D88F29' },
Value: { Ref: 'AccessLogBucketDA470295' },
},
{
Key: 'access_logs.s3.prefix',
Expand All @@ -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' }, '/*']],
},
},
Expand All @@ -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' } },
Expand All @@ -433,14 +439,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();
Expand Down
1 change: 1 addition & 0 deletions packages/aws-cdk-lib/rosetta/aws_elasticloadbalancingv2/default.ts-fixture
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down

0 comments on commit 2cc2449

Please sign in to comment.