Wrong canonical string created while trying multiple comma separated values in IncludedData

[krutipatel-8794]

I am trying to call getCatalogItem operation with multiple comma separated values in IncludedData parameter.
When I am including more than one value (i.e. attributes,identifiers,images,summaries), it has generated wrong canonical string and throws a signature error. (Note: It is adding 'includedData=' for each values)

@tsibelman I think this is caused by https://github.com/tsibelman/aws-signer-v4-dot-net/pull/25/files changes. I have tried to revert those changes in my code with some modification and it gives me correct Querystring (includedData=attributes%2Csummaries%2Cidentifiers%2Cimages&locale=en-US&marketplaceIds=XXXXXXXXXXXXXX)

I am using 1.0.3.0 package version of Aws4RequestSigner.

Generated canonical string using current code/package:
GET
/catalog/2020-12-01/items/
includedData=attributes&includedData=identifiers&includedData=images&includedData=summaries&locale=en-US&marketplaceIds=XXXXXXXXXXXXXX
host:sellingpartnerapi-fe.amazon.com
x-amz-access-token: <XXXXXXX.....>
x-amz-content-sha256:<XXXXXXX.....>
x-amz-date:20220420T001006Z
x-amz-security-token:<XXXXXXX.....>

Error:
{
"errors": [
{
"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.

The Canonical String for this request should have been
'GET
/catalog/2020-12-01/items/
includedData=attributes%2Csummaries%2Cidentifiers%2Cimages&locale=en-US&marketplaceIds=XXXXXXXXXXXXXX
host:sellingpartnerapi-fe.amazon.com
x-amz-access-token:<XXXXXXX.....>
x-amz-content-sha256:<XXXXXXX.....>
x-amz-date:20220419T235917Z
x-amz-security-token:<XXXXXXX.....>

host;x-amz-access-token;x-amz-content-sha256;x-amz-date;x-amz-security-token
<XXXXXXX.....>'

The String-to-Sign should have been
'AWS4-HMAC-SHA256
20220419T235917Z
20220419/us-west-2/execute-api/aws4_request
'
",
"code": "InvalidSignature"
}
]
}

See below for reference:
https://developer-docs.amazon.com/sp-api/docs/catalog-items-api-v2020-12-01-use-case-guide
Endpoint: GET https://sellingpartnerapi-na.amazon.com/catalog/2020-12-01/items/XXXXXXXXXX
?marketplaceIds=ATVPDKIKX0DER
&includedData=attributes,identifiers,images,summaries

Does anyone have any idea how I can resolve this issue?

[krutipatel-8794]

@tsibelman Could you please help me with this? or do you have any other alternatives to resolve this issue?

[krutipatel-8794]

@ronenfe Could you please help me with the issue here?

[ronenfe]

Hi, please provide your usage of the library, ommiting private data. I am not sure what is getCatalogItem and what is includedData.

[krutipatel-8794]

Hi @ronenfe,
I am facing a problem while generating URLs with query parameters having multiple values.

When I have a query parameter with more than one value, it generates a URL by adding a query parameter name for each value.

For example: Here includedData is one of the query parameters for the getCatalogItem endpoint and it has values like attributes, identifiers, images, and summaries. I have added my current result URL generated by the method GetCanonicalQueryParams() in AWS4RequestSigner.cs

Current result: (Here you can see there is "includedData=" for each value){URL}/includedData=attributes&includedData=identifiers&includedData=images&includedData=summaries&locale=en-US&marketplaceIds=XXXXXXXXXXXXXX

Expected result: (Expected result should have multiple comma separated values for one query parameter)
{URL}/includedData=attributes,summaries,identifiers,images&locale=en-US&marketplaceIds=XXXXXXXXXXXXXX

Let me know if you need more information, Thanks.

[ronenfe]

Hi, are you saying you found the problem that's causing the issue and fixed it? It's hard for me to understand and reproduce without you sending a sample code that is not working for you.

[krutipatel-8794]

Hi @ronenfe ,
The below code for generating query parameters is not working for me:

private static string GetCanonicalQueryParams(HttpRequestMessage request)
    {
      SortedDictionary<string, IEnumerable<string>> source = new SortedDictionary<string, IEnumerable<string>>((IComparer<string>) StringComparer.Ordinal);
      NameValueCollection queryString = HttpUtility.ParseQueryString(request.RequestUri.Query);
      foreach (string allKey in queryString.AllKeys)
      {
        string key = allKey;
        if (key == null)
        {
          source.Add(Uri.EscapeDataString(queryString[key]), (IEnumerable<string>) new string[1]
          {
            Uri.EscapeDataString(queryString[key]) + "="
          });
        }
        else
        {
          IEnumerable<string> strings = ((IEnumerable<string>) queryString[key].Split(',')).OrderBy<string, string>((Func<string, string>) (v => v)).Select<string, string>((Func<string, string>) (v => Uri.EscapeDataString(key) + "=" + Uri.EscapeDataString(v)));
          source.Add(Uri.EscapeDataString(key), strings);
        }
      }
      return string.Join("&", source.SelectMany<KeyValuePair<string, IEnumerable<string>>, string>((Func<KeyValuePair<string, IEnumerable<string>>, IEnumerable<string>>) (a => a.Value)));
    }

