SignV2: Fix signature calculation in virtual host style

api-presigned.go

@@ -119,7 +119,9 @@ func (c Client) PresignedPostPolicy(p *PostPolicy) (u *url.URL, formData map[str
 		return nil, nil, err
 	}
 
-	u, err = c.makeTargetURL(bucketName, "", location, nil)
+	isVirtualHost := c.isVirtualHostStyleRequest(*c.endpointURL, bucketName)
+
+	u, err = c.makeTargetURL(bucketName, "", location, isVirtualHost, nil)
 	if err != nil {
 		return nil, nil, err
 	}

api.go

@@ -258,8 +258,7 @@ func (c *Client) redirectHeaders(req *http.Request, via []*http.Request) error {
 		}
 		switch {
 		case signerType.IsV2():
-			// Add signature version '2' authorization header.
-			req = s3signer.SignV2(*req, accessKeyID, secretAccessKey)
+			return errors.New("signature V2 cannot support redirection")
 		case signerType.IsV4():
 			req = s3signer.SignV4(*req, accessKeyID, secretAccessKey, sessionToken, getDefaultLocation(*c.endpointURL, region))
 		}
@@ -694,8 +693,11 @@ func (c Client) newRequest(method string, metadata requestMetadata) (req *http.R
 		}
 	}
 
+	// Look if target url supports virtual host.
+	isVirtualHost := c.isVirtualHostStyleRequest(*c.endpointURL, metadata.bucketName)
+
 	// Construct a new target URL.
-	targetURL, err := c.makeTargetURL(metadata.bucketName, metadata.objectName, location, metadata.queryValues)
+	targetURL, err := c.makeTargetURL(metadata.bucketName, metadata.objectName, location, isVirtualHost, metadata.queryValues)
 	if err != nil {
 		return nil, err
 	}
