mirror of
				https://github.com/minio/minio.git
				synced 2025-10-29 15:55:00 -04:00 
			
		
		
		
	Fix signature v2 handling for resource names (#4965)
Previously we were wrongly adding `?` as part of the resource name, add a test case to check if this is handled properly. Thanks to @kannappanr for reproducing this. Without this change presigned URL generated with following command would fail with signature mismatch. ``` aws s3 presign s3://testbucket/functional-tests.sh ```
This commit is contained in:
		
							parent
							
								
									0bf981278e
								
							
						
					
					
						commit
						6dcfaa877c
					
				| @ -287,7 +287,7 @@ func canonicalizedAmzHeadersV2(headers http.Header) string { | ||||
| } | ||||
| 
 | ||||
| // Return canonical resource string. | ||||
| func canonicalizedResourceV2(encodedQuery string) string { | ||||
| func canonicalizedResourceV2(encodedResource, encodedQuery string) string { | ||||
| 	queries := strings.Split(encodedQuery, "&") | ||||
| 	keyval := make(map[string]string) | ||||
| 	for _, query := range queries { | ||||
| @ -316,7 +316,11 @@ func canonicalizedResourceV2(encodedQuery string) string { | ||||
| 
 | ||||
| 	// The queries will be already sorted as resourceList is sorted, if canonicalQueries | ||||
| 	// is empty strings.Join returns empty. | ||||
| 	return strings.Join(canonicalQueries, "&") | ||||
| 	canonicalQuery := strings.Join(canonicalQueries, "&") | ||||
| 	if canonicalQuery != "" { | ||||
| 		return encodedResource + "?" + canonicalQuery | ||||
| 	} | ||||
| 	return encodedResource | ||||
| } | ||||
| 
 | ||||
| // Return string to sign under two different conditions. | ||||
| @ -350,16 +354,5 @@ func getStringToSignV2(method string, encodedResource, encodedQuery string, head | ||||
| 		canonicalHeaders, | ||||
| 	}, "\n") | ||||
| 
 | ||||
| 	// For presigned signature no need to filter out based on resourceList, | ||||
| 	// just sign whatever is with the request. | ||||
| 	if expires != "" { | ||||
| 		return stringToSign + encodedResource + "?" + encodedQuery | ||||
| 	} | ||||
| 
 | ||||
| 	canonicalResource := canonicalizedResourceV2(encodedQuery) | ||||
| 	if canonicalResource != "" { | ||||
| 		return stringToSign + encodedResource + "?" + canonicalResource | ||||
| 	} | ||||
| 
 | ||||
| 	return stringToSign + encodedResource | ||||
| 	return stringToSign + canonicalizedResourceV2(encodedResource, encodedQuery) | ||||
| } | ||||
|  | ||||
| @ -105,13 +105,18 @@ func TestDoesPresignedV2SignatureMatch(t *testing.T) { | ||||
| 			}, | ||||
| 			expected: ErrSignatureDoesNotMatch, | ||||
| 		}, | ||||
| 		// (6) Should error when the signature does not match. | ||||
| 		// (6) Should not error signature matches with extra query params. | ||||
| 		{ | ||||
| 			queryParams: map[string]string{ | ||||
| 				"response-content-disposition": "attachment; filename=\"4K%2d4M.txt\"", | ||||
| 			}, | ||||
| 			expected: ErrNone, | ||||
| 		}, | ||||
| 		// (7) Should not error signature matches with no special query params. | ||||
| 		{ | ||||
| 			queryParams: map[string]string{}, | ||||
| 			expected:    ErrNone, | ||||
| 		}, | ||||
| 	} | ||||
| 
 | ||||
| 	// Run each test case individually. | ||||
|  | ||||
| @ -20,11 +20,14 @@ import ( | ||||
| 	"bufio" | ||||
| 	"bytes" | ||||
| 	"crypto/ecdsa" | ||||
| 	"crypto/hmac" | ||||
| 	crand "crypto/rand" | ||||
| 	"crypto/rsa" | ||||
| 	"crypto/sha1" | ||||
| 	"crypto/tls" | ||||
| 	"crypto/x509" | ||||
| 	"crypto/x509/pkix" | ||||
| 	"encoding/base64" | ||||
| 	"encoding/hex" | ||||
| 	"encoding/json" | ||||
| 	"encoding/pem" | ||||
| @ -880,8 +883,56 @@ func preSignV2(req *http.Request, accessKeyID, secretAccessKey string, expires i | ||||
| 	if accessKeyID == "" || secretAccessKey == "" { | ||||
| 		return errors.New("Presign cannot be generated without access and secret keys") | ||||
| 	} | ||||
| 	// Success. | ||||
| 	req = s3signer.PreSignV2(*req, accessKeyID, secretAccessKey, expires) | ||||
| 
 | ||||
| 	// FIXME: Remove following portion of code after fixing a bug in minio-go preSignV2. | ||||
| 
 | ||||
| 	d := UTCNow() | ||||
| 	// Find epoch expires when the request will expire. | ||||
| 	epochExpires := d.Unix() + expires | ||||
| 
 | ||||
| 	// Add expires header if not present. | ||||
| 	expiresStr := req.Header.Get("Expires") | ||||
| 	if expiresStr == "" { | ||||
| 		expiresStr = strconv.FormatInt(epochExpires, 10) | ||||
| 		req.Header.Set("Expires", expiresStr) | ||||
| 	} | ||||
| 
 | ||||
| 	// url.RawPath will be valid if path has any encoded characters, if not it will | ||||
| 	// be empty - in which case we need to consider url.Path (bug in net/http?) | ||||
| 	encodedResource := req.URL.RawPath | ||||
| 	encodedQuery := req.URL.RawQuery | ||||
| 	if encodedResource == "" { | ||||
| 		splits := strings.SplitN(req.URL.Path, "?", 2) | ||||
| 		encodedResource = splits[0] | ||||
| 		if len(splits) == 2 { | ||||
| 			encodedQuery = splits[1] | ||||
| 		} | ||||
| 	} | ||||
| 
 | ||||
| 	unescapedQueries, err := unescapeQueries(encodedQuery) | ||||
| 	if err != nil { | ||||
| 		return err | ||||
| 	} | ||||
| 
 | ||||
| 	// Get presigned string to sign. | ||||
| 	stringToSign := getStringToSignV2(req.Method, encodedResource, strings.Join(unescapedQueries, "&"), req.Header, expiresStr) | ||||
| 	hm := hmac.New(sha1.New, []byte(secretAccessKey)) | ||||
| 	hm.Write([]byte(stringToSign)) | ||||
| 
 | ||||
| 	// Calculate signature. | ||||
| 	signature := base64.StdEncoding.EncodeToString(hm.Sum(nil)) | ||||
| 
 | ||||
| 	query := req.URL.Query() | ||||
| 	// Handle specially for Google Cloud Storage. | ||||
| 	query.Set("AWSAccessKeyId", accessKeyID) | ||||
| 	// Fill in Expires for presigned query. | ||||
| 	query.Set("Expires", strconv.FormatInt(epochExpires, 10)) | ||||
| 
 | ||||
| 	// Encode query and save. | ||||
| 	req.URL.RawQuery = query.Encode() | ||||
| 
 | ||||
| 	// Save signature finally. | ||||
| 	req.URL.RawQuery += "&Signature=" + url.QueryEscape(signature) | ||||
| 	return nil | ||||
| } | ||||
| 
 | ||||
|  | ||||
		Loading…
	
	
			
			x
			
			
		
	
		Reference in New Issue
	
	Block a user