feat: check origin access control usage for cloudfront with s3 origin

.projenrc.js

@@ -6,7 +6,7 @@ const { awscdk, vscode, Task } = require('projen');
 const project = new awscdk.AwsCdkConstructLibrary({
   author: 'Arun Donti',
   authorAddress: 'donti@amazon.com',
-  cdkVersion: '2.116.0',
+  cdkVersion: '2.156.0',
   defaultReleaseBranch: 'main',
   majorVersion: 2,
   npmDistTag: 'latest',

src/packs/aws-solutions.ts

@@ -26,6 +26,7 @@ import {
   CloudFrontDistributionGeoRestrictions,
   CloudFrontDistributionHttpsViewerNoOutdatedSSL,
   CloudFrontDistributionNoOutdatedSSL,
+  CloudFrontDistributionS3OriginAccessControl,
   CloudFrontDistributionS3OriginAccessIdentity,
   CloudFrontDistributionWAFIntegration,
 } from '../rules/cloudfront';
@@ -858,13 +859,22 @@ export class AwsSolutionsChecks extends NagPack {
     });
     this.applyRule({
       ruleSuffixOverride: 'CFR6',
-      info: 'The CloudFront distribution does not use an origin access identity with an S3 origin.',
+      info: 'The CloudFront Streaming distribution does not use an origin access identity with an S3 origin.',
       explanation:
         'Origin access identities help with security by restricting any direct access to objects through S3 URLs.',
       level: NagMessageLevel.ERROR,
       rule: CloudFrontDistributionS3OriginAccessIdentity,
       node: node,
     });
+    this.applyRule({
+      ruleSuffixOverride: 'CFR7',
+      info: 'The CloudFront distribution does not use an origin access control with an S3 origin.',
+      explanation:
+        'Origin access controls help with security by restricting any direct access to objects through S3 URLs.',
+      level: NagMessageLevel.ERROR,
+      rule: CloudFrontDistributionS3OriginAccessControl,
+      node: node,
+    });
     this.applyRule({
       ruleSuffixOverride: 'APIG1',
       info: 'The API does not have access logging enabled.',

src/rules/cloudfront/CloudFrontDistributionS3OriginAccessControl.ts

@@ -0,0 +1,64 @@
+/*
+Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
+SPDX-License-Identifier: Apache-2.0
+*/
+import { parse } from 'path';
+import { CfnResource, Stack } from 'aws-cdk-lib';
+import { CfnDistribution } from 'aws-cdk-lib/aws-cloudfront';
+import { CfnBucket } from 'aws-cdk-lib/aws-s3';
+import { NagRuleCompliance, NagRules } from '../../nag-rules';
+
+/**
+ * CloudFront distributions use an origin access control for S3 origins
+ * @param node the CfnResource to check
+ */
+export default Object.defineProperty(
+  (node: CfnResource): NagRuleCompliance => {
+    if (node instanceof CfnDistribution) {
+      const distributionConfig = Stack.of(node).resolve(
+        node.distributionConfig
+      );
+      if (distributionConfig.origins != undefined) {
+        const origins = Stack.of(node).resolve(distributionConfig.origins);
+        for (const origin of origins) {
+          const resolvedOrigin = Stack.of(node).resolve(origin);
+          const resolvedDomainName = Stack.of(node).resolve(
+            resolvedOrigin.domainName
+          );
+          const originLogicalId = NagRules.resolveResourceFromInstrinsic(
+            node,
+            resolvedDomainName
+          );
+          for (const child of Stack.of(node).node.findAll()) {
+            if (child instanceof CfnBucket) {
+              const childLogicalId = NagRules.resolveResourceFromInstrinsic(
+                child,
+                child.ref
+              );
+              if (originLogicalId === childLogicalId) {
+                const resolvedAccessControlId = Stack.of(node).resolve(
+                  resolvedOrigin.originAccessControlId
+                );
+                const originAccessControlId =
+                  NagRules.resolveResourceFromInstrinsic(
+                    node,
+                    resolvedAccessControlId
+                  );
+                if (originAccessControlId == undefined) {
+                  return NagRuleCompliance.NON_COMPLIANT;
+                }
+                if (originAccessControlId.replace(/\s/g, '').length == 0) {
+                  return NagRuleCompliance.NON_COMPLIANT;
+                }
+              }
+            }
+          }
+        }
+      }
+      return NagRuleCompliance.COMPLIANT;
+    }
+    return NagRuleCompliance.NOT_APPLICABLE;
+  },
+  'name',
+  { value: parse(__filename).name }
+);

src/rules/cloudfront/CloudFrontDistributionS3OriginAccessIdentity.ts

@@ -4,49 +4,17 @@ SPDX-License-Identifier: Apache-2.0
 */
 import { parse } from 'path';
 import { CfnResource, Stack } from 'aws-cdk-lib';
-import {
-  CfnDistribution,
-  CfnStreamingDistribution,
-} from 'aws-cdk-lib/aws-cloudfront';
+import { CfnStreamingDistribution } from 'aws-cdk-lib/aws-cloudfront';
 import { NagRuleCompliance } from '../../nag-rules';
 
 /**
- * CloudFront distributions use an origin access identity for S3 origins
+ * CloudFront Streaming distributions use an origin access identity for S3 origins
+ * Only applying to CloudFront Streaming distributions because CloudFront distributions should use origin access control instead
  * @param node the CfnResource to check
  */
 export default Object.defineProperty(
   (node: CfnResource): NagRuleCompliance => {
-    if (node instanceof CfnDistribution) {
-      const distributionConfig = Stack.of(node).resolve(
-        node.distributionConfig
-      );
-      if (distributionConfig.origins != undefined) {
-        const origins = Stack.of(node).resolve(distributionConfig.origins);
-        for (const origin of origins) {
-          const resolvedOrigin = Stack.of(node).resolve(origin);
-          const resolvedDomainName = Stack.of(node).resolve(
-            resolvedOrigin.domainName
-          );
-          const s3Regex =
-            /^.+\.s3(?:-website)?(?:\..+)?(?:(?:\.amazonaws\.com(?:\.cn)?)|(?:\.c2s\.ic\.gov)|(?:\.sc2s\.sgov\.gov))$/;
-          if (s3Regex.test(resolvedDomainName)) {
-            if (resolvedOrigin.s3OriginConfig == undefined) {
-              return NagRuleCompliance.NON_COMPLIANT;
-            }
-            const resolvedConfig = Stack.of(node).resolve(
-              resolvedOrigin.s3OriginConfig
-            );
-            if (
-              resolvedConfig.originAccessIdentity == undefined ||
-              resolvedConfig.originAccessIdentity.replace(/\s/g, '').length == 0
-            ) {
-              return NagRuleCompliance.NON_COMPLIANT;
-            }
-          }
-        }
-      }
-      return NagRuleCompliance.COMPLIANT;
-    } else if (node instanceof CfnStreamingDistribution) {
+    if (node instanceof CfnStreamingDistribution) {
       const distributionConfig = Stack.of(node).resolve(
         node.streamingDistributionConfig
       );

src/rules/cloudfront/index.ts

@@ -8,3 +8,4 @@ export { default as CloudFrontDistributionHttpsViewerNoOutdatedSSL } from './Clo
 export { default as CloudFrontDistributionNoOutdatedSSL } from './CloudFrontDistributionNoOutdatedSSL';
 export { default as CloudFrontDistributionS3OriginAccessIdentity } from './CloudFrontDistributionS3OriginAccessIdentity';
 export { default as CloudFrontDistributionWAFIntegration } from './CloudFrontDistributionWAFIntegration';
+export { default as CloudFrontDistributionS3OriginAccessControl } from './CloudFrontDistributionS3OriginAccessControl';

test/rules/CloudFront.test.ts

@@ -11,7 +11,11 @@ import {
   OriginProtocolPolicy,
   OriginSslPolicy,
 } from 'aws-cdk-lib/aws-cloudfront';
-import { HttpOrigin, S3Origin } from 'aws-cdk-lib/aws-cloudfront-origins';
+import {
+  HttpOrigin,
+  S3Origin,
+  S3BucketOrigin,
+} from 'aws-cdk-lib/aws-cloudfront-origins';
 import { Bucket } from 'aws-cdk-lib/aws-s3';
 import { CfnWebACL } from 'aws-cdk-lib/aws-wafv2';
 import { Aspects, Stack } from 'aws-cdk-lib/core';
@@ -23,6 +27,7 @@ import {
   CloudFrontDistributionNoOutdatedSSL,
   CloudFrontDistributionS3OriginAccessIdentity,
   CloudFrontDistributionWAFIntegration,
+  CloudFrontDistributionS3OriginAccessControl,
 } from '../../src/rules/cloudfront';
 
 const testPack = new TestPack([
@@ -32,6 +37,7 @@ const testPack = new TestPack([
   CloudFrontDistributionNoOutdatedSSL,
   CloudFrontDistributionS3OriginAccessIdentity,
   CloudFrontDistributionWAFIntegration,
+  CloudFrontDistributionS3OriginAccessControl,
 ]);
 let stack: Stack;
 
@@ -284,10 +290,28 @@ describe('Amazon CloudFront', () => {
     });
   });
 
-  describe('CloudFrontDistributionS3OriginAccessIdentity: CloudFront distributions use an origin access identity for S3 origins', () => {
+  describe('CloudFrontDistributionS3OriginAccessIdentity: CloudFront Streaming distributions use an origin access identity for S3 origins', () => {
     const ruleId = 'CloudFrontDistributionS3OriginAccessIdentity';
-    test('Noncompliance 1', () => {
-      new CfnDistribution(stack, 'rDistribution', {
+    test('Noncompliance', () => {
+      new CfnStreamingDistribution(stack, 'rStreamingDistribution', {
+        streamingDistributionConfig: {
+          comment: 'foo',
+          enabled: true,
+          s3Origin: {
+            domainName: 'foo.s3.us-east-1.amazonaws.com',
+            originAccessIdentity: '',
+          },
+          trustedSigners: {
+            awsAccountNumbers: ['1111222233334444'],
+            enabled: true,
+          },
+        },
+        tags: [{ key: 'foo', value: 'bar' }],
+      });
+      validateStack(stack, ruleId, TestType.NON_COMPLIANCE);
+    });
+    test('Compliance', () => {
+      new CfnDistribution(stack, 'rDistribution1', {
         distributionConfig: {
           comment: 'foo',
           defaultCacheBehavior: {
@@ -307,41 +331,11 @@ describe('Amazon CloudFront', () => {
         },
         tags: [{ key: 'foo', value: 'bar' }],
       });
-      validateStack(stack, ruleId, TestType.NON_COMPLIANCE);
-    });
-    test('Noncompliance 2', () => {
-      new Distribution(stack, 'rDistribution', {
+      new Distribution(stack, 'rDistribution2', {
         defaultBehavior: {
           origin: new HttpOrigin('foo.s3-website.amazonaws.com'),
         },
       });
-      validateStack(stack, ruleId, TestType.NON_COMPLIANCE);
-    });
-    test('Noncompliance 3', () => {
-      new CfnStreamingDistribution(stack, 'rStreamingDistribution', {
-        streamingDistributionConfig: {
-          comment: 'foo',
-          enabled: true,
-          s3Origin: {
-            domainName: 'foo.s3.us-east-1.amazonaws.com',
-            originAccessIdentity: '',
-          },
-          trustedSigners: {
-            awsAccountNumbers: ['1111222233334444'],
-            enabled: true,
-          },
-        },
-        tags: [{ key: 'foo', value: 'bar' }],
-      });
-      validateStack(stack, ruleId, TestType.NON_COMPLIANCE);
-    });
-    test('Compliance', () => {
-      new Distribution(stack, 'rDistribution', {
-        defaultBehavior: {
-          origin: new S3Origin(new Bucket(stack, 'rOriginBucket')),
-        },
-      });
-
       new CfnStreamingDistribution(stack, 'rStreamingDistribution', {
         streamingDistributionConfig: {
           comment: 'foo',
@@ -397,4 +391,36 @@ describe('Amazon CloudFront', () => {
       validateStack(stack, ruleId, TestType.COMPLIANCE);
     });
   });
+
+  describe('CloudFrontDistributionS3OriginAccessControl: CloudFront distributions use an origin access control for S3 origins', () => {
+    const ruleId = 'CloudFrontDistributionS3OriginAccessControl';
+    test('Noncompliance 1', () => {
+      new Distribution(stack, 'rDistribution', {
+        defaultBehavior: {
+          origin: new S3Origin(new Bucket(stack, 'rOriginBucket')),
+        },
+      });
+      validateStack(stack, ruleId, TestType.NON_COMPLIANCE);
+    });
+    test('Noncompliance 2', () => {
+      new Distribution(stack, 'rDistribution', {
+        defaultBehavior: {
+          origin: S3BucketOrigin.withOriginAccessIdentity(
+            new Bucket(stack, 'rOriginBucket')
+          ),
+        },
+      });
+      validateStack(stack, ruleId, TestType.NON_COMPLIANCE);
+    });
+    test('Compliance', () => {
+      new Distribution(stack, 'rDistribution', {
+        defaultBehavior: {
+          origin: S3BucketOrigin.withOriginAccessControl(
+            new Bucket(stack, 'rOriginBucket')
+          ),
+        },
+      });
+      validateStack(stack, ruleId, TestType.COMPLIANCE);
+    });
+  });
 });

test/rules/QuickSight.test.ts

@@ -20,15 +20,21 @@ describe('Amazon QuickSight', () => {
     const ruleId = 'QuicksightSSLConnections';
     test('Noncompliance 1', () => {
       new CfnDataSource(stack, 'rDashboard', {
+        name: 'datasource',
+        type: 'AMAZON_ELASTICSEARCH',
         sslProperties: { disableSsl: true },
       });
       validateStack(stack, ruleId, TestType.NON_COMPLIANCE);
     });
     test('Compliance', () => {
       new CfnDataSource(stack, 'rDashboard', {
+        name: 'datasource',
+        type: 'AMAZON_ELASTICSEARCH',
         sslProperties: { disableSsl: false },
       });
       new CfnDataSource(stack, 'rDashboard2', {
+        name: 'datasource',
+        type: 'AMAZON_ELASTICSEARCH',
         sslProperties: {},
       });
       validateStack(stack, ruleId, TestType.COMPLIANCE);