@@ -737,7 +739,7 @@ func (c Client) newRequest(method string, metadata requestMetadata) (req *http.R
 		}
 		if signerType.IsV2() {
 			// Presign URL with signature v2.
-			req = s3signer.PreSignV2(*req, accessKeyID, secretAccessKey, metadata.expires)
+			req = s3signer.PreSignV2(*req, accessKeyID, secretAccessKey, metadata.expires, isVirtualHost)
 		} else if signerType.IsV4() {
 			// Presign URL with signature v4.
 			req = s3signer.PreSignV4(*req, accessKeyID, secretAccessKey, sessionToken, location, metadata.expires)
@@ -783,7 +785,7 @@ func (c Client) newRequest(method string, metadata requestMetadata) (req *http.R
 	switch {
 	case signerType.IsV2():
 		// Add signature version '2' authorization header.
-		req = s3signer.SignV2(*req, accessKeyID, secretAccessKey)
+		req = s3signer.SignV2(*req, accessKeyID, secretAccessKey, isVirtualHost)
 	case metadata.objectName != "" && method == "PUT" && metadata.customHeader.Get("X-Amz-Copy-Source") == "" && !c.secure:
 		// Streaming signature is used by default for a PUT object request. Additionally we also
 		// look if the initialized client is secure, if yes then we don't need to perform
@@ -815,7 +817,7 @@ func (c Client) setUserAgent(req *http.Request) {
 }
 
 // makeTargetURL make a new target url.
-func (c Client) makeTargetURL(bucketName, objectName, bucketLocation string, queryValues url.Values) (*url.URL, error) {
+func (c Client) makeTargetURL(bucketName, objectName, bucketLocation string, isVirtualHostStyle bool, queryValues url.Values) (*url.URL, error) {
 	host := c.endpointURL.Host
 	// For Amazon S3 endpoint, try to fetch location based endpoint.
 	if s3utils.IsAmazonEndpoint(*c.endpointURL) {
@@ -854,8 +856,6 @@ func (c Client) makeTargetURL(bucketName, objectName, bucketLocation string, que
 	// Make URL only if bucketName is available, otherwise use the
 	// endpoint URL.
 	if bucketName != "" {
-		// Save if target url will have buckets which suppport virtual host.
-		isVirtualHostStyle := c.isVirtualHostStyleRequest(*c.endpointURL, bucketName)
 		// If endpoint supports virtual host style use that always.
 		// Currently only S3 and Google Cloud Storage would support
 		// virtual host style.
@@ -883,12 +883,17 @@ func (c Client) makeTargetURL(bucketName, objectName, bucketLocation string, que
 
 // returns true if virtual hosted style requests are to be used.
 func (c *Client) isVirtualHostStyleRequest(url url.URL, bucketName string) bool {
+	if bucketName == "" {
+		return false
+	}
+
 	if c.lookup == BucketLookupDNS {
 		return true
 	}
 	if c.lookup == BucketLookupPath {
 		return false
 	}
+
 	// default to virtual only for Amazon/Google  storage. In all other cases use
 	// path style requests
 	return s3utils.IsVirtualHostSupported(url, bucketName)

api_unit_test.go

@@ -190,7 +190,8 @@ func TestMakeTargetURL(t *testing.T) {
 	for i, testCase := range testCases {
 		// Initialize a Minio client
 		c, _ := New(testCase.addr, "foo", "bar", testCase.secure)
-		u, err := c.makeTargetURL(testCase.bucketName, testCase.objectName, testCase.bucketLocation, testCase.queryValues)
+		isVirtualHost := c.isVirtualHostStyleRequest(*c.endpointURL, testCase.bucketName)
+		u, err := c.makeTargetURL(testCase.bucketName, testCase.objectName, testCase.bucketLocation, isVirtualHost, testCase.queryValues)
 		// Check the returned error
 		if testCase.expectedErr == nil && err != nil {
 			t.Fatalf("Test %d: Should succeed but failed with err = %v", i+1, err)

bucket-cache.go

@@ -203,7 +203,9 @@ func (c Client) getBucketLocationRequest(bucketName string) (*http.Request, erro
 	}
 
 	if signerType.IsV2() {
-		req = s3signer.SignV2(*req, accessKeyID, secretAccessKey)
+		// Get Bucket Location calls should be always path style
+		isVirtualHost := false
+		req = s3signer.SignV2(*req, accessKeyID, secretAccessKey, isVirtualHost)
 		return req, nil
 	}
 

bucket-cache_test.go

@@ -122,7 +122,7 @@ func TestGetBucketLocationRequest(t *testing.T) {
 			req.Header.Set("X-Amz-Content-Sha256", contentSha256)
 			req = s3signer.SignV4(*req, accessKeyID, secretAccessKey, sessionToken, "us-east-1")
 		case signerType.IsV2():
-			req = s3signer.SignV2(*req, accessKeyID, secretAccessKey)
+			req = s3signer.SignV2(*req, accessKeyID, secretAccessKey, false)
 		}
 
 		return req, nil

pkg/s3signer/request-signature-v2.go

@@ -25,7 +25,6 @@ import (
 	"fmt"
 	"net/http"
 	"net/url"
-	"path/filepath"
 	"sort"
 	"strconv"
 	"strings"
@@ -40,29 +39,25 @@ const (
 )
 
 // Encode input URL path to URL encoded path.
-func encodeURL2Path(req *http.Request) (path string) {
-	reqHost := getHostAddr(req)
-	// Encode URL path.
-	if isS3, _ := filepath.Match("*.s3*.amazonaws.com", reqHost); isS3 {
-		bucketName := reqHost[:strings.LastIndex(reqHost, ".s3")]
-		path = "/" + bucketName
-		path += req.URL.Path
-		path = s3utils.EncodePath(path)
-		return
-	}
-	if strings.HasSuffix(reqHost, ".storage.googleapis.com") {
-		path = "/" + strings.TrimSuffix(reqHost, ".storage.googleapis.com")
-		path += req.URL.Path
-		path = s3utils.EncodePath(path)
-		return
+func encodeURL2Path(req *http.Request, virtualHost bool) (path string) {
+	if virtualHost {
+		reqHost := getHostAddr(req)
+		dotPos := strings.Index(reqHost, ".")
+		if dotPos > -1 {
+			bucketName := reqHost[:dotPos]
+			path = "/" + bucketName
+			path += req.URL.Path
+			path = s3utils.EncodePath(path)
+			return
+		}
 	}
 	path = s3utils.EncodePath(req.URL.Path)
 	return
 }
 
 // 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) *http.Request {
+func PreSignV2(req http.Request, accessKeyID, secretAccessKey string, expires int64, virtualHost bool) *http.Request {
 	// Presign is not needed for anonymous credentials.
 	if accessKeyID == "" || secretAccessKey == "" {
 		return &req
@@ -78,7 +73,7 @@ func PreSignV2(req http.Request, accessKeyID, secretAccessKey string, expires in
 	}
 
 	// Get presigned string to sign.
-	stringToSign := preStringToSignV2(req)
+	stringToSign := preStringToSignV2(req, virtualHost)
 	hm := hmac.New(sha1.New, []byte(secretAccessKey))
 	hm.Write([]byte(stringToSign))
 
@@ -132,7 +127,7 @@ func PostPresignSignatureV2(policyBase64, secretAccessKey string) string {
 // CanonicalizedProtocolHeaders = <described below>
 
 // SignV2 sign the request before Do() (AWS Signature Version 2).
-func SignV2(req http.Request, accessKeyID, secretAccessKey string) *http.Request {
+func SignV2(req http.Request, accessKeyID, secretAccessKey string, virtualHost bool) *http.Request {
 	// Signature calculation is not needed for anonymous credentials.
 	if accessKeyID == "" || secretAccessKey == "" {
 		return &req
@@ -147,7 +142,7 @@ func SignV2(req http.Request, accessKeyID, secretAccessKey string) *http.Request
 	}
 
 	// Calculate HMAC for secretAccessKey.
-	stringToSign := stringToSignV2(req)
+	stringToSign := stringToSignV2(req, virtualHost)
 	hm := hmac.New(sha1.New, []byte(secretAccessKey))
 	hm.Write([]byte(stringToSign))
 
@@ -172,14 +167,14 @@ func SignV2(req http.Request, accessKeyID, secretAccessKey string) *http.Request
 //	 Expires + "\n" +
 //	 CanonicalizedProtocolHeaders +
 //	 CanonicalizedResource;
-func preStringToSignV2(req http.Request) string {
+func preStringToSignV2(req http.Request, virtualHost bool) string {
 	buf := new(bytes.Buffer)
 	// Write standard headers.
 	writePreSignV2Headers(buf, req)
 	// Write canonicalized protocol headers if any.
 	writeCanonicalizedHeaders(buf, req)
 	// Write canonicalized Query resources if any.
-	writeCanonicalizedResource(buf, req)
+	writeCanonicalizedResource(buf, req, virtualHost)
 	return buf.String()
 }
 
@@ -199,14 +194,14 @@ func writePreSignV2Headers(buf *bytes.Buffer, req http.Request) {
 //	 Date + "\n" +
 //	 CanonicalizedProtocolHeaders +
 //	 CanonicalizedResource;
-func stringToSignV2(req http.Request) string {
+func stringToSignV2(req http.Request, virtualHost bool) string {
 	buf := new(bytes.Buffer)
 	// Write standard headers.
 	writeSignV2Headers(buf, req)
 	// Write canonicalized protocol headers if any.
 	writeCanonicalizedHeaders(buf, req)
 	// Write canonicalized Query resources if any.
-	writeCanonicalizedResource(buf, req)
+	writeCanonicalizedResource(buf, req, virtualHost)
 	return buf.String()
 }
 
@@ -288,11 +283,11 @@ var resourceList = []string{
 // CanonicalizedResource = [ "/" + Bucket ] +
 // 	  <HTTP-Request-URI, from the protocol name up to the query string> +
 // 	  [ sub-resource, if present. For example "?acl", "?location", "?logging", or "?torrent"];
-func writeCanonicalizedResource(buf *bytes.Buffer, req http.Request) {
+func writeCanonicalizedResource(buf *bytes.Buffer, req http.Request, virtualHost bool) {
 	// Save request URL.
 	requestURL := req.URL
 	// Get encoded URL path.
-	buf.WriteString(encodeURL2Path(&req))
+	buf.WriteString(encodeURL2Path(&req, virtualHost))
 	if requestURL.RawQuery != "" {
 		var n int
 		vals, _ := url.ParseQuery(requestURL.RawQuery)

pkg/s3signer/request-signature_test.go

@@ -24,7 +24,7 @@ import (
 )
 
 // Tests signature calculation.
-func TestSignatureCalculation(t *testing.T) {
+func TestSignatureCalculationV4(t *testing.T) {
 	req, err := http.NewRequest("GET", "https://s3.amazonaws.com", nil)
 	if err != nil {
 		t.Fatal("Error:", err)
@@ -39,16 +39,6 @@ func TestSignatureCalculation(t *testing.T) {
 		t.Fatal("Error: anonymous credentials should not have Signature query resource.")
 	}
 
-	req = SignV2(*req, "", "")
-	if req.Header.Get("Authorization") != "" {
-		t.Fatal("Error: anonymous credentials should not have Authorization header.")
-	}
-
-	req = PreSignV2(*req, "", "", 0)
-	if strings.Contains(req.URL.RawQuery, "Signature") {
-		t.Fatal("Error: anonymous credentials should not have Signature query resource.")
-	}
-
 	req = SignV4(*req, "ACCESS-KEY", "SECRET-KEY", "", "us-east-1")
 	if req.Header.Get("Authorization") == "" {
 		t.Fatal("Error: normal credentials should have Authorization header.")
@@ -58,14 +48,42 @@ func TestSignatureCalculation(t *testing.T) {
 	if !strings.Contains(req.URL.RawQuery, "X-Amz-Signature") {
 		t.Fatal("Error: normal credentials should have Signature query resource.")
 	}
+}
 
-	req = SignV2(*req, "ACCESS-KEY", "SECRET-KEY")
-	if req.Header.Get("Authorization") == "" {
-		t.Fatal("Error: normal credentials should have Authorization header.")
+func TestSignatureCalculationV2(t *testing.T) {
+
+	var testCases = []struct {
+		endpointURL string
+		virtualHost bool
+	}{
+		{endpointURL: "https://s3.amazonaws.com/", virtualHost: false},
+		{endpointURL: "https://testbucket.s3.amazonaws.com/", virtualHost: true},
 	}
 
-	req = PreSignV2(*req, "ACCESS-KEY", "SECRET-KEY", 0)
-	if !strings.Contains(req.URL.RawQuery, "Signature") {
-		t.Fatal("Error: normal credentials should not have Signature query resource.")
+	for i, testCase := range testCases {
+		req, err := http.NewRequest("GET", testCase.endpointURL, nil)
+		if err != nil {
+			t.Fatalf("Test %d, Error: %v", i+1, err)
+		}
+
+		req = SignV2(*req, "", "", testCase.virtualHost)
+		if req.Header.Get("Authorization") != "" {
+			t.Fatalf("Test %d, Error: anonymous credentials should not have Authorization header.", i+1)
+		}
+
+		req = PreSignV2(*req, "", "", 0, testCase.virtualHost)
+		if strings.Contains(req.URL.RawQuery, "Signature") {
+			t.Fatalf("Test %d, Error: anonymous credentials should not have Signature query resource.", i+1)
+		}
+
+		req = SignV2(*req, "ACCESS-KEY", "SECRET-KEY", testCase.virtualHost)
+		if req.Header.Get("Authorization") == "" {
+			t.Fatalf("Test %d, Error: normal credentials should have Authorization header.", i+1)
+		}
+
+		req = PreSignV2(*req, "ACCESS-KEY", "SECRET-KEY", 0, testCase.virtualHost)
+		if !strings.Contains(req.URL.RawQuery, "Signature") {
+			t.Fatalf("Test %d, Error: normal credentials should not have Signature query resource.", i+1)
+		}
 	}
 }

pkg/s3signer/utils_test.go

@@ -27,48 +27,59 @@ import (
 // Tests url encoding.
 func TestEncodeURL2Path(t *testing.T) {
 	type urlStrings struct {
+		virtualHost    bool
 		bucketName     string
 		objName        string
 		encodedObjName string
 	}
 
-	bucketName := "bucketName"
 	want := []urlStrings{
 		{
+			virtualHost:    true,
 			bucketName:     "bucketName",
 			objName:        "本語",
 			encodedObjName: "%E6%9C%AC%E8%AA%9E",
 		},
 		{
+			virtualHost:    true,
 			bucketName:     "bucketName",
 			objName:        "本語.1",
 			encodedObjName: "%E6%9C%AC%E8%AA%9E.1",
 		},
 		{
+			virtualHost:    true,
 			objName:        ">123>3123123",
 			bucketName:     "bucketName",
 			encodedObjName: "%3E123%3E3123123",
 		},
 		{
+			virtualHost:    true,
 			bucketName:     "bucketName",
 			objName:        "test 1 2.txt",
 			encodedObjName: "test%201%202.txt",
 		},
 		{
+			virtualHost:    false,
 			bucketName:     "test.bucketName",
 			objName:        "test++ 1.txt",
 			encodedObjName: "test%2B%2B%201.txt",
 		},
 	}
 
-	for _, o := range want {
-		u, err := url.Parse(fmt.Sprintf("https://%s.s3.amazonaws.com/%s", bucketName, o.objName))
+	for i, o := range want {
+		var hostURL string
+		if o.virtualHost {
+			hostURL = fmt.Sprintf("https://%s.s3.amazonaws.com/%s", o.bucketName, o.objName)
+		} else {
+			hostURL = fmt.Sprintf("https://s3.amazonaws.com/%s/%s", o.bucketName, o.objName)
+		}
+		u, err := url.Parse(hostURL)
 		if err != nil {
-			t.Fatal("Error:", err)
+			t.Fatalf("Test %d, Error: %v", i+1, err)
 		}
-		urlPath := "/" + bucketName + "/" + o.encodedObjName
-		if urlPath != encodeURL2Path(&http.Request{URL: u}) {
-			t.Fatal("Error")
+		expectedPath := "/" + o.bucketName + "/" + o.encodedObjName
+		if foundPath := encodeURL2Path(&http.Request{URL: u}, o.virtualHost); foundPath != expectedPath {
+			t.Fatalf("Test %d, Error: expected = `%v`, found = `%v`", i+1, expectedPath, foundPath)
 		}
 	}