diff --git a/packages/backend/src/lib/safeFetch.ts b/packages/backend/src/lib/safeFetch.ts index 78cf2cd6..6527e54b 100644 --- a/packages/backend/src/lib/safeFetch.ts +++ b/packages/backend/src/lib/safeFetch.ts @@ -343,6 +343,25 @@ export async function safeFetch( init: RequestInit = {}, options: SafeFetchOptions = {}, ): Promise { + return (await safeFetchWithFinalUrl(input, init, options)).response; +} + +/** + * Same as {@link safeFetch}, but also returns the URL of the final hop + * (after following redirects). Callers that bind trust to the requested URL + * (e.g. ActivityPub object/key fetches) must check that it did not change. + * + * @param input - URL to fetch + * @param init - Fetch init (the `redirect` option is ignored) + * @param options - Redirect and validation options + * @returns The final response and the URL it was served from + * @throws {UnsafeUrlError} If any hop targets a non-public address or the redirect limit is exceeded + */ +export async function safeFetchWithFinalUrl( + input: string | URL, + init: RequestInit = {}, + options: SafeFetchOptions = {}, +): Promise<{ response: Response; finalUrl: URL }> { const maxRedirects = options.maxRedirects ?? DEFAULT_MAX_REDIRECTS; const fetchImpl = options.fetchImpl ?? fetch; @@ -361,12 +380,12 @@ export async function safeFetch( }); if (!REDIRECT_STATUSES.has(response.status)) { - return response; + return { response, finalUrl: current }; } const location = response.headers.get("location"); if (!location) { - return response; + return { response, finalUrl: current }; } // Discard the redirect body diff --git a/packages/backend/src/middleware/verifySignature.ts b/packages/backend/src/middleware/verifySignature.ts index b86913b1..7f0be3e7 100644 --- a/packages/backend/src/middleware/verifySignature.ts +++ b/packages/backend/src/middleware/verifySignature.ts @@ -21,11 +21,17 @@ import type { IUserRepository } from "../interfaces/repositories/IUserRepository import { CacheTTL, CachePrefix } from "../adapters/cache/DragonflyCacheAdapter.js"; import { RemoteFetchService, type SignatureConfig } from "../services/ap/RemoteFetchService.js"; import { logger } from "../lib/logger.js"; +import { + hasAmbiguousPath, + isLocalUrl, + normalizeUrl, + parseHttpUrl, +} from "../utils/federationUrl.js"; /** * In-memory cache fallback for public keys when Redis is unavailable */ -const publicKeyMemoryCache = new Map(); +const publicKeyMemoryCache = new Map(); /** * In-memory cache for fetch failures to prevent repeated requests to failing servers @@ -148,58 +154,229 @@ async function getSignatureConfig( } /** - * Fetch remote actor's public key - * - * Retrieves the public key from the remote actor's document. - * Results are cached in Redis (1 hour) with in-memory fallback. + * Public key bound to its verified owner (actor URI) + */ +export interface VerifiedPublicKey { + /** PEM-encoded public key */ + publicKeyPem: string; + /** Actor URI that owns this key (verified against the fetched document) */ + owner: string; +} + +/** + * Validate a signature keyId before fetching it. * - * For Authorized Fetch servers (which return 401/403 on unsigned requests), - * automatically retries with a signed request using local admin credentials. + * Rejects keyIds that are not http(s) URLs, contain ambiguous path components + * (dot segments, backslashes, whitespace) that URL normalization would rewrite, + * or point at this instance (local users never deliver to our inbox over HTTP). * - * @param keyId - Public key identifier URL - * @param cacheService - Optional cache service for Redis caching - * @param userRepository - Optional user repository for signed fetch retry - * @param baseUrl - Optional base URL of this instance for signed fetch - * @returns PEM-formatted public key - * @throws Error if key cannot be fetched + * @param keyId - KeyId from the Signature header + * @returns URL to fetch the key document from (keyId without fragment) + * @throws Error if the keyId is not acceptable */ -async function fetchPublicKey( - keyId: string, - cacheService?: ICacheService, - userRepository?: IUserRepository, - baseUrl?: string, -): Promise { - const cacheKey = `${CachePrefix.PUBLIC_KEY}:${keyId}`; +export function validateKeyId(keyId: string): string { + if (!parseHttpUrl(keyId)) { + throw new Error("Invalid keyId: not an http(s) URL"); + } + if (hasAmbiguousPath(keyId)) { + throw new Error("Invalid keyId: ambiguous path"); + } + if (isLocalUrl(keyId)) { + throw new Error("Invalid keyId: points to local host"); + } + const fetchUrl = keyId.split("#")[0]; + if (!fetchUrl || normalizeUrl(fetchUrl) === null) { + throw new Error("Invalid keyId format"); + } + return fetchUrl; +} - // Check Redis cache first - if (cacheService?.isAvailable()) { - const cached = await cacheService.get(cacheKey); - if (cached) { - return cached; +/** + * A fetched key/actor document together with the URL it was served from. + */ +export interface FetchedKeyDocument { + /** Parsed JSON document */ + data: unknown; + /** URL of the final response (after redirects) */ + finalUrl: string; +} + +/** + * Fetcher used by {@link resolveVerifiedPublicKey} to retrieve documents. + * Must throw on network/HTTP failure. + */ +export type KeyDocumentFetcher = (url: string) => Promise; + +/** + * Key entry found in a document, with the owner it claims. + */ +interface KeyClaim { + publicKeyPem: string; + owner: string; +} + +/** + * Find the key whose `id` equals `keyId` inside an actor document's `publicKey` + * (object or array). + */ +function findActorKey( + doc: { publicKey?: unknown }, + normalizedKeyId: string, +): { publicKeyPem?: unknown; owner?: unknown } | null { + const candidates: unknown[] = Array.isArray(doc.publicKey) ? doc.publicKey : [doc.publicKey]; + const key = candidates.find( + (k) => + !!k && + typeof k === "object" && + typeof (k as { id?: unknown }).id === "string" && + normalizeUrl((k as { id: string }).id) === normalizedKeyId, + ); + return (key as { publicKeyPem?: unknown; owner?: unknown } | undefined) ?? null; +} + +/** + * Extract the key claim for `keyId` from a fetched document (actor document + * with `publicKey`, or standalone Key document). Does not establish trust. + */ +function extractKeyClaim(keyId: string, doc: unknown): KeyClaim { + if (!doc || typeof doc !== "object") { + throw new Error("Key document is not an object"); + } + const document = doc as { + id?: unknown; + owner?: unknown; + publicKey?: unknown; + publicKeyPem?: unknown; + }; + const normalizedKeyId = normalizeUrl(keyId); + if (!normalizedKeyId) { + throw new Error("Invalid keyId"); + } + const docId = typeof document.id === "string" ? document.id : null; + + let claim: { publicKeyPem?: unknown; owner?: unknown } | null = null; + if (document.publicKey) { + claim = findActorKey(document, normalizedKeyId); + if (!claim) { + throw new Error("publicKey.id does not match keyId"); } - } else { - // Fallback to in-memory cache - const cached = publicKeyMemoryCache.get(keyId); - if (cached && cached.expires > Date.now()) { - return cached.key; + claim = { publicKeyPem: claim.publicKeyPem, owner: claim.owner ?? docId }; + } else if (typeof document.publicKeyPem === "string") { + if (!docId || normalizeUrl(docId) !== normalizedKeyId) { + throw new Error("Key document id does not match keyId"); } + claim = { publicKeyPem: document.publicKeyPem, owner: document.owner }; } - // Fetch actor document - // keyId format: https://example.com/users/alice#main-key - // Actor URL: https://example.com/users/alice - const actorUrl = keyId.split("#")[0]; + if (!claim || typeof claim.publicKeyPem !== "string" || !claim.publicKeyPem) { + throw new Error("Public key not found in key document"); + } + if (typeof claim.owner !== "string" || !parseHttpUrl(claim.owner)) { + throw new Error("Key owner is missing or not an http(s) URL"); + } + if (isLocalUrl(claim.owner)) { + throw new Error("Key owner points to local host"); + } + return { publicKeyPem: claim.publicKeyPem, owner: claim.owner }; +} - if (!actorUrl) { - throw new Error("Invalid keyId format"); +/** + * Resolve the public key for `keyId` and verify which actor owns it. + * + * Trust is established Mastodon-style: + * 1. The key document (keyId without fragment) is fetched and the entry with + * `id === keyId` is located; its owner is `publicKey.owner` (or the document id). + * 2. If the document is the owner's actor document served from exactly the + * requested URL (document id === owner === fetched URL, no redirect), it is + * accepted directly. + * 3. Otherwise (GoToSocial `/main-key` stubs, standalone `/publickey` Key + * documents, redirected responses, …) the owner's actor document is fetched + * from the owner URL itself. It must be served without redirect, have + * `id === owner`, and list a key with `id === keyId` and the identical PEM. + * + * Same-origin content (open redirects, user-uploaded JSON, proxies) therefore + * cannot bind a key to an actor unless the actor document itself lists it. + * + * @param keyId - KeyId from the Signature header + * @param fetchUrl - URL the key document is fetched from (keyId without fragment) + * @param fetchDocument - Fetcher for documents (throws on failure) + * @returns Verified PEM and owner actor URI + * @throws Error if ownership cannot be verified + */ +export async function resolveVerifiedPublicKey( + keyId: string, + fetchUrl: string, + fetchDocument: KeyDocumentFetcher, +): Promise { + const first = await fetchDocument(fetchUrl); + const claim = extractKeyClaim(keyId, first.data); + + const normalizedFetchUrl = normalizeUrl(fetchUrl); + const firstDocId = (first.data as { id?: unknown }).id; + const servedFromRequestedUrl = normalizeUrl(first.finalUrl) === normalizedFetchUrl; + + if ( + servedFromRequestedUrl && + typeof firstDocId === "string" && + normalizeUrl(firstDocId) === normalizedFetchUrl && + claim.owner === firstDocId && + (first.data as { publicKey?: unknown }).publicKey + ) { + return claim; } - // Check if we have a cached failure for this actor - const cachedFailure = getCachedFailure(actorUrl); + // Fetch the owner's actor document and require it to list the same key + const ownerDoc = await fetchDocument(claim.owner); + if (normalizeUrl(ownerDoc.finalUrl) !== normalizeUrl(claim.owner)) { + throw new Error("Owner actor document was served from a different URL"); + } + const actor = ownerDoc.data as { id?: unknown; publicKey?: unknown } | null; + if (!actor || typeof actor !== "object" || actor.id !== claim.owner || !actor.publicKey) { + throw new Error("Owner actor document id does not match key owner"); + } + const normalizedKeyId = normalizeUrl(keyId)!; + const ownerKey = findActorKey(actor, normalizedKeyId); + if (!ownerKey || ownerKey.publicKeyPem !== claim.publicKeyPem) { + throw new Error("Owner actor document does not list this key"); + } + if (ownerKey.owner !== undefined && ownerKey.owner !== claim.owner) { + throw new Error("Owner actor document key has a different owner"); + } + return claim; +} + +/** + * Check whether a cached value has the current {@link VerifiedPublicKey} shape. + * Legacy cache entries (plain PEM strings) are treated as a miss. + */ +function isVerifiedPublicKey(value: unknown): value is VerifiedPublicKey { + return ( + !!value && + typeof value === "object" && + typeof (value as VerifiedPublicKey).publicKeyPem === "string" && + typeof (value as VerifiedPublicKey).owner === "string" + ); +} + +/** + * Fetch a key/actor document for signature verification. + * + * Tries an unsigned fetch first, then retries with an HTTP Signature for + * Authorized Fetch servers (401/403). Failures are cached per URL. + * + * @throws Error if the document cannot be fetched + */ +async function fetchKeyDocument( + url: string, + userRepository?: IUserRepository, + baseUrl?: string, +): Promise { + // Check if we have a cached failure for this URL + const cachedFailure = getCachedFailure(url); if (cachedFailure) { logger.debug( { - actorUrl, + actorUrl: url, errorType: cachedFailure.errorType, ttlRemaining: cachedFailure.expires - Date.now(), }, @@ -210,12 +387,8 @@ async function fetchPublicKey( const fetchService = new RemoteFetchService(); - logger.debug({ keyId, actorUrl }, "Fetching public key for signature verification"); - // First, try unsigned fetch (works for most servers) - let result = await fetchService.fetchActivityPubObject<{ - publicKey?: { publicKeyPem?: string }; - }>(actorUrl, { maxRetries: 1 }); + let result = await fetchService.fetchActivityPubObject(url, { maxRetries: 1 }); // If unsigned fetch failed with 401/403, retry with signed request // This is required for Authorized Fetch servers (e.g., some Mastodon instances) @@ -227,16 +400,14 @@ async function fetchPublicKey( baseUrl ) { logger.debug( - { actorUrl, statusCode: result.error.statusCode }, + { actorUrl: url, statusCode: result.error.statusCode }, "Unsigned fetch failed, retrying with HTTP Signature for Authorized Fetch server", ); const signatureConfig = await getSignatureConfig(userRepository, baseUrl); if (signatureConfig) { - result = await fetchService.fetchActivityPubObject<{ - publicKey?: { publicKeyPem?: string }; - }>(actorUrl, { + result = await fetchService.fetchActivityPubObject(url, { signature: signatureConfig, maxRetries: 2, }); @@ -248,17 +419,17 @@ async function fetchPublicKey( const errorType = categorizeError(result.error); const errorMessage = result.error?.message || "Unknown error"; - cacheFailure(actorUrl, errorType, errorMessage); + cacheFailure(url, errorType, errorMessage); // Use appropriate log level based on error type if (errorType === "permanent_error") { logger.warn( - { err: result.error, actorUrl, errorType }, + { err: result.error, actorUrl: url, errorType }, "Failed to fetch public key (permanent error)", ); } else { logger.debug( - { err: result.error, actorUrl, errorType }, + { err: result.error, actorUrl: url, errorType }, "Failed to fetch public key (transient error)", ); } @@ -266,28 +437,67 @@ async function fetchPublicKey( throw new Error(`Failed to fetch actor: ${errorMessage}`); } - const actor = result.data!; + return { data: result.data, finalUrl: result.finalUrl ?? url }; +} + +/** + * Fetch remote actor's public key + * + * Retrieves the public key for `keyId` and verifies which actor owns it + * (see {@link resolveVerifiedPublicKey}). + * Results are cached in Redis (1 hour) with in-memory fallback. + * + * @param keyId - Public key identifier URL + * @param cacheService - Optional cache service for Redis caching + * @param userRepository - Optional user repository for signed fetch retry + * @param baseUrl - Optional base URL of this instance for signed fetch + * @returns PEM-formatted public key and its verified owner + * @throws Error if key cannot be fetched or verified + */ +async function fetchPublicKey( + keyId: string, + cacheService?: ICacheService, + userRepository?: IUserRepository, + baseUrl?: string, +): Promise { + // keyId format: https://example.com/users/alice#main-key + // Fetch URL: https://example.com/users/alice + const actorUrl = validateKeyId(keyId); + + const cacheKey = `${CachePrefix.PUBLIC_KEY}:${keyId}`; - // Extract public key - let publicKey: string; - if (actor.publicKey && actor.publicKey.publicKeyPem) { - publicKey = actor.publicKey.publicKeyPem; + // Check Redis cache first + if (cacheService?.isAvailable()) { + const cached = await cacheService.get(cacheKey); + if (isVerifiedPublicKey(cached)) { + return cached; + } } else { - throw new Error("Public key not found in actor document"); + // Fallback to in-memory cache + const cached = publicKeyMemoryCache.get(keyId); + if (cached && cached.expires > Date.now()) { + return cached.key; + } } + logger.debug({ keyId, actorUrl }, "Fetching public key for signature verification"); + + const verified = await resolveVerifiedPublicKey(keyId, actorUrl, (url) => + fetchKeyDocument(url, userRepository, baseUrl), + ); + // Cache in Redis (1 hour) if (cacheService?.isAvailable()) { - await cacheService.set(cacheKey, publicKey, { ttl: CacheTTL.LONG }); + await cacheService.set(cacheKey, verified, { ttl: CacheTTL.LONG }); } else { // Fallback to in-memory cache publicKeyMemoryCache.set(keyId, { - key: publicKey, + key: verified, expires: Date.now() + CacheTTL.LONG * 1000, }); } - return publicKey; + return verified; } /** @@ -339,9 +549,14 @@ export async function verifySignatureMiddleware(c: Context, next: Next): Promise // Fetch public key (with Redis caching and signed fetch retry) logger.debug({ keyId: params.keyId }, "Fetching public key"); - const publicKey = await fetchPublicKey(params.keyId, cacheService, userRepository, baseUrl); + const { publicKeyPem: publicKey, owner: signatureActor } = await fetchPublicKey( + params.keyId, + cacheService, + userRepository, + baseUrl, + ); logger.debug( - { keyId: params.keyId, keyLength: publicKey.length }, + { keyId: params.keyId, owner: signatureActor, keyLength: publicKey.length }, "Public key fetched successfully", ); @@ -389,8 +604,10 @@ export async function verifySignatureMiddleware(c: Context, next: Next): Promise c.set("requestBody", body); } - // Store keyId for activity validation + // Store keyId and the verified key owner for activity validation. + // The inbox route must require activity.actor === signatureActor. c.set("signatureKeyId", params.keyId); + c.set("signatureActor", signatureActor); logger.debug({ keyId: params.keyId }, "Signature verified successfully"); diff --git a/packages/backend/src/routes/ap/inbox.ts b/packages/backend/src/routes/ap/inbox.ts index 25b64785..00901a71 100644 --- a/packages/backend/src/routes/ap/inbox.ts +++ b/packages/backend/src/routes/ap/inbox.ts @@ -74,7 +74,8 @@ inbox.post( // Enhanced activity validation const signatureKeyId = c.get("signatureKeyId"); - const validationResult = validateActivity(activity, signatureKeyId); + const signatureActor = c.get("signatureActor"); + const validationResult = validateActivity(activity, signatureKeyId, signatureActor); if (!validationResult.valid) { logger.warn( @@ -207,7 +208,8 @@ inbox.post( // Enhanced activity validation const signatureKeyId = c.get("signatureKeyId"); - const validationResult = validateActivity(activity, signatureKeyId); + const signatureActor = c.get("signatureActor"); + const validationResult = validateActivity(activity, signatureKeyId, signatureActor); if (!validationResult.valid) { logger.warn( diff --git a/packages/backend/src/services/ap/RemoteActorService.ts b/packages/backend/src/services/ap/RemoteActorService.ts index 07bd26a0..f938a668 100644 --- a/packages/backend/src/services/ap/RemoteActorService.ts +++ b/packages/backend/src/services/ap/RemoteActorService.ts @@ -15,6 +15,7 @@ import { generateId } from "shared"; import { RemoteFetchService, type SignatureConfig } from "./RemoteFetchService.js"; import { CacheTTL, CachePrefix } from "../../adapters/cache/DragonflyCacheAdapter.js"; import { logger } from "../../lib/logger.js"; +import { isLocalHostname } from "../../utils/federationUrl.js"; /** * ActivityPub Emoji tag @@ -170,10 +171,9 @@ export class RemoteActorService { */ async resolveActor(actorUri: string, forceRefresh = false): Promise { // Check if this is a local URI - if so, find user by URI directly (local users have host=null) - const localHost = new URL(process.env.URL || "http://localhost:3000").hostname; const actorHost = new URL(actorUri).hostname; - if (actorHost === localHost) { + if (isLocalHostname(actorHost)) { // This is a local user URI - find by URI (which includes local users) const localUser = await this.userRepository.findByUri(actorUri); if (localUser) { @@ -539,8 +539,7 @@ export class RemoteActorService { } // Check if local user - const localHost = new URL(process.env.URL || "http://localhost:3000").hostname; - if (host === localHost) { + if (isLocalHostname(host)) { const localUser = await this.userRepository.findByUsername(username, null); if (!localUser) { throw new Error(`Local user not found: ${username}`); @@ -566,6 +565,9 @@ export class RemoteActorService { Accept: "application/jrd+json, application/json", }, allowedContentTypes: ["application/jrd+json", "application/json"], + // Split-domain setups redirect WebFinger to the AP host; the actor + // document itself is fetched (same-origin only) afterwards. + allowCrossOriginRedirect: true, }, ); diff --git a/packages/backend/src/services/ap/RemoteFetchService.ts b/packages/backend/src/services/ap/RemoteFetchService.ts index 306d715d..b61d7b23 100644 --- a/packages/backend/src/services/ap/RemoteFetchService.ts +++ b/packages/backend/src/services/ap/RemoteFetchService.ts @@ -14,7 +14,7 @@ import { signRequest, getSignedHeaders } from "../../utils/crypto.js"; import { logger } from "../../lib/logger.js"; -import { safeFetch, UnsafeUrlError } from "../../lib/safeFetch.js"; +import { safeFetchWithFinalUrl, UnsafeUrlError } from "../../lib/safeFetch.js"; /** * Content types accepted for ActivityPub object responses by default. @@ -55,6 +55,13 @@ export interface RemoteFetchOptions { * Defaults to {@link ACTIVITYPUB_CONTENT_TYPES}. */ allowedContentTypes?: string[]; + /** + * Allow redirects to a different origin than the requested URL (default: false). + * ActivityPub objects are trusted by their URL's origin, so cross-origin + * redirects are rejected unless explicitly allowed (e.g. WebFinger on + * split-domain setups). + */ + allowCrossOriginRedirect?: boolean; } /** @@ -63,6 +70,8 @@ export interface RemoteFetchOptions { export interface RemoteFetchResult { success: boolean; data?: T; + /** URL the response was actually served from (after redirects), on success */ + finalUrl?: string; error?: { type: "timeout" | "network" | "rate_limit" | "server_error" | "invalid_response"; message: string; @@ -121,6 +130,7 @@ export class RemoteFetchService { headers = {}, signature, allowedContentTypes = ACTIVITYPUB_CONTENT_TYPES, + allowCrossOriginRedirect = false, } = options; let lastError: RemoteFetchResult["error"]; @@ -161,6 +171,7 @@ export class RemoteFetchService { timeout, requestHeaders, allowedContentTypes, + allowCrossOriginRedirect, ); if (result.success) { @@ -220,6 +231,7 @@ export class RemoteFetchService { * @param timeout - Timeout in milliseconds * @param headers - HTTP headers * @param allowedContentTypes - Accepted response media types + * @param allowCrossOriginRedirect - Whether the final URL may differ in origin from `url` * @returns Fetch result * * @private @@ -229,6 +241,7 @@ export class RemoteFetchService { timeout: number, headers: Record, allowedContentTypes: string[], + allowCrossOriginRedirect = false, ): Promise> { try { // Create abort controller for timeout @@ -236,12 +249,13 @@ export class RemoteFetchService { const timeoutId = setTimeout(() => controller.abort(), timeout); let response: Response; + let finalUrl: URL; try { // SSRF protection: blocks private destinations and re-validates redirects - response = await safeFetch(url, { + ({ response, finalUrl } = await safeFetchWithFinalUrl(url, { headers, signal: controller.signal, - }); + })); } catch (error) { if (error instanceof UnsafeUrlError) { return { @@ -257,6 +271,18 @@ export class RemoteFetchService { clearTimeout(timeoutId); } + // Reject cross-origin redirects: the content would be attributed to the requested origin + if (!allowCrossOriginRedirect && finalUrl.origin !== new URL(url).origin) { + await response.body?.cancel().catch(() => {}); + return { + success: false, + error: { + type: "invalid_response", + message: `Cross-origin redirect rejected: ${finalUrl.origin}`, + }, + }; + } + // Handle HTTP errors if (!response.ok) { // Rate limiting @@ -331,6 +357,7 @@ export class RemoteFetchService { return { success: true, data, + finalUrl: finalUrl.href, }; } catch (error) { // Timeout or network error diff --git a/packages/backend/src/services/ap/RemoteNoteService.ts b/packages/backend/src/services/ap/RemoteNoteService.ts index 43041f9f..3684934f 100644 --- a/packages/backend/src/services/ap/RemoteNoteService.ts +++ b/packages/backend/src/services/ap/RemoteNoteService.ts @@ -15,6 +15,7 @@ import type { Note } from "shared"; import { generateId } from "shared"; import { RemoteActorService } from "./RemoteActorService.js"; import { logger } from "../../lib/logger.js"; +import { extractRefUri, isHttpUrl, isLocalUrl, isSameOrigin } from "../../utils/federationUrl.js"; /** * ActivityPub Note object @@ -76,7 +77,9 @@ export class RemoteNoteService { * Resolves the author and creates the note in the database. * * @param noteObject - ActivityPub Note object - * @returns Created Note record + * @returns Created Note record (or the existing note when already known / local) + * @throws Error if the note id is not an http(s) URL, the note is attributed to a + * local user, or its id origin does not match its author's origin * * @example * ```typescript @@ -85,6 +88,11 @@ export class RemoteNoteService { * ``` */ async processNote(noteObject: APNote): Promise { + // Note id must be an http(s) URL (rejects e.g. `javascript:` ids stored as note.uri) + if (!isHttpUrl(noteObject?.id)) { + throw new Error("Invalid note: id must be an http(s) URL"); + } + // Check if note already exists const existing = await this.noteRepository.findByUri(noteObject.id); if (existing) { @@ -92,13 +100,31 @@ export class RemoteNoteService { return existing; } + // Local notes are never created from remote documents: look up the existing one. + if (isLocalUrl(noteObject.id)) { + const localId = new URL(noteObject.id).pathname.match(/^\/notes\/([^/]+)$/)?.[1]; + const localNote = localId ? await this.noteRepository.findById(localId) : null; + if (localNote) { + return localNote; + } + throw new Error(`Local note not found: ${noteObject.id}`); + } + + const authorUri = extractRefUri(noteObject.attributedTo); + if (!isHttpUrl(authorUri)) { + throw new Error("Invalid note: attributedTo must be an http(s) URL"); + } + // Remote notes must not be attributed to local users, and must be + // hosted on the same origin as their author. + if (isLocalUrl(authorUri)) { + throw new Error("Invalid note: remote note attributed to a local user"); + } + if (!isSameOrigin(authorUri, noteObject.id)) { + throw new Error("Invalid note: id origin does not match attributedTo origin"); + } + // Resolve author (use injected service or create new instance for backward compatibility) const actorService = this.remoteActorService ?? new RemoteActorService(this.userRepository); - const authorUri = - typeof noteObject.attributedTo === "string" - ? noteObject.attributedTo - : noteObject.attributedTo.id; - const author = await actorService.resolveActor(authorUri); // Extract text content diff --git a/packages/backend/src/services/ap/inbox/handlers/AnnounceHandler.ts b/packages/backend/src/services/ap/inbox/handlers/AnnounceHandler.ts index ac25e495..ad2add9f 100644 --- a/packages/backend/src/services/ap/inbox/handlers/AnnounceHandler.ts +++ b/packages/backend/src/services/ap/inbox/handlers/AnnounceHandler.ts @@ -11,6 +11,20 @@ import { getObjectUri } from "../types.js"; import { BaseHandler } from "./BaseHandler.js"; import { RemoteFetchService, type SignatureConfig } from "../../RemoteFetchService.js"; import { logger } from "../../../../lib/logger.js"; +import { + extractRefUri, + isHttpUrl, + isLocalUrl, + isSameOrigin, + normalizeUrl, +} from "../../../../utils/federationUrl.js"; + +/** ActivityStreams public collection identifiers */ +const PUBLIC_ADDRESSES = new Set([ + "https://www.w3.org/ns/activitystreams#Public", + "as:Public", + "Public", +]); /** * Handler for Announce activities @@ -52,6 +66,80 @@ export class AnnounceHandler extends BaseHandler { } } + /** + * Fetch a remote object, retrying with an HTTP Signature for Authorized Fetch servers. + * + * @returns The fetched document and the URL it was served from, or null on failure + */ + private async fetchRemoteObject( + url: string, + c: import("hono").Context, + baseUrl: string, + ): Promise<{ data: any; finalUrl: string } | null> { + const fetchService = new RemoteFetchService(); + + // Try unsigned fetch first + let result = await fetchService.fetchActivityPubObject(url); + + // If failed with 401/403/404, try with HTTP Signature (Authorized Fetch) + if ( + !result.success && + result.error?.statusCode && + [401, 403, 404].includes(result.error.statusCode) + ) { + logger.debug( + { noteUri: url, statusCode: result.error.statusCode }, + "Unsigned fetch failed, retrying with HTTP Signature", + ); + + // Get a local user's credentials for signing + const signatureConfig = await this.getSignatureConfig(c, baseUrl); + + if (signatureConfig) { + result = await fetchService.fetchActivityPubObject(url, { + signature: signatureConfig, + }); + } + } + + if (!result.success) { + // 404 errors are common (deleted notes) - log at debug level + // Other errors (network issues, auth failures) are logged at warn level + const is404 = result.error?.statusCode === 404; + const logLevel = is404 ? "debug" : "warn"; + logger[logLevel]( + { noteUri: url, err: result.error }, + is404 ? "Remote note not found (possibly deleted)" : "Failed to fetch remote note", + ); + return null; + } + + const finalUrl = result.finalUrl ?? url; + if (!isSameOrigin(finalUrl, url)) { + logger.warn({ noteUri: url, finalUrl }, "Remote note served from a different origin"); + return null; + } + + return { data: result.data, finalUrl }; + } + + /** + * Derive the renote visibility from the Announce addressing. + * + * - Public collection in `to` → public + * - Public collection in `cc` → home (unlisted) + * - Otherwise → followers + */ + private determineRenoteVisibility(activity: Activity): "public" | "home" | "followers" { + const toArray = (v: unknown): string[] => + Array.isArray(v) ? v.filter((x) => typeof x === "string") : typeof v === "string" ? [v] : []; + const isPublic = (uri: string) => PUBLIC_ADDRESSES.has(uri); + + if (toArray(activity.to).some(isPublic)) return "public"; + if (toArray(activity.cc).some(isPublic)) return "home"; + return "followers"; + } + async handle(activity: Activity, context: HandlerContext): Promise { const { c } = context; @@ -65,8 +153,16 @@ export class AnnounceHandler extends BaseHandler { this.log("📥", `Announce: ${activity.actor} → ${objectUri}`); - // Resolve remote actor const actorUri = this.getActorUri(activity); + + // Announce の id は renote の URI として保存されるため、actor と同一オリジンの + // http(s) URL のみ許可する (javascript: や他サーバー URI の乗っ取りを防ぐ) + if (!isHttpUrl(activity.id) || !isSameOrigin(activity.id, actorUri)) { + this.warn(`Announce rejected: invalid activity id ${String(activity.id)}`); + return this.failure("Invalid Announce activity: id must be on the actor's origin"); + } + + // Resolve remote actor const remoteActor = await this.resolveActor(actorUri, c); // Find or fetch the note being announced @@ -74,52 +170,67 @@ export class AnnounceHandler extends BaseHandler { const remoteNoteService = this.getRemoteNoteService(c); let targetNote = await noteRepository.findByUri(objectUri); + // ローカルノートは HTTP で自サーバーを取得せず、ID で直接検索する + if (!targetNote && isLocalUrl(objectUri)) { + const localId = new URL(objectUri).pathname.match(/^\/notes\/([^/]+)$/)?.[1]; + targetNote = localId ? await noteRepository.findById(localId) : null; + if (!targetNote) { + this.warn(`Local note not found: ${objectUri}`); + return this.failure(`Local note not found: ${objectUri}`); + } + } + // If note doesn't exist locally, fetch it from remote if (!targetNote) { this.log("ℹ️", `Target note not found locally, fetching: ${objectUri}`); - const fetchService = new RemoteFetchService(); - - // Try unsigned fetch first - let result = await fetchService.fetchActivityPubObject(objectUri); - - // If failed with 401/403/404, try with HTTP Signature (Authorized Fetch) - if ( - !result.success && - result.error?.statusCode && - [401, 403, 404].includes(result.error.statusCode) - ) { - logger.debug( - { noteUri: objectUri, statusCode: result.error.statusCode }, - "Unsigned fetch failed, retrying with HTTP Signature", - ); - - // Get a local user's credentials for signing - const signatureConfig = await this.getSignatureConfig(c, context.baseUrl); - - if (signatureConfig) { - result = await fetchService.fetchActivityPubObject(objectUri, { - signature: signatureConfig, - }); + let fetched = await this.fetchRemoteObject(objectUri, c, context.baseUrl); + if (!fetched) { + return this.failure(`Failed to fetch remote note: ${objectUri}`); + } + + let noteObject = fetched.data; + if (!noteObject || typeof noteObject !== "object" || !isHttpUrl(noteObject.id)) { + this.warn(`Announce rejected: fetched object has no valid id (${objectUri})`); + return this.failure("Fetched object id origin does not match requested URI"); + } + + // The fetched document must be the object we asked for. If it claims a + // different id, refetch it from that id once and require an exact match. + if (normalizeUrl(noteObject.id) !== normalizeUrl(objectUri)) { + const canonicalUri: string = noteObject.id; + this.log("ℹ️", `Fetched id differs (${canonicalUri}), refetching canonical object`); + fetched = await this.fetchRemoteObject(canonicalUri, c, context.baseUrl); + noteObject = fetched?.data; + if ( + !noteObject || + typeof noteObject !== "object" || + typeof noteObject.id !== "string" || + normalizeUrl(noteObject.id) !== normalizeUrl(canonicalUri) + ) { + this.warn(`Announce rejected: canonical object id mismatch for ${canonicalUri}`); + return this.failure("Fetched object id origin does not match requested URI"); } } - if (!result.success) { - // 404 errors are common (deleted notes) - log at debug level - // Other errors (network issues, auth failures) are logged at warn level - const is404 = result.error?.statusCode === 404; - const logLevel = is404 ? "debug" : "warn"; - logger[logLevel]( - { noteUri: objectUri, err: result.error }, - is404 ? "Remote note not found (possibly deleted)" : "Failed to fetch remote note", - ); - return this.failure(`Failed to fetch remote note: ${objectUri}`); + if (isLocalUrl(extractRefUri(noteObject.attributedTo))) { + this.warn("Announce rejected: fetched object claims a local author"); + return this.failure("Fetched object claims a local author"); } - const noteObject = result.data as any; targetNote = await remoteNoteService.processNote(noteObject); } + // 公開範囲の確認: public / home 以外 (followers / specified / localOnly) は renote 不可 + if ( + targetNote.isDeleted || + targetNote.localOnly || + (targetNote.visibility !== "public" && targetNote.visibility !== "home") + ) { + this.warn(`Announce rejected: target note ${targetNote.id} is not publicly renotable`); + return this.failure("Target note cannot be renoted"); + } + // Create a renote (quote without text = pure boost) const id = this.generateId(); await noteRepository.create({ @@ -127,7 +238,7 @@ export class AnnounceHandler extends BaseHandler { userId: remoteActor.id, text: null, // No text = pure boost cw: null, - visibility: "public", + visibility: this.determineRenoteVisibility(activity), localOnly: false, replyId: null, renoteId: targetNote.id, @@ -135,7 +246,7 @@ export class AnnounceHandler extends BaseHandler { mentions: [], emojis: [], tags: [], - uri: activity.id ?? null, // Use the Announce activity ID as the note URI + uri: activity.id, // Use the Announce activity ID as the note URI (validated above) isDeleted: false, deletedAt: null, deletedById: null, diff --git a/packages/backend/src/services/ap/inbox/handlers/CreateHandler.ts b/packages/backend/src/services/ap/inbox/handlers/CreateHandler.ts index 0ba7f26d..c9ee2c4e 100644 --- a/packages/backend/src/services/ap/inbox/handlers/CreateHandler.ts +++ b/packages/backend/src/services/ap/inbox/handlers/CreateHandler.ts @@ -9,6 +9,7 @@ import type { Activity, HandlerContext, HandlerResult } from "../types.js"; import { BaseHandler } from "./BaseHandler.js"; import { getTimelineStreamService } from "../../../TimelineStreamService.js"; +import { extractRefUri, isHttpUrl, isSameOrigin } from "../../../../utils/federationUrl.js"; /** * Handler for Create activities @@ -45,7 +46,20 @@ export class CreateHandler extends BaseHandler { to?: string | string[]; cc?: string | string[]; id?: string; + attributedTo?: unknown; }; + + // 署名済みアクター以外の投稿を作成させない (author/origin binding) + const actorUri = this.getActorUri(activity); + const attributedTo = extractRefUri(apObject.attributedTo); + if (!attributedTo || attributedTo !== actorUri) { + this.warn(`Create rejected: attributedTo (${attributedTo}) does not match actor`); + return this.failure("Invalid Create activity: attributedTo does not match actor"); + } + if (!isHttpUrl(apObject.id) || !isSameOrigin(apObject.id, actorUri)) { + this.warn(`Create rejected: object id is not an http(s) URL on the actor's origin`); + return this.failure("Invalid Create activity: object id origin does not match actor"); + } this.log( "📥", `Create: Receiving ${objectType} from ${activity.actor} | to: ${JSON.stringify(apObject.to)} | cc: ${JSON.stringify(apObject.cc)}`, diff --git a/packages/backend/src/services/ap/inbox/handlers/DeleteHandler.ts b/packages/backend/src/services/ap/inbox/handlers/DeleteHandler.ts index 9b6dafc3..7d36a963 100644 --- a/packages/backend/src/services/ap/inbox/handlers/DeleteHandler.ts +++ b/packages/backend/src/services/ap/inbox/handlers/DeleteHandler.ts @@ -44,6 +44,12 @@ export class DeleteHandler extends BaseHandler { // Otherwise, try to resolve the actor and handle note deletion const remoteActor = await this.resolveActor(actorUri, c); + // ローカルユーザーのノートを連合経由で削除させない + if (remoteActor.host === null) { + this.warn(`Delete rejected: actor ${actorUri} resolves to a local user`); + return this.failure("Cannot delete local notes via federation"); + } + this.log("📥", `Delete: ${remoteActor.username}@${remoteActor.host} → ${objectUri}`); // Try to find and delete the note diff --git a/packages/backend/src/services/ap/inbox/handlers/UpdateHandler.ts b/packages/backend/src/services/ap/inbox/handlers/UpdateHandler.ts index e0b7c0eb..e4c4918f 100644 --- a/packages/backend/src/services/ap/inbox/handlers/UpdateHandler.ts +++ b/packages/backend/src/services/ap/inbox/handlers/UpdateHandler.ts @@ -32,6 +32,13 @@ export class UpdateHandler extends BaseHandler { const actorUri = this.getActorUri(activity); const remoteActor = await this.resolveActor(actorUri, c); + + // ローカルユーザー/ローカルノートを連合経由で更新させない + if (remoteActor.host === null) { + this.warn(`Update rejected: actor ${actorUri} resolves to a local user`); + return this.failure("Cannot update local users or notes via federation"); + } + const objectType = (object as { type?: string }).type; // Handle Person update (profile update) diff --git a/packages/backend/src/tests/unit/RemoteFetchService.redirect.test.ts b/packages/backend/src/tests/unit/RemoteFetchService.redirect.test.ts new file mode 100644 index 00000000..9eada13a --- /dev/null +++ b/packages/backend/src/tests/unit/RemoteFetchService.redirect.test.ts @@ -0,0 +1,87 @@ +/** + * RemoteFetchService redirect origin tests + * + * ActivityPub objects are trusted by the origin of their URL, so a redirect to + * another origin must not be followed silently. Also verifies that the final + * URL is exposed to callers. + */ + +import { describe, test, expect, beforeAll, afterAll } from "bun:test"; +import { safeFetchWithFinalUrl } from "../../lib/safeFetch.js"; +import { RemoteFetchService } from "../../services/ap/RemoteFetchService.js"; + +describe("RemoteFetchService cross-origin redirects", () => { + let serverA: ReturnType; + let serverB: ReturnType; + let baseA: string; + let baseB: string; + let previousAllow: string | undefined; + + const json = (body: unknown) => + new Response(JSON.stringify(body), { + headers: { "Content-Type": "application/activity+json" }, + }); + + beforeAll(() => { + previousAllow = process.env.ALLOW_PRIVATE_NETWORK_FETCH; + process.env.ALLOW_PRIVATE_NETWORK_FETCH = "true"; + serverB = Bun.serve({ + port: 0, + hostname: "127.0.0.1", + fetch: () => json({ id: "attacker" }), + }); + baseB = `http://127.0.0.1:${serverB.port}`; + serverA = Bun.serve({ + port: 0, + hostname: "127.0.0.1", + fetch(req) { + const { pathname } = new URL(req.url); + if (pathname === "/cross") { + return new Response(null, { status: 302, headers: { Location: `${baseB}/doc` } }); + } + if (pathname === "/same") { + return new Response(null, { status: 302, headers: { Location: "/doc" } }); + } + return json({ id: `${baseA}/doc` }); + }, + }); + baseA = `http://127.0.0.1:${serverA.port}`; + }); + + afterAll(() => { + serverA.stop(true); + serverB.stop(true); + if (previousAllow === undefined) delete process.env.ALLOW_PRIVATE_NETWORK_FETCH; + else process.env.ALLOW_PRIVATE_NETWORK_FETCH = previousAllow; + }); + + test("safeFetchWithFinalUrl exposes the final URL after redirects", async () => { + const { response, finalUrl } = await safeFetchWithFinalUrl(`${baseA}/same`); + await response.body?.cancel(); + expect(finalUrl.href).toBe(`${baseA}/doc`); + }); + + test("rejects a cross-origin redirect by default", async () => { + const service = new RemoteFetchService(); + const result = await service.fetchActivityPubObject(`${baseA}/cross`, { maxRetries: 0 }); + expect(result.success).toBe(false); + expect(result.error?.message).toContain("Cross-origin redirect"); + }); + + test("follows a same-origin redirect and reports finalUrl", async () => { + const service = new RemoteFetchService(); + const result = await service.fetchActivityPubObject(`${baseA}/same`, { maxRetries: 0 }); + expect(result.success).toBe(true); + expect(result.finalUrl).toBe(`${baseA}/doc`); + }); + + test("allows a cross-origin redirect when explicitly enabled", async () => { + const service = new RemoteFetchService(); + const result = await service.fetchActivityPubObject(`${baseA}/cross`, { + maxRetries: 0, + allowCrossOriginRedirect: true, + }); + expect(result.success).toBe(true); + expect(result.finalUrl).toBe(`${baseB}/doc`); + }); +}); diff --git a/packages/backend/src/tests/unit/RemoteNoteService.test.ts b/packages/backend/src/tests/unit/RemoteNoteService.test.ts index 4ea02f8f..10c4fb47 100644 --- a/packages/backend/src/tests/unit/RemoteNoteService.test.ts +++ b/packages/backend/src/tests/unit/RemoteNoteService.test.ts @@ -148,6 +148,79 @@ describe("RemoteNoteService", () => { expect(mockNoteRepository.create).not.toHaveBeenCalled(); }); + describe("origin validation", () => { + beforeEach(() => { + process.env.URL = "http://localhost:3000"; + }); + + test("should reject notes with a javascript: id", async () => { + await expect( + service.processNote({ + id: "javascript:alert(document.cookie)", + type: "Note", + attributedTo: "https://remote.example/users/testuser", + content: "

x

", + }), + ).rejects.toThrow(); + expect(mockNoteRepository.findByUri).not.toHaveBeenCalled(); + expect(mockNoteRepository.create).not.toHaveBeenCalled(); + }); + + test("should reject remote notes attributed to a local user", async () => { + await expect( + service.processNote({ + id: "https://remote.example/notes/evil", + type: "Note", + attributedTo: "http://localhost:3000/users/alice", + content: "

x

", + }), + ).rejects.toThrow(); + expect(mockNoteRepository.create).not.toHaveBeenCalled(); + }); + + test("should reject notes whose id origin differs from attributedTo", async () => { + await expect( + service.processNote({ + id: "https://evil.example/notes/1", + type: "Note", + attributedTo: "https://remote.example/users/testuser", + content: "

x

", + }), + ).rejects.toThrow(); + expect(mockNoteRepository.create).not.toHaveBeenCalled(); + }); + + test("should return existing local note instead of creating for local ids", async () => { + const localNote = { id: "local-1", userId: "alice", uri: null }; + mockNoteRepository.findById = mock(() => Promise.resolve(localNote as any)); + + const result = await service.processNote({ + id: "http://localhost:3000/notes/local-1", + type: "Note", + attributedTo: "http://localhost:3000/users/alice", + content: "

x

", + }); + + expect(result.id).toBe("local-1"); + expect(mockNoteRepository.findById).toHaveBeenCalledWith("local-1"); + expect(mockNoteRepository.create).not.toHaveBeenCalled(); + }); + + test("should throw for unknown local note ids", async () => { + mockNoteRepository.findById = mock(() => Promise.resolve(null)); + + await expect( + service.processNote({ + id: "http://localhost:3000/notes/missing", + type: "Note", + attributedTo: "http://localhost:3000/users/alice", + content: "

x

", + }), + ).rejects.toThrow(); + expect(mockNoteRepository.create).not.toHaveBeenCalled(); + }); + }); + test("should process note with attachments", async () => { const apNote = { id: "https://remote.example/notes/456", diff --git a/packages/backend/src/tests/unit/inbox-handlers/AnnounceHandler.test.ts b/packages/backend/src/tests/unit/inbox-handlers/AnnounceHandler.test.ts index 8ab57ec9..ad9623a2 100644 --- a/packages/backend/src/tests/unit/inbox-handlers/AnnounceHandler.test.ts +++ b/packages/backend/src/tests/unit/inbox-handlers/AnnounceHandler.test.ts @@ -12,23 +12,35 @@ import { AnnounceHandler } from "../../../services/ap/inbox/handlers/AnnounceHan import type { Activity, HandlerContext } from "../../../services/ap/inbox/types"; import type { Context } from "hono"; +// Document returned by the mocked RemoteFetchService (overridable per test) +const defaultFetchedNote = { + type: "Note", + id: "https://another.example.com/notes/1", + content: "

Test note

", + attributedTo: "https://another.example.com/users/author", +}; +let fetchedNote: Record = { ...defaultFetchedNote }; +// Per-URL documents (take precedence over fetchedNote) and recorded fetch URLs +let fetchedDocs: Record = {}; +let fetchCalls: string[] = []; +let fetchedFinalUrls: Record = {}; + // Mock the RemoteFetchService mock.module("../../../services/ap/RemoteFetchService", () => ({ RemoteFetchService: class { - fetchActivityPubObject = mock(() => - Promise.resolve({ + fetchActivityPubObject = mock((url: string) => { + fetchCalls.push(url); + return Promise.resolve({ success: true, - data: { - type: "Note", - id: "https://remote.example.com/notes/1", - content: "

Test note

", - attributedTo: "https://remote.example.com/users/remoteuser", - }, - }), - ); + data: fetchedDocs[url] ?? fetchedNote, + finalUrl: fetchedFinalUrls[url] ?? url, + }); + }); }, })); +const renotable = { visibility: "public", localOnly: false, isDeleted: false }; + describe("AnnounceHandler", () => { let handler: AnnounceHandler; let mockContext: HandlerContext; @@ -45,6 +57,7 @@ describe("AnnounceHandler", () => { id: "note-123", uri: "http://localhost:3000/notes/note-123", text: "Original note", + ...renotable, }), ), create: mock(() => Promise.resolve({ id: "renote-456" })), @@ -57,6 +70,7 @@ describe("AnnounceHandler", () => { id: "remote-note-789", uri: "https://remote.example.com/notes/1", text: "Remote note", + ...renotable, }), ), }; @@ -81,6 +95,11 @@ describe("AnnounceHandler", () => { }; beforeEach(() => { + fetchedNote = { ...defaultFetchedNote }; + fetchedDocs = {}; + fetchCalls = []; + fetchedFinalUrls = {}; + process.env.URL = "http://localhost:3000"; handler = new AnnounceHandler(); const honoContext = createMockHonoContext() as Context; @@ -134,6 +153,106 @@ describe("AnnounceHandler", () => { expect(mockRemoteNoteService.processNote).toHaveBeenCalled(); }); + test("should not trust a cross-origin id served by the requested origin", async () => { + mockNoteRepository.findByUri = mock(() => Promise.resolve(null)); + // another.example.com serves a document claiming to be victim's note + fetchedDocs = { + "https://another.example.com/notes/1": { + ...defaultFetchedNote, + id: "https://victim.example.com/notes/1", + attributedTo: "https://victim.example.com/users/bob", + content: "

