test(sdk/rust): Signal cross-language interop verification (part 8 of #18) — completes the port#80
Conversation
|
@graycyrus is attempting to deploy a commit to the Vezures Team on Vercel. A member of the Team first needs to authorize it. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1610d8f290
ℹ️ 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".
| Error::InvalidArgument("Sender key signature verification failed".into()) | ||
| })?; | ||
|
|
||
| let message_key = self.message_key_for(message.iteration)?; |
There was a problem hiding this comment.
Bind sender-key iterations before ratcheting
In the current group envelope format (website/src/common/group-messaging.ts), senderKeyIteration is metadata outside the signed body, while the sender signs only the ciphertext. If a relay or stored envelope supplies a valid ciphertext/signature with a tampered future iteration, this call ratchets and caches/removes keys before decrypt performs the MAC check; when MAC then fails, the receiver state is already advanced, so the real message for that iteration can no longer be decrypted. Please either include the iteration in the signature/AD or compute against a copy and commit state only after decrypt succeeds.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch. Binding the iteration into the signature/AD would break wire-compatibility with the TS sender (where senderKeyIteration is likewise unsigned metadata), so I took the other route you suggested: the receiver now ratchets a scratch copy and commits state only after the AEAD MAC passes (b74b9d5). A forged/tampered iteration can no longer advance or poison the chain — added a regression test (forged_iteration_does_not_advance_receiver_state) proving the genuine message at its real iteration still decrypts after a forged one fails.
… #18) The headline deliverable of #18: prove the Rust Signal port is byte-for-byte compatible with the flagship TypeScript SDK. Adds `tests/signal_interop.rs`, which loads `tests/vectors/signal_vectors.json` — vectors produced by `gen_signal_vectors.mjs` running the REAL TS Signal modules (the same pinned set the Python port verifies against) — and checks every layer: - crypto KDFs (HKDF root/chain, message-key derivation) + HMAC reproduce TS bytes - AEAD both directions: Rust decrypts TS ciphertext AND re-encrypts to identical bytes (incl. empty plaintext) - X3DH: X25519 pubkeys match; the Rust responder reaches the exact secret the TS initiator derived - Double Ratchet: decrypts the TS message stream, reproduces TS ciphertext byte-for-byte, and handles out-of-order delivery - Sender Keys: verifies TS ed25519 signatures + decrypts TS group ciphertexts, and reproduces TS ciphertexts/signatures 9 interop tests, all green. Vectors copied verbatim from `sdk/python/tests/vectors/` (regenerate via that dir's `gen_signal_vectors.mjs`). This completes the Rust Signal E2E port (parts 1-8). Closes #18. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1610d8f to
4a92248
Compare
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds two new files to the Rust SDK test suite: a pinned JSON fixture file ( ChangesSignal Protocol Rust Cross-Language Interop Tests
Sender Key Receiver State Hardening
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
The group message iteration is metadata outside the Ed25519-signed body (to stay wire-compatible with the TS sender format), so a tampered future iteration can pass signature verification yet fail the AEAD MAC. Previously the receiver ratcheted its chain and cached/removed skipped keys *before* the MAC check, so a forged or corrupt message advanced/poisoned the chain and the genuine message at that iteration became undecryptable. Ratchet a scratch copy and commit state only after decrypt succeeds. Adds a regression test. Addresses CodeRabbit review on #80. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The headline deliverable of #18: prove the Rust Signal port is byte-for-byte compatible with the flagship TypeScript SDK.
Adds
tests/signal_interop.rs, loadingtests/vectors/signal_vectors.json— vectors produced bygen_signal_vectors.mjsrunning the real TS Signal modules (the same pinned set the Python port checks against, so a pass is genuine cross-language interop, not a tautology). Verifies every layer:9 interop tests, all green. Vectors copied verbatim from
sdk/python/tests/vectors/(regenerate via that dir'sgen_signal_vectors.mjs).Validation (
sdk/rust/)cargo fmt --check✅ ·cargo clippy --all-targets -- -D warnings✅ ·cargo test✅Closes #18
🤖 Generated with Claude Code
Summary by CodeRabbit