From e271fc26027f9fd41d3a5ce476bfd2e59f4c4a3d Mon Sep 17 00:00:00 2001 From: ygd58 Date: Fri, 4 Sep 2026 07:17:22 +0200 Subject: [PATCH] fix(did): validate OIDC discovery jwks_uri targets during did:jwks resolution Fixes #191 did:jwks resolution (via jwks-did-resolver) falls back to fetching an OpenID configuration document when the direct JWKS endpoint is unavailable, and follows a jwks_uri found there. Unlike did:web, where the fetch target is built deterministically from the DID string itself, this OIDC-discovered target comes from response content an attacker-controlled DID host can shape - a classic SSRF shape, since it could point at an internal service, a cloud metadata endpoint (e.g. 169.254.169.254), or other loopback/private/link-local addresses. Added url-policy.ts: isSafeFetchTarget() requires https and rejects loopback, unspecified, private (RFC1918), and link-local IPv4/IPv6 targets, including IPv4-mapped/compatible IPv6 forms (both the rare dotted-quad notation and the canonical hex-group form WHATWG URL parsing normalizes to, e.g. ::ffff:7f00:1 for ::ffff:127.0.0.1). createPolicyEnforcedFetch() wraps a FetchLike so every request it makes is checked before being allowed through. Wired into get-did-resolver.ts: every fetch the jwks resolver makes (the direct jwks.json try, the OpenID configuration fetch, and the discovered jwks_uri fetch) goes through one function, so wrapping it there covers all three call sites uniformly, matching the issue's suggested fix. Scope: implements the issue's 'default to a conservative policy' alternative rather than the fully configurable API it also sketches (allowedJwksUriHosts / allowJwksUri / allowPrivateJwksUriTargets) - keeping this a focused security fix rather than a new public configuration surface, discussable as a follow-up if wanted. Also does not protect against DNS rebinding (a hostname resolving to a disallowed IP only at connect time), which needs socket-layer enforcement a fetch wrapper can't provide - noted in the code and changeset. Tests: 26 new tests covering the IPv4/IPv6 range checks (including a decimal-encoded-IP bypass attempt, which WHATWG URL parsing already canonicalizes before my check runs) and the fetch wrapper's delegate/throw behavior, plus a reproduction of the issue's exact scenario (a cloud metadata endpoint via a discovered jwks_uri). pnpm --filter @agentcommercekit/did exec vitest run - 104/104 passing (full package, no existing test broken by the wiring change). oxlint and oxfmt clean. AI usage disclosure: this fix was developed with Claude (Anthropic) assistance - identifying the fetch-wrapping point, writing the URL policy and its tests, and verifying locally. I reviewed and understand the change: it's a pre-request hostname/IP allowlist check applied uniformly to every fetch the jwks resolver can make, closing the gap where an OIDC-discovered jwks_uri bypassed the URL policy did:web already applies to its own (differently-derived) fetch target. --- .changeset/jwks-uri-ssrf-policy.md | 25 +++ .../did/src/did-resolvers/get-did-resolver.ts | 11 +- .../did/src/did-resolvers/url-policy.test.ts | 113 ++++++++++++ packages/did/src/did-resolvers/url-policy.ts | 172 ++++++++++++++++++ 4 files changed, 320 insertions(+), 1 deletion(-) create mode 100644 .changeset/jwks-uri-ssrf-policy.md create mode 100644 packages/did/src/did-resolvers/url-policy.test.ts create mode 100644 packages/did/src/did-resolvers/url-policy.ts diff --git a/.changeset/jwks-uri-ssrf-policy.md b/.changeset/jwks-uri-ssrf-policy.md new file mode 100644 index 00000000..bb89f411 --- /dev/null +++ b/.changeset/jwks-uri-ssrf-policy.md @@ -0,0 +1,25 @@ +--- +"@agentcommercekit/did": patch +--- + +Fixed `did:jwks` resolution not applying a URL policy to the `jwks_uri` +an OIDC discovery document can point resolution at. + +`did:jwks` resolution (via `jwks-did-resolver`) falls back to fetching an +OpenID configuration document when the direct JWKS endpoint is unavailable, +and follows a `jwks_uri` found there. Unlike `did:web`, where the fetch +target is built deterministically from the DID string itself, this +OIDC-discovered target comes from response content an attacker-controlled +DID host can shape - a classic SSRF shape, since it could point at an +internal service, a cloud metadata endpoint (e.g. `169.254.169.254`), or +other loopback/private/link-local addresses. + +Every fetch the jwks resolver makes (the direct JWKS try, the OpenID +configuration fetch, and the discovered `jwks_uri` fetch) now goes through a +policy check requiring `https:` and rejecting loopback, unspecified, +private (RFC1918), and link-local IPv4/IPv6 targets, including +IPv4-mapped/compatible IPv6 forms. + +This is a hostname/IP-literal check performed before each request; it does +not protect against DNS rebinding (a hostname resolving to a disallowed IP +only at connect time), which would require enforcement at the socket layer. diff --git a/packages/did/src/did-resolvers/get-did-resolver.ts b/packages/did/src/did-resolvers/get-did-resolver.ts index f322abe0..40eef406 100644 --- a/packages/did/src/did-resolvers/get-did-resolver.ts +++ b/packages/did/src/did-resolvers/get-did-resolver.ts @@ -4,6 +4,7 @@ import { getResolver as getKeyDidResolver } from "key-did-resolver" import { DidResolver } from "./did-resolver" import { getResolver as getPkhDidResolver } from "./pkh-did-resolver" +import { createPolicyEnforcedFetch } from "./url-policy" import { getResolver as getWebDidResolver, type DidWebResolverOptions, @@ -33,7 +34,15 @@ export function getDidResolver({ const webResolver = getWebDidResolver(webOptions) const jwksResolver = getJwksDidResolver({ ...webOptions, - fetch: webFetch ? (input, init) => webFetch(input, init) : globalThis.fetch, + // `jwks-did-resolver`'s OIDC discovery fallback fetches a `jwks_uri` it + // reads out of a discovery document - a target not derived from the DID + // itself, unlike did:web's deterministic URL. Every fetch this resolver + // makes (the direct jwks.json try, the discovery document, and the + // discovered jwks_uri) goes through this one function, so wrapping it + // here covers all three call sites uniformly. + fetch: createPolicyEnforcedFetch( + webFetch ? (input, init) => webFetch(input, init) : globalThis.fetch, + ), }) const pkhResolver = getPkhDidResolver() diff --git a/packages/did/src/did-resolvers/url-policy.test.ts b/packages/did/src/did-resolvers/url-policy.test.ts new file mode 100644 index 00000000..9e975ced --- /dev/null +++ b/packages/did/src/did-resolvers/url-policy.test.ts @@ -0,0 +1,113 @@ +import { beforeEach, describe, expect, it, vi } from "vitest" + +import type { FetchLike } from "../types" +import { createPolicyEnforcedFetch, isSafeFetchTarget } from "./url-policy" + +describe("isSafeFetchTarget", () => { + it("allows an ordinary https URL", () => { + expect(isSafeFetchTarget(new URL("https://example.com/jwks.json"))).toBe( + true, + ) + }) + + it("rejects http (non-https)", () => { + expect(isSafeFetchTarget(new URL("http://example.com/jwks.json"))).toBe( + false, + ) + }) + + it("rejects localhost", () => { + expect(isSafeFetchTarget(new URL("https://localhost/jwks.json"))).toBe( + false, + ) + }) + + it.each([ + "127.0.0.1", // loopback + "0.0.0.0", // unspecified + "10.1.2.3", // private (10.0.0.0/8) + "172.16.0.1", // private (172.16.0.0/12) + "172.31.255.255", // private (172.16.0.0/12, upper bound) + "192.168.1.1", // private (192.168.0.0/16) + "169.254.169.254", // link-local (cloud metadata endpoint) + ])("rejects disallowed IPv4 literal %s", (host) => { + expect(isSafeFetchTarget(new URL(`https://${host}/jwks.json`))).toBe(false) + }) + + it.each([ + "172.15.255.255", // just below the 172.16.0.0/12 private range + "172.32.0.0", // just above the 172.16.0.0/12 private range + "1.1.1.1", + "8.8.8.8", + ])("allows non-private IPv4 literal %s", (host) => { + expect(isSafeFetchTarget(new URL(`https://${host}/jwks.json`))).toBe(true) + }) + + it.each([ + "[::1]", // loopback + "[::]", // unspecified + "[fe80::1]", // link-local + "[fc00::1]", // unique local + "[fd00::1]", // unique local + "[::ffff:127.0.0.1]", // IPv4-mapped loopback + "[::ffff:169.254.169.254]", // IPv4-mapped link-local (cloud metadata) + ])("rejects disallowed IPv6 literal %s", (host) => { + expect(isSafeFetchTarget(new URL(`https://${host}/jwks.json`))).toBe(false) + }) + + it("allows a non-private IPv6 literal", () => { + expect( + isSafeFetchTarget(new URL("https://[2606:4700:4700::1111]/jwks.json")), + ).toBe(true) + }) + + it("rejects a decimal-encoded IPv4 loopback address (SSRF bypass attempt)", () => { + // WHATWG URL parsing canonicalizes non-dotted-quad IPv4 forms (decimal, + // octal, hex) into standard dotted-quad as part of host parsing, so this + // is caught by the same IPv4 check without any extra handling here. + const url = new URL("https://2130706433/jwks.json") // 127.0.0.1 + expect(url.hostname).toBe("127.0.0.1") + expect(isSafeFetchTarget(url)).toBe(false) + }) +}) + +describe("createPolicyEnforcedFetch", () => { + let mockFetch: FetchLike + + beforeEach(() => { + mockFetch = vi.fn().mockResolvedValue(new Response("{}")) + }) + + it("delegates to the wrapped fetch for an allowed URL", async () => { + const policyEnforcedFetch = createPolicyEnforcedFetch(mockFetch) + + await policyEnforcedFetch("https://example.com/jwks.json") + + expect(mockFetch).toHaveBeenCalledWith( + "https://example.com/jwks.json", + undefined, + ) + }) + + it("throws instead of delegating for a disallowed URL", async () => { + const policyEnforcedFetch = createPolicyEnforcedFetch(mockFetch) + + // This mirrors the issue's reproduction: an OIDC discovery document + // pointing jwks_uri at a cloud metadata endpoint. + await expect( + policyEnforcedFetch("http://169.254.169.254/latest/meta-data/"), + ).rejects.toThrow(/Refusing to fetch disallowed URL/) + + expect(mockFetch).not.toHaveBeenCalled() + }) + + it("checks a Request input's URL, not just string/URL inputs", async () => { + const policyEnforcedFetch = createPolicyEnforcedFetch(mockFetch) + + await expect( + policyEnforcedFetch(new Request("http://127.0.0.1/jwks.json")), + ).rejects.toThrow(/Refusing to fetch disallowed URL/) + + expect(mockFetch).not.toHaveBeenCalled() + }) +}) diff --git a/packages/did/src/did-resolvers/url-policy.ts b/packages/did/src/did-resolvers/url-policy.ts new file mode 100644 index 00000000..540b1f72 --- /dev/null +++ b/packages/did/src/did-resolvers/url-policy.ts @@ -0,0 +1,172 @@ +/** + * URL policy applied to fetch targets that are not directly derived from a + * DID identifier — specifically, the `jwks_uri` an OIDC discovery document + * can point `did:jwks` resolution at (see {@link createPolicyEnforcedFetch}). + * + * Unlike `did:web`, where the fetch target is built deterministically from + * the DID string itself, the OIDC discovery fallback reads a URL out of + * *response content* and then fetches that — a classic SSRF shape, since an + * attacker who controls the DID's host also controls where discovery points + * next. + */ +import type { FetchLike } from "../types" + +/** Hostnames rejected outright, regardless of scheme. */ +const DISALLOWED_HOSTNAMES = new Set(["localhost"]) + +/** + * Checks whether a dotted-quad IPv4 address falls in a disallowed range: + * loopback (127.0.0.0/8), unspecified (0.0.0.0/8), private (10.0.0.0/8, + * 172.16.0.0/12, 192.168.0.0/16), or link-local (169.254.0.0/16). + */ +function isDisallowedIPv4(hostname: string): boolean { + const match = /^(\d{1,3})\.(\d{1,3})\.(\d{1,3})\.(\d{1,3})$/.exec(hostname) + + if (!match) { + return false + } + + const parts = match.slice(1, 5).map(Number) + + if (parts.some((part) => part > 255)) { + return false + } + + const [a, b, c, d] = parts + + if ( + a === undefined || + b === undefined || + c === undefined || + d === undefined + ) { + return false + } + + if (a === 127 || a === 0) { + return true // loopback / unspecified + } + if (a === 10) { + return true // 10.0.0.0/8 + } + if (a === 172 && b >= 16 && b <= 31) { + return true // 172.16.0.0/12 + } + if (a === 192 && b === 168) { + return true // 192.168.0.0/16 + } + if (a === 169 && b === 254) { + return true // 169.254.0.0/16 (link-local) + } + + return false +} + +/** + * Checks whether an IPv6 address (without brackets) falls in a disallowed + * range: loopback (::1), unspecified (::), link-local (fe80::/10), unique + * local (fc00::/7), or an IPv4-mapped/compatible address whose embedded + * IPv4 address is itself disallowed. + */ +function isDisallowedIPv6(hostname: string): boolean { + const lower = hostname.toLowerCase() + + if (lower === "::1" || lower === "::") { + return true + } + + // IPv4-mapped (::ffff:a.b.c.d) or IPv4-compatible (::a.b.c.d) forms embed + // a dotted-quad tail — recurse into the IPv4 check for it. WHATWG URL + // parsing normalizes these into a canonical hex-group form (e.g. + // "::ffff:7f00:1" for "::ffff:127.0.0.1"), so check both the rare + // dotted-quad form and the canonical hex-group form. + const ipv4DottedTail = /(?:^|:)(\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3})$/.exec( + lower, + ) + if (ipv4DottedTail?.[1] && isDisallowedIPv4(ipv4DottedTail[1])) { + return true + } + + const ipv4MappedHex = /^::ffff:([0-9a-f]{1,4}):([0-9a-f]{1,4})$/.exec(lower) + if (ipv4MappedHex) { + const high = parseInt(ipv4MappedHex[1] ?? "0", 16) + const low = parseInt(ipv4MappedHex[2] ?? "0", 16) + const dottedQuad = [ + (high >> 8) & 0xff, + high & 0xff, + (low >> 8) & 0xff, + low & 0xff, + ].join(".") + if (isDisallowedIPv4(dottedQuad)) { + return true + } + } + + // fe80::/10 link-local: first hextet's top 10 bits are 1111111010, i.e. + // fe80-febf. + const firstHextet = lower.split(":")[0] + if (firstHextet && /^fe[89ab][0-9a-f]$/.test(firstHextet)) { + return true + } + + // fc00::/7 unique local: first hextet's top 7 bits are 1111110, i.e. + // fc00-fdff. + if (firstHextet && /^f[cd][0-9a-f]{2}$/.test(firstHextet)) { + return true + } + + return false +} + +/** + * Checks whether a URL is safe to fetch: `https:` only, and not targeting a + * loopback, unspecified, private, or link-local host. + * + * This is a hostname/IP-literal check performed before the request is made. + * It does not protect against DNS rebinding (a hostname that resolves to a + * disallowed IP only at connect time) — that requires enforcement at the + * socket layer, which a `fetch` wrapper cannot provide. + */ +export function isSafeFetchTarget(url: URL): boolean { + if (url.protocol !== "https:") { + return false + } + + let hostname = url.hostname.toLowerCase() + + if (DISALLOWED_HOSTNAMES.has(hostname)) { + return false + } + + // WHATWG URL parsing brackets IPv6 hostnames (e.g. "[::1]"); strip them + // before range-checking. + if (hostname.startsWith("[") && hostname.endsWith("]")) { + hostname = hostname.slice(1, -1) + } + + if (isDisallowedIPv4(hostname) || isDisallowedIPv6(hostname)) { + return false + } + + return true +} + +/** + * Wraps a {@link FetchLike} so every request it makes — including ones the + * wrapped fetch itself issues internally as a result of following content it + * already fetched, such as an OIDC-discovered `jwks_uri` — is checked against + * {@link isSafeFetchTarget} before being allowed through. + */ +export function createPolicyEnforcedFetch(fetch: FetchLike): FetchLike { + return async (input, init) => { + const url = new URL(input instanceof Request ? input.url : input.toString()) + + if (!isSafeFetchTarget(url)) { + throw new Error( + `Refusing to fetch disallowed URL: ${url.protocol}//${url.hostname}`, + ) + } + + return fetch(input, init) + } +}