Fix presigned URL for access key with special characters (#6012)

Fixes #6011

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

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"}

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,

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()

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,
+	query := url.Values{}
-		"X-Amz-Credential=" + strings.Replace(credential, "/", "%2F", -1),
+	query.Set("X-Amz-Algorithm", signV4Algorithm)
-		"X-Amz-Date=" + dateStr,
+	query.Set("X-Amz-Credential", credential)
-		"X-Amz-Expires=" + expiryStr,
+	query.Set("X-Amz-Date", dateStr)
-		"X-Amz-SignedHeaders=host",
+	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