security(lnbits): Schnorr-verify inbound reply events before decrypting
LnbitsClient.handleReply matched replies by request_id alone — no
verification that the inbound event was actually signed by the
configured LNbits server pubkey. The relay's subscription `authors`
filter is relay-honour, not relay-enforced; a malicious or buggy
relay could forward an event with the server's pubkey in the body
but signed by a different key (or with a tampered body whose id no
longer matches).
Adds `isAuthenticServerEvent(ev, expectedPubkey)`:
- `ev.pubkey === expectedPubkey` (explicit, not relying on filter)
- `verifyEvent(ev)` (catches sig/id/content tampering)
handleReply calls it before passing the event to decryptContentV2.
NIP-44 v2 already binds ciphertext to sender via ECDH, so a relay
without the server's nsec can't forge decryptable content — but
verifying the outer event keeps `ev.id` trustworthy for any
downstream dedup/logging code and matches the symmetric defence on
the server side (`nostr_transport/relay_pool.py:~320`) and on the
lnbits-bunker-client side (`aiolabs/lnbits` commit 4ebcd959,
`NsecBunkerAdminClient._match_response`).
Test `packages/lnbits/src/__tests__/client.test.ts` covers:
- legitimate server-signed event accepted
- event whose pubkey field doesn't match config rejected
- forged event (signed by attacker, pubkey overwritten to server)
rejected — recomputed id no longer matches stored id
- event with tampered content (id mismatch) rejected
Gotcha worth noting: `finalizeEvent` stamps `event[verifiedSymbol] = true`
to cache the verification result, and `{...ev}` spread copies symbol-
keyed properties. So forged/tampered events constructed via spread
inherit the cached `true` and `verifyEvent` short-circuits. The test
JSON-round-trips through `stripVerifiedCache` to drop the cache.
Closes aiolabs/lamassu-next#49.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
a980dcd3e9
commit
e2351f8f7b
2 changed files with 164 additions and 1 deletions
129
packages/lnbits/src/__tests__/client.test.ts
Normal file
129
packages/lnbits/src/__tests__/client.test.ts
Normal file
|
|
@ -0,0 +1,129 @@
|
|||
/**
|
||||
* Tests for LnbitsClient's defensive authentication of inbound reply events.
|
||||
*
|
||||
* Covers the patch that mirrors lnbits commit `4ebcd959`
|
||||
* (NsecBunkerAdminClient._match_response Schnorr-verifies before decrypting).
|
||||
* See aiolabs/lamassu-next#49 + the cross-codebase coordination memory.
|
||||
*/
|
||||
|
||||
import { describe, it, expect } from 'vitest'
|
||||
import {
|
||||
generateSecretKey,
|
||||
getPublicKey,
|
||||
finalizeEvent,
|
||||
getEventHash,
|
||||
type Event as NostrEvent,
|
||||
type UnsignedEvent,
|
||||
} from 'nostr-tools'
|
||||
import { isAuthenticServerEvent } from '../client.js'
|
||||
|
||||
const KIND_LNBITS_RPC = 21000
|
||||
|
||||
/**
|
||||
* Forge an event by signing with one key, then overwriting the `pubkey`
|
||||
* field to claim a different author. The id field stays bound to the
|
||||
* original (attacker) pubkey, so `verifyEvent` catches the mismatch:
|
||||
* recomputed id (over the claimed pubkey) != stored id. This models a
|
||||
* malicious relay that constructs a kind-21000 event claiming to be
|
||||
* from the server. Mirror of the lnbits-side test
|
||||
* `test_match_response_rejects_forged_signature`.
|
||||
*
|
||||
* Important: `finalizeEvent` stamps `event[verifiedSymbol] = true` on
|
||||
* its return value, and `verifyEvent` short-circuits on that cached
|
||||
* flag. Object spread copies symbol-keyed properties, so we round-trip
|
||||
* through JSON to drop the cache and force re-verification.
|
||||
*/
|
||||
function forgeEvent(
|
||||
template: Omit<UnsignedEvent, 'pubkey'>,
|
||||
claimedPubkey: string,
|
||||
signingKey: Uint8Array,
|
||||
): NostrEvent {
|
||||
const signed = finalizeEvent(template, signingKey)
|
||||
const stripped = JSON.parse(JSON.stringify(signed)) as NostrEvent
|
||||
return { ...stripped, pubkey: claimedPubkey }
|
||||
}
|
||||
|
||||
/** Drop the cached `verifiedSymbol` flag that `finalizeEvent` stamps. */
|
||||
function stripVerifiedCache(ev: NostrEvent): NostrEvent {
|
||||
return JSON.parse(JSON.stringify(ev)) as NostrEvent
|
||||
}
|
||||
|
||||
describe('isAuthenticServerEvent', () => {
|
||||
const serverKey = generateSecretKey()
|
||||
const serverPubkey = getPublicKey(serverKey)
|
||||
const recipientKey = generateSecretKey()
|
||||
const recipientPubkey = getPublicKey(recipientKey)
|
||||
|
||||
it('accepts a genuine event signed by the configured server', () => {
|
||||
const ev = finalizeEvent(
|
||||
{
|
||||
kind: KIND_LNBITS_RPC,
|
||||
content: '<ciphertext>',
|
||||
tags: [['p', recipientPubkey]],
|
||||
created_at: Math.floor(Date.now() / 1000),
|
||||
},
|
||||
serverKey,
|
||||
)
|
||||
expect(isAuthenticServerEvent(ev, serverPubkey)).toBe(true)
|
||||
})
|
||||
|
||||
it('rejects an event whose pubkey field does not match the configured server', () => {
|
||||
const otherKey = generateSecretKey()
|
||||
const ev = finalizeEvent(
|
||||
{
|
||||
kind: KIND_LNBITS_RPC,
|
||||
content: '<ciphertext>',
|
||||
tags: [['p', recipientPubkey]],
|
||||
created_at: Math.floor(Date.now() / 1000),
|
||||
},
|
||||
otherKey,
|
||||
)
|
||||
// Sanity: this event is internally consistent and would verify
|
||||
// under its own pubkey. The point is it's NOT from our server.
|
||||
expect(ev.pubkey).not.toBe(serverPubkey)
|
||||
expect(isAuthenticServerEvent(ev, serverPubkey)).toBe(false)
|
||||
})
|
||||
|
||||
it('rejects a forged event with serverPubkey claim but attacker-signed', () => {
|
||||
// A malicious relay signs an event with its own key, then overwrites
|
||||
// the `pubkey` field to claim it came from the server. The id is
|
||||
// still bound to the attacker's pubkey under the original signature,
|
||||
// so the recomputed id (over the spoofed pubkey) won't match —
|
||||
// verifyEvent must reject.
|
||||
const attackerKey = generateSecretKey()
|
||||
const forged = forgeEvent(
|
||||
{
|
||||
kind: KIND_LNBITS_RPC,
|
||||
content: '<bogus ciphertext>',
|
||||
tags: [['p', recipientPubkey]],
|
||||
created_at: Math.floor(Date.now() / 1000),
|
||||
},
|
||||
serverPubkey,
|
||||
attackerKey,
|
||||
)
|
||||
// Sanity: the body claims serverPubkey…
|
||||
expect(forged.pubkey).toBe(serverPubkey)
|
||||
// …but the stored id was computed over the attacker's pubkey, so
|
||||
// recomputing it over the claimed pubkey gives a different value.
|
||||
expect(forged.id).not.toBe(getEventHash(forged as UnsignedEvent))
|
||||
expect(isAuthenticServerEvent(forged, serverPubkey)).toBe(false)
|
||||
})
|
||||
|
||||
it('rejects an event with tampered content (id mismatch)', () => {
|
||||
// Real signed event, then content is rewritten — id won't match.
|
||||
const ev = finalizeEvent(
|
||||
{
|
||||
kind: KIND_LNBITS_RPC,
|
||||
content: '<original ciphertext>',
|
||||
tags: [['p', recipientPubkey]],
|
||||
created_at: Math.floor(Date.now() / 1000),
|
||||
},
|
||||
serverKey,
|
||||
)
|
||||
const tampered: NostrEvent = {
|
||||
...stripVerifiedCache(ev),
|
||||
content: '<tampered ciphertext>',
|
||||
}
|
||||
expect(isAuthenticServerEvent(tampered, serverPubkey)).toBe(false)
|
||||
})
|
||||
})
|
||||
|
|
@ -27,7 +27,7 @@ import {
|
|||
encryptContentV2,
|
||||
decryptContentV2,
|
||||
} from '@bitSpire/nostr-client'
|
||||
import { finalizeEvent } from 'nostr-tools'
|
||||
import { finalizeEvent, verifyEvent } from 'nostr-tools'
|
||||
|
||||
import type {
|
||||
LnbitsConfig,
|
||||
|
|
@ -48,6 +48,39 @@ import type {
|
|||
|
||||
const LNBITS_KIND_RPC = 21000
|
||||
|
||||
/**
|
||||
* Authenticate an inbound kind-21000 event as genuinely from the configured
|
||||
* LNbits server before trusting any of its fields. Two checks:
|
||||
*
|
||||
* 1. `ev.pubkey === expectedPubkey` — defends against a relay that
|
||||
* ignores our subscription `authors` filter and forwards events from
|
||||
* other authors (the filter is relay-honour, not relay-enforced).
|
||||
*
|
||||
* 2. `verifyEvent(ev)` — Schnorr-verify the outer signature against the
|
||||
* claimed pubkey. Defends against a malicious relay that constructs an
|
||||
* event with the server's pubkey in the `pubkey` field but signs with
|
||||
* its own key (or any non-server key). Without this check, a
|
||||
* malformed/tampered event would fall through to `decryptContentV2`,
|
||||
* which would fail at the MAC layer because NIP-44 v2 binds the
|
||||
* ciphertext to the sender's key — but defense in depth is worth the
|
||||
* one extra Schnorr verification, and it keeps `ev.id` trustworthy
|
||||
* for any logging/dedup code further downstream.
|
||||
*
|
||||
* Mirror of the same defence in `aiolabs/lnbits` commit `4ebcd959`
|
||||
* (`NsecBunkerAdminClient._match_response`). Symmetric trust boundary
|
||||
* across server inbound, lnbits-bunker-client inbound, and ATM-client
|
||||
* inbound — see `aiolabs/lamassu-next#49` and the cross-codebase
|
||||
* coordination memory.
|
||||
*/
|
||||
export function isAuthenticServerEvent(
|
||||
ev: NostrEvent,
|
||||
expectedPubkey: string,
|
||||
): boolean {
|
||||
if (ev.pubkey !== expectedPubkey) return false
|
||||
if (!verifyEvent(ev)) return false
|
||||
return true
|
||||
}
|
||||
|
||||
/** Active streaming subscription state held client-side. */
|
||||
interface ActiveSubscription {
|
||||
subscriptionId: string
|
||||
|
|
@ -446,6 +479,7 @@ export class LnbitsClient {
|
|||
|
||||
private handleReply(ev: NostrEvent): void {
|
||||
if (!this.identity) return
|
||||
if (!isAuthenticServerEvent(ev, this.config.serverPubkey)) return
|
||||
let plaintext: string
|
||||
try {
|
||||
plaintext = decryptContentV2(this.identity, this.config.serverPubkey, ev.content)
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue