From 370b905c0e4710ea79a295938589426bb22fa8cf Mon Sep 17 00:00:00 2001
From: Matteo Madeddu <matteo.madeddu@gmail.com>
Date: Fri, 21 Jun 2019 10:41:24 +0200
Subject: [PATCH] fix(elbv2): restrict ALB access logs bucket permissions to
 minimum (#2929)

---
 .../lib/alb/application-load-balancer.ts      |  6 ++---
 .../test/alb/test.load-balancer.ts            | 24 ++++++++++++-------
 2 files changed, 18 insertions(+), 12 deletions(-)

diff --git a/packages/@aws-cdk/aws-elasticloadbalancingv2/lib/alb/application-load-balancer.ts b/packages/@aws-cdk/aws-elasticloadbalancingv2/lib/alb/application-load-balancer.ts
index be1042c3eeaf2..3c67a0a33fba8 100644
--- a/packages/@aws-cdk/aws-elasticloadbalancingv2/lib/alb/application-load-balancer.ts
+++ b/packages/@aws-cdk/aws-elasticloadbalancingv2/lib/alb/application-load-balancer.ts
@@ -52,7 +52,7 @@ export class ApplicationLoadBalancer extends BaseLoadBalancer implements IApplic
    * Import an existing Application Load Balancer
    */
   public static fromApplicationLoadBalancerAttributes(
-      scope: Construct, id: string, attrs: ApplicationLoadBalancerAttributes): IApplicationLoadBalancer {
+    scope: Construct, id: string, attrs: ApplicationLoadBalancerAttributes): IApplicationLoadBalancer {
 
     return new ImportedApplicationLoadBalancer(scope, id, attrs);
   }
@@ -97,7 +97,7 @@ export class ApplicationLoadBalancer extends BaseLoadBalancer implements IApplic
     }
 
     prefix = prefix || '';
-    bucket.grantPut(new iam.AccountPrincipal(account), prefix + '*');
+    bucket.grantPut(new iam.AccountPrincipal(account), `${(prefix ? prefix + "/" : "")}AWSLogs/${Stack.of(this).account}/*`);
 
     // make sure the bucket's policy is created before the ALB (see https://github.com/awslabs/aws-cdk/issues/1633)
     this.node.addDependency(bucket);
@@ -519,7 +519,7 @@ export interface ApplicationLoadBalancerAttributes {
 }
 
 // https://docs.aws.amazon.com/elasticloadbalancing/latest/application/load-balancer-access-logs.html#access-logging-bucket-permissions
-const ELBV2_ACCOUNTS: {[region: string]: string } = {
+const ELBV2_ACCOUNTS: { [region: string]: string } = {
   'us-east-1': '127311923021',
   'us-east-2': '033677994240',
   'us-west-1': '027434742980',
diff --git a/packages/@aws-cdk/aws-elasticloadbalancingv2/test/alb/test.load-balancer.ts b/packages/@aws-cdk/aws-elasticloadbalancingv2/test/alb/test.load-balancer.ts
index 8569931dc0508..7f661f4d86345 100644
--- a/packages/@aws-cdk/aws-elasticloadbalancingv2/test/alb/test.load-balancer.ts
+++ b/packages/@aws-cdk/aws-elasticloadbalancingv2/test/alb/test.load-balancer.ts
@@ -110,7 +110,7 @@ export = {
 
   'Access logging'(test: Test) {
     // GIVEN
-    const stack = new cdk.Stack(undefined, undefined, { env: { region: 'us-east-1' }});
+    const stack = new cdk.Stack(undefined, undefined, { env: { region: 'us-east-1' } });
     const vpc = new ec2.Vpc(stack, 'Stack');
     const bucket = new s3.Bucket(stack, 'AccessLoggingBucket');
     const lb = new elbv2.ApplicationLoadBalancer(stack, 'LB', { vpc });
@@ -140,10 +140,13 @@ export = {
         Version: '2012-10-17',
         Statement: [
           {
-            Action: [ "s3:PutObject*", "s3:Abort*" ],
+            Action: ["s3:PutObject*", "s3:Abort*"],
             Effect: 'Allow',
-            Principal: { AWS: { "Fn::Join": [ "", [ "arn:", { Ref: "AWS::Partition" }, ":iam::127311923021:root" ] ] } },
-            Resource: { "Fn::Join": [ "", [ { "Fn::GetAtt": [ "AccessLoggingBucketA6D88F29", "Arn" ] }, "/*" ] ] }
+            Principal: { AWS: { "Fn::Join": ["", ["arn:", { Ref: "AWS::Partition" }, ":iam::127311923021:root"]] } },
+            Resource: {
+              "Fn::Join": ["", [{ "Fn::GetAtt": ["AccessLoggingBucketA6D88F29", "Arn"] }, "/AWSLogs/",
+              { Ref: "AWS::AccountId" }, "/*"]]
+            }
           }
         ]
       }
@@ -151,7 +154,7 @@ export = {
 
     // verify the ALB depends on the bucket *and* the bucket policy
     expect(stack).to(haveResource('AWS::ElasticLoadBalancingV2::LoadBalancer', {
-      DependsOn: [ 'AccessLoggingBucketPolicy700D7CC6', 'AccessLoggingBucketA6D88F29' ]
+      DependsOn: ['AccessLoggingBucketPolicy700D7CC6', 'AccessLoggingBucketA6D88F29']
     }, ResourcePart.CompleteDefinition));
 
     test.done();
@@ -159,7 +162,7 @@ export = {
 
   'access logging with prefix'(test: Test) {
     // GIVEN
-    const stack = new cdk.Stack(undefined, undefined, { env: { region: 'us-east-1' }});
+    const stack = new cdk.Stack(undefined, undefined, { env: { region: 'us-east-1' } });
     const vpc = new ec2.Vpc(stack, 'Stack');
     const bucket = new s3.Bucket(stack, 'AccessLoggingBucket');
     const lb = new elbv2.ApplicationLoadBalancer(stack, 'LB', { vpc });
@@ -192,10 +195,13 @@ export = {
         Version: '2012-10-17',
         Statement: [
           {
-            Action: [ "s3:PutObject*", "s3:Abort*" ],
+            Action: ["s3:PutObject*", "s3:Abort*"],
             Effect: 'Allow',
-            Principal: { AWS: { "Fn::Join": [ "", [ "arn:", { Ref: "AWS::Partition" }, ":iam::127311923021:root" ] ] } },
-            Resource: { "Fn::Join": [ "", [ { "Fn::GetAtt": [ "AccessLoggingBucketA6D88F29", "Arn" ] }, "/prefix-of-access-logs*" ] ] }
+            Principal: { AWS: { "Fn::Join": ["", ["arn:", { Ref: "AWS::Partition" }, ":iam::127311923021:root"]] } },
+            Resource: {
+              "Fn::Join": ["", [{ "Fn::GetAtt": ["AccessLoggingBucketA6D88F29", "Arn"] }, "/prefix-of-access-logs/AWSLogs/",
+              { Ref: "AWS::AccountId" }, "/*"]]
+            }
           }
         ]
       }