Skip to content

security: verify WorkerAnnouncement peer_id matches signer (#144)#151

Merged
intendednull merged 1 commit into
mainfrom
fix/issue-144
Apr 19, 2026
Merged

security: verify WorkerAnnouncement peer_id matches signer (#144)#151
intendednull merged 1 commit into
mainfrom
fix/issue-144

Conversation

@intendednull
Copy link
Copy Markdown
Owner

Summary

  • parse_worker_message now verifies the inner peer_id of
    WorkerAnnouncement and WorkerWireMessage::Departure against
    the Ed25519 signer returned by unpack_wire.
  • Mismatches surface a new WorkerMessageAction::PeerIdMismatch { signer, claimed, kind } variant; the network actor logs
    the rejection at warn level (signer + claimed peer + kind).
  • Genuine messages still parse to Ignore so the existing no-op
    behavior of the Worker arm is preserved until WorkerCache
    integration lands — but it's now a verified drop, not a
    silent one.

Threat addressed

Without this check, any signer on the _willow_workers topic
could forge:

  • WorkerAnnouncement for a different peer → poison a recipient's
    WorkerCache (when integrated) by impersonating an arbitrary
    worker's role/server list.
  • Departure { peer_id } for a different peer → silently evict
    a legitimate worker from a recipient's cache.

The Ed25519 signature already proved who sent the envelope; this
PR ties the self-declared identity inside the message to that
proven sender.

Found by security audit of fix/issue-108-final (#144).

Test plan

  • cargo test -p willow-worker --lib actors::network — 17/17 pass.
  • just check (fmt + clippy + workspace tests + WASM check) — clean.
  • New parse_worker_announcement_ignored_when_peer_id_matches_signer
    test covers the happy path (real Ed25519 keypair, pack_wire
    parse_worker_messageIgnore).
  • New parse_worker_announcement_rejected_when_peer_id_mismatches_signer
    test covers the sad path and asserts the reported signer,
    claimed, and kind fields.
  • Equivalent happy/sad pair added for Departure.
  • Existing unsigned_bytes_rejected, non_worker_wire_message_ignored,
    and the request-path tests still pass — no behavior regression for
    Request/Response flows.

Closes #144.

Generated with Claude Code

`parse_worker_message` previously discarded the verified Ed25519 signer
returned by `unpack_wire` and silently dropped `Announcement` and
`Departure` messages. While the Worker arm is still a no-op pending
WorkerCache integration, an unverified self-declared `peer_id` is a
trust bug: any signer could forge announcements (poisoning a
recipient's WorkerCache) or departures (evicting legitimate workers)
on behalf of a different peer.

Capture the signer from `unpack_wire`, compare it against the inner
`peer_id` for `Announcement` and `Departure`, and surface a new
`WorkerMessageAction::PeerIdMismatch { signer, claimed, kind }`
variant. The network actor logs the rejection at warn level with
both peer IDs so audit logs distinguish a *verified* drop from a
silent one. Genuine messages still parse to `Ignore` so the no-op
behavior is preserved until WorkerCache lands.

Add unit tests using real Ed25519 keypairs:
- happy path: signer == announcement.peer_id → accepted (Ignore)
- sad path: signer != announcement.peer_id → rejected (PeerIdMismatch)
- both paths covered for Announcement and Departure variants.

Closes #144.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@intendednull intendednull merged commit af71349 into main Apr 19, 2026
4 checks passed
@intendednull intendednull deleted the fix/issue-144 branch April 19, 2026 05:26
intendednull added a commit that referenced this pull request Apr 19, 2026
Merge race between #151 and #157: #157 added `pending_count` to
`WorkerRoleInfo::Replay`, then #151's test fixture for the
WorkerAnnouncement sad-path case landed without the new field and
broke `cargo test --workspace` on main.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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.

security: verify WorkerAnnouncement.peer_id against Ed25519 signer

1 participant