Use UNSIGNED-PAYLOAD if sha256 is not calculated #661
Conversation
| if metadata.contentSHA256Bytes == nil { | ||
| shaHeader = hex.EncodeToString(sum256([]byte{})) | ||
| } else { | ||
| if len(metadata.contentSHA256Bytes) > 0 { |
There was a problem hiding this comment.
Shouldn't this specifically be for presigned operations? - why is it done for regular signature V4. When there is no data it should be calculated as "sha256" of an empty byte array.
http://docs.aws.amazon.com/AmazonS3/latest/API/sig-v4-header-based-auth.html
Hex(SHA256Hash(""))
There was a problem hiding this comment.
no, actually this is unrelated to presigned url.
i.e the following code gives error without this fix:
krishna@escape:~/tmp$ cat test.go
package main
import (
"log"
"strings"
minio "github.com/minio/minio-go"
)
func main() {
client, err := minio.NewCore("localhost:9000", "SXO8VW2OFKKP2OG7AC85", "CKWSSgrUgvfUMTaNBkB63exet4WW+uNhQvi91Bc3", false)
if err != nil {
log.Fatal(err)
}
bucket := "testbucket"
object := "testobject"
content := "hello world"
size := int64(len(content))
r := strings.NewReader(content)
_, err = client.PutObject(bucket, object, size, r, nil, nil, nil)
if err != nil {
log.Fatal(err)
}
}
krishna@escape:~/tmp$ go run test.go
2017/04/23 19:16:13 The provided 'x-amz-content-sha256' header does not match what was computed.
exit status 1
krishna@escape:~/tmp$
There was a problem hiding this comment.
The issue was found when testing S3 gateway. In gateway setup, if client does
presignedPUT the sha256 value is not valid in which case we should use
UNSIGNED-PAYLOAD instead of calculating sha256sum() on empty byte array
meaning, for presignedPUT the gateway calls:
Core.PutObject() with sha256bytes as empty byte array
Now minio-go sets SHA256 header as Hex(SHA256Hash("")) because of the bug instead of UNSIGNED-PAYLOAD
There was a problem hiding this comment.
But we are breaking backward compatibility here. The example is wrong it is expected in signature-v4 for PUT you are passing in the payload checksum. So setting it as nil is wrong.
What should be set is perhaps []byte("UNSIGNED-PAYLOAD") is passed in from PutObject() perspective indicating that the payload checksum is going to be unsigned and internal code differentiates this.
Otherwise we are breaking backward compatibility i.e for some servers which do not implement UNSIGNED-PAYLOAD verification for GET, HEAD operations would lead to signature issues.
There was a problem hiding this comment.
@krishnasrinivas It is not wrong to compute sha256 sum of []byte{} and set x-amz-content-sha256 header with it. See Important section here http://docs.aws.amazon.com/AmazonS3/latest/API/sig-v4-header-based-auth.html. UNSIGNED-PAYLOAD is an alternate. So, the server should work with requests signed using either of the approaches.
There was a problem hiding this comment.
Otherwise we are breaking backward compatibility i.e for some servers which do not implement UNSIGNED-PAYLOAD verification for GET, HEAD operations would lead to signature issues.
but even without this fix, for secure connections, we are sending UNSIGNED-PAYLOAD for GET/HEAD, take a look at this:
krishna@escape:~$ mc --debug cat s3/krisbuck/passwd
mc: <DEBUG> GET /krisbuck/?location= HTTP/1.1
Host: s3.amazonaws.com
User-Agent: Minio (linux; amd64) minio-go/2.0.4 mc/DEVELOPMENT.GOGET
Authorization: AWS4-HMAC-SHA256 Credential=**REDACTED**/20170424/us-east-1/s3/aws4_request, SignedHeaders=host;x-amz-content-sha256;x-amz-date, Signature=**REDACTED**
X-Amz-Content-Sha256: UNSIGNED-PAYLOAD
X-Amz-Date: 20170424T063041Z
Accept-Encoding: gzip
mc: <DEBUG> HTTP/1.1 200 OK
Transfer-Encoding: chunked
Content-Type: application/xml
Date: Mon, 24 Apr 2017 06:30:42 GMT
Server: AmazonS3
X-Amz-Id-2: tC2i4HFd7VHFDaUQ4gzO37h+PxBAs+yI/zogjuNoMtB7rFEQ0aItQHAt8jmp2W5xJQvWJmY9IYs=
X-Amz-Request-Id: 87492B7834CCCD81
mc: <DEBUG> Response Time: 482.121718ms
mc: <DEBUG> GET /?delimiter=%2F&fetch-owner=true&list-type=2&max-keys=1000&prefix=passwd HTTP/1.1
Host: krisbuck.s3.amazonaws.com
User-Agent: Minio (linux; amd64) minio-go/2.0.4 mc/DEVELOPMENT.GOGET
Authorization: AWS4-HMAC-SHA256 Credential=**REDACTED**/20170424/us-east-1/s3/aws4_request, SignedHeaders=host;x-amz-content-sha256;x-amz-date, Signature=**REDACTED**
X-Amz-Content-Sha256: UNSIGNED-PAYLOAD
X-Amz-Date: 20170424T063041Z
Accept-Encoding: gzip
mc: <DEBUG> HTTP/1.1 200 OK
Transfer-Encoding: chunked
Content-Type: application/xml
Date: Mon, 24 Apr 2017 06:30:43 GMT
Server: AmazonS3
X-Amz-Bucket-Region: us-east-1
X-Amz-Id-2: g4oRn0k5X1eNANYJaF92HPR1WlgrXmWEQQernVrGXxgP0uIUmnEQjUud1bbjorMEE7V5eYnB7/s=
X-Amz-Request-Id: 0D01FE6F31848632
mc: <DEBUG> Response Time: 726.363076ms
mc: <DEBUG> HEAD /passwd HTTP/1.1
Host: krisbuck.s3.amazonaws.com
User-Agent: Minio (linux; amd64) minio-go/2.0.4 mc/DEVELOPMENT.GOGET
Authorization: AWS4-HMAC-SHA256 Credential=**REDACTED**/20170424/us-east-1/s3/aws4_request, SignedHeaders=host;x-amz-content-sha256;x-amz-date, Signature=**REDACTED**
X-Amz-Content-Sha256: UNSIGNED-PAYLOAD
X-Amz-Date: 20170424T063042Z
mc: <DEBUG> HTTP/1.1 200 OK
Content-Length: 2284
Accept-Ranges: bytes
Content-Type: application/octet-stream
Date: Mon, 24 Apr 2017 06:30:43 GMT
Etag: "73c2ee0acb590362c0ac3740f8c39523"
Last-Modified: Mon, 24 Apr 2017 06:30:34 GMT
Server: AmazonS3
X-Amz-Id-2: 0NnKbRgQIE5Kq2yRJ+zdQX0fKijcJ413XDhRhqq42bv6ZbmvykpmcvM8ZleG5AjiPO4uwzAqKCs=
X-Amz-Request-Id: 1E6CE7455F322E3F
mc: <DEBUG> Response Time: 91.483497ms
mc: <DEBUG> GET /passwd HTTP/1.1
Host: krisbuck.s3.amazonaws.com
User-Agent: Minio (linux; amd64) minio-go/2.0.4 mc/DEVELOPMENT.GOGET
Authorization: AWS4-HMAC-SHA256 Credential=**REDACTED**/20170424/us-east-1/s3/aws4_request, SignedHeaders=host;x-amz-content-sha256;x-amz-date, Signature=**REDACTED**
X-Amz-Content-Sha256: UNSIGNED-PAYLOAD
X-Amz-Date: 20170424T063042Z
Accept-Encoding: gzip
mc: <DEBUG> HTTP/1.1 200 OK
Content-Length: 2284
Accept-Ranges: bytes
Content-Type: application/octet-stream
Date: Mon, 24 Apr 2017 06:30:43 GMT
Etag: "73c2ee0acb590362c0ac3740f8c39523"
Last-Modified: Mon, 24 Apr 2017 06:30:34 GMT
Server: AmazonS3
X-Amz-Id-2: dFrgt4Y2e6WL8Y/r0eWFk+pC9wF5C+cjc0w0NKbtn1ksJXNB9zkR16GR9z/M2Ju8s2EjZqprw10=
X-Amz-Request-Id: 0BF985468EB69ADF
mc: <DEBUG> Response Time: 90.616068ms
root:x:0:0:root:/root:/bin/bash
....
...
But I see what you mean here, minio-js was written such that it uses UNSIGNED-PAYLOAD only for "upload object" operations.
What should be set is perhaps []byte("UNSIGNED-PAYLOAD") is passed in from PutObject() perspective indicating that the payload checksum is going to be unsigned and internal code differentiates this.
It can be fixed in a cleaner way if we want to fix it. i.e metadata.contentSHA256Bytes being nil should not indicate whether the body itself is empty. If the body is empty the caller should set metadata.contentSHA256Bytes to Hex(SHA256Hash("")) and If metadata.contentSHA256Bytes is set to nil then we simple use UNSIGNED-PAYLOAD
@krishnasrinivas It is not wrong to compute sha256 sum of []byte{} and set x-amz-content-sha256 header with it. See Important section here http://docs.aws.amazon.com/AmazonS3/latest/API/sig-v4-header-based-auth.html. UNSIGNED-PAYLOAD is an alternate. So, the server should work with requests signed using either of the approaches.
Ofcourse. This commit does not counter that fact.
There was a problem hiding this comment.
Ofcourse. This commit does not counter that fact.
Oops. This is completely different from what I was referring to. UNSIGNED-PAYLOAD got me.
This makes sense we should do it @krishnasrinivas in this PR. |
|
Closing this PR. opened new PR #668 (this PR was from master branch, mistake) |
The issue was found when testing S3 gateway. In gateway setup, if client does
presignedPUT the sha256 value is not valid in which case we should use
UNSIGNED-PAYLOAD instead of calculating sha256sum() on empty byte array