docs(signal-protocol): document the pluggable X25519 key agreement - #479
Conversation
Adds an "X25519 Key Agreement" subsection alongside the existing AES-256-CBC / HKDF-SHA256 primitives, reflecting whatsapp-rust PR #1218: calculate_agreement now routes through SignalCryptoProvider, and a substituted backend can refuse the operation (CurveError::AgreementFailed / SignalProtocolError::KeyAgreementFailed). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
📝 WalkthroughWalkthroughThe Signal protocol documentation now describes X25519 key agreement, ChangesSignal protocol documentation
Estimated code review effort: 1 (Trivial) | ~5 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
| Filename | Overview |
|---|---|
| advanced/signal-protocol.mdx | Documents X25519 agreement lifecycle, provider customization, and failure propagation without leaving an eligible follow-up issue. |
Reviews (2): Last reviewed commit: "docs(signal-protocol): fix agreement-per..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@advanced/signal-protocol.mdx`:
- Around line 985-987: Update the “X25519 Key Agreement” subsection heading to
sentence case, “X25519 key agreement.” Rewrite its prose in active voice
addressing the reader as “you,” replace the vague “this” reference with
`calculate_agreement`, and split the final paragraph into separate concise
sentences while preserving the technical meaning.
- Line 995: Update the paragraph describing the pluggable crypto-provider flow
to explicitly name SignalCryptoProvider::x25519_agreement as the trait hook used
by PrivateKey::calculate_agreement, while preserving the existing failure and
error-priority details.
- Line 995: Update the crypto-provider discussion in the Signal protocol section
to add a callout requiring callers to invoke set_crypto_provider before any
cryptographic operation, including key agreement. State that the custom backend
is installed only once and that implicit or duplicate initialization after the
default provider has initialized fails.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 26b54f07-6031-4617-8538-6ce3e3495b92
📒 Files selected for processing (1)
advanced/signal-protocol.mdx
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 98a8e2bc38
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Addresses review feedback on PR #479: - Correct the DH-ratchet cost model: a ratchet step performs two X25519 agreements (new receiving chain + new sending chain via DeferredSenderRatchet::apply), not one per message; ordinary chain advances (step_with_message_keys) cost none. Verified against session_cipher.rs and ratchet/keys.rs. - Name SignalCryptoProvider::x25519_agreement explicitly, and stop implying HKDF-SHA256 is provider-routed (it always calls hkdf::Hkdf directly) — only AES-256-CBC shares the hook with the agreement. Verified against ratchet.rs::derive_keys_with_label. provider (get_or_init) so a later set_crypto_provider call returns Err instead of replacing it. Verified against provider.rs. - Rewrite in active/second-person voice, sentence-case heading, and split run-on sentences. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Dismissed because a newer commit was pushed; Greptile will re-review the current head.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
0 issues found across 1 file (changes from recent commits).
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Auto-approved: Documentation-only update adding an X25519 key agreement subsection to signal-protocol.mdx; no code, API, config, or behavior changes. The diff is purely explanatory, so no product, security, or operational tradeoff remains for human sign-off.
Re-trigger cubic
2 participants
Summary
advanced/signal-protocol.mdx, alongside the existing AES-256-CBC and HKDF-SHA256 primitive write-ups.PrivateKey::calculate_agreementnow routes through the pluggableSignalCryptoProvidertrait (x25519_agreement, overridden viaset_crypto_provider) instead of calling the X25519 implementation directly.CurveError::AgreementFailedandSignalProtocolError::KeyAgreementFailed, which is treated as a local failure and outranks the MAC-based decrypt verdicts rather than being reported as message corruption.Reflects oxidezap/whatsapp-rust#1218, which added this trait method (default = today's behavior, byte-for-byte unchanged; no build without a custom provider can produce the new error).
Scope note
This repo has no existing page describing the
SignalCryptoProvidertrait itself (its extension points for AES-256-CBC/AES-256-GCM/HMAC-SHA256/transport AEAD are undocumented; onlyCryptoProviderErrorleaks into a few error-reference pages as an opaquesource()). Writing that page is a larger, separate documentation effort. This PR stays scoped to the one primitive PR #1218 actually changed, added in the same style as the primitives already documented on this page.Test plan
Generated by Claude Code
Summary by cubic
Adds an X25519 key agreement subsection to
advanced/signal-protocol.mdx, covering the new pluggable path and updated DH ratchet details.It explains how
calculate_agreementnow usesSignalCryptoProviderand clarifies failure handling and provider setup.calculate_agreementthroughSignalCryptoProvider::x25519_agreement(set viaset_crypto_provider; installs once; default path unchanged).CurveError::AgreementFailed/SignalProtocolError::KeyAgreementFailed, treated as a local failure ahead of MAC verdicts.Written for commit 746335d. Summary will update on new commits.
Summary by CodeRabbit
calculate_agreementAPI and configurable cryptographic provider behavior.