Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

aws/signer/v4: Correct V4 presign signature to include content sha25 in URL #1469

Merged
merged 3 commits into from
Aug 15, 2017
Merged

aws/signer/v4: Correct V4 presign signature to include content sha25 in URL #1469

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
62cae32
aws/signer/v4: Correct V4 presign signature to include content sha25 …
jasdel Aug 14, 2017
175fb3e
fix unit tests for change
jasdel Aug 15, 2017
ab0b0f3
remove assert from v4 package.
jasdel Aug 15, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 9 additions & 3 deletions aws/request/request.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -269,11 +269,17 @@ func (r *Request) Presign(expireTime time.Duration) (string, error) {
return r.HTTPRequest.URL.String(), nil
}

// PresignRequest behaves just like presign, but hoists all headers and signs them.
// Also returns the signed hash back to the user
// PresignRequest behaves just like presign, with the addition of returning a
// set of headers that were signed.
//
// Returns the URL string for the API operation with signature in the query string,
// and the HTTP headers that were included in the signature. These headers must
// be included in any HTTP request made with the presigned URL.
//
// To prevent hoisting any headers to the query string set NotHoist to true on
// this Request value prior to calling PresignRequest.
func (r *Request) PresignRequest(expireTime time.Duration) (string, http.Header, error) {
r.ExpireTime = expireTime
r.NotHoist = true
r.Sign()
if r.Error != nil {
return "", nil, r.Error
Expand Down
104 changes: 75 additions & 29 deletions aws/signer/v4/functional_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,15 @@ package v4_test
import (
"net/http"
"net/url"
"reflect"
"strings"
"testing"
"time"

"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/aws/signer/v4"
"github.com/aws/aws-sdk-go/awstesting/unit"
"github.com/aws/aws-sdk-go/service/s3"
"github.com/stretchr/testify/assert"
)

var standaloneSignCases = []struct {
Expand Down Expand Up @@ -40,24 +41,43 @@ func TestPresignHandler(t *testing.T) {
req.Time = time.Unix(0, 0)
urlstr, err := req.Presign(5 * time.Minute)

assert.NoError(t, err)
if err != nil {
t.Fatalf("expect no error, got %v", err)
}

expectedHost := "bucket.s3.mock-region.amazonaws.com"
expectedDate := "19700101T000000Z"
expectedHeaders := "content-disposition;host;x-amz-acl"
expectedSig := "2d76a414208c0eac2a23ef9c834db9635ecd5a0fbb447a00ad191f82d854f55b"
expectedSig := "a46583256431b09eb45ba4af2e6286d96a9835ed13721023dc8076dfdcb90fcb"
expectedCred := "AKID/19700101/mock-region/s3/aws4_request"

u, _ := url.Parse(urlstr)
urlQ := u.Query()
assert.Equal(t, expectedHost, u.Host)
assert.Equal(t, expectedSig, urlQ.Get("X-Amz-Signature"))
assert.Equal(t, expectedCred, urlQ.Get("X-Amz-Credential"))
assert.Equal(t, expectedHeaders, urlQ.Get("X-Amz-SignedHeaders"))
assert.Equal(t, expectedDate, urlQ.Get("X-Amz-Date"))
assert.Equal(t, "300", urlQ.Get("X-Amz-Expires"))

assert.NotContains(t, urlstr, "+") // + encoded as %20
if e, a := expectedHost, u.Host; e != a {
t.Errorf("expect %v, got %v", e, a)
}
if e, a := expectedSig, urlQ.Get("X-Amz-Signature"); e != a {
t.Errorf("expect %v, got %v", e, a)
}
if e, a := expectedCred, urlQ.Get("X-Amz-Credential"); e != a {
t.Errorf("expect %v, got %v", e, a)
}
if e, a := expectedHeaders, urlQ.Get("X-Amz-SignedHeaders"); e != a {
t.Errorf("expect %v, got %v", e, a)
}
if e, a := expectedDate, urlQ.Get("X-Amz-Date"); e != a {
t.Errorf("expect %v, got %v", e, a)
}
if e, a := "300", urlQ.Get("X-Amz-Expires"); e != a {
t.Errorf("expect %v, got %v", e, a)
}
if e, a := "UNSIGNED-PAYLOAD", urlQ.Get("X-Amz-Content-Sha256"); e != a {
t.Errorf("expect %v, got %v", e, a)
}

if e, a := "+", urlstr; strings.Contains(a, e) { // + encoded as %20
t.Errorf("expect %v not to be in %v", e, a)
}
}

func TestPresignRequest(t *testing.T) {
Expand All @@ -71,30 +91,50 @@ func TestPresignRequest(t *testing.T) {
req.Time = time.Unix(0, 0)
urlstr, headers, err := req.PresignRequest(5 * time.Minute)

assert.NoError(t, err)
if err != nil {
t.Fatalf("expect no error, got %v", err)
}

expectedHost := "bucket.s3.mock-region.amazonaws.com"
expectedDate := "19700101T000000Z"
expectedHeaders := "content-disposition;host;x-amz-acl;x-amz-content-sha256"
expectedSig := "a5b2b500dfbf2eab5b4f55bec3e3752e04536ea1d5c047aa93bc9f1130a72cd2"
expectedHeaders := "content-disposition;host;x-amz-acl"
expectedSig := "a46583256431b09eb45ba4af2e6286d96a9835ed13721023dc8076dfdcb90fcb"
expectedCred := "AKID/19700101/mock-region/s3/aws4_request"
expectedHeaderMap := http.Header{
"x-amz-acl": []string{"public-read"},
"content-disposition": []string{"a+b c$d"},
"x-amz-content-sha256": []string{"UNSIGNED-PAYLOAD"},
"x-amz-acl": []string{"public-read"},
"content-disposition": []string{"a+b c$d"},
}

u, _ := url.Parse(urlstr)
urlQ := u.Query()
assert.Equal(t, expectedHost, u.Host)
assert.Equal(t, expectedSig, urlQ.Get("X-Amz-Signature"))
assert.Equal(t, expectedCred, urlQ.Get("X-Amz-Credential"))
assert.Equal(t, expectedHeaders, urlQ.Get("X-Amz-SignedHeaders"))
assert.Equal(t, expectedDate, urlQ.Get("X-Amz-Date"))
assert.Equal(t, expectedHeaderMap, headers)
assert.Equal(t, "300", urlQ.Get("X-Amz-Expires"))

assert.NotContains(t, urlstr, "+") // + encoded as %20
if e, a := expectedHost, u.Host; e != a {
t.Errorf("expect %v, got %v", e, a)
}
if e, a := expectedSig, urlQ.Get("X-Amz-Signature"); e != a {
t.Errorf("expect %v, got %v", e, a)
}
if e, a := expectedCred, urlQ.Get("X-Amz-Credential"); e != a {
t.Errorf("expect %v, got %v", e, a)
}
if e, a := expectedHeaders, urlQ.Get("X-Amz-SignedHeaders"); e != a {
t.Errorf("expect %v, got %v", e, a)
}
if e, a := expectedDate, urlQ.Get("X-Amz-Date"); e != a {
t.Errorf("expect %v, got %v", e, a)
}
if e, a := expectedHeaderMap, headers; !reflect.DeepEqual(e, a) {
t.Errorf("expect %v, got %v", e, a)
}
if e, a := "300", urlQ.Get("X-Amz-Expires"); e != a {
t.Errorf("expect %v, got %v", e, a)
}
if e, a := "UNSIGNED-PAYLOAD", urlQ.Get("X-Amz-Content-Sha256"); e != a {
t.Errorf("expect %v, got %v", e, a)
}

if e, a := "+", urlstr; strings.Contains(a, e) { // + encoded as %20
t.Errorf("expect %v not to be in %v", e, a)
}
}

func TestStandaloneSign_CustomURIEscape(t *testing.T) {
Expand All @@ -107,14 +147,20 @@ func TestStandaloneSign_CustomURIEscape(t *testing.T) {

host := "https://subdomain.us-east-1.es.amazonaws.com"
req, err := http.NewRequest("GET", host, nil)
assert.NoError(t, err)
if err != nil {
t.Fatalf("expect no error, got %v", err)
}

req.URL.Path = `/log-*/_search`
req.URL.Opaque = "//subdomain.us-east-1.es.amazonaws.com/log-%2A/_search"

_, err = signer.Sign(req, nil, "es", "us-east-1", time.Unix(0, 0))
assert.NoError(t, err)
if err != nil {
t.Fatalf("expect no error, got %v", err)
}

actual := req.Header.Get("Authorization")
assert.Equal(t, expectSig, actual)
if e, a := expectSig, actual; e != a {
t.Errorf("expect %v, got %v", e, a)
}
}
3 changes: 2 additions & 1 deletion aws/signer/v4/v4.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -502,6 +502,8 @@ func (ctx *signingCtx) build(disableHeaderHoisting bool) {
ctx.buildTime() // no depends
ctx.buildCredentialString() // no depends

ctx.buildBodyDigest()

unsignedHeaders := ctx.Request.Header
if ctx.isPresign {
if !disableHeaderHoisting {
Expand All @@ -513,7 +515,6 @@ func (ctx *signingCtx) build(disableHeaderHoisting bool) {
}
}

ctx.buildBodyDigest()
ctx.buildCanonicalHeaders(ignoredHeaders, unsignedHeaders)
ctx.buildCanonicalString() // depends on canon headers / signed headers
ctx.buildStringToSign() // depends on canon string
Expand Down
Loading