Skip to content

fix(capability): unify control-plane version and fix TS2021 endianness - #94

Merged
forkwright merged 3 commits into
mainfrom
fix/64-capability-version-be
Aug 16, 2026
Merged

fix(capability): unify control-plane version and fix TS2021 endianness#94
forkwright merged 3 commits into
mainfrom
fix/64-capability-version-be

Conversation

@forkwright

@forkwright forkwright commented Aug 16, 2026

Copy link
Copy Markdown
Owner

Finding

dictyon sent the 2-byte TS2021 initiation version little-endian against a big-endian reference protocol, and independently restated the capability version at four call sites that had already drifted apart (1, 71, 68, and one omitted field) instead of deriving all of them from one truthful value.

Evidence

  • crates/dictyon/src/noise/mod.rs:228 (pre-fix) wrote the version field with PROTOCOL_VERSION.to_le_bytes(). The reference implementation (control/controlbase/messages.go in the Tailscale Go source) writes and reads this field with binary.BigEndian.PutUint16 / binary.BigEndian.Uint16 — confirmed by fetching that file directly. PROTOCOL_VERSION was 1 (crates/dictyon/src/noise/mod.rs:40, pre-fix), so hamma put 01 00 on the wire; a conforming peer reading that big-endian decodes 256, not 1.
  • crates/dictyon/src/wire/mod.rs:47 (pre-fix) independently declared KEY_PATH: &str = "/key?v=71".
  • crates/dictyon/src/control/mod.rs:441 (pre-fix) independently declared MapRequest { version: 68, .. }.
  • crates/mitos/src/types/mod.rs:30-58 (pre-fix) RegisterRequest had no Version field at all, though the reference RegisterRequest in tailcfg.go carries one.
  • Confirmed via the Tailscale Go source that all four surfaces — the /key?v=N query parameter (control/controlclient/direct.go: fmt.Sprintf("%v/key?v=%d", serverURL, tailcfg.CurrentCapabilityVersion)), the TS2021 wire version, RegisterRequest.Version, and MapRequest.Version — are one value, tailcfg.CurrentCapabilityVersion, not four independently-set numbers.
  • crates/dictyon/tests/wire_integration.rs (pre-fix) — the mock server ("oracle") hardcoded its own NOISE_PROLOGUE: &[u8] = b"...v1" and its decode_handshake_header helper decoded but never asserted the version bytes, so a client-side endianness regression would pass every test in the suite: both sides were wrong the same way.

Why this matters

A wire-format mismatch this specific can't be caught by a round-trip test against the client's own encoder/decoder — that only proves hamma agrees with itself. Against any conforming TS2021 peer the two sides mix different bytes into the Noise handshake hash and the transcript cannot authenticate, so dictyon cannot complete a handshake with a real control server. That is Phase A's stated acceptance condition (README.md: "Phase A — dictyon client against tailscale.com. Validates the Rust client on a production reference server").

Four independent literals for one protocol value is also a structural invitation for exactly this kind of divergence to happen again the next time any one of these four call sites is touched without the others.

Desired correction

  • Added mitos::capability::CapabilityVersion / mitos::CAPABILITY_VERSION (crates/mitos/src/capability.rs) as the single typed source. to_be_bytes() is the only way to get wire bytes out of it, so there is no bare u16 left for a call site to independently mis-encode.
  • noise/mod.rs now writes CAPABILITY_VERSION.to_be_bytes() and builds the prologue from CAPABILITY_VERSION; wire/mod.rs's key_path(), control/mod.rs's MapRequest/RegisterRequest construction, and mitos::types::RegisterRequest's new Version field all derive from the same constant. crates/dictyon/src/transport.rs and crates/dictyon/examples/connect.rs had their own hand-rolled snow fixtures still declaring "...v1" literally — fixed inline, since a fixture whose prologue silently stops matching the real client's is a handshake failure waiting to happen, not a cosmetic string.
  • Set to 71, not upstream's current CurrentCapabilityVersion (well past 100): that value gates client features (node attrs, DNS extensions, ...) hamma does not implement, so advertising it would be a false capability claim. 71 was already the value dictyon spoke at its most-deliberately-reasoned call site (the /key endpoint's own prior comment: "the wire version dictyon speaks") before the three other call sites had drifted away from it.
  • Tests, each independently exhibiting the property named:
    • mitos::capability::tests::capability_version_encodes_big_endian pins CAPABILITY_VERSION.to_be_bytes() to the literal [0x00, 0x47] — not re-derived through the function under test.
    • dictyon::noise::tests::handshake_initiation_produces_message and initiation_frame_has_correct_structure now assert the same literal big-endian bytes on NoiseHandshake::initiation_message()'s real output (previously asserted the little-endian bug as correct behavior).
    • wire_integration.rs's mock server now asserts the exact GET /key?v=71 HTTP/1.1 request line the real client sent over TLS, and decode_handshake_header asserts the initiation's version bytes equal CAPABILITY_VERSION.to_be_bytes() — this is the "independently implemented oracle" the issue asked for: a raw snow::Builder responder, not dictyon::noise, checking the real bytes the real client put on the wire over a real TLS connection.
    • control::tests::register_and_map_requests_advertise_the_same_capability_version builds both real JSON payloads and compares them to each other, not just each to the constant — a future one-sided hardcode fails this even if it coincidentally still equals 71 today.

