chore(bootstrap): split file/image publishing roles

For security purposes, we decided that it would be lower risk to assume a different role when we publish S3 assets and when we publish ECR assets. The reason is that ECR publishers execute `docker build` which can potentially execute 3rd party code (via a base docker image). This change modifies the conventional name for the publishing roles as well as adds a set of properties to the `DefaultStackSynthesizer` to allow customization as needed.
aws · Jun 2, 2020 · 8510987 · 8510987
1 parent e257dc8
commit 8510987
Show file tree

Hide file tree

Showing 3 changed files with 142 additions and 25 deletions.
diff --git a/packages/@aws-cdk/core/lib/stack-synthesizers/default-synthesizer.ts b/packages/@aws-cdk/core/lib/stack-synthesizers/default-synthesizer.ts
@@ -44,24 +44,44 @@ export interface DefaultStackSynthesizerProps {
   readonly imageAssetsRepositoryName?: string;
 
   /**
-   * The role to use to publish assets to this environment
+   * The role to use to publish file assets to the S3 bucket in this environment
    *
    * You must supply this if you have given a non-standard name to the publishing role.
    *
    * The placeholders `${Qualifier}`, `${AWS::AccountId}` and `${AWS::Region}` will
    * be replaced with the values of qualifier and the stack's account and region,
    * respectively.
    *
-   * @default DefaultStackSynthesizer.DEFAULT_ASSET_PUBLISHING_ROLE_ARN
+   * @default DefaultStackSynthesizer.DEFAULT_FILE_ASSET_PUBLISHING_ROLE_ARN
    */
-  readonly assetPublishingRoleArn?: string;
+  readonly fileAssetPublishingRoleArn?: string;
 
   /**
-   * External ID to use when assuming role for asset publishing
+   * External ID to use when assuming role for file asset publishing
    *
    * @default - No external ID
    */
-  readonly assetPublishingExternalId?: string;
+  readonly fileAssetPublishingExternalId?: string;
+
+  /**
+   * The role to use to publish image assets to the ECR repository in this environment
+   *
+   * You must supply this if you have given a non-standard name to the publishing role.
+   *
+   * The placeholders `${Qualifier}`, `${AWS::AccountId}` and `${AWS::Region}` will
+   * be replaced with the values of qualifier and the stack's account and region,
+   * respectively.
+   *
+   * @default DefaultStackSynthesizer.DEFAULT_IMAGE_ASSET_PUBLISHING_ROLE_ARN
+   */
+  readonly imageAssetPublishingRoleArn?: string;
+
+  /**
+   * External ID to use when assuming role for image asset publishing
+   *
+   * @default - No external ID
+   */
+  readonly imageAssetPublishingExternalId?: string;
 
   /**
    * The role to assume to initiate a deployment in this environment
@@ -126,9 +146,14 @@ export class DefaultStackSynthesizer implements IStackSynthesizer {
   public static readonly DEFAULT_DEPLOY_ROLE_ARN = 'arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-${Qualifier}-deploy-role-${AWS::AccountId}-${AWS::Region}';
 
   /**
-   * Default asset publishing role ARN.
+   * Default asset publishing role ARN for file (S3) assets.
+   */
+  public static readonly DEFAULT_FILE_ASSET_PUBLISHING_ROLE_ARN = 'arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-${Qualifier}-file-publishing-role-${AWS::AccountId}-${AWS::Region}';
+
+  /**
+   * Default asset publishing role ARN for image (ECR) assets.
    */
-  public static readonly DEFAULT_ASSET_PUBLISHING_ROLE_ARN = 'arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-${Qualifier}-publishing-role-${AWS::AccountId}-${AWS::Region}';
+  public static readonly DEFAULT_IMAGE_ASSET_PUBLISHING_ROLE_ARN = 'arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-${Qualifier}-image-publishing-role-${AWS::AccountId}-${AWS::Region}';
 
   /**
    * Default image assets repository name
@@ -145,7 +170,8 @@ export class DefaultStackSynthesizer implements IStackSynthesizer {
   private repositoryName?: string;
   private _deployRoleArn?: string;
   private _cloudFormationExecutionRoleArn?: string;
-  private assetPublishingRoleArn?: string;
+  private fileAssetPublishingRoleArn?: string;
+  private imageAssetPublishingRoleArn?: string;
 
   private readonly files: NonNullable<asset_schema.ManifestFile['files']> = {};
   private readonly dockerImages: NonNullable<asset_schema.ManifestFile['dockerImages']> = {};
@@ -178,7 +204,8 @@ export class DefaultStackSynthesizer implements IStackSynthesizer {
     this.repositoryName = specialize(this.props.imageAssetsRepositoryName ?? DefaultStackSynthesizer.DEFAULT_IMAGE_ASSETS_REPOSITORY_NAME);
     this._deployRoleArn = specialize(this.props.deployRoleArn ?? DefaultStackSynthesizer.DEFAULT_DEPLOY_ROLE_ARN);
     this._cloudFormationExecutionRoleArn = specialize(this.props.cloudFormationExecutionRole ?? DefaultStackSynthesizer.DEFAULT_CLOUDFORMATION_ROLE_ARN);
-    this.assetPublishingRoleArn = specialize(this.props.assetPublishingRoleArn ?? DefaultStackSynthesizer.DEFAULT_ASSET_PUBLISHING_ROLE_ARN);
+    this.fileAssetPublishingRoleArn = specialize(this.props.fileAssetPublishingRoleArn ?? DefaultStackSynthesizer.DEFAULT_FILE_ASSET_PUBLISHING_ROLE_ARN);
+    this.imageAssetPublishingRoleArn = specialize(this.props.imageAssetPublishingRoleArn ?? DefaultStackSynthesizer.DEFAULT_IMAGE_ASSET_PUBLISHING_ROLE_ARN);
     // tslint:enable:max-line-length
   }
 
@@ -199,8 +226,8 @@ export class DefaultStackSynthesizer implements IStackSynthesizer {
           bucketName: this.bucketName,
           objectKey,
           region: resolvedOr(this.stack.region, undefined),
-          assumeRoleArn: this.assetPublishingRoleArn,
-          assumeRoleExternalId: this.props.assetPublishingExternalId,
+          assumeRoleArn: this.fileAssetPublishingRoleArn,
+          assumeRoleExternalId: this.props.fileAssetPublishingExternalId,
         },
       },
     };
@@ -237,8 +264,8 @@ export class DefaultStackSynthesizer implements IStackSynthesizer {
           repositoryName: this.repositoryName,
           imageTag,
           region: resolvedOr(this.stack.region, undefined),
-          assumeRoleArn: this.assetPublishingRoleArn,
-          assumeRoleExternalId: this.props.assetPublishingExternalId,
+          assumeRoleArn: this.imageAssetPublishingRoleArn,
+          assumeRoleExternalId: this.props.imageAssetPublishingExternalId,
         },
       },
     };

diff --git a/packages/@aws-cdk/core/test/stack-synthesis/test.new-style-synthesis.ts b/packages/@aws-cdk/core/test/stack-synthesis/test.new-style-synthesis.ts
@@ -2,7 +2,7 @@ import * as asset_schema from '@aws-cdk/cdk-assets-schema';
 import * as cxapi from '@aws-cdk/cx-api';
 import * as fs from 'fs';
 import { Test } from 'nodeunit';
-import { App, CfnResource, FileAssetPackaging, Stack } from '../../lib';
+import { App, CfnResource, DefaultStackSynthesizer, FileAssetPackaging, Stack } from '../../lib';
 import { evaluateCFN } from '../evaluate-cfn';
 
 const CFN_CONTEXT = {
@@ -50,7 +50,7 @@ export = {
         'current_account-current_region': {
           bucketName: 'cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}',
           objectKey: '4bdae6e3b1b15f08c889d6c9133f24731ee14827a9a9ab9b6b6a9b42b6d34910',
-          assumeRoleArn: 'arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-publishing-role-${AWS::AccountId}-${AWS::Region}',
+          assumeRoleArn: 'arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-file-publishing-role-${AWS::AccountId}-${AWS::Region}',
         },
       },
     });
@@ -106,22 +106,75 @@ export = {
     const asm = app.synth();
 
     // THEN - we have an asset manifest with both assets and the stack template in there
-    const manifestArtifact = asm.artifacts.filter(isAssetManifest)[0];
-    test.ok(manifestArtifact);
-    const manifest: asset_schema.ManifestFile = JSON.parse(fs.readFileSync(manifestArtifact.file, { encoding: 'utf-8' }));
+    const manifest = readAssetManifest(asm);
 
     test.equals(Object.keys(manifest.files || {}).length, 2);
     test.equals(Object.keys(manifest.dockerImages || {}).length, 1);
 
     // THEN - every artifact has an assumeRoleArn
-    for (const file of Object.values({...manifest.files, ...manifest.dockerImages})) {
+    for (const file of Object.values(manifest.files ?? {})) {
+      for (const destination of Object.values(file.destinations)) {
+        test.deepEqual(destination.assumeRoleArn, 'arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-file-publishing-role-${AWS::AccountId}-${AWS::Region}');
+      }
+    }
+
+    for (const file of Object.values(manifest.dockerImages ?? {})) {
       for (const destination of Object.values(file.destinations)) {
-        test.ok(destination.assumeRoleArn);
+        test.deepEqual(destination.assumeRoleArn, 'arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-image-publishing-role-${AWS::AccountId}-${AWS::Region}');
       }
     }
 
     test.done();
   },
+
+  'customize publishing resources'(test: Test) {
+    // GIVEN
+    const myapp = new App();
+
+    // WHEN
+    const mystack = new Stack(myapp, 'mystack', {
+      synthesizer: new DefaultStackSynthesizer({
+        fileAssetsBucketName: 'file-asset-bucket',
+        fileAssetPublishingRoleArn: 'file:role:arn',
+        fileAssetPublishingExternalId: 'file-external-id',
+
+        imageAssetsRepositoryName: 'image-ecr-repository',
+        imageAssetPublishingRoleArn: 'image:role:arn',
+        imageAssetPublishingExternalId: 'image-external-id',
+      }),
+    });
+
+    mystack.synthesizer.addFileAsset({
+      fileName: __filename,
+      packaging: FileAssetPackaging.FILE,
+      sourceHash: 'file-asset-hash',
+    });
+
+    mystack.synthesizer.addDockerImageAsset({
+      directoryName: '.',
+      sourceHash: 'docker-asset-hash',
+    });
+
+    // THEN
+    const asm = myapp.synth();
+    const manifest = readAssetManifest(asm);
+
+    test.deepEqual(manifest.files?.['file-asset-hash']?.destinations?.['current_account-current_region'], {
+      bucketName: 'file-asset-bucket',
+      objectKey: 'file-asset-hash',
+      assumeRoleArn: 'file:role:arn',
+      assumeRoleExternalId: 'file-external-id',
+    });
+
+    test.deepEqual(manifest.dockerImages?.['docker-asset-hash']?.destinations?.['current_account-current_region'] , {
+      repositoryName: 'image-ecr-repository',
+      imageTag: 'docker-asset-hash',
+      assumeRoleArn: 'image:role:arn',
+      assumeRoleExternalId: 'image-external-id',
+    });
+
+    test.done();
+  },
 };
 
 /**
@@ -135,4 +188,11 @@ function evalCFN(value: any) {
 
 function isAssetManifest(x: cxapi.CloudArtifact): x is cxapi.AssetManifestArtifact {
   return x instanceof cxapi.AssetManifestArtifact;
+}
+
+function readAssetManifest(asm: cxapi.CloudAssembly): asset_schema.ManifestFile {
+  const manifestArtifact = asm.artifacts.filter(isAssetManifest)[0];
+  if (!manifestArtifact) { throw new Error('no asset manifest in assembly'); }
+
+  return JSON.parse(fs.readFileSync(manifestArtifact.file, { encoding: 'utf-8' }));
 }
diff --git a/packages/aws-cdk/lib/api/bootstrap/bootstrap-template.yaml b/packages/aws-cdk/lib/api/bootstrap/bootstrap-template.yaml
@@ -158,7 +158,7 @@ Resources:
           - HasCustomContainerAssetsRepositoryName
           - Fn::Sub: "${ContainerAssetsRepositoryName}"
           - Fn::Sub: cdk-${Qualifier}-container-assets-${AWS::AccountId}-${AWS::Region}
-  PublishingRole:
+  FilePublishingRole:
     Type: AWS::IAM::Role
     Properties:
       AssumeRolePolicyDocument:
@@ -177,8 +177,28 @@ Resources:
                     Ref: TrustedAccounts
               - Ref: AWS::NoValue
       RoleName:
-        Fn::Sub: cdk-${Qualifier}-publishing-role-${AWS::AccountId}-${AWS::Region}
-  PublishingRoleDefaultPolicy:
+        Fn::Sub: cdk-${Qualifier}-file-publishing-role-${AWS::AccountId}-${AWS::Region}
+  ImagePublishingRole:
+    Type: AWS::IAM::Role
+    Properties:
+      AssumeRolePolicyDocument:
+        Statement:
+          - Action: sts:AssumeRole
+            Effect: Allow
+            Principal:
+              AWS:
+                Ref: AWS::AccountId
+          - Fn::If:
+              - HasTrustedAccounts
+              - Action: sts:AssumeRole
+                Effect: Allow
+                Principal:
+                  AWS:
+                    Ref: TrustedAccounts
+              - Ref: AWS::NoValue
+      RoleName:
+        Fn::Sub: cdk-${Qualifier}-image-publishing-role-${AWS::AccountId}-${AWS::Region}
+  FilePublishingRoleDefaultPolicy:
     Type: AWS::IAM::Policy
     Properties:
       PolicyDocument:
@@ -206,6 +226,16 @@ Resources:
                 - CreateNewKey
                 - Fn::Sub: "${FileAssetsBucketEncryptionKey.Arn}"
                 - Fn::Sub: arn:${AWS::Partition}:kms:${AWS::Region}:${AWS::AccountId}:key/${FileAssetsBucketKmsKeyId}
+        Version: '2012-10-17'
+      Roles:
+        - Ref: FilePublishingRole
+      PolicyName:
+        Fn::Sub: cdk-${Qualifier}-file-publishing-role-default-policy-${AWS::AccountId}-${AWS::Region}
+  ImagePublishingRoleDefaultPolicy:
+    Type: AWS::IAM::Policy
+    Properties:
+      PolicyDocument:
+        Statement:
           - Action:
               - ecr:PutImage
               - ecr:InitiateLayerUpload
@@ -223,9 +253,9 @@ Resources:
             Effect: Allow
         Version: '2012-10-17'
       Roles:
-        - Ref: PublishingRole
+        - Ref: ImagePublishingRole
       PolicyName:
-        Fn::Sub: cdk-${Qualifier}-publishing-role-default-policy-${AWS::AccountId}-${AWS::Region}
+        Fn::Sub: cdk-${Qualifier}-image-publishing-role-default-policy-${AWS::AccountId}-${AWS::Region}
   DeploymentActionRole:
     Type: AWS::IAM::Role
     Properties: