aws / aws-sdk-js-v3

Modularized AWS SDK for JavaScript.
Apache License 2.0
3.12k stars 578 forks source link

Canonical request host header does not include the port number #1941

Closed ffigiel closed 9 months ago

ffigiel commented 3 years ago

Describe the bug Specifying the endpoint as a string with a port number in a Client config, the result of SignatureV4.createCanonicalRequest does not include the port in the host header, which causes signature verification errors. This behavior differs from sdk v2, where the port was present and signature checks passed.

SDK version number 3.3.0

Is the issue in the browser/Node.js/ReactNative? browser

Details of the browser/Node.js/ReactNative version Chrome: 87.0.4280.88

To Reproduce (observed behavior) I debugged the signature verification errors by modifying node_modules/@aws-sdk/signature-v4/dist/es/SignatureV4.js to log the result of createCanonicalRequest function. When I ran this code:

const iam = new IAMClient({
  endpoint: 'http://example.local:32003',
  region: 'global',
  credentials: {
    accessKeyId: '...'
    secretAccessKey: '...',
  },
})
iam.send(new ListUsersCommand({}))

In the result of createCanonicalRequest, the host header did not include the port:

host:example.local

Expected behavior createCanonicalRequest should include the port in the host header if endpoint is a string with a port

Additional context I managed to get correct signature by defining endpoint as an object in IAMClient config like this:

  endpoint: {
    protocol: 'http',
    hostname: 'example.local:32003',
  },
ajredniwja commented 3 years ago

Hey @megapctr, thanks for opening this issue, Might be related to this https://github.com/aws/aws-sdk-js-v3/issues/1930 which is already related to another issue which is in teams TODO list. I can update this issue once the original issue is fixed.

DiegoRBaquero commented 3 years ago

Using Endpoint type worked. path is required, at least for the S3 client.

endpoint: {
  protocol: 'http',
  hostname: '127.0.0.1:9000',
  path: '/',
},
MichaelMitchellM commented 2 years ago

@DiegoRBaquero the work around isn't working for me :( i tried on node 16+ and i get ENOTFOUND due to the port being include don the dns.lookup() call. do you have any suggestions?

server_1         | Error: getaddrinfo ENOTFOUND minio.localhost:4000
server_1         |     at GetAddrInfoReqWrap.onlookup [as oncomplete] (node:dns:72:26) {
server_1         |   errno: -3008,
server_1         |   code: 'ENOTFOUND',
server_1         |   syscall: 'getaddrinfo',
server_1         |   hostname: 'minio.localhost:4000',
server_1         |   '$metadata': { attempts: 1, totalRetryDelay: 0 }
server_1         | }

doing wget minio.localhost:4000 from within the container works fine

DiegoRBaquero commented 2 years ago

I no longer use this, sorry

Diego

On 13/03/2022, at 4:52 PM, Michael Mitchell @.***> wrote:

 @DiegoRBaquero the work around isn't working for me :( i tried on node 16+ and i get ENOTFOUND due to the port being include don the dns.lookup() call. do you have any suggestions?

server_1 | Error: getaddrinfo ENOTFOUND minio.localhost:4000 server_1 | at GetAddrInfoReqWrap.onlookup [as oncomplete] (node:dns:72:26) { server_1 | errno: -3008, server_1 | code: 'ENOTFOUND', server_1 | syscall: 'getaddrinfo', server_1 | hostname: 'minio.localhost:4000', server_1 | '$metadata': { attempts: 1, totalRetryDelay: 0 } server_1 | } — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.

github-actions[bot] commented 9 months ago

Greetings! We’re closing this issue because it has been open a long time and hasn’t been updated in a while and may not be getting the attention it deserves. We encourage you to check if this is still an issue in the latest release and if you find that this is still a problem, please feel free to comment or open a new issue.

github-actions[bot] commented 8 months ago

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs and link to relevant comments in this thread.