Conversation
📝 WalkthroughWalkthroughThe pull request upgrades the core CryptoBase encryption module from PBKDF2-SHA256 + AES-GCM to Argon2id + AES-256-GCM for standard data protection, and introduces four new advanced cryptographic APIs: a two-layer cascade mode, a sealed-box mode for asymmetric encryption, and a post-quantum hybrid mode combining Curve25519 and ML-KEM. Documentation and examples are updated accordingly. ChangesCryptographic Primitives Upgrade & New Hybrid Encryption Modes
Sequence DiagramssequenceDiagram
participant Caller
participant ProtectDataCascade
participant Argon2id
participant AES as AES-256-GCM
participant XChaCha as XChaCha20-Poly1305
Caller->>ProtectDataCascade: plaintext, password
ProtectDataCascade->>ProtectDataCascade: Generate 32-byte salt
ProtectDataCascade->>Argon2id: Derive 64-byte key
Argon2id->>ProtectDataCascade: master_key (split into 32+32)
ProtectDataCascade->>AES: Encrypt plaintext (inner layer)
AES->>ProtectDataCascade: aes_nonce + aes_ciphertext
ProtectDataCascade->>XChaCha: Encrypt inner payload (outer layer)
XChaCha->>ProtectDataCascade: xnonce + outer_ciphertext
ProtectDataCascade->>Caller: 0x02 + salt + xnonce + outer_payload
sequenceDiagram
participant Caller
participant ProtectDataQuantumHybrid
participant Curve25519
participant MLKEM as ML-KEM
participant BLAKE3
participant XChaCha as XChaCha20-Poly1305
Caller->>ProtectDataQuantumHybrid: plaintext, recipientCurvePub, recipientKemPub
ProtectDataQuantumHybrid->>Curve25519: Generate ephemeral keypair
ProtectDataQuantumHybrid->>Curve25519: ECDH shared secret
Curve25519->>ProtectDataQuantumHybrid: classic_secret
ProtectDataQuantumHybrid->>MLKEM: Encapsulate with recipientKemPub
MLKEM->>ProtectDataQuantumHybrid: pq_secret, kem_ciphertext
ProtectDataQuantumHybrid->>BLAKE3: Hash classic_secret + pq_secret
BLAKE3->>ProtectDataQuantumHybrid: hybrid_key
ProtectDataQuantumHybrid->>XChaCha: Encrypt plaintext
XChaCha->>ProtectDataQuantumHybrid: nonce + ciphertext_with_tag
ProtectDataQuantumHybrid->>Caller: {ciphertext, ephemeralPublicKey, kemCiphertext}
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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. Review rate limit: 0/1 reviews remaining, refill in 60 minutes.Comment |
|
Seems you are using me but didn't get OPENAI_API_KEY seted in Variables/Secrets for this repo. you could follow readme for more information |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cryptobase.psm1`:
- Around line 182-184: The code is calling [Curve25519]::DeriveSharedSecret
(used by the sealed-box and hybrid APIs) but the underlying implementation
builds an ECDiffieHellman on NIST P-256, so either replace the current
DeriveSharedSecret implementation with a true X25519/X448 key-exchange (and
ensure HKDF is applied to the X25519 shared secret) or rename and document the
public APIs (sealed-box, hybrid, and [Curve25519]::DeriveSharedSecret) to
indicate they use P-256/NIST instead of Curve25519; update all callers that
derive symmetricKey (the calls that pass "CryptoBase_SealedBox_v1" / HKDF) to
match the chosen curve implementation and adjust tests/docs accordingly.
- Around line 94-95: The code is calling [Argon2id]::Hash which currently
aliases to Rfc2898DeriveBytes in Private/PasswordHashing.psm1 (lines around the
Hash implementation), so ProtectData, UnprotectData and the cascade APIs are not
getting a real Argon2id KDF; replace the PBKDF2-backed implementation with a
true Argon2id backend (or wire a native/nuget Argon2id implementation) and
update all call sites that use [Argon2id]::Hash (seen in cryptobase.psm1 where
$key is assigned, plus the other occurrences you noted around lines 115-116,
155-156, and 240) so they invoke the real Argon2id routine and preserve the same
parameter semantics (salt, memory, iterations, parallelism, output length)
expected by ProtectData/UnprotectData and the cascade APIs.
🪄 Autofix (Beta)
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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9f2e03fa-5a25-4090-90bd-0d3062fb7a1e
📒 Files selected for processing (3)
README.mdcryptobase.psm1docs/CryptoBase.md
| $key = [Argon2id]::Hash($passBytes, $salt, 65536, 3, 4, 32) | ||
| $ciphertext = [byte[]]::new($plaintext.Length) |
There was a problem hiding this comment.
The new password KDF path is still PBKDF2-backed.
[Argon2id]::Hash(...) currently resolves to Rfc2898DeriveBytes in Private/PasswordHashing.psm1:56-61, so ProtectData, UnprotectData, and the cascade APIs do not get the advertised memory-hard properties. That makes the implementation and the updated docs/README materially misleading until a real Argon2id backend is wired in.
Also applies to: 115-116, 155-156, 240-240
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cryptobase.psm1` around lines 94 - 95, The code is calling [Argon2id]::Hash
which currently aliases to Rfc2898DeriveBytes in Private/PasswordHashing.psm1
(lines around the Hash implementation), so ProtectData, UnprotectData and the
cascade APIs are not getting a real Argon2id KDF; replace the PBKDF2-backed
implementation with a true Argon2id backend (or wire a native/nuget Argon2id
implementation) and update all call sites that use [Argon2id]::Hash (seen in
cryptobase.psm1 where $key is assigned, plus the other occurrences you noted
around lines 115-116, 155-156, and 240) so they invoke the real Argon2id routine
and preserve the same parameter semantics (salt, memory, iterations,
parallelism, output length) expected by ProtectData/UnprotectData and the
cascade APIs.
| $sharedSecret = [Curve25519]::DeriveSharedSecret($senderPrivateKey, $recipientPublicKey) | ||
| $info = [Encoding]::UTF8.GetBytes("CryptoBase_SealedBox_v1") | ||
| $symmetricKey = [HKDF]::DeriveKey($sharedSecret, $null, $info, 32) |
There was a problem hiding this comment.
These new flows are using NIST P-256, not Curve25519/X25519.
Private/Curve25519.psm1:7-24 builds ECDiffieHellman on nistP256, so the sealed-box and hybrid APIs are mislabeled and won't interoperate with real Curve25519 key material. Please either switch the implementation to actual X25519 or rename/document these APIs as P-256.
Also applies to: 196-197
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cryptobase.psm1` around lines 182 - 184, The code is calling
[Curve25519]::DeriveSharedSecret (used by the sealed-box and hybrid APIs) but
the underlying implementation builds an ECDiffieHellman on NIST P-256, so either
replace the current DeriveSharedSecret implementation with a true X25519/X448
key-exchange (and ensure HKDF is applied to the X25519 shared secret) or rename
and document the public APIs (sealed-box, hybrid, and
[Curve25519]::DeriveSharedSecret) to indicate they use P-256/NIST instead of
Curve25519; update all callers that derive symmetricKey (the calls that pass
"CryptoBase_SealedBox_v1" / HKDF) to match the chosen curve implementation and
adjust tests/docs accordingly.
1 participant
Motivation
ProtectData/UnprotectDatato a modern memory-hard primitive and provide higher-assurance, expert-grade encryption helpers.Description
CryptoBase::ProtectData/UnprotectDatawithArgon2idusing parameters(memory=65536, iterations=3, parallelism=4, keyLen=32).ProtectDataCascadeandUnprotectDataCascadewhich derive a 64-byte master key withArgon2id, split it into two independent 32-byte keys, encrypt the inner layer with AES-256-GCM, and outer-wrap it withXChaCha20-Poly1305; cascade payload layout is0x02 || salt(32) || xnonce(24) || outerPayload.CreateSealedBoxwhich performs Curve25519 ECDH, expands the shared secret withHKDF-SHA256, and encrypts the payload withXChaCha20-Poly1305returningnonce||ciphertext.ProtectDataQuantumHybridwhich generates an ephemeral Curve25519 keypair, does classical ECDH plusMLKemencapsulation, mixes the two secrets withBLAKE3, and encrypts withXChaCha20-Poly1305, returning a hashtable withCiphertext,EphemeralCurvePub, andKemCiphertext.README.mdanddocs/CryptoBase.mdwith examples and notes describing Argon2id, the cascade API, the sealed-box API, and the PQ hybrid API.Testing
CryptoBasevia PowerShell withpwsh -NoLogo -NoProfile -Command "Import-Module ./cryptobase.psd1 -Force; [CryptoBase] | Out-Null; 'ok'", but this automated import failed in the environment becausepwshis not installed (pwsh: command not found).Codex Task
Summary by CodeRabbit
New Features
Documentation