Skip to content

fix(cloudfront-signer): should not normalize URL path #7220

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

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all 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
24 changes: 24 additions & 0 deletions packages/cloudfront-signer/src/getPathnameWithDots.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import { describe, expect, it } from "vitest";

import { getPathnameWithDots } from "./getPathnameWithDots";

describe("getPathnameWithDots", () => {
const origin = "http://d1234.cloudfront.net";

it("should return if there's no pathname", () => {
expect(getPathnameWithDots(origin)).toBe("/");
expect(getPathnameWithDots(`${origin}/`)).toBe("/");
});

it("should return if there's no '/./' or '/../' in pathname", () => {
const expectedPathname = `/foo/bar/index.html`;
expect(getPathnameWithDots(`${origin}${expectedPathname}`)).toBe(expectedPathname);
});

it.each(["/./", "/../"])("should include '%s' in pathname if it exists", (folderName) => {
const expectedPathname = `/foo${folderName}bar/index.html`;
expect(getPathnameWithDots(`${origin}${expectedPathname}`)).toBe(expectedPathname);
expect(getPathnameWithDots(`${origin}${expectedPathname}?foo=bar`)).toBe(expectedPathname);
expect(getPathnameWithDots(`${origin}${expectedPathname}#foo=bar`)).toBe(expectedPathname);
});
});
20 changes: 20 additions & 0 deletions packages/cloudfront-signer/src/getPathnameWithDots.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
/**
* Returns pathname with `/./` and `/../` allowed by S3 Objects
*
* @param urlString string provided for generating the URL.
*/
export const getPathnameWithDots = (urlString: string) => {
const url = new URL(urlString);

if (url.pathname === "/") return url.pathname;

if (!urlString.includes("/./") && !urlString.includes("/../")) return url.pathname;

const startIndex = url.origin.length;
const endIndex = urlString.includes("?")
? urlString.indexOf("?")
: urlString.includes("#")
? urlString.indexOf("#")
: urlString.length;
return urlString.substring(startIndex, endIndex);
};
37 changes: 37 additions & 0 deletions packages/cloudfront-signer/src/sign.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,17 +46,21 @@ function createSignature(data: string): string {
signer.update(data);
return normalizeBase64(signer.sign(privateKey, "base64"));
}

function verifySignature(signature: string, data: string): boolean {
const verifier = createVerify("RSA-SHA1");
verifier.update(data);
return verifier.verify(privateKey, signature, "base64");
}

function encodeToBase64(str: string): string {
return normalizeBase64(Buffer.from(str).toString("base64"));
}

function normalizeBase64(str: string): string {
return str.replace(/\+/g, "-").replace(/=/g, "_").replace(/\//g, "~");
}

function denormalizeBase64(str: string): string {
return str.replace(/\-/g, "+").replace(/_/g, "=").replace(/~/g, "/");
}
Expand All @@ -78,6 +82,7 @@ describe("getSignedUrl", () => {
}
expect(result.query["foo"]).toBe("bar &=; baz");
});

