Skip to content

fix(pairing): deliver the paired token only to the client that requested it - #592

Draft
MichaelTaylor3d wants to merge 2 commits into
developfrom
fix/3191-pairing-redemption
Draft

fix(pairing): deliver the paired token only to the client that requested it#592
MichaelTaylor3d wants to merge 2 commits into
developfrom
fix/3191-pairing-redemption

Conversation

@MichaelTaylor3d

Copy link
Copy Markdown
Contributor

Hardens the OPEN pairing plane so the paired token is delivered only to the client that asked for it, and so a pending pairing request the node has already accepted is never displaced by a later one.

Refs DIG-Network/dig_ecosystem#3191

What changes

1. The value the operator handles is no longer the value that redeems the token

pairing.request now returns a redemption_secret alongside pairing_id, and pairing.poll requires it.

The two values have different jobs, and separating them is the point:

  • pairing_id is a handle. It is what control.pairing.list shows the operator and what dig-node pair approve <pairing_id> takes. It identifies a pending request; it does not authorize anything. Its appearance in an operator's terminal, shell history or process arguments is therefore correct and needs no change.
  • redemption_secret is a credential. It is minted from the OS CSPRNG in the same fail-closed step as the pairing id and code, returned only in the pairing.request response to the requesting client, and is never displayed, never returned by control.pairing.list or control.pairing.approve, and never logged.

pairing.poll behaves three ways at the decision point:

caller supplies response
no redemption_secret INVALID_PARAMS, naming the missing field
a redemption_secret that does not match {"status":"unknown"} — byte-identical to the answer for an id that was never issued, and the pending entry is left intact
the matching redemption_secret unchanged behaviour: pending, or approved with the token delivered exactly once, or expired

A missing field is a shape error that reveals nothing about any particular id, so it gets a named, diagnosable refusal. A wrong value must be indistinguishable from a wrong id — otherwise poll becomes an existence oracle over the id space. The comparison is constant-time.

