Signer.Sign creates invalid signatures when host includes a port #1537
Labels
feature-request
A feature should be added or improved.
investigating
This issue is being investigated and/or work is in progress to resolve the issue.
Version of AWS SDK for Go?
1.10.48, but was in previous versions as well.
Version of Go (
go version
)?go version go1.9 darwin/amd64
What issue did you see?
http.Requests signed with Signer.Sign were not being accepted. I happened to be testing through a reverse proxy and was using a non-standard port to access the Elasticsearch service. Eventually I found that the root cause was that my host included a port and that the Signer was signing that whole value for the host header. However, the http host header value doesn't allow a port, so the signature didn't match.
Steps to reproduce
Get some code making a request to some service, ES is pretty simple to do with just an http.Request. Sign the request with Signer.Sign and ensure it works.
Then set up a reverse proxy in front of the resource. mitmproxy works well for this. Use a non-standard port. At this point, in order to connect, you'll need something like
http://127.0.0.1:9876/blarg
whereas before introducing the reverse proxy, you'd havehttp://real-domain-name/blarg
.At this point, the requests will start to fail with a 403 response.
Before signing the http.Request, set the
req.Host
toreq.URL.Hostname()
, try again. The requests should start working again.The Signer will, when getting the value to include for the
host
header value, usereq.Host
first, and if it is blank,req.URL.Host
second. Both of those will have the port included. Setting the value explicitly to thereq.URL.Hostname()
excludes the port and allows the signature to be correct again.My Diagnosis (take it with a grain of salt)
Here is the area that seems to be the problem: https://github.com/aws/aws-sdk-go/blob/master/aws/signer/v4/v4.go#L606
I would think that, at least in the second case, the thing should take
ctx.Request.URL.Hostname()
. The first case should account for it too somehow, but I'm not sure what the best strategy is...The text was updated successfully, but these errors were encountered: