Make IP/user-agent validation in CDN work

src/util/Signing.ts

@@ -19,14 +19,105 @@
 import { Config } from "@spacebar/util";
 import { createHmac, timingSafeEqual } from "crypto";
 import ms, { StringValue } from "ms";
-import { ParsedQs } from "qs";
-import { Request } from "express";
-import attachments from "../cdn/routes/attachments";
+import * as console from "node:console";
 
-export const getUrlSignature = (
-	path: string,
-	req: Request,
-) => {
+export class NewUrlUserSignatureData {
+	ip?: string;
+	userAgent?: string;
+	constructor(data: NewUrlUserSignatureData) {
+		this.ip = data.ip;
+		this.userAgent = data.userAgent;
+	}
+}
+
+export class NewUrlSignatureData extends NewUrlUserSignatureData {
+	path?: string;
+	url?: string;
+
+	constructor(data: NewUrlSignatureData) {
+		super(data);
+		this.path = data.path;
+		this.url = data.url;
+		if (!this.path && !this.url) {
+			throw new Error("Either path or url must be provided for URL signing");
+		}
+		if (this.path && this.url) {
+			console.warn(
+				"[Signing] Both path and url are provided, using path for signing",
+			);
+		}
+		if (this.url) {
+			try {
+				const parsedUrl = new URL(this.url);
+				this.path = parsedUrl.pathname;
+			} catch (e) {
+				throw new Error("Invalid URL provided for signing: " + this.url);
+			}
+		}
+	}
+}
+
+export class UrlSignatureData extends NewUrlSignatureData {
+	issuedAt: string;
+	expiresAt: string;
+
+	constructor(data: UrlSignatureData) {
+		super(data);
+		this.issuedAt = data.issuedAt;
+		this.expiresAt = data.expiresAt;
+	}
+}
+
+export class UrlSignResult {
+	path: string;
+	hash: string;
+	issuedAt: string;
+	expiresAt: string;
+
+	/*
+	 * @param data {UrlSignResult}
+	 */
+	constructor(data: any) {
+		for (const key in data) {
+			// @ts-ignore TS7053 - We dont care about string indexing a class
+			this[key] = data[key];
+		}
+	}
+
+	applyToUrl(url: URL | string): URL {
+		if (typeof url === "string") {
+			url = new URL(url);
+		}
+		url.searchParams.set("ex", this.expiresAt);
+		url.searchParams.set("is", this.issuedAt);
+		url.searchParams.set("hm", this.hash);
+		return url;
+	}
+
+	static fromUrl(url: URL | string): UrlSignResult {
+		if (typeof url === "string") {
+			console.debug("[Signing] Parsing URL from string:", url);
+			url = new URL(url);
+		}
+		console.debug("[Signing] Parsing URL from URL object:", url.toString());
+		const ex = url.searchParams.get("ex");
+		const is = url.searchParams.get("is");
+		const hm = url.searchParams.get("hm");
+
+		if (!ex || !is || !hm) {
+			throw new Error("Invalid URL signature parameters");
+		}
+
+		return new UrlSignResult({
+			path: url.pathname,
+			issuedAt: is,
+			expiresAt: ex,
+			hash: hm,
+		});
+	}
+}
+
+export const getUrlSignature = (data: NewUrlSignatureData): UrlSignResult => {
 	const { cdnSignatureKey, cdnSignatureDuration } = Config.get().security;
 
 	// calculate the expiration time
@@ -37,135 +128,142 @@ export const getUrlSignature = (
 	);
 
 	// hash the url with the cdnSignatureKey
-	const hash = calculateHash(path, issuedAt, expiresAt, req);
-
-	return {
-		hash,
-		issuedAt,
-		expiresAt,
-	};
+	return calculateHash(
+		new UrlSignatureData({
+			...data,
+			issuedAt,
+			expiresAt,
+		}),
+	);
 };
 
-export const calculateHash = (
-	url: string,
-	issuedAt: string,
-	expiresAt: string,
-	req: Request,
-) => {
+function calculateHash(request: UrlSignatureData): UrlSignResult {
 	const { cdnSignatureKey } = Config.get().security;
 	const data = createHmac("sha256", cdnSignatureKey as string)
-		.update(url)
-		.update(issuedAt)
-		.update(expiresAt);
+		.update(request.path!)
+		.update(request.issuedAt)
+		.update(request.expiresAt);
 
 	if (Config.get().security.cdnSignatureIncludeIp) {
-		if (!req || !req.ip)
+		if (!request.ip)
 			console.log(
-				"[Signing] CDN Signature IP is enabled but no request object was provided. This may cause issues with signature validation. Please report this to the Spacebar team!",
+				"[Signing] CDN Signature IP is enabled but we couldn't find the IP field in the request. This may cause issues with signature validation. Please report this to the Spacebar team!",
 			);
-		console.log("[Signing] CDN Signature IP is enabled, adding IP to hash:", req.ip);
-		data.update(req.ip!);
+		else {
+			console.log(
+				"[Signing] CDN Signature IP is enabled, adding IP to hash:",
+				request.ip,
+			);
+			data.update(request.ip!);
+		}
 	}
 
 	if (Config.get().security.cdnSignatureIncludeUserAgent) {
-		if (!req || !req.headers || !req.headers["user-agent"])
+		if (!request.userAgent)
 			console.log(
-				"[Signing] CDN Signature User-Agent is enabled but no request object was provided. This may cause issues with signature validation. Please report this to the Spacebar team!",
+				"[Signing] CDN Signature User-Agent is enabled but we couldn't find the user-agent header in the request. This may cause issues with signature validation. Please report this to the Spacebar team!",
 			);
-		data.update(req.headers["user-agent"] as string);
+		else {
+			console.log(
+				"[Signing] CDN Signature User-Agent is enabled, adding User-Agent to hash:",
+				request.userAgent,
+			);
+			data.update(request.userAgent!);
+		}
 	}
 
-
 	const hash = data.digest("hex");
+	const result = new UrlSignResult({
+		path: request.path,
+		issuedAt: request.issuedAt,
+		expiresAt: request.expiresAt,
+		hash,
+	});
 	console.log("[Signing]", {
-		url,
-		issuedAt,
-		expiresAt,
-		includeUA: Config.get().security.cdnSignatureIncludeUserAgent,
-		ua: req.headers["user-agent"],
-		includeIP: Config.get().security.cdnSignatureIncludeIp,
-		ip: req.ip,
-	}, "->", hash);
-	return hash;
-};
+		path: request.path,
+		validity: request.issuedAt + " .. " + request.expiresAt,
+		ua: Config.get().security.cdnSignatureIncludeUserAgent ? request.userAgent : "[disabled]",
+		ip: Config.get().security.cdnSignatureIncludeIp ? request.ip : "[disabled]"
+	}, "->", result);
+	return result;
+}
 
-export const isExpired = (ex: string, is: string) => {
+export const isExpired = (data: UrlSignResult | UrlSignatureData) => {
 	// convert issued at
-	const issuedAt = parseInt(is, 16);
-	const expiresAt = parseInt(ex, 16);
+	const issuedAt = parseInt(data.issuedAt, 16);
+	const expiresAt = parseInt(data.expiresAt, 16);
 
 	if (Number.isNaN(issuedAt) || Number.isNaN(expiresAt)) {
-		// console.debug("Invalid timestamps in query");
+		console.debug("[Signing] Invalid timestamps in query");
 		return true;
 	}
 
 	const currentTime = Date.now();
+
 	const isExpired = expiresAt < currentTime;
+	if (isExpired) {
+		console.debug("[Signing] Signature expired");
+		return true;
+	}
+
 	const isValidIssuedAt = issuedAt < currentTime;
-	if (isExpired || !isValidIssuedAt) {
-		// console.debug("Signature expired");
+	if (!isValidIssuedAt) {
+		console.debug("[Signing] Signature issued in the future");
 		return true;
 	}
 
 	return false;
 };
 
-export const hasValidSignature = (path: string, query: ParsedQs, req: Request) => {
-	// get url path
-	const { ex, is, hm } = query;
-
+export const hasValidSignature = (req: NewUrlUserSignatureData, sig: UrlSignResult) => {
 	// if the required query parameters are not present, return false
-	if (!ex || !is || !hm) {
-		console.debug("Missing required query parameters for signature validation");
+	if (!sig.expiresAt || !sig.issuedAt || !sig.hash) {
+		console.warn(
+			"[Signing] Missing required query parameters for signature validation",
+		);
 		return false;
 	}
 
 	// check if the signature is expired
-	if (isExpired(ex as string, is as string)) {
-		console.debug("Signature is expired");
+	if (isExpired(sig)) {
+		console.warn("[Signing] Signature is expired");
 		return false;
 	}
 
-	const calcd = calculateHash(path, is as string, ex as string, req);
+	const calcResult = calculateHash(new UrlSignatureData({
+		path: sig.path,
+		issuedAt: sig.issuedAt,
+		expiresAt: sig.expiresAt,
+		ip: req.ip,
+		userAgent: req.userAgent
+	}));
+	const calcd = calcResult.hash;
 	const calculated = Buffer.from(calcd);
-	const received = Buffer.from(hm as string);
+	const received = Buffer.from(sig.hash as string);
+
+	console.assert(sig.issuedAt == calcResult.issuedAt, "[Signing] Mismatched issuedAt", {
+		is: sig.issuedAt,
+		calculated: calcResult.issuedAt,
+	});
+
+	console.assert(sig.expiresAt == calcResult.expiresAt, "[Signing] Mismatched expiresAt", {
+		ex: sig.expiresAt,
+		calculated: calcResult.expiresAt,
+	});
+
+	console.assert(calculated.length === received.length, "[Signing] Mismatched hash length", {
+		calculated: calculated.length,
+		received: received.length,
+	});
 
 	const isHashValid =
 		calculated.length === received.length &&
 		timingSafeEqual(calculated, received);
 
-	console.debug(
-		`Signature validation for ${path} - isHashValid: ${isHashValid}, calculated: ${calcd}, received: ${hm}`,
-	);
+	if (!isHashValid)
+		console.warn(
+			`Signature validation for ${sig.path} (is=${sig.issuedAt}, ex=${sig.expiresAt}) failed: calculated: ${calcd}, received: ${sig.hash}`,
+		);
 
 	return isHashValid;
 };
-
-export const resignUrl = (attachmentUrl: string, req: Request) => {
-	const url = new URL(attachmentUrl);
-
-	// if theres an existing signature, check if its expired or not. no reason to resign if its not expired
-	if (url.searchParams.has("ex") && url.searchParams.has("is")) {
-		// extract the ex and is
-		const ex = url.searchParams.get("ex");
-		const is = url.searchParams.get("is");
-
-		if (!isExpired(ex as string, is as string)) {
-			// if the signature is not expired, return the url as is
-			return attachmentUrl;
-		}
-	}
-
-	let path = url.pathname;
-	// strip / from the start
-	if (path.startsWith("/")) {
-		path = path.slice(1);
-	}
-
-	const { hash, issuedAt, expiresAt } = getUrlSignature(path, req);
-	url.searchParams.set("ex", expiresAt);
-	url.searchParams.set("is", issuedAt);
-	url.searchParams.set("hm", hash);
-
-	return url.toString();
-};

src/util/entities/Attachment.ts

@@ -28,8 +28,11 @@ import { URL } from "url";
 import { deleteFile } from "../util/cdn";
 import { BaseClass } from "./BaseClass";
 import { dbEngine } from "../util/Database";
-import { Request } from "express";
-import { resignUrl } from "../Signing";
+import {
+	getUrlSignature,
+	NewUrlUserSignatureData,
+	NewUrlSignatureData,
+} from "../Signing";
 
 @Entity({
 	name: "attachments",
@@ -76,11 +79,15 @@ export class Attachment extends BaseClass {
 		return deleteFile(new URL(this.url).pathname);
 	}
 
-	signUrls(req: Request) {
+	signUrls(data: NewUrlUserSignatureData): Attachment {
 		return {
 			...this,
-			url: resignUrl(this.url, req),
-			proxy_url: resignUrl(this.proxy_url, req),
-		}
+			url: getUrlSignature(
+				new NewUrlSignatureData({ ...data, url: this.url }),
+			).applyToUrl(this.url).toString(),
+			proxy_url: getUrlSignature(
+				new NewUrlSignatureData({ ...data, url: this.proxy_url }),
+			).applyToUrl(this.proxy_url).toString(),
+		};
 	}
 }

src/util/entities/Message.ts

@@ -40,7 +40,7 @@ import { Webhook } from "./Webhook";
 import { Sticker } from "./Sticker";
 import { Attachment } from "./Attachment";
 import { dbEngine } from "../util/Database";
-import { Request } from "express";
+import { NewUrlUserSignatureData } from "../Signing";
 
 export enum MessageType {
 	DEFAULT = 0,
@@ -262,11 +262,11 @@ export class Message extends BaseClass {
 		};
 	}
 
-	withSignedAttachments(req: Request) {
+	withSignedAttachments(data: NewUrlUserSignatureData) {
 		return {
 			...this,
 			attachments: this.attachments?.map((attachment: Attachment) =>
-				Attachment.prototype.signUrls.call(attachment, req)
+				Attachment.prototype.signUrls.call(attachment, data)
 			)
 		};
 	}