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.

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

HTTP request signature fails if the path contains an '*' #866

Closed
dcu opened this issue Sep 30, 2016 · 12 comments
Closed

HTTP request signature fails if the path contains an '*' #866

dcu opened this issue Sep 30, 2016 · 12 comments
Assignees
Labels
bug This issue is a bug. service-api This issue is due to a problem in a service API, not the SDK implementation.

Comments

@dcu
Copy link

dcu commented Sep 30, 2016

For example, when using elastic search service it's normal to specify the index with an "*" and in that case it fails.

This is related to edoardo849/apex-aws-signer#1

@jasdel jasdel self-assigned this Sep 30, 2016
@jasdel
Copy link
Contributor

jasdel commented Sep 30, 2016

Thanks for contacting us @dcu. Are you seeing this issue with a service client API operation, or is the signer being used standalone by its self? The Signer will escape query paths containing non alpha numeric characters. Do you have an example of what the request would look like?

@jasdel jasdel added the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. label Sep 30, 2016
@dcu
Copy link
Author

dcu commented Sep 30, 2016

@jasdel see the linked ticket. It's this code: https://github.com/edoardo849/apex-aws-signer/blob/master/signer.go#L49

If I change it to:

    originalPath := req.URL.Path
    req.URL.Path = req.URL.EscapedPath()
    _, err := t.awsSigner.Sign(req, payload, t.awsServiceName, t.awsRegion, time.Now())
    if err != nil {
        log.WithError(err).Error("Couldn't sign the request")
        return nil, err
    }

    req.URL.Path = originalPath

it works fine

@jasdel
Copy link
Contributor

jasdel commented Sep 30, 2016

Thanks for the update @dcu. Could you include a example of the request's URL that is being used in the signer? Specifically looking for what the path looks like. It would also be helpful to see the error message from the service. In addition, what the service is being used?

@dcu
Copy link
Author

dcu commented Sep 30, 2016

The url looks like this:

https://search-my-elastic-cluster-slk6s42dsd1jdm2yju3w7kddswds.us-east-1.es.amazonaws.com/logs-*/_search?pretty=true

and it's a POST

I'm using the AWS ES service with https://godoc.org/gopkg.in/olivere/elastic.v3

@jasdel
Copy link
Contributor

jasdel commented Oct 3, 2016

@dcu I'm looking into this issue. Its correct that the signer needs to have fields escaped before they are sent. To dig deeper into what the cause of the problem is though it would be very helpful if you could provide the HTTP wire log of the request(minus body) and service's error response. Specifically we're looking for any information in the service's error message that may provide more insight into why the request signature does not match what is expected.

The httputil.DumpRequest and httputil.DumpResponse are good utilities for logging these responses. This is how the SDK uses these utilities.

In addition could you enable the debug with the signer.

    v4 := NewSigner(req.Config.Credentials, func(v4 *Signer) {
        v4.Debug = aws.LogDebugWithSigning
        v4.Logger = aws.NewDefaultLogger()
    })

@jasdel jasdel added service-api This issue is due to a problem in a service API, not the SDK implementation. and removed response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. labels Oct 4, 2016
@jasdel
Copy link
Contributor

jasdel commented Oct 4, 2016

I investigated this issue some more and it looks like the SDK's signer performs escaping of the * before the request is sent. This is expected per the Elastic Cache Auth docs, and AWS v4 request signing.

The reason the signed request needs to be double encoded is because the URI /logs-*/_search will be encoded to /logs-%2A/_search by the Go HTTP client when the request is made to the service. The canonical string must contain the escaped version of this path. Meaning that the path needs to be signed as/logs-%252A/_search in the signature's canonical string. S3 is the only service where additional encoding of the canonical string's path is not needed.

I think a workaround for this is to add a configuration flag to the v4 signer which will enable double escaping of the URI. I prototyped this locally and it looks to resolve the issue.

@jasdel jasdel added the feature-request A feature should be added or improved. label Oct 4, 2016
@dcu
Copy link
Author

dcu commented Oct 4, 2016

good to know @jasdel
do you have the code in a branch or something?

@jasdel
Copy link
Contributor

jasdel commented Oct 5, 2016

@dcu I'm experimenting with https://github.com/jasdel/aws-sdk-go/tree/feature/SignEscapeStrategy, but I'm not really happy with exposing the double escaping this way. I think it would be nice to have a better solution would hide this complexity.

@jasdel
Copy link
Contributor

jasdel commented Oct 10, 2016

@dcu I created PR #885 that address this issue. The PR improves both the documentation of the signer, outlying its requirements for pre-escaping the URI path, and adds support for URL.EscapedPath. This should make it so additional escaping in your code is not needed.

jasdel added a commit to jasdel/aws-sdk-go that referenced this issue Oct 10, 2016
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 aws#866
jasdel added a commit that referenced this issue Oct 10, 2016
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
@jasdel jasdel added bug This issue is a bug. and removed feature-request A feature should be added or improved. labels Oct 10, 2016
@jasdel
Copy link
Contributor

jasdel commented Oct 10, 2016

@dcu The PR #885 was merged in that resolves the issue you were experiencing with the signer and how the signature was being calculated. Additional documentation was also added to clarify the usage of the signer outside of the SDK.

@dcu
Copy link
Author

dcu commented Oct 12, 2016

thanks! are you planning to release this change soon?

@jasdel
Copy link
Contributor

jasdel commented Oct 12, 2016

This change should be be included in the SDK's next release. I'll update here, when that release goes live.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue is a bug. service-api This issue is due to a problem in a service API, not the SDK implementation.
Projects
None yet
Development

No branches or pull requests

2 participants