fix(bootstrap): disallow AssumeRole with ExternalId by default (#811)

By default, CDK Bootstrap roles are not designed to be deputized.

(Deputized means that you give an external entity access to assume roles
on your behalf. They will supply an ExternalId to avoid [Confused Deputy
attacks](https://docs.aws.amazon.com/IAM/latest/UserGuide/confused-deputy.html))

If a deputy system (i.e., a system that assumes IAM Roles on behalf of
its tenants) is using CDK and its policies are not configured carefully,
it can be tricked into assuming its own CDK roles.

Because CDK Roles are not intended to be used in this way, we are adding
a default security control that will make this misconfiguration less
likely: AssumeRole calls with ExternalIds will be denied by default.

What if I do want to use ExternalIds?
-------------------------------------

If you are currently passing `ExternalId`s in an `AssumeRole` call to
CDK bootstrap roles *inside your own trusted organization* (expecting
the ExternalId to be present but ignored), this protection can be
disabled by calling:

```
$ cdk bootstrap --no-deny-external-id
```

If you want to give permissions for other organizations to assume your
CDK bootstrap roles in a deputized way, customize the bootstrap template
and add a proper `ExternalId` condition.

---
By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache-2.0 license

---------

Signed-off-by: github-actions <github-actions@github.com>
Co-authored-by: Ian Hou <45278651+iankhou@users.noreply.github.com>
Co-authored-by: github-actions <github-actions@github.com>

Author: 3 people

packages/@aws-cdk-testing/cli-integ/lib/with-cdk-app.ts

@@ -312,6 +312,11 @@ export interface CdkModernBootstrapCommandOptions extends CommonCdkBootstrapComm
    */
   readonly customPermissionsBoundary?: string;
 
+  /**
+   * @default true
+   */
+  readonly denyExternalId?: boolean;
+
   /**
    * @default undefined
    */
@@ -509,6 +514,10 @@ export class TestFixture extends ShellHelper {
     } else if (options.examplePermissionsBoundary !== undefined) {
       args.push('--example-permissions-boundary');
     }
+    if (options.denyExternalId !== undefined) {
+      args.push(options.denyExternalId ? '--deny-external-id' : '--no-deny-external-id');
+    }
+
     if (options.usePreviousParameters === false) {
       args.push('--no-previous-parameters');
     }

packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/bootstrap/cdk-bootstrap-deploy-role-cannot-be-assumed-with-external-id.integtest.ts

@@ -0,0 +1,47 @@
+import { AssumeRoleCommand } from '@aws-sdk/client-sts';
+import { integTest, withoutBootstrap } from '../../../lib';
+
+jest.setTimeout(2 * 60 * 60_000); // Includes the time to acquire locks, worst-case single-threaded runtime
+
+integTest('deploy role cannot be assumed with external id', withoutBootstrap(async (fixture) => {
+  const bootstrapStackName = fixture.bootstrapStackName;
+
+  await fixture.cdkBootstrapModern({
+    toolkitStackName: bootstrapStackName,
+    cfnExecutionPolicy: 'arn:aws:iam::aws:policy/AdministratorAccess',
+  });
+
+  const account = await fixture.aws.account();
+  const deployRoleArn = `arn:aws:iam::${account}:role/cdk-${fixture.qualifier}-deploy-role-${account}-${fixture.aws.region}`;
+
+  // Attempt to assume the deploy role with an external ID should fail
+  await expect(
+    fixture.aws.sts.send(new AssumeRoleCommand({
+      RoleArn: deployRoleArn,
+      RoleSessionName: 'test-external-id-failure',
+      ExternalId: 'some-external-id',
+    })),
+  ).rejects.toThrow();
+}));
+
+integTest('deploy role can be assumed with ExternalId if protection is switched off', withoutBootstrap(async (fixture) => {
+  const bootstrapStackName = fixture.bootstrapStackName;
+
+  await fixture.cdkBootstrapModern({
+    toolkitStackName: bootstrapStackName,
+    cfnExecutionPolicy: 'arn:aws:iam::aws:policy/AdministratorAccess',
+    denyExternalId: false,
+  });
+
+  const account = await fixture.aws.account();
+  const deployRoleArn = `arn:aws:iam::${account}:role/cdk-${fixture.qualifier}-deploy-role-${account}-${fixture.aws.region}`;
+
+  // Attempt to assume the deploy role with an external ID should fail
+  await expect(
+    fixture.aws.sts.send(new AssumeRoleCommand({
+      RoleArn: deployRoleArn,
+      RoleSessionName: 'test-external-id-failure',
+      ExternalId: 'some-external-id',
+    })),
+  ).resolves.toBeTruthy();
+}));

packages/@aws-cdk/toolkit-lib/lib/api/bootstrap/bootstrap-environment.ts

@@ -213,27 +213,37 @@ export class Bootstrapper {
       }
     }
 
-    return current.update(
-      bootstrapTemplate,
-      {
-        FileAssetsBucketName: params.bucketName,
-        FileAssetsBucketKmsKeyId: kmsKeyId,
-        // Empty array becomes empty string
-        TrustedAccounts: trustedAccounts.join(','),
-        TrustedAccountsForLookup: trustedAccountsForLookup.join(','),
-        CloudFormationExecutionPolicies: cloudFormationExecutionPolicies.join(','),
-        Qualifier: params.qualifier,
-        PublicAccessBlockConfiguration:
-          params.publicAccessBlockConfiguration || params.publicAccessBlockConfiguration === undefined
-            ? 'true'
-            : 'false',
-        InputPermissionsBoundary: policyName,
-      },
-      {
-        ...options,
-        terminationProtection: options.terminationProtection ?? current.terminationProtection,
-      },
-    );
+    const bootstrapTemplateParameters: Record<string, string | undefined> = {
+      FileAssetsBucketName: params.bucketName,
+      FileAssetsBucketKmsKeyId: kmsKeyId,
+      // Empty array becomes empty string
+      TrustedAccounts: trustedAccounts.join(','),
+      TrustedAccountsForLookup: trustedAccountsForLookup.join(','),
+      CloudFormationExecutionPolicies: cloudFormationExecutionPolicies.join(','),
+      Qualifier: params.qualifier,
+      PublicAccessBlockConfiguration:
+        params.publicAccessBlockConfiguration || params.publicAccessBlockConfiguration === undefined
+          ? 'true'
+          : 'false',
+      InputPermissionsBoundary: policyName,
+    };
+
+    const templateParameters = await this.templateParameters();
+
+    // Conditionally set these parameters: only set these parameters if they are accepted by the template.
+    // If we pass them unconditionally, older customized templates that don't know about these
+    // parameters yet will fail to deploy.
+    if (params.denyExternalId !== undefined) {
+      if (!templateParameters.includes('DenyExternalId')) {
+        throw new ToolkitError('The selected bootstrap template does not accept the DenyExternalId parameter');
+      }
+      bootstrapTemplateParameters.DenyExternalId = `${params.denyExternalId}`;
+    }
+
+    return current.update(bootstrapTemplate, bootstrapTemplateParameters, {
+      ...options,
+      terminationProtection: options.terminationProtection ?? current.terminationProtection,
+    });
   }
 
   private async getPolicyName(
@@ -368,14 +378,23 @@ export class Bootstrapper {
     }
   }
 
-  private async loadTemplate(params: BootstrappingParameters = {}): Promise<any> {
+  /**
+   * Return the set of parameter names accepted by the current bootstrapping template
+   */
+  private async templateParameters(legacyParams: BootstrappingParameters = {}): Promise<string[]> {
+    const template = await this.loadTemplate(legacyParams);
+
+    return Object.keys(template.Parameters ?? {});
+  }
+
+  private async loadTemplate(legacyParams: BootstrappingParameters = {}): Promise<any> {
     switch (this.source.source) {
       case 'custom':
         return loadStructuredFile(this.source.templateFile);
       case 'default':
         return loadStructuredFile(path.join(bundledPackageRootDir(__dirname), 'lib', 'api', 'bootstrap', 'bootstrap-template.yaml'));
       case 'legacy':
-        return legacyBootstrapTemplate(params);
+        return legacyBootstrapTemplate(legacyParams);
     }
   }
 }

packages/@aws-cdk/toolkit-lib/lib/api/bootstrap/bootstrap-props.ts

@@ -146,4 +146,11 @@ export interface BootstrappingParameters {
    * @default - No value, optional argument
    */
   readonly customPermissionsBoundary?: string;
+
+  /**
+   * Whether to deny AssumeRole calls with an ExternalId
+   *
+   * @default - template default (true)
+   */
+  readonly denyExternalId?: boolean;
 }

packages/@aws-cdk/toolkit-lib/test/api/bootstrap/bootstrap2.test.ts

@@ -645,6 +645,86 @@ describe('Bootstrapping v2', () => {
     );
   });
 