forged

", + }, + // victim.example.com's real document for that id + "https://victim.example.com/notes/1": { + ...defaultFetchedNote, + id: "https://victim.example.com/notes/1", + attributedTo: "https://victim.example.com/users/bob", + content: "

real

", + }, + }; + + const activity: Activity = { + "@context": "https://www.w3.org/ns/activitystreams", + type: "Announce", + id: "https://remote.example.com/activities/announce-1", + actor: "https://remote.example.com/users/remoteuser", + object: "https://another.example.com/notes/1", + }; + + const result = await handler.handle(activity, mockContext); + + expect(result.success).toBe(true); + expect(fetchCalls).toEqual([ + "https://another.example.com/notes/1", + "https://victim.example.com/notes/1", + ]); + // Only the document fetched from the victim's own origin is processed + expect(mockRemoteNoteService.processNote.mock.calls[0][0].content).toBe("

real

"); + }); + + test("should reject a fetch whose final URL is on another origin", async () => { + mockNoteRepository.findByUri = mock(() => Promise.resolve(null)); + fetchedFinalUrls = { "https://another.example.com/notes/1": "https://evil.example.com/x" }; + + const activity: Activity = { + "@context": "https://www.w3.org/ns/activitystreams", + type: "Announce", + id: "https://remote.example.com/activities/announce-1", + actor: "https://remote.example.com/users/remoteuser", + object: "https://another.example.com/notes/1", + }; + + const result = await handler.handle(activity, mockContext); + + expect(result.success).toBe(false); + expect(mockRemoteNoteService.processNote).not.toHaveBeenCalled(); + expect(mockNoteRepository.create).not.toHaveBeenCalled(); + }); + + test("should reject fetched note attributed to a local user", async () => { + mockNoteRepository.findByUri = mock(() => Promise.resolve(null)); + fetchedNote = { + ...defaultFetchedNote, + attributedTo: "http://localhost:3000/users/alice", + }; + + const activity: Activity = { + "@context": "https://www.w3.org/ns/activitystreams", + type: "Announce", + id: "https://remote.example.com/activities/announce-1", + actor: "https://remote.example.com/users/remoteuser", + object: "https://another.example.com/notes/1", + }; + + const result = await handler.handle(activity, mockContext); + + expect(result.success).toBe(false); + expect(result.message).toContain("local author"); + expect(mockRemoteNoteService.processNote).not.toHaveBeenCalled(); + }); + + test("should resolve local note by id without fetching when uri lookup misses", async () => { + mockNoteRepository.findByUri = mock(() => Promise.resolve(null)); + mockNoteRepository.findById = mock(() => + Promise.resolve({ id: "note-123", uri: null, text: "Local note", ...renotable }), + ); + + const activity: Activity = { + "@context": "https://www.w3.org/ns/activitystreams", + type: "Announce", + id: "https://remote.example.com/activities/announce-1", + actor: "https://remote.example.com/users/remoteuser", + object: "http://localhost:3000/notes/note-123", + }; + + const result = await handler.handle(activity, mockContext); + + expect(result.success).toBe(true); + expect(mockNoteRepository.findById).toHaveBeenCalledWith("note-123"); + expect(mockRemoteNoteService.processNote).not.toHaveBeenCalled(); + }); + test("should reject activity with missing object", async () => { const activity: Activity = { "@context": "https://www.w3.org/ns/activitystreams", @@ -199,4 +318,114 @@ describe("AnnounceHandler", () => { expect(result.success).toBe(false); expect(result.message).toContain("Failed to handle Announce activity"); }); + + describe("security checks", () => { + const announce = (overrides: Partial = {}): Activity => ({ + "@context": "https://www.w3.org/ns/activitystreams", + type: "Announce", + id: "https://remote.example.com/activities/announce-1", + actor: "https://remote.example.com/users/remoteuser", + object: "http://localhost:3000/notes/note-123", + ...overrides, + }); + + test("should refetch by canonical id when fetched id differs, and accept exact match", async () => { + mockNoteRepository.findByUri = mock(() => Promise.resolve(null)); + const requested = "https://another.example.com/@author/1"; + const canonical = "https://another.example.com/notes/1"; + fetchedDocs = { + [requested]: { ...defaultFetchedNote, id: canonical }, + [canonical]: { ...defaultFetchedNote, id: canonical }, + }; + + const result = await handler.handle(announce({ object: requested }), mockContext); + + expect(result.success).toBe(true); + expect(fetchCalls).toEqual([requested, canonical]); + expect(mockRemoteNoteService.processNote.mock.calls[0][0].id).toBe(canonical); + }); + + test("should reject when the canonical refetch returns yet another id", async () => { + mockNoteRepository.findByUri = mock(() => Promise.resolve(null)); + const requested = "https://another.example.com/@author/1"; + const canonical = "https://another.example.com/notes/1"; + fetchedDocs = { + [requested]: { ...defaultFetchedNote, id: canonical }, + [canonical]: { ...defaultFetchedNote, id: "https://another.example.com/notes/2" }, + }; + + const result = await handler.handle(announce({ object: requested }), mockContext); + + expect(result.success).toBe(false); + expect(mockRemoteNoteService.processNote).not.toHaveBeenCalled(); + }); + + test("should reject renote of a followers-only local note", async () => { + mockNoteRepository.findByUri = mock(() => + Promise.resolve({ id: "note-123", ...renotable, visibility: "followers" }), + ); + + const result = await handler.handle(announce(), mockContext); + + expect(result.success).toBe(false); + expect(mockNoteRepository.create).not.toHaveBeenCalled(); + }); + + test("should reject renote of a specified (DM) note found via id fallback", async () => { + mockNoteRepository.findByUri = mock(() => Promise.resolve(null)); + mockNoteRepository.findById = mock(() => + Promise.resolve({ id: "note-123", uri: null, ...renotable, visibility: "specified" }), + ); + + const result = await handler.handle(announce(), mockContext); + + expect(result.success).toBe(false); + expect(mockNoteRepository.create).not.toHaveBeenCalled(); + }); + + test("should reject renote of a localOnly note", async () => { + mockNoteRepository.findByUri = mock(() => + Promise.resolve({ id: "note-123", ...renotable, localOnly: true }), + ); + + const result = await handler.handle(announce(), mockContext); + + expect(result.success).toBe(false); + expect(mockNoteRepository.create).not.toHaveBeenCalled(); + }); + + test("should derive renote visibility from to/cc", async () => { + const PUBLIC = "https://www.w3.org/ns/activitystreams#Public"; + + await handler.handle(announce({ to: [PUBLIC] }), mockContext); + await handler.handle( + announce({ to: ["https://remote.example.com/users/remoteuser/followers"], cc: [PUBLIC] }), + mockContext, + ); + await handler.handle( + announce({ to: ["https://remote.example.com/users/remoteuser/followers"] }), + mockContext, + ); + + const visibilities = mockNoteRepository.create.mock.calls.map((c: any[]) => c[0].visibility); + expect(visibilities).toEqual(["public", "home", "followers"]); + }); + + test("should reject a javascript: activity id", async () => { + const result = await handler.handle(announce({ id: "javascript:alert(1)" }), mockContext); + + expect(result.success).toBe(false); + expect(mockNoteRepository.create).not.toHaveBeenCalled(); + }); + + test("should reject an activity id on another origin (URI squatting)", async () => { + const result = await handler.handle( + announce({ id: "https://victim.example.com/notes/42" }), + mockContext, + ); + + expect(result.success).toBe(false); + expect(mockNoteRepository.create).not.toHaveBeenCalled(); + }); + }); }); diff --git a/packages/backend/src/tests/unit/inbox-handlers/CreateHandler.test.ts b/packages/backend/src/tests/unit/inbox-handlers/CreateHandler.test.ts index 74d0c52d..065edf00 100644 --- a/packages/backend/src/tests/unit/inbox-handlers/CreateHandler.test.ts +++ b/packages/backend/src/tests/unit/inbox-handlers/CreateHandler.test.ts @@ -168,4 +168,77 @@ describe("CreateHandler", () => { expect(result.success).toBe(false); expect(result.message).toContain("Failed to handle Create activity"); }); + describe("author/origin binding", () => { + const baseActivity = (object: Record): Activity => ({ + "@context": "https://www.w3.org/ns/activitystreams", + type: "Create", + id: "https://remote.example.com/activities/create-x", + actor: "https://remote.example.com/users/remoteuser", + object: { type: "Note", content: "

