mirror of
https://github.com/vitorpamplona/amethyst.git
synced 2026-08-12 09:13:23 +00:00
fix(nwc): verify response author against expected wallet-service pubkey
The previous commit dropped `authors` and `#p` from the relay subscription
filter to match Primal's interop shape. Without those, the relay will
deliver any signed kind-23195 event that carries our request id in `#e`,
so an attacker who can observe the request on the relay could forge a
"response" with their own keypair: Amethyst would happily derive a shared
secret from `event.pubKey` (the attacker), decrypt the payload, and
display attacker-controlled balance/transaction data. Even worse,
`paymentTracker.onResponseReceived` removed the pending entry on first
match — so the legitimate wallet reply that followed was silently dropped.
Move the author check from the relay layer into NwcPaymentTracker:
- `registerRequest` now requires the expected wallet-service pubkey
(read from the request's `p` tag). LocalCache extracts it during
`consume(LnZapPaymentRequestEvent)` and refuses to register if the
request has no `p` tag.
- `onResponseReceived` takes the response author and returns a sealed
MatchResult of NoMatch / WrongAuthor / Matched. A WrongAuthor result
leaves the pending entry in the map so the legitimate response can
still resolve it.
- Android LocalCache and DesktopLocalCache both adopt the new API and
log a warning on suspected spoof attempts.
End-to-end the response is still encrypted under the per-connection shared
secret, so this is a second layer of defence rather than the only one,
but matching the author keeps a forged kind-23195 from consuming the
pending slot and DoSing the legitimate reply.
This commit is contained in:
@@ -2075,6 +2075,12 @@ object LocalCache : ILocalCache, ICacheProvider {
|
||||
if (note.event != null) return false
|
||||
|
||||
if (wasVerified || justVerify(event)) {
|
||||
val expectedServicePubkey =
|
||||
event.walletServicePubKey() ?: run {
|
||||
Log.w("LocalCache", "NWC request ${event.id} has no `p` tag; cannot register for response.")
|
||||
return false
|
||||
}
|
||||
|
||||
note.loadEvent(event, author, emptyList())
|
||||
|
||||
relay?.let {
|
||||
@@ -2083,7 +2089,7 @@ object LocalCache : ILocalCache, ICacheProvider {
|
||||
|
||||
zappedNote?.addZapPayment(note, null)
|
||||
|
||||
paymentTracker.registerRequest(event.id, zappedNote, onResponse)
|
||||
paymentTracker.registerRequest(event.id, expectedServicePubkey, zappedNote, onResponse)
|
||||
|
||||
refreshNewNoteObservers(note)
|
||||
|
||||
@@ -2100,13 +2106,28 @@ object LocalCache : ILocalCache, ICacheProvider {
|
||||
): Boolean {
|
||||
val requestId = event.requestId()
|
||||
val pending =
|
||||
paymentTracker.onResponseReceived(requestId) ?: run {
|
||||
Log.w(
|
||||
"LocalCache",
|
||||
"NWC response ${event.id} from ${event.pubKey} references request e=$requestId but no pending request is registered. " +
|
||||
"The response was either delivered after timeout, the user holds a stale subscription, or the wallet service set the wrong e tag.",
|
||||
)
|
||||
return false
|
||||
when (val match = paymentTracker.onResponseReceived(requestId, event.pubKey)) {
|
||||
is NwcPaymentTracker.MatchResult.Matched -> match.pending
|
||||
is NwcPaymentTracker.MatchResult.WrongAuthor -> {
|
||||
// Possible spoof: a kind-23195 event from someone other than
|
||||
// the wallet service we sent the request to. The pending
|
||||
// entry is left in place so the real response can still
|
||||
// resolve it; we silently drop this one.
|
||||
Log.w(
|
||||
"LocalCache",
|
||||
"Rejecting NWC response ${event.id}: expected author ${match.expected} but event was signed by ${match.actual}. " +
|
||||
"This may be a spoofed reply — keeping the request pending for the legitimate wallet response.",
|
||||
)
|
||||
return false
|
||||
}
|
||||
NwcPaymentTracker.MatchResult.NoMatch -> {
|
||||
Log.w(
|
||||
"LocalCache",
|
||||
"NWC response ${event.id} from ${event.pubKey} references request e=$requestId but no pending request is registered. " +
|
||||
"The response was either delivered after timeout, the user holds a stale subscription, or the wallet service set the wrong e tag.",
|
||||
)
|
||||
return false
|
||||
}
|
||||
}
|
||||
|
||||
val zappedNote = pending.zappedNote
|
||||
|
||||
+50
-10
@@ -33,18 +33,28 @@ import java.util.concurrent.ConcurrentHashMap
|
||||
* for the core request/response matching logic.
|
||||
*
|
||||
* Flow:
|
||||
* 1. When sending payment request: [registerRequest] stores callback
|
||||
* 2. When response arrives: [onResponseReceived] retrieves and removes pending request
|
||||
* 1. When sending payment request: [registerRequest] stores callback and the
|
||||
* expected wallet-service pubkey
|
||||
* 2. When response arrives: [onResponseReceived] retrieves the pending request
|
||||
* only if the response author matches the expected wallet-service pubkey
|
||||
* 3. Caller invokes callback and links notes via Note.addZapPayment()
|
||||
*
|
||||
* Author verification is required because the relay subscription filters by
|
||||
* request id only (matching Primal's filter shape). An attacker who observes
|
||||
* the request on the relay could otherwise forge a response with their own
|
||||
* keypair and trick the client into displaying attacker-controlled data.
|
||||
*/
|
||||
class NwcPaymentTracker {
|
||||
/**
|
||||
* Data for a pending payment request.
|
||||
*
|
||||
* @property expectedServicePubkey Pubkey of the wallet service we sent
|
||||
* the request to. Only a response signed by this key is accepted.
|
||||
* @property zappedNote The note being zapped, if payment is for a zap
|
||||
* @property onResponse Callback to invoke when wallet responds
|
||||
*/
|
||||
data class PendingRequest(
|
||||
val expectedServicePubkey: HexKey,
|
||||
val zappedNote: Note?,
|
||||
val onResponse: suspend (LnZapPaymentResponseEvent) -> Unit,
|
||||
)
|
||||
@@ -55,27 +65,57 @@ class NwcPaymentTracker {
|
||||
* Registers a pending payment request.
|
||||
*
|
||||
* @param requestId Event ID of the LnZapPaymentRequestEvent
|
||||
* @param expectedServicePubkey Wallet service pubkey from the request's `p` tag
|
||||
* @param zappedNote The note being zapped (null if not a zap payment)
|
||||
* @param onResponse Callback invoked when response arrives
|
||||
*/
|
||||
fun registerRequest(
|
||||
requestId: HexKey,
|
||||
expectedServicePubkey: HexKey,
|
||||
zappedNote: Note?,
|
||||
onResponse: suspend (LnZapPaymentResponseEvent) -> Unit,
|
||||
) {
|
||||
awaitingRequests[requestId] = PendingRequest(zappedNote, onResponse)
|
||||
awaitingRequests[requestId] = PendingRequest(expectedServicePubkey, zappedNote, onResponse)
|
||||
}
|
||||
|
||||
/** Outcome of matching a response event against the pending-requests table. */
|
||||
sealed interface MatchResult {
|
||||
/** No pending request for this request id (or the id was null). */
|
||||
data object NoMatch : MatchResult
|
||||
|
||||
/** A pending request exists but the response author does not match. */
|
||||
data class WrongAuthor(
|
||||
val expected: HexKey,
|
||||
val actual: HexKey,
|
||||
) : MatchResult
|
||||
|
||||
/** Response is authentic; the pending request has been removed. */
|
||||
data class Matched(
|
||||
val pending: PendingRequest,
|
||||
) : MatchResult
|
||||
}
|
||||
|
||||
/**
|
||||
* Called when a payment response event is received.
|
||||
* Retrieves and removes the pending request for the given request ID.
|
||||
* Looks up the pending request for the given response. Only consumes (and
|
||||
* returns [MatchResult.Matched]) when the response author matches the
|
||||
* stored [PendingRequest.expectedServicePubkey] — a forged response is
|
||||
* left in the map so the legitimate one can still resolve it.
|
||||
*
|
||||
* @param requestId The 'e' tag from the response, pointing to original request
|
||||
* @return PendingRequest if found, null otherwise
|
||||
* @param requestId The `e` tag from the response, pointing to original request
|
||||
* @param responseAuthor The `pubkey` field of the response event
|
||||
*/
|
||||
fun onResponseReceived(requestId: HexKey?): PendingRequest? {
|
||||
if (requestId == null) return null
|
||||
return awaitingRequests.remove(requestId)
|
||||
fun onResponseReceived(
|
||||
requestId: HexKey?,
|
||||
responseAuthor: HexKey,
|
||||
): MatchResult {
|
||||
if (requestId == null) return MatchResult.NoMatch
|
||||
val pending = awaitingRequests[requestId] ?: return MatchResult.NoMatch
|
||||
if (pending.expectedServicePubkey != responseAuthor) {
|
||||
return MatchResult.WrongAuthor(pending.expectedServicePubkey, responseAuthor)
|
||||
}
|
||||
// Author matches — atomically remove and return.
|
||||
val removed = awaitingRequests.remove(requestId) ?: return MatchResult.NoMatch
|
||||
return MatchResult.Matched(removed)
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
Vendored
+18
-2
@@ -512,6 +512,11 @@ class DesktopLocalCache : ICacheProvider {
|
||||
wasVerified: Boolean = false,
|
||||
): Boolean {
|
||||
if (!wasVerified && !justVerify(event)) return false
|
||||
val expectedServicePubkey =
|
||||
event.walletServicePubKey() ?: run {
|
||||
Log.w("DesktopLocalCache") { "NWC request ${event.id} has no `p` tag; cannot register for response." }
|
||||
return false
|
||||
}
|
||||
val note = getOrCreateNote(event.id)
|
||||
val author = getOrCreateUser(event.pubKey)
|
||||
|
||||
@@ -523,7 +528,7 @@ class DesktopLocalCache : ICacheProvider {
|
||||
relay?.let { note.addRelay(it) }
|
||||
|
||||
zappedNote?.addZapPayment(note, null)
|
||||
paymentTracker.registerRequest(event.id, zappedNote, onResponse)
|
||||
paymentTracker.registerRequest(event.id, expectedServicePubkey, zappedNote, onResponse)
|
||||
|
||||
return true
|
||||
}
|
||||
@@ -543,7 +548,18 @@ class DesktopLocalCache : ICacheProvider {
|
||||
): Boolean {
|
||||
if (!wasVerified && !justVerify(event)) return false
|
||||
val requestId = event.requestId()
|
||||
val pending = paymentTracker.onResponseReceived(requestId) ?: return false
|
||||
val pending =
|
||||
when (val match = paymentTracker.onResponseReceived(requestId, event.pubKey)) {
|
||||
is NwcPaymentTracker.MatchResult.Matched -> match.pending
|
||||
is NwcPaymentTracker.MatchResult.WrongAuthor -> {
|
||||
Log.w("DesktopLocalCache") {
|
||||
"Rejecting NWC response ${event.id}: expected author ${match.expected} but event was signed by ${match.actual}. " +
|
||||
"This may be a spoofed reply — keeping the request pending for the legitimate wallet response."
|
||||
}
|
||||
return false
|
||||
}
|
||||
NwcPaymentTracker.MatchResult.NoMatch -> return false
|
||||
}
|
||||
|
||||
val requestNote = requestId?.let { getNoteIfExists(it) }
|
||||
val note = getOrCreateNote(event.id)
|
||||
|
||||
Reference in New Issue
Block a user