service/s3: Add ContentMD5 validation of S3 Objects #1827
Conversation
jasdel
force-pushed
the
feature/S3IntegChecks
branch
from
900ef34
to
8108894
Compare
Adds support for setting the ContentMD5 of objects uploaded to S3. Also adds validating the Object's MD5 hash downloaded matches that of the hash the object was uploaded with. This feature provides validation and early detection when S3 Object contents have changed unexpectedly.
|
Shouldn't we also strip off the content length of the hash once we remove it? |
service/s3/body_hash.go
Outdated
| if aws.BoolValue(r.Config.S3DisableContentMD5Validation) { | ||
| return | ||
| } | ||
| if r.ExpireTime != 0 { |
There was a problem hiding this comment.
How do you feel about putting this in a helper method like IsPresign? I think it'll increase the readability here.
There was a problem hiding this comment.
Sounds good will create a helper for this.
| encoded := make([]byte, md5Base64EncLen) | ||
|
|
||
| base64.StdEncoding.Encode(encoded, md5Hash.Sum(sum[0:0])) | ||
| r.HTTPRequest.Header[contentMD5Header] = []string{string(encoded)} |
There was a problem hiding this comment.
What if this header is already set? Wouldn't this stomp over it?
There was a problem hiding this comment.
This this block is only performed when there is no header present, the check was further up in the function. I'll add docs to clarify this.
| sum := make([]byte, sha256.Size) | ||
|
|
||
| hex.Encode(encoded, sha256Hash.Sum(sum[0:0])) | ||
| r.HTTPRequest.Header[contentSha256Header] = []string{string(encoded)} |
jasdel
force-pushed
the
feature/S3IntegChecks
branch
from
308c677
to
60a323a
Compare
Updates the S3 client to disable automatic ContentMD5 validation of S3 Put and Get objects. This disables the validation added in #1827. Unexpected case whereContent-Length response header not set in an a GetObject API call prevents this the content MD5 validation feature from being successfully used until the SDK can handle the case. The SDK will still set the Content-MD5 header for PutObject and UploadPart API calls. Related to: #1837
|
@jasdel Just to make sure I read the code right, this PR adds end-to-end MD5 validation to all the following functions?
Thanks! |
|
@Quentin-M This functionality actually had to be disabled while we resolve a issue with
|
|
Thank you so much for your answer/work, much appreciated!
|
|
I created CPU profile and found that
|
Adds support for setting the ContentMD5 of objects uploaded to S3. Also
adds validating the Object's MD5 hash downloaded matches that of the
hash the object was uploaded with. This feature provides validation and
early detection when S3 Object contents have changed unexpectedly.