hi

", ...object }, + }); + + test("should reject Create whose attributedTo differs from actor", async () => { + const result = await handler.handle( + baseActivity({ + id: "https://remote.example.com/notes/1", + attributedTo: "https://remote.example.com/users/victim", + }), + mockContext, + ); + + expect(result.success).toBe(false); + expect(result.message).toContain("attributedTo does not match actor"); + expect(mockRemoteNoteService.processNote).not.toHaveBeenCalled(); + }); + + test("should reject Create with missing attributedTo", async () => { + const result = await handler.handle( + baseActivity({ id: "https://remote.example.com/notes/1" }), + mockContext, + ); + + expect(result.success).toBe(false); + expect(mockRemoteNoteService.processNote).not.toHaveBeenCalled(); + }); + + test("should accept attributedTo given as an object with matching id", async () => { + const result = await handler.handle( + baseActivity({ + id: "https://remote.example.com/notes/1", + attributedTo: { id: "https://remote.example.com/users/remoteuser", type: "Person" }, + }), + mockContext, + ); + + expect(result.success).toBe(true); + expect(mockRemoteNoteService.processNote).toHaveBeenCalled(); + }); + + test("should reject Create with a javascript: object id", async () => { + const result = await handler.handle( + baseActivity({ + id: "javascript:alert(1)", + attributedTo: "https://remote.example.com/users/remoteuser", + }), + mockContext, + ); + + expect(result.success).toBe(false); + expect(result.message).toContain("object id origin does not match actor"); + expect(mockRemoteNoteService.processNote).not.toHaveBeenCalled(); + }); + + test("should reject Create whose object id is on another origin", async () => { + const result = await handler.handle( + baseActivity({ + id: "https://victim.example.com/notes/1", + attributedTo: "https://remote.example.com/users/remoteuser", + }), + mockContext, + ); + + expect(result.success).toBe(false); + expect(mockRemoteNoteService.processNote).not.toHaveBeenCalled(); + }); + }); }); diff --git a/packages/backend/src/tests/unit/inbox-handlers/DeleteHandler.test.ts b/packages/backend/src/tests/unit/inbox-handlers/DeleteHandler.test.ts index d970a4e4..296aca40 100644 --- a/packages/backend/src/tests/unit/inbox-handlers/DeleteHandler.test.ts +++ b/packages/backend/src/tests/unit/inbox-handlers/DeleteHandler.test.ts @@ -113,6 +113,25 @@ describe("DeleteHandler", () => { expect(mockNoteRepository.delete).not.toHaveBeenCalled(); }); + test("should reject note deletion when the actor resolves to a local user", async () => { + mockRemoteActorService.resolveActor = mock(() => + Promise.resolve({ id: "remote-user-456", username: "alice", host: null }), + ); + + const activity: Activity = { + "@context": "https://www.w3.org/ns/activitystreams", + type: "Delete", + id: "http://localhost:3000/activities/delete-1", + actor: "http://localhost:3000/users/alice", + object: "http://localhost:3000/notes/1", + }; + + const result = await handler.handle(activity, mockContext); + + expect(result.success).toBe(false); + expect(mockNoteRepository.delete).not.toHaveBeenCalled(); + }); + test("should handle non-existent note gracefully", async () => { mockNoteRepository.findByUri = mock(() => Promise.resolve(null)); mockUserRepository.findByUri = mock(() => Promise.resolve(null)); diff --git a/packages/backend/src/tests/unit/inbox-handlers/UpdateHandler.test.ts b/packages/backend/src/tests/unit/inbox-handlers/UpdateHandler.test.ts new file mode 100644 index 00000000..bf90ad44 --- /dev/null +++ b/packages/backend/src/tests/unit/inbox-handlers/UpdateHandler.test.ts @@ -0,0 +1,138 @@ +/** + * UpdateHandler Unit Tests + * + * Tests the Update activity handler including: + * - Remote profile updates + * - Rejection of updates targeting local users / other actors + * - Note edit ownership checks + */ + +import { describe, test, expect, beforeEach, mock } from "bun:test"; +import { UpdateHandler } from "../../../services/ap/inbox/handlers/UpdateHandler"; +import type { Activity, HandlerContext } from "../../../services/ap/inbox/types"; +import type { Context } from "hono"; + +describe("UpdateHandler", () => { + let handler: UpdateHandler; + let mockContext: HandlerContext; + let mockUserRepository: any; + let mockNoteRepository: any; + let mockRemoteActorService: any; + let resolvedActor: { id: string; username: string; host: string | null }; + + const actorUri = "https://remote.example.com/users/remoteuser"; + + beforeEach(() => { + handler = new UpdateHandler(); + resolvedActor = { id: "remote-user-456", username: "remoteuser", host: "remote.example.com" }; + + mockUserRepository = { + update: mock(() => Promise.resolve()), + }; + mockNoteRepository = { + findByUri: mock(() => + Promise.resolve({ + id: "note-1", + userId: "remote-user-456", + uri: "https://remote.example.com/notes/1", + }), + ), + update: mock(() => Promise.resolve()), + }; + mockRemoteActorService = { + resolveActor: mock(() => Promise.resolve(resolvedActor)), + }; + + const contextMap = new Map(); + contextMap.set("userRepository", mockUserRepository); + contextMap.set("noteRepository", mockNoteRepository); + contextMap.set("remoteActorService", mockRemoteActorService); + + mockContext = { + c: { get: (key: string) => contextMap.get(key) } as Context, + recipientId: null, + baseUrl: "http://localhost:3000", + }; + }); + + test("should update the actor's own remote profile", async () => { + const activity: Activity = { + type: "Update", + id: `${actorUri}#updates/1`, + actor: actorUri, + object: { type: "Person", id: actorUri, name: "New Name" }, + }; + + const result = await handler.handle(activity, mockContext); + + expect(result.success).toBe(true); + expect(mockUserRepository.update).toHaveBeenCalledWith("remote-user-456", { + name: "New Name", + }); + }); + + test("should reject Update of a local user", async () => { + resolvedActor = { id: "local-user-1", username: "alice", host: null }; + const localUri = "http://localhost:3000/users/alice"; + + const activity: Activity = { + type: "Update", + id: `${localUri}#updates/1`, + actor: localUri, + object: { type: "Person", id: localUri, name: "pwned" }, + }; + + const result = await handler.handle(activity, mockContext); + + expect(result.success).toBe(false); + expect(mockUserRepository.update).not.toHaveBeenCalled(); + }); + + test("should reject Update of another actor", async () => { + const activity: Activity = { + type: "Update", + id: `${actorUri}#updates/1`, + actor: actorUri, + object: { type: "Person", id: "https://remote.example.com/users/other", name: "x" }, + }; + + const result = await handler.handle(activity, mockContext); + + expect(result.success).toBe(false); + expect(mockUserRepository.update).not.toHaveBeenCalled(); + }); + + test("should reject Note update by a non-owner", async () => { + mockNoteRepository.findByUri = mock(() => + Promise.resolve({ id: "note-1", userId: "someone-else", uri: "https://x/notes/1" }), + ); + + const activity: Activity = { + type: "Update", + id: `${actorUri}#updates/2`, + actor: actorUri, + object: { type: "Note", id: "https://remote.example.com/notes/1", content: "edited" }, + }; + + const result = await handler.handle(activity, mockContext); + + expect(result.success).toBe(false); + expect(mockNoteRepository.update).not.toHaveBeenCalled(); + }); + + test("should reject Note update when the actor resolves to a local user", async () => { + resolvedActor = { id: "remote-user-456", username: "alice", host: null }; + + const activity: Activity = { + type: "Update", + id: `${actorUri}#updates/3`, + actor: actorUri, + object: { type: "Note", id: "http://localhost:3000/notes/1", content: "edited" }, + }; + + const result = await handler.handle(activity, mockContext); + + expect(result.success).toBe(false); + expect(mockNoteRepository.update).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/backend/src/tests/unit/verifySignature.test.ts b/packages/backend/src/tests/unit/verifySignature.test.ts index 7c9235d9..94101f9b 100644 --- a/packages/backend/src/tests/unit/verifySignature.test.ts +++ b/packages/backend/src/tests/unit/verifySignature.test.ts @@ -1,14 +1,20 @@ /** * Verify Signature Middleware Unit Tests * - * Tests the fetch failure caching functionality in the HTTP Signature verification middleware. + * Tests the fetch failure caching functionality in the HTTP Signature verification middleware, + * keyId validation, key document → owner binding, and activity actor binding. */ import { describe, test, expect, beforeEach } from "bun:test"; import { clearFetchFailureCache, getFetchFailureCacheStats, + type KeyDocumentFetcher, + resolveVerifiedPublicKey, + validateKeyId, } from "../../middleware/verifySignature.js"; +import { validateActivity, ValidationErrorType } from "../../utils/activityValidation.js"; +import { isLocalHostname, isLocalUrl } from "../../utils/federationUrl.js"; describe("verifySignature fetch failure cache", () => { beforeEach(() => { @@ -38,3 +44,360 @@ describe("verifySignature fetch failure cache", () => { expect(Array.isArray(stats.entries)).toBe(true); }); }); + +describe("validateKeyId", () => { + beforeEach(() => { + process.env.URL = "http://localhost:3000"; + }); + + test("accepts a normal keyId and strips the fragment", () => { + expect(validateKeyId("https://remote.example/users/bob#main-key")).toBe( + "https://remote.example/users/bob", + ); + }); + + test("rejects dot-segment keyIds", () => { + expect(() => validateKeyId("https://victim.example/users/bob/../eve#main-key")).toThrow(); + expect(() => validateKeyId("https://victim.example/users/bob/%2e%2e/eve#main-key")).toThrow(); + expect(() => validateKeyId("https://victim.example/users/./bob#main-key")).toThrow(); + expect(() => validateKeyId("https://victim.example/users/bob\\..\\eve#main-key")).toThrow(); + }); + + test("rejects non-http(s) keyIds", () => { + expect(() => validateKeyId("javascript:alert(1)")).toThrow(); + expect(() => validateKeyId("not a url")).toThrow(); + }); + + test("rejects keyIds on the local host (e.g. /api/proxy)", () => { + expect(() => + validateKeyId("http://localhost:3000/api/proxy?url=https://evil.example/key#main-key"), + ).toThrow(); + }); +}); + +describe("resolveVerifiedPublicKey", () => { + const pem = "-----BEGIN PUBLIC KEY-----\nAAA\n-----END PUBLIC KEY-----"; + const otherPem = "-----BEGIN PUBLIC KEY-----\nBBB\n-----END PUBLIC KEY-----"; + + beforeEach(() => { + process.env.URL = "http://localhost:3000"; + }); + + /** + * Build a fetcher from a URL → document map. Documents are served from the + * requested URL unless `redirects` maps the URL to a different final URL. + */ + const makeFetcher = ( + docs: Record, + redirects: Record = {}, + ): KeyDocumentFetcher & { calls: string[] } => { + const calls: string[] = []; + const fetcher = (async (url: string) => { + calls.push(url); + if (!(url in docs)) throw new Error(`404 ${url}`); + return { data: docs[url], finalUrl: redirects[url] ?? url }; + }) as KeyDocumentFetcher & { calls: string[] }; + fetcher.calls = calls; + return fetcher; + }; + + const actorDoc = (id: string, keyId: string, keyPem = pem, owner: string | undefined = id) => ({ + id, + type: "Person", + publicKey: { id: keyId, ...(owner ? { owner } : {}), publicKeyPem: keyPem }, + }); + + test("accepts Mastodon/Misskey actor document without a second fetch", async () => { + const fetcher = makeFetcher({ + "https://remote.example/users/bob": actorDoc( + "https://remote.example/users/bob", + "https://remote.example/users/bob#main-key", + ), + }); + const result = await resolveVerifiedPublicKey( + "https://remote.example/users/bob#main-key", + "https://remote.example/users/bob", + fetcher, + ); + expect(result).toEqual({ publicKeyPem: pem, owner: "https://remote.example/users/bob" }); + expect(fetcher.calls).toHaveLength(1); + }); + + test("defaults owner to the document id when publicKey.owner is missing", async () => { + const fetcher = makeFetcher({ + "https://remote.example/users/bob": actorDoc( + "https://remote.example/users/bob", + "https://remote.example/users/bob#main-key", + pem, + undefined, + ), + }); + const result = await resolveVerifiedPublicKey( + "https://remote.example/users/bob#main-key", + "https://remote.example/users/bob", + fetcher, + ); + expect(result.owner).toBe("https://remote.example/users/bob"); + }); + + test("accepts GoToSocial stub via a separate owner fetch", async () => { + const keyId = "https://gts.example/users/bob/main-key"; + const fetcher = makeFetcher({ + [keyId]: actorDoc("https://gts.example/users/bob", keyId), + "https://gts.example/users/bob": actorDoc("https://gts.example/users/bob", keyId), + }); + const result = await resolveVerifiedPublicKey(keyId, keyId, fetcher); + expect(result.owner).toBe("https://gts.example/users/bob"); + expect(fetcher.calls).toEqual([keyId, "https://gts.example/users/bob"]); + }); + + test("accepts a standalone Key document when the owner actor lists the key", async () => { + const keyId = "https://remote.example/users/bob/publickey"; + const fetcher = makeFetcher({ + [keyId]: { + id: keyId, + type: "Key", + owner: "https://remote.example/users/bob", + publicKeyPem: pem, + }, + "https://remote.example/users/bob": actorDoc("https://remote.example/users/bob", keyId), + }); + const result = await resolveVerifiedPublicKey(keyId, keyId, fetcher); + expect(result.owner).toBe("https://remote.example/users/bob"); + }); + + test("rejects same-origin attacker Key document when owner's PEM differs", async () => { + // e.g. user-uploaded JSON on the victim's origin claiming bob's ownership + const keyId = "https://victim.example/files/evil.json"; + const fetcher = makeFetcher({ + [keyId]: { + id: keyId, + owner: "https://victim.example/users/bob", + publicKeyPem: otherPem, + }, + "https://victim.example/users/bob": actorDoc( + "https://victim.example/users/bob", + "https://victim.example/users/bob#main-key", + ), + }); + await expect(resolveVerifiedPublicKey(keyId, keyId, fetcher)).rejects.toThrow(); + }); + + test("rejects when the owner lists the keyId but with a different PEM", async () => { + const keyId = "https://gts.example/users/bob/main-key"; + const fetcher = makeFetcher({ + [keyId]: actorDoc("https://gts.example/users/bob", keyId, otherPem), + "https://gts.example/users/bob": actorDoc("https://gts.example/users/bob", keyId, pem), + }); + await expect(resolveVerifiedPublicKey(keyId, keyId, fetcher)).rejects.toThrow(); + }); + + test("rejects a redirected key fetch unless the owner document confirms it", async () => { + // keyId URL redirects (open redirect) to an attacker document claiming bob + const keyId = "https://victim.example/users/bob#main-key"; + const fetcher = makeFetcher( + { + "https://victim.example/users/bob": actorDoc( + "https://victim.example/users/bob", + keyId, + otherPem, + ), + }, + { "https://victim.example/users/bob": "https://victim.example/redirect-target" }, + ); + await expect( + resolveVerifiedPublicKey(keyId, "https://victim.example/users/bob", fetcher), + ).rejects.toThrow(); + }); + + test("rejects when the owner actor document itself was redirected", async () => { + const keyId = "https://gts.example/users/bob/main-key"; + const fetcher = makeFetcher( + { + [keyId]: actorDoc("https://gts.example/users/bob", keyId), + "https://gts.example/users/bob": actorDoc("https://gts.example/users/bob", keyId), + }, + { "https://gts.example/users/bob": "https://gts.example/elsewhere" }, + ); + await expect(resolveVerifiedPublicKey(keyId, keyId, fetcher)).rejects.toThrow(); + }); + + test("rejects owner mismatch (attacker doc claiming a victim owner)", async () => { + const keyId = "https://evil.example/users/eve#main-key"; + const fetcher = makeFetcher({ + "https://evil.example/users/eve": actorDoc( + "https://evil.example/users/eve", + keyId, + pem, + "https://victim.example/users/bob", + ), + "https://victim.example/users/bob": actorDoc( + "https://victim.example/users/bob", + "https://victim.example/users/bob#main-key", + otherPem, + ), + }); + await expect( + resolveVerifiedPublicKey(keyId, "https://evil.example/users/eve", fetcher), + ).rejects.toThrow(); + }); + + test("rejects a document whose publicKey.id does not match keyId", async () => { + const fetcher = makeFetcher({ + "https://remote.example/users/bob2": actorDoc( + "https://remote.example/users/bob2", + "https://remote.example/users/bob2#other-key", + ), + }); + await expect( + resolveVerifiedPublicKey( + "https://remote.example/users/bob2#main-key", + "https://remote.example/users/bob2", + fetcher, + ), + ).rejects.toThrow(); + }); + + test("rejects a key owner on the local host", async () => { + const keyId = "https://evil.example/keys/1"; + const fetcher = makeFetcher({ + [keyId]: { id: keyId, owner: "http://localhost:3000/users/alice", publicKeyPem: pem }, + }); + await expect(resolveVerifiedPublicKey(keyId, keyId, fetcher)).rejects.toThrow(); + }); + + test("rejects document without a public key", async () => { + const fetcher = makeFetcher({ + "https://remote.example/users/bob": { id: "https://remote.example/users/bob" }, + }); + await expect( + resolveVerifiedPublicKey( + "https://remote.example/users/bob#main-key", + "https://remote.example/users/bob", + fetcher, + ), + ).rejects.toThrow(); + }); +}); + +describe("validateActivity actor binding", () => { + beforeEach(() => { + process.env.URL = "http://localhost:3000"; + }); + + const activity = (actor: string) => ({ + "@context": "https://www.w3.org/ns/activitystreams", + id: `${actor}/activities/1`, + type: "Follow", + actor, + object: "http://localhost:3000/users/alice", + }); + + test("accepts actor equal to the verified key owner", () => { + const result = validateActivity( + activity("https://remote.example/users/bob"), + "https://remote.example/users/bob#main-key", + "https://remote.example/users/bob", + ); + expect(result.valid).toBe(true); + }); + + test("accepts keyId on a different path as long as the owner matches", () => { + const result = validateActivity( + activity("https://remote.example/users/bob"), + "https://remote.example/users/bob/publickey", + "https://remote.example/users/bob", + ); + expect(result.valid).toBe(true); + }); + + test("rejects actor that only shares a prefix with the keyId (bob vs bob2)", () => { + const result = validateActivity( + activity("https://remote.example/users/bob"), + "https://remote.example/users/bob2#main-key", + "https://remote.example/users/bob2", + ); + expect(result.valid).toBe(false); + expect(result.errors[0]?.type).toBe(ValidationErrorType.ACTOR_MISMATCH); + }); + + test("rejects dot-segment keyId whose verified owner is a different actor", () => { + const result = validateActivity( + activity("https://victim.example/users/bob"), + "https://victim.example/users/bob/../eve#main-key", + "https://victim.example/users/eve", + ); + expect(result.valid).toBe(false); + }); + + test("rejects when keyId is present but owner was not verified", () => { + const result = validateActivity( + activity("https://remote.example/users/bob"), + "https://remote.example/users/bob#main-key", + ); + expect(result.valid).toBe(false); + }); + + test("rejects activities whose actor is on the local host", () => { + const result = validateActivity( + activity("http://localhost:3000/users/alice"), + "http://localhost:3000/users/alice#main-key", + "http://localhost:3000/users/alice", + ); + expect(result.valid).toBe(false); + expect(result.errors[0]?.type).toBe(ValidationErrorType.ACTOR_MISMATCH); + }); + + test("rejects activities from a trailing-dot local hostname", () => { + const result = validateActivity( + activity("http://LOCALHOST.:3000/users/alice"), + "http://LOCALHOST.:3000/users/alice#main-key", + "http://LOCALHOST.:3000/users/alice", + ); + expect(result.valid).toBe(false); + }); + + test("rejects a javascript: activity id", () => { + const actor = "https://remote.example/users/bob"; + const result = validateActivity( + { ...activity(actor), id: "javascript:alert(1)" }, + `${actor}#main-key`, + actor, + ); + expect(result.valid).toBe(false); + expect(result.errors.some((e) => e.field === "id")).toBe(true); + }); + + test("rejects an activity id on another origin than the actor", () => { + const actor = "https://remote.example/users/bob"; + const result = validateActivity( + { ...activity(actor), id: "https://victim.example/users/alice/statuses/1/activity" }, + `${actor}#main-key`, + actor, + ); + expect(result.valid).toBe(false); + expect(result.errors.some((e) => e.field === "id")).toBe(true); + }); +}); + +describe("federationUrl local host detection", () => { + beforeEach(() => { + process.env.URL = "https://rox.example.com"; + }); + + test("treats trailing-dot and uppercase hostnames as local", () => { + expect(isLocalUrl("https://rox.example.com./users/alice")).toBe(true); + expect(isLocalUrl("https://ROX.Example.COM/users/alice")).toBe(true); + expect(isLocalHostname("rox.example.com.")).toBe(true); + }); + + test("does not treat other hosts as local", () => { + expect(isLocalUrl("https://rox.example.com.evil.example/users/alice")).toBe(false); + expect(isLocalHostname("other.example.com")).toBe(false); + }); + + test("normalizes a trailing dot in the configured URL", () => { + process.env.URL = "https://rox.example.com."; + expect(isLocalUrl("https://rox.example.com/users/alice")).toBe(true); + }); +}); diff --git a/packages/backend/src/utils/activityValidation.ts b/packages/backend/src/utils/activityValidation.ts index bb36fe70..b06ddb49 100644 --- a/packages/backend/src/utils/activityValidation.ts +++ b/packages/backend/src/utils/activityValidation.ts @@ -7,6 +7,8 @@ * @module utils/activityValidation */ +import { isHttpUrl, isLocalUrl, isSameOrigin } from "./federationUrl.js"; + /** * Validation error types */ @@ -179,6 +181,19 @@ export function validateRequiredFields(activity: any): ValidationResult { }); } + // Activity id (when present) must be an http(s) URI on the actor's origin. + // Prevents `javascript:` ids and squatting of other servers' activity ids + // (which are persisted as note URIs and used for deduplication). + if (activity.id !== undefined && activity.id !== null) { + if (!isHttpUrl(activity.id) || (actorUri && !isSameOrigin(activity.id, actorUri))) { + errors.push({ + type: ValidationErrorType.INVALID_URI, + field: "id", + message: "Activity id must be an HTTP(S) URI on the actor's origin", + }); + } + } + // Type-specific validation switch (activity.type) { case "Create": @@ -297,42 +312,55 @@ export function validateTimestamps(activity: any): ValidationResult { } /** - * Validate actor/keyId consistency + * Validate actor/signature consistency * - * Ensures that the signature keyId matches the actor. - * The keyId should be in format: {actorUri}#main-key or similar. + * Ensures that the activity actor is exactly the owner of the key that signed + * the request, as verified by the signature middleware against the fetched + * key document. A string prefix comparison between keyId and actor is NOT + * sufficient (e.g. `.../users/bob2#main-key` or `.../users/bob/../eve#main-key`). + * + * Also rejects activities whose actor is on the local host: local users never + * deliver to our own inbox over HTTP. * * @param activity - ActivityPub activity * @param keyId - KeyId from HTTP Signature + * @param signatureActor - Key owner verified by the signature middleware * @returns Validation result */ export function validateActorKeyIdConsistency( activity: any, keyId: string | undefined, + signatureActor?: string, ): ValidationResult { const errors: ValidationError[] = []; - if (!keyId) { - // If no keyId provided, skip this validation - // (signature verification middleware should handle this) - return { valid: true, errors: [] }; - } - const actorUri = extractActorUri(activity); if (!actorUri) { // Actor validation is handled by validateRequiredFields return { valid: true, errors: [] }; } - // KeyId should start with actor URI - // Common formats: - // - https://example.com/users/alice#main-key - // - https://example.com/users/alice/publickey - if (!keyId.startsWith(actorUri)) { + if (isLocalUrl(actorUri)) { errors.push({ type: ValidationErrorType.ACTOR_MISMATCH, field: "actor", - message: "Activity actor does not match signature keyId", + message: "Activity actor must not be a local user", + }); + return { valid: false, errors }; + } + + if (!keyId && !signatureActor) { + // If no signature info provided, skip this validation + // (signature verification middleware should handle this) + return { valid: true, errors: [] }; + } + + // Exact comparison against the verified key owner + if (!signatureActor || actorUri !== signatureActor) { + errors.push({ + type: ValidationErrorType.ACTOR_MISMATCH, + field: "actor", + message: "Activity actor does not match signature key owner", }); } @@ -349,13 +377,18 @@ export function validateActorKeyIdConsistency( * * @param activity - ActivityPub activity * @param keyId - Optional keyId from HTTP Signature + * @param signatureActor - Optional key owner verified by the signature middleware * @returns Validation result */ -export function validateActivity(activity: any, keyId?: string): ValidationResult { +export function validateActivity( + activity: any, + keyId?: string, + signatureActor?: string, +): ValidationResult { const results: ValidationResult[] = [ validateRequiredFields(activity), validateTimestamps(activity), - validateActorKeyIdConsistency(activity, keyId), + validateActorKeyIdConsistency(activity, keyId, signatureActor), ]; const allErrors = results.flatMap((r) => r.errors); diff --git a/packages/backend/src/utils/federationUrl.ts b/packages/backend/src/utils/federationUrl.ts new file mode 100644 index 00000000..912d933d --- /dev/null +++ b/packages/backend/src/utils/federationUrl.ts @@ -0,0 +1,148 @@ +/** + * Federation URL Utilities + * + * Helpers for validating ActivityPub identifiers received from remote servers + * (actor URIs, keyIds, object ids). Used to bind HTTP Signatures to actors and + * to enforce origin checks on inbound objects. + * + * @module utils/federationUrl + */ + +/** + * Get the hostname of this instance (derived from `process.env.URL`). + * + * @returns Local hostname (e.g. "rox.example.com") + */ +export function getLocalHostname(): string { + try { + return normalizeHostname(new URL(process.env.URL || "http://localhost:3000").hostname); + } catch { + return "localhost"; + } +} + +/** + * Normalize a hostname for comparison (lowercase, trailing dot removed). + * + * `example.com.` is the fully-qualified form of `example.com` and resolves to + * the same host, so both must compare equal. + * + * @param hostname - Hostname to normalize + * @returns Normalized hostname + */ +export function normalizeHostname(hostname: string): string { + return hostname.toLowerCase().replace(/\.+$/, ""); +} + +/** + * Check whether a hostname refers to this instance. + * + * @param hostname - Hostname to check (e.g. from `new URL(x).hostname`) + * @returns True if it equals the local hostname after normalization + */ +export function isLocalHostname(hostname: string): boolean { + return normalizeHostname(hostname) === getLocalHostname(); +} + +/** + * Parse a value as an absolute http(s) URL. + * + * @param value - Value to parse + * @returns Parsed URL, or null if the value is not a string or not an http(s) URL + */ +export function parseHttpUrl(value: unknown): URL | null { + if (typeof value !== "string" || value.length === 0) { + return null; + } + try { + const url = new URL(value); + if (url.protocol !== "https:" && url.protocol !== "http:") { + return null; + } + return url; + } catch { + return null; + } +} + +/** + * Check whether a value is an absolute http(s) URL. + * + * @param value - Value to check + * @returns True if the value is an http(s) URL + */ +export function isHttpUrl(value: unknown): value is string { + return parseHttpUrl(value) !== null; +} + +/** + * Check whether a URL points at this instance. + * + * @param value - URL to check + * @returns True if the URL's hostname equals the local hostname + */ +export function isLocalUrl(value: unknown): boolean { + const url = parseHttpUrl(value); + return url !== null && isLocalHostname(url.hostname); +} + +/** + * Check whether two URLs share the same origin (scheme, host and port). + * + * @param a - First URL + * @param b - Second URL + * @returns True if both are http(s) URLs with the same origin + */ +export function isSameOrigin(a: unknown, b: unknown): boolean { + const urlA = parseHttpUrl(a); + const urlB = parseHttpUrl(b); + return urlA !== null && urlB !== null && urlA.origin === urlB.origin; +} + +/** + * Normalize a URL string (WHATWG serialization). + * + * @param value - URL to normalize + * @returns Normalized href, or null if the value is not an http(s) URL + */ +export function normalizeUrl(value: unknown): string | null { + return parseHttpUrl(value)?.href ?? null; +} + +/** + * Check whether a raw URL string would be rewritten by URL path normalization. + * + * Detects dot segments (`.`, `..`, including percent-encoded forms), + * backslashes (treated as `/` by WHATWG parsers) and whitespace/control + * characters (silently stripped by WHATWG parsers). Such identifiers can make + * a string look like it belongs to one actor while resolving to another. + * + * @param value - Raw URL string + * @returns True if the string contains ambiguous path components + */ +export function hasAmbiguousPath(value: string): boolean { + // oxlint-disable-next-line no-control-regex + if (/[\\\s\u0000-\u001f\u007f]/.test(value)) { + return true; + } + return /\/(?:\.|%2e){1,2}(?=\/|\?|#|$)/i.test(value); +} + +/** + * Extract the URI of an `attributedTo`/`actor`-style reference. + * + * @param ref - String URI or object with an `id` + * @returns URI string, or null if it cannot be determined + */ +export function extractRefUri(ref: unknown): string | null { + if (typeof ref === "string") { + return ref; + } + if (ref && typeof ref === "object" && !Array.isArray(ref)) { + const id = (ref as { id?: unknown }).id; + if (typeof id === "string") { + return id; + } + } + return null; +}