The consent model is untouched: approval still requires the master control token, and the compare-codes step is unchanged. The master token is not widened in any way (see dig-node#403 — it stays 0600 root:root).

2. A pending request is never displaced

At capacity, pairing.request previously evicted the oldest pending entry to make room for the newcomer. It now refuses the newcomer instead, with a new error code PAIRING_PENDING_LIMITED (-32034, retriable).

This matches the discipline this module already applies and documents for an over-long client_name: it refuses rather than silently shortening, on the reasoning that a value the node rewrote is a value the node partly wrote. A pending request the node has accepted is a commitment to the client that made it, and quietly dropping it to serve a later caller is the same failure in a different costume.

The change also closes a second, quieter path in the same block: the eviction candidate was filtered to unapproved entries, but the insert that followed ran unconditionally. With every held entry approved-but-not-yet-polled there was no candidate to evict, nothing was removed, and the map grew past MAX_PENDING with no ceiling — approved entries are retained by prune() regardless of expiry, so only a successful poll removes them. Refusing at capacity bounds both.

A pairing-specific token bucket (capacity 8, refilling one per 10s) additionally bounds the rate of pairing.request.

3. Where the bound lives, and why it is not the ingress limiter

The bucket is enforced inside pairing::request, not in the server dispatcher.

pairing.request is dispatched from two places — the HTTP JSON-RPC path and the WebSocket path. A bound written at a dispatch site is a bound the next person must remember to write twice, and the second transport is the one that ships unbounded. Putting it in the callee makes "both planes are covered" true by construction.

It is deliberately not routed through control_ingress_admits. That gate exists to bound an anonymous, network-reachable caller and admits RequestorId::Local unconditionally, by design and with a regression test asserting exactly that. This change does not modify, weaken or reference it — server.rs is untouched by this PR.

The justification for bounding pairing.request where that limiter declines to bound an open read is a difference in kind, not a difference in size: an open control read costs nothing durable, whereas each admitted pairing.request holds one of a small number of pending slots for up to five minutes. The bound also does not touch pairing.poll, which is the polled method in this flow — so a client polling for approval on a normal interval is never refused by it. a_normal_operator_pairing_sequence_is_never_refused pins that.

Consumer compatibility

Consumer compatibility. pairing.poll now requires the redemption_secret that pairing.request returns, so the paired token is delivered only to the client that made the request. The currently published dig-chrome-extension calls pairing.poll with pairing_id alone (src/background/index.ts:510) and will therefore not complete a new pairing against a node running this release. The failure is bounded and already-localized, not a crash: the extension's poll helper returns null on a non-result response (src/lib/dig-pairing.ts:147), the controller keeps polling until the 5-minute deadline, and the flow terminates in the existing expired phase, rendering the shipped control.pairing.expired.* strings in all supported locales. Extensions that are already paired are unaffected — the paired-token store, the token gate and pairing.revoke are untouched, and hydrate() restores phase:"paired" as before. The extension change is a one-line pass-through of the new field and is tracked separately.

The in-repo consumer, dign pair, is updated in this PR: pair.rs carries the redemption_secret from the request response through to the poll.

What this does not promise

With a fixed number of shared pending slots and no per-caller identity available on a loopback transport, a local process can still occupy pending slots and cause a legitimate pairing to be refused. What this PR does: it removes the ability to displace a request the node already accepted, closes the unbounded-growth path, and converts a silent failure into a visible, retriable, self-healing refusal that resolves within the pending TTL. What it does not do: guarantee a pairing slot is available on a machine running hostile local software. That is a scoping statement, not a mitigation.

Contract crate

The wire types and KATs live in the published dig-node-control-interface crate. pairing.rs handles these envelopes with raw serde_json, so this change compiles against the pinned version untouched. Carrying redemption_secret and PAIRING_PENDING_LIMITED into the typed structs and KATs, publishing, and then migrating pairing.rs onto them is tracked separately and follows release-first ordering.

Tests

Each asserts at the decision point, inside pairing::request / pairing::poll:

  • a_poll_without_the_requesters_redemption_secret_does_not_deliver_the_token — and a correct poll afterwards still delivers, proving the refused poll did not consume the entry
  • a_poll_with_a_wrong_redemption_secret_is_indistinguishable_from_an_unknown_id — byte-equal responses, entry survives
  • the_redemption_secret_is_never_serialized_by_list_or_approve
  • a_pending_request_is_never_displaced_by_a_later_request
  • a_pending_map_at_capacity_of_approved_entries_does_not_grow
  • a_burst_of_pairing_requests_is_bounded_by_the_pairing_slot_budget
  • a_normal_operator_pairing_sequence_is_never_refused
  • the_pair_client_polls_with_the_redemption_secret_it_received

… the pending-slot bound

TDD red checkpoint, salvaged from an interrupted lane. These tests do not
compile yet: they name the redemption secret, the slot budget and the bucket
fields that the follow-up commit introduces. Committed so the work is not lost.

Adds ErrorCode::PairingPendingLimited (-32034, PAIRING_PENDING_LIMITED, node
class, retriable) and seven tests asserting at the decision point in
pairing::request / pairing::poll.

Refs DIG-Network/dig_ecosystem#3191
…ted it

pairing.request now returns a redemption_secret alongside the pairing_id, and
pairing.poll requires it. The pairing_id stays the handle the operator sees and
approves; the redemption_secret is the credential that redeems the minted token,
is returned only to the requesting client, and is never displayed, listed or
logged. A poll carrying a wrong secret is answered exactly like a poll for an
unknown id, so the endpoint does not become an existence oracle over the id
space; comparison is constant-time.

A pending request the node has already accepted is now never displaced to make
room for a later one: at capacity pairing.request is refused with
PAIRING_PENDING_LIMITED rather than evicting the oldest entry. This matches the
refusal discipline this module already applies to an over-long client_name, and
it also removes a path where the map could grow past MAX_PENDING, since the
eviction candidate was filtered but the insert was not. A pairing-specific token
bucket bounds request rate; it lives inside pairing::request, so both the HTTP
and the WebSocket dispatch inherit it rather than each needing its own guard.

dign pair now carries the redemption_secret from request through to poll.

Refs DIG-Network/dig_ecosystem#3191
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant