presigning an endpoint with port doesn't work

[vlovich]

Checkboxes for prior research

• [X] I've gone through Developer Guide and API reference
• [X] I've checked AWS Forums and StackOverflow.
• [X] I've searched for previous similar issues and didn't find any solution.

Describe the bug

There was a previously filed bug https://github.com/aws/aws-sdk-js-v3/issues/2726. The fix was attempted here: https://github.com/aws/aws-sdk-js-v3/pull/2773. It seems like the bug still exists because having a non-default port number still causes the SDK to generate the wrong signature.

SDK version number

@aws-sdk/s3-request-presigner@3.145.0

Which JavaScript Runtime is this issue in?

Node.js

Details of the browser/Node.js/ReactNative version

v16.11.1

Reproduction Steps

import * as S3 from '@aws-sdk/client-s3'
import { getSignedUrl } from '@aws-sdk/s3-request-presigner'

const s3 = new S3.S3({
  region: 'us-east-1', // <-- needs to be set correctly if actually fetching
  credentials: {
    accessKeyId: '...', // <-- needs to be set correctly if actually fetching
    secretAcessKey: '...', // <-- needs to be set correctly if actually fetching
  },
  endpoint: 'https://127.0.0.1:12345', // <-- needs to be set correctly if actually fetching
  forcePathStyle: true,
})
const url = getSignedUrl(s3, new S3.GetObjectCommand({ Bucket: 'bucket', Key: 'key' })
await fetch(url)

Observed Behavior

The signature is generated WITHOUT the port number included.

Expected Behavior

The signature is generated WITH the port number included.

Possible Solution

The previous fix was:

    if (!requestToSign.headers["host"]) {
      requestToSign.headers.host = requestToSign.hostname;
+      if (requestToSign.port) {
+       requestToSign.headers.host = `${requestToSign.headers.host}:${requestToSign.port}`;
+      }
    }

I think the correct fix is:

    if (!requestToSign.headers["host"]) {
      requestToSign.headers.host = requestToSign.hostname;
    }
+      if (requestToSign.port) {
+       requestToSign.headers.host = `${requestToSign.headers.host}:${requestToSign.port}`;
+      }

In other words the port needs to be appended even if "host" appears in the header. The unit test added in that PR unfortunately doesn't catch this issue because it missed that the middleware @aws-sdk/middleware-host-header injects the host header unconditionally AFAICT (or at least it does today).

Additional Information/Context

No response

[ajredniwja]

Hey @vlovich thanks for reporting this, you can open a PR for the team to review or I can take it up soon.

[github-actions[bot]]

This issue has not received a response in 1 week. If you still think there is a problem, please leave a comment to avoid the issue from automatically closing.

[KnownRock]

For anyone who get here, this is a way to bypass this bug.

import { SignatureV4 } from '@smithy/signature-v4'
import { Sha256 } from '@aws-crypto/sha256-browser'
import { HttpRequest } from '@aws-sdk/types'
import {
  S3Client,
  ListBucketsCommand
} from '@aws-sdk/client-s3'
const s3 = new S3Client({
  region: "us-east-1",
  credentials: {
    accessKeyId: 'xxxxxx',
    secretAccessKey: 'xxxxxx',
  },
  endpoint: "http://127.0.0.1:9000",
  forcePathStyle: true,
  signer: async () => ({
    sign: async (request: HttpRequest) => {
      request.headers['host'] = `${request.hostname}:${request.port}`

      const signatureV4 = new SignatureV4({
        credentials: {
          accessKeyId: 'xxxxxx',
          secretAccessKey: 'xxxxxx',
        },
        region: 'us-east-1',
        service: 's3',
        sha256: Sha256,
      });

      const authorizatedRequest = await signatureV4.sign(request);

      return authorizatedRequest
    }
  })
});

s3.send(new ListBucketsCommand({})).then((data) => {
  console.log(data)
})

(edit: Thanks to oscarhermoso for the correction of the dependency)

[oscarhermoso]

@KnownRock - Beautiful! I replaced @aws-sdk/signature-v4 with @smithy/signature-v4 because the former is deprecated and it worked perfectly.

- import { SignatureV4 } from '@aws-sdk/signature-v4'
+ import { SignatureV4 } from '@smithy/signature-v4';

[KnownRock]

@KnownRock - Beautiful! I replaced @aws-sdk/signature-v4 with @smithy/signature-v4 because the former is deprecated and it worked perfectly.

- import { SignatureV4 } from '@aws-sdk/signature-v4'
+ import { SignatureV4 } from '@smithy/signature-v4';

Thank you for the correction to the dependencies, I'll change the above example😉

[github-actions[bot]]

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.