Repository navigation
feat(cipher): AES-CBC with PKCS#7 padding on every backend - #381
Conversation
Quest-Claim: 63d1a48f035f68c1
Adds `cipher::AesCbcEncryptingKey` and `AesCbcDecryptingKey`, which encrypt and decrypt AES-CBC with PKCS#7 padding, as WebCrypto and Dart use it, or whole blocks without padding, as pyca's `modes.CBC` does. Decryption checks the padding in constant time and clears the buffer if it is malformed; the docs steer callers to a MAC over the ciphertext or an AEAD. Backends: aws-lc's AES-NI `aes_hw_cbc_encrypt` (eight-block decryption), vpaes on x86_64 and aarch64, aesv8-armx on the armv7 crypto extension, a port of aes_nohw.c's CBC on SSE2, aarch64 AES intrinsics decrypting eight blocks at a time, and the portable armv7 cipher block by block. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Your OpenCode Go account is out of credit. Top it up, then re-trigger Pullfrog. OpenCode Go's response
|
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 44 minutes. View limit details
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. |
kixelated
left a comment
There was a problem hiding this comment.
Automated review by review (OpenAI)
Reviewed head: ae394b9
No actionable defects found in this static review. Checked the public API and PKCS#7 validation/clearing, in-place CBC chaining and batch tails across x86_64/AArch64/armv7, AES-NI decryption-key layout, assembly wrappers/imports, and the vector, differential, fuzz, and zeroing test additions.
Direction: the separate encryption/decryption keys and separate padded/unpadded methods fit the stated consumers. Keep the verify-MAC-before-decrypt and unpredictable-IV guidance prominent. The new tests cover the important padding and batch-boundary cases.
Verification limits: GitHub-only inspection; I did not execute builds, tests, ctgrind, fuzzing, proofs, or benchmarks, so the PR's reported local results were not independently reproduced. At review time, regeneration passed, the proof workflow was still running, and Queue builds were intentionally skipped on the PR head. This comment is not an approval or a claim that runtime/constant-time validation is complete.

