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

Query string sorting in canonicalisation #1031

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

endgame
Copy link
Collaborator

@endgame endgame commented Mar 10, 2025

@nitinprakash96 please check if this works for you.

This PR fixes the ordering of query parameters during request signing. They should now sort correctly in all instances. Although I've added tests, I really don't want to break anything here. If you use Amazonka with an API that uses query parameters in its request format (potentially: IAM, SNS, SimpleDB, CloudWatch, SES, RDS, STS, Elastic Load Balancing (v1 or v2), SQS, DocumentDB, CloudSearch, Redshift, Application Auto Scaling, CloudFormation, Neptune, Elasticache (control plane), Elastic Beanstalk, or Import/Export), I'd love to hear from you.

Fixes #965

@endgame endgame added this to the 2.1 milestone Mar 10, 2025
@endgame
Copy link
Collaborator Author

endgame commented Mar 10, 2025

@nitinprakash96 I should also say, S3 Select is not being offered to new customers, so while I no longer get the signing error, I do get this:

[ServiceError] {
  service    = "S3"
  status     = 405 "Method Not Allowed"
  code       = "MethodNotAllowed"
  message    = Just "The specified method is not allowed against this resource."
  request-id = Just "E26Y9BHFKJWPTA3C"
}

I cannot test further, so please let me know if it works for you.

@endgame endgame force-pushed the query-string-sorting-in-canonicalisation branch 2 times, most recently from e1aeafd to fd01689 Compare March 10, 2025 11:01
endgame added 3 commits March 11, 2025 09:07
The "canonical requests" used in AWS signing require query parameters
to be sorted by key. In rare cases, sorting by joined `key=value` can
create invalid canonical requests.

Example: S3 Select wants to canonicalise a query string of
`select&select-type=2`. `select=` sorts after `select-type=2` because
`-` sorts before `=`. But we need to sort `select` first.
@endgame endgame force-pushed the query-string-sorting-in-canonicalisation branch from fd01689 to 23343f5 Compare March 10, 2025 23:08
@nitinprakash96
Copy link

nitinprakash96 commented Mar 11, 2025

Thanks for looking into this @endgame . Appreciate it. I can test it out over the weekend if you don't mind.

@endgame
Copy link
Collaborator Author

endgame commented Mar 11, 2025

@nitinprakash96 I'd appreciate that a lot. All I can do is check that signature verification passes, but I can't tell whether S3 Select actually works now.

@nitinprakash96
Copy link

While the signature issue is resolved by this error, I run into the following exception:

ghci> selectObjectContent ("nitin-testing") ("airtravel.csv") "select * from S3Object limit 5"
[Client Request] {
  host      = nitin-testing.s3.us-east-1.amazonaws.com:443
  secure    = True
  method    = POST
  target    = Nothing
  timeout   = ResponseTimeoutMicro 70000000
  redirects = 0
  path      = /airtravel.csv
  query     = ?select=&select-type=2
  headers   =  <headers>
  body      = non-printable strict ByteString (480 bytes)
}
[Client Response] {
  status  = 200 OK
  headers = <headers>
}
[SerializeError] {
  service = S3
  status  = 200 OK
  message = Data.Conduit.Text.decode: Error decoding stream of UTF-8 bytes. Error encountered in stream at offset 3. Encountered at byte sequence "\211\NUL\NUL\NUL"
  body    = Just non-printable lazy ByteString (467 bytes)
}
*** Exception: SerializeError (SerializeError' {abbrev = Abbrev {fromAbbrev = "S3"}, status = Status {statusCode = 200, statusMessage = "OK"}, body = Just "\NUL\NUL\NUL\211\NUL\NUL\NULU\176v\DEL\155\r:message-type\a\NUL\ENQevent\v:event-type\a\NUL\aRecords\r:content-type\a\NUL\CANapplication/octet-streamJAN,  340,  360,  417\nFEB,  318,  342,  391\nMAR,  362,  406,  419\nAPR,  348,  396,  461\nMAY,  363,  420,  472\n\226\184<\178\NUL\NUL\NUL\200\NUL\NUL\NULCS\146lY\r:message-type\a\NUL\ENQevent\v:event-type\a\NUL\ENQStats\r:content-type\a\NUL\btext/xml<Stats><BytesScanned>321</BytesScanned><BytesProcessed>321</BytesProcessed><BytesReturned>110</BytesReturned></Stats>\214\238T\191\NUL\NUL\NUL8\NUL\NUL\NUL(\193\198\132\212\r:message-type\a\NUL\ENQevent\v:event-type\a\NUL\ETXEnd\207\151\211\146", message = "Data.Conduit.Text.decode: Error decoding stream of UTF-8 bytes. Error encountered in stream at offset 3. Encountered at byte sequence \"\\211\\NUL\\NUL\\NUL\""})

But I'm not sure if this is due to this PR or the calling code is incorrect. I'm using the same snippet as described in the corresponding issue.
Also, I tried against multiple CSVs and all of them resulted in the same error. Just that the encountered byte sequence was different each time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

S3.SelectObjectContent throws SignatureDoesNotMatch exception
2 participants