Skip to content

feat(cipher): AES-CBC with PKCS#7 padding on every backend - #381

Merged
kixelated merged 4 commits into
mainfrom
quest/m2/adoption/webcrypto/aes-cbc
Nov 5, 2026
Merged

kixelated merged 4 commits into
mainfrom
quest/m2/adoption/webcrypto/aes-cbc

Conversation

@kixelated

@kixelated kixelated commented Nov 5, 2026 •

Copy link
Copy Markdown
Collaborator

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's AesCbc need AES-CBC, which rind lacked, so those adopters would fill the gap with a second library.

Approach

  • rind::cipher::AesCbcEncryptingKey and AesCbcDecryptingKey, 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 returns Error::DecryptFailed and clears the buffer. A ciphertext that is empty or not whole blocks also fails with DecryptFailed, leaving the buffer untouched.
    • encrypt_blocks and decrypt_blocks do whole blocks with no padding, for pyca's modes.CBC and aws-lc-rs's EncryptingKey::cbc.
  • Docs steer away from bare CBC. The cipher module and both types say that decrypting before verifying a MAC is a padding oracle. The types point to encrypt-then-MAC (as A128CBC-HS256 does) or aead instead. The encrypting key also says each message needs a fresh random IV from rand::fill.

  • Backends, with serial encryption and parallel decryption:

    Path Source
    x86_64 AES-NI aws-lc's aes_hw_cbc_encrypt (eight-block decryption), imported by admin/import-aws-lc
    x86_64 SSSE3 aws-lc's vpaes_cbc_encrypt
    x86_64 SSE2 aes_nohw.c's CBC, ported into the existing aes_nohw.rs (decrypts 8 blocks per bitsliced batch)
    aarch64 AES rind intrinsics: encryption keeps only the aese/aesmc chain on the critical path, as aesv8-armx does, and decryption runs 8 blocks at a time, as rind's CTR does
    aarch64 NEON aws-lc's vpaes_cbc_encrypt and vpaes_cbc_decrypt (2 blocks at a time)
    armv7 crypto ext aws-lc's aesv8-armx aes_hw_cbc_encrypt (3 blocks at a time)
    armv7 portable the existing libcrux-based cipher, one block at a time, since it bitslices a single block
  • The x86_64 AES-NI decryption key now uses aws-lc's AES_KEY layout (HwDecKey holds an AesKey128/AesKey256 of inverted round keys), so the CBC routine can read it. This does not change hw_decrypt_block or AES-KW.

  • No existing aws-lc import changed: rerunning admin/import-aws-lc only added the new files and the aesv8-armx.s routine.

Decisions

