From 3afb21b9920e7a50991abe433533d063df657dad Mon Sep 17 00:00:00 2001 From: Emre Tinaztepe Date: Sun, 26 Apr 2026 20:20:51 +0800 Subject: [PATCH 1/2] fix(core): make HTTPS authoritative for key resolution (CWE-367) resolvePublicKey raced HTTPS and DNS via Promise.any. The HTTPS branch resolved with { key, code: KEY_REVOKED } for revoked keys, but if a stale DNS TXT record returned a still-valid copy faster, DNS won the race and the revocation status from HTTPS was silently dropped. verifySignatureEntry then proceeded against the DNS key without the revocation check, returning valid: true. tryAllKeys had the same class of bypass: fetchPublicKeys filters revoked keys out, so only DNS could supply a now-revoked-but-stale key, and any matching signature would verify against it. Adopt an HTTPS-authoritative resolution policy: - await HTTPS first. - Honor any definitive HTTPS answer: a key (valid/expired/revoked) or KEY_NOT_FOUND from a successful response. - Fall back to DNS only when HTTPS is unreachable (network error, non-OK response, or fetchPublicKeys threw). - Drop the Promise.any race entirely. Add regression tests: - HTTPS revoked + DNS valid (slow HTTPS, fast DNS) -> KEY_REVOKED. - HTTPS unreachable + DNS valid -> success via DNS fallback. Fixes #2 Made-with: Cursor -e Signed-off-by: Emre Tinaztepe --- packages/core/src/verify.ts | 44 +++++++++++++------------- packages/core/test/verify.spec.ts | 51 +++++++++++++++++++++++++++++++ 2 files changed, 73 insertions(+), 22 deletions(-) diff --git a/packages/core/src/verify.ts b/packages/core/src/verify.ts index 1a2de42..a49f0e3 100644 --- a/packages/core/src/verify.ts +++ b/packages/core/src/verify.ts @@ -461,32 +461,28 @@ async function resolvePublicKey( options?: VerifyOptions, ): Promise { const resolveTxt = options?.resolveTxt !== false; + const httpsResult = await resolvePublicKeyFromHttps(author, keyId, options); - const httpsPromise = resolvePublicKeyFromHttps(author, keyId, options); + // HTTPS is authoritative whenever it produces a definitive answer: + // - A key (valid, revoked, or expired). + // - KEY_NOT_FOUND from a successful response (publisher's manifest excludes the keyId). + // DNS is consulted only as a fallback when HTTPS is unreachable + // (network error, non-OK response, or otherwise no usable answer). + const httpsAuthoritative = + !!httpsResult.key || httpsResult.code === VerifyErrorCode.KEY_NOT_FOUND; - if (!resolveTxt) { - return httpsPromise; + if (!resolveTxt || httpsAuthoritative) { + return httpsResult; } - const dnsPromise = fetchPublicKeyFromDns(author, keyId, { + const dns = await fetchPublicKeyFromDns(author, keyId, { fetch: options?.fetch, now: options?.now, - }).then((r): ResolvedKey => ({ - key: r.key, - code: r.code, - source: r.source, - })); - - try { - return await Promise.any([ - httpsPromise.then((r) => r.key ? r : Promise.reject(r)), - dnsPromise.then((r) => r.key ? r : Promise.reject(r)), - ]); - } catch { - // Both failed — return HTTPS result for its error info - const httpsResult = await httpsPromise; - return httpsResult; + }); + if (dns.key) { + return { key: dns.key, code: dns.code, source: "dns" }; } + return httpsResult; } // -- Unified verify ----------------------------------------------------------- @@ -518,20 +514,24 @@ async function tryAllKeys( options?: VerifyOptions, ): Promise { const candidates: Array<{ key: PublicKeyEntry; source: "https" | "dns" }> = []; + let httpsAvailable = false; try { const keys = await fetchPublicKeys(publisher, options); + httpsAvailable = true; for (const key of keys) candidates.push({ key, source: "https" }); } catch { /* HTTPS unavailable */ } - if (options?.resolveTxt !== false) { + // Only consult DNS when HTTPS is unreachable. Otherwise HTTPS is authoritative + // (including for revocation), so a now-revoked-but-stale DNS record cannot + // override the publisher's HTTPS key manifest. + if (!httpsAvailable && options?.resolveTxt !== false) { const dns = await fetchPublicKeyFromDns(publisher, "key", { fetch: options?.fetch, now: options?.now, }); if (dns.key && !dns.code) { - const dup = candidates.some((c) => c.key.publicKey === dns.key!.publicKey); - if (!dup) candidates.push({ key: dns.key, source: "dns" }); + candidates.push({ key: dns.key, source: "dns" }); } } diff --git a/packages/core/test/verify.spec.ts b/packages/core/test/verify.spec.ts index b30519c..0e9a094 100644 --- a/packages/core/test/verify.spec.ts +++ b/packages/core/test/verify.spec.ts @@ -455,6 +455,57 @@ signatures: expect(result.code).toBe(VerifyErrorCode.NO_SIGNATURES); }); + it("rejects revoked HTTPS key even when DNS reports it valid (no race bypass)", async () => { + const { privateKey, publicKey } = await generateKeyPair(); + const signed = await signFile(SAMPLE_MD, privateKey, { keyId: "key_test", publisher: "example.com" }); + const pubKeyB64 = uint8ToBase64(publicKey); + + const fetchMock = (async (input: string | URL | Request) => { + const url = typeof input === "string" ? input : input instanceof URL ? input.href : input.url; + if (url.includes("/.well-known/notar-keys.json")) { + await new Promise((r) => setTimeout(r, 100)); + return new Response(JSON.stringify({ + keys: [{ + keyId: "key_test", algorithm: "ed25519", publicKey: pubKeyB64, + expires: "2099-01-01T00:00:00.000Z", revoked: true, + }], + }), { status: 200 }); + } + if (url.includes("cloudflare-dns.com/dns-query")) { + const txt = `v=sk1; k=ed25519; p=${pubKeyB64}; exp=4102444800`; + return new Response(JSON.stringify({ Status: 0, Answer: [{ type: 16, data: `"${txt}"` }] }), { status: 200 }); + } + return new Response("not found", { status: 404 }); + }) as typeof globalThis.fetch; + + const result = await verifyFromAuthor(signed, { fetch: fetchMock }); + expect(result.valid).toBe(false); + expect(result.details?.signers![0].code).toBe(VerifyErrorCode.KEY_REVOKED); + expect(result.details?.signers![0].keySource).toBe("https"); + }); + + it("falls back to DNS only when HTTPS is unreachable", async () => { + const { privateKey, publicKey } = await generateKeyPair(); + const signed = await signFile(SAMPLE_MD, privateKey, { keyId: "key_test", publisher: "example.com" }); + const pubKeyB64 = uint8ToBase64(publicKey); + + const fetchMock = (async (input: string | URL | Request) => { + const url = typeof input === "string" ? input : input instanceof URL ? input.href : input.url; + if (url.includes("/.well-known/notar-keys.json")) { + throw new Error("HTTPS unreachable"); + } + if (url.includes("cloudflare-dns.com/dns-query")) { + const txt = `v=sk1; k=ed25519; p=${pubKeyB64}; exp=4102444800`; + return new Response(JSON.stringify({ Status: 0, Answer: [{ type: 16, data: `"${txt}"` }] }), { status: 200 }); + } + return new Response("not found", { status: 404 }); + }) as typeof globalThis.fetch; + + const result = await verifyFromAuthor(signed, { fetch: fetchMock }); + expect(result.valid).toBe(true); + expect(result.details?.signers![0].keySource).toBe("dns"); + }); + it("unsigned ZIP returns MISSING_MANIFEST", async () => { const zip = zipSync({ "file.txt": enc("hello") }); const result = await verifyFromAuthor(zip, { From f250f24a3a3b554768504d037b13ed550ffc31a0 Mon Sep 17 00:00:00 2001 From: Emre Tinaztepe Date: Sun, 26 Apr 2026 20:31:08 +0800 Subject: [PATCH 2/2] test(core): cover HTTPS-authoritative KEY_NOT_FOUND and tryAllKeys revocation Address Copilot review feedback on #4 by adding two regression tests: - HTTPS 200 + KEY_NOT_FOUND must be authoritative; DNS must not be consulted even if it publishes a matching TXT record. Asserts KEY_NOT_FOUND with keySource=https and that the DNS fetch is never invoked. - No-keyId (tryAllKeys) path: when HTTPS succeeds but lists only a revoked key (so fetchPublicKeys yields no candidates) and DNS still publishes a valid copy of the same key, verification must not succeed via DNS. This covers the second bypass class fixed in this PR. Made-with: Cursor -e Signed-off-by: Emre Tinaztepe --- packages/core/test/verify.spec.ts | 67 +++++++++++++++++++++++++++++++ 1 file changed, 67 insertions(+) diff --git a/packages/core/test/verify.spec.ts b/packages/core/test/verify.spec.ts index 0e9a094..956e620 100644 --- a/packages/core/test/verify.spec.ts +++ b/packages/core/test/verify.spec.ts @@ -506,6 +506,73 @@ signatures: expect(result.details?.signers![0].keySource).toBe("dns"); }); + it("treats HTTPS 200 + KEY_NOT_FOUND as authoritative; does not consult DNS", async () => { + const { privateKey, publicKey } = await generateKeyPair(); + const signed = await signFile(SAMPLE_MD, privateKey, { keyId: "key_test", publisher: "example.com" }); + const pubKeyB64 = uint8ToBase64(publicKey); + + let dnsCalled = false; + const fetchMock = (async (input: string | URL | Request) => { + const url = typeof input === "string" ? input : input instanceof URL ? input.href : input.url; + if (url.includes("/.well-known/notar-keys.json")) { + // HTTPS responds successfully but the manifest does not list key_test. + return new Response(JSON.stringify({ + keys: [{ + keyId: "other_key", algorithm: "ed25519", publicKey: pubKeyB64, + expires: "2099-01-01T00:00:00.000Z", + }], + }), { status: 200 }); + } + if (url.includes("cloudflare-dns.com/dns-query")) { + dnsCalled = true; + const txt = `v=sk1; k=ed25519; p=${pubKeyB64}; exp=4102444800`; + return new Response(JSON.stringify({ Status: 0, Answer: [{ type: 16, data: `"${txt}"` }] }), { status: 200 }); + } + return new Response("not found", { status: 404 }); + }) as typeof globalThis.fetch; + + const result = await verifyFromAuthor(signed, { fetch: fetchMock }); + expect(result.valid).toBe(false); + expect(result.details?.signers![0].code).toBe(VerifyErrorCode.KEY_NOT_FOUND); + expect(result.details?.signers![0].keySource).toBe("https"); + expect(dnsCalled).toBe(false); + }); + + it("no-keyId path: revoked HTTPS key + valid DNS key must not verify (tryAllKeys)", async () => { + const { privateKey, publicKey } = await generateKeyPair(); + // Sign with a real keyId so signFile produces a valid signature, then + // strip the keyId from the front matter so verifyFromAuthor takes the + // tryAllKeys path. + const signed = await signFile(SAMPLE_MD, privateKey, { keyId: "key_test", publisher: "example.com" }); + const stripped = signed.replace(/keyId: key_test/, "keyId: \"\""); + const pubKeyB64 = uint8ToBase64(publicKey); + + const fetchMock = (async (input: string | URL | Request) => { + const url = typeof input === "string" ? input : input instanceof URL ? input.href : input.url; + if (url.includes("/.well-known/notar-keys.json")) { + // HTTPS succeeds but the only listed key is revoked. + // fetchPublicKeys filters revoked keys, so candidates are empty, + // but the HTTPS source is still considered "available". + return new Response(JSON.stringify({ + keys: [{ + keyId: "key_test", algorithm: "ed25519", publicKey: pubKeyB64, + expires: "2099-01-01T00:00:00.000Z", revoked: true, + }], + }), { status: 200 }); + } + if (url.includes("cloudflare-dns.com/dns-query")) { + // DNS still publishes a valid copy of the same key — must not be used. + const txt = `v=sk1; k=ed25519; p=${pubKeyB64}; exp=4102444800`; + return new Response(JSON.stringify({ Status: 0, Answer: [{ type: 16, data: `"${txt}"` }] }), { status: 200 }); + } + return new Response("not found", { status: 404 }); + }) as typeof globalThis.fetch; + + const result = await verifyFromAuthor(stripped, { fetch: fetchMock }); + expect(result.valid).toBe(false); + expect(result.details?.signers![0].code).toBe(VerifyErrorCode.KEY_NOT_FOUND); + }); + it("unsigned ZIP returns MISSING_MANIFEST", async () => { const zip = zipSync({ "file.txt": enc("hello") }); const result = await verifyFromAuthor(zip, {