New checksum header seems to break presigned URLs #1253
Comments
atwam
added
bug
github-actions
bot
added
the
potential-regression
|
Also experiencing this issue here. Looks like it started 2 weeks ago |
|
i am also running into the same issue |
|
Got a workaround here by adding the following RequestCheckSumCalculation to the client - it was previously working without this, and we didn't upgrade anything. This only occurred in our deployed app. In local testing the headers did not appear. For local testing, setting a checksum mode on the presigned builder revealed this would be a workaround. |
|
Could you share some details about how you are calling the presigned request? Are you making it against S3 or another service with an S3-like API? We have an example of this in our CI that continues to work and I was just able to successfully run both presigned GET and PUT requests to my own bucket with the following dependencies in my Cargo.toml: [dependencies]
aws-config = "1.5.16"
aws-sdk-s3 = "1.73.0"For reference my test code looks like: let presigning_config = PresigningConfig::builder()
.expires_in(std::time::Duration::from_secs(300))
.build()
.unwrap();
let get_request = client
.get_object()
.bucket("NOT_A_REAL_BUCKET")
.key("NOT_A_REAL_KEY")
.presigned(presigning_config)
.await
.unwrap();
let resp = reqwest::get(get_request.uri().to_string())
.await
.unwrap()
.text()
.await
.unwrap();
println!("PRESIGNED GET RESPONSE: {resp:#?}") |
|
We are making our request directly to s3. Looking at it on our side, and just printing the URL, I get: let client = S3Client::from_env().build().await;
let presigning_config = PresigningConfig::builder()
.expires_in(std::time::Duration::from_secs(300))
.build()
.unwrap();
let request = self
.get_object()
.bucket("PRIVATE_BUCKET")
.key("PRIVATE_KEY")
.request_payer(RequestPayer::Requester)
.presigned(presigning_config)
.await
.unwrap();
println!("Presigned URI: {}", presigned.uri());
println!("Presigned headers: {:?}", presigned.headers().collect::<Vec<_>>());My guess is that your test works because it is just using the normal URI for its GET request, and not adding any header. In our case, you can see that the presigned request has headers Changing your test to have a header before presigning (such as let resp = reqwest::get(get_request.uri().to_string()).headers(get_request.headers())will use whatever headers are included in the presigned url, and should fail like ours do. It might be worth expanding your test slightly, to include headers before presigning happens. |
|
Thanks for the info, was able to repro with that and am working on a fix |
## Motivation and Context <!--- Why is this change required? What problem does it solve? --> <!--- If it fixes an open issue, please link to the issue here --> Fixes a bug reported in awslabs/aws-sdk-rust#1253 with flexible checksum headers sneaking into presigned requests. ## Description <!--- Describe your changes in detail --> Short circuit the closure that mutates the `checksum-mode` header when we detect that we are making a presigned request. ## Testing <!--- Please describe in detail how you tested your changes --> <!--- Include details of your testing environment, and the tests you ran to --> <!--- see how your change affects other areas of the code, etc. --> Update our presigned canary tests to do both PUT and GET and have extra headers included that aren't in the query string that will expose this issue if there is a regression. ## Checklist <!--- If a checkbox below is not applicable, then please DELETE it rather than leaving it unchecked --> - [x] For changes to the AWS SDK, generated SDK code, or SDK runtime crates, I have created a changelog entry Markdown file in the `.changelog` directory, specifying "aws-sdk-rust" in the `applies_to` key. ---- _By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice._
|
Fix for this was merged in smithy-lang/smithy-rs#4000 and should deploy to crates.io early next week |
ysaito1001
added
the
pending-release
|
The fix was released as part of release-2025-02-13. Let us know if you still observe the issue after you've upgraded |
|
Comments on closed issues are hard for our team to see. |
Describe the bug
Issue
When using the presign functionality, a new
x-amx-checksum-modeheader gets added to the request after its signature, causing validation of presigned URL to fail.Regression Issue
Expected Behavior
New checksum header gets added before signing, so the signed url is correct.
Current Behavior
When using the presign functionality, a new
x-amx-checksum-modeheader gets added to the request after its signature, causing validation to fail with:Reproduction Steps
I have not extracted a fully running code, can do if necessary, but the gist of it is the following:
Trying to access the signed URL now fails with the error above, which seems to imply that its signature happens before addition of the
x-amz-checksum-modeheader.I have looked at #1246 , which also seems to be related to a late addition of the header.
Possible Solution
I think the
HttpResponseChecksumDecoratorruns after thePreSigningInterceptor.Short term solution would be to ensure that PreSigningInterceptor runs last, or alternatively have it somehow "freeze" the headers/payload, making it read-only, so that future similar regressions don't happen.
A short term workaround in our case is to set the checksum config to
WhenRequiredto prevent decoration with the new header. Another solution is to manually remove the newly-added header from the presigned request (at the risk of having again verification failure once the header is properly included in the signature). But this is a local hack-ish workaround, not a proper fix of the s3 sdk.Version
Versions used
Environment details (OS name and version, etc.)
Linux 6.8.0-51-generic-Ubuntu SMP PREEMPT_DYNAMIC x86_64 x86_64 x86_64 GNU/Linux
The text was updated successfully, but these errors were encountered: