Force poly1305's soft backend: 424 intrinsics were reaching the binary - #271
Conversation
crates/encryption -> chacha20poly1305 -> poly1305, and poly1305 0.8 auto-selects
its AVX2 backend on any x86/x86_64 target unless told otherwise. Nothing told it
otherwise, so it was selected: 424 `_mm*` intrinsic calls under 30 `unsafe`
occurrences in one file, src/backend/avx2/helpers.rs.
That is a second unaudited SIMD surface beside ndarray::simd, in the crypto
path, and over seven times the size of the curve25519-dalek surface the same
config file already neutralizes. Unlike dalek's -- which was gated off and
therefore never in the binary -- this one was live.
Reachability, not porting. The whole backend sits behind one cfg
(src/backend.rs: `#[cfg(all(any(target_arch = "x86", target_arch = "x86_64"),
not(poly1305_force_soft)))] pub(crate) mod avx2;`), so one rustflag compiles it
out without touching a line of crypto. Same shape as the
curve25519_dalek_backend="serial" line directly above it.
This is deliberately NOT a Poly1305 implementation. The operator-ratified
doctrine in .claude/CHACHA20_MATRYOSHKA_PLAN.md is explicit: "RustCrypto owns
the algorithm; ndarray owns the SIMD... Rolling your own AEAD (HChaCha20 +
Poly1305 + framing) is the footgun; it is forbidden." The sanctioned way to
accelerate this later is the chacha20 route -- vendor the fork, give
RustCrypto's own backend an ndarray::simd lane -- never a reimplementation.
Verified, and not merely that the flag was passed:
cargo build -p encryption -v emits poly1305_force_soft
nm libpoly1305-*.rlib | grep avx2 0 symbols <- the code is GONE
nm libpoly1305-*.rlib | grep soft 6 symbols
cargo test -p encryption 46 + 13 passing, incl. the
XChaCha20-Poly1305 AEAD round-trips
Cost, accepted and NOT measured: the soft backend (poly1305-donna, the same
algorithm) instead of AVX2, on the AEAD's MAC. That is a real throughput cost
on large payloads. It is accepted because the alternative is an unaudited
intrinsic surface in the crypto path, and because the sanctioned acceleration
route stays open.
The rustflags block is x86_64-gated, so no other target is affected.
Claude-Session: https://claude.ai/code/session_01VdfbkUCBbtZhy3yjSfCDHp
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughChangesBackend configuration
Estimated code review effort: 1 (Trivial) | ~3 minutes Possibly related PRs
Suggested reviewers: 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 |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_a8fa4c6e-e388-4a14-b66e-1ca14127e427) |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7042995ecd
ℹ️ 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".
| "--cfg", | ||
| "poly1305_force_soft", |
There was a problem hiding this comment.
Preserve the soft-backend cfg when RUSTFLAGS is set
When this workspace is built with RUSTFLAGS set, Cargo uses that environment source instead of the matching target.<cfg>.rustflags array, so poly1305_force_soft is silently dropped; this occurs in every GitHub Actions job because .github/workflows/ci.yaml:23 globally sets RUSTFLAGS: "-D warnings", and likewise affects any downstream build that supplies its own compiler flags. I confirmed this precedence with Cargo 1.95, and the Cargo configuration reference specifies that only the first applicable rustflags source is used. Consequently, an x86_64 build of encryption in those environments still compiles Poly1305's AVX2 backend, defeating the purpose of this change; ensure supported RUSTFLAGS settings include this cfg or enforce the backend through a mechanism that cannot be replaced by them.
Useful? React with 👍 / 👎.
crates/encryption→chacha20poly1305→poly1305, and poly1305 0.8 auto-selects its AVX2 backend on any x86/x86_64 target unless told otherwise. Nothing told it otherwise.So it was selected: 424
_mm*intrinsic calls under 30unsafeoccurrences in a single file,src/backend/avx2/helpers.rs.That is a second unaudited SIMD surface beside
ndarray::simd, in the crypto path, and over seven times the size of thecurve25519-daleksurface this same config file already neutralizes. The difference matters: dalek's was gated off and never in the binary. This one was live.Reachability, not porting
The whole backend sits behind one cfg:
One rustflag compiles it out without touching a line of crypto — the same shape as the
curve25519_dalek_backend="serial"line directly above it, and the same lesson the dalek EPIPHANIES entry minted: "'it contains raw intrinsics' and 'raw intrinsics reach the binary' are different claims."This is deliberately not a Poly1305 implementation
.claude/CHACHA20_MATRYOSHKA_PLAN.md, operator-ratified 2026-07-11:The sanctioned way to accelerate this later is the chacha20 route — vendor the fork and give RustCrypto's own backend an
ndarray::simdlane, never a reimplementation. That path stays open; this change doesn't close it.Verified — and not merely that the flag was passed
cargo build -p encryption -vpoly1305_force_softnm libpoly1305-*.rlib | grep avx2nm libpoly1305-*.rlib | grep softcargo test -p encryptionCost — accepted, and not measured
The soft backend (poly1305-donna, the same algorithm) instead of AVX2, on the AEAD's MAC. That is a real throughput cost on large payloads and no benchmark was run. It's accepted because the alternative is an unaudited intrinsic surface in the crypto path, and because the sanctioned acceleration route remains available.
The
rustflagsblock isx86_64-gated, so no other target is affected.Generated by Claude Code
Summary by CodeRabbit