fix(s3-request-presigner): skip hoisting SSE headers (#1701)

* feat(signature-v4): presigner skips headers from hosting to query

* fix(s3-request-presigner): skip hoisting SSE headers

Co-authored-by: Attila Večerek <[email protected]>

Author: AllanZhengYP

Diff for: packages/s3-request-presigner/README.md

@@ -34,6 +34,10 @@ You can get signed URL for other S3 operations too, like `PutObjectCommand`.
 `expiresIn` config from the examples above is optional. If not set, it's default
 at `900`.
 
+If your request contains server-side encryption(`SSE*`) configurations, because
+of S3 limitation, you need to send corresponding headers along with the
+presigned url. For more information, please go to [S3 SSE reference](https://docs.aws.amazon.com/AmazonS3/latest/dev/KMSUsingRESTAPI.html)
+
 If you already have a request, you can pre-sign the request following the
 section bellow.
 
@@ -51,7 +55,7 @@ const signer = new S3RequestPresigner({
   sha256: Hash.bind(null, "sha256"), // In Node.js
   //sha256: Sha256 // In browsers
 });
-const url = await signer.presign(request);
+const presigned = await signer.presign(request);
 ```
 
 ES6 Example:
@@ -66,7 +70,7 @@ const signer = new S3RequestPresigner({
   sha256: Hash.bind(null, "sha256"), // In Node.js
   //sha256: Sha256 // In browsers
 });
-const url = await signer.presign(request);
+const presigned = await signer.presign(request);
 ```
 
 To avoid redundant construction parameters when instantiating the s3 presigner,
@@ -79,3 +83,12 @@ const signer = new S3RequestPresigner({
   ...s3.config,
 });
 ```
+
+If your request contains server-side encryption(`x-amz-server-side-encryption*`)
+headers, because of S3 limitation, you need to send these headers along
+with the presigned url. That is to say, the url only from calling `formatUrl()`
+to `presigned` is not sufficient to make a request. You need to send the
+server-side encryption headers along with the url. These headers remain in the
+`presigned.headers`
+
+For more information, please go to [S3 SSE reference](https://docs.aws.amazon.com/AmazonS3/latest/dev/KMSUsingRESTAPI.html)

Diff for: packages/s3-request-presigner/src/presigner.spec.ts

@@ -87,4 +87,24 @@ describe("s3 presigner", () => {
       [EXPIRES_QUERY_PARAM]: "900",
     });
   });
+
+  it("should disable hoisting server-side-encryption headers to query", async () => {
+    const signer = new S3RequestPresigner(s3ResolvedConfig);
+    const signed = await signer.presign({
+      ...minimalRequest,
+      headers: {
+        ...minimalRequest.headers,
+        "x-amz-server-side-encryption": "kms",
+        "x-amz-server-side-encryption-customer-algorithm": "AES256",
+      },
+    });
+    expect(signed.headers).toMatchObject({
+      "x-amz-server-side-encryption": "kms",
+    });
+    const signedHeadersHeader = signed.query?.["X-Amz-SignedHeaders"];
+    const signedHeaders =
+      typeof signedHeadersHeader === "string" ? signedHeadersHeader.split(";") : signedHeadersHeader;
+    expect(signedHeaders).toContain("x-amz-server-side-encryption");
+    expect(signedHeaders).toContain("x-amz-server-side-encryption-customer-algorithm");
+  });
 });

Diff for: packages/s3-request-presigner/src/presigner.ts

@@ -31,13 +31,22 @@ export class S3RequestPresigner implements RequestPresigner {
 
   public async presign(
     requestToSign: IHttpRequest,
-    { unsignableHeaders = new Set(), ...options }: RequestPresigningArguments = {}
+    { unsignableHeaders = new Set(), unhoistableHeaders = new Set(), ...options }: RequestPresigningArguments = {}
   ): Promise<IHttpRequest> {
     unsignableHeaders.add("content-type");
+    // S3 requires SSE headers to be signed in headers instead of query
+    // See: https://github.com/aws/aws-sdk-js-v3/issues/1576
+    Object.keys(requestToSign.headers)
+      .map((header) => header.toLowerCase())
+      .filter((header) => header.startsWith("x-amz-server-side-encryption"))
+      .forEach((header) => {
+        unhoistableHeaders.add(header);
+      });
     requestToSign.headers[SHA256_HEADER] = UNSIGNED_PAYLOAD;
     return this.signer.presign(requestToSign, {
       expiresIn: 900,
       unsignableHeaders,
+      unhoistableHeaders,
       ...options,
     });
   }

Diff for: packages/signature-v4/src/SignatureV4.spec.ts

@@ -66,6 +66,30 @@ describe("SignatureV4", () => {
       });
     });
 
+    it("should sign request without hoisting some headers", async () => {
+      const { query, headers } = await signer.presign(
+        {
+          ...minimalRequest,
+          headers: {
+            ...minimalRequest.headers,
+            "x-amz-not-hoisted": "test",
+          },
+        },
+        { ...presigningOptions, unhoistableHeaders: new Set(["x-amz-not-hoisted"]) }
+      );
+      expect(query).toEqual({
+        [ALGORITHM_QUERY_PARAM]: ALGORITHM_IDENTIFIER,
+        [CREDENTIAL_QUERY_PARAM]: "foo/20000101/us-bar-1/foo/aws4_request",
+        [AMZ_DATE_QUERY_PARAM]: "20000101T000000Z",
+        [EXPIRES_QUERY_PARAM]: presigningOptions.expiresIn.toString(),
+        [SIGNED_HEADERS_QUERY_PARAM]: `${HOST_HEADER};x-amz-not-hoisted`,
+        [SIGNATURE_QUERY_PARAM]: "3c3ef586754b111e9528009710b797a07457d6a671058ba89041a06bab45f585",
+      });
+      expect(headers).toMatchObject({
+        "x-amz-not-hoisted": "test",
+      });
+    });
+
     it("should support overriding region and service in the signer instance", async () => {
       const signer = new SignatureV4({
         ...signerInit,

Diff for: packages/signature-v4/src/SignatureV4.ts

@@ -120,6 +120,7 @@ export class SignatureV4 implements RequestPresigner, RequestSigner, StringSigne
       signingDate = new Date(),
       expiresIn = 3600,
       unsignableHeaders,
+      unhoistableHeaders,
       signableHeaders,
       signingRegion,
       signingService,
@@ -135,7 +136,7 @@ export class SignatureV4 implements RequestPresigner, RequestSigner, StringSigne
     }
 
     const scope = createScope(shortDate, region, signingService ?? this.service);
-    const request = moveHeadersToQuery(prepareRequest(originalRequest));
+    const request = moveHeadersToQuery(prepareRequest(originalRequest), { unhoistableHeaders });
 
     if (credentials.sessionToken) {
       request.query[TOKEN_QUERY_PARAM] = credentials.sessionToken;

Diff for: packages/signature-v4/src/moveHeadersToQuery.spec.ts

@@ -68,4 +68,35 @@ describe("moveHeadersToQuery", () => {
       "X-Amz-Storage-Class": "STANDARD_IA",
     });
   });
+
+  it("should skip hoisting headers to the querystring supplied in unhoistedHeaders", () => {
+    const req = moveHeadersToQuery(
+      new HttpRequest({
+        ...minimalRequest,
+        headers: {
+          Host: "www.example.com",
+          "X-Amz-Website-Redirect-Location": "/index.html",
+          Foo: "bar",
+          fizz: "buzz",
+          SNAP: "crackle, pop",
+          "X-Amz-Storage-Class": "STANDARD_IA",
+        },
+      }),
+      {
+        unhoistableHeaders: new Set(["x-amz-website-redirect-location"]),
+      }
+    );
+
+    expect(req.query).toEqual({
+      "X-Amz-Storage-Class": "STANDARD_IA",
+    });
+
+    expect(req.headers).toEqual({
+      Host: "www.example.com",
+      "X-Amz-Website-Redirect-Location": "/index.html",
+      Foo: "bar",
+      fizz: "buzz",
+      SNAP: "crackle, pop",
+    });
+  });
 });

Diff for: packages/signature-v4/src/moveHeadersToQuery.ts

@@ -5,12 +5,15 @@ import { cloneRequest } from "./cloneRequest";
 /**
  * @internal
  */
-export function moveHeadersToQuery(request: HttpRequest): HttpRequest & { query: QueryParameterBag } {
+export function moveHeadersToQuery(
+  request: HttpRequest,
+  options: { unhoistableHeaders?: Set<string> } = {}
+): HttpRequest & { query: QueryParameterBag } {
   const { headers, query = {} as QueryParameterBag } =
     typeof (request as any).clone === "function" ? (request as any).clone() : cloneRequest(request);
   for (const name of Object.keys(headers)) {
     const lname = name.toLowerCase();
-    if (lname.substr(0, 6) === "x-amz-") {
+    if (lname.substr(0, 6) === "x-amz-" && !options.unhoistableHeaders?.has(lname)) {
       query[name] = headers[name];
       delete headers[name];
     }

Diff for: packages/types/src/signature.ts

@@ -52,6 +52,17 @@ export interface RequestPresigningArguments extends RequestSigningArguments {
    * The number of seconds before the presigned URL expires
    */
   expiresIn?: number;
+
+  /**
+   * A set of strings whose representing headers that should not be hoisted
+   * to presigned request's query string. If not supplied, the presigner
+   * moves all the AWS-specific headers (starting with `x-amz-`) to the request
+   * query string. If supplied, these headers remain in the presigned request's
+   * header.
+   * All headers in the provided request will have their names converted to
+   * lower case and then checked for existence in the unhoistableHeaders set.
+   */
+  unhoistableHeaders?: Set<string>;
 }
 
 export interface EventSigningArguments extends SigningArguments {