aws/signer/v4: Fix X-Amz-Content-Sha256 being in to query for presign

Fixes the bug which would allow the X-Amz-Content-Sha256 header to be
promoted to the query string when presigning a S3 request.

This bug also was preventing users from setting their own sha256 value
for a presigned URL. Presigned requests generated with the custom sha256
would of always failed with invalid signature.

S3 presign requests without a user specified X-Amz-Content-Sha256 will
no longer include the X-Amz-Content-Sha256 in the header nor query. The
X-Amz-Content-Sha256 header only needs to be set if it contains a non
UNSIGNED-PAYLOAD value.

Fix #1974

aws/signer/v4/functional_test.go

@@ -48,7 +48,7 @@ func TestPresignHandler(t *testing.T) {
 	expectedHost := "bucket.s3.mock-region.amazonaws.com"
 	expectedDate := "19700101T000000Z"
 	expectedHeaders := "content-disposition;host;x-amz-acl"
-	expectedSig := "a46583256431b09eb45ba4af2e6286d96a9835ed13721023dc8076dfdcb90fcb"
+	expectedSig := "2d76a414208c0eac2a23ef9c834db9635ecd5a0fbb447a00ad191f82d854f55b"
 	expectedCred := "AKID/19700101/mock-region/s3/aws4_request"
 
 	u, _ := url.Parse(urlstr)
@@ -71,8 +71,8 @@ func TestPresignHandler(t *testing.T) {
 	if e, a := "300", urlQ.Get("X-Amz-Expires"); e != a {
 		t.Errorf("expect %v, got %v", e, a)
 	}
-	if e, a := "UNSIGNED-PAYLOAD", urlQ.Get("X-Amz-Content-Sha256"); e != a {
-		t.Errorf("expect %v, got %v", e, a)
+	if a := urlQ.Get("X-Amz-Content-Sha256"); len(a) != 0 {
+		t.Errorf("expect no content sha256 got %v", a)
 	}
 
 	if e, a := "+", urlstr; strings.Contains(a, e) { // + encoded as %20
@@ -98,7 +98,7 @@ func TestPresignRequest(t *testing.T) {
 	expectedHost := "bucket.s3.mock-region.amazonaws.com"
 	expectedDate := "19700101T000000Z"
 	expectedHeaders := "content-disposition;host;x-amz-acl"
-	expectedSig := "a46583256431b09eb45ba4af2e6286d96a9835ed13721023dc8076dfdcb90fcb"
+	expectedSig := "2d76a414208c0eac2a23ef9c834db9635ecd5a0fbb447a00ad191f82d854f55b"
 	expectedCred := "AKID/19700101/mock-region/s3/aws4_request"
 	expectedHeaderMap := http.Header{
 		"x-amz-acl":           []string{"public-read"},
@@ -128,8 +128,8 @@ func TestPresignRequest(t *testing.T) {
 	if e, a := "300", urlQ.Get("X-Amz-Expires"); e != a {
 		t.Errorf("expect %v, got %v", e, a)
 	}
-	if e, a := "UNSIGNED-PAYLOAD", urlQ.Get("X-Amz-Content-Sha256"); e != a {
-		t.Errorf("expect %v, got %v", e, a)
+	if a := urlQ.Get("X-Amz-Content-Sha256"); len(a) != 0 {
+		t.Errorf("expect no content sha256 got %v", a)
 	}
 
 	if e, a := "+", urlstr; strings.Contains(a, e) { // + encoded as %20
@@ -169,7 +169,7 @@ func TestStandaloneSign_WithPort(t *testing.T) {
 
 	cases := []struct {
 		description string
-		url            string
+		url         string
 		expectedSig string
 	}{
 		{
@@ -213,7 +213,7 @@ func TestStandalonePresign_WithPort(t *testing.T) {
 
 	cases := []struct {
 		description string
-		url            string
+		url         string
 		expectedSig string
 	}{
 		{
@@ -241,7 +241,7 @@ func TestStandalonePresign_WithPort(t *testing.T) {
 	for _, c := range cases {
 		signer := v4.NewSigner(unit.Session.Config.Credentials)
 		req, _ := http.NewRequest("GET", c.url, nil)
-		_, err := signer.Presign(req, nil, "es", "us-east-1", 5 * time.Minute, time.Unix(0, 0))
+		_, err := signer.Presign(req, nil, "es", "us-east-1", 5*time.Minute, time.Unix(0, 0))
 		if err != nil {
 			t.Fatalf("expect no error, got %v", err)
 		}

aws/signer/v4/v4.go

@@ -135,6 +135,7 @@ var requiredSignedHeaders = rules{
 			"X-Amz-Server-Side-Encryption-Customer-Key-Md5":               struct{}{},
 			"X-Amz-Storage-Class":                                         struct{}{},
 			"X-Amz-Website-Redirect-Location":                             struct{}{},
+			"X-Amz-Content-Sha256":                                        struct{}{},
 		},
 	},
 	patterns{"X-Amz-Meta-"},
@@ -671,8 +672,15 @@ func (ctx *signingCtx) buildSignature() {
 func (ctx *signingCtx) buildBodyDigest() error {
 	hash := ctx.Request.Header.Get("X-Amz-Content-Sha256")
 	if hash == "" {
-		if ctx.unsignedPayload || (ctx.isPresign && ctx.ServiceName == "s3") {
+		includeSHA256Header := ctx.unsignedPayload ||
+			ctx.ServiceName == "s3" ||
+			ctx.ServiceName == "glacier"
+
+		s3Presign := ctx.isPresign && ctx.ServiceName == "s3"
+
+		if ctx.unsignedPayload || s3Presign {
 			hash = "UNSIGNED-PAYLOAD"
+			includeSHA256Header = !s3Presign
 		} else if ctx.Body == nil {
 			hash = emptyStringSHA256
 		} else {
@@ -681,7 +689,8 @@ func (ctx *signingCtx) buildBodyDigest() error {
 			}
 			hash = hex.EncodeToString(makeSha256Reader(ctx.Body))
 		}
-		if ctx.unsignedPayload || ctx.ServiceName == "s3" || ctx.ServiceName == "glacier" {
+
+		if includeSHA256Header {
 			ctx.Request.Header.Set("X-Amz-Content-Sha256", hash)
 		}
 	}

aws/signer/v4/v4_test.go

@@ -227,9 +227,8 @@ func TestPresignEmptyBodyS3(t *testing.T) {
 	req, body := buildRequest("s3", "us-east-1", "hello")
 	signer := buildSigner()
 	signer.Presign(req, body, "s3", "us-east-1", 5*time.Minute, time.Now())
-	hash := req.Header.Get("X-Amz-Content-Sha256")
-	if e, a := "UNSIGNED-PAYLOAD", hash; e != a {
-		t.Errorf("expect %v, got %v", e, a)
+	if a := req.Header.Get("X-Amz-Content-Sha256"); len(a) != 0 {
+		t.Errorf("expect no content sha256 got %v", a)
 	}
 }