Skip to content

Add inbound profile signature verification#360

Open
alltheseas wants to merge 1 commit intonostr-dev-kit:masterfrom
alltheseas:profile-sig-verify
Open

Add inbound profile signature verification#360
alltheseas wants to merge 1 commit intonostr-dev-kit:masterfrom
alltheseas:profile-sig-verify

Conversation

@alltheseas
Copy link
Contributor

problem

ndk does not validate inbound profile/kind0 events

suggested solution

  • Always treat kind‑0 events as must‑verify: bypass sampling/trust and enforce synchronous signature checks so invalid profiles are dropped immediately.
    • Reuse verifyAndReport for both relay/no‑relay paths; add early returns to keep control flow simple.
    • Preserve existing behavior for other kinds (sampling + optional async verification).

context

discovered lack of profile validation via yakihonne-web: nostrability/nostrability#262 (comment)
closes #359

Copy link
Collaborator

@pablof7z pablof7z left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review: Critical Issues Found

This PR has the right intent (enforce signature verification on kind:0/Metadata events), but has two critical bugs that prevent merging:

1. verifyAndReport method doesn't exist

The PR calls this.verifyAndReport(ndkEvent, event, relay) and this.verifyAndReport(ndkEvent, event), but this method does not exist on NDKSubscription. This would cause a runtime crash for any profile event received.

2. All relay paths silently drop events

Every code path inside the if (relay) block now ends with return, which means events are never cached or emitted:

  • Non-verified events: relay.addNonValidatedEvent(); return; — drops valid events that didn't need verification
  • Sync-verified events: relay.addValidatedEvent(); return; — drops events that PASS verification
  • Async-verified events: ndkEvent.verifySignature(true); return; — drops events during async verification

In the original code, after the relay verification block, execution continues to caching (setEvent) and emission (emitEvent). The new code skips both for ALL events from relays.

Suggested Fix

  • Define verifyAndReport() as a private method on NDKSubscription (or inline the logic)
  • Only return (drop) events that FAIL verification
  • Let events that pass verification (or skip verification) continue to the existing caching and emission logic below

The security concept is sound — we should always verify kind:0 events. Happy to help iterate on a fix!

@alltheseas alltheseas force-pushed the profile-sig-verify branch 5 times, most recently from 47fc8e9 to 2977577 Compare March 11, 2026 07:59
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>
@alltheseas
Copy link
Contributor Author

addressed comments @pablof7z and added tests. lmk if you need anything else ser

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

add signature validation for inbound profile/kind0 events

2 participants