Send paths on the wire in the form they were signed - #47
Open
toots wants to merge 2 commits into
Open
Conversation
The path and query were rebuilt through Uri, which decodes reserved characters and so put a literal '+' on the wire where the signature covered "%2B", making S3 answer 403 for every key containing one. Encode both in http instead, the same way the signer canonises them.
encode_string let a '%' through whenever two hex digits followed, assuming an escape somebody had already applied, but callers build their path from a raw key, so a key holding one was signed as a literal and sent as an escape; the branch had no else either, so a '%' among the last two characters was dropped outright. Pad every escape to two digits while here, since "%X" renders a byte below 0x10 as a single digit.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The request target was rebuilt with
Uri.make, which decodes reserved characters, so a key containing+was signed as%2Bbut sent as a literal+and S3 answered 403; the path and query are now encoded inHttpthe same way the signer canonises them.Util.encode_stringhad the mirror-image bug: it let a%through whenever two hex digits followed, on the assumption that the caller had already escaped the string, but every caller builds its path as/bucket/keyfrom a raw key, so%2Fwent on the wire where the signature covered a literal%-- and since that branch had noelse, a%among the last two characters was dropped from the path outright.Inline tests cover both, including the
%Xformat that rendered a byte below0x10as a single hex digit.