From eafc15cd470ed60f6920081011cd67cf7a6a2df4 Mon Sep 17 00:00:00 2001 From: Harshavardhana Date: Tue, 5 Jun 2018 10:48:51 -0700 Subject: [PATCH] Fix presigned URL for access key with special characters (#6012) Fixes #6011 --- cmd/signature-v4-utils.go | 43 ---------------------------------- cmd/signature-v4-utils_test.go | 33 -------------------------- cmd/signature-v4.go | 3 ++- cmd/test-utils_test.go | 7 +++--- cmd/web-handlers.go | 21 ++++++++++------- 5 files changed, 18 insertions(+), 89 deletions(-) diff --git a/cmd/signature-v4-utils.go b/cmd/signature-v4-utils.go index b8ca0a5d0..136271313 100644 --- a/cmd/signature-v4-utils.go +++ b/cmd/signature-v4-utils.go @@ -18,12 +18,9 @@ package cmd import ( "crypto/hmac" - "encoding/hex" "net/http" - "regexp" "strconv" "strings" - "unicode/utf8" "github.com/minio/sha256-simd" ) @@ -104,46 +101,6 @@ func sumHMAC(key []byte, data []byte) []byte { return hash.Sum(nil) } -// Reserved string regexp. -var reservedNames = regexp.MustCompile("^[a-zA-Z0-9-_.~/]+$") - -// getURLEncodedName encode the strings from UTF-8 byte representations to HTML hex escape sequences -// -// This is necessary since regular url.Parse() and url.Encode() functions do not support UTF-8 -// non english characters cannot be parsed due to the nature in which url.Encode() is written -// -// This function on the other hand is a direct replacement for url.Encode() technique to support -// pretty much every UTF-8 character. -func getURLEncodedName(name string) string { - // if object matches reserved string, no need to encode them - if reservedNames.MatchString(name) { - return name - } - var encodedName string - for _, s := range name { - if 'A' <= s && s <= 'Z' || 'a' <= s && s <= 'z' || '0' <= s && s <= '9' { // §2.3 Unreserved characters (mark) - encodedName = encodedName + string(s) - continue - } - switch s { - case '-', '_', '.', '~', '/': // §2.3 Unreserved characters (mark) - encodedName = encodedName + string(s) - continue - default: - len := utf8.RuneLen(s) - if len > 0 { - u := make([]byte, len) - utf8.EncodeRune(u, s) - for _, r := range u { - hex := hex.EncodeToString([]byte{r}) - encodedName = encodedName + "%" + strings.ToUpper(hex) - } - } - } - } - return encodedName -} - // extractSignedHeaders extract signed headers from Authorization header func extractSignedHeaders(signedHeaders []string, r *http.Request) (http.Header, APIErrorCode) { reqHeaders := r.Header diff --git a/cmd/signature-v4-utils_test.go b/cmd/signature-v4-utils_test.go index e10188b0e..dafea54a8 100644 --- a/cmd/signature-v4-utils_test.go +++ b/cmd/signature-v4-utils_test.go @@ -120,39 +120,6 @@ func TestIsValidRegion(t *testing.T) { } } -// Tests validate the URL path encoder. -func TestGetURLEncodedName(t *testing.T) { - testCases := []struct { - // Input. - inputStr string - // Expected result. - result string - }{ - // % should be encoded as %25 - {"thisisthe%url", "thisisthe%25url"}, - // UTF-8 encoding. - {"本語", "%E6%9C%AC%E8%AA%9E"}, - // UTF-8 encoding with ASCII. - {"本語.1", "%E6%9C%AC%E8%AA%9E.1"}, - // Unusual ASCII characters. - {">123", "%3E123"}, - // Fragment path characters. - {"myurl#link", "myurl%23link"}, - // Space should be set to %20 not '+'. - {"space in url", "space%20in%20url"}, - // '+' shouldn't be treated as space. - {"url+path", "url%2Bpath"}, - } - - // Tests generated values from url encoded name. - for i, testCase := range testCases { - result := getURLEncodedName(testCase.inputStr) - if testCase.result != result { - t.Errorf("Test %d: Expected URLEncoded result to be \"%s\", but found it to be \"%s\" instead", i+1, testCase.result, result) - } - } -} - // TestExtractSignedHeaders - Tests validate extraction of signed headers using list of signed header keys. func TestExtractSignedHeaders(t *testing.T) { signedHeaders := []string{"host", "x-amz-content-sha256", "x-amz-date", "transfer-encoding"} diff --git a/cmd/signature-v4.go b/cmd/signature-v4.go index 343962663..ede827884 100644 --- a/cmd/signature-v4.go +++ b/cmd/signature-v4.go @@ -35,6 +35,7 @@ import ( "strings" "time" + "github.com/minio/minio-go/pkg/s3utils" sha256 "github.com/minio/sha256-simd" ) @@ -92,7 +93,7 @@ func getSignedHeaders(signedHeaders http.Header) string { // func getCanonicalRequest(extractedSignedHeaders http.Header, payload, queryStr, urlPath, method string) string { rawQuery := strings.Replace(queryStr, "+", "%20", -1) - encodedPath := getURLEncodedName(urlPath) + encodedPath := s3utils.EncodePath(urlPath) canonicalRequest := strings.Join([]string{ method, encodedPath, diff --git a/cmd/test-utils_test.go b/cmd/test-utils_test.go index b940dfc0c..724bd83dd 100644 --- a/cmd/test-utils_test.go +++ b/cmd/test-utils_test.go @@ -54,6 +54,7 @@ import ( "github.com/fatih/color" "github.com/gorilla/mux" "github.com/minio/minio-go/pkg/s3signer" + "github.com/minio/minio-go/pkg/s3utils" "github.com/minio/minio/cmd/logger" "github.com/minio/minio/pkg/auth" "github.com/minio/minio/pkg/bpool" @@ -749,7 +750,7 @@ func signStreamingRequest(req *http.Request, accessKey, secretKey string, currTi req.URL.RawQuery = strings.Replace(req.URL.Query().Encode(), "+", "%20", -1) // Get canonical URI. - canonicalURI := getURLEncodedName(req.URL.Path) + canonicalURI := s3utils.EncodePath(req.URL.Path) // Get canonical request. // canonicalRequest = @@ -1104,7 +1105,7 @@ func signRequestV4(req *http.Request, accessKey, secretKey string) error { req.URL.RawQuery = strings.Replace(req.URL.Query().Encode(), "+", "%20", -1) // Get canonical URI. - canonicalURI := getURLEncodedName(req.URL.Path) + canonicalURI := s3utils.EncodePath(req.URL.Path) // Get canonical request. // canonicalRequest = @@ -1455,7 +1456,7 @@ func makeTestTargetURL(endPoint, bucketName, objectName string, queryValues url. urlStr = urlStr + bucketName + "/" } if objectName != "" { - urlStr = urlStr + getURLEncodedName(objectName) + urlStr = urlStr + s3utils.EncodePath(objectName) } if len(queryValues) > 0 { urlStr = urlStr + "?" + queryValues.Encode() diff --git a/cmd/web-handlers.go b/cmd/web-handlers.go index 1d5e41ab9..998e99cea 100644 --- a/cmd/web-handlers.go +++ b/cmd/web-handlers.go @@ -23,6 +23,7 @@ import ( "fmt" "io" "net/http" + "net/url" "os" "path" "runtime" @@ -34,6 +35,7 @@ import ( "github.com/gorilla/mux" "github.com/gorilla/rpc/v2/json2" miniogopolicy "github.com/minio/minio-go/pkg/policy" + "github.com/minio/minio-go/pkg/s3utils" "github.com/minio/minio/browser" "github.com/minio/minio/cmd/logger" "github.com/minio/minio/pkg/auth" @@ -1005,26 +1007,27 @@ func presignedGet(host, bucket, object string, expiry int64) string { if expiry < 604800 && expiry > 0 { expiryStr = strconv.FormatInt(expiry, 10) } - query := strings.Join([]string{ - "X-Amz-Algorithm=" + signV4Algorithm, - "X-Amz-Credential=" + strings.Replace(credential, "/", "%2F", -1), - "X-Amz-Date=" + dateStr, - "X-Amz-Expires=" + expiryStr, - "X-Amz-SignedHeaders=host", - }, "&") + + query := url.Values{} + query.Set("X-Amz-Algorithm", signV4Algorithm) + query.Set("X-Amz-Credential", credential) + query.Set("X-Amz-Date", dateStr) + query.Set("X-Amz-Expires", expiryStr) + query.Set("X-Amz-SignedHeaders", "host") + queryStr := s3utils.QueryEncode(query) path := "/" + path.Join(bucket, object) // "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") + canonicalRequest := getCanonicalRequest(extractedSignedHeaders, unsignedPayload, queryStr, path, "GET") stringToSign := getStringToSign(canonicalRequest, date, getScope(date, region)) signingKey := getSigningKey(secretKey, date, region) signature := getSignature(signingKey, stringToSign) // Construct the final presigned URL. - return host + getURLEncodedName(path) + "?" + query + "&" + "X-Amz-Signature=" + signature + return host + s3utils.EncodePath(path) + "?" + queryStr + "&" + "X-Amz-Signature=" + signature } // toJSONError converts regular errors into more user friendly