From 281a8fdf478b968bc847af7d2d3cf2de291f7ee1 Mon Sep 17 00:00:00 2001 From: alltheseas Date: Wed, 11 Mar 2026 02:33:04 -0500 Subject: [PATCH] fix: enforce sync signature verification on kind:0 profile events Add forceSync parameter to verifySignature() so kind:0 events always get synchronous verification regardless of asyncSigVerification setting. This prevents undefined return values from dropping valid profile events. Only failed verifications return early; valid events continue to caching and emission. Non-relay kind:0 events are also verified. Harden signature cache against poisoning: positive cache hits now require the cached sig to match the current event's sig before short-circuiting. Negative cache entries always trigger re-verification. Co-Authored-By: Claude Opus 4.6 --- core/src/events/validation.ts | 21 ++- core/src/relay/signature-verification.test.ts | 150 ++++++++++++++++++ core/src/subscription/index.test.ts | 141 +++++++++++++++- core/src/subscription/index.ts | 34 ++-- 4 files changed, 321 insertions(+), 25 deletions(-) diff --git a/core/src/events/validation.ts b/core/src/events/validation.ts index 308f8dbae..861c78841 100644 --- a/core/src/events/validation.ts +++ b/core/src/events/validation.ts @@ -41,18 +41,23 @@ export const verifiedSignatures = new LRUCache({ * @param event {NDKEvent} The event to verify * @returns {boolean | undefined} True if the signature is valid, false if it is invalid, and undefined if the signature has not been verified yet. */ -export function verifySignature(this: NDKEvent, persist: boolean): boolean | undefined { +export function verifySignature(this: NDKEvent, persist: boolean, forceSync = false): boolean | undefined { if (typeof this.signatureVerified === "boolean") return this.signatureVerified; - const prevVerification = verifiedSignatures.get(this.id); - if (prevVerification !== null) { - this.signatureVerified = !!prevVerification; - return this.signatureVerified; - } - try { + const prevVerification = verifiedSignatures.get(this.id); + if (prevVerification !== null && prevVerification !== false) { + if (prevVerification === this.sig && this.getEventHash() === this.id) { + // Positive cache hit — sig matches and payload hashes to the claimed id + this.signatureVerified = true; + return true; + } + // Sig mismatch or payload tampered — re-verify + } + // Negative or missing cache entries always re-verify + // Use async verification if enabled (either via worker or custom function) - if (this.ndk?.asyncSigVerification) { + if (this.ndk?.asyncSigVerification && !forceSync) { // Capture the relay in a closure before the async call const relayForVerification = this.relay; diff --git a/core/src/relay/signature-verification.test.ts b/core/src/relay/signature-verification.test.ts index 10ef01e9c..cb6cf5d82 100644 --- a/core/src/relay/signature-verification.test.ts +++ b/core/src/relay/signature-verification.test.ts @@ -1,6 +1,8 @@ import { beforeEach, describe, expect, test, vi } from "vitest"; import { NDKEvent } from "../events/index"; +import { verifiedSignatures } from "../events/validation"; import { NDK } from "../ndk/index"; +import { NDKPrivateKeySigner } from "../signers/private-key/index"; import { NDKRelay } from "./index"; describe("Signature Verification Sampling", () => { @@ -190,3 +192,151 @@ describe("Invalid Signature Handling", () => { expect(ratio).toBeCloseTo(0.1, 1); }); }); + +describe("Signature cache poisoning resistance", () => { + beforeEach(() => { + verifiedSignatures.clear(); + }); + + test("negative cache entry triggers re-verification instead of short-circuiting", () => { + const ndk = new NDK(); + const event = new NDKEvent(ndk, { + kind: 0, + created_at: 1234567890, + pubkey: "a".repeat(64), + id: "poisoned-id", + sig: "some-sig", + tags: [], + content: "{}", + }); + + // Poison the cache with a negative entry + verifiedSignatures.set("poisoned-id", false); + + // Spy on serialize to prove verification code path was entered + // (serialize is called inside the sync verification branch) + const serializeSpy = vi.spyOn(event, "serialize"); + + event.verifySignature(false); + + // If the negative cache short-circuited, serialize would never be called. + // The fact that it IS called proves re-verification happened. + expect(serializeSpy).toHaveBeenCalled(); + serializeSpy.mockRestore(); + }); + + test("positive cache hit with matching sig and valid hash short-circuits schnorr verify", async () => { + const ndk = new NDK(); + const signer = NDKPrivateKeySigner.generate(); + + // Create a real signed event so id matches the payload hash + const event = new NDKEvent(ndk); + event.kind = 0; + event.content = '{"name":"cached"}'; + await event.sign(signer); + + // Populate cache with the real sig + verifiedSignatures.set(event.id, event.sig!); + + // Clear the per-instance flag so cache lookup is exercised + event.signatureVerified = undefined as any; + + const result = event.verifySignature(false); + + // Should return true from cache — hash matches, sig matches + expect(result).toBe(true); + }); + + test("positive cache hit with mismatched sig triggers re-verification", () => { + const ndk = new NDK(); + const event = new NDKEvent(ndk, { + kind: 0, + created_at: 1234567890, + pubkey: "a".repeat(64), + id: "cached-id", + sig: "different-sig", + tags: [], + content: "{}", + }); + + // Cache was set by a different event with a different sig + verifiedSignatures.set("cached-id", "original-sig"); + + const serializeSpy = vi.spyOn(event, "serialize"); + event.verifySignature(false); + + // Mismatched sig should NOT trust the cache — must re-verify + expect(serializeSpy).toHaveBeenCalled(); + serializeSpy.mockRestore(); + }); + + test("real signed event passes after negative cache poisoning", async () => { + const ndk = new NDK(); + const signer = NDKPrivateKeySigner.generate(); + + // Create and sign a real kind:0 profile event + const event = new NDKEvent(ndk); + event.kind = 0; + event.content = '{"name":"alice"}'; + await event.sign(signer); + + // Poison the cache: a forged event with the same id was seen first + verifiedSignatures.set(event.id, false); + + // Despite the poisoned cache, the valid event must still verify + const result = event.verifySignature(true, true); + expect(result).toBe(true); + }); + + test("real signed event rejected when cache holds a different valid sig", async () => { + const ndk = new NDK(); + const signer = NDKPrivateKeySigner.generate(); + + // Create and sign a real event + const event = new NDKEvent(ndk); + event.kind = 0; + event.content = '{"name":"bob"}'; + await event.sign(signer); + + // Cache holds a "valid" sig that doesn't match this event's actual sig + verifiedSignatures.set(event.id, "aaa" + event.sig!.slice(3)); + + // Should not trust the cached sig — must re-verify cryptographically + // The real sig is valid, so it should pass + const result = event.verifySignature(true, true); + expect(result).toBe(true); + }); + + test("tampered payload with replayed id+sig is rejected despite cache hit", async () => { + const ndk = new NDK(); + const signer = NDKPrivateKeySigner.generate(); + + // Create and sign a legitimate kind:0 event + const event = new NDKEvent(ndk); + event.kind = 0; + event.content = '{"name":"real"}'; + await event.sign(signer); + + const originalId = event.id; + const originalSig = event.sig!; + + // First verification succeeds and populates the cache + expect(event.verifySignature(true, true)).toBe(true); + expect(verifiedSignatures.get(originalId)).toBe(originalSig); + + // Simulate a relay replaying the same (id, sig) but with tampered content + const tampered = new NDKEvent(ndk, { + kind: 0, + created_at: event.created_at!, + pubkey: event.pubkey, + id: originalId, + sig: originalSig, + tags: [], + content: '{"name":"evil"}', // altered payload + }); + + // Cache has matching id+sig, but payload hash won't match the id + const result = tampered.verifySignature(true, true); + expect(result).toBe(false); + }); +}); diff --git a/core/src/subscription/index.test.ts b/core/src/subscription/index.test.ts index 0b0b93f6c..a7b2693e1 100644 --- a/core/src/subscription/index.test.ts +++ b/core/src/subscription/index.test.ts @@ -1,5 +1,7 @@ -import { describe, expect, it, vi } from "vitest"; // Added describe, it, expect +import { beforeEach, describe, expect, it, vi } from "vitest"; import { NDKEvent } from "../events"; +import { NDKKind } from "../events/kinds/index"; +import { verifiedSignatures } from "../events/validation"; import { NDK } from "../ndk"; import { NDKSubscription } from "."; @@ -111,3 +113,140 @@ describe("NDKSubscriptionFilters", () => { }); }); }); + +describe("Kind:0 profile signature enforcement", () => { + const validPubkey = "a".repeat(64); + + function makeProfileEvent(testNdk: NDK): NDKEvent { + return new NDKEvent(testNdk, { + kind: NDKKind.Metadata, + created_at: Math.floor(Date.now() / 1000), + pubkey: validPubkey, + id: Math.random().toString(36).substring(2), + sig: "b".repeat(128), + tags: [], + content: '{"name":"test"}', + }); + } + + function mockRelay() { + return { + shouldValidateEvent: () => true, + addValidatedEvent: vi.fn(), + addNonValidatedEvent: vi.fn(), + url: "wss://mock.relay", + }; + } + + beforeEach(() => { + verifiedSignatures.clear(); + }); + + it("drops kind:0 events with invalid signatures via relay", () => { + const testNdk = new NDK(); + const sub = new NDKSubscription(testNdk, { kinds: [0] }, { skipValidation: true }); + const event = makeProfileEvent(testNdk); + const relay = mockRelay(); + + const mockedVerify = vi.spyOn(event, "verifySignature").mockReturnValue(false); + const mockedEmit = vi.spyOn(sub, "emit" as any); + + sub.eventReceived(event, relay as any); + + expect(mockedVerify).toHaveBeenCalled(); + expect(mockedEmit).not.toHaveBeenCalled(); + mockedVerify.mockRestore(); + mockedEmit.mockRestore(); + }); + + it("emits kind:0 events with valid signatures via relay", () => { + const testNdk = new NDK(); + const sub = new NDKSubscription(testNdk, { kinds: [0] }, { skipValidation: true }); + const event = makeProfileEvent(testNdk); + const relay = mockRelay(); + + const mockedVerify = vi.spyOn(event, "verifySignature").mockReturnValue(true); + const mockedEmit = vi.spyOn(sub, "emit" as any); + + sub.eventReceived(event, relay as any); + + expect(mockedVerify).toHaveBeenCalled(); + expect(mockedEmit).toHaveBeenCalled(); + mockedVerify.mockRestore(); + mockedEmit.mockRestore(); + }); + + it("uses forceSync=true for kind:0 even when asyncSigVerification is enabled", () => { + const testNdk = new NDK(); + testNdk.asyncSigVerification = true; + const sub = new NDKSubscription(testNdk, { kinds: [0] }, { skipValidation: true }); + const event = makeProfileEvent(testNdk); + const relay = mockRelay(); + + const mockedVerify = vi.spyOn(event, "verifySignature").mockReturnValue(true); + + sub.eventReceived(event, relay as any); + + // Should be called with (true, true) — persist=true, forceSync=true + expect(mockedVerify).toHaveBeenCalledWith(true, true); + mockedVerify.mockRestore(); + }); + + it("drops kind:0 events with invalid signatures without relay", () => { + const testNdk = new NDK(); + const sub = new NDKSubscription(testNdk, { kinds: [0] }, { skipValidation: true }); + const event = makeProfileEvent(testNdk); + + const mockedVerify = vi.spyOn(event, "verifySignature").mockReturnValue(false); + const mockedEmit = vi.spyOn(sub, "emit" as any); + + // No relay passed + sub.eventReceived(event, undefined); + + expect(mockedVerify).toHaveBeenCalledWith(true, true); + expect(mockedEmit).not.toHaveBeenCalled(); + mockedVerify.mockRestore(); + mockedEmit.mockRestore(); + }); + + it("emits kind:0 events with valid signatures without relay", () => { + const testNdk = new NDK(); + const sub = new NDKSubscription(testNdk, { kinds: [0] }, { skipValidation: true }); + const event = makeProfileEvent(testNdk); + + const mockedVerify = vi.spyOn(event, "verifySignature").mockReturnValue(true); + const mockedEmit = vi.spyOn(sub, "emit" as any); + + sub.eventReceived(event, undefined); + + expect(mockedVerify).toHaveBeenCalled(); + expect(mockedEmit).toHaveBeenCalled(); + mockedVerify.mockRestore(); + mockedEmit.mockRestore(); + }); + + it("does not force-verify non-kind:0 events without relay", () => { + const testNdk = new NDK(); + const sub = new NDKSubscription(testNdk, { kinds: [1] }, { skipValidation: true }); + const event = new NDKEvent(testNdk, { + kind: NDKKind.Text, + created_at: Math.floor(Date.now() / 1000), + pubkey: validPubkey, + id: Math.random().toString(36).substring(2), + sig: "b".repeat(128), + tags: [], + content: "hello", + }); + + const mockedVerify = vi.spyOn(event, "verifySignature"); + const mockedEmit = vi.spyOn(sub, "emit" as any); + + // kind:1 without relay — should NOT trigger verification + sub.eventReceived(event, undefined); + + expect(mockedVerify).not.toHaveBeenCalled(); + expect(mockedEmit).toHaveBeenCalled(); + mockedVerify.mockRestore(); + mockedEmit.mockRestore(); + }); +}); diff --git a/core/src/subscription/index.ts b/core/src/subscription/index.ts index 382dff286..992ea6aff 100644 --- a/core/src/subscription/index.ts +++ b/core/src/subscription/index.ts @@ -2,7 +2,7 @@ import { EventEmitter } from "tseep"; import type { NDKEventId, NDKSignedEvent, NostrEvent } from "../events/index.js"; import { createSignedEvent, NDKEvent } from "../events/index.js"; -import type { NDKKind } from "../events/kinds/index.js"; +import { NDKKind } from "../events/kinds/index.js"; import { verifiedSignatures } from "../events/validation.js"; import { wrapEvent } from "../events/wrap.js"; import type { NDK } from "../ndk/index.js"; @@ -869,36 +869,38 @@ export class NDKSubscription extends EventEmitter<{ // verify it if (relay) { - // Check if we need to verify this event based on sampling - const shouldVerify = relay.shouldValidateEvent(); + const mustVerify = ndkEvent.kind === NDKKind.Metadata; + const shouldVerify = mustVerify || relay.shouldValidateEvent(); if (shouldVerify && !this.skipVerification) { - // Set the relay on the event for async verification ndkEvent.relay = relay; - // Attempt verification - if (this.ndk.asyncSigVerification) { - // Async verification - call verifySignature but don't wait for result - // The validation stats will be tracked in the async callback - ndkEvent.verifySignature(true); - } else { - // Sync verification - check result immediately - if (!ndkEvent.verifySignature(true)) { + if (mustVerify || !this.ndk.asyncSigVerification) { + // Sync verification: always for kind:0, otherwise when async is disabled + if (!ndkEvent.verifySignature(true, mustVerify)) { this.debug("Event failed signature validation", event); - // Report the invalid signature with relay information through the centralized method this.ndk.reportInvalidSignature(ndkEvent, relay); return; } - - // Track successful validation relay.addValidatedEvent(); + } else { + // Async verification for non-profile events when async is enabled + ndkEvent.verifySignature(true); } } else { - // We skipped verification for this event relay.addNonValidatedEvent(); } } + // Verify kind:0 events even without a relay reference + if (!relay && ndkEvent.kind === NDKKind.Metadata && !this.skipVerification) { + if (!ndkEvent.verifySignature(true, true)) { + this.debug("Event failed signature validation (no relay)", event); + this.ndk.reportInvalidSignature(ndkEvent); + return; + } + } + if (this.ndk.cacheAdapter && !this.opts.dontSaveToCache && !kindIsEphemeral(ndkEvent.kind as NDKKind) && !fromCache) { this.ndk.cacheAdapter.setEvent(ndkEvent, this.filters, relay); }