SaveIdentity accepts a trust root it cannot use #103

Closed
opened 2026-08-23 10:13:14 +00:00 by a73x · 1 comment
a73x commented 2026-08-23 10:13:14 +00:00 (Migrated from git.a73x.sh)

[claude 2026-08-23] Found while writing TestJoinPinsTheBlobsCertFingerprint for 25dd592e (item 7). The test pins the wiring; this makes the mistake unrepresentable.

state.SaveIdentity persists whatever ServerCertSHA256 it is handed — including "" and including garbage. The agent's TLS VerifyConnection callback compares every peer against that stored string, so an unpinned or malformed identity fails closed: the agent enrolls successfully, reports success, and then refuses every server it ever dials. The failure surfaces at first sync as a certificate mismatch, nowhere near the join that caused it.

joinblob.validate already enforces exactly this shape (64 lowercase hex) on the way IN. The gap is that nothing enforces it on the way DOWN, so any future caller that assembles an Identity by hand — or any refactor that reorders the field assignments in join() — can persist a trust root the agent cannot use, and the only alarm is a cryptic dial failure later.

Proposed: SaveIdentity returns an error when ServerCertSHA256 is not 64 lowercase hex. Fail loudly at join time, where the operator is still watching, instead of at first dial. This repo has precedent for refusing to persist/boot on an unusable config — the sshgate wildcard-bind refusal is the same move.

Scope: internal/agent/state. One guard, one test asserting the refusal, plus whatever join() needs to surface it. Check no test constructs an Identity with a placeholder fingerprint before landing it.

Related: the same reasoning would cover ServerQUICAddr (an empty address is equally unusable), but that is a separate judgement — this issue is scoped to the trust root.


Migrated from git-collab issue 9fef9fb17e6ee157d43088d674474ece8b0a45ab.

Original author: a73x dev@a73x.sh.

Close reason: [claude 2026-08-23] Merged to main in d0c032f (pushed to both remotes). Every test proven by the mutant it kills; Fable's checkpoint review passed it merge-as-is. make ci green, boot gate PASS (full smoke incl. UDP, cold_start=20s).

[claude 2026-08-23] Found while writing `TestJoinPinsTheBlobsCertFingerprint` for 25dd592e (item 7). The test pins the wiring; this makes the mistake unrepresentable. `state.SaveIdentity` persists whatever `ServerCertSHA256` it is handed — including `""` and including garbage. The agent's TLS `VerifyConnection` callback compares every peer against that stored string, so an unpinned or malformed identity fails **closed**: the agent enrolls successfully, reports success, and then refuses every server it ever dials. The failure surfaces at first sync as a certificate mismatch, nowhere near the join that caused it. `joinblob.validate` already enforces exactly this shape (64 lowercase hex) on the way IN. The gap is that nothing enforces it on the way DOWN, so any future caller that assembles an `Identity` by hand — or any refactor that reorders the field assignments in `join()` — can persist a trust root the agent cannot use, and the only alarm is a cryptic dial failure later. **Proposed:** `SaveIdentity` returns an error when `ServerCertSHA256` is not 64 lowercase hex. Fail loudly at join time, where the operator is still watching, instead of at first dial. This repo has precedent for refusing to persist/boot on an unusable config — the sshgate wildcard-bind refusal is the same move. **Scope:** `internal/agent/state`. One guard, one test asserting the refusal, plus whatever `join()` needs to surface it. Check no test constructs an `Identity` with a placeholder fingerprint before landing it. Related: the same reasoning would cover `ServerQUICAddr` (an empty address is equally unusable), but that is a separate judgement — this issue is scoped to the trust root. --- Migrated from git-collab issue `9fef9fb17e6ee157d43088d674474ece8b0a45ab`. Original author: a73x <dev@a73x.sh>. Close reason: [claude 2026-08-23] Merged to main in d0c032f (pushed to both remotes). Every test proven by the mutant it kills; Fable's checkpoint review passed it merge-as-is. make ci green, boot gate PASS (full smoke incl. UDP, cold_start=20s).
a73x commented 2026-08-23 10:37:54 +00:00 (Migrated from git.a73x.sh)

[claude 2026-08-23] Done on branch loud-misconfiguration. SaveIdentity now refuses a ServerCertSHA256 that is not 64 lowercase hex (names.IsSHA256Hex), naming the consequence in the error: the string is the agent's only means of authenticating the control plane, and an unusable one enrols cleanly then refuses every server the host ever dials. join() wraps it to say the enroll succeeded, nothing was written, and the blob's one-shot token is spent. Proven by the mutant: guard disabled, TestSaveIdentityRefusesAnUnusableTrustRoot fails on the empty fingerprint and nothing else in the package fails. One fixture had to be fixed properly: run.storeWithIdentity persisted an Identity with no fingerprint at all. ServerQUICAddr NOT included, reasoning on the branch report.

[claude 2026-08-23] Done on branch loud-misconfiguration. SaveIdentity now refuses a ServerCertSHA256 that is not 64 lowercase hex (names.IsSHA256Hex), naming the consequence in the error: the string is the agent's only means of authenticating the control plane, and an unusable one enrols cleanly then refuses every server the host ever dials. join() wraps it to say the enroll succeeded, nothing was written, and the blob's one-shot token is spent. Proven by the mutant: guard disabled, TestSaveIdentityRefusesAnUnusableTrustRoot fails on the empty fingerprint and nothing else in the package fails. One fixture had to be fixed properly: run.storeWithIdentity persisted an Identity with no fingerprint at all. ServerQUICAddr NOT included, reasoning on the branch report.
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
a73x/eitri#103
No description provided.