CI-red fix (adversarial review round 2)

An independent review found CI genuinely red and deterministic — clippy::doc_markdown (implied by -D warnings) failing on bare technical words in new doc comments, and flagged that because clippy stops at the first failing crate without --keep-going, dictyon (which carries most of this PR's new doc comments) had never actually been lint-checked against this diff.

Three bare identifiers, found and fixed one compile-cycle at a time against the real GH Actions gate / full-gate-build job (each fix confirmed against the next run's actual error, not guessed):

  • crates/mitos/src/capability.rs:29 — bare WireGuard in the CAPABILITY_VERSION doc comment (the originally-reported failure; run 31919989643/job 95098225345, error at capability.rs:29:22).
  • crates/dictyon/src/control/tests.rs:446 — bare CAPABILITY_VERSION in a test's doc comment, only reachable once the mitos fix above let clippy proceed into dictyon (run 31921865061/job 95102872229, error at control/tests.rs:446:40, could not compile dictyon (lib test)).
  • crates/dictyon/src/noise/tests.rs:439 — bare MAX_FRAME_PAYLOAD in a test's doc comment, same class as the previous fix, in the same target; fixed in the same round rather than waiting for a fourth cycle to confirm it separately.

Verified clean beyond these three: every //////! doc comment across the whole mitos + dictyon crate trees (diff and pre-existing) was scanned for the same two clippy doc_markdown triggers (a bare identifier containing _, and a bare mixed-case CamelCase word not glued to adjacent text) — no other candidate remains, and this matches the real CI runs above turning up exactly these three, one dependency-layer at a time, and nothing else in the crates that had never been reached before.

Confirmed on the current head (a33b569): gate / full-gate-build — which runs fmt, check --workspace --all-targets, clippy --workspace --all-targets -- -D warnings, nextest --workspace, and doctest as sequential steps in one job — passes (run 31922095589, 1m36s), so clippy has now genuinely completed across the whole workspace, not just the first crate, and nextest passed too. gate / gate (the evaluator) also passes.

No new check is added by this round — these are doc-comment punctuation fixes only, so no negative fixture applies.

Closes #64

forkwright added 3 commits August 15, 2026 22:06
noise/mod.rs wrote the 2-byte TS2021 initiation version with
to_le_bytes(); the reference protocol (control/controlbase/messages.go)
requires big-endian, so a conforming peer decoded a different version
number than the one hamma believed it advertised and the Noise
transcript could not authenticate.

The wire bug was a symptom of a deeper defect: the capability version
was independently restated at four call sites (noise prologue/wire
version = 1, the /key endpoint = 71, MapRequest.Version = 68,
RegisterRequest with no Version field at all) that had already drifted
apart from each other. Introduces mitos::capability::CAPABILITY_VERSION
as the one typed source every control-plane surface derives from, so a
future divergent literal is either a compile error (no bare u16 to
restate) or a failing consistency test, not a silent handshake failure.
@forkwright
forkwright force-pushed the fix/64-capability-version-be branch from a33b569 to 40ebd1d Compare August 16, 2026 03:06
@forkwright
forkwright merged commit d07ecbf into main Aug 16, 2026
10 checks passed
@forkwright
forkwright deleted the fix/64-capability-version-be branch August 16, 2026 03:29
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.

Derive every control-plane version from one truthful capability and encode TS2021 big-endian

1 participant