it("should include url path in policy of signed URL", () => {
const url = "https://example.com/private.jpeg?foo=bar";
const result = parseUrl(
Expand Down Expand Up @@ -108,6 +113,7 @@ describe("getSignedUrl", () => {
});
expect(verifySignature(signatureQueryParam, policyStr)).toBeTruthy();
});

it("should sign a URL with a canned policy", () => {
const result = getSignedUrl({
url,
Expand Down Expand Up @@ -135,6 +141,7 @@ describe("getSignedUrl", () => {
const signatureQueryParam = denormalizeBase64(parsedUrl.query!["Signature"] as string);
expect(verifySignature(signatureQueryParam, policyStr)).toBeTruthy();
});

it("should sign a URL with a custom policy containing a start date", () => {
const result = getSignedUrl({
url,
Expand Down Expand Up @@ -166,6 +173,7 @@ describe("getSignedUrl", () => {
const signatureQueryParam = denormalizeBase64(parsedUrl.query!["Signature"] as string);
expect(verifySignature(signatureQueryParam, policyStr)).toBeTruthy();
});

it("should sign a URL with a custom policy containing an ip address", () => {
const result = getSignedUrl({
url,
Expand Down Expand Up @@ -197,6 +205,7 @@ describe("getSignedUrl", () => {
const signatureQueryParam = denormalizeBase64(parsedUrl.query!["Signature"] as string);
expect(verifySignature(signatureQueryParam, policyStr)).toBeTruthy();
});

it("should sign a URL with a custom policy containing a start date and ip address", () => {
const result = getSignedUrl({
url,
Expand Down Expand Up @@ -232,6 +241,7 @@ describe("getSignedUrl", () => {
const signatureQueryParam = denormalizeBase64(parsedUrl.query!["Signature"] as string);
expect(verifySignature(signatureQueryParam, policyStr)).toBeTruthy();
});

it("should allow an ip address with and without a mask", () => {
const baseArgs = {
url,
Expand All @@ -253,6 +263,7 @@ describe("getSignedUrl", () => {
})
).toBeTruthy();
});

it("should throw an error when the ip address is invalid", () => {
const baseArgs = {
url,
Expand Down Expand Up @@ -298,6 +309,7 @@ describe("getSignedUrl", () => {
})
).toThrow('IP address "10.0.0.256" is invalid due to invalid IP octets.');
});

it("should sign a RTMP URL", () => {
const url = "rtmp://d111111abcdef8.cloudfront.net/private-content/private.jpeg";
const result = getSignedUrl({
Expand Down Expand Up @@ -325,6 +337,7 @@ describe("getSignedUrl", () => {
);
expect(verifySignature(denormalizeBase64(signature), policyStr)).toBeTruthy();
});

it("should sign a URL with a policy provided by the user", () => {
const policy = '{"foo":"bar"}';
const result = getSignedUrl({
Expand All @@ -339,6 +352,7 @@ describe("getSignedUrl", () => {
const signatureQueryParam = denormalizeBase64(signature);
expect(verifySignature(signatureQueryParam, policy)).toBeTruthy();
});

it("should sign a URL automatically extracted from a policy provided by the user", () => {
const policy = JSON.stringify({ Statement: [{ Resource: url }] });
const result = getSignedUrl({
Expand All @@ -352,6 +366,18 @@ describe("getSignedUrl", () => {
const signatureQueryParam = denormalizeBase64(signature);
expect(verifySignature(signatureQueryParam, policy)).toBeTruthy();
});

describe("should not normalize the URL", () => {
it.each([".", ".."])("with '%s'", (folderName) => {
const urlWithFolderName = `https://d1234.cloudfront.net/public-content/${folderName}/private-content/private.jpeg`;
const policy = JSON.stringify({ Statement: [{ Resource: urlWithFolderName }] });
const result = getSignedUrl({ keyPairId, privateKey, policy, passphrase });
const signature = createSignature(policy);
expect(result.startsWith(urlWithFolderName)).toBeTruthy();
const signatureQueryParam = denormalizeBase64(signature);
expect(verifySignature(signatureQueryParam, policy)).toBeTruthy();
});
});
});

describe("getSignedCookies", () => {
Expand All @@ -376,6 +402,7 @@ describe("getSignedCookies", () => {
})
).toBeTruthy();
});

it("should throw an error when the ip address is invalid", () => {
const baseArgs = {
url,
Expand Down Expand Up @@ -421,6 +448,7 @@ describe("getSignedCookies", () => {
})
).toThrow('IP address "10.0.0.256" is invalid due to invalid IP octets.');
});

it("should be able sign cookies that contain a URL with wildcards", () => {
const url = "https://example.com/private-content/*";
const result = getSignedCookies({
Expand All @@ -444,6 +472,7 @@ describe("getSignedCookies", () => {
});
expect(verifySignature(denormalizeBase64(result["CloudFront-Signature"]), policyStr)).toBeTruthy();
});

it("should sign cookies with a canned policy", () => {
const result = getSignedCookies({
url,
Expand Down Expand Up @@ -475,6 +504,7 @@ describe("getSignedCookies", () => {
expect(result["CloudFront-Signature"]).toBe(expected["CloudFront-Signature"]);
expect(verifySignature(denormalizeBase64(result["CloudFront-Signature"]), policyStr)).toBeTruthy();
});

it("should sign cookies with a custom policy containing a start date", () => {
const result = getSignedCookies({
url,
Expand Down Expand Up @@ -510,6 +540,7 @@ describe("getSignedCookies", () => {
expect(result["CloudFront-Signature"]).toBe(expected["CloudFront-Signature"]);
expect(verifySignature(denormalizeBase64(result["CloudFront-Signature"]), policyStr)).toBeTruthy();
});

it("should sign cookies with a custom policy containing an ip address", () => {
const result = getSignedCookies({
url,
Expand Down Expand Up @@ -545,6 +576,7 @@ describe("getSignedCookies", () => {
expect(result["CloudFront-Signature"]).toBe(expected["CloudFront-Signature"]);
expect(verifySignature(denormalizeBase64(result["CloudFront-Signature"]), policyStr)).toBeTruthy();
});

it("should sign cookies with a custom policy containing a start date and ip address", () => {
const result = getSignedCookies({
url,
Expand Down Expand Up @@ -584,6 +616,7 @@ describe("getSignedCookies", () => {
expect(result["CloudFront-Signature"]).toBe(expected["CloudFront-Signature"]);
expect(verifySignature(denormalizeBase64(result["CloudFront-Signature"]), policyStr)).toBeTruthy();
});

it("should sign cookies with a policy provided by the user without a url", () => {
const policy = '{"foo":"bar"}';
const result = getSignedCookies({
Expand Down Expand Up @@ -612,6 +645,7 @@ describe("getSignedUrl- when signing a URL with a date range", () => {
const dateGreaterThanNumber = 1716034245000;
const dateObject = new Date(dateString);
const dateGreaterThanObject = new Date(dateGreaterThanString);

it("allows string input compatible with Date constructor", () => {
const epochDateLessThan = Math.round(new Date(dateString).getTime() / 1000);
const resultUrl = getSignedUrl({
Expand Down Expand Up @@ -653,6 +687,7 @@ describe("getSignedUrl- when signing a URL with a date range", () => {
expect(resultUrl).toContain(`Expires=${epochDateLessThan}`);
expect(resultCookies["CloudFront-Expires"]).toBe(epochDateLessThan);
});

it("allows Date object input", () => {
const epochDateLessThan = Math.round(dateObject.getTime() / 1000);
const resultUrl = getSignedUrl({
Expand All @@ -673,6 +708,7 @@ describe("getSignedUrl- when signing a URL with a date range", () => {
expect(resultUrl).toContain(`Expires=${epochDateLessThan}`);
expect(resultCookies["CloudFront-Expires"]).toBe(epochDateLessThan);
});

it("allows string input for date range", () => {
const result = getSignedUrl({
url,
Expand Down Expand Up @@ -736,6 +772,7 @@ describe("getSignedUrl- when signing a URL with a date range", () => {
const signatureQueryParam = denormalizeBase64(parsedUrl.query!["Signature"] as string);
expect(verifySignature(signatureQueryParam, policyStr)).toBeTruthy();
});

it("allows Date object input for date range", () => {
const result = getSignedUrl({
url,
Expand Down
16 changes: 16 additions & 0 deletions packages/cloudfront-signer/src/sign.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import { createSign } from "crypto";

import { getPathnameWithDots } from "./getPathnameWithDots";

/**
* Input type to getSignedUrl and getSignedCookies.
* @public
Expand All @@ -12,8 +14,10 @@ export type CloudfrontSignInput = CloudfrontSignInputWithParameters | Cloudfront
export type CloudfrontSignerCredentials = {
/** The ID of the Cloudfront key pair. */
keyPairId: string;

/** The content of the Cloudfront private key. */
privateKey: string | Buffer;

/** The passphrase of RSA-SHA1 key*/
passphrase?: string;
};
Expand All @@ -24,12 +28,16 @@ export type CloudfrontSignerCredentials = {
export type CloudfrontSignInputWithParameters = CloudfrontSignerCredentials & {
/** The URL string to sign. */
url: string;

/** The date string for when the signed URL or cookie can no longer be accessed */
dateLessThan: string | number | Date;

/** The date string for when the signed URL or cookie can start to be accessed. */
dateGreaterThan?: string | number | Date;

/** The IP address string to restrict signed URL access to. */
ipAddress?: string;

/**
* [policy] should not be provided when using separate
* dateLessThan, dateGreaterThan, or ipAddress inputs.
Expand All @@ -50,12 +58,16 @@ export type CloudfrontSignInputWithPolicy = CloudfrontSignerCredentials & {
* This will be ignored if calling getSignedCookies with a policy.
*/
url?: string;

/** The JSON-encoded policy string */
policy: string;

/** When using a policy, a separate dateLessThan should not be provided. */
dateLessThan?: never;

/** When using a policy, a separate dateGreaterThan should not be provided. */
dateGreaterThan?: never;

/** When using a policy, a separate ipAddress should not be provided. */
ipAddress?: never;
};
Expand All @@ -66,10 +78,13 @@ export type CloudfrontSignInputWithPolicy = CloudfrontSignerCredentials & {
export interface CloudfrontSignedCookiesOutput {
/** ID of the Cloudfront key pair. */
"CloudFront-Key-Pair-Id": string;

/** Hashed, signed, and base64-encoded version of the JSON policy. */
"CloudFront-Signature": string;

/** The unix date time for when the signed URL or cookie can no longer be accessed. */
"CloudFront-Expires"?: number;

/** Base64-encoded version of the JSON policy. */
"CloudFront-Policy"?: string;
}
Expand Down Expand Up @@ -124,6 +139,7 @@ export function getSignedUrl({
}

const newURL = new URL(baseUrl!);
newURL.pathname = getPathnameWithDots(baseUrl!);
newURL.search = Array.from(newURL.searchParams.entries())
.concat(Object.entries(cloudfrontSignBuilder.createCloudfrontAttribute()))
.filter(([, value]) => value !== undefined)
Expand Down
Loading