Finishes quest/m2/adoption/webcrypto/aes-cbc.md.
Problem
WebCrypto's AES-CBC (llrt, Deno, Servo), pyca's
modes.CBC(pyca-compatible pyrind), and Dart'sAesCbcneed AES-CBC, which rind lacked, so those adopters would fill the gap with a second library.Approach
rind::cipher::AesCbcEncryptingKeyandAesCbcDecryptingKey, one key schedule each:encrypt(&iv, in_out)pads with PKCS#7 and encrypts in place, appending 1 to 16 bytes. It cannot fail.decrypt(&iv, in_out)decrypts and returns the plaintext. It checks the padding in constant time, and on any malformed padding returnsError::DecryptFailedand clears the buffer. A ciphertext that is empty or not whole blocks also fails withDecryptFailed, leaving the buffer untouched.encrypt_blocksanddecrypt_blocksdo whole blocks with no padding, for pyca'smodes.CBCand aws-lc-rs'sEncryptingKey::cbc.Docs steer away from bare CBC. The
ciphermodule and both types say that decrypting before verifying a MAC is a padding oracle. The types point to encrypt-then-MAC (asA128CBC-HS256does) oraeadinstead. The encrypting key also says each message needs a fresh random IV fromrand::fill.Backends, with serial encryption and parallel decryption:
aes_hw_cbc_encrypt(eight-block decryption), imported byadmin/import-aws-lcvpaes_cbc_encryptaes_nohw.c's CBC, ported into the existingaes_nohw.rs(decrypts 8 blocks per bitsliced batch)aese/aesmcchain on the critical path, as aesv8-armx does, and decryption runs 8 blocks at a time, as rind's CTR doesvpaes_cbc_encryptandvpaes_cbc_decrypt(2 blocks at a time)aes_hw_cbc_encrypt(3 blocks at a time)The x86_64 AES-NI decryption key now uses aws-lc's
AES_KEYlayout (HwDecKeyholds anAesKey128/AesKey256of inverted round keys), so the CBC routine can read it. This does not changehw_decrypt_blockor AES-KW.No existing aws-lc import changed: rerunning
admin/import-aws-lconly added the new files and theaesv8-armx.sroutine.Decisions
Decided by the maintainer on 2026-11-05; each is implemented (✅ marks the choice).
PaddedBlockEncryptingKey/PaddedBlockDecryptingKeydo ✅ / oneAesCbckey holding both schedules, or the raw key expanded per call asAesKekdoes.encrypt_blocks/decrypt_blocks, for pyca'smodes.CBC(the planned pyca-compatible pyrind) and aws-lc-rs'sEncryptingKey::cbc✅ / PKCS#7 only / aPaddingargument tonew.AesCtrdo ✅ / a random-IVencryptlike aws-lc-rs's.aeadopen andAesKek::unwrapdo ✅ / leave the decrypted bytes, as aws-lc-rs does.Debugthat shows the algorithm only, never key bytes:AesCbcEncryptingKey { algorithm: Algorithm { key_len: 32 }, .. }✅. feat!: audit rind's public API, and apply its decisions, before 0.2.0 #383 has not landed, so its other naming and placement picks are not applied here; the CBC names fall under none of its items.Impact
rind::cipher::{AesCbcEncryptingKey, AesCbcDecryptingKey}, withnew,encrypt,encrypt_blocks,decrypt, anddecrypt_blocks, and aDebugthat shows only the algorithm. Each key stores its key length for thatDebug, wiped on drop with the key.MIGRATING.md: aws-lc-rs'sPaddedBlockEncryptingKey::cbc_pkcs7,PaddedBlockDecryptingKey::cbc_pkcs7, andEncryptingKey::cbc/DecryptingKey::cbcnow map to rind. The webrtc note no longer lists AES-CBC as missing.aes_hw_cbc_encrypt.aes_cbc: a differential test against aws-lc-rs on tampered ciphertexts.Validation
Unit tests (
mid::aes_cbc):Per-backend
paths_matchtests (x86_64, aarch64) compare every path the CPU offers against a single-block model, at 0 to 23 blocks.Wycheproof
aes_cbc_pkcs5_test.json: all 144 AES-128/256 tests pass, including everyBadPaddingandNoPaddingcase. The 72 AES-192 tests are skipped until AES-192.crosschecks::cipher::aes_cbc_pkcs7andaes_cbc_unpaddedare proptests against aws-lc-rs. They cover messages up to 1100 bytes and ciphertexts with chosen last bytes, so rind and aws-lc-rs must accept or refuse the same padding.rind/tests/zeroing.rscovers both key types.debug_shows_only_the_algorithmchecks theDebugoutput.ctgrind (
admin/ctgrind unit --release mid::aes_cbc): 0 errors on AES-NI, and on the vpaes and SSE2 paths with the CIno-aesandno-ssse3denials. To check the harness, I added a branch on the padding byte, and memcheck flagged it.ct-qemu (
admin/ct-qemu ct_qemu_aes_cbc): armv7 crypto-extension encryption and decryption, with identical traces for both seeds.armv7 under qemu, crypto extension and portable paths: the CBC unit, Wycheproof, and crosscheck tests pass, and clippy is clean.
aarch64 under qemu-aarch64 (an ad hoc cross shell), AES instructions and vpaes (
RIND_CPU_DISABLE_aes): the same tests pass.After merging
mainat f0b7931:just checkpasses (607 tests, 9 skipped). On the first head,just cialso passed.I could not run the fuzz target here (no cargo-fuzz). It compiles and passes clippy; CI's fuzz workflow builds and runs it.
nix develop .#prove --command admin/provepasses on the merged head (2455 Lean jobs).mid::aes::expand_key, which the x86_64 AES-GCM proof extracts, is unchanged on this branch: an early generic version of it was reverted before the first push.Bench, local i7-8700 (Coffee Lake, AES-NI),
cipherbench, padded, median:Both run the same aws-lc assembly on this CPU, so the bulk numbers match; rind leads on small messages, where aws-lc-rs's EVP setup dominates. The bench workflow's
--bench cipherrows cover aarch64 and the other tiers.Review
Codex
gpt-6.1-solat medium effort, throughquest review --base origin/main, round 1, on headae394b93559035092b88e33569cb5c5b4882828b: no findings.Alternatives
AesDecKey::decrypt_blockper block on every backend. Rejected: the per-call overhead made SSE2 re-bitslice the round keys for every block, and on in-order cores it loses the parallelism decryption allows.Follow-ups
None.
(Written by Claude Opus 5.5)
🤖 Generated with Claude Code