Skip to content

Tickets can be reused by multiple registrants (missing advertiser binding) #68

Description

@srene

Summary

The topic-registration ticket issued by a registrar is not bound to the advertiser that requested it. As a result, a single ticket can be redeemed by multiple distinct identities within its validity window, defeating the wait-time admission control that DISC-NG relies on.

This was raised in external review of discv5-theory.md and discv5-wire.md and is real in the current p2p/discover/topicindex/ticket.go implementation.

Where the ticket lives

The Ticket struct (p2p/discover/topicindex/ticket.go:43):

type Ticket struct {
    KeyID          uint16
    Topic          TopicID         // service binding
    WaitTimeIssued time.Duration
    LastUsed       mclock.AbsTime  // timing
    FirstIssued    mclock.AbsTime  // timing
}
// + 32-byte HMAC, computed over the encoded struct

There is no field tying the ticket to the advertiser's node ID / ENR. The HMAC binds the ticket to the registrar (only the issuer can produce a valid one) but says nothing about who it was issued to.

TicketSealer.Unpack (ticket.go:93) verifies MAC, topic, and the time window — but never compares the ticket to the calling source identity.

Attack

Sybil sharing of a single ticket:

1. Attacker (Alice) → REGTOPIC(topic=T, ENR=Alice_ENR, ticket=empty)
2. Registrar         → TICKET(t_T_W, wait=W)      # binds (T, timing) only
3. Alice broadcasts t_T_W to her swarm {Bob, Carol, Dave, ...}
4. After W elapses, all of them send:
   Bob   → REGTOPIC(T, ENR_Bob,   ticket=t_T_W)
   Carol → REGTOPIC(T, ENR_Carol, ticket=t_T_W)
   Dave  → REGTOPIC(T, ENR_Dave,  ticket=t_T_W)
5. Registrar: MAC ✓, topic ✓, time window ✓ → admits all three.

One wait → N admissions, where N is bounded only by how many Sybil identities the attacker controls and how fast they can fire REGTOPIC retries before ticketValidityWindow (5s) elapses.

The IP similarity tree dampens future wait times if the Sybils cluster in nearby subnets, but for the current admission it does not gate the redemption — and an attacker with addresses spread across distinct subnets is unaffected.

What the spec says

discv5-theory.md:509:

A ticket is bound to the advertisement, the registrar, and timing information.

Per discv5-theory.md:443, an "advertisement" identifies the advertiser by ENR. So the theory already requires advertiser binding — the implementation is non-compliant.

(A separate spec issue is that the theory references the wire-format document for the exact encoding while the wire spec says the ticket is opaque, so the binding requirement is not nailed down anywhere explicit. That will be addressed in a parallel devp2p PR.)

Proposed fix

Add a 32-byte advertiser identity to the ticket struct and include it under the HMAC. Verify it against the REGTOPIC packet's authenticated source on every retry:

type Ticket struct {
    KeyID          uint16
    Topic          TopicID
    AdvertiserID   enode.ID        // NEW: 32-byte node ID of the advertiser
    WaitTimeIssued time.Duration
    LastUsed       mclock.AbsTime
    FirstIssued    mclock.AbsTime
}
  • Issuance: TicketSealer.Pack populates AdvertiserID from the REGTOPIC source node ID.
  • Redemption: on every retry, the registrar compares ticket.AdvertiserID to the source node ID of the incoming REGTOPIC; mismatch → reject (treat as a fresh attempt with no ticket).

The MAC continues to cover the entire encoded struct including the new field, so any tampering / sharing breaks the MAC check (or, for a non-tampered shared ticket, fails the ID equality check explicitly).

Encoded size impact

Current ticket: 32 (MAC) + 2 + 32 + 8 + 8 + 8 = 90 bytes.

With the new field: 32 (MAC) + 2 + 32 + 32 + 8 + 8 + 8 = 122 bytes.

Still well below any wire-message size constraint.

Wire compatibility

No wire-format change — REGTOPIC carries the ticket as an opaque byte array. The ticket's internal layout is registrar-internal; only the issuing registrar inspects it. Implementations that pre-date this change continue to interoperate (they just don't get the binding protection on their own issued tickets).

Tests to add

  • Unit test in topicindex/ticket_test.go: pack a ticket for advertiser A; attempting to unpack/redeem with expectedAdvertiserID = B returns an error.
  • Integration test in discover/: end-to-end REGTOPIC retry with a swapped source node ID is rejected.

Related

  • External reviewer feedback (also raised: the wire spec doesn't specify ticket internals and the theory ticket-encoding cross-reference is circular; a parallel devp2p PR will tighten the required-properties section in discv5-theory.md).

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions