fix(cli): asset uploads fail if Object Lock is enabled on access bucket#31937
fix(cli): asset uploads fail if Object Lock is enabled on access bucket#31937mergify[bot] merged 13 commits intomainfrom
Conversation
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.
aws-cdk-automation
left a comment
There was a problem hiding this comment.
The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons. If you believe this pull request should receive an exemption, please comment and provide a justification.
A comment requesting an exemption should contain the text Exemption Request. Additionally, if clarification is needed add Clarification Request to a comment.
mrgrain
left a comment
There was a problem hiding this comment.
Don't we need to fix this in cdklabs/cdk-assets?
|
|
||
| // THEN - should not fail | ||
| } finally { | ||
| // Clean up stack and bootstrap stack |
There was a problem hiding this comment.
Happens automatically. I should remove the finally.
| if (crypto.getFips() && apiRequiresMd5Checksum) { | ||
| // This should disappear for SDKv3; in SDKv3, we can always force the client to use SHA256 checksums | ||
| throw new Error('This operation requires MD5 for integrity purposes; unfortunately, it therefore not available in a FIPS environment.'); | ||
| } | ||
|
|
||
| if (crypto.getFips()) { |
There was a problem hiding this comment.
💅🏻 This might be slightly easier to undertand.
| if (crypto.getFips() && apiRequiresMd5Checksum) { | |
| // This should disappear for SDKv3; in SDKv3, we can always force the client to use SHA256 checksums | |
| throw new Error('This operation requires MD5 for integrity purposes; unfortunately, it therefore not available in a FIPS environment.'); | |
| } | |
| if (crypto.getFips()) { | |
| if (crypto.getFips()) { | |
| if (apiRequiresMd5Checksum) { | |
| // This should disappear for SDKv3; in SDKv3, we can always force the client to use SHA256 checksums | |
| throw new Error('This operation requires MD5 for integrity purposes; unfortunately, it therefore not available in a FIPS environment.'); | |
| } |
mrgrain
left a comment
There was a problem hiding this comment.
Conditionally approved. The error message needs a grammar fix and I would suggest we upgrade cdk-assets in this PR as well, just to be on the safe side.
Co-authored-by: Momo Kornher <kornherm@amazon.co.uk>
|
Test pipeline is blocked on #31946 |
|
➡️ PR build request submitted to A maintainer must now check the pipeline and add the |
✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.
|
Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
|
Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
|
Comments on closed issues and PRs are hard for our team to see. |
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