+  describe('ExternalId protection', () => {
+    test('denyExternalId parameter is not present by default', async () => {
+      // GIVEN
+      const mockSdk = new MockSdkProvider();
+      (ToolkitInfo as any).lookup = jest.fn().mockResolvedValue(ToolkitInfo.fromStack(mockBootstrapStack({
+        Outputs: [
+          { OutputKey: 'BootstrapVersion', OutputValue: '1' },
+        ],
+      })));
+
+      // WHEN
+      await bootstrapper.bootstrapEnvironment(env, mockSdk, {
+        parameters: {},
+      });
+
+      // THEN
+      expect(mockDeployStack).toHaveBeenCalledWith(
+        expect.objectContaining({
+          parameters: expect.not.objectContaining({
+            DenyExternalId: 'true',
+          }),
+        }),
+        expect.anything(),
+      );
+    });
+
+    test.each([false, true])('denyExternalId parameter can be set to %p', async (param) => {
+      // GIVEN
+      const mockSdk2 = new MockSdkProvider();
+      (ToolkitInfo as any).lookup = jest.fn().mockResolvedValue(ToolkitInfo.fromStack(mockBootstrapStack({
+        Outputs: [
+          { OutputKey: 'BootstrapVersion', OutputValue: '1' },
+        ],
+      })));
+
+      // WHEN
+      await bootstrapper.bootstrapEnvironment(env, mockSdk2, {
+        parameters: {
+          denyExternalId: param,
+        },
+      });
+
+      // THEN
+      expect(mockDeployStack).toHaveBeenCalledWith(
+        expect.objectContaining({
+          parameters: expect.objectContaining({
+            DenyExternalId: `${param}`,
+          }),
+        }),
+        expect.anything(),
+      );
+    });
+
+    test('bootstrap template contains ExternalId conditions', async () => {
+      // GIVEN
+      const testBootstrapper = new Bootstrapper({ source: 'default' }, ioHelper);
+
+      // WHEN
+      const template = await (testBootstrapper as any).loadTemplate();
+
+      // THEN
+      expect(template.Parameters.DenyExternalId).toBeDefined();
+      expect(template.Parameters.DenyExternalId.Default).toBe('true');
+      expect(template.Conditions.ShouldDenyExternalId).toBeDefined();
+
+      // Check that roles have the ExternalId condition
+      const filePublishingRole = template.Resources.FilePublishingRole;
+      expect(filePublishingRole.Properties.AssumeRolePolicyDocument.Statement).toEqual(
+        expect.arrayContaining([
+          expect.objectContaining({
+            Action: 'sts:AssumeRole',
+            Condition: expect.objectContaining({
+              'Fn::If': expect.arrayContaining(['ShouldDenyExternalId']),
+            }),
+          }),
+        ]),
+      );
+    });
+  });
+
   describe('contains sts:TagSession on trusted accounts', () => {
     let template: Template;
 
@@ -659,7 +739,7 @@ describe('Bootstrapping v2', () => {
         'Fn::If': Match.arrayWith([
           conditionName,
           Match.objectLike({
-            Action: Match.arrayWith(['sts:AssumeRole', 'sts:TagSession']),
+            Action: Match.arrayWith(['sts:TagSession']),
           }),
         ]),
       });
@@ -680,23 +760,14 @@ describe('Bootstrapping v2', () => {
       template = Template.fromJSON(rawTemplate);
     });
 
-    test('in the FilePublishingRole', async () => {
-      template.hasResource('AWS::IAM::Role', {
-        Properties: {
-          RoleName: iamRoleName('file-publishing-role'),
-          AssumeRolePolicyDocument: {
-            Statement: Match.arrayWith([
-              statementWithCondition('HasTrustedAccounts'),
-            ]),
-          },
-        },
-      });
-    });
-
-    test('in the ImagePublishingRole', async () => {
+    test.each([
+      ['FilePublishingRole', 'file-publishing-role'],
+      ['ImagePublishingRole', 'image-publishing-role'],
+      ['DeploymentActionRole', 'deploy-role'],
+    ])('in the %p', async (_, roleName) => {
       template.hasResource('AWS::IAM::Role', {
         Properties: {
-          RoleName: iamRoleName('image-publishing-role'),
+          RoleName: iamRoleName(roleName),
           AssumeRolePolicyDocument: {
             Statement: Match.arrayWith([
               statementWithCondition('HasTrustedAccounts'),
@@ -719,18 +790,5 @@ describe('Bootstrapping v2', () => {
         },
       });
     });
-
-    test('in the DeploymentActionRole', async () => {
-      template.hasResource('AWS::IAM::Role', {
-        Properties: {
-          RoleName: iamRoleName('deploy-role'),
-          AssumeRolePolicyDocument: {
-            Statement: Match.arrayWith([
-              statementWithCondition('HasTrustedAccounts'),
-            ]),
-          },
-        },
-      });
-    });
   });
 });

packages/@aws-cdk/toolkit-lib/test/api/bootstrap/external-id.test.ts

@@ -0,0 +1,73 @@
+import { Bootstrapper } from '../../../lib/api/bootstrap';
+import type { IIoHost } from '../../../lib/api/io';
+import { asIoHelper } from '../../../lib/api/io/private';
+
+describe('ExternalId Protection Integration Test', () => {
+  let ioHost: IIoHost;
+  let ioHelper: any;
+
+  beforeEach(() => {
+    ioHost = {
+      notify: jest.fn(),
+      requestResponse: jest.fn(),
+    };
+    ioHelper = asIoHelper(ioHost, 'bootstrap');
+  });
+
+  test('bootstrap template denies AssumeRole with ExternalId by default', async () => {
+    // GIVEN
+    const bootstrapper = new Bootstrapper({ source: 'default' }, ioHelper);
+
+    // WHEN
+    const template = await (bootstrapper as any).loadTemplate();
+
+    // THEN
+    // Verify the parameter exists
+    expect(template.Parameters.DenyExternalId).toMatchObject({
+      Type: 'String',
+      Default: 'true',
+      AllowedValues: ['true', 'false'],
+    });
+
+    // Verify the condition exists
+    expect(template.Conditions.ShouldDenyExternalId).toEqual({
+      'Fn::Equals': ['true', { Ref: 'DenyExternalId' }],
+    });
+
+    // Verify each role has the ExternalId condition
+    const rolesToCheck = [
+      'FilePublishingRole',
+      'ImagePublishingRole',
+      'LookupRole',
+      'DeploymentActionRole',
+    ];
+
+    for (const roleName of rolesToCheck) {
+      const role = template.Resources[roleName];
+      expect(role).toBeDefined();
+
+      // Find AssumeRole statements for AWS principals (not service principals)
+      const assumeRoleStatements = role.Properties.AssumeRolePolicyDocument.Statement.filter(
+        (stmt: any) => stmt.Action === 'sts:AssumeRole' && stmt.Principal?.AWS,
+      );
+
+      // Each AssumeRole statement should have the ExternalId condition
+      for (const stmt of assumeRoleStatements) {
+        expect(stmt.Condition).toEqual({
+          'Fn::If': [
+            'ShouldDenyExternalId',
+            { Null: { 'sts:ExternalId': 'true' } },
+            { Ref: 'AWS::NoValue' },
+          ],
+        });
+      }
+    }
+
+    // Verify CloudFormationExecutionRole does NOT have the condition (it's assumed by service)
+    const cfnRole = template.Resources.CloudFormationExecutionRole;
+    const cfnStatements = cfnRole.Properties.AssumeRolePolicyDocument.Statement;
+    for (const stmt of cfnStatements) {
+      expect(stmt.Condition).toBeUndefined();
+    }
+  });
+});