Skip to content

Commit 281a8fd

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 281a8fd

File tree

4 files changed

+321
-25
lines changed

4 files changed

+321
-25
lines changed

core/src/events/validation.ts

Lines changed: 13 additions & 8 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

47-
const prevVerification = verifiedSignatures.get(this.id);
48-
if (prevVerification !== null) {
49-
this.signatureVerified = !!prevVerification;
50-
return this.signatureVerified;
51-
}
52-
5347
try {
48+
const prevVerification = verifiedSignatures.get(this.id);
49+
if (prevVerification !== null && prevVerification !== false) {
50+
if (prevVerification === this.sig && this.getEventHash() === this.id) {
51+
// Positive cache hit — sig matches and payload hashes to the claimed id
52+
this.signatureVerified = true;
53+
return true;
54+
}
55+
// Sig mismatch or payload tampered — re-verify
56+
}
57+
// Negative or missing cache entries always re-verify
58+
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: 150 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,151 @@ 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 and valid hash short-circuits schnorr verify", async () => {
229+
const ndk = new NDK();
230+
const signer = NDKPrivateKeySigner.generate();
231+
232+
// Create a real signed event so id matches the payload hash
233+
const event = new NDKEvent(ndk);
234+
event.kind = 0;
235+
event.content = '{"name":"cached"}';
236+
await event.sign(signer);
237+
238+
// Populate cache with the real sig
239+
verifiedSignatures.set(event.id, event.sig!);
240+
241+
// Clear the per-instance flag so cache lookup is exercised
242+
event.signatureVerified = undefined as any;
243+
244+
const result = event.verifySignature(false);
245+
246+
// Should return true from cache — hash matches, sig matches
247+
expect(result).toBe(true);
248+
});
249+
250+
test("positive cache hit with mismatched sig triggers re-verification", () => {
251+
const ndk = new NDK();
252+
const event = new NDKEvent(ndk, {
253+
kind: 0,
254+
created_at: 1234567890,
255+
pubkey: "a".repeat(64),
256+
id: "cached-id",
257+
sig: "different-sig",
258+
tags: [],
259+
content: "{}",
260+
});
261+
262+
// Cache was set by a different event with a different sig
263+
verifiedSignatures.set("cached-id", "original-sig");
264+
265+
const serializeSpy = vi.spyOn(event, "serialize");
266+
event.verifySignature(false);
267+
268+
// Mismatched sig should NOT trust the cache — must re-verify
269+
expect(serializeSpy).toHaveBeenCalled();
270+
serializeSpy.mockRestore();
271+
});
272+
273+
test("real signed event passes after negative cache poisoning", async () => {
274+
const ndk = new NDK();
275+
const signer = NDKPrivateKeySigner.generate();
276+
277+
// Create and sign a real kind:0 profile event
278+
const event = new NDKEvent(ndk);
279+
event.kind = 0;
280+
event.content = '{"name":"alice"}';
281+
await event.sign(signer);
282+
283+
// Poison the cache: a forged event with the same id was seen first
284+
verifiedSignatures.set(event.id, false);
285+
286+
// Despite the poisoned cache, the valid event must still verify
287+
const result = event.verifySignature(true, true);
288+
expect(result).toBe(true);
289+
});
290+
291+
test("real signed event rejected when cache holds a different valid sig", async () => {
292+
const ndk = new NDK();
293+
const signer = NDKPrivateKeySigner.generate();
294+
295+
// Create and sign a real event
296+
const event = new NDKEvent(ndk);
297+
event.kind = 0;
298+
event.content = '{"name":"bob"}';
299+
await event.sign(signer);
300+
301+
// Cache holds a "valid" sig that doesn't match this event's actual sig
302+
verifiedSignatures.set(event.id, "aaa" + event.sig!.slice(3));
303+
304+
// Should not trust the cached sig — must re-verify cryptographically
305+
// The real sig is valid, so it should pass
306+
const result = event.verifySignature(true, true);
307+
expect(result).toBe(true);
308+
});
309+
310+
test("tampered payload with replayed id+sig is rejected despite cache hit", async () => {
311+
const ndk = new NDK();
312+
const signer = NDKPrivateKeySigner.generate();
313+
314+
// Create and sign a legitimate kind:0 event
315+
const event = new NDKEvent(ndk);
316+
event.kind = 0;
317+
event.content = '{"name":"real"}';
318+
await event.sign(signer);
319+
320+
const originalId = event.id;
321+
const originalSig = event.sig!;
322+
323+
// First verification succeeds and populates the cache
324+
expect(event.verifySignature(true, true)).toBe(true);
325+
expect(verifiedSignatures.get(originalId)).toBe(originalSig);
326+
327+
// Simulate a relay replaying the same (id, sig) but with tampered content
328+
const tampered = new NDKEvent(ndk, {
329+
kind: 0,
330+
created_at: event.created_at!,
331+
pubkey: event.pubkey,
332+
id: originalId,
333+
sig: originalSig,
334+
tags: [],
335+
content: '{"name":"evil"}', // altered payload
336+
});
337+
338+
// Cache has matching id+sig, but payload hash won't match the id
339+
const result = tampered.verifySignature(true, true);
340+
expect(result).toBe(false);
341+
});
342+
});

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

0 commit comments

Comments
 (0)