fix(elbv2): restrict ALB access logs bucket permissions to minimum (#…

…2929)
aws · Jun 21, 2019 · 370b905 · 370b905
1 parent 58a80ab
commit 370b905
Show file tree

Hide file tree

Showing 2 changed files with 18 additions and 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
@@ -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
@@ -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,26 +140,29 @@ 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" }, "/*"]]
+            }
           }
         ]
       }
     }));
 
     // 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();
   },
 
   '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" }, "/*"]]
+            }
           }
         ]
       }