Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 14 commits into from
Mar 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions packages/aws-cdk-lib/aws-elasticloadbalancingv2/README.md
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
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
shikha372 marked this conversation as resolved.
Show resolved Hide resolved
*/

if (bucket.encryptionKey) {
throw new Error('Encryption key detected. Bucket encryption using KMS keys is unsupported');
}
GavinZZ marked this conversation as resolved.
Show resolved Hide resolved

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
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
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
Loading