Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 3 commits into from
Nov 23, 2020
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 15 additions & 2 deletions packages/s3-request-presigner/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand All @@ -50,7 +54,7 @@ const signer = new S3Presigner({
credentials: credentialsProvider,
sha256: nodeSha256, //if the signer is used in browser, use `browserSha256` then
});
const url = await signer.presign(request);
const presigned = await signer.presign(request);
```

ES6 Example:
Expand All @@ -64,7 +68,7 @@ const signer = new S3RequestPresigner({
credentials: credentialsProvider,
sha256: nodeSha256, //if the signer is used in browser, use `browserSha256` then
});
const url = await signer.presign(request);
const presigned = await signer.presign(request);
```

To avoid redundant construction parameters when instantiating the s3 presigner,
Expand All @@ -77,3 +81,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 url. These headers remain in the
AllanZhengYP marked this conversation as resolved.
Show resolved Hide resolved
`presigned.headers`

For more information, please go to [S3 SSE reference](https://docs.aws.amazon.com/AmazonS3/latest/dev/KMSUsingRESTAPI.html)
15 changes: 15 additions & 0 deletions packages/s3-request-presigner/src/presigner.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,4 +87,19 @@ 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",
AllanZhengYP marked this conversation as resolved.
Show resolved Hide resolved
},
});
expect(signed.headers).toMatchObject({
"x-amz-server-side-encryption": "kms",
});
expect(signed.query?.["X-Amz-SignedHeaders"]).toEqual(expect.stringContaining("x-amz-server-side-encryption"));
});
});
11 changes: 10 additions & 1 deletion packages/s3-request-presigner/src/presigner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
});
}
Expand Down
24 changes: 24 additions & 0 deletions packages/signature-v4/src/SignatureV4.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
3 changes: 2 additions & 1 deletion packages/signature-v4/src/SignatureV4.ts
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,7 @@ export class SignatureV4 implements RequestPresigner, RequestSigner, StringSigne
signingDate = new Date(),
expiresIn = 3600,
unsignableHeaders,
unhoistableHeaders,
signableHeaders,
signingRegion,
signingService,
Expand All @@ -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;
Expand Down
31 changes: 31 additions & 0 deletions packages/signature-v4/src/moveHeadersToQuery.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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",
});
});
});
7 changes: 5 additions & 2 deletions packages/signature-v4/src/moveHeadersToQuery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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];
}
Expand Down
11 changes: 11 additions & 0 deletions packages/types/src/signature.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,17 @@ export interface RequestPresigningArguments extends RequestSigningArguments {
* The number of seconds before the presigned URL expires
*/
expiresIn?: number;

/**
* A set of string whose members represents headers that should not be hoisted
* to presigned request's query string. If not supplied, the presigner will
* move 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.
AllanZhengYP marked this conversation as resolved.
Show resolved Hide resolved
*/
unhoistableHeaders?: Set<string>;
}

export interface EventSigningArguments extends SigningArguments {
Expand Down