fix(mc-host): authenticate daemon version transcripts - #58
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
|
You have reached your Codex usage limits for security reviews. Please try again later. |
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Files Reviewed (6 files)
Reviewed by gemini-3.7-flash · Input: 315.9K · Output: 24.1K · Cached: 1.4M |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0be6776def
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| SERVER_PROOF_DOMAIN, | ||
| clientNonce, | ||
| server.serverNonce, | ||
| server.daemonVer, |
There was a problem hiding this comment.
Version the changed authentication transcript
When an updated client encounters an already-running daemon from the preceding release (or an older client encounters the updated daemon), both peers still advertise wire_version: 2 and use the same subc-*-v1 domains, but they now calculate different HMACs because only one side includes daemon_ver; the handshake therefore fails with proof_mismatch before compatibility policy can inspect the daemon version. Introduce a separately identifiable proof/wire version or an explicit migration path so mixed-generation upgrades remain operable.
Useful? React with 👍 / 👎.
| * `HMAC-SHA256(key, ASCII(domain) || client_nonce || server_nonce || daemon_id)` | ||
| * `HMAC-SHA256(key, ASCII(domain) || client_nonce || server_nonce || | ||
| * u32be(len(daemon_ver)) || UTF8(daemon_ver) || daemon_id)` | ||
| * per wire doc Section 5.2. Exported so tests can reproduce the committed |
There was a problem hiding this comment.
Update the normative wire protocol with the new transcript
This comment says the new construction is defined by wire-doc Section 5.2, but docs/mc-host-wire-protocol.md:196-206 still publishes the old proof arrays and HMAC(... || server_nonce || daemon_id) formula. Because that document declares itself the direct-only wire authority, an independent implementation or conformance vector built from it will reject the updated Rust and TypeScript peers; update the canonical formula and both literal proof vectors in the same change.
Useful? React with 👍 / 👎.
Summary
Daemon version metadata is now authenticated in both handshake directions. The proof transcript length-prefixes the UTF-8 version, and Rust, TypeScript, raw-client, and fake-peer vectors share the same construction.
Verification
cargo test -p mc-host auth --lib --testsbun test packages/plugin/src/shared/mc-host-client/auth.test.ts packages/plugin/src/shared/mc-host-client/client.test.tsStack
Stacked on #46.