adobe-apiplatform / api-gateway-aws

AWS SDK for NGINX with Lua
Apache License 2.0
171 stars 44 forks source link

[Bug] InvalidSignatureException - Ordering issue in canonical headers #30

Open mhertogs opened 3 years ago

mhertogs commented 3 years ago

Problem

When using the AWSSignatureV4 with DynamoDB, I ran into an issue where I was sometimes successfully retrieving an item from DynamoDB, but other times, I was receiving this error:

{"__type":"com.amazon.coral.service#InvalidSignatureException","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."}

Root Cause

According to the AWS documentation, the canonical headers need to be in order (similar to the query params):

Build the canonical headers list by sorting the (lowercase) headers by character code and then iterating through the header names.

There are a few places where the library already hardcodes the two headers host;x-amz-date but in one spot, the library tries to read from a table of headers using pairs (see https://github.com/adobe-apiplatform/api-gateway-aws/blob/master/src/lua/api-gateway/aws/AwsV4Signature.lua#L61).

According to lua documentation, traversing using pairs does not preserve order of the table, and this can lead to a case where the canonical headers are in the wrong order:

x-amz-date:20210316T224733Z
host:dynamodb.us-east-1.amazonaws.com

instead of the correct ordering of:

host:dynamodb.us-east-1.amazonaws.com
x-amz-date:20210316T224733Z

Proposed Solution

Since the library is already hardcoding the Signed-Headers, the easiest fix would be to simply change from a traversal of all headers and simply look for the 'host' and 'x-amz-date' headers explicitly in order. This is the solution that I am running locally which seems to work (although I am currently testing some more to verify the fix). There are definitely more elegant solutions which are less rigid, but wanted to share the simplest possible solution in case anyone else is blocked on this.

paulpearcy commented 3 years ago

Glad to know I am not alone :)

I just stumbled across this, as well. I was only hitting it on Mac so far and when I had a session token. I wasn't hitting it on linux EC2 using IAM -> STS role, but looking at the code looks like it should be hit half the time.

My current fix looks like this (in get_hashed_canonical_request):

local function get_hashed_canonical_request(method, uri, querystring, host, amzDate, requestPayload)
    local hash = method .. '\n' ..
                 uri .. '\n' ..
                (querystring or "") .. '\n'
    -- add canonicalHeaders. Headers must be in alphabetical order
    local canonicalHeaders = "host:" .. host .. "\n" .. "x-amz-date:" .. amzDate .. "\n"
    local signedHeaders = "host;x-amz-date"

Will there ever be other headers than those two? I don't think so, so rigid is probably fine.

evgenyfadeev commented 2 years ago

@paulpearcy's fork works for me. Perhaps you could merge it?