in reviewhumanPR #166Identity
fix(identity): ed25519_rotating replay-corruption + destructive re-registration hardening
Red-team remediation of the merged ed25519_rotating identity plugin (#25), following the precedent of #108 red-teaming merged work.
Author
@tony-nexartis
github profile →- Status
- In review
- Opened on
- Jul 10
- Branch
- hackathon/fix-ed25519-rotation-integrity
Description
The pitch.
## What this fixes Red-team remediation of the merged `ed25519_rotating` identity plugin (#25), following the precedent of #108 red-teaming merged work. Every finding below was **reproduced against the merged code with a runnable probe** before fixing — no inspection-only claims. | Finding | Severity | Fix | | --- | --- | --- | | `apply_rotation` replay corrupts key history | Medium (integrity) | Idempotent + chain-tip-only: replaying a rotation announcement is a no-op; a conflicting one raises. `resolve()` determinism restored under replay | | `register_peer` silently wipes a known agent's history | Medium (integrity) | Non-destructive: re-announcing the current key is a no-op; a *different* key for a known agent raises (must go through `apply_rotation`) | | `verify` accepts signatures dated outside the signing key's window | Hardening | `verify` now also rejects when `sig.signed_at` is present and outside the key's validity window — adopted from a partner analysis, whose variant we also fixed (it rejected honest cross-clock peer signatures) | | `verify_continuity` "already applied" escape matches on `new_key_id` only | Defence-in-depth | Now cross-checks `new_public_key` + `issued_at` (not exploitable after the idempotency fix, but closes the gap for downstream consumers) | | Same-tick sign+rotate ambiguity | Inherent limitation — NOT claimed fixed | An honest co-tick old-key signature and a post-rotation forgery are byte-identical under scalar logical time (the repo's own Hypothesis property produces the counterexample at `delta=0`). Instead of silently dropping honest messages or reopening forgery, `rotate_key` now **raises** if called on the tick the agent last signed — fail loud, documented on `verify` | ## Why it matters Identity is the layer every other layer trusts. Before this fix, a single replayed rotation announcement (a plausible byzantine or at-least-once-delivery event) permanently corrupted a peer's key history, and any agent could silently reset another agent's registered key. Both attacks are one message each. ## Verification ```bash uv sync uv run ruff check . && uv run ruff format --check . && uv run pyright uv run pytest packages/nest-plugins-reference/tests/test_ed25519_rotating.py -v uv run pytest -q # full suite green; identity suite 24 -> 34 tests ``` 10 new regression tests: replay-idempotency ×3, non-destructive re-registration ×2, window-bound `signed_at` ×2 (including the honest cross-clock case the partner variant broke), same-tick guard ×2, continuity cross-check ×1. Adjacent-code check: every existing caller of `rotate_key` / `register_peer` / `apply_rotation` in the repo (scenarios `identity_rotation`, `parc_migration`; `trust/parc`; their tests) already advances the clock between sign and rotate, registers each peer once, and applies each rotation once — the stricter semantics break no existing contract, and the full suite stays green. ## Honest limitations - The same-tick boundary is **not fixable in `verify`** without readmitting forgery; we ship a loud grant-side guard instead and document the as-of verification model. - Signatures are bound to a window, not to a point within it (as-of model) — documented, partly mitigated by the `signed_at` window check. ## Credit This remediation was researched and built by **Bin (Yanez team, Nexartis hack squad)**; committed and pushed on his behalf from this account as a maintenance contribution. The `signed_at` window rule was adopted from his partner analysis (BIN-HACK-FIX).
Try it
Open PR on GitHubView diffCheckout locally
git fetch origin pull/166/head:pr-166
git checkout pr-166