I feel like changes from https://github.com/tsibelman/aws-signer-v4-dot-net/pull/25/files are causing the issue somewhere. I might be wrong, as I am not 100% sure what scenarios you want to achieve using PR.

But you got the scenario that is not working in my case, right?

[ronenfe]

I see it's commited by John Lagnese, maybe reach out for him, I can try to debug if you give me a code sample of your usage of the library.

[ronenfe]

I see here there is no standard when sending multiple values per key in the querystring: https://stackoverflow.com/questions/24059773/correct-way-to-pass-multiple-values-for-same-parameter-name-in-get-request
If amazon requires the format for the signature to be one key and separated by comma values, maybe we should change it back to that.
But then why was it changed to that by John Lagnese? I don't know what's his github username so I can't tag him here.

[krutipatel-8794]

@jlagnese-allata Could you please have a look at this issue and the above conversation?

@ronenfe I am not getting what actually you want as a code sample of my usage of the library? I have added some information in my very first comment by omitting private data, was it not useful?

[ronenfe]

@krutipatel-8794
Yes, it's ok, I understand that your request.uri has a querystring key with multiple values and the code change made duplication of the key to each value which fails the Aws signature.

[krutipatel-8794]

Hi @ronenfe
Have you had a chance to look more into this issue and how and when we are going to fix this?
Unfortunately, @jlagnese-allata is not replying, so we are not aware of why he has changed the code in that PR?

[gao87926]

The data for calculating "Canonical Query String" needs to do uriEncode first, which means the comma should be replaced by "%2C"

https://docs.aws.amazon.com/AmazonS3/latest/API/sigv4-query-string-auth.html

[devgitprojects]

the same issue when you try to use Amazon Managed Service for Prometheus APIs for reading prometheus metrics.
Prometheus query may contain multiple comma separated values:

query={instance="987654321",label="123456789"}

https://aps-workspaces.eu-central-1.amazonaws.com/workspaces/ws-963ed5f0-12f5-4f19-8911-2b41dc12397f/api/v1/query?query={instance=\"987654321\",label=\"123456789\"}

The solution is to change GetCanonicalQueryParams - remove Split

`
private static string GetCanonicalQueryParams(HttpRequestMessage request)
{
var values = new SortedDictionary<string, IEnumerable>(StringComparer.Ordinal);

        var querystring = HttpUtility.ParseQueryString(request.RequestUri.Query);
        foreach (var key in querystring.AllKeys)
        {
            if (key == null)//Handles keys without values
            {
                values.Add(Uri.EscapeDataString(querystring[key]), new[] { $"{Uri.EscapeDataString(querystring[key])}=" });
            }
            else
            {
                // Handles multiple values per query parameter
                var queryValues = new[] { querystring[key] }
                    // Order by value alphanumerically (required for correct canonical string)
                    .OrderBy(v => v)
                    // Query params must be escaped in upper case (i.e. "%2C", not "%2c").
                    .Select(v => $"{Uri.EscapeDataString(key)}={Uri.EscapeDataString(v)}");

                values.Add(Uri.EscapeDataString(key), queryValues);
            }
        }

        var queryParams = values.SelectMany(a => a.Value);
        var canonicalQueryParams = string.Join("&", queryParams);
        return canonicalQueryParams;
    }

`

[nrcdvyskrebets]

Same issue here
When I try to sign request to https://111111111-vpce-2222222.execute-api.us-east-1.amazonaws.com/dev/me/videos?filter_tag_all_of=value1,value2 (I replaced some values), response from Gateway is 403 Forbidden.
Though same request from the Postman has correct response with correct data.
Also, doing this thing helps me string uri = $"me/videos?filter_tag_all_of={HttpUtility.UrlEncode($"{envTag},{moduleTag}")}", but then wrong value passed to Gateway and parameters basically ignored.

signing process looks strightforward

        protected override async Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, CancellationToken cancellationToken)
        {
            var creds = await _credentials.GetCredentialsAsync();
            var signer = new AWS4RequestSigner(creds.AccessKey, creds.SecretKey);

            var signedRequest = await signer.Sign(request, _awsServiceName, _region);
            return await base.SendAsync(signedRequest, cancellationToken);
        }