Cannot upload files via a pre-signed URL to buckets with enforced server-side-encryption

[vecerek]

Describe the bug
Cannot generate a correct presigned URL for buckets with enforced server-side-encryption. Similar to the issue in aws-sdk-ruby and to this SO question.

SDK version number
v3; 1.0.0-gamma.4

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

Details of the browser/Node.js/ReactNative version
v14.11.0

To Reproduce (observed behavior)

Have a bucket policy containing a DenyIncorrectEncryptionHeader and a DenyUnEncryptedObjectUploads statement like so:

{
    "Version": "2012-10-17",
    "Statement": [
        {
            "Sid": "GrantMyUserAccess",
            "Effect": "Allow",
            "Principal": {
                "AWS": "arn:aws:iam::589470111111:user/my-bucket"
            },
            "Action": [
                "s3:PutObject",
                "s3:GetObject",
                "s3:ListBucket"
            ],
            "Resource": [
                "arn:aws:s3:::my-bucket",
                "arn:aws:s3:::my-bucket/*"
            ]
        },
        {
            "Sid": "DenyIncorrectEncryptionHeader",
            "Effect": "Deny",
            "Principal": "*",
            "Action": "s3:PutObject",
            "Resource": "arn:aws:s3:::my-bucket/*",
            "Condition": {
                "StringNotEquals": {
                    "s3:x-amz-server-side-encryption": "AES256"
                }
            }
        },
        {
            "Sid": "DenyUnEncryptedObjectUploads",
            "Effect": "Deny",
            "Principal": "*",
            "Action": "s3:PutObject",
            "Resource": "arn:aws:s3:::my-bucket/*",
            "Condition": {
                "Null": {
                    "s3:x-amz-server-side-encryption": "true"
                }
            }
        }
    ]
}

Server-side code:

import { S3 } from "@aws-sdk/client-s3";
import { NodeHttpHandler } from "@aws-sdk/node-http-handler";
import { PutObjectCommand } from "@aws-sdk/client-s3";
import { createRequest } from "@aws-sdk/util-create-request";
import { formatUrl } from "@aws-sdk/util-format-url";
import { S3RequestPresigner } from "@aws-sdk/s3-request-presigner";

const config = {
  credentials: {
    accessKeyId: "access-key",
    secretAccessKey: "secret",
  },
  encryption: "AES256",
  region: "us-east-1",
  endpoint: "the-endpoint"
};

const s3 = new S3({
    ...config,
    requestHandler: new NodeHttpHandler(),
});

const request = await createRequest(
    s3,
    new PutObjectCommand({
      Bucket: "my-bucket",
      ContentDisposition: "attachment; filename=\"foo.png\"",
      ContentLength: "531",
      ContentType: "image/png",
      Key: "my/key",
      ServerSideEncryption: "AES256",
    })
  );
  const signedRequest = new S3RequestPresigner(config).presign(request, {
    expiresIn: 3600,
  });

  const url = formatUrl(signedRequest);
  /* This produces something similar to:
      https://my-bucket.s3.amazonaws.com/my/key?
        X-Amz-Algorithm=AWS4-HMAC-SHA256&
        X-Amz-Content-Sha256=UNSIGNED-PAYLOAD&
        X-Amz-Credential=AKIAYSPZOXOPSGBVQTMA%2F20201016%2Fus-east-1%2Fs3%2Faws4_request&
        X-Amz-Date=20201016T100604Z&
        X-Amz-Expires=3600&
        X-Amz-Signature=80da2e37cf11126295969b4a1d4c1dba8ee619c6a1c483266ad7a5d1b82c2ed3&
        X-Amz-SignedHeaders=content-disposition%3Bcontent-length%3Bhost&
        x-amz-server-side-encryption=AES256&
        x-id=PutObject
  */

Client-side code:

curl -X PUT \
  -T foo.png \
  -H 'Content-Type: image/png' \
  -H 'Content-Disposition: attachment; filename="foo.png"' \
  -H 'Content-Length: 531' \
  -H 'X-Amz-Server-Side-Encryption: AES256' \
  -L "https://my-bucket.s3.amazonaws.com/my/key?X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Content-Sha256=UNSIGNED-PAYLOAD&X-Amz-Credential=AKIAYSPZOXOPSGBVQTMA%2F20201016%2Fus-east-1%2Fs3%2Faws4_request&X-Amz-Date=20201016T100604Z&X-Amz-Expires=3600&X-Amz-Signature=80da2e37cf11126295969b4a1d4c1dba8ee619c6a1c483266ad7a5d1b82c2ed3&X-Amz-SignedHeaders=content-disposition%3Bcontent-length%3Bhost&x-amz-server-side-encryption=AES256&x-id=PutObject" \
  -v 

