docs: align README Security section with AES-256-GCM-SIV/BE32#27
Conversation
Reviewer's GuideDocumentation-only PR updating the README security and streaming format sections to accurately describe the VAULTX03 AES-256-GCM-SIV/BE32 implementation, Argon2id parameters, and integrity/nonce properties. Flow diagram for VAULTX03 key derivation and nonce handlingflowchart TD
P[passphrase]
S[16-byte OsRng salt]
P -->|combine| A[Argon2id
m=64MB, t=3, p=4]
S --> A
A --> M[64-byte master_key]
M --> K[HKDF-SHA512 → 32-byte encryption_key]
M --> N[HKDF-SHA512 → 32-byte nonce_seed]
N --> R[OsRng 7-byte stream_nonce per file]
R --> E[EncryptorBE32]
E --> X[12-byte nonce
7-byte stream_nonce
+ 4-byte counter
+ 1-byte flag]
K --> C[AES-256-GCM-SIV
1 MB chunks]
X --> C
C --> T[128-bit auth tag per chunk]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughREADME documentation for Neuron Encrypt's VAULTX03 format is updated: the nonce field description is revised to a 7-byte OsRng nonce expanded to 12 bytes by EncryptorBE32; the Cipher Suite/Key Derivation section switches V3 to AES-256-GCM-SIV (BE32), adjusts Argon2id to p=4, and corrects V3/V2 salt sizes; and the Threat Model bullets are aligned to per-chunk GCM-SIV tags and the updated nonce scheme. ChangesVAULTX03 Documentation Update
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The description of integrity as
GHASH (GCM-SIV)is slightly misleading since AES-GCM-SIV uses POLYVAL rather than GHASH; consider updating the wording to reflect the actual primitive used or use a more generic "Carter–Wegman MAC" description. - The claim that a fresh 56-bit nonce per file makes collision probability negligible could be tightened by briefly quantifying the birthday-bound (e.g., safe up to ~2^28 files) or otherwise framing the security level so readers understand the operational limits of the scheme.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The description of integrity as `GHASH (GCM-SIV)` is slightly misleading since AES-GCM-SIV uses POLYVAL rather than GHASH; consider updating the wording to reflect the actual primitive used or use a more generic "Carter–Wegman MAC" description.
- The claim that a fresh 56-bit nonce per file makes collision probability negligible could be tightened by briefly quantifying the birthday-bound (e.g., safe up to ~2^28 files) or otherwise framing the security level so readers understand the operational limits of the scheme.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Up to standards ✅🟢 Issues
|
![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: 78684aad7f
ℹ️ 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".
| - **Magic**: Literal `b"VAULTX03"` — identifies the streaming format version | ||
| - **Salt**: 128-bit fresh `OsRng`-generated per file | ||
| - **Nonce**: 56-bit fresh `OsRng`-generated per file; `EncryptorBE32` appends an internal 5-byte counter/flag (12 bytes total internally) | ||
| - **Nonce**: 56-bit (7 bytes) fresh `OsRng`-generated per file; `EncryptorBE32` expands this to a 12-byte nonce internally by appending a 5-byte counter + 1-byte flag |
There was a problem hiding this comment.
For anyone implementing or verifying VAULTX03 from the README, this describes a 7-byte prefix plus a 5-byte counter plus a 1-byte flag, which totals 13 bytes even though the expanded AEAD nonce is 12 bytes. The implementation documents the internal suffix as 5 bytes total (4-byte counter + 1-byte flag) in neuron-encrypt/src/crypto.rs:53-54, so this should say 4-byte counter + 1-byte flag or 5-byte counter/flag to avoid misdocumenting the file format.
Useful? React with 👍 / 👎.
| - Ciphertext tampering: Every 1 MB chunk has an independent Poly1305 authentication tag. Corruption of any byte fails decryption of that chunk. | ||
| - Nonce reuse: Each encryption generates a fresh 24-byte random nonce via `OsRng`. With 192-bit nonces, collision probability is negligible even at billions of files. | ||
| - Ciphertext tampering: Every 1 MB chunk has an independent GCM-SIV authentication tag (GHASH). Corruption of any byte fails decryption of that chunk. | ||
| - Nonce reuse: Each encryption generates a fresh 7-byte (56-bit) random nonce via `OsRng`. `EncryptorBE32` expands it to 12 bytes internally with a per-chunk counter. Fresh nonce per file makes collision probability negligible. |
There was a problem hiding this comment.
For high-volume or automated use, this new 56-bit nonce statement overstates the collision margin: with only 7 random bytes and no uniqueness tracking in encrypt_file, birthday probability is already about 6.7% at 100 million encryptions and effectively certain around 1 billion. The old 192-bit claim made that wording true, but with VAULTX03 this should either quantify the risk or rely on AES-GCM-SIV's nonce-misuse resistance instead of saying random per-file collisions are negligible.
Useful? React with 👍 / 👎.
Code Review Roast 🔥Verdict: 1 Issue Found | Recommendation: Address before merge Overview
Issue Details (click to expand)
🏆 Best part: Finally got the V3 nonce size right at 7 bytes (56-bit) instead of the 24-byte XChaCha nonce. I was half-expecting to find a 192-bit nonce still lingering around like a bad smell. 💀 Worst part: The nonce math doesn't add up. You claim BE32 appends "a 5-byte counter + 1-byte flag" which totals 6 bytes, but the code comment (crypto.rs:53) and the actual BE32 spec both confirm it's 4-byte counter + 1-byte flag = 5 bytes. Seven bytes plus five bytes equals twelve, not thirteen. Did the Counter class teach you nothing about addition? 📊 Overall: Like a math test with one wrong answer — technically proficient but leaves me questioning your arithmetic homework. Files Reviewed (1 files)
Reviewed by laguna-m.1-20260312:free · 702,817 tokens |
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| - **Magic**: Literal `b"VAULTX03"` — identifies the streaming format version | ||
| - **Salt**: 128-bit fresh `OsRng`-generated per file | ||
| - **Nonce**: 56-bit fresh `OsRng`-generated per file; `EncryptorBE32` appends an internal 5-byte counter/flag (12 bytes total internally) | ||
| - **Nonce**: 56-bit (7 bytes) fresh `OsRng`-generated per file; `EncryptorBE32` expands this to a 12-byte nonce internally by appending a 5-byte counter + 1-byte flag |
There was a problem hiding this comment.
🔥 The Roast: Quick math check: 7 bytes + 5 bytes + 1 byte = 13 bytes, not 12. Your calculator needs recalibration. The BE32 construction splits the 12-byte nonce as 7-byte stream_nonce + 4-byte counter + 1-byte flag, not 5-byte counter.
🩹 The Fix: Change '5-byte counter + 1-byte flag' to '4-byte counter + 1-byte flag' to accurately reflect the BE32 nonce expansion.
📏 Severity: suggestion
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
1 participant
Summary by Sourcery
Update security documentation to reflect the AES-256-GCM-SIV BE32 streaming design and current key-derivation parameters.
Documentation:
Summary by CodeRabbit