Skip to content

Commit

Permalink
fix: force SHA256 checksums
Browse files Browse the repository at this point in the history
By default, the S3 client will use MD5 checksums. But in FIPS
environments, those MD5 checksums will not work because Node will not
have access to an M5 digest.

Force the usage of SHA256 for these checksums instead.
  • Loading branch information
rix0rrr committed Oct 29, 2024
1 parent 3047036 commit 03f2455
Show file tree
Hide file tree
Showing 4 changed files with 63 additions and 16 deletions.
30 changes: 16 additions & 14 deletions lib/private/handlers/files.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import * as mime from 'mime';
import { destinationToClientOptions } from '.';
import { FileManifestEntry } from '../../asset-manifest';
import { IS3Client } from '../../aws';
import { PutObjectCommandInput } from '../../aws-types';
import { EventType } from '../../progress';
import { zipDirectory } from '../archive';
import { IAssetHandler, IHandlerHost, type PublishOptions } from '../asset-handler';
Expand Down Expand Up @@ -88,22 +89,22 @@ export class FileAssetHandler implements IAssetHandler {
case BucketOwnership.SOMEONE_ELSES_AND_HAVE_ACCESS:
if (!allowCrossAccount) {
throw new Error(
`❗❗ UNEXPECTED BUCKET OWNER DETECTED ❗❗
We've detected that the S3 bucket ${destination.bucketName} was
originally created in account ${await account()} as part of the CloudFormation stack CDKToolkit,
but now resides in a different AWS account. To prevent cross-account asset bucket access of your
`❗❗ UNEXPECTED BUCKET OWNER DETECTED ❗❗
We've detected that the S3 bucket ${destination.bucketName} was
originally created in account ${await account()} as part of the CloudFormation stack CDKToolkit,
but now resides in a different AWS account. To prevent cross-account asset bucket access of your
deployments, CDK will stop now.
If this situation is intentional and you own the AWS account that the bucket has moved to, remove the
resource named StagingBucket from the template of CloudFormation stack CDKToolkit and try again.
If this situation is not intentional, we strongly recommend auditing your account to make sure all
resources are configured the way you expect them [1]. For questions or concerns, please contact
AWS Support [2].
If this situation is intentional and you own the AWS account that the bucket has moved to, remove the
resource named StagingBucket from the template of CloudFormation stack CDKToolkit and try again.
If this situation is not intentional, we strongly recommend auditing your account to make sure all
resources are configured the way you expect them [1]. For questions or concerns, please contact
AWS Support [2].
[1] https://repost.aws/knowledge-center/potential-account-compromise
[2] https://aws.amazon.com/support`
);
}
Expand Down Expand Up @@ -162,7 +163,8 @@ export class FileAssetHandler implements IAssetHandler {
Key: destination.objectKey,
Body: createReadStream(publishFile.packagedPath),
ContentType: publishFile.contentType,
},
ChecksumAlgorithm: 'SHA256',
} satisfies PutObjectCommandInput,
paramsEncryption
);

Expand Down
12 changes: 12 additions & 0 deletions scripts/manual-test-manifest.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
{
"version": "38.0.1",
"files": {
"asset1": {
"type": "file",
"source": { "path": "/Users/huijbers/Downloads/HEY-arm64.dmg" },
"destinations": {
"dest1": { "bucketName": "huijbers-test-objectlock", "objectKey": "hey.dmg" }
}
}
}
}
22 changes: 22 additions & 0 deletions scripts/manual-test.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
#!/bin/bash
set -eu
scriptdir=$(cd $(dirname $0) && pwd)

path=$(realpath $1)

cat <<EOF > $scriptdir/manual-test-manifest.json
{
"version": "38.0.1",
"files": {
"asset1": {
"type": "file",
"source": { "path": "$path" },
"destinations": {
"dest1": { "bucketName": "$2", "objectKey": "$3" }
}
}
}
}
EOF

npx ts-node $scriptdir/../bin/cdk-assets.ts -v -p $scriptdir/manual-test-manifest.json publish
15 changes: 13 additions & 2 deletions test/files.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
import { mockClient } from 'aws-sdk-client-mock';

jest.mock('child_process');

import 'aws-sdk-client-mock-jest';
Expand All @@ -11,6 +9,7 @@ import {
PutObjectCommand,
S3Client,
} from '@aws-sdk/client-s3';
import { mockClient } from 'aws-sdk-client-mock';
import { FakeListener } from './fake-listener';
import { MockAws, mockS3 } from './mock-aws';
import { mockSpawn } from './mock-child_process';
Expand Down Expand Up @@ -191,6 +190,18 @@ test('upload with server side encryption AES256 header', async () => {
});
});

test('will use SHA256 content checksum', async () => {
const s3 = mockClient(S3Client);
s3.on(ListObjectsV2Command).resolves({ Contents: [{ Key: 'some_key.but_not_the_one' }] });

const pub = new AssetPublishing(AssetManifest.fromPath(mockfs.path('/types/cdk.out')), { aws });
await pub.publish();

expect(s3).toHaveReceivedCommandWith(PutObjectCommand, {
ChecksumAlgorithm: 'SHA256',
});
});

test('upload with server side encryption aws:kms header and key id', async () => {
const s3 = mockClient(S3Client);
s3.on(GetBucketEncryptionCommand).resolves({
Expand Down

0 comments on commit 03f2455

Please sign in to comment.