The above command results in HTTP/1.1 403 Forbidden:

<?xml version="1.0" encoding="UTF-8"?>
<Error>
  <Code>AccessDenied</Code>
  <Message>There were headers present in the request which were not signed</Message>
  <HeadersNotSigned>x-amz-server-side-encryption</HeadersNotSigned>
  <RequestId>...</RequestId>
  <HostId>...</HostId>
</Error>

Notice that x-amz-server-side-encryption does not have the same casing as the other X-Amz query params and follows the X-Amz-SignedHeaders param. Should it probably be part of the signed headers instead of being a query param?

If I decide to not send the X-Amz-Server-Side-Encryption header along the client-side request, I get the following HTTP/1.1 403 Forbidden error:

<?xml version="1.0" encoding="UTF-8"?>
<Error>
  <Code>AccessDenied</Code>
  <Message>Access Denied</Message>
  <RequestId>...</RequestId>
  <HostId>...</HostId>
</Error>

Also, I was able to confirm that removing the two statements from the bucket policy results in a successful file upload. However, that is not an acceptable/possible workaround.

Expected behavior
I expect the curl command to result in 200 OK and an uploaded file in the bucket.

Screenshots
If applicable, add screenshots to help explain your problem.

Additional context
Add any other context about the problem here.

Dependencies:

{
  "@aws-sdk/client-s3": "1.0.0-gamma.4",
  "@aws-sdk/node-http-handler": "1.0.0-gamma.3",
  "@aws-sdk/s3-request-presigner": "1.0.0-gamma.3",
  "@aws-sdk/util-create-request": "1.0.0-gamma.3",
  "@aws-sdk/util-format-url": "1.0.0-gamma.3",
}

[vecerek]

After some digging in the request pre-signer code, I found the part that is responsible for moving x-amz-server-side-encryption from headers to the query params.

https://github.com/aws/aws-sdk-js-v3/blob/master/packages/signature-v4/src/moveHeadersToQuery.ts#L13

This code hoists all headers starting with x-amz (case insensitive match) to query params. After reading aws/aws-sdk-ruby#874 more thoroughly, my understanding is that x-amz-server-side-encryption must be signed as a header for the PUT request to be compliant with the bucket policy.

[vecerek]

Using aws-sdk v2, I get a working pre-signed URL.

const url = await s3V2.getSignedUrlPromise("putObject", {
  Bucket,
  ContentDisposition,
  ContentType,
  Key,
  ServerSideEncryption,
});

/*
  This produces something similar to:
  https://my-bucket.s3.amazonaws.com/my/key?
    Content-Type=image%2Fpng&
    X-Amz-Algorithm=AWS4-HMAC-SHA256&
    X-Amz-Credential=ACCESSKEY%2F20201019%2Fus-east-1%2Fs3%2Faws4_request&
    X-Amz-Date=20201019T145206Z&
    X-Amz-Expires=3600&
    X-Amz-Signature=ab09bf88717c30daa6fca8d8add3f0b1b2403a64ef18f175284e12fa88f9a49e&
    X-Amz-SignedHeaders=content-disposition%3Bhost%3Bx-amz-server-side-encryption&
    x-amz-server-side-encryption=AES256
*/

Notice that in the above example Content-Length is missing. It's because the v2 sign method throws an error when content-length is attempted to be signed. I'm not sure if v3 should have the same behavior or not.
Also, notice that x-amz-server-side-encryption is both in query params and in X-Amz-SignedHeaders. Should v3 retain this behavior? Is there a particular reason behind having it in both places of the URL?

[AllanZhengYP]

@vecerek Thanks a lot for the deep dive you already done. As it mentioned in conversation in ruby SDK, S3 requires x-amz-server-side-encryption to be signed and sent in request header, unlike other headers that can be moved from headers to query string. However, the current SignatureV4 signer automatically hoists all the headers to the query string. So the x-amz-server-side-encryption* headers are not signed as headers.

I will add a config to SignatureV4 allow disabling hosting the headers to query string when presign a request. The s3-request-presigner will automatically sign the x-amz-server-side-encryption* headers. I will also add a note in the README highlighting that if x-amz-server-side-encryption* headers exists, users are suppose to send the headers along with the presigned url.

[vecerek]

@AllanZhengYP do I understand correctly that the s3-request-presigner already implements the needed logic? I wasn't aware of that package at all 😅 I only remember the generic request-presigner being mentioned in the documentation.

[AllanZhengYP]

@vecerek Yes, s3-request-presigner should contain all the logics, but I'm not aware of the S3 limitation before. I will update the signer package shortly.

[github-actions]

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.