chore(logging): Add Accelerator IAM role validation and add accelerat…

…or roles to central bucket policy
awslabs · Sep 19, 2023 · 4cff4bf · 4cff4bf
1 parent 1b019ac
commit 4cff4bf
Show file tree

Hide file tree

Showing 6 changed files with 97 additions and 3 deletions.
diff --git a/source/packages/@aws-accelerator/accelerator/lib/config-validator.ts b/source/packages/@aws-accelerator/accelerator/lib/config-validator.ts
@@ -269,11 +269,12 @@ function runValidators(
   }
 
   // IAM config validator
-  if (accountsConfig && iamConfig && networkConfig && organizationConfig && securityConfig) {
+  if (accountsConfig && globalConfig && iamConfig && networkConfig && organizationConfig && securityConfig) {
     try {
       new IamConfigValidator(
         iamConfig,
         accountsConfig,
+        globalConfig,
         networkConfig,
         organizationConfig,
         securityConfig,

diff --git a/source/packages/@aws-accelerator/accelerator/lib/stacks/logging-stack.ts b/source/packages/@aws-accelerator/accelerator/lib/stacks/logging-stack.ts
@@ -1943,6 +1943,7 @@ export class LoggingStack extends AcceleratorStack {
         crossAccountAccessRoleName:
           this.acceleratorResourceNames.roles.crossAccountCentralLogBucketCmkArnSsmParameterAccess,
         cmkArnSsmParameterName: this.acceleratorResourceNames.parameters.centralLogBucketCmkArn,
+        managementAccountAccessRole: this.props.globalConfig.managementAccountAccessRole,
       });
 
       // AwsSolutions-IAM5: The IAM entity contains wildcard permissions and does not have a cdk_nag rule suppression with evidence for those permission.

diff --git a/source/packages/@aws-accelerator/config/validator/iam-config-validator.ts b/source/packages/@aws-accelerator/config/validator/iam-config-validator.ts
@@ -18,6 +18,7 @@ import { createLogger } from '@aws-accelerator/utils';
 import { AccountsConfig } from '../lib/accounts-config';
 import { CommonValidatorFunctions } from './common/common-validator-functions';
 import * as t from '../lib/common-types';
+import { GlobalConfig } from '../lib/global-config';
 import { IamConfig, IamConfigTypes } from '../lib/iam-config';
 import { NetworkConfig } from '../lib/network-config';
 import { OrganizationConfig } from '../lib/organization-config';
@@ -38,6 +39,7 @@ export class IamConfigValidator {
   constructor(
     values: IamConfig,
     accountsConfig: AccountsConfig,
+    globalConfig: GlobalConfig,
     networkConfig: NetworkConfig,
     organizationConfig: OrganizationConfig,
     securityConfig: SecurityConfig,
@@ -48,6 +50,7 @@ export class IamConfigValidator {
 
     const errors: string[] = [];
     const logger = createLogger(['iam-config-validator']);
+    const acceleratorPrefix = process.env['ACCELERATOR_PREFIX'] ?? 'AWSAccelerator';
 
     logger.info(`${IamConfig.FILENAME} file validation started`);
 
@@ -78,7 +81,7 @@ export class IamConfigValidator {
     //
     // Validate IAM roles
     //
-    this.validateRoles(values, errors);
+    this.validateRoles(values, globalConfig, acceleratorPrefix, errors);
 
     // Validate target OU names
     this.validateDeploymentTargetOUs(values, ouIdNames, errors);
@@ -224,11 +227,14 @@ export class IamConfigValidator {
    * @param values
    * @param errors
    */
-  private validateRoles(values: IamConfig, errors: string[]) {
+  private validateRoles(values: IamConfig, global: GlobalConfig, acceleratorPrefix: string, errors: string[]) {
     //
     // Validate role names
     //
     this.validateRoleNames(values, errors);
+
+    //Validate that accelerator named IAM roles are not created through the accelerator
+    this.validateForAcceleratorRoleNames(values, global, acceleratorPrefix, errors);
   }
 
   /**
@@ -251,6 +257,31 @@ export class IamConfigValidator {
     }
   }
 
+  /**
+   * Checks role names for accelerator naming conventions
+   * @param values
+   * @param global
+   * @param errors
+   */
+  private validateForAcceleratorRoleNames(
+    values: IamConfig,
+    global: GlobalConfig,
+    acceleratorPrefix: string,
+    errors: string[],
+  ) {
+    const reservedRoleNamePatterns = [global.managementAccountAccessRole, 'cdk-accel', acceleratorPrefix];
+
+    values.roleSets?.forEach(roleSet => {
+      roleSet.roles?.forEach(role => {
+        if (reservedRoleNamePatterns.some(name => role.name.startsWith(name))) {
+          errors.push(
+            `The IAM Role: ${role.name} is using a reserved naming convention. Please change the name of this role.`,
+          );
+        }
+      });
+    });
+  }
+
   /**
    * Function to validate Identity Center object
    * @param values

diff --git a/source/packages/@aws-accelerator/constructs/lib/aws-s3/central-logs-bucket.ts b/source/packages/@aws-accelerator/constructs/lib/aws-s3/central-logs-bucket.ts
@@ -14,6 +14,7 @@
 import * as cdk from 'aws-cdk-lib';
 import { Construct } from 'constructs';
 import { Bucket, BucketEncryptionType } from '@aws-accelerator/constructs';
+import { GlobalConfig } from '@aws-accelerator/config';
 import { S3LifeCycleRule } from './bucket';
 import { BucketPrefixProps } from './bucket-prefix';
 import { AwsPrincipalAccessesType, BucketAccessType } from '@aws-accelerator/utils';
@@ -36,6 +37,7 @@ export interface CentralLogsBucketProps {
    */
   awsPrincipalAccesses?: AwsPrincipalAccessesType[];
   bucketPrefixProps?: BucketPrefixProps;
+  globalConfig?: GlobalConfig;
   /**
    * Accelerator Prefix
    */
@@ -48,6 +50,10 @@ export interface CentralLogsBucketProps {
    * Accelerator central log bucket cmk arn ssm parameter name
    */
   readonly cmkArnSsmParameterName: string;
+  /**
+   * Accelerator management account access role.
+   */
+  readonly managementAccountAccessRole: string;
 }
 
 /**
@@ -189,6 +195,13 @@ export class CentralLogsBucket extends Construct {
               StringEquals: {
                 ...props.principalOrgIdCondition,
               },
+              ArnLike: {
+                'aws:PrincipalARN': [
+                  `arn:${cdk.Stack.of(this).partition}:iam::*:role/${props.acceleratorPrefix}-*`,
+                  `arn:${cdk.Stack.of(this).partition}:iam::*:role/cdk-accel-*`,
+                  `arn:${cdk.Stack.of(this).partition}:iam::*:role/${props.managementAccountAccessRole}-*`,
+                ],
+              },
             },
           }),
         );
@@ -222,6 +235,13 @@ export class CentralLogsBucket extends Construct {
           StringEquals: {
             ...props.principalOrgIdCondition,
           },
+          ArnLike: {
+            'aws:PrincipalARN': [
+              `arn:${cdk.Stack.of(this).partition}:iam::*:role/${props.acceleratorPrefix}-*`,
+              `arn:${cdk.Stack.of(this).partition}:iam::*:role/cdk-accel-*`,
+              `arn:${cdk.Stack.of(this).partition}:iam::*:role/${props.managementAccountAccessRole}-*`,
+            ],
+          },
         },
       }),
     );

diff --git a/...s/@aws-accelerator/constructs/test/aws-s3/__snapshots__/central-logs-buckets.test.ts.snap b/...s/@aws-accelerator/constructs/test/aws-s3/__snapshots__/central-logs-buckets.test.ts.snap
@@ -611,6 +611,46 @@ exports[`CentralLogsBucket Construct(CentralLogsBucket):  Snapshot Test 1`] = `
                 "s3:ListBucket",
               ],
               "Condition": {
+                "ArnLike": {
+                  "aws:PrincipalARN": [
+                    {
+                      "Fn::Join": [
+                        "",
+                        [
+                          "arn:",
+                          {
+                            "Ref": "AWS::Partition",
+                          },
+                          ":iam::*:role/AWSAccelerator-*",
+                        ],
+                      ],
+                    },
+                    {
+                      "Fn::Join": [
+                        "",
+                        [
+                          "arn:",
+                          {
+                            "Ref": "AWS::Partition",
+                          },
+                          ":iam::*:role/cdk-accel-*",
+                        ],
+                      ],
+                    },
+                    {
+                      "Fn::Join": [
+                        "",
+                        [
+                          "arn:",
+                          {
+                            "Ref": "AWS::Partition",
+                          },
+                          ":iam::*:role/AWSControlTowerExecution-*",
+                        ],
+                      ],
+                    },
+                  ],
+                },
                 "StringEquals": {
                   "aws:PrincipalOrgID": "acceleratorOrg",
                 },

diff --git a/source/packages/@aws-accelerator/constructs/test/aws-s3/central-logs-buckets.test.ts b/source/packages/@aws-accelerator/constructs/test/aws-s3/central-logs-buckets.test.ts
@@ -36,6 +36,7 @@ new CentralLogsBucket(stack, 'CentralLogsBucket', {
   acceleratorPrefix: 'AWSAccelerator',
   crossAccountAccessRoleName: 'AWSAccelerator-CentralBucket-KeyArnParam-Role',
   cmkArnSsmParameterName: '/accelerator/logging/central-bucket/kms/arn',
+  managementAccountAccessRole: 'AWSControlTowerExecution',
 });
 
 /**