From 1d99a560e3a9264307e651ad05a58ee7eba81f0f Mon Sep 17 00:00:00 2001 From: Krishna Srinivas Date: Wed, 5 Apr 2017 17:00:24 -0700 Subject: [PATCH] refactor: extractSignedHeaders() handles headers removed by Go http server (#4054) * refactor: extractSignedHeaders() handles headers removed by Go http server. * Cleanup extractSignedHeaders() TestExtractSignedHeaders() --- cmd/signature-v4-utils.go | 39 ++++++++++++++++++------------- cmd/signature-v4-utils_test.go | 34 +++++++++++++-------------- cmd/signature-v4.go | 42 ++++++++-------------------------- cmd/streaming-signature-v4.go | 2 +- cmd/test-utils_test.go | 7 +++--- cmd/web-handlers.go | 8 +++---- 6 files changed, 59 insertions(+), 73 deletions(-) diff --git a/cmd/signature-v4-utils.go b/cmd/signature-v4-utils.go index 47834cd3d..e35a97d02 100644 --- a/cmd/signature-v4-utils.go +++ b/cmd/signature-v4-utils.go @@ -21,6 +21,7 @@ import ( "encoding/hex" "net/http" "regexp" + "strconv" "strings" "unicode/utf8" @@ -119,7 +120,14 @@ func extractSignedHeaders(signedHeaders []string, r *http.Request) (http.Header, // `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 { + if ok { + for _, enc := range val { + extractedSignedHeaders.Add(header, enc) + } + continue + } + switch header { + case "expect": // Golang http server strips off 'Expect' header, if the // client sent this as part of signed headers we need to // handle otherwise we would see a signature mismatch. @@ -136,24 +144,23 @@ func extractSignedHeaders(signedHeaders []string, r *http.Request) (http.Header, // be sent, for the time being keep this work around. // Adding a *TODO* to remove this later when Golang server // doesn't filter out the 'Expect' header. - if header == "expect" { - extractedSignedHeaders[header] = []string{"100-continue"} - continue + extractedSignedHeaders.Set(header, "100-continue") + case "host": + // Go http server removes "host" from Request.Header + extractedSignedHeaders.Set(header, r.Host) + case "transfer-encoding": + // Go http server removes "host" from Request.Header + for _, enc := range r.TransferEncoding { + extractedSignedHeaders.Add(header, enc) } - // the "host" field will not be found in the header map, it can be found in req.Host. - // but its necessary to make sure that the "host" field exists in the list of signed parameters, - // the check is done above. - if header == "host" { - continue - } - if header == "transfer-encoding" { - extractedSignedHeaders[header] = r.TransferEncoding - continue - } - // If not found continue, we will stop here. + case "content-length": + // Signature-V4 spec excludes Content-Length from signed headers list for signature calculation. + // But some clients deviate from this rule. Hence we consider Content-Length for signature + // calculation to be compatible with such clients. + extractedSignedHeaders.Set(header, strconv.FormatInt(r.ContentLength, 10)) + default: return nil, ErrUnsignedHeaders } - extractedSignedHeaders[header] = val } return extractedSignedHeaders, ErrNone } diff --git a/cmd/signature-v4-utils_test.go b/cmd/signature-v4-utils_test.go index 830d040cd..27e05b499 100644 --- a/cmd/signature-v4-utils_test.go +++ b/cmd/signature-v4-utils_test.go @@ -141,8 +141,9 @@ func TestExtractSignedHeaders(t *testing.T) { expectedContentSha256 := "1234abcd" expectedTime := UTCNow().Format(iso8601Format) expectedTransferEncoding := "gzip" + expectedExpect := "100-continue" - r, err := http.NewRequest("GET", "http://localhost", nil) + r, err := http.NewRequest("GET", "http://play.minio.io:9000", nil) if err != nil { t.Fatal("Unable to create http.Request :", err) } @@ -150,9 +151,8 @@ func TestExtractSignedHeaders(t *testing.T) { // Creating input http header. inputHeader := r.Header - inputHeader.Set(signedHeaders[0], expectedHost) - inputHeader.Set(signedHeaders[1], expectedContentSha256) - inputHeader.Set(signedHeaders[2], expectedTime) + inputHeader.Set("x-amz-content-sha256", expectedContentSha256) + inputHeader.Set("x-amz-date", expectedTime) // calling the function being tested. extractedSignedHeaders, errCode := extractSignedHeaders(signedHeaders, r) if errCode != ErrNone { @@ -160,24 +160,24 @@ func TestExtractSignedHeaders(t *testing.T) { } // "x-amz-content-sha256" header value from the extracted result. - extractedContentSha256 := extractedSignedHeaders[signedHeaders[1]] + extractedContentSha256 := extractedSignedHeaders.Get("x-amz-content-sha256") // "host" header value from the extracted result. - extractedHost := extractedSignedHeaders[signedHeaders[0]] + extractedHost := extractedSignedHeaders.Get("host") // "x-amz-date" header from the extracted result. - extractedDate := extractedSignedHeaders[signedHeaders[2]] + extractedDate := extractedSignedHeaders.Get("x-amz-date") // extracted `expect` header. - extractedExpect := extractedSignedHeaders["expect"][0] + extractedExpect := extractedSignedHeaders.Get("expect") - extractedTransferEncoding := extractedSignedHeaders["transfer-encoding"][0] + extractedTransferEncoding := extractedSignedHeaders.Get("transfer-encoding") - if expectedHost != extractedHost[0] { + if expectedHost != extractedHost { t.Errorf("host header mismatch: expected `%s`, got `%s`", expectedHost, extractedHost) } // assert the result with the expected value. - if expectedContentSha256 != extractedContentSha256[0] { + if expectedContentSha256 != extractedContentSha256 { t.Errorf("x-amz-content-sha256 header mismatch: expected `%s`, got `%s`", expectedContentSha256, extractedContentSha256) } - if expectedTime != extractedDate[0] { + if expectedTime != extractedDate { t.Errorf("x-amz-date header mismatch: expected `%s`, got `%s`", expectedTime, extractedDate) } if extractedTransferEncoding != expectedTransferEncoding { @@ -185,12 +185,12 @@ func TestExtractSignedHeaders(t *testing.T) { } // Since the list of signed headers value contained `expect`, the default value of `100-continue` will be added to extracted signed headers. - if extractedExpect != "100-continue" { - t.Errorf("expect header incorrect value: expected `%s`, got `%s`", "100-continue", extractedExpect) + if extractedExpect != expectedExpect { + t.Errorf("expect header incorrect value: expected `%s`, got `%s`", expectedExpect, extractedExpect) } - // case where the headers doesn't contain the one of the signed header in the signed headers list. - signedHeaders = append(signedHeaders, " X-Amz-Credential") + // case where the headers don't contain the one of the signed header in the signed headers list. + signedHeaders = append(signedHeaders, "X-Amz-Credential") // expected to fail with `ErrUnsignedHeaders`. _, errCode = extractSignedHeaders(signedHeaders, r) if errCode != ErrUnsignedHeaders { @@ -198,7 +198,7 @@ func TestExtractSignedHeaders(t *testing.T) { } // case where the list of signed headers doesn't contain the host field. - signedHeaders = signedHeaders[1:] + signedHeaders = signedHeaders[2:5] // expected to fail with `ErrUnsignedHeaders`. _, errCode = extractSignedHeaders(signedHeaders, r) if errCode != ErrUnsignedHeaders { diff --git a/cmd/signature-v4.go b/cmd/signature-v4.go index 59acce0f6..2d5fc79ac 100644 --- a/cmd/signature-v4.go +++ b/cmd/signature-v4.go @@ -46,33 +46,26 @@ const ( ) // getCanonicalHeaders generate a list of request headers with their values -func getCanonicalHeaders(signedHeaders http.Header, host string) string { +func getCanonicalHeaders(signedHeaders http.Header) string { var headers []string vals := make(http.Header) for k, vv := range signedHeaders { headers = append(headers, strings.ToLower(k)) vals[strings.ToLower(k)] = vv } - headers = append(headers, presignedHostHeader) sort.Strings(headers) var buf bytes.Buffer for _, k := range headers { buf.WriteString(k) buf.WriteByte(':') - switch { - case k == presignedHostHeader: - buf.WriteString(host) - fallthrough - default: - for idx, v := range vals[k] { - if idx > 0 { - buf.WriteByte(',') - } - buf.WriteString(signV4TrimAll(v)) + for idx, v := range vals[k] { + if idx > 0 { + buf.WriteByte(',') } - buf.WriteByte('\n') + buf.WriteString(signV4TrimAll(v)) } + buf.WriteByte('\n') } return buf.String() } @@ -83,7 +76,6 @@ func getSignedHeaders(signedHeaders http.Header) string { for k := range signedHeaders { headers = append(headers, strings.ToLower(k)) } - headers = append(headers, presignedHostHeader) sort.Strings(headers) return strings.Join(headers, ";") } @@ -98,14 +90,14 @@ func getSignedHeaders(signedHeaders http.Header) string { // \n // // -func getCanonicalRequest(extractedSignedHeaders http.Header, payload, queryStr, urlPath, method, host string) string { +func getCanonicalRequest(extractedSignedHeaders http.Header, payload, queryStr, urlPath, method string) string { rawQuery := strings.Replace(queryStr, "+", "%20", -1) encodedPath := getURLEncodedName(urlPath) canonicalRequest := strings.Join([]string{ method, encodedPath, rawQuery, - getCanonicalHeaders(extractedSignedHeaders, host), + getCanonicalHeaders(extractedSignedHeaders), getSignedHeaders(extractedSignedHeaders), payload, }, "\n") @@ -304,7 +296,7 @@ func doesPresignedSignatureMatch(hashedPayload string, r *http.Request, region s /// Verify finally if signature is same. // Get canonical request. - presignedCanonicalReq := getCanonicalRequest(extractedSignedHeaders, hashedPayload, encodedQuery, req.URL.Path, req.Method, req.Host) + presignedCanonicalReq := getCanonicalRequest(extractedSignedHeaders, hashedPayload, encodedQuery, req.URL.Path, req.Method) // Get string to sign from canonical request. presignedStringToSign := getStringToSign(presignedCanonicalReq, t, pSignValues.Credential.getScope()) @@ -346,19 +338,6 @@ func doesSignatureMatch(hashedPayload string, r *http.Request, region string) AP return ErrContentSHA256Mismatch } - header := req.Header - - // Signature-V4 spec excludes Content-Length from signed headers list for signature calculation. - // But some clients deviate from this rule. Hence we consider Content-Length for signature - // calculation to be compatible with such clients. - for _, h := range signV4Values.SignedHeaders { - if h == "content-length" { - header = cloneHeader(req.Header) - header.Set("content-length", strconv.FormatInt(r.ContentLength, 10)) - break - } - } - // Extract all the signed headers along with its values. extractedSignedHeaders, errCode := extractSignedHeaders(signV4Values.SignedHeaders, r) if errCode != ErrNone { @@ -401,8 +380,7 @@ func doesSignatureMatch(hashedPayload string, r *http.Request, region string) AP queryStr := req.URL.Query().Encode() // Get canonical request. - canonicalRequest := getCanonicalRequest(extractedSignedHeaders, hashedPayload, queryStr, req.URL.Path, req.Method, req.Host) - + canonicalRequest := getCanonicalRequest(extractedSignedHeaders, hashedPayload, queryStr, req.URL.Path, req.Method) // Get string to sign from canonical request. stringToSign := getStringToSign(canonicalRequest, t, signV4Values.Credential.getScope()) diff --git a/cmd/streaming-signature-v4.go b/cmd/streaming-signature-v4.go index cbbf515c8..87d1f1449 100644 --- a/cmd/streaming-signature-v4.go +++ b/cmd/streaming-signature-v4.go @@ -133,7 +133,7 @@ func calculateSeedSignature(r *http.Request) (signature string, date time.Time, queryStr := req.URL.Query().Encode() // Get canonical request. - canonicalRequest := getCanonicalRequest(extractedSignedHeaders, payload, queryStr, req.URL.Path, req.Method, req.Host) + canonicalRequest := getCanonicalRequest(extractedSignedHeaders, payload, queryStr, req.URL.Path, req.Method) // Get string to sign from canonical request. stringToSign := getStringToSign(canonicalRequest, date, signV4Values.Credential.getScope()) diff --git a/cmd/test-utils_test.go b/cmd/test-utils_test.go index 46b1fab17..a78a4183e 100644 --- a/cmd/test-utils_test.go +++ b/cmd/test-utils_test.go @@ -889,11 +889,12 @@ func preSignV4(req *http.Request, accessKeyID, secretAccessKey string, expires i 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 + // "host" is the only header required to be signed for Presigned URLs. + extractedSignedHeaders := make(http.Header) + extractedSignedHeaders.Set("host", req.Host) queryStr := strings.Replace(query.Encode(), "+", "%20", -1) - canonicalRequest := getCanonicalRequest(extractedSignedHeaders, unsignedPayload, queryStr, req.URL.Path, req.Method, req.Host) + canonicalRequest := getCanonicalRequest(extractedSignedHeaders, unsignedPayload, queryStr, req.URL.Path, req.Method) stringToSign := getStringToSign(canonicalRequest, date, scope) signingKey := getSigningKey(secretAccessKey, date, region) signature := getSignature(signingKey, stringToSign) diff --git a/cmd/web-handlers.go b/cmd/web-handlers.go index 685136ac4..7f6e1d870 100644 --- a/cmd/web-handlers.go +++ b/cmd/web-handlers.go @@ -868,10 +868,10 @@ func presignedGet(host, bucket, object string, expiry int64) string { path := "/" + path.Join(bucket, object) - // Headers are empty, since "host" is the only header required to be signed for Presigned URLs. - var extractedSignedHeaders http.Header - - canonicalRequest := getCanonicalRequest(extractedSignedHeaders, unsignedPayload, query, path, "GET", host) + // "host" is the only header required to be signed for Presigned URLs. + extractedSignedHeaders := make(http.Header) + extractedSignedHeaders.Set("host", host) + canonicalRequest := getCanonicalRequest(extractedSignedHeaders, unsignedPayload, query, path, "GET") stringToSign := getStringToSign(canonicalRequest, date, getScope(date, region)) signingKey := getSigningKey(secretKey, date, region) signature := getSignature(signingKey, stringToSign)