v4/presign: Fix presign requests when there are more signed headers. (#3222)

This fix removes a wrong logic which fails for requests which
have more signed headers in a presign request.

Fixes #3217

cmd/auth-handler.go

@@ -163,9 +163,8 @@ func isReqAuthenticated(r *http.Request, region string) (s3Error APIErrorCode) {
 	}
 	// Populate back the payload.
 	r.Body = ioutil.NopCloser(bytes.NewReader(payload))
+	// Skips calculating sha256 on the payload on server, if client requested for it.
 	var sha256sum string
-	// Skips calculating sha256 on the payload on server,
-	// if client requested for it.
 	if skipContentSha256Cksum(r) {
 		sha256sum = unsignedPayload
 	} else {

cmd/object-handlers_test.go

@@ -30,7 +30,6 @@ import (
 	"strconv"
 	"sync"
 	"testing"
-	"time"
 )
 
 // Type to capture different modifications to API request to simulate failure cases.
@@ -2111,7 +2110,24 @@ func testAPIPutObjectPartHandlerPreSign(obj ObjectLayer, instanceType, bucketNam
 		t.Fatalf("[%s] - Failed to create an unsigned request to put object part for %s/%s <ERROR> %v",
 			instanceType, bucketName, testObject, err)
 	}
-	err = preSignV2(req, credentials.AccessKeyID, credentials.SecretAccessKey, int64(10*time.Minute))
+	err = preSignV2(req, credentials.AccessKeyID, credentials.SecretAccessKey, int64(10*60*60))
+	if err != nil {
+		t.Fatalf("[%s] - Failed to presign an unsigned request to put object part for %s/%s <ERROR> %v",
+			instanceType, bucketName, testObject, err)
+	}
+	apiRouter.ServeHTTP(rec, req)
+	if rec.Code != http.StatusOK {
+		t.Errorf("Test %d %s expected to succeed but failed with HTTP status code %d", 1, instanceType, rec.Code)
+	}
+
+	rec = httptest.NewRecorder()
+	req, err = newTestRequest("PUT", getPutObjectPartURL("", bucketName, testObject, mpartResp.UploadID, "1"),
+		int64(len("hello")), bytes.NewReader([]byte("hello")))
+	if err != nil {
+		t.Fatalf("[%s] - Failed to create an unsigned request to put object part for %s/%s <ERROR> %v",
+			instanceType, bucketName, testObject, err)
+	}
+	err = preSignV4(req, credentials.AccessKeyID, credentials.SecretAccessKey, int64(10*60*60))
 	if err != nil {
 		t.Fatalf("[%s] - Failed to presign an unsigned request to put object part for %s/%s <ERROR> %v",
 			instanceType, bucketName, testObject, err)
@@ -2582,7 +2598,27 @@ func testAPIListObjectPartsHandlerPreSign(obj ObjectLayer, instanceType, bucketN
 			instanceType, bucketName, mpartResp.UploadID)
 	}
 
-	err = preSignV2(req, credentials.AccessKeyID, credentials.SecretAccessKey, int64(10*time.Minute))
+	err = preSignV2(req, credentials.AccessKeyID, credentials.SecretAccessKey, int64(10*60*60))
+	if err != nil {
+		t.Fatalf("[%s] - Failed to presignV2 an unsigned request to list object parts for bucket %s, uploadId %s",
+			instanceType, bucketName, mpartResp.UploadID)
+	}
+	apiRouter.ServeHTTP(rec, req)
+	if rec.Code != http.StatusOK {
+		t.Errorf("Test %d %s expected to succeed but failed with HTTP status code %d",
+			1, instanceType, rec.Code)
+	}
+
+	rec = httptest.NewRecorder()
+	req, err = newTestRequest("GET",
+		getListMultipartURLWithParams("", bucketName, testObject, mpartResp.UploadID, "", "", ""),
+		0, nil)
+	if err != nil {
+		t.Fatalf("[%s] - Failed to create an unsigned request to list object parts for bucket %s, uploadId %s",
+			instanceType, bucketName, mpartResp.UploadID)
+	}
+
+	err = preSignV4(req, credentials.AccessKeyID, credentials.SecretAccessKey, int64(10*60*60))
 	if err != nil {
 		t.Fatalf("[%s] - Failed to presignV2 an unsigned request to list object parts for bucket %s, uploadId %s",
 			instanceType, bucketName, mpartResp.UploadID)

cmd/signature-v4-parser.go

@@ -184,11 +184,6 @@ func parsePreSignV4(query url.Values) (preSignValues, APIErrorCode) {
 	if err != ErrNone {
 		return preSignValues{}, err
 	}
-	// `host` is the only header used during the presigned request.
-	// Malformed signed headers has be caught here, otherwise it'll lead to signature mismatch.
-	if preSignV4Values.SignedHeaders[0] != "host" {
-		return preSignValues{}, ErrUnsignedHeaders
-	}
 
 	// Save signature.
 	preSignV4Values.Signature, err = parseSignature("Signature=" + query.Get("X-Amz-Signature"))

cmd/signature-v4-utils.go

@@ -33,10 +33,17 @@ const unsignedPayload = "UNSIGNED-PAYLOAD"
 
 // http Header "x-amz-content-sha256" == "UNSIGNED-PAYLOAD" indicates that the
 // client did not calculate sha256 of the payload. Hence we skip calculating sha256.
-// We also skip calculating sha256 for presigned requests without "x-amz-content-sha256" header.
+// We also skip calculating sha256 for presigned requests without "x-amz-content-sha256"
+// query header.
 func skipContentSha256Cksum(r *http.Request) bool {
-	contentSha256 := r.Header.Get("X-Amz-Content-Sha256")
-	return isRequestUnsignedPayload(r) || (isRequestPresignedSignatureV4(r) && contentSha256 == "")
+	queryContentSha256 := r.URL.Query().Get("X-Amz-Content-Sha256")
+	isRequestPresignedUnsignedPayload := func(r *http.Request) bool {
+		if isRequestPresignedSignatureV4(r) {
+			return queryContentSha256 == "" || queryContentSha256 == unsignedPayload
+		}
+		return false
+	}
+	return isRequestUnsignedPayload(r) || isRequestPresignedUnsignedPayload(r)
 }
 
 // isValidRegion - verify if incoming region value is valid with configured Region.
@@ -99,27 +106,24 @@ func getURLEncodedName(name string) string {
 	return encodedName
 }
 
-// find whether "host" is part of list of signed headers.
 func findHost(signedHeaders []string) APIErrorCode {
-	for _, header := range signedHeaders {
-		if header == "host" {
-			return ErrNone
-		}
+	if contains(signedHeaders, "host") {
+		return ErrNone
 	}
 	return ErrUnsignedHeaders
 }
 
 // extractSignedHeaders extract signed headers from Authorization header
 func extractSignedHeaders(signedHeaders []string, reqHeaders http.Header) (http.Header, APIErrorCode) {
-	errCode := findHost(signedHeaders)
-	if errCode != ErrNone {
-		return nil, errCode
+	// find whether "host" is part of list of signed headers.
+	// if not return ErrUnsignedHeaders. "host" is mandatory.
+	if !contains(signedHeaders, "host") {
+		return nil, ErrUnsignedHeaders
 	}
 	extractedSignedHeaders := make(http.Header)
 	for _, header := range signedHeaders {
 		// `host` will not be found in the headers, can be found in r.Host.
 		// but its alway necessary that the list of signed headers containing host in it.
-
 		val, ok := reqHeaders[http.CanonicalHeaderKey(header)]
 		if !ok {
 			// Golang http server strips off 'Expect' header, if the
@@ -153,7 +157,6 @@ func extractSignedHeaders(signedHeaders []string, reqHeaders http.Header) (http.
 		}
 		extractedSignedHeaders[header] = val
 	}
-
 	return extractedSignedHeaders, ErrNone
 }
 

cmd/signature-v4_test.go

@@ -111,6 +111,8 @@ func TestDoesPresignedSignatureMatch(t *testing.T) {
 	now := time.Now().UTC()
 	credentialTemplate := "%s/%s/%s/s3/aws4_request"
 
+	region := serverConfig.GetRegion()
+	accessKeyID := serverConfig.GetCredential().AccessKeyID
 	testCases := []struct {
 		queryParams map[string]string
 		headers     map[string]string
@@ -143,7 +145,7 @@ func TestDoesPresignedSignatureMatch(t *testing.T) {
 				"X-Amz-Expires":        "60",
 				"X-Amz-Signature":      "badsignature",
 				"X-Amz-SignedHeaders":  "host;x-amz-content-sha256;x-amz-date",
-				"X-Amz-Credential":     fmt.Sprintf(credentialTemplate, serverConfig.GetCredential().AccessKeyID, now.Format(yyyymmdd), "us-west-1"),
+				"X-Amz-Credential":     fmt.Sprintf(credentialTemplate, accessKeyID, now.Format(yyyymmdd), "us-west-1"),
 				"X-Amz-Content-Sha256": "ThisIsNotThePayloadHash",
 			},
 			region:   "us-west-1",
@@ -157,7 +159,7 @@ func TestDoesPresignedSignatureMatch(t *testing.T) {
 				"X-Amz-Expires":        "60",
 				"X-Amz-Signature":      "badsignature",
 				"X-Amz-SignedHeaders":  "host;x-amz-content-sha256;x-amz-date",
-				"X-Amz-Credential":     fmt.Sprintf(credentialTemplate, serverConfig.GetCredential().AccessKeyID, now.Format(yyyymmdd), "us-west-1"),
+				"X-Amz-Credential":     fmt.Sprintf(credentialTemplate, accessKeyID, now.Format(yyyymmdd), "us-west-1"),
 				"X-Amz-Content-Sha256": payloadSHA256,
 			},
 			region:   "us-east-1",
@@ -171,7 +173,7 @@ func TestDoesPresignedSignatureMatch(t *testing.T) {
 				"X-Amz-Expires":        "60",
 				"X-Amz-Signature":      "badsignature",
 				"X-Amz-SignedHeaders":  "host;x-amz-content-sha256;x-amz-date",
-				"X-Amz-Credential":     fmt.Sprintf(credentialTemplate, serverConfig.GetCredential().AccessKeyID, now.Format(yyyymmdd), "us-west-1"),
+				"X-Amz-Credential":     fmt.Sprintf(credentialTemplate, accessKeyID, now.Format(yyyymmdd), "us-west-1"),
 				"X-Amz-Content-Sha256": payloadSHA256,
 			},
 			region:   "us-west-1",
@@ -185,10 +187,10 @@ func TestDoesPresignedSignatureMatch(t *testing.T) {
 				"X-Amz-Expires":        "60",
 				"X-Amz-Signature":      "badsignature",
 				"X-Amz-SignedHeaders":  "x-amz-content-sha256;x-amz-date",
-				"X-Amz-Credential":     fmt.Sprintf(credentialTemplate, serverConfig.GetCredential().AccessKeyID, now.Format(yyyymmdd), serverConfig.GetRegion()),
+				"X-Amz-Credential":     fmt.Sprintf(credentialTemplate, accessKeyID, now.Format(yyyymmdd), region),
 				"X-Amz-Content-Sha256": payloadSHA256,
 			},
-			region:   serverConfig.GetRegion(),
+			region:   region,
 			expected: ErrUnsignedHeaders,
 		},
 		// (6) Should give an expired request if it has expired.
@@ -199,14 +201,14 @@ func TestDoesPresignedSignatureMatch(t *testing.T) {
 				"X-Amz-Expires":        "60",
 				"X-Amz-Signature":      "badsignature",
 				"X-Amz-SignedHeaders":  "host;x-amz-content-sha256;x-amz-date",
-				"X-Amz-Credential":     fmt.Sprintf(credentialTemplate, serverConfig.GetCredential().AccessKeyID, now.Format(yyyymmdd), serverConfig.GetRegion()),
+				"X-Amz-Credential":     fmt.Sprintf(credentialTemplate, accessKeyID, now.Format(yyyymmdd), region),
 				"X-Amz-Content-Sha256": payloadSHA256,
 			},
 			headers: map[string]string{
 				"X-Amz-Date":           now.AddDate(0, 0, -2).Format(iso8601Format),
 				"X-Amz-Content-Sha256": payloadSHA256,
 			},
-			region:   serverConfig.GetRegion(),
+			region:   region,
 			expected: ErrExpiredPresignRequest,
 		},
 		// (7) Should error if the signature is incorrect.
@@ -217,14 +219,14 @@ func TestDoesPresignedSignatureMatch(t *testing.T) {
 				"X-Amz-Expires":        "60",
 				"X-Amz-Signature":      "badsignature",
 				"X-Amz-SignedHeaders":  "host;x-amz-content-sha256;x-amz-date",
-				"X-Amz-Credential":     fmt.Sprintf(credentialTemplate, serverConfig.GetCredential().AccessKeyID, now.Format(yyyymmdd), serverConfig.GetRegion()),
+				"X-Amz-Credential":     fmt.Sprintf(credentialTemplate, accessKeyID, now.Format(yyyymmdd), region),
 				"X-Amz-Content-Sha256": payloadSHA256,
 			},
 			headers: map[string]string{
 				"X-Amz-Date":           now.Format(iso8601Format),
 				"X-Amz-Content-Sha256": payloadSHA256,
 			},
-			region:   serverConfig.GetRegion(),
+			region:   region,
 			expected: ErrSignatureDoesNotMatch,
 		},
 		// (8) Should error if the request is not ready yet, ie X-Amz-Date is in the future.
@@ -235,14 +237,14 @@ func TestDoesPresignedSignatureMatch(t *testing.T) {
 				"X-Amz-Expires":        "60",
 				"X-Amz-Signature":      "badsignature",
 				"X-Amz-SignedHeaders":  "host;x-amz-content-sha256;x-amz-date",
-				"X-Amz-Credential":     fmt.Sprintf(credentialTemplate, serverConfig.GetCredential().AccessKeyID, now.Format(yyyymmdd), serverConfig.GetRegion()),
+				"X-Amz-Credential":     fmt.Sprintf(credentialTemplate, accessKeyID, now.Format(yyyymmdd), region),
 				"X-Amz-Content-Sha256": payloadSHA256,
 			},
 			headers: map[string]string{
 				"X-Amz-Date":           now.Format(iso8601Format),
 				"X-Amz-Content-Sha256": payloadSHA256,
 			},
-			region:   serverConfig.GetRegion(),
+			region:   region,
 			expected: ErrRequestNotReadyYet,
 		},
 		// (9) Should not error with invalid region instead, call should proceed
@@ -254,7 +256,7 @@ func TestDoesPresignedSignatureMatch(t *testing.T) {
 				"X-Amz-Expires":        "60",
 				"X-Amz-Signature":      "badsignature",
 				"X-Amz-SignedHeaders":  "host;x-amz-content-sha256;x-amz-date",
-				"X-Amz-Credential":     fmt.Sprintf(credentialTemplate, serverConfig.GetCredential().AccessKeyID, now.Format(yyyymmdd), serverConfig.GetRegion()),
+				"X-Amz-Credential":     fmt.Sprintf(credentialTemplate, accessKeyID, now.Format(yyyymmdd), region),
 				"X-Amz-Content-Sha256": payloadSHA256,
 			},
 			headers: map[string]string{
@@ -273,7 +275,7 @@ func TestDoesPresignedSignatureMatch(t *testing.T) {
 				"X-Amz-Expires":         "60",
 				"X-Amz-Signature":       "badsignature",
 				"X-Amz-SignedHeaders":   "host;x-amz-content-sha256;x-amz-date",
-				"X-Amz-Credential":      fmt.Sprintf(credentialTemplate, serverConfig.GetCredential().AccessKeyID, now.Format(yyyymmdd), serverConfig.GetRegion()),
+				"X-Amz-Credential":      fmt.Sprintf(credentialTemplate, accessKeyID, now.Format(yyyymmdd), region),
 				"X-Amz-Content-Sha256":  payloadSHA256,
 				"response-content-type": "application/json",
 			},
@@ -284,6 +286,24 @@ func TestDoesPresignedSignatureMatch(t *testing.T) {
 			region:   "",
 			expected: ErrSignatureDoesNotMatch,
 		},
+		// (11) Should error with unsigned headers.
+		{
+			queryParams: map[string]string{
+				"X-Amz-Algorithm":       signV4Algorithm,
+				"X-Amz-Date":            now.Format(iso8601Format),
+				"X-Amz-Expires":         "60",
+				"X-Amz-Signature":       "badsignature",
+				"X-Amz-SignedHeaders":   "host;x-amz-content-sha256;x-amz-date",
+				"X-Amz-Credential":      fmt.Sprintf(credentialTemplate, accessKeyID, now.Format(yyyymmdd), region),
+				"X-Amz-Content-Sha256":  payloadSHA256,
+				"response-content-type": "application/json",
+			},
+			headers: map[string]string{
+				"X-Amz-Date": now.Format(iso8601Format),
+			},
+			region:   "",
+			expected: ErrUnsignedHeaders,
+		},
 	}
 
 	// Run each test case individually.

cmd/test-utils_test.go

@@ -844,6 +844,45 @@ func queryEncode(v url.Values) string {
 	return buf.String()
 }
 
+// preSignV4 presign the request, in accordance with
+// http://docs.aws.amazon.com/AmazonS3/latest/API/sigv4-query-string-auth.html.
+func preSignV4(req *http.Request, accessKeyID, secretAccessKey string, expires int64) error {
+	// Presign is not needed for anonymous credentials.
+	if accessKeyID == "" || secretAccessKey == "" {
+		return errors.New("Presign cannot be generated without access and secret keys")
+	}
+
+	region := serverConfig.GetRegion()
+	date := time.Now().UTC()
+	credential := fmt.Sprintf("%s/%s", accessKeyID, getScope(date, region))
+
+	// Set URL query.
+	query := req.URL.Query()
+	query.Set("X-Amz-Algorithm", signV4Algorithm)
+	query.Set("X-Amz-Date", date.Format(iso8601Format))
+	query.Set("X-Amz-Expires", strconv.FormatInt(expires, 10))
+	query.Set("X-Amz-SignedHeaders", "host")
+	query.Set("X-Amz-Credential", credential)
+	query.Set("X-Amz-Content-Sha256", unsignedPayload)
+
+	// Headers are empty, since "host" is the only header required to be signed for Presigned URLs.
+	var extractedSignedHeaders http.Header
+
+	queryStr := strings.Replace(query.Encode(), "+", "%20", -1)
+	canonicalRequest := getCanonicalRequest(extractedSignedHeaders, unsignedPayload, queryStr, req.URL.Path, req.Method, req.Host)
+	stringToSign := getStringToSign(canonicalRequest, date, region)
+	signingKey := getSigningKey(secretAccessKey, date, region)
+	signature := getSignature(signingKey, stringToSign)
+
+	req.URL.RawQuery = query.Encode()
+
+	// Add signature header to RawQuery.
+	req.URL.RawQuery += "&X-Amz-Signature=" + signature
+
+	// Construct the final presigned URL.
+	return nil
+}
+
 // preSignV2 - presign the request in following style.
 // https://${S3_BUCKET}.s3.amazonaws.com/${S3_OBJECT}?AWSAccessKeyId=${S3_ACCESS_KEY}&Expires=${TIMESTAMP}&Signature=${SIGNATURE}.
 func preSignV2(req *http.Request, accessKeyID, secretAccessKey string, expires int64) error {

cmd/web-handlers.go

@@ -796,7 +796,7 @@ func presignedGet(host, bucket, object string) string {
 	secretKey := cred.SecretAccessKey
 
 	date := time.Now().UTC()
-	dateStr := date.Format("20060102T150405Z")
+	dateStr := date.Format(iso8601Format)
 	credential := fmt.Sprintf("%s/%s", accessKey, getScope(date, region))
 
 	query := strings.Join([]string{