-
Notifications
You must be signed in to change notification settings - Fork 3.9k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
feat: disallow cross account asset publishing in some scenarios (#31623)
### Issue # (if applicable) Closes #<issue number here>. ### Reason for this change ### Description of changes ### Description of how you validated changes ### Checklist - [ ] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md) ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
- Loading branch information
Showing
9 changed files
with
259 additions
and
34 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,74 @@ | ||
| import { debug } from '../../logging'; | ||
| import { ISDK } from '../aws-auth/sdk'; | ||
|
|
||
| export async function determineAllowCrossAccountAssetPublishing(sdk: ISDK, customStackName?: string): Promise<boolean> { | ||
| try { | ||
| const stackName = customStackName || 'CDKToolkit'; | ||
| const stackInfo = await getBootstrapStackInfo(sdk, stackName); | ||
|
|
||
| if (!stackInfo.hasStagingBucket) { | ||
| // indicates an intentional cross account setup | ||
| return true; | ||
| } | ||
|
|
||
| if (stackInfo.bootstrapVersion >= 21) { | ||
| // bootstrap stack version 21 contains a fix that will prevent cross | ||
| // account publishing on the IAM level | ||
| // https://github.com/aws/aws-cdk/pull/30823 | ||
| return true; | ||
| } | ||
|
|
||
| // other scenarios are highly irregular and potentially dangerous so we prevent it by | ||
| // instructing cdk-assets to detect foreign bucket ownership and reject. | ||
| return false; | ||
| } catch (e) { | ||
| debug(`Error determining cross account asset publishing: ${e}`); | ||
| debug('Defaulting to disallowing cross account asset publishing'); | ||
| return false; | ||
| } | ||
| } | ||
|
|
||
| interface BootstrapStackInfo { | ||
| hasStagingBucket: boolean; | ||
| bootstrapVersion: number; | ||
| } | ||
|
|
||
| export async function getBootstrapStackInfo(sdk: ISDK, stackName: string): Promise<BootstrapStackInfo> { | ||
| try { | ||
| const cfn = sdk.cloudFormation(); | ||
| const stackResponse = await cfn.describeStacks({ StackName: stackName }).promise(); | ||
|
|
||
| if (!stackResponse.Stacks || stackResponse.Stacks.length === 0) { | ||
| throw new Error(`Toolkit stack ${stackName} not found`); | ||
| } | ||
|
|
||
| const stack = stackResponse.Stacks[0]; | ||
| const versionOutput = stack.Outputs?.find(output => output.OutputKey === 'BootstrapVersion'); | ||
|
|
||
| if (!versionOutput?.OutputValue) { | ||
| throw new Error(`Unable to find BootstrapVersion output in the toolkit stack ${stackName}`); | ||
| } | ||
|
|
||
| const bootstrapVersion = parseInt(versionOutput.OutputValue); | ||
| if (isNaN(bootstrapVersion)) { | ||
| throw new Error(`Invalid BootstrapVersion value: ${versionOutput.OutputValue}`); | ||
| } | ||
|
|
||
| // try to get bucketname from the logical resource id | ||
| let bucketName: string | undefined; | ||
| const resourcesResponse = await cfn.describeStackResources({ StackName: stackName }).promise(); | ||
| const bucketResource = resourcesResponse.StackResources?.find(resource => | ||
| resource.ResourceType === 'AWS::S3::Bucket', | ||
| ); | ||
| bucketName = bucketResource?.PhysicalResourceId; | ||
|
|
||
| let hasStagingBucket = !!bucketName; | ||
|
|
||
| return { | ||
| hasStagingBucket, | ||
| bootstrapVersion, | ||
| }; | ||
| } catch (e) { | ||
| throw new Error(`Error retrieving toolkit stack info: ${e}`); | ||
| } | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,123 @@ | ||
| import * as AWS from 'aws-sdk'; | ||
| import * as AWSMock from 'aws-sdk-mock'; | ||
| import { ISDK } from '../../../lib/api/aws-auth'; | ||
| import { determineAllowCrossAccountAssetPublishing, getBootstrapStackInfo } from '../../../lib/api/util/checks'; | ||
|
|
||
| describe('determineAllowCrossAccountAssetPublishing', () => { | ||
| let mockSDK: ISDK; | ||
|
|
||
| beforeEach(() => { | ||
| mockSDK = { | ||
| cloudFormation: () => new AWS.CloudFormation(), | ||
| } as ISDK; | ||
| }); | ||
|
|
||
| afterEach(() => { | ||
| AWSMock.restore(); | ||
| }); | ||
|
|
||
| it('should return true when hasStagingBucket is false', async () => { | ||
| AWSMock.mock('CloudFormation', 'describeStacks', (_params: any, callback: Function) => { | ||
| callback(null, { | ||
| Stacks: [{ | ||
| Outputs: [{ OutputKey: 'BootstrapVersion', OutputValue: '1' }], | ||
| }], | ||
| }); | ||
| }); | ||
|
|
||
| AWSMock.mock('CloudFormation', 'describeStackResources', (_params: any, callback: Function) => { | ||
| callback(null, { StackResources: [] }); | ||
| }); | ||
|
|
||
| const result = await determineAllowCrossAccountAssetPublishing(mockSDK); | ||
| expect(result).toBe(true); | ||
| }); | ||
|
|
||
| it('should return true when bootstrap version is >= 21', async () => { | ||
| AWSMock.mock('CloudFormation', 'describeStacks', (_params: any, callback: Function) => { | ||
| callback(null, { | ||
| Stacks: [{ | ||
| Outputs: [{ OutputKey: 'BootstrapVersion', OutputValue: '21' }], | ||
| }], | ||
| }); | ||
| }); | ||
|
|
||
| AWSMock.mock('CloudFormation', 'describeStackResources', (_params: any, callback: Function) => { | ||
| callback(null, { StackResources: [{ ResourceType: 'AWS::S3::Bucket', PhysicalResourceId: 'some-bucket' }] }); | ||
| }); | ||
|
|
||
| const result = await determineAllowCrossAccountAssetPublishing(mockSDK); | ||
| expect(result).toBe(true); | ||
| }); | ||
|
|
||
| it('should return false for other scenarios', async () => { | ||
| AWSMock.mock('CloudFormation', 'describeStacks', (_params: any, callback: Function) => { | ||
| callback(null, { | ||
| Stacks: [{ | ||
| Outputs: [{ OutputKey: 'BootstrapVersion', OutputValue: '20' }], | ||
| }], | ||
| }); | ||
| }); | ||
|
|
||
| AWSMock.mock('CloudFormation', 'describeStackResources', (_params: any, callback: Function) => { | ||
| callback(null, { StackResources: [{ ResourceType: 'AWS::S3::Bucket', PhysicalResourceId: 'some-bucket' }] }); | ||
| }); | ||
|
|
||
| const result = await determineAllowCrossAccountAssetPublishing(mockSDK); | ||
| expect(result).toBe(false); | ||
| }); | ||
| }); | ||
|
|
||
| describe('getBootstrapStackInfo', () => { | ||
| let mockSDK: ISDK; | ||
|
|
||
| beforeEach(() => { | ||
| mockSDK = { | ||
| cloudFormation: () => new AWS.CloudFormation(), | ||
| } as ISDK; | ||
| }); | ||
|
|
||
| afterEach(() => { | ||
| AWSMock.restore(); | ||
| }); | ||
|
|
||
| it('should return correct BootstrapStackInfo', async () => { | ||
| AWSMock.mock('CloudFormation', 'describeStacks', (_params: any, callback: Function) => { | ||
| callback(null, { | ||
| Stacks: [{ | ||
| Outputs: [{ OutputKey: 'BootstrapVersion', OutputValue: '21' }], | ||
| }], | ||
| }); | ||
| }); | ||
|
|
||
| AWSMock.mock('CloudFormation', 'describeStackResources', (_params: any, callback: Function) => { | ||
| callback(null, { StackResources: [{ ResourceType: 'AWS::S3::Bucket', PhysicalResourceId: 'some-bucket' }] }); | ||
| }); | ||
|
|
||
| const result = await getBootstrapStackInfo(mockSDK, 'CDKToolkit'); | ||
| expect(result).toEqual({ | ||
| hasStagingBucket: true, | ||
| bootstrapVersion: 21, | ||
| }); | ||
| }); | ||
|
|
||
| it('should throw error when stack is not found', async () => { | ||
| AWSMock.mock('CloudFormation', 'describeStacks', (_params: any, callback: Function) => { | ||
| callback(null, { Stacks: [] }); | ||
| }); | ||
|
|
||
| await expect(getBootstrapStackInfo(mockSDK, 'CDKToolkit')).rejects.toThrow('Toolkit stack CDKToolkit not found'); | ||
| }); | ||
|
|
||
| it('should throw error when BootstrapVersion output is missing', async () => { | ||
| AWSMock.mock('CloudFormation', 'describeStacks', (_params: any, callback: Function) => { | ||
| callback(null, { | ||
| Stacks: [{ | ||
| Outputs: [], | ||
| }], | ||
| }); | ||
| }); | ||
|
|
||
| await expect(getBootstrapStackInfo(mockSDK, 'CDKToolkit')).rejects.toThrow('Unable to find BootstrapVersion output in the toolkit stack CDKToolkit'); | ||
| }); | ||
| }); |
Oops, something went wrong.