Skip to content

Commit ab1e91d

Browse files
authored
fix(cli): asset uploads fail if Object Lock is enabled on access bucket (#31937)
Object Lock requires passing an object checksum. By default, SDKv2 only calculates MD5 checksums. We used to turn off checksums altogether and rely on SigV4 checksums to produce a workable setup for both FIPS and non-FIPS users, but in case of Object Lock this doesn't work: we must definitely have an S3 content checksum, and the the SigV4 checksum alone is not good enough. Since SDKv2 only supports MD5 checksums, we now only disable checksums for FIPS environments. The unfortunate result is that Object Lock will not work in a FIPS environment, but there's no way around that for now. When we migrate to SDKv3, which can be configured to checksum using SHA256, Object Lock + FIPS will work again. Relates to #31926 (This PR also adds tests for the PluginHost because otherwise the build fails due to coverage requirements) ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
1 parent 4e715b8 commit ab1e91d

File tree

8 files changed

+171
-18
lines changed

8 files changed

+171
-18
lines changed

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

+2-1
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,7 @@ export class AwsClients {
127127
}
128128
}
129129

130-
public async emptyBucket(bucketName: string) {
130+
public async emptyBucket(bucketName: string, options?: { bypassGovernance?: boolean }) {
131131
const objects = await this.s3.send(
132132
new ListObjectVersionsCommand({
133133
Bucket: bucketName,
@@ -154,6 +154,7 @@ export class AwsClients {
154154
Objects: deletes,
155155
Quiet: false,
156156
},
157+
BypassGovernanceRetention: options?.bypassGovernance ? true : undefined,
157158
}),
158159
);
159160
}

packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/cli.integtest.ts

+38
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import {
1717
PutRolePolicyCommand,
1818
} from '@aws-sdk/client-iam';
1919
import { InvokeCommand } from '@aws-sdk/client-lambda';
20+
import { PutObjectLockConfigurationCommand } from '@aws-sdk/client-s3';
2021
import { CreateTopicCommand, DeleteTopicCommand } from '@aws-sdk/client-sns';
2122
import { AssumeRoleCommand, GetCallerIdentityCommand } from '@aws-sdk/client-sts';
2223
import {
@@ -1318,6 +1319,43 @@ integTest(
13181319
}),
13191320
);
13201321