Decided by the maintainer on 2026-11-05; each is implemented (✅ marks the choice).

  1. Separate encrypting and decrypting keys, each expanding only its own schedule, as aws-lc-rs's PaddedBlockEncryptingKey/PaddedBlockDecryptingKey do ✅ / one AesCbc key holding both schedules, or the raw key expanded per call as AesKek does.
  2. Unpadded CBC included as encrypt_blocks/decrypt_blocks, for pyca's modes.CBC (the planned pyca-compatible pyrind) and aws-lc-rs's EncryptingKey::cbc ✅ / PKCS#7 only / a Padding argument to new.
  3. The caller supplies the IV, as WebCrypto, pyca, Dart, and rind's AesCtr do ✅ / a random-IV encrypt like aws-lc-rs's.
  4. Malformed padding clears the buffer, as rind's aead open and AesKek::unwrap do ✅ / leave the decrypted bytes, as aws-lc-rs does.
  5. Following the API audit (moq-dev/rind#383, D5), both key types have a Debug that 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

  • New public API, additive only, so it ships in a patch release: rind::cipher::{AesCbcEncryptingKey, AesCbcDecryptingKey}, with new, encrypt, encrypt_blocks, decrypt, and decrypt_blocks, and a Debug that shows only the algorithm. Each key stores its key length for that Debug, wiped on drop with the key.
  • MIGRATING.md: aws-lc-rs's PaddedBlockEncryptingKey::cbc_pkcs7, PaddedBlockDecryptingKey::cbc_pkcs7, and EncryptingKey::cbc/DecryptingKey::cbc now map to rind. The webrtc note no longer lists AES-CBC as missing.
  • README's algorithm tables gain AES-CBC rows; LICENSE names aes_hw_cbc_encrypt.
  • New fuzz target aes_cbc: a differential test against aws-lc-rs on tampered ciphertexts.
  • No wire impact.

Validation

  • Unit tests (mid::aes_cbc):

    • NIST SP 800-38A F.2.1 to F.2.6, padded and unpadded.
    • Every length from 0 to 150.
    • Every well-formed padding length from 1 to 16. Every single-byte corruption of the padding, and last bytes 0, 17, 0x20, 0x80, and 0xff, each refused with the buffer cleared.
    • Wrong key lengths.
  • Per-backend paths_match tests (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 every BadPadding and NoPadding case. The 72 AES-192 tests are skipped until AES-192.

  • crosschecks::cipher::aes_cbc_pkcs7 and aes_cbc_unpadded are 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.rs covers both key types. debug_shows_only_the_algorithm checks the Debug output.

  • ctgrind (admin/ctgrind unit --release mid::aes_cbc): 0 errors on AES-NI, and on the vpaes and SSE2 paths with the CI no-aes and no-ssse3 denials. 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 main at f0b7931: just check passes (607 tests, 9 skipped). On the first head, just ci also 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/prove passes 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), cipher bench, padded, median:

    aws-lc-rs rind
    aes128-cbc-encrypt 32B 49.0 ns 22.7 ns
    aes128-cbc-encrypt 1350B 836 ns 824 ns
    aes128-cbc-encrypt 16KB 9.99 µs 9.97 µs
    aes128-cbc-decrypt 32B 33.0 ns 26.3 ns
    aes128-cbc-decrypt 1350B 220 ns 223 ns
    aes128-cbc-decrypt 16KB 2.55 µs 2.54 µs
    aes256-cbc-encrypt 16KB 14.1 µs 14.2 µs
    aes256-cbc-decrypt 16KB 3.59 µs 3.60 µs

    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 cipher rows cover aarch64 and the other tiers.

Review

Codex gpt-6.1-sol at medium effort, through quest review --base origin/main, round 1, on head ae394b93559035092b88e33569cb5c5b4882828b: no findings.

Alternatives

  • Rust intrinsics for x86_64 AES-NI CBC, as aarch64 has. Rejected: aws-lc's routine already decrypts eight blocks at a time and its import machinery exists, and the quest plan names aws-lc's assembly where it exists.
  • Importing aesv8-armx's CBC for aarch64. Rejected: it decrypts only 3 blocks at a time, and rind already prefers its own 8-block intrinsics for aarch64 CTR.
  • Wiring the CBC decrypt through AesDecKey::decrypt_block per 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

kixelated and others added 2 commits November 5, 2026 12:59
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>
@kixelated kixelated changed the title AES-CBC feat(cipher): AES-CBC with PKCS#7 padding on every backend Nov 5, 2026
@kixelated
kixelated marked this pull request as ready for review November 5, 2026 21:57
@pullfrog

pullfrog Bot commented Nov 5, 2026 •

Copy link
Copy Markdown

Your OpenCode Go account is out of credit. Top it up, then re-trigger Pullfrog.

OpenCode Go's response
provider error: Upstream request failed: Insufficient account funds

Top up OpenCode Go →

Pullfrog  | Rerun failed job ➔ | View workflow run | via Pullfrog | Using glm-5.3

@coderabbitai

coderabbitai Bot commented Nov 5, 2026

Copy link
Copy Markdown

Warning

Review limit reached

You'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.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

Run configuration

  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: aa020b2a-bc3d-4661-bef8-4ddf7d299792
Commits

Reviewing files that changed from the base of the PR and between f0b7931 and ae394b9.

Files selected for processing: 37

  • LICENSE
  • MIGRATING.md
  • README.md
  • admin/import-aws-lc
  • fuzz/Cargo.toml
  • fuzz/fuzzer/aes_cbc.rs
  • quest/m2/adoption/webcrypto/README.md
  • quest/m2/adoption/webcrypto/aes-cbc.md
  • rind-bench/benches/cipher.rs
  • rind/src/cipher.rs
  • rind/src/lib.rs
  • rind/src/low/aarch64/aes_cbc.rs
  • rind/src/low/aarch64/aws_lc/vpaes_cbc_encrypt.apple.s
  • rind/src/low/aarch64/aws_lc/vpaes_cbc_encrypt.linux.s
  • rind/src/low/aarch64/aws_lc/vpaes_decrypt_2x.apple.s
  • rind/src/low/aarch64/aws_lc/vpaes_decrypt_2x.linux.s
  • rind/src/low/aarch64/mod.rs
  • rind/src/low/aarch64/vpaes_armv8.rs
  • rind/src/low/arm/aes_cbc.rs
  • rind/src/low/arm/aesv8_armx.rs
  • rind/src/low/arm/aws_lc/aesv8-armx.s
  • rind/src/low/arm/mod.rs
  • rind/src/low/mod.rs
  • rind/src/low/x86_64/aes.rs
  • rind/src/low/x86_64/aes_cbc.rs
  • rind/src/low/x86_64/aes_nohw.rs
  • rind/src/low/x86_64/aws_lc/aes_hw_cbc_encrypt.apple.s
  • rind/src/low/x86_64/aws_lc/aes_hw_cbc_encrypt.linux.s
  • rind/src/low/x86_64/aws_lc/vpaes_cbc_encrypt.apple.s
  • rind/src/low/x86_64/aws_lc/vpaes_cbc_encrypt.linux.s
  • rind/src/low/x86_64/mod.rs
  • rind/src/low/x86_64/vpaes_x86_64.rs
  • rind/src/mid/aes_cbc.rs
  • rind/src/mid/mod.rs
  • rind/tests/crosschecks.rs
  • rind/tests/wycheproof.rs
  • rind/tests/zeroing.rs
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

Share

@kixelated
kixelated added this pull request to the merge queue Nov 5, 2026

@kixelated kixelated left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Merged via the queue into main with commit e69a6a1 Nov 5, 2026
9 of 10 checks passed
@moq-bot moq-bot Bot mentioned this pull request Nov 5, 2026
This was referenced Nov 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant