From 2e196573ea1e257fc9266d6c63289c51fbaa4e12 Mon Sep 17 00:00:00 2001 From: Rory& Date: Fri, 4 Jul 2025 09:10:42 +0200 Subject: [PATCH] Make IP/user-agent validation in CDN work --- src/api/routes/attachments/refresh-urls.ts | 10 +- .../#channel_id/messages/#message_id/index.ts | 2 +- .../channels/#channel_id/messages/index.ts | 17 +- src/cdn/routes/attachments.ts | 14 +- src/gateway/events/Connection.ts | 28 +- src/gateway/listener/listener.ts | 8 +- src/gateway/util/WebSocket.ts | 6 +- src/util/Signing.ts | 276 ++++++++++++------ src/util/entities/Attachment.ts | 19 +- src/util/entities/Message.ts | 6 +- 10 files changed, 264 insertions(+), 122 deletions(-) diff --git a/src/api/routes/attachments/refresh-urls.ts b/src/api/routes/attachments/refresh-urls.ts index 78f6b7fc..0ad86e5c 100644 --- a/src/api/routes/attachments/refresh-urls.ts +++ b/src/api/routes/attachments/refresh-urls.ts @@ -17,7 +17,7 @@ */ import { route } from "@spacebar/api"; -import { RefreshUrlsRequestSchema, resignUrl } from "@spacebar/util"; +import { RefreshUrlsRequestSchema, getUrlSignature, NewUrlSignatureData } from "@spacebar/util"; import { Request, Response, Router } from "express"; const router = Router(); @@ -37,7 +37,13 @@ router.post( async (req: Request, res: Response) => { const { attachment_urls } = req.body as RefreshUrlsRequestSchema; - const refreshed_urls = attachment_urls.map(url => resignUrl(url, req)); + const refreshed_urls = attachment_urls.map(url => { + return getUrlSignature(new NewUrlSignatureData({ + url: url, + ip: req.ip, + userAgent: req.headers["user-agent"] as string + })).applyToUrl(url).toString(); + }); return res.status(200).json({ refreshed_urls, diff --git a/src/api/routes/channels/#channel_id/messages/#message_id/index.ts b/src/api/routes/channels/#channel_id/messages/#message_id/index.ts index 5388c363..d91c705d 100644 --- a/src/api/routes/channels/#channel_id/messages/#message_id/index.ts +++ b/src/api/routes/channels/#channel_id/messages/#message_id/index.ts @@ -30,7 +30,7 @@ import { emitEvent, getPermission, getRights, - uploadFile, Config, getUrlSignature, + uploadFile } from "@spacebar/util"; import { Request, Response, Router } from "express"; import { HTTPError } from "lambert-server"; diff --git a/src/api/routes/channels/#channel_id/messages/index.ts b/src/api/routes/channels/#channel_id/messages/index.ts index 2e1e3a28..743bc07e 100644 --- a/src/api/routes/channels/#channel_id/messages/index.ts +++ b/src/api/routes/channels/#channel_id/messages/index.ts @@ -35,8 +35,8 @@ import { emitEvent, getPermission, isTextChannel, - resignUrl, - uploadFile, + getUrlSignature, + uploadFile, NewUrlSignatureData, } from "@spacebar/util"; import { Request, Response, Router } from "express"; import { HTTPError } from "lambert-server"; @@ -210,8 +210,17 @@ router.get( y.proxy_url = url.toString(); - y.proxy_url = resignUrl(y.proxy_url, req); - y.url = resignUrl(y.url, req); + y.proxy_url = getUrlSignature(new NewUrlSignatureData({ + url: y.proxy_url, + userAgent: req.headers["user-agent"], + ip: req.ip, + })).applyToUrl(y.proxy_url).toString(); + + y.url = getUrlSignature(new NewUrlSignatureData({ + url: y.url, + userAgent: req.headers["user-agent"], + ip: req.ip, + })).applyToUrl(y.url).toString(); }); /** diff --git a/src/cdn/routes/attachments.ts b/src/cdn/routes/attachments.ts index d252a386..4447edeb 100644 --- a/src/cdn/routes/attachments.ts +++ b/src/cdn/routes/attachments.ts @@ -18,9 +18,8 @@ import { Config, - getUrlSignature, - hasValidSignature, - Snowflake, + hasValidSignature, NewUrlUserSignatureData, + Snowflake, UrlSignResult, } from "@spacebar/util"; import { Request, Response, Router } from "express"; import FileType from "file-type"; @@ -94,9 +93,16 @@ router.get( const path = `attachments/${channel_id}/${id}/${filename}`; + const fullUrl = (req.headers["x-forwarded-proto"] ?? req.protocol) + "://" + + (req.headers["x-forwarded-host"] ?? req.hostname) + + req.originalUrl; + if ( Config.get().security.cdnSignUrls && - !hasValidSignature(path, req.query, req) + !hasValidSignature(new NewUrlUserSignatureData({ + ip: req.ip, + userAgent: req.headers["user-agent"] as string, + }), UrlSignResult.fromUrl(fullUrl)) ) { return res.status(404).send("This content is no longer available."); } diff --git a/src/gateway/events/Connection.ts b/src/gateway/events/Connection.ts index e0c82426..7dfd44d9 100644 --- a/src/gateway/events/Connection.ts +++ b/src/gateway/events/Connection.ts @@ -28,7 +28,6 @@ import { Message } from "./Message"; import { Deflate, Inflate } from "fast-zlib"; import { URL } from "url"; import { Config, ErlpackType } from "@spacebar/util"; -import { Request } from "express"; let erlpack: ErlpackType | null = null; try { @@ -48,11 +47,28 @@ export async function Connection( ) { const forwardedFor = Config.get().security.forwardedFor; const ipAddress = forwardedFor - ? (request.headers[forwardedFor] as string) + ? (request.headers[forwardedFor.toLowerCase()] as string) : request.socket.remoteAddress; socket.ipAddress = ipAddress; - socket.request = request as unknown as Request; + socket.userAgent = request.headers["user-agent"] as string; + + if (!ipAddress && Config.get().security.cdnSignatureIncludeIp) { + return socket.close( + CLOSECODES.Decode_error, + "Gateway connection rejected: IP address is required.", + ); + } + + if ( + !socket.userAgent && + Config.get().security.cdnSignatureIncludeUserAgent + ) { + return socket.close( + CLOSECODES.Decode_error, + "Gateway connection rejected: User-Agent header is required.", + ); + } //Create session ID when the connection is opened. This allows gateway dump to group the initial websocket messages with the rest of the conversation. const session_id = genSessionId(); @@ -66,9 +82,9 @@ export async function Connection( socket.on("error", (err) => console.error("[Gateway]", err)); - // console.log( - // `[Gateway] New connection from ${socket.ipAddress}, total ${this.clients.size}`, - // ); + console.log( + `[Gateway] New connection from ${ipAddress}, total ${this.clients.size}`, + ); if (process.env.WS_LOGEVENTS) [ diff --git a/src/gateway/listener/listener.ts b/src/gateway/listener/listener.ts index 86f50e39..2eb18dfe 100644 --- a/src/gateway/listener/listener.ts +++ b/src/gateway/listener/listener.ts @@ -27,7 +27,7 @@ import { EVENTEnum, Relationship, RelationshipType, - Message, + Message, NewUrlUserSignatureData, } from "@spacebar/util"; import { OPCODES } from "../util/Constants"; import { Send } from "../util/Send"; @@ -290,8 +290,12 @@ async function consume(this: WebSocket, opts: EventOpts) { switch (event) { case "MESSAGE_CREATE": case "MESSAGE_UPDATE": + // console.log(this.request) if(data["attachments"]) - data["attachments"] = Message.prototype.withSignedAttachments.call(data, this.request).attachments; + data["attachments"] = Message.prototype.withSignedAttachments.call(data, new NewUrlUserSignatureData({ + ip: this.ipAddress, + userAgent: this.userAgent + })).attachments; break; default: break; diff --git a/src/gateway/util/WebSocket.ts b/src/gateway/util/WebSocket.ts index a1e9c0b0..4dcec55e 100644 --- a/src/gateway/util/WebSocket.ts +++ b/src/gateway/util/WebSocket.ts @@ -29,6 +29,7 @@ export interface WebSocket extends WS { encoding: "etf" | "json"; compress?: "zlib-stream"; ipAddress?: string; + userAgent?: string; // for cdn request signing shard_count?: bigint; shard_id?: bigint; deflate?: Deflate; @@ -43,9 +44,4 @@ export interface WebSocket extends WS { listen_options: ListenEventOpts; capabilities?: Capabilities; large_threshold: number; - /** - * The request object for the WebSocket connection. - * WARNING: This is not a proper Express Request object, it may be missing expected properties. - */ - request: Request; // For signed attachment URLs } diff --git a/src/util/Signing.ts b/src/util/Signing.ts index 2d3abe07..9d80297d 100644 --- a/src/util/Signing.ts +++ b/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(); -}; diff --git a/src/util/entities/Attachment.ts b/src/util/entities/Attachment.ts index 1141d08f..c29ba310 100644 --- a/src/util/entities/Attachment.ts +++ b/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(), + }; } } diff --git a/src/util/entities/Message.ts b/src/util/entities/Message.ts index cc96843f..52c82cd5 100644 --- a/src/util/entities/Message.ts +++ b/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) ) }; }