1322+
integTest('deploy stack with Lambda Asset to Object Lock-enabled asset bucket', withoutBootstrap(async (fixture) => {
1323+
// Bootstrapping with custom toolkit stack name and qualifier
1324+
const qualifier = fixture.qualifier;
1325+
const toolkitStackName = fixture.bootstrapStackName;
1326+
await fixture.cdkBootstrapModern({
1327+
verbose: true,
1328+
toolkitStackName: toolkitStackName,
1329+
qualifier: qualifier,
1330+
});
1331+
1332+
const bucketName = `cdk-${qualifier}-assets-${await fixture.aws.account()}-${fixture.aws.region}`;
1333+
await fixture.aws.s3.send(new PutObjectLockConfigurationCommand({
1334+
Bucket: bucketName,
1335+
ObjectLockConfiguration: {
1336+
ObjectLockEnabled: 'Enabled',
1337+
Rule: {
1338+
DefaultRetention: {
1339+
Days: 1,
1340+
Mode: 'GOVERNANCE',
1341+
},
1342+
},
1343+
},
1344+
}));
1345+
1346+
// Deploy a stack that definitely contains a file asset
1347+
await fixture.cdkDeploy('lambda', {
1348+
options: [
1349+
'--toolkit-stack-name', toolkitStackName,
1350+
'--context', `@aws-cdk/core:bootstrapQualifier=${qualifier}`,
1351+
],
1352+
});
1353+
1354+
// THEN - should not fail. Now clean the bucket with governance bypass: a regular delete
1355+
// operation will fail.
1356+
await fixture.aws.emptyBucket(bucketName, { bypassGovernance: true });
1357+
}));
1358+
13211359
integTest(
13221360
'cdk ls',
13231361
withDefaultFixture(async (fixture) => {

packages/@aws-cdk/integ-runner/package.json

+1-1
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@
7474
"@aws-cdk/cloud-assembly-schema": "^38.0.1",
7575
"@aws-cdk/cloudformation-diff": "0.0.0",
7676
"@aws-cdk/cx-api": "0.0.0",
77-
"cdk-assets": "^2.155.17",
77+
"cdk-assets": "^2.155.20",
7878
"@aws-cdk/aws-service-spec": "^0.1.30",
7979
"@aws-cdk/cdk-cli-wrapper": "0.0.0",
8080
"aws-cdk": "0.0.0",

packages/aws-cdk/lib/api/aws-auth/sdk.ts

+16-3
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import * as crypto from 'crypto';
12
import * as AWS from 'aws-sdk';
23
import type { ConfigurationOptions } from 'aws-sdk/lib/config-base';
34
import { debug, trace } from './_env';
@@ -188,14 +189,26 @@ export class SDK implements ISDK {
188189
}: S3ClientOptions = {}): AWS.S3 {
189190
const config = { ...this.config };
190191

191-
if (!apiRequiresMd5Checksum) {
192+
if (crypto.getFips() && apiRequiresMd5Checksum) {
193+
// This should disappear for SDKv3; in SDKv3, we can always force the client to use SHA256 checksums
194+
throw new Error('This operation requires MD5 for integrity purposes; unfortunately, it therefore is not available in FIPS enabled environments.');
195+
}
196+
197+
if (crypto.getFips()) {
192198
// In FIPS enabled environments, the MD5 algorithm is not available for use in crypto module.
193199
// However by default the S3 client is using an MD5 checksum for content integrity checking.
194200
// While this usage is technically allowed in FIPS (MD5 is only prohibited for cryptographic use),
195201
// in practice it is just easier to use an allowed checksum mechanism.
196202
// We are disabling the S3 content checksums, and are re-enabling the regular SigV4 body signing.
197-
// SigV4 uses SHA256 for their content checksum. This configuration matches the default behavior
198-
// of the AWS SDKv3 and is a safe choice for all users, except in the above APIs.
203+
// SigV4 uses SHA256 for their content checksum.
204+
//
205+
// As far as we know, this configuration will work for most APIs except:
206+
// - DeleteObjects (note the plural)
207+
// - PutObject to a bucket with Object Lock enabled.
208+
//
209+
// These APIs refuse to work without a content checksum at the S3 level (a SigV4 checksum is not
210+
// good enough). There is no way to get those to work with SHA256 in the SDKv2, but this limitation
211+
// will be alleviated once we migrate to SDKv3.
199212
config.s3DisableBodySigning = false;
200213
config.computeChecksums = false;
201214
}

packages/aws-cdk/lib/api/plugin/plugin.ts

+9-3
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,12 @@ import { error } from './_env';
55
import { ContextProviderPlugin, isContextProviderPlugin } from './context-provider-plugin';
66
import { CredentialProviderSource } from './credential-provider-source';
77

8+
export let TESTING = false;
9+
10+
export function markTesting() {
11+
TESTING = true;
12+
}
13+
814
/**
915
* The basic contract for plug-ins to adhere to::
1016
*
@@ -52,7 +58,7 @@ export class PluginHost {
5258
public readonly contextProviderPlugins: Record<string, ContextProviderPlugin> = {};
5359

5460
constructor() {
55-
if (PluginHost.instance && PluginHost.instance !== this) {
61+
if (!TESTING && PluginHost.instance && PluginHost.instance !== this) {
5662
throw new Error('New instances of PluginHost must not be built. Use PluginHost.instance instead!');
5763
}
5864
}
@@ -71,10 +77,10 @@ export class PluginHost {
7177
error(`Module ${chalk.green(moduleSpec)} is not a valid plug-in, or has an unsupported version.`);
7278
throw new Error(`Module ${moduleSpec} does not define a valid plug-in.`);
7379
}
74-
if (plugin.init) { plugin.init(PluginHost.instance); }
80+
if (plugin.init) { plugin.init(this); }
7581
} catch (e: any) {
7682
error(`Unable to load ${chalk.green(moduleSpec)}: ${e.stack}`);
77-
throw new Error(`Unable to load plug-in: ${moduleSpec}`);
83+
throw new Error(`Unable to load plug-in: ${moduleSpec}: ${e}`);
7884
}
7985

8086
function isPlugin(x: any): x is Plugin {

packages/aws-cdk/package.json

+1-1
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@
104104
"archiver": "^5.3.2",
105105
"aws-sdk": "^2.1691.0",
106106
"camelcase": "^6.3.0",
107-
"cdk-assets": "^2.155.17",
107+
"cdk-assets": "^2.155.20",
108108
"cdk-from-cfn": "^0.162.0",
109109
"chalk": "^4",
110110
"chokidar": "^3.6.0",
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
import { ContextProviderPlugin } from '../../../lib/api/plugin/context-provider-plugin';
2+
import { CredentialProviderSource } from '../../../lib/api/plugin/credential-provider-source';
3+
import { PluginHost, markTesting } from '../../../lib/api/plugin/plugin';
4+
5+
markTesting();
6+
7+
beforeEach(() => {
8+
jest.resetModules();
9+
});
10+
11+
const THE_PLUGIN = 'the-plugin';
12+
13+
test('load a plugin using the PluginHost', () => {
14+
const host = new PluginHost();
15+
16+
jest.mock(THE_PLUGIN, () => {
17+
return {
18+
version: '1',
19+
init() {
20+
},
21+
};
22+
}, { virtual: true });
23+
24+
host.load(THE_PLUGIN);
25+
});
26+
27+
test('fail to load a plugin using the PluginHost', () => {
28+
const host = new PluginHost();
29+
30+
// This is not a plugin
31+
jest.mock(THE_PLUGIN, () => {
32+
return {};
33+
}, { virtual: true });
34+
35+
expect(() => host.load(THE_PLUGIN)).toThrow(/Unable to load plug-in/);
36+
});
37+
38+
test('plugin that registers a Credential Provider', () => {
39+
const host = new PluginHost();
40+
41+
jest.mock(THE_PLUGIN, () => {
42+
return {
43+
version: '1',
44+
init(h: PluginHost) {
45+
h.registerCredentialProviderSource({
46+
canProvideCredentials() { return Promise.resolve(false); },
47+
name: 'test',
48+
isAvailable() { return Promise.resolve(false); },
49+
getProvider() { return Promise.reject('Dont call me'); },
50+
} satisfies CredentialProviderSource);
51+
52+
},
53+
};
54+
}, { virtual: true });
55+
56+
host.load(THE_PLUGIN);
57+
58+
expect(host.credentialProviderSources).toHaveLength(1);
59+
});
60+
61+
test('plugin that registers a Context Provider', () => {
62+
const host = new PluginHost();
63+
64+
jest.mock(THE_PLUGIN, () => {
65+
return {
66+
version: '1',
67+
init(h: PluginHost) {
68+
h.registerContextProviderAlpha('name', {
69+
getValue(_args: Record<string, any>) {
70+
return Promise.resolve('asdf');
71+
},
72+
} satisfies ContextProviderPlugin);
73+
},
74+
};
75+
}, { virtual: true });
76+
77+
host.load(THE_PLUGIN);
78+
79+
expect(Object.keys(host.contextProviderPlugins)).toHaveLength(1);
80+
});
81+
82+
test('plugin that registers an invalid Context Provider throws', () => {
83+
const host = new PluginHost();
84+
85+
jest.mock(THE_PLUGIN, () => {
86+
return {
87+
version: '1',
88+
init(h: PluginHost) {
89+
h.registerContextProviderAlpha('name', {} as any);
90+
},
91+
};
92+
}, { virtual: true });
93+
94+
expect(() => host.load(THE_PLUGIN)).toThrow(/does not look like a ContextProviderPlugin/);
95+
});

yarn.lock

+9-9
Original file line numberDiff line numberDiff line change
@@ -79,10 +79,10 @@
7979
jsonschema "^1.4.1"
8080
semver "^7.6.3"
8181

82-
"@aws-cdk/cx-api@^2.163.1":
83-
version "2.163.1"
84-
resolved "https://registry.npmjs.org/@aws-cdk/cx-api/-/cx-api-2.163.1.tgz#ef55da9f471c963d877b23d3201ca4560d656b2e"
85-
integrity sha512-0bVL/pX0UcliCdXVcgtLVL3W5EHAp4RgW7JN3prz1dIOmLZzZ30DW0qWSc0D0EVE3rVG6RVgfIiuFBFK6WFZ+w==
82+
"@aws-cdk/cx-api@^2.164.1":
83+
version "2.164.1"
84+
resolved "https://registry.npmjs.org/@aws-cdk/cx-api/-/cx-api-2.164.1.tgz#dce8eaede6b9ec95c4a69f7acbe486b499c32516"
85+
integrity sha512-VwYDcI8b5KYS2VptkIAm75yK1SwLAClFnlyH0Ea5dI3YJrIYtvxW930nhppxmwPihbMJa4Z0sxic7EBTt4ZaBQ==
8686
dependencies:
8787
semver "^7.6.3"
8888

@@ -6646,13 +6646,13 @@ [email protected], case@^1.6.3:
66466646
resolved "https://registry.npmjs.org/case/-/case-1.6.3.tgz#0a4386e3e9825351ca2e6216c60467ff5f1ea1c9"
66476647
integrity sha512-mzDSXIPaFwVDvZAHqZ9VlbyF4yyXRuX6IvB06WvPYkqJVO24kX1PPhv9bfpKNFZyxYFmmgo03HUiD8iklmJYRQ==
66486648

6649-
cdk-assets@^2.155.17:
6650-
version "2.155.17"
6651-
resolved "https://registry.npmjs.org/cdk-assets/-/cdk-assets-2.155.17.tgz#d6c285d0279aec8226b45577a151e6dd32a12fa5"
6652-
integrity sha512-+hJlYYlsPHhPCeMC/V3pMyrjz5K8p9SQdC50qMg6a8/w/3w0WY1ZixyKGtpJfFB11C3Ubb04l2miieaAH00CIA==
6649+
cdk-assets@^2.155.20:
6650+
version "2.155.20"
6651+
resolved "https://registry.npmjs.org/cdk-assets/-/cdk-assets-2.155.20.tgz#a7a380f820001d2087d0dce802eac4c71a688100"
6652+
integrity sha512-NXU7RCJsPecQbRVkQ6iPyOV3jDEojENaxWs9956pYddY5Pq0onSibXItivavQC74i0YZdyWDdlH6RcLPzFQhPQ==
66536653
dependencies:
66546654
"@aws-cdk/cloud-assembly-schema" "^38.0.1"
6655-
"@aws-cdk/cx-api" "^2.163.1"
6655+
"@aws-cdk/cx-api" "^2.164.1"
66566656
archiver "^5.3.2"
66576657
aws-sdk "^2.1691.0"
66586658
glob "^7.2.3"

0 commit comments

Comments
 (0)