Skip to content

Commit 47fc8e9

Browse files
alltheseasclaude
andcommitted
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 <noreply@anthropic.com>
1 parent 58f2539 commit 47fc8e9

File tree

4 files changed

+287
-22
lines changed

4 files changed

+287
-22
lines changed

core/src/events/validation.ts

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -41,18 +41,23 @@ export const verifiedSignatures = new LRUCache<string, false | string>({
4141
* @param event {NDKEvent} The event to verify
4242
* @returns {boolean | undefined} True if the signature is valid, false if it is invalid, and undefined if the signature has not been verified yet.
4343
*/
44-
export function verifySignature(this: NDKEvent, persist: boolean): boolean | undefined {
44+
export function verifySignature(this: NDKEvent, persist: boolean, forceSync = false): boolean | undefined {
4545
if (typeof this.signatureVerified === "boolean") return this.signatureVerified;
4646

4747
const prevVerification = verifiedSignatures.get(this.id);
48-
if (prevVerification !== null) {
49-
this.signatureVerified = !!prevVerification;
50-
return this.signatureVerified;
48+
if (prevVerification !== null && prevVerification !== false) {
49+
if (prevVerification === this.sig) {
50+
// Positive cache hit — same signature was previously verified valid
51+
this.signatureVerified = true;
52+
return true;
53+
}
54+
// Cached sig doesn't match this event's sig — re-verify
5155
}
56+
// Negative or missing cache entries always re-verify
5257

5358
try {
5459
// Use async verification if enabled (either via worker or custom function)
55-
if (this.ndk?.asyncSigVerification) {
60+
if (this.ndk?.asyncSigVerification && !forceSync) {
5661
// Capture the relay in a closure before the async call
5762
const relayForVerification = this.relay;
5863

core/src/relay/signature-verification.test.ts

Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
import { beforeEach, describe, expect, test, vi } from "vitest";
22
import { NDKEvent } from "../events/index";
3+
import { verifiedSignatures } from "../events/validation";
34
import { NDK } from "../ndk/index";
5+
import { NDKPrivateKeySigner } from "../signers/private-key/index";
46
import { NDKRelay } from "./index";
57

68
describe("Signature Verification Sampling", () => {
@@ -190,3 +192,120 @@ describe("Invalid Signature Handling", () => {
190192
expect(ratio).toBeCloseTo(0.1, 1);
191193
});
192194
});
195+
196+
describe("Signature cache poisoning resistance", () => {
197+
beforeEach(() => {
198+
verifiedSignatures.clear();
199+
});
200+
201+
test("negative cache entry triggers re-verification instead of short-circuiting", () => {
202+
const ndk = new NDK();
203+
const event = new NDKEvent(ndk, {
204+
kind: 0,
205+
created_at: 1234567890,
206+
pubkey: "a".repeat(64),
207+
id: "poisoned-id",
208+
sig: "some-sig",
209+
tags: [],
210+
content: "{}",
211+
});
212+
213+
// Poison the cache with a negative entry
214+
verifiedSignatures.set("poisoned-id", false);
215+
216+
// Spy on serialize to prove verification code path was entered
217+
// (serialize is called inside the sync verification branch)
218+
const serializeSpy = vi.spyOn(event, "serialize");
219+
220+
event.verifySignature(false);
221+
222+
// If the negative cache short-circuited, serialize would never be called.
223+
// The fact that it IS called proves re-verification happened.
224+
expect(serializeSpy).toHaveBeenCalled();
225+
serializeSpy.mockRestore();
226+
});
227+
228+
test("positive cache hit with matching sig short-circuits verification", () => {
229+
const ndk = new NDK();
230+
const event = new NDKEvent(ndk, {
231+
kind: 0,
232+
created_at: 1234567890,
233+
pubkey: "a".repeat(64),
234+
id: "valid-cached-id",
235+
sig: "cached-sig",
236+
tags: [],
237+
content: "{}",
238+
});
239+
240+
// Cache a positive entry with matching sig
241+
verifiedSignatures.set("valid-cached-id", "cached-sig");
242+
243+
const serializeSpy = vi.spyOn(event, "serialize");
244+
const result = event.verifySignature(false);
245+
246+
// Should return true from cache without re-verifying
247+
expect(result).toBe(true);
248+
expect(serializeSpy).not.toHaveBeenCalled();
249+
serializeSpy.mockRestore();
250+
});
251+
252+
test("positive cache hit with mismatched sig triggers re-verification", () => {
253+
const ndk = new NDK();
254+
const event = new NDKEvent(ndk, {
255+
kind: 0,
256+
created_at: 1234567890,
257+
pubkey: "a".repeat(64),
258+
id: "cached-id",
259+
sig: "different-sig",
260+
tags: [],
261+
content: "{}",
262+
});
263+
264+
// Cache was set by a different event with a different sig
265+
verifiedSignatures.set("cached-id", "original-sig");
266+
267+
const serializeSpy = vi.spyOn(event, "serialize");
268+
event.verifySignature(false);
269+
270+
// Mismatched sig should NOT trust the cache — must re-verify
271+
expect(serializeSpy).toHaveBeenCalled();
272+
serializeSpy.mockRestore();
273+
});
274+
275+
test("real signed event passes after negative cache poisoning", async () => {
276+
const ndk = new NDK();
277+
const signer = NDKPrivateKeySigner.generate();
278+
279+
// Create and sign a real kind:0 profile event
280+
const event = new NDKEvent(ndk);
281+
event.kind = 0;
282+
event.content = '{"name":"alice"}';
283+
await event.sign(signer);
284+
285+
// Poison the cache: a forged event with the same id was seen first
286+
verifiedSignatures.set(event.id, false);
287+
288+
// Despite the poisoned cache, the valid event must still verify
289+
const result = event.verifySignature(true, true);
290+
expect(result).toBe(true);
291+
});
292+
293+
test("real signed event rejected when cache holds a different valid sig", async () => {
294+
const ndk = new NDK();
295+
const signer = NDKPrivateKeySigner.generate();
296+
297+
// Create and sign a real event
298+
const event = new NDKEvent(ndk);
299+
event.kind = 0;
300+
event.content = '{"name":"bob"}';
301+
await event.sign(signer);
302+
303+
// Cache holds a "valid" sig that doesn't match this event's actual sig
304+
verifiedSignatures.set(event.id, "aaa" + event.sig!.slice(3));
305+
306+
// Should not trust the cached sig — must re-verify cryptographically
307+
// The real sig is valid, so it should pass
308+
const result = event.verifySignature(true, true);
309+
expect(result).toBe(true);
310+
});
311+
});

core/src/subscription/index.test.ts

Lines changed: 140 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
1-
import { describe, expect, it, vi } from "vitest"; // Added describe, it, expect
1+
import { beforeEach, describe, expect, it, vi } from "vitest";
22
import { NDKEvent } from "../events";
3+
import { NDKKind } from "../events/kinds/index";
4+
import { verifiedSignatures } from "../events/validation";
35
import { NDK } from "../ndk";
46
import { NDKSubscription } from ".";
57

@@ -111,3 +113,140 @@ describe("NDKSubscriptionFilters", () => {
111113
});
112114
});
113115
});
116+
117+
describe("Kind:0 profile signature enforcement", () => {
118+
const validPubkey = "a".repeat(64);
119+
120+
function makeProfileEvent(testNdk: NDK): NDKEvent {
121+
return new NDKEvent(testNdk, {
122+
kind: NDKKind.Metadata,
123+
created_at: Math.floor(Date.now() / 1000),
124+
pubkey: validPubkey,
125+
id: Math.random().toString(36).substring(2),
126+
sig: "b".repeat(128),
127+
tags: [],
128+
content: '{"name":"test"}',
129+
});
130+
}
131+
132+
function mockRelay() {
133+
return {
134+
shouldValidateEvent: () => true,
135+
addValidatedEvent: vi.fn(),
136+
addNonValidatedEvent: vi.fn(),
137+
url: "wss://mock.relay",
138+
};
139+
}
140+
141+
beforeEach(() => {
142+
verifiedSignatures.clear();
143+
});
144+
145+
it("drops kind:0 events with invalid signatures via relay", () => {
146+
const testNdk = new NDK();
147+
const sub = new NDKSubscription(testNdk, { kinds: [0] }, { skipValidation: true });
148+
const event = makeProfileEvent(testNdk);
149+
const relay = mockRelay();
150+
151+
const mockedVerify = vi.spyOn(event, "verifySignature").mockReturnValue(false);
152+
const mockedEmit = vi.spyOn(sub, "emit" as any);
153+
154+
sub.eventReceived(event, relay as any);
155+
156+
expect(mockedVerify).toHaveBeenCalled();
157+
expect(mockedEmit).not.toHaveBeenCalled();
158+
mockedVerify.mockRestore();
159+
mockedEmit.mockRestore();
160+
});
161+
162+
it("emits kind:0 events with valid signatures via relay", () => {
163+
const testNdk = new NDK();
164+
const sub = new NDKSubscription(testNdk, { kinds: [0] }, { skipValidation: true });
165+
const event = makeProfileEvent(testNdk);
166+
const relay = mockRelay();
167+
168+
const mockedVerify = vi.spyOn(event, "verifySignature").mockReturnValue(true);
169+
const mockedEmit = vi.spyOn(sub, "emit" as any);
170+
171+
sub.eventReceived(event, relay as any);
172+
173+
expect(mockedVerify).toHaveBeenCalled();
174+
expect(mockedEmit).toHaveBeenCalled();
175+
mockedVerify.mockRestore();
176+
mockedEmit.mockRestore();
177+
});
178+
179+
it("uses forceSync=true for kind:0 even when asyncSigVerification is enabled", () => {
180+
const testNdk = new NDK();
181+
testNdk.asyncSigVerification = true;
182+
const sub = new NDKSubscription(testNdk, { kinds: [0] }, { skipValidation: true });
183+
const event = makeProfileEvent(testNdk);
184+
const relay = mockRelay();
185+
186+
const mockedVerify = vi.spyOn(event, "verifySignature").mockReturnValue(true);
187+
188+
sub.eventReceived(event, relay as any);
189+
190+
// Should be called with (true, true) — persist=true, forceSync=true
191+
expect(mockedVerify).toHaveBeenCalledWith(true, true);
192+
mockedVerify.mockRestore();
193+
});
194+
195+
it("drops kind:0 events with invalid signatures without relay", () => {
196+
const testNdk = new NDK();
197+
const sub = new NDKSubscription(testNdk, { kinds: [0] }, { skipValidation: true });
198+
const event = makeProfileEvent(testNdk);
199+
200+
const mockedVerify = vi.spyOn(event, "verifySignature").mockReturnValue(false);
201+
const mockedEmit = vi.spyOn(sub, "emit" as any);
202+
203+
// No relay passed
204+
sub.eventReceived(event, undefined);
205+
206+
expect(mockedVerify).toHaveBeenCalledWith(true, true);
207+
expect(mockedEmit).not.toHaveBeenCalled();
208+
mockedVerify.mockRestore();
209+
mockedEmit.mockRestore();
210+
});
211+
212+
it("emits kind:0 events with valid signatures without relay", () => {
213+
const testNdk = new NDK();
214+
const sub = new NDKSubscription(testNdk, { kinds: [0] }, { skipValidation: true });
215+
const event = makeProfileEvent(testNdk);
216+
217+
const mockedVerify = vi.spyOn(event, "verifySignature").mockReturnValue(true);
218+
const mockedEmit = vi.spyOn(sub, "emit" as any);
219+
220+
sub.eventReceived(event, undefined);
221+
222+
expect(mockedVerify).toHaveBeenCalled();
223+
expect(mockedEmit).toHaveBeenCalled();
224+
mockedVerify.mockRestore();
225+
mockedEmit.mockRestore();
226+
});
227+
228+
it("does not force-verify non-kind:0 events without relay", () => {
229+
const testNdk = new NDK();
230+
const sub = new NDKSubscription(testNdk, { kinds: [1] }, { skipValidation: true });
231+
const event = new NDKEvent(testNdk, {
232+
kind: NDKKind.Text,
233+
created_at: Math.floor(Date.now() / 1000),
234+
pubkey: validPubkey,
235+
id: Math.random().toString(36).substring(2),
236+
sig: "b".repeat(128),
237+
tags: [],
238+
content: "hello",
239+
});
240+
241+
const mockedVerify = vi.spyOn(event, "verifySignature");
242+
const mockedEmit = vi.spyOn(sub, "emit" as any);
243+
244+
// kind:1 without relay — should NOT trigger verification
245+
sub.eventReceived(event, undefined);
246+
247+
expect(mockedVerify).not.toHaveBeenCalled();
248+
expect(mockedEmit).toHaveBeenCalled();
249+
mockedVerify.mockRestore();
250+
mockedEmit.mockRestore();
251+
});
252+
});

core/src/subscription/index.ts

Lines changed: 18 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { EventEmitter } from "tseep";
22

33
import type { NDKEventId, NDKSignedEvent, NostrEvent } from "../events/index.js";
44
import { createSignedEvent, NDKEvent } from "../events/index.js";
5-
import type { NDKKind } from "../events/kinds/index.js";
5+
import { NDKKind } from "../events/kinds/index.js";
66
import { verifiedSignatures } from "../events/validation.js";
77
import { wrapEvent } from "../events/wrap.js";
88
import type { NDK } from "../ndk/index.js";
@@ -869,36 +869,38 @@ export class NDKSubscription extends EventEmitter<{
869869

870870
// verify it
871871
if (relay) {
872-
// Check if we need to verify this event based on sampling
873-
const shouldVerify = relay.shouldValidateEvent();
872+
const mustVerify = ndkEvent.kind === NDKKind.Metadata;
873+
const shouldVerify = mustVerify || relay.shouldValidateEvent();
874874

875875
if (shouldVerify && !this.skipVerification) {
876-
// Set the relay on the event for async verification
877876
ndkEvent.relay = relay;
878877

879-
// Attempt verification
880-
if (this.ndk.asyncSigVerification) {
881-
// Async verification - call verifySignature but don't wait for result
882-
// The validation stats will be tracked in the async callback
883-
ndkEvent.verifySignature(true);
884-
} else {
885-
// Sync verification - check result immediately
886-
if (!ndkEvent.verifySignature(true)) {
878+
if (mustVerify || !this.ndk.asyncSigVerification) {
879+
// Sync verification: always for kind:0, otherwise when async is disabled
880+
if (!ndkEvent.verifySignature(true, mustVerify)) {
887881
this.debug("Event failed signature validation", event);
888-
// Report the invalid signature with relay information through the centralized method
889882
this.ndk.reportInvalidSignature(ndkEvent, relay);
890883
return;
891884
}
892-
893-
// Track successful validation
894885
relay.addValidatedEvent();
886+
} else {
887+
// Async verification for non-profile events when async is enabled
888+
ndkEvent.verifySignature(true);
895889
}
896890
} else {
897-
// We skipped verification for this event
898891
relay.addNonValidatedEvent();
899892
}
900893
}
901894

895+
// Verify kind:0 events even without a relay reference
896+
if (!relay && ndkEvent.kind === NDKKind.Metadata && !this.skipVerification) {
897+
if (!ndkEvent.verifySignature(true, true)) {
898+
this.debug("Event failed signature validation (no relay)", event);
899+
this.ndk.reportInvalidSignature(ndkEvent);
900+
return;
901+
}
902+
}
903+
902904
if (this.ndk.cacheAdapter && !this.opts.dontSaveToCache && !kindIsEphemeral(ndkEvent.kind as NDKKind) && !fromCache) {
903905
this.ndk.cacheAdapter.setEvent(ndkEvent, this.filters, relay);
904906
}

0 commit comments

Comments
 (0)