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

InvalidSignatureException when using custom httpclient #5099

Closed
sarahzinger opened this issue Dec 6, 2023 · 7 comments
Closed

InvalidSignatureException when using custom httpclient #5099

sarahzinger opened this issue Dec 6, 2023 · 7 comments
Labels
guidance Question that needs advice or information.

Comments

@sarahzinger
Copy link

Describe the bug

When we try to create a session with a custom httpclient like so:

sess, err := e.sessions.GetSession(awsds.SessionConfig{
  HTTPClient: Somecustomhttpclient
})

we will intermittently get back InvalidSignatureException for requests that are otherwise valid. We hit the same problem when modifying sess.Config.HTTPClient.Transport but otherwise using the sdk's default httpclient.

Expected Behavior

I would expect to be able to override the HTTPClient without hitting auth errors (or would at least expect to reliably hit the same auth error).

Current Behavior

Intermittently getting back the response:

InvalidSignatureException: The request signature we calculated does not match the signature you provided. Check your AWS Secret Access Key and signing method. Consult the service documentation for details.

despite having the same auth each time.

Using the default httpclient makes these intermittent errors go away.

Reproduction Steps

It's very inconsistent but our source code can be found here:
https://github.com/grafana/grafana/blob/main/pkg/tsdb/cloudwatch/cloudwatch.go#L260-L287

You can see where we've commented out the custom HTTPClient after several customers ran into the same errors and where we've attempted to overwrite just Transport for a new feature which unfortunately also seems to be returning the same signature exception errors.

I wonder if this is somehow related to #4496

Possible Solution

Identify what the default httpclient does that is unique, and find a way to build that on top of a custom httpclient?

Additional Information/Context

No response

SDK version used

v1.44.325

Environment details (Version of Go (go version)? OS name and version, etc.)

go version 1.21

@sarahzinger sarahzinger added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Dec 6, 2023
@lucix-aws
Copy link
Contributor

@sarahzinger -- Is your custom HTTP client implementation modifying the request in any way? (it's not immediately clear to me from the linked code)

Modifying the request within the HTTP client itself is liable to invalidate the request signature.

@lucix-aws lucix-aws added response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. and removed needs-triage This issue or PR still needs to be triaged. labels Dec 12, 2023
Copy link

This issue has not received a response in 1 week. If you want to keep this issue open, please just leave a comment below and auto-close will be canceled.

@github-actions github-actions bot added the closing-soon This issue will automatically close in 4 days unless further comments are made. label Dec 15, 2023
@dafydd-t
Copy link

Hi @lucix-aws . We send the request via a SOCKs proxy, but the request is not modified in any way. As the code shows, however, we do replace the HTTPClient's transport to use our Transport which contains the socks dialer.

I wonder, what is happening within your transport that might cause replacing it to invalidate the signature?

@github-actions github-actions bot removed closing-soon This issue will automatically close in 4 days unless further comments are made. response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. labels Dec 16, 2023
@lucix-aws lucix-aws added guidance Question that needs advice or information. and removed bug This issue is a bug. labels Dec 18, 2023
@lucix-aws
Copy link
Contributor

I wonder, what is happening within your transport that might cause replacing it to invalidate the signature?

As far as we're concerned the SDK is just using the default net/http client (not literally http.DefaultClient per se but we are using the standard library's implementation)

Note that I'm not necessarily just talking about mutation of the request, extension (generally via additional headers inserted by the proxy) can also invalidate the signature. We've seen this happen fairly often with inserting proxies into the request workflow - generally the proxy will add a forwarding header (usually X-Forwarded-For) which breaks the signature, which is calculated using all X-* headers.

@lucix-aws lucix-aws added the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. label Dec 18, 2023
@dafydd-t
Copy link

I am quite sure that the use of the SOCKS proxy dialer is completely transparent to the target of the original request: The socks proxy we use can forward any TCP requests, so it would not make any changes specific to the HTTP protocol (like headers).

Are anything other than X-* headers used to calculate the signature?

@github-actions github-actions bot removed the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. label Dec 21, 2023
@lucix-aws
Copy link
Contributor

You can read more about the sigv4 algorithm here.

Beyond that though I'm going to close this issue. If the default configuration is working, and a caller-provided custom transport causes the issue to surface, we can only assume it's a fault of that customization absent evidence to the contrary. The only advice I can give you based on this information is to compare requests as outgoing from the SDK and as outgoing from the proxy to verify whether anything is being changed.

Copy link

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
guidance Question that needs advice or information.
Projects
None yet
Development

No branches or pull requests

3 participants