mirror of
https://relay.ngit.dev/npub15qydau2hjma6ngxkl2cyar74wzyjshvl65za5k5rl69264ar2exs5cyejr/ngit-grasp.git
synced 2026-10-05 23:18:24 +00:00
fix(nip09): bind deletion tombstones to author to prevent cross-author censorship
A kind-5 deletion that arrives before its target laid down an author-agnostic tombstone: is_event_deleted matched any kind-5 carrying the e-tag id, and is_coordinate_deleted matched any kind-5 carrying the a-tag coordinate, regardless of who signed the deletion. This let any pubkey pre-emptively censor another's events by racing a kind-5 ahead of the target (out-of-order delete). Bind both checks to the rightful author: - is_event_deleted now takes the candidate event's author and only honours a kind-5 signed by that same pubkey; - is_coordinate_deleted requires the kind-5 author to match the pubkey embedded in the coordinate (<kind>:<pubkey>:<d>). Add integration test out_of_order_deletion_by_other_pubkey_not_actioned and unit tests covering both the e-tag and a-tag cross-author cases.
This commit is contained in:
@@ -634,8 +634,14 @@ impl Nip34WritePolicy {
|
||||
return Some(reject_invalid("this pubkey has requested to vanish"));
|
||||
}
|
||||
|
||||
// 2. Deleted event id
|
||||
if self.ctx.tombstones.is_event_deleted(&event.id).await {
|
||||
// 2. Deleted event id (author-bound: only the event's own author may
|
||||
// have deleted it)
|
||||
if self
|
||||
.ctx
|
||||
.tombstones
|
||||
.is_event_deleted(&event.id, &event.pubkey)
|
||||
.await
|
||||
{
|
||||
tracing::debug!(
|
||||
event_id = %event.id.to_hex(),
|
||||
"Rejected re-submission of deleted event"
|
||||
|
||||
@@ -728,7 +728,9 @@ mod tests {
|
||||
assert!(matches!(result, WritePolicyResult::Accept));
|
||||
// No tombstone recorded -> re-submission of the target is NOT blocked.
|
||||
assert!(
|
||||
!ctx.tombstones.is_event_deleted(&target).await,
|
||||
!ctx.tombstones
|
||||
.is_event_deleted(&target, &keys.public_key())
|
||||
.await,
|
||||
"Disrespector mode must NOT record a deletion tombstone"
|
||||
);
|
||||
}
|
||||
|
||||
+80
-9
@@ -131,14 +131,21 @@ impl Tombstones {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Has this event id been deleted by a recorded kind-5 (`e` tag)?
|
||||
/// Has this event id been deleted by a recorded kind-5 (`e` tag) authored by
|
||||
/// `author`?
|
||||
///
|
||||
/// Mirrors the backend's `is_deleted` check that previously fed the
|
||||
/// relay-builder `check_id` gate.
|
||||
pub async fn is_event_deleted(&self, id: &EventId) -> bool {
|
||||
// Any stored kind-5 carrying this id in an `e` tag means it was deleted.
|
||||
/// relay-builder `check_id` gate, but with explicit author binding: only the
|
||||
/// event's own author may delete it (NIP-09). A deletion request signed by a
|
||||
/// different pubkey must NOT block (re-)submission of `author`'s event —
|
||||
/// otherwise any pubkey could pre-emptively censor another's events by
|
||||
/// racing a kind-5 ahead of the target (out-of-order delete).
|
||||
pub async fn is_event_deleted(&self, id: &EventId, author: &PublicKey) -> bool {
|
||||
// A stored kind-5 authored by `author` carrying this id in an `e` tag
|
||||
// means `author` deleted their own event.
|
||||
let filter = Filter::new()
|
||||
.kind(Kind::EventDeletion)
|
||||
.author(*author)
|
||||
.custom_tag(SingleLetterTag::lowercase(Alphabet::E), id.to_hex());
|
||||
match self.db.query(filter).await {
|
||||
Ok(events) => !events.is_empty(),
|
||||
@@ -159,19 +166,31 @@ impl Tombstones {
|
||||
/// to the deletion request's `created_at`. So a candidate event is blocked
|
||||
/// only if a recorded deletion for the same coordinate has
|
||||
/// `deletion.created_at >= event_created_at`.
|
||||
///
|
||||
/// Author binding: a coordinate is `<kind>:<pubkey>:<d>`, so only the pubkey
|
||||
/// embedded in the coordinate may delete it. A kind-5 signed by a different
|
||||
/// pubkey referencing this coordinate is ignored, otherwise any pubkey could
|
||||
/// pre-emptively censor another's addressable events.
|
||||
pub async fn is_coordinate_deleted(
|
||||
&self,
|
||||
coordinate: &str,
|
||||
event_created_at: Timestamp,
|
||||
) -> bool {
|
||||
// The coordinate owner (pubkey hex) is the only party allowed to delete
|
||||
// it. Extract it so we can require the kind-5 author to match.
|
||||
let coord_owner_hex = coordinate.split(':').nth(1);
|
||||
|
||||
let filter = Filter::new().kind(Kind::EventDeletion).custom_tag(
|
||||
SingleLetterTag::lowercase(Alphabet::A),
|
||||
coordinate.to_string(),
|
||||
);
|
||||
match self.db.query(filter).await {
|
||||
Ok(events) => events
|
||||
.iter()
|
||||
.any(|deletion| deletion.created_at >= event_created_at),
|
||||
Ok(events) => events.iter().any(|deletion| {
|
||||
deletion.created_at >= event_created_at
|
||||
&& coord_owner_hex
|
||||
.map(|owner| owner == deletion.pubkey.to_hex())
|
||||
.unwrap_or(false)
|
||||
}),
|
||||
Err(e) => {
|
||||
tracing::error!(error = %e, "Tombstone query failed for is_coordinate_deleted");
|
||||
false
|
||||
@@ -212,12 +231,39 @@ mod tests {
|
||||
let keys = Keys::generate();
|
||||
let target = EventId::all_zeros();
|
||||
|
||||
assert!(!store.is_event_deleted(&target).await);
|
||||
assert!(!store.is_event_deleted(&target, &keys.public_key()).await);
|
||||
|
||||
let deletion = deletion_by_event(&keys, target);
|
||||
store.record_deletion(&deletion).await.unwrap();
|
||||
|
||||
assert!(store.is_event_deleted(&target).await);
|
||||
assert!(store.is_event_deleted(&target, &keys.public_key()).await);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn deletion_does_not_block_a_different_authors_event() {
|
||||
// A kind-5 from pubkey B targeting an event id must NOT mark that id as
|
||||
// deleted for a different author A: only the event's own author may
|
||||
// delete it (NIP-09). Guards against pre-emptive cross-author censorship
|
||||
// when the deletion races ahead of the target (out-of-order delete).
|
||||
let store = Tombstones::in_memory();
|
||||
let attacker = Keys::generate();
|
||||
let victim = Keys::generate();
|
||||
let target = EventId::all_zeros();
|
||||
|
||||
let deletion = deletion_by_event(&attacker, target);
|
||||
store.record_deletion(&deletion).await.unwrap();
|
||||
|
||||
// The attacker "deleted" it for themselves...
|
||||
assert!(
|
||||
store
|
||||
.is_event_deleted(&target, &attacker.public_key())
|
||||
.await
|
||||
);
|
||||
// ...but the victim's identical id is NOT considered deleted.
|
||||
assert!(
|
||||
!store.is_event_deleted(&target, &victim.public_key()).await,
|
||||
"a deletion from another pubkey must not block the victim's event"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
@@ -252,6 +298,31 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn coordinate_deletion_by_other_pubkey_is_ignored() {
|
||||
// A kind-5 from pubkey B carrying an `a` tag for a coordinate owned by
|
||||
// pubkey A must NOT mark that coordinate as deleted: only the pubkey
|
||||
// embedded in the coordinate may delete it.
|
||||
let store = Tombstones::in_memory();
|
||||
let victim = Keys::generate();
|
||||
let attacker = Keys::generate();
|
||||
let coord = format!("30618:{}:my-repo", victim.public_key().to_hex());
|
||||
|
||||
let deletion = EventBuilder::new(Kind::EventDeletion, "")
|
||||
.tags(vec![Tag::custom("a", vec![coord.clone()])])
|
||||
.custom_created_at(Timestamp::from_secs(1000))
|
||||
.finalize(&attacker)
|
||||
.unwrap();
|
||||
store.record_deletion(&deletion).await.unwrap();
|
||||
|
||||
assert!(
|
||||
!store
|
||||
.is_coordinate_deleted(&coord, Timestamp::from_secs(1000))
|
||||
.await,
|
||||
"a coordinate deletion signed by a different pubkey must be ignored"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn detects_pubkey_vanish() {
|
||||
let store = Tombstones::in_memory();
|
||||
|
||||
+94
-12
@@ -278,10 +278,9 @@ async fn deletion_of_nonexistent_event_is_accepted() {
|
||||
.expect("create audit client");
|
||||
|
||||
// A random id the relay has never stored.
|
||||
let nonexistent = EventId::from_hex(
|
||||
"0000000000000000000000000000000000000000000000000000000000000001",
|
||||
)
|
||||
.expect("parse nonexistent id");
|
||||
let nonexistent =
|
||||
EventId::from_hex("0000000000000000000000000000000000000000000000000000000000000001")
|
||||
.expect("parse nonexistent id");
|
||||
|
||||
let deletion = build_deletion(&client, &[nonexistent], &[]);
|
||||
|
||||
@@ -317,7 +316,12 @@ async fn deletion_before_target_rejects_later_submission() {
|
||||
let (announcement, _repo_id) = publish_served_repo(&client, "delete-first").await;
|
||||
|
||||
let issue = client
|
||||
.create_issue(&announcement, "Issue deleted before it arrives", "delete me", vec![])
|
||||
.create_issue(
|
||||
&announcement,
|
||||
"Issue deleted before it arrives",
|
||||
"delete me",
|
||||
vec![],
|
||||
)
|
||||
.expect("build issue");
|
||||
let issue_id = issue.id;
|
||||
|
||||
@@ -371,6 +375,84 @@ async fn deletion_before_target_rejects_later_submission() {
|
||||
);
|
||||
}
|
||||
|
||||
/// 2c. Out-of-order deletion by a DIFFERENT pubkey must NOT be actioned.
|
||||
///
|
||||
/// Author B sends a kind-5 targeting (by `e` tag) an event id that the relay
|
||||
/// has never seen — an id that will later belong to author A's event. Because
|
||||
/// B is not the author of the target, the deletion must not lay down a
|
||||
/// tombstone that censors A: when A subsequently submits the genuine event it
|
||||
/// must be accepted and served.
|
||||
///
|
||||
/// This is the cross-author counterpart of
|
||||
/// `deletion_before_target_rejects_later_submission`. The earlier
|
||||
/// `deletion_author_mismatch_rejected` test only covers the case where the
|
||||
/// target already exists; this one covers the harder out-of-order case where B
|
||||
/// races ahead of A and could otherwise pre-emptively block A's event.
|
||||
#[tokio::test]
|
||||
async fn out_of_order_deletion_by_other_pubkey_not_actioned() {
|
||||
let relay = TestRelay::start().await;
|
||||
|
||||
let author_a = AuditClient::new(relay.url(), AuditConfig::isolated())
|
||||
.await
|
||||
.expect("create author A client");
|
||||
let author_b = AuditClient::new(relay.url(), AuditConfig::isolated())
|
||||
.await
|
||||
.expect("create author B client");
|
||||
|
||||
// A promotes a repo and builds (but does NOT yet send) an issue, so we know
|
||||
// the id of A's future event.
|
||||
let (announcement_a, _repo_id) = publish_served_repo(&author_a, "other-pubkey-race").await;
|
||||
let issue = author_a
|
||||
.create_issue(&announcement_a, "A's issue", "owned by A", vec![])
|
||||
.expect("build issue");
|
||||
let issue_id = issue.id;
|
||||
|
||||
// Sanity: the relay has never seen the issue.
|
||||
assert!(
|
||||
!author_a
|
||||
.is_event_on_relay(issue_id)
|
||||
.await
|
||||
.expect("query issue before anything is sent"),
|
||||
"issue should not be served before it is ever submitted"
|
||||
);
|
||||
|
||||
// B races ahead and sends a deletion targeting A's not-yet-seen event id.
|
||||
// The relay cannot yet know the author of the target. Whether the relay
|
||||
// accepts B's kind-5 (as a pre-emptive no-op) or rejects it outright, the
|
||||
// critical requirement is that it MUST NOT lay down a tombstone that blocks
|
||||
// A's genuine event.
|
||||
let deletion = build_deletion(&author_b, &[issue_id], &[]);
|
||||
let _ = author_b.send_event(deletion).await;
|
||||
|
||||
tokio::time::sleep(Duration::from_millis(200)).await;
|
||||
|
||||
// A now submits the genuine event. It must be accepted: B's deletion of an
|
||||
// event B does not own must never have been actioned.
|
||||
let result = author_a.send_event(issue.clone()).await;
|
||||
|
||||
tokio::time::sleep(Duration::from_millis(200)).await;
|
||||
|
||||
let served = author_a
|
||||
.is_event_on_relay(issue_id)
|
||||
.await
|
||||
.expect("query A's issue after submission");
|
||||
|
||||
relay.stop().await;
|
||||
|
||||
assert!(
|
||||
result.is_ok(),
|
||||
"A's genuine event must be accepted: a deletion from another pubkey \
|
||||
must not block it, but submission was rejected: {:?}",
|
||||
result.err()
|
||||
);
|
||||
assert!(
|
||||
served,
|
||||
"A's issue {} must be served: a deletion request from another pubkey \
|
||||
must not be actioned against A's events",
|
||||
issue_id
|
||||
);
|
||||
}
|
||||
|
||||
/// 3. Author B cannot delete an event owned by author A: the relay rejects the
|
||||
/// kind-5 with a message mentioning the authorship problem.
|
||||
#[tokio::test]
|
||||
@@ -583,9 +665,9 @@ async fn deletion_invalid_address_format_handled_gracefully() {
|
||||
.expect("create audit client");
|
||||
|
||||
let malformed = [
|
||||
"invalid:format", // only two parts
|
||||
"notanumber:pk:id", // unparseable kind
|
||||
"30617:badhex:id", // unparseable pubkey hex
|
||||
"invalid:format", // only two parts
|
||||
"notanumber:pk:id", // unparseable kind
|
||||
"30617:badhex:id", // unparseable pubkey hex
|
||||
];
|
||||
|
||||
for coord in malformed {
|
||||
@@ -612,10 +694,10 @@ async fn deletion_invalid_address_format_handled_gracefully() {
|
||||
// prove it by round-tripping a normal acceptance.
|
||||
let probe = build_deletion(
|
||||
&client,
|
||||
&[EventId::from_hex(
|
||||
"0000000000000000000000000000000000000000000000000000000000000002",
|
||||
)
|
||||
.expect("parse probe id")],
|
||||
&[
|
||||
EventId::from_hex("0000000000000000000000000000000000000000000000000000000000000002")
|
||||
.expect("parse probe id"),
|
||||
],
|
||||
&[],
|
||||
);
|
||||
let probe_result = client.send_event(probe).await;
|
||||
|
||||
Reference in New Issue
Block a user