aws/aws-sdk-go

Signing spec for unordered multi value query parameters uncertainty

Closed this issue · 7 comments

Hi guys you merged my PR to fix the signing to match the public spec with regards unordered multi value query parameters.

Since then I've been having a conversation with @mhart about the same issue on his javascript library and he has raised that the published spec could well be the actual source of the issue and the library might have been doing the correct thing.

The main reason for this theory is existence of an official AWS v4 test suite, which I was previous unaware of.

The suite includes a specific test to confirm that key values are ordered, hence it raises questions over the completeness of the official specification.

So the questions are:

  1. Should we revert the changes from PR #1494 (included in v1.10.34) until a conclusion is found?
  2. What is the canonical source of truth for AWS v4 signing, the test suite or the spec?
  3. If the answer to the above is the test suite then how do we get the spec updated so that it's complete and hence avoids issues like this?

In addition to this @mhart also points out addition issues with how the canonical string is calculated which also need bottoming out too.

Thanks @stevenh. I would of expected the spec and test suite to align, but in cases where they do not I can work with our internal teams to investigate and clarify the discrepancies.

I think a big area the SDK is lacking is that it does not run tests against the official test suite. We need to correct this and add the test suite validation to the SDK.

For #1494 I'll take a look that again today to see if it needs to be reverted. Did you discover services/APIs that break if the query values are not sorted as well?

I'm going to revert #1491 for now @stevenh until a clearer picture of the signing requirements are available. This work should also include integrating the test cases into the SDK.

Yep I agree with that, thanks for your time.

To answer your question no we haven't come across any services that break with unsorted values but I definitely think there is a risk of that.

mhart commented

I can certainly confirm that a number of AWS services expect, in the canonical string, query values for the same key to be sorted. Or at the very least, their error messages strongly imply this.

Here's an example (assumes AWS_ACCESS_KEY_ID declared and uses node for the date formatting)

curl 'https://lambda.us-east-1.amazonaws.com/2015-03-31/functions/?b=1&a=2&a=1' \
  -H "X-Amz-Date: $(node -p 'new Date().toISOString().slice(0, 19).replace(/[-:]/g, "")')Z" \
  -H "Authorization: AWS4-HMAC-SHA256 Credential=${AWS_ACCESS_KEY_ID}/$(node -p 'new Date().toISOString().slice(0, 10).replace(/-/g, "")')/us-east-1/lambda/aws4_request, SignedHeaders=host;x-amz-date, Signature=1283cc55db9b511d289cf9518fd97c71a958adc6a7151d50fd4cd42d2a0dcfc6"

Or with python:

curl 'https://lambda.us-east-1.amazonaws.com/2015-03-31/functions/?b=1&a=2&a=1' \
  -H "X-Amz-Date: $(python -c'import time;print(time.strftime("%Y%m%dT%H%M%SZ", time.gmtime()))')" \
  -H "Authorization: AWS4-HMAC-SHA256 Credential=${AWS_ACCESS_KEY_ID}/$(python -c'import time;print(time.strftime("%Y%m%d", time.gmtime()))')/us-east-1/lambda/aws4_request, SignedHeaders=host;x-amz-date, Signature=1283cc55db9b511d289cf9518fd97c71a958adc6a7151d50fd4cd42d2a0dcfc6"

This deliberately uses a bogus Signature and result in something like:

{"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.\n\nThe Canonical String for this request should have been\n'GET\n/2015-03-31/functions/\na=1&a=2&b=1\nhost:lambda.us-east-1.amazonaws.com\nx-amz-date:20170830T205130Z\n\nhost;x-amz-date\ne3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855'\n\nThe String-to-Sign should have been\n'AWS4-HMAC-SHA256\n20170830T205130Z\n20170830/us-east-1/lambda/aws4_request\n9e3851521dca49b1ba0bad9261e650e5eba22fa1c39f684b62e752a6c166e2da'\n"}

The relevant portion of which is:

The Canonical String for this request should have been
'GET
/2015-03-31/functions/
a=1&a=2&b=1
host:lambda.us-east-1.amazonaws.com
x-amz-date:20170830T205130Z

host;x-amz-date
e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855'

Note that the expected canonical string has sorted the query params by value for the same key (after sorting the keys) – b=1&a=2&a=1 has become a=1&a=2&b=1

mhart commented

Of course, S3 being S3, it has completely different expectations and eliminates duplicate query param values altogether:

curl 'https://s3.us-east-1.amazonaws.com/?b=1&a=2&a=1' \
  -H "X-Amz-Date: $(node -p 'new Date().toISOString().slice(0, 19).replace(/[-:]/g, "")')Z" \
  -H "X-Amz-Content-Sha256: UNSIGNED-PAYLOAD" \
  -H "Authorization: AWS4-HMAC-SHA256 Credential=${AWS_ACCESS_KEY_ID}/$(node -p 'new Date().toISOString().slice(0, 10).replace(/-/g, "")')/us-east-1/s3/aws4_request, SignedHeaders=host;x-amz-date, Signature=1283cc55db9b511d289cf9518fd97c71a958adc6a7151d50fd4cd42d2a0dcfc6"

Yields:

<?xml version="1.0" encoding="UTF-8"?>
<Error>
<Code>SignatureDoesNotMatch</Code>
<Message>The request signature we calculated does not match the signature you provided. Check your key and signing method.</Message>
...
<CanonicalRequest>GET
/
a=2&amp;b=1
host:s3.us-east-1.amazonaws.com
x-amz-date:20170830T212017Z

host;x-amz-date
UNSIGNED-PAYLOAD</CanonicalRequest>
...
</Error>

There are a bunch more issues with S3 which you can read about here (I notice some have since been addressed in the documentation, eg "you do not normalize URI paths for requests to Amazon S3")

Thanks for the additional information @stevenh. I'll raise the duplicate query parameters issue with S3 as well. Even if none of the APIs are lists, I wouldn't expect the service to invalidate the signature because of it.

After reviewing this the SDK is going to continue to sort the query values instead to the query fields when calculating the AWS Sig v4 Signature. While the Sigv4 signing spec does not explicitly state that the query values need to be sorted in addition to the keys, the test suite does exercise this expectation.

For the duplicate query parameters in S3, there doesn't seem to be any API operation which could trigger this from the SDKs. I suggest raising this issue further on the AWS S3 Forums.