Closed samjarman closed 6 months ago
Hi @samjarman, thanks for opening this issue. Are you providing the hash in the headers when executing the request?. In your case since you are using postman, are you including the header "x-amz-checksum-sha256" with the hash as value when sending the request?
Thanks!
No, it's appended to the url as a query parameter. Has something changed there? Do we need to put the same info in the headers now too?
Hi @samjarman,
Presigned urls do not have headers associated with them (they are a url string), this means that in order to get signed headers, they are hoisted into the query string.
The SDKs got a shared feature request to implement flexible checksums for s3 uploads. Unfortunately that cross SDK feature request did not have specifications for presigned URLs so there is no support server-side for reading checksum headers from the query string. We have informed the s3 team about this a while ago and it seems like it only recently got prioritized.
Right now there is no way to provide a checksum with a presigned url. The service will return a 200 but will silently ignore the checksum validation.
I'll see if I can find out more internally.
Thanks, Ran~
P68355453
@RanVaknin thanks for your detailed reply! Has something changed server side recently? I looked up the dates this was working/stopped working for us and it was around 31st Jan (working) and not working again on the 2nd of Feb. For my own sanity, I found recording of a meeting internally where I tested that checksum checking was kicking in, and it was indeed working on the 31st of Jan with a query string alone, no checkum related headers set.
Also, just confirming:
Right now there is no way to provide a checksum with a presigned url. The service will return a 200 but will silently ignore the checksum validation.
So the JS SDK accepts the checksum in the PutObjectCommandInput but as of now, the server ignores it. And any fix would be server side?
@samjarman, I want to mention that this just happens when using pre-signed urls, but if you do a normal putObject operation with the SDK the checksum validation will take place.
Thanks!
@yenfryherrerafeliz All good. This is all about presigned urls, not PutObject operations (as per the issue) :)
Hi @samjarman,
Thanks for the follow up question.
Regular presigned URL headers are hoisted and are respected server-side. I was referring specifically to the hoisted checksum header not having support service side.
Let us know if you have any other questions. Thanks, Ran~
Hi @samjarman ,
I had to read your response a few times, I think I might have misunderstood you initially.
I'm not sure how you got it to work. This is indeed odd. I know the S3 team is working on a fix, they might have been experimenting with a solution but I highly doubt that it went live if its not fully baked yet. We don't really have visibility to service's work so all I can do is speculate in that direction.
Another theory I have is that you used the v2 SDK? If Im not mistaken in Go v1 SDK we construct the presigned urls a bit differently than the newer Go v2 SDK resulting in a different presigned URL with a checksum that does work.
The reason for the different behavior is a result of efforts from the SDK team + S3 to standardize checksum behavior across all of the newer SDKs.
Can you find some more information from your recording? like request and response logs? I can take another look at your code later this week and see if I can find out more.
Thanks, Ran~
Hey @RanVaknin sorry for the confusion.
I went back and checked the file I uploaded in a demo I gave and the file in S3 says "Additional checksums: Off". I don't have the request IDs sorry. If the Etag would help let me know where I can securely send it to.
It was the V3 SDK, too. (Delightful SDKs compared to V2 imho, nice work all there).
No logs sadly, we don't have server logging on the bucket I used at the time.
Here's a screenshot of the recording I had And I managed to dig up the original (now expired) url, with some redactions: REDACTED
https://our-bucket.s3.eu-west-1.amazonaws.com/PATH_PREFIX/FILE_NAME.zip?X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Content-Sha256=UNSIGNED-PAYLOAD&X-Amz-Credential=ASIASR3PP3MCCV7GB6TM%2F20230131%2Feu-west-1%2Fs3%2Faws4_request&X-Amz-Date=20230131T024659Z&X-Amz-Expires=300&X-Amz-Security-Token=IQoJb3JpZ2luX2VjECMaCWV1LXdlc3QtMSJIMEYCIQCPIK%2FquU%2B9JkKHfFidbX5gkKQSVzvB6kBBGpmfzPZPzgIhAKsGd4ltvsq%2BSxn7u0TArRekZVomDe%2FswFe7h4H9MKVFKoEDCJz%2F%2F%2F%2F%2F%2F%2F%2F%2F%2FwEQABoMMTc1NzkwNjExMjA0IgwCvUcDy2N6GDE3ll0q1QKD3TUsTQpxSDHkn5kHTHTFqkEysWeO%2FCqNLThPVvwwy3ibQ5JhkrDN7fTE%2BueAx6qVkYOJR3sLBC1Abg7ofZu5vAlJ4QjHmD9AwR%2FWbbDUBOK%2BPgIa%2BxcdmWwFcRJDESfanBQ12rVYOZJlD3vTaUUZ%2BatnCruXE5rXN1aJenLrSXnGKOfxHGHalei1h%2F8jdsKhnhVPPjpRQtkCCfJLvmfrKP9QH1NmvWE8mFDlAXWFoANFSjTKdkFYs7HPoVxL19ME6qp%2BXBlyaQMGmpgN%2FPS772WT9n3wE5htDqlY64MPf6n6cBdEQ7Tn9%2F3apwHCp3CN5P9Nfitx4%2FeB9pcVQZCPPypoZ%2Bn%2FMuuZSLp8Oe0bRjTPVAZM2sNUlrfxIGlPbZCy%2FSmPEqr2%2FTWbCcAwIYuuVDV5Az1e%2FcBAUeJGO4hWfEFe8Gni%2BbE3gBIc1lWWMqnihdij%2BzCR%2F%2BGeBjqdAa5mVcHVRXWwRXJeqXNORufdCd4D5PTC5GEkR2zZmd%2FghTV0uoTgalcUUIgKFrwU6rGd6bb%2FO5wCaBOsM8%2B4V3L2PPoF2qsM%2BZdODSMIBVWPBTzwkgfmdoX3kXHpXOcm%2Bf8vnEWEwsRTOvUvpMkHB7lJRSHTRfBlNcI0kKqjEWvewIiETrsuuR54tpLj3s%2B9XWNo8h4BNsf8OPAnpak%3D&X-Amz-Signature=f3752ecb73cbee9bf95279c637814a6f22c76ec7f2bb185214d122a810834f42&X-Amz-SignedHeaders=host&x-amz-checksum-sha256=aK387%204DYuhodn%20vKRJg8JxszSURmF61kuVyKeeNV%20U%3D&x-amz-meta-userid=dbaee777-e365-5ea9-91a9-cf46fa942fdb&x-amz-sdk-checksum-algorithm=SHA256&x-id=PutObject
Maybe there are some clues in there for you?
--
So to clarify, the code we're doing above won't enforce a checksum check when PUT'ing the object, never has, anything we saw was a mistake on our part or due to the wrong HTTP verb (you get a similar error when trying to GET).
So finally, is there a way to achieve what we're after - hand out urls to be used by a client where they upload an object that MUST match a certain checksum? We like the added security we get from such a feature.
Hi @samjarman ,
I had to remove that image you added because it had some sensitive info.
So to clarify, the code we're doing above won't enforce a checksum check when PUT'ing the object, never has, anything we saw was a mistake on our part or due to the wrong HTTP verb (you get a similar error when trying to GET).
It was probably a mistake. I have run my own implementation of presigned url with checksum enabled and this is definitely a false positive.
So finally, is there a way to achieve what we're after
I'm not aware of a way you can achieve what you are asking at this moment. We recognize that this is a very useful feature and also a pretty serious issue that the s3 service gives a false positive with regard to data integrity.
Like I mentioned, although this issue was known a while ago, this has only been recently prioritized by the S3 team, and aside from the tracking status, we don't really have visibility to how far they are into the fix.
Keep an eye on this thread for updates. Apologies for the inconveniences. Ran~
Thanks @RanVaknin we'll let what we saw be a mystery of the universe. Shall we keep this issue open as a request for this feature?
I had to remove that image you added because it had some sensitive info.
@RanVaknin FYI, just editing the post is insufficient, because anyone can look at the edit history. You will specifically need to delete the older versions. https://docs.github.com/en/communities/moderating-comments-and-conversations/tracking-changes-in-a-comment#deleting-sensitive-information-from-a-comments-history
@rittneje wow! today I learned.
Thanks for the tip! Ran~
@samjarman , Yes definitely we will keep this open.
I heard from the SDK's Manager that is working with s3 that there is a doc in place and an item on the roadmap to fix this. Still no dates (we can't share them even if we did have them) but I really hope that this is going to get fixed in the near future.
Thanks, Ran~
Hi @RanVaknin, I'm encountering the same issue with presigned urls for put_object
and upload_part
generated from boto3. I did not see any recent announcement related to this in S3 announcement board. Any update on the fix? Thanks!
Hi @RanVaknin, seconding @dnlby96's request for an update on "SDK's Manager that is working with s3 that there is a doc in place and an item on the roadmap to fix this." Thanks.
Right now there is no way to provide a checksum with a presigned url. The service will return a 200 but will silently ignore the checksum validation.
The fact that you can PUT files to S3 without metadata like ContentType and CacheControl is worrying, but silently ignoring the checksum validation is terrible.
Allowing specification of this in the presigned query parameters then ignoring validation of them server side is very bad practice, and I haven't seen any documentation explaining this beyond this thread either.
I would expect that s3 either implement the validation or return an error when the params are provided explaining that it is not supported and document the limitation.
@RanVaknin Any updates on the plan to address this? Does the validation currently work for presigned POST requests or is it broken for all presigned URLs? Also is there even a way to enable the additional checksums for presigned urls? When I specify the flag in the parameters for the presigned url generation and it appears in the presigned url, it seems to just be ignored as well with the additional checksums set to off.
If the ability to send eg; SHA256 for a PUT with a presigned URL is added in s3 proper-- would there be anything else required to enable multipart uploads with SHA256 checksums for the individual parts?
(we use boto3 but I assume the solution will be roughly the same)
@RanVaknin @ashishdhingra any update on this issue for the golang sdk v2 ??
I was able to get this to work correctly, i.e. fail for bad checksums, and enable "Additional Checksums" for successfully uploaded objects, by setting DisableHeaderHoisting
on the presigner.
I'd at the least recommend updating the Go doc strings to:
a) Clearly state that Checksums will be ignored in presigned urls by default. Perhaps this documentation should be on the PutObjectInput
checksum fields, as well as the PresignPutObject
docs. This is a security issue.
b) Explain that the solution is to set DisableHeaderHoisting
on the presigner.
There's details on the DisableHeaderHoisting
solution on this duped ticket.
And I put an example on Github. See here.
I would appreciate another set on eyes on this workaround. So shout out if this works for you, or if there are additional concerns I haven't thought of.
Short snippet (see the links above for more details):
checksumBytes := sha256.Sum256(body)
checksum := base64.StdEncoding.EncodeToString(checksumBytes[:])
withPresigner := func(opt *s3.PresignOptions) {
opt.Presigner = v4.NewSigner(func(so *v4.SignerOptions) {
// I copied the Default settings.
o := s3c.Options()
so.Logger = o.Logger
so.LogSigning = o.ClientLogMode.IsSigning()
so.DisableURIPathEscaping = true
// This is the magic sauce which makes SHA256 checksums work.
so.DisableHeaderHoisting = true
})
}
psc := s3.NewPresignClient(s3c)
in := &s3.PutObjectInput{
Bucket: aws.String(bucket),
Key: aws.String(key),
ChecksumAlgorithm: types.ChecksumAlgorithmSha256,
ChecksumSHA256: aws.String(checksum),
// Try it with an incorrect checksum to make sure it fails.
//ChecksumSHA256: aws.String("bad+nDnO0vTnour8G+5YPPx4tLMvxgS0K3ZBusogZxU="),
ContentLength: aws.Int64(int64(len(body))),
}
req, err := psc.PresignPutObject(ctx, in, s3.WithPresignExpires(time.Hour), withPresigner)
Output from a request with a bad checksum:
400 Bad Request: <Error><Code>BadDigest</Code><Message>The SHA256 you specified did not match the calculated checksum. ...
Service team has product feature request for this issue in their backlog. We can't share specific timelines on when this might be implemented in GitHub; you should monitor the AWS News Blog for updates. Closing this issue since the feature is already in service team backlog.
This issue is now closed.
Comments on closed issues are hard for our team to see. If you need more assistance, please either tag a team member or open a new issue that references this one. If you wish to keep having a conversation with other community members under this issue feel free to do so.
Based on xxgreg's comment on disabling header hoisting, I was able to get S3 to verify the SHA of each uploaded part in a multipart upload using presigned urls. There was no issue with Content-MD5, but the other checksum algorithms were failing.
Sample code in js for anyone else who is facing this issue.
Checkboxes for prior research
Describe the bug
Up until around January 2023, we were successfully using the S3 SDK and S3 Request pre-signer. However, it just stopped working, with no code changes and no dependency changes.
This is the code we are using:
S3 Support has not been helpful in this matter unfortunately. They referred us to a python blog post, which uses a different technique, more similar to using
@aws-sdk/s3-presigned-post
. However, we're happy with and continue to be interested in using PUT over POST for our use case.SDK version number
@aws-sdk/s3-request-presigner@3.272.0, @aws-sdk/client-s3@3.272.0
Which JavaScript Runtime is this issue in?
Node.js
Details of the browser/Node.js/ReactNative version
v18.13.0
Reproduction Steps
Run the above code, get a URL.
Paste into POSTMAN, attach a/any file.
Observed Behavior
Actual: A 200, file was uploaded to bucket
Expected Behavior
Expect: To see a 4XX - Referring to mismatch file integrity
Possible Solution
No response
Additional Information/Context
No response