SignV2: Fix signature calculation in virtual host style #921
Conversation
vadmeste
changed the title
vadmeste
force-pushed
the
issue/920
branch
4 times, most recently
from
a0168a8 to
36b5062
Compare
pkg/s3signer/request-signature-v2.go
Outdated
| } | ||
| 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, vhost bool) *http.Request { |
There was a problem hiding this comment.
it might be better to rename vhost as isVirtualHostStyle - vhost is cryptic
There was a problem hiding this comment.
it might be better to rename vhost as isVirtualHostStyle - vhost is cryptic
Fixed.
pkg/s3signer/request-signature-v2.go
Outdated
| @@ -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, vhost bool) *http.Request { | |||
There was a problem hiding this comment.
rename vhost -> isVirtualHostStyle in this & preStringToSignV2() stringToSignV2() and writeCanonicalizedResource()
There was a problem hiding this comment.
rename vhost -> isVirtualHostStyle in this & preStringToSignV2() stringToSignV2() and writeCanonicalizedResource()
Fixed.
pkg/s3signer/utils_test.go
Outdated
| if urlPath != encodeURL2Path(&http.Request{URL: u}) { | ||
| t.Fatal("Error") | ||
| expectedPath := "/" + bucketName + "/" + o.encodedObjName | ||
| if foundPath := encodeURL2Path(&http.Request{URL: u}, true); foundPath != expectedPath { |
There was a problem hiding this comment.
we should have tests for vhost being both true and false.
There was a problem hiding this comment.
we should have tests for vhost being both true and false.
yeah, done.
| @@ -39,12 +39,12 @@ func TestSignatureCalculation(t *testing.T) { | |||
| t.Fatal("Error: anonymous credentials should not have Signature query resource.") | |||
| } | |||
|
|
|||
| req = SignV2(*req, "", "") | |||
| req = SignV2(*req, "", "", false) | |||
There was a problem hiding this comment.
we should add tests for vhost = true too.
There was a problem hiding this comment.
we should add tests for vhost = true too.
Done
In signature V2, the Resource Path that will be signed should have the form of /bucket/path/.. even in the case of vhost requests. The commit fixes the issue. The downside is to forbid automatic http redirection for V2 requests.
|
ping @krisis |
In signature V2, the Resource Path that will be signed should have
the form of /bucket/path/.. even in the case of vhost requests. The
commit fixes the issue. The downside is to forbid automatic http
redirection for V2 requests.