-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Fixed v4 signing for unordered multi value query parameters #1491
Conversation
Remove sorting of query string values when calculating v4 signing as this is not part of the spec: http://docs.aws.amazon.com/general/latest/gr/sigv4-create-canonical-request.html The spec only requires the keys, not values, to be sorted which is achieved by Query.Encode(). Also fixed TestBuildCanonicalRequest, which was incorrectly testing without this important part of the build process leading to a false success.
Thanks for putting this PR together, verifying the v4 signature, and correcting the test case. |
…ses (#1499) Reverts #1491 change since it conflicts with the AWS v4 signer test cases found in #1495. This case is not documented in the V4 spec. Additional research is needed before #1491 can be accepted. As either a implementation detail of the test cases, or an undocumented requirement of the spec.
I'm not sure if I'm understanding what the ultimate goal is, but if the use case here is that the SDK should calculate the signature correctly regardless, it appears that the Elasticsearch Kibana plugin breaks if you do not sort the field values prior to calculating the signature. The examples below vary only the search_fields fields in the query string as to the order in which they are presented. Example where SDK calculates signature incorrectly: HTTP/1.1 403 Forbidden {"message":"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."} Example where SDK calculates signature correctly: HTTP/1.1 200 OK ... (chunked encoding and JSON object of search results) I am of the opinion that the SDK should handle this automatically, that I should not have to recreate the URL with the fields sorted by key and value in the correct order so that the SDK can sign the request correctly. I believe that this is the same issue, so I am adding this as a comment here. Please advise if I need to open a new issue or if this is sufficient. |
Thank you for making me look at my glide.{yaml,lock} files. You may have just saved me a fair amount of work. It looks like the version it's using is version ddfd17e (tag v1.4.10), which is obviously pretty old. I will update to v1.14.29 and try again. If I continue to have issues, I will open a new ticket. Sorry for what may be nothing more than a false alarm at this point. |
Feel free to disregard my previous comment. Didn't work on v1.4.10, works fine on v1.14.29. Sorry for the false alarm. |
Remove sorting of query string values when calculating v4 signing as this is not part of the spec:
http://docs.aws.amazon.com/general/latest/gr/sigv4-create-canonical-request.html
The spec only requires the keys, not values, to be sorted which is achieved by Query.Encode().
Also fixed TestBuildCanonicalRequest, which was incorrectly testing without this important part of the build process leading to a false success.