mirror of
https://github.com/jmcorgan/fips.git
synced 2026-10-05 19:18:25 +00:00
fix(nostr): stop signing traversal deletion requests with the routing key
After a traversal attempt, both sides published a NIP-09 deletion request signed by the node's routing key and naming the attempt's offer and answer wraps. Each request put the node's public identity next to the ids of its traversal signals on every relay it reached, which the one-time signing keys on the wraps are meant to avoid. What the requests deleted differed by side. A relay honouring NIP-59 deletes a gift wrap when the request is signed by the wrap's p-tagged recipient, so the initiator's request, sent after a successful punch, did delete the answer wrap addressed to it; strfry in the NAT lab logged that deletion. The offer it also named is addressed to the responder, and the responder's requests named only the answer it had sent to the initiator, so those deleted nothing. Drop all three traversal calls (the initiator after a punch, the responder on refusing an offer, and the responder after its punch attempt), and accept that an answer wrap now stays on a relay that stores it until its NIP-40 expiration. The wraps are ephemeral kinds, so relays that do not store ephemeral events never held them. The advertisement retraction keeps its deletion request, since it names an event the routing key signed itself. The signal envelope's event id had no other reader and is removed with its import. The NAT lab now asserts that its relay holds no kind 5 event after the cone, symmetric and lan scenarios, scanning the relay's own store and failing if the scan cannot run or be parsed. Before the fix each scenario failed it: cone and lan held two requests (the initiator's naming offer and answer, the responder's naming the answer) and symmetric held the responder's one. After it, all three held none. The responder's refusal path is not reached by any lab scenario and is covered by reading only.
This commit is contained in:
@@ -850,7 +850,6 @@ impl NostrRendezvous {
|
||||
{
|
||||
let _ = tx.send(SignalEnvelope {
|
||||
payload: answer,
|
||||
event_id: event.id,
|
||||
sender_npub: sender_npub.clone(),
|
||||
});
|
||||
}
|
||||
@@ -1301,10 +1300,6 @@ impl NostrRendezvous {
|
||||
"traversal: initiator punch succeeded"
|
||||
);
|
||||
|
||||
let _ = self
|
||||
.publish_delete(&relays, [offer_event.id, answer.event_id])
|
||||
.await;
|
||||
|
||||
self.failure_state
|
||||
.record_success(&peer_config.npub, now_ms());
|
||||
|
||||
@@ -1471,7 +1466,6 @@ impl NostrRendezvous {
|
||||
"traversal: answer sent"
|
||||
);
|
||||
if !accepted {
|
||||
let _ = self.publish_delete(&relays, [answer_event.id]).await;
|
||||
return Ok(());
|
||||
}
|
||||
|
||||
@@ -1521,8 +1515,6 @@ impl NostrRendezvous {
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
let _ = self.publish_delete(&relays, [answer_event.id]).await;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
|
||||
@@ -1,4 +1,3 @@
|
||||
use nostr::EventId;
|
||||
use nostr::nips::{nip44, nip59};
|
||||
use nostr::prelude::{
|
||||
Event, EventBuilder, JsonUtil, Kind, NostrSigner, PublicKey, Tag, Timestamp, UnsignedEvent,
|
||||
@@ -19,7 +18,6 @@ pub(crate) const FRESHNESS_SKEW_TOLERANCE_MS: u64 = 60_000;
|
||||
|
||||
pub(super) struct SignalEnvelope<T> {
|
||||
pub(super) payload: T,
|
||||
pub(super) event_id: EventId,
|
||||
pub(super) sender_npub: String,
|
||||
}
|
||||
|
||||
|
||||
@@ -448,6 +448,45 @@ ping_peer() {
|
||||
fi
|
||||
}
|
||||
|
||||
# Fail if the relay holds any kind 5 deletion request.
|
||||
#
|
||||
# A node signs a deletion request with its routing key, so one naming a
|
||||
# traversal signal's wrap would tie that key to a signal sent under a one-time
|
||||
# key. Nodes delete only adverts they withdraw, and the lab's nodes advertise
|
||||
# throughout, so after a scenario the relay should hold none. strfry scan reads
|
||||
# the relay's own store and prints one event per line. A scan that cannot run
|
||||
# or print parseable events has observed nothing, so it fails the check too.
|
||||
assert_no_deletion_requests() {
|
||||
local relay="$1" events="" report=""
|
||||
if ! events="$(docker exec "$relay" strfry scan '{"kinds":[5]}' 2>/dev/null)"; then
|
||||
echo "FAIL: could not scan $relay for deletion requests" >&2
|
||||
return 1
|
||||
fi
|
||||
if ! report="$(python3 -c '
|
||||
import json, sys
|
||||
events = []
|
||||
for line in sys.stdin:
|
||||
line = line.strip()
|
||||
if line:
|
||||
events.append(json.loads(line))
|
||||
for ev in events:
|
||||
ids = [t[1] for t in ev.get("tags", []) if len(t) > 1 and t[0] == "e"]
|
||||
print(" kind 5 by %s... naming %s" % (ev["pubkey"][:16], ", ".join(ids)))
|
||||
print(len(events))
|
||||
' <<<"$events")"; then
|
||||
echo "FAIL: could not parse $relay's scan for deletion requests" >&2
|
||||
return 1
|
||||
fi
|
||||
local count="${report##*$'\n'}"
|
||||
if [ "$count" != "0" ]; then
|
||||
echo "FAIL: $relay holds $count deletion request(s):" >&2
|
||||
sed '$d' <<<"$report" >&2
|
||||
return 1
|
||||
fi
|
||||
echo " $relay holds no deletion requests"
|
||||
return 0
|
||||
}
|
||||
|
||||
# A relay that faulted while a scenario's assertions still passed is a finding
|
||||
# about the relay, not about the scenario, so it is reported and not made a
|
||||
# failure: the scenario proved what it set out to prove.
|
||||
@@ -497,6 +536,10 @@ run_cone() {
|
||||
dump_cone_diagnostics
|
||||
return 1
|
||||
}
|
||||
assert_no_deletion_requests "$RELAY_CONTAINER" || {
|
||||
dump_cone_diagnostics
|
||||
return 1
|
||||
}
|
||||
note_relay_event
|
||||
cleanup
|
||||
}
|
||||
@@ -543,6 +586,10 @@ run_symmetric() {
|
||||
dump_symmetric_diagnostics
|
||||
return 1
|
||||
}
|
||||
assert_no_deletion_requests "$RELAY_CONTAINER" || {
|
||||
dump_symmetric_diagnostics
|
||||
return 1
|
||||
}
|
||||
note_relay_event
|
||||
cleanup
|
||||
}
|
||||
@@ -586,6 +633,10 @@ run_lan() {
|
||||
dump_lan_diagnostics
|
||||
return 1
|
||||
}
|
||||
assert_no_deletion_requests "$RELAY_CONTAINER" || {
|
||||
dump_lan_diagnostics
|
||||
return 1
|
||||
}
|
||||
note_relay_event
|
||||
# Skip the final teardown when the mesh-lab harness wraps this
|
||||
# script: it needs to docker-logs the containers before teardown,
|
||||
|
||||
@@ -115,9 +115,9 @@ dump_diagnostics() {
|
||||
#
|
||||
# Which rejection they take is not what the event's gibberish `content`
|
||||
# suggests. `parse_overlay_advert_event` looks for the `protocol` tag
|
||||
# first (src/nostr/runtime.rs:1665-1671) and this event carries only `d`
|
||||
# first (src/nostr/runtime.rs:1657-1663) and this event carries only `d`
|
||||
# and `app`, so it fails with `missing required protocol tag` and never
|
||||
# reaches the `serde_json::from_str` at :1679. The content is therefore
|
||||
# reaches the `serde_json::from_str` at :1671. The content is therefore
|
||||
# belt and braces rather than the thing under test.
|
||||
#
|
||||
# Neither branch logs anything: see the coverage-gap note in run_test.
|
||||
@@ -236,7 +236,7 @@ while int.from_bytes(secret, "big") == 0 or int.from_bytes(secret, "big") >= N:
|
||||
pubkey = xonly_pubkey(secret).hex()
|
||||
created_at = int(time.time())
|
||||
# Both of these must match the consumers' subscription filter, which is
|
||||
# kind + identifier and no author clause (src/nostr/runtime.rs:1042-1044).
|
||||
# kind + identifier and no author clause (src/nostr/runtime.rs:1041-1043).
|
||||
# The literals are ADVERT_KIND and ADVERT_IDENTIFIER in src/nostr/types.rs
|
||||
# and are duplicated here rather than derived, so changing either there
|
||||
# silently stops this event reaching the daemons while the relay goes on
|
||||
|
||||
Reference in New Issue
Block a user