fix(core): make HTTPS authoritative for key resolution (closes #2) by emretinaztepe · Pull Request #4 · binalyze/notar

emretinaztepe · 2026-04-26T12:21:09Z

Summary

Closes #2 — Revoked HTTPS key bypassed by faster valid DNS key (CWE-367).

resolvePublicKey raced HTTPS and DNS via Promise.any. Because the HTTPS branch resolves with { key, code: KEY_REVOKED } for revoked keys, a stale-but-still-valid DNS TXT record returning faster would win the race, and the HTTPS revocation signal was silently dropped. tryAllKeys (the no-keyId path) had the same bypass class — fetchPublicKeys filters revoked HTTPS keys out, so DNS could supply a now-revoked-but-stale matching key.

Changes

resolvePublicKey: await HTTPS first; HTTPS is authoritative when it returns a key (valid / expired / revoked) or KEY_NOT_FOUND from a successful response. DNS is consulted only when HTTPS is unreachable.
tryAllKeys: only consult DNS if fetchPublicKeys threw (HTTPS unreachable). Otherwise the publisher's HTTPS key manifest is the source of truth.
The Promise.any race is removed.
Regression tests:
- rejects revoked HTTPS key even when DNS reports it valid (no race bypass) — slow HTTPS + fast DNS, expects KEY_REVOKED.
- falls back to DNS only when HTTPS is unreachable — preserves the legitimate fallback behavior.

Trade-off

We give up the marginal latency benefit of resolving HTTPS and DNS in parallel, but HTTPS is a single RTT and is the publisher's source of truth for revocation. The race was a security regression masquerading as an optimization.

Test plan

pnpm --filter @binalyze/notar test — 123 passed
pnpm --filter @binalyze/notar typecheck — clean
pnpm lint — 0 warnings / 0 errors
New regression tests cover the PoC and the legitimate-fallback scenario

Made with Cursor

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 <emre@binalyze.com>

Copilot

Pull request overview

Security fix to eliminate a race-based revocation bypass in author-based signature verification by making the publisher’s HTTPS key manifest authoritative over DNS TXT lookups (closing issue #2 / CWE-367).

Changes:

Remove Promise.any key-resolution race; prefer HTTPS results (including revoked/expired) and treat HTTPS KEY_NOT_FOUND as definitive.
Restrict DNS TXT usage to a fallback only when HTTPS key retrieval is unavailable (applies to both resolvePublicKey and the no-keyId path via tryAllKeys).
Add regression tests covering revoked-HTTPS-vs-DNS and DNS fallback when HTTPS throws.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
packages/core/src/verify.ts	Reworks key resolution precedence to avoid DNS overriding HTTPS revocation/absence signals; limits DNS usage to HTTPS-unavailable cases.
packages/core/test/verify.spec.ts	Adds regression tests for revoked HTTPS key precedence and DNS fallback when HTTPS is unreachable.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-26T12:26:18Z

+  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);
+


The added regression tests cover the resolvePublicKey (keyId present) path, but the related fix in tryAllKeys (keyId omitted/empty) isn’t exercised here. Consider adding a test where HTTPS returns a manifest containing only a revoked key (so fetchPublicKeys succeeds but yields no candidates) while DNS returns a still-valid TXT key; verification should remain invalid and must not succeed via DNS.

Good catch — added in f250f24. New test no-keyId path: revoked HTTPS key + valid DNS key must not verify (tryAllKeys) strips the keyId from the signature to force the tryAllKeys path, has HTTPS list only a revoked key (so fetchPublicKeys yields no candidates) while DNS publishes a still-valid copy, and asserts verification fails (KEY_NOT_FOUND) — confirming DNS cannot rescue a revoked HTTPS state.

Copilot · 2026-04-26T12:26:19Z

+  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")) {


The new httpsAuthoritative behavior treats an HTTPS 200 + KEY_NOT_FOUND as definitive (DNS should not be consulted). Consider adding a regression test where HTTPS responds 200 with a manifest that does not include key_test, while DNS returns a valid TXT record for key_test; expected outcome should be KEY_NOT_FOUND with keySource: "https".

Good catch — added in f250f24. New test treats HTTPS 200 + KEY_NOT_FOUND as authoritative; does not consult DNS mocks an HTTPS manifest that does not list key_test while DNS publishes a valid TXT record for it, asserts KEY_NOT_FOUND with keySource: "https", and verifies the DNS endpoint is never fetched.

…vocation 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 <emre@binalyze.com>

Copilot AI review requested due to automatic review settings April 26, 2026 12:21

Copilot started reviewing on behalf of emretinaztepe April 26, 2026 12:21 View session

emretinaztepe mentioned this pull request Apr 26, 2026

Revoked HTTPS key bypassed by faster valid DNS key due to race-based key resolution #2

Closed

Copilot AI reviewed Apr 26, 2026

View reviewed changes

emretinaztepe merged commit be634fc into main Apr 26, 2026
2 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(core): make HTTPS authoritative for key resolution (closes #2)#4

fix(core): make HTTPS authoritative for key resolution (closes #2)#4
emretinaztepe merged 2 commits into
mainfrom
fix/issue-2-revocation-race

emretinaztepe commented Apr 26, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 26, 2026

Uh oh!

emretinaztepe Apr 26, 2026

Uh oh!

Copilot AI Apr 26, 2026

Uh oh!

emretinaztepe Apr 26, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

emretinaztepe commented Apr 26, 2026

Summary

Changes

Trade-off

Test plan

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 26, 2026

Choose a reason for hiding this comment

Uh oh!

emretinaztepe Apr 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 26, 2026

Choose a reason for hiding this comment

Uh oh!

emretinaztepe Apr 26, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants