From 81b9c8faa5d0bf0e9740096d1e31e84788e76e65 Mon Sep 17 00:00:00 2001 From: Jason Del Ponte Date: Tue, 4 Oct 2016 16:04:47 -0700 Subject: [PATCH 1/3] aws/signer/v4: Add support for URL.EscapedPath to signer Adds support for the URL.EscapedPath method added in Go1.5. This allows you to hint to the signer and Go HTTP client what the escaped form of the request's URI path will be. This is needed when using the AWS v4 Signer outside of the context of the SDK on http.Requests you manage. Also adds documentation to the signer that pre-escaping of the URI path is needed, and suggestions how how to do this. aws/signer/v4 TestStandaloneSign test function is an example using the request signer outside of the SDK. Fix #866 --- aws/signer/v4/functional_1_4_test.go | 40 ++++++++++ aws/signer/v4/functional_1_5_test.go | 40 ++++++++++ aws/signer/v4/functional_test.go | 43 ++++++++++ aws/signer/v4/uri_path.go | 24 ++++++ aws/signer/v4/uri_path_1_4.go | 24 ++++++ aws/signer/v4/v4.go | 114 ++++++++++++++++++++++----- aws/signer/v4/v4_test.go | 2 - 7 files changed, 265 insertions(+), 22 deletions(-) create mode 100644 aws/signer/v4/functional_1_4_test.go create mode 100644 aws/signer/v4/functional_1_5_test.go create mode 100644 aws/signer/v4/uri_path.go create mode 100644 aws/signer/v4/uri_path_1_4.go diff --git a/aws/signer/v4/functional_1_4_test.go b/aws/signer/v4/functional_1_4_test.go new file mode 100644 index 00000000000..e559838b848 --- /dev/null +++ b/aws/signer/v4/functional_1_4_test.go @@ -0,0 +1,40 @@ +// +build !go1.5 + +package v4_test + +import ( + "fmt" + "net/http" + "testing" + "time" + + "github.com/aws/aws-sdk-go/aws/signer/v4" + "github.com/aws/aws-sdk-go/awstesting/unit" + "github.com/stretchr/testify/assert" +) + +func TestStandaloneSign(t *testing.T) { + creds := unit.Session.Config.Credentials + signer := v4.NewSigner(creds) + + for _, c := range standaloneSignCases { + host := fmt.Sprintf("%s.%s.%s.amazonaws.com", + c.SubDomain, c.Region, c.Service) + + req, err := http.NewRequest("GET", fmt.Sprintf("https://%s", host), nil) + assert.NoError(t, err) + + req.URL.Path = c.OrigURI + req.URL.RawQuery = c.OrigQuery + req.URL.Opaque = fmt.Sprintf("//%s%s", host, c.EscapedURI) + opaqueURI := req.URL.Opaque + + _, err = signer.Sign(req, nil, c.Service, c.Region, time.Unix(0, 0)) + assert.NoError(t, err) + + actual := req.Header.Get("Authorization") + assert.Equal(t, c.ExpSig, actual) + assert.Equal(t, c.OrigURI, req.URL.Path) + assert.Equal(t, opaqueURI, req.URL.Opaque) + } +} diff --git a/aws/signer/v4/functional_1_5_test.go b/aws/signer/v4/functional_1_5_test.go new file mode 100644 index 00000000000..6f0d549d529 --- /dev/null +++ b/aws/signer/v4/functional_1_5_test.go @@ -0,0 +1,40 @@ +// +build go1.5 + +package v4_test + +import ( + "fmt" + "net/http" + "testing" + "time" + + "github.com/aws/aws-sdk-go/aws/signer/v4" + "github.com/aws/aws-sdk-go/awstesting/unit" + "github.com/stretchr/testify/assert" +) + +func TestStandaloneSign(t *testing.T) { + creds := unit.Session.Config.Credentials + signer := v4.NewSigner(creds) + + for _, c := range standaloneSignCases { + host := fmt.Sprintf("https://%s.%s.%s.amazonaws.com", + c.SubDomain, c.Region, c.Service) + + req, err := http.NewRequest("GET", host, nil) + assert.NoError(t, err) + + // URL.EscapedPath() will be used by the signer to get the + // escaped form of the request's URI path. + req.URL.Path = c.OrigURI + req.URL.RawQuery = c.OrigQuery + + _, err = signer.Sign(req, nil, c.Service, c.Region, time.Unix(0, 0)) + assert.NoError(t, err) + + actual := req.Header.Get("Authorization") + assert.Equal(t, c.ExpSig, actual) + assert.Equal(t, c.OrigURI, req.URL.Path) + assert.Equal(t, c.EscapedURI, req.URL.EscapedPath()) + } +} diff --git a/aws/signer/v4/functional_test.go b/aws/signer/v4/functional_test.go index e4329c6249a..6a5d5dec1d0 100644 --- a/aws/signer/v4/functional_test.go +++ b/aws/signer/v4/functional_test.go @@ -7,11 +7,28 @@ import ( "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 { + OrigURI string + OrigQuery string + Region, Service, SubDomain string + ExpSig string + EscapedURI string +}{ + { + OrigURI: `/logs-*/_search`, + OrigQuery: `pretty=true`, + Region: "us-west-2", Service: "es", SubDomain: "hostname-clusterkey", + EscapedURI: `/logs-%2A/_search`, + ExpSig: `AWS4-HMAC-SHA256 Credential=AKID/19700101/us-west-2/es/aws4_request, SignedHeaders=host;x-amz-date;x-amz-security-token, Signature=79d0760751907af16f64a537c1242416dacf51204a7dd5284492d15577973b91`, + }, +} + func TestPresignHandler(t *testing.T) { svc := s3.New(unit.Session) req, _ := svc.PutObjectRequest(&s3.PutObjectInput{ @@ -75,3 +92,29 @@ func TestPresignRequest(t *testing.T) { assert.NotContains(t, urlstr, "+") // + encoded as %20 } + +func TestStandaloneSign_CustomURIEscape(t *testing.T) { + var expectSig = `AWS4-HMAC-SHA256 Credential=AKID/19700101/us-east-1/es/aws4_request, SignedHeaders=host;x-amz-date;x-amz-security-token, Signature=0c2bebb26be55ecfb2c51068a753da9242910b551d01738f20232b116bff9c24` + var customEscapeCalled bool + + creds := unit.Session.Config.Credentials + signer := v4.NewSigner(creds, func(s *v4.Signer) { + s.URIPathEscapeFn = func(in string) string { + customEscapeCalled = true + return `/log-%252A/_search` + } + }) + + host := "https://subdomain.us-east-1.es.amazonaws.com" + req, err := http.NewRequest("GET", host, nil) + assert.NoError(t, err) + + req.URL.Path = `/log-*/_search` + + _, err = signer.Sign(req, nil, "es", "us-east-1", time.Unix(0, 0)) + assert.NoError(t, err) + + actual := req.Header.Get("Authorization") + assert.Equal(t, expectSig, actual) + assert.True(t, customEscapeCalled) +} diff --git a/aws/signer/v4/uri_path.go b/aws/signer/v4/uri_path.go new file mode 100644 index 00000000000..bd082e9d1f7 --- /dev/null +++ b/aws/signer/v4/uri_path.go @@ -0,0 +1,24 @@ +// +build go1.5 + +package v4 + +import ( + "net/url" + "strings" +) + +func getURIPath(u *url.URL) string { + var uri string + + if len(u.Opaque) > 0 { + uri = "/" + strings.Join(strings.Split(u.Opaque, "/")[3:], "/") + } else { + uri = u.EscapedPath() + } + + if len(uri) == 0 { + uri = "/" + } + + return uri +} diff --git a/aws/signer/v4/uri_path_1_4.go b/aws/signer/v4/uri_path_1_4.go new file mode 100644 index 00000000000..796604121ce --- /dev/null +++ b/aws/signer/v4/uri_path_1_4.go @@ -0,0 +1,24 @@ +// +build !go1.5 + +package v4 + +import ( + "net/url" + "strings" +) + +func getURIPath(u *url.URL) string { + var uri string + + if len(u.Opaque) > 0 { + uri = "/" + strings.Join(strings.Split(u.Opaque, "/")[3:], "/") + } else { + uri = u.Path + } + + if len(uri) == 0 { + uri = "/" + } + + return uri +} diff --git a/aws/signer/v4/v4.go b/aws/signer/v4/v4.go index eb79ded9321..33daf132f99 100644 --- a/aws/signer/v4/v4.go +++ b/aws/signer/v4/v4.go @@ -2,6 +2,47 @@ // // Provides request signing for request that need to be signed with // AWS V4 Signatures. +// +// Standalone Signer +// +// Generally using the signer outside of the SDK should not require any additional +// logic when using Go v1.5 or higher. The signer does this by taking advantage +// of the URL.EscapedURL method. If your request URI requires additional escaping +// you many need to use the URL.Opaque to define what the raw URI should be sent +// to the service as. +// +// The signer will first check the URL.Opaque field, and use its value if set. +// The signer does require the URL.Opaque field to be set in the form of: +// +// "///" +// +// // e.g. +// "//example.com/some/path" +// +// The leading "//" and hostname are required or the URL.Opaque escaping will +// not work correctly. +// +// If URL.Opaque is not set the signer will vallback to the URL.EscapedPath() +// method and using the returned value. If you're using Go v1.4 you must set +// URL.Opaque if the URI path needs escaping. +// +// AWS v4 signature validation requires that the canonical string's URI path +// element must be the URI escaped form of the HTTP request's path. +// http://docs.aws.amazon.com/general/latest/gr/sigv4-create-canonical-request.html +// +// The Go HTTP client will perform escaping automatically on the request. Some +// of these escaping may cause signature validation errors because the HTTP +// request differs from the URI path or query that the signature was generated. +// https://golang.org/pkg/net/url/#URL.EscapedPath +// +// Because of this, it is recommended that when using the signer outside of the +// SDK that explicitly escaping the request prior to being signed is preferable, +// and will help prevent signature validation errors. This can be done by setting +// the URL.Opaque or URL.RawPath. The SDK will use URL.Opaque first and then +// call URL.EscapedPath() if Opaque is not set. +// +// Test `TestStandaloneSign` provides a complete example of using the signer +// outside of the SDK and pre-escaping the URI path. package v4 import ( @@ -120,12 +161,43 @@ type Signer struct { // request's query string. DisableHeaderHoisting bool + // The func to use for escaping URI paths. Default for AWS signed request + // is to escape all but [a-zA-Z0-9-_.~]. The escaping of '+' is not a part + // of this func and is done for all requests. If not set, DefaultURIPathEscape + // will be used. + // + // Generally the escaping function does not need to be set. The only time + // this may be useful to set is if custom escaping logic is needed for the + // request's URI path escaping. + // + // http://docs.aws.amazon.com/general/latest/gr/sigv4-create-canonical-request.html + URIPathEscapeFn URIPathEscapeFunc + // currentTimeFn returns the time value which represents the current time. // This value should only be used for testing. If it is nil the default // time.Now will be used. currentTimeFn func() time.Time } +// URIPathEscapeFunc is the type for strategies that can be used by the signer +// for escaping URI path. This is not needed when signing requests generated with +// the SDK's API operations. +type URIPathEscapeFunc func(u string) string + +// DefaultURIPathEscape is the default escape URI path func the AWS signer +// will use. Performs escaping based on the AWS Signature V4 rules. +// +// http://docs.aws.amazon.com/general/latest/gr/sigv4-create-canonical-request.html +func DefaultURIPathEscape(u string) string { + return rest.EscapePath(u, false) +} + +// noURIPathEscape performs no escaping. Used for services like S3 that do not +// need double escaping for the signature validation to function properly. +func noURIPathEscape(u string) string { + return u +} + // NewSigner returns a Signer pointer configured with the credentials and optional // option values provided. If not options are provided the Signer will use its // default configuration. @@ -151,6 +223,8 @@ type signingCtx struct { ExpireTime time.Duration SignedHeaderVals http.Header + URIPathEscapeFn URIPathEscapeFunc + credValues credentials.Value isPresign bool formattedTime string @@ -236,14 +310,15 @@ func (v4 Signer) signWithBody(r *http.Request, body io.ReadSeeker, service, regi } ctx := &signingCtx{ - Request: r, - Body: body, - Query: r.URL.Query(), - Time: signTime, - ExpireTime: exp, - isPresign: exp != 0, - ServiceName: service, - Region: region, + Request: r, + Body: body, + Query: r.URL.Query(), + Time: signTime, + ExpireTime: exp, + isPresign: exp != 0, + ServiceName: service, + Region: region, + URIPathEscapeFn: v4.URIPathEscapeFn, } if ctx.isRequestSigned() { @@ -257,6 +332,10 @@ func (v4 Signer) signWithBody(r *http.Request, body io.ReadSeeker, service, regi return http.Header{}, err } + if ctx.URIPathEscapeFn == nil { + ctx.URIPathEscapeFn = DefaultURIPathEscape + } + ctx.assignAmzQueryValues() ctx.build(v4.DisableHeaderHoisting) @@ -354,6 +433,10 @@ func signSDKRequestWithCurrTime(req *request.Request, curTimeFn func() time.Time v4.Logger = req.Config.Logger v4.DisableHeaderHoisting = req.NotHoist v4.currentTimeFn = curTimeFn + if name == "s3" { + // S3 service should not have any escapting applied + v4.URIPathEscapeFn = noURIPathEscape + } }) signingTime := req.Time @@ -510,19 +593,10 @@ func (ctx *signingCtx) buildCanonicalHeaders(r rule, header http.Header) { func (ctx *signingCtx) buildCanonicalString() { ctx.Request.URL.RawQuery = strings.Replace(ctx.Query.Encode(), "+", "%20", -1) - uri := ctx.Request.URL.Opaque - if uri != "" { - uri = "/" + strings.Join(strings.Split(uri, "/")[3:], "/") - } else { - uri = ctx.Request.URL.Path - } - if uri == "" { - uri = "/" - } - if ctx.ServiceName != "s3" { - uri = rest.EscapePath(uri, false) - } + uri := getURIPath(ctx.Request.URL) + + uri = ctx.URIPathEscapeFn(uri) ctx.canonicalString = strings.Join([]string{ ctx.Request.Method, diff --git a/aws/signer/v4/v4_test.go b/aws/signer/v4/v4_test.go index fec1db18ba1..cf7a9acbc29 100644 --- a/aws/signer/v4/v4_test.go +++ b/aws/signer/v4/v4_test.go @@ -2,7 +2,6 @@ package v4 import ( "bytes" - "fmt" "io" "io/ioutil" "net/http" @@ -217,7 +216,6 @@ func TestIgnorePreResignRequestWithValidCreds(t *testing.T) { SignSDKRequest(r) sig := r.HTTPRequest.URL.Query().Get("X-Amz-Signature") - fmt.Println(sig) signSDKRequestWithCurrTime(r, func() time.Time { // Simulate one second has passed so that signature's date changes // when it is resigned. From ba9a0490775b5ce39a29aa3df2dbda958fc353f8 Mon Sep 17 00:00:00 2001 From: Jason Del Ponte Date: Mon, 10 Oct 2016 14:13:44 -0700 Subject: [PATCH 2/3] fix spelling in signer doc --- aws/signer/v4/v4.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/aws/signer/v4/v4.go b/aws/signer/v4/v4.go index 33daf132f99..2d2070ca160 100644 --- a/aws/signer/v4/v4.go +++ b/aws/signer/v4/v4.go @@ -22,9 +22,10 @@ // The leading "//" and hostname are required or the URL.Opaque escaping will // not work correctly. // -// If URL.Opaque is not set the signer will vallback to the URL.EscapedPath() +// If URL.Opaque is not set the signer will fallback to the URL.EscapedPath() // method and using the returned value. If you're using Go v1.4 you must set -// URL.Opaque if the URI path needs escaping. +// URL.Opaque if the URI path needs escaping. If URL.Opaque is not set with +// Go v1.5 the signer will fallback to URL.Path. // // AWS v4 signature validation requires that the canonical string's URI path // element must be the URI escaped form of the HTTP request's path. From 06487763a3e68be2ab1c8aa2d3d6ad25909bac5f Mon Sep 17 00:00:00 2001 From: Jason Del Ponte Date: Mon, 10 Oct 2016 14:46:44 -0700 Subject: [PATCH 3/3] Removed escape strategy func, replace with flag --- aws/signer/v4/functional_test.go | 10 ++--- aws/signer/v4/v4.go | 66 ++++++++++---------------------- 2 files changed, 24 insertions(+), 52 deletions(-) diff --git a/aws/signer/v4/functional_test.go b/aws/signer/v4/functional_test.go index 6a5d5dec1d0..f86293a6077 100644 --- a/aws/signer/v4/functional_test.go +++ b/aws/signer/v4/functional_test.go @@ -94,15 +94,11 @@ func TestPresignRequest(t *testing.T) { } func TestStandaloneSign_CustomURIEscape(t *testing.T) { - var expectSig = `AWS4-HMAC-SHA256 Credential=AKID/19700101/us-east-1/es/aws4_request, SignedHeaders=host;x-amz-date;x-amz-security-token, Signature=0c2bebb26be55ecfb2c51068a753da9242910b551d01738f20232b116bff9c24` - var customEscapeCalled bool + var expectSig = `AWS4-HMAC-SHA256 Credential=AKID/19700101/us-east-1/es/aws4_request, SignedHeaders=host;x-amz-date;x-amz-security-token, Signature=6601e883cc6d23871fd6c2a394c5677ea2b8c82b04a6446786d64cd74f520967` creds := unit.Session.Config.Credentials signer := v4.NewSigner(creds, func(s *v4.Signer) { - s.URIPathEscapeFn = func(in string) string { - customEscapeCalled = true - return `/log-%252A/_search` - } + s.DisableURIPathEscaping = true }) host := "https://subdomain.us-east-1.es.amazonaws.com" @@ -110,11 +106,11 @@ func TestStandaloneSign_CustomURIEscape(t *testing.T) { assert.NoError(t, 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) actual := req.Header.Get("Authorization") assert.Equal(t, expectSig, actual) - assert.True(t, customEscapeCalled) } diff --git a/aws/signer/v4/v4.go b/aws/signer/v4/v4.go index 2d2070ca160..986530b4019 100644 --- a/aws/signer/v4/v4.go +++ b/aws/signer/v4/v4.go @@ -7,7 +7,7 @@ // // Generally using the signer outside of the SDK should not require any additional // logic when using Go v1.5 or higher. The signer does this by taking advantage -// of the URL.EscapedURL method. If your request URI requires additional escaping +// of the URL.EscapedPath method. If your request URI requires additional escaping // you many need to use the URL.Opaque to define what the raw URI should be sent // to the service as. // @@ -162,17 +162,14 @@ type Signer struct { // request's query string. DisableHeaderHoisting bool - // The func to use for escaping URI paths. Default for AWS signed request - // is to escape all but [a-zA-Z0-9-_.~]. The escaping of '+' is not a part - // of this func and is done for all requests. If not set, DefaultURIPathEscape - // will be used. + // Disables the automatic escaping of the URI path of the request for the + // siganture's canonical string's path. For services that do not need additional + // escaping then use this to disable the signer escaping the path. // - // Generally the escaping function does not need to be set. The only time - // this may be useful to set is if custom escaping logic is needed for the - // request's URI path escaping. + // S3 is an example of a service that does not need additional escaping. // // http://docs.aws.amazon.com/general/latest/gr/sigv4-create-canonical-request.html - URIPathEscapeFn URIPathEscapeFunc + DisableURIPathEscaping bool // currentTimeFn returns the time value which represents the current time. // This value should only be used for testing. If it is nil the default @@ -180,25 +177,6 @@ type Signer struct { currentTimeFn func() time.Time } -// URIPathEscapeFunc is the type for strategies that can be used by the signer -// for escaping URI path. This is not needed when signing requests generated with -// the SDK's API operations. -type URIPathEscapeFunc func(u string) string - -// DefaultURIPathEscape is the default escape URI path func the AWS signer -// will use. Performs escaping based on the AWS Signature V4 rules. -// -// http://docs.aws.amazon.com/general/latest/gr/sigv4-create-canonical-request.html -func DefaultURIPathEscape(u string) string { - return rest.EscapePath(u, false) -} - -// noURIPathEscape performs no escaping. Used for services like S3 that do not -// need double escaping for the signature validation to function properly. -func noURIPathEscape(u string) string { - return u -} - // NewSigner returns a Signer pointer configured with the credentials and optional // option values provided. If not options are provided the Signer will use its // default configuration. @@ -224,7 +202,7 @@ type signingCtx struct { ExpireTime time.Duration SignedHeaderVals http.Header - URIPathEscapeFn URIPathEscapeFunc + DisableURIPathEscaping bool credValues credentials.Value isPresign bool @@ -311,15 +289,15 @@ func (v4 Signer) signWithBody(r *http.Request, body io.ReadSeeker, service, regi } ctx := &signingCtx{ - Request: r, - Body: body, - Query: r.URL.Query(), - Time: signTime, - ExpireTime: exp, - isPresign: exp != 0, - ServiceName: service, - Region: region, - URIPathEscapeFn: v4.URIPathEscapeFn, + Request: r, + Body: body, + Query: r.URL.Query(), + Time: signTime, + ExpireTime: exp, + isPresign: exp != 0, + ServiceName: service, + Region: region, + DisableURIPathEscaping: v4.DisableURIPathEscaping, } if ctx.isRequestSigned() { @@ -333,10 +311,6 @@ func (v4 Signer) signWithBody(r *http.Request, body io.ReadSeeker, service, regi return http.Header{}, err } - if ctx.URIPathEscapeFn == nil { - ctx.URIPathEscapeFn = DefaultURIPathEscape - } - ctx.assignAmzQueryValues() ctx.build(v4.DisableHeaderHoisting) @@ -435,8 +409,8 @@ func signSDKRequestWithCurrTime(req *request.Request, curTimeFn func() time.Time v4.DisableHeaderHoisting = req.NotHoist v4.currentTimeFn = curTimeFn if name == "s3" { - // S3 service should not have any escapting applied - v4.URIPathEscapeFn = noURIPathEscape + // S3 service should not have any escaping applied + v4.DisableURIPathEscaping = true } }) @@ -597,7 +571,9 @@ func (ctx *signingCtx) buildCanonicalString() { uri := getURIPath(ctx.Request.URL) - uri = ctx.URIPathEscapeFn(uri) + if !ctx.DisableURIPathEscaping { + uri = rest.EscapePath(uri, false) + } ctx.canonicalString = strings.Join([]string{ ctx.Request.Method,