Use single pass ChaCha20 setup and session key schedule in sodium backend

[bdraco]

Summary

Adds a fast path for the esphome libsodium port (esphome-libs/libsodium#32) in the sodium ChaChaPoly backend, feature detected via sodium/sodium_esphome.h; the current implementation stays as the fallback for stock libsodium.

The main consumer of this path today is log message streaming over the encrypted API, small messages where the per message fixed cost dominates, which is what these changes target. OTA over noise is planned eventually; that shifts the focus to bulk throughput, where larger noise frames amortize the fixed cost and a hand tuned chacha loop remains as a follow up.

The changes:

1. The cipher state holds a session persistent ChaCha20 key schedule (crypto_stream_chacha20_ietf_session_state) loaded once in init_key, instead of running a full key setup twice per message; per message only the nonce and counter words are rewritten.
2. Encrypt generates the Poly1305 key block and encrypts the payload in a single cipher pass; decrypt derives the key the same way, then continues the keystream from counter 1 after the MAC check.
3. The whole MAC transcript (ad, padding, ciphertext, padding, lengths) is one crypto_onetimeauth_poly1305_aead_mac call instead of five dispatched init/update/final calls.

Benchmarks

ESP32 (Xtensa LX6, 240 MHz, ESP-IDF), noise_cipherstate_encrypt with the same pattern as APINoiseFrameHelper::write_protobuf_messages, together with the libsodium patches:

Size	Before	After
50 B	37.4 µs/op	27.4 µs/op (27% faster)
100 B	48.8 µs/op	38.9 µs/op (20% faster)
1000 B	234.3 µs/op	222.2 µs/op (5% faster)

ESP8266 (Xtensa LX106, 80 MHz, Arduino), same benchmark:

Size	Before	After
50 B	190.4 µs/op	160.2 µs/op (16% faster)
100 B	267.6 µs/op	240.8 µs/op (10% faster)
1000 B	1672.6 µs/op	1650.4 µs/op (1% faster)

Memory

Measured on the same builds: ESP32 flash grows 124 bytes and static RAM is unchanged; ESP8266 flash shrinks 956 bytes and static RAM shrinks 52 bytes (the fast path uses fewer libsodium entry points, so more code is dead stripped). Each cipher state grows 32 bytes (key schedule instead of raw key), 64 bytes per connection. Peak stack per crypto call drops from about 880 to about 528 bytes because the per operation scratch, the stacked cipher context and two wrapper frames are gone.

Verification

The RFC 7539 A.5 ChaChaPoly vector (tests/unit/test-cipherstate.c, nonzero nonce, nonzero ad, byte exact ciphertext and tag, decrypt round trip and MAC failure paths) passes on a host build linked against the patched libsodium port with the fast path confirmed in the binary; to reproduce, add_subdirectory both repos with the port's patches applied (the port exports the sodium CMake target that this repo's CMakeLists links) and build the cipherstate test against noise_c. An encrypted API session (handshake plus bidirectional traffic) also works on ESP32 and ESP8266; the MAC transcript is unchanged, so it interoperates with existing peers; building against stock libsodium keeps the previous implementation via the capability guard, and NOISE_DISABLE_SODIUM_FAST_PATH forces it.

Depends on a libsodium release containing esphome-libs/libsodium#32; the dependency pin bump can follow the usual flow once that ships.

[esphbot]

Previous review — superseded by a newer review below.

[esphbot]

Tip

No blocking issues found — ready to merge.

[bdraco]

Addressed the review: the gate is now versioned (SODIUM_ESPHOME_NOISE_FAST_PATH is an integer the port bumps on any semantics change, consumed as >= 1) and NOISE_DISABLE_SODIUM_FAST_PATH forces the fallback. On test reachability, the RFC 7539 A.5 cipherstate vector now runs on a host build linked against the patched port with the fast path confirmed in the binary, and it passes; the reproduction recipe is in the PR description. On the decrypt asymmetry, the counter continuation is a documented contract of the session API (block0_xor always advances past block 0) and the versioned gate is the guard against cross repo drift; an explicit _ic variant would add API surface for one call site, happy to add it if preferred. One correction to the review: the memory note compares against the struct before #20; on current main the cipher state is parent plus a 32 byte key, so holding the 64 byte key schedule grows it by 32 bytes as the description says.

[Copilot]

Pull request overview

This PR adds a capability-gated fast path in the libsodium ChaCha20-Poly1305 backend for the ESPHome libsodium port, using session-persistent ChaCha20 setup and consolidated MAC computation to reduce per-message fixed overhead while keeping the existing implementation as a fallback for stock libsodium.

Changes:

• Detect ESPHome libsodium support via sodium/sodium_esphome.h and enable a SODIUM_ESPHOME_NOISE_FAST_PATH-gated implementation (with an opt-out macro).
• Cache a session-persistent ChaCha20 key schedule in the cipher state, rewriting only nonce/counter per message.
• Use one-pass cipher processing and a single crypto_onetimeauth_poly1305_aead_mac call to cover the full transcript.

Suppressed comments (1)

src/backend/sodium/cipher-chachapoly.c:94

• In the fast-path decrypt, block contains the derived Poly1305 key and is not wiped on either the success or MAC-failure path. This increases the lifetime of key material on the stack and differs from the fallback implementation’s explicit scratch wiping. Consider wiping block (and mac) before returning, including on MAC failure.

    uint8_t block[64] __attribute__((aligned(4)));
    uint8_t mac[16];

    crypto_stream_chacha20_ietf_session_block0_xor
        (&(st->chacha_st), block, NULL, NULL, 0, state->n);

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Suppressed comments (2)

src/backend/sodium/cipher-chachapoly.c:96

• block is only wiped for the first 32 bytes. The remaining bytes still hold ChaCha20 block-0 keystream derived from the session key, and unlike the fallback path they remain on the stack after return. Wipe the entire block; while here, consider also clearing the computed mac on the success path for consistent cleanup.

    crypto_stream_chacha20_ietf_session_block0_xor
        (&(st->chacha_st), block, NULL, NULL, 0, state->n);
    crypto_onetimeauth_poly1305_aead_mac(mac, ad, ad_len, data, len, block);
    sodium_memzero(block, 32);
    if (!noise_is_equal(mac, data + len, 16)) {

src/backend/sodium/cipher-chachapoly.c:81

• Only the first 32 bytes of block are wiped, but the remaining 32 bytes still contain ChaCha20 block-0 keystream derived from the session key. Consider wiping the full 64-byte block to avoid leaving key-dependent material on the stack (the fallback path wipes the full scratch struct).

This issue also appears on line 92 of the same file.

    /* Poly1305 key generation and payload encryption share one cipher
       setup; the key in block is cleared as soon as the MAC is done */
    crypto_stream_chacha20_ietf_session_block0_xor
        (&(st->chacha_st), block, data, data, len, state->n);
    crypto_onetimeauth_poly1305_aead_mac
        (data + len, ad, ad_len, data, len, block);
    sodium_memzero(block, 32);
    return NOISE_ERROR_NONE;

[esphbot]

@bdraco: Addressed the review: the gate is now versioned (SODIUM_ESPHOME_NOISE_FAST_PATH is an integer the po...

Versioned gate and opt-out confirmed in the diff. Both prior blockers closed.

Your memory correction is right; my note was wrong. Verified origin/main: struct is parent + chacha_k[32] after #20. 64-byte schedule replacing 32-byte key is +32. Same for sodium_memzero — main already uses it with the ~47% rationale. Both retracted.

Decrypt asymmetry: recorded as a note, not a request. No _ic needed. The versioned gate covers drift.

Also checked noise_cipherstate_rekey (cipherstate.c:574) — encrypt then init_key on the same object. session_init overwriting in place matches the fallback's memcpy. No issue.

One small thing: fast path drops the fallback's if (ad_len) guard. noise_cipherstate_encrypt() passes ad = NULL. Every transport message. Works today. Worth one line of contract docs.

[esphbot]

Previous review — superseded by a newer review below.

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Suppressed comments (2)

src/backend/sodium/cipher-chachapoly.c:95

• crypto_stream_chacha20_ietf_session_block0_xor() and crypto_onetimeauth_poly1305_aead_mac() are invoked with NULL pointers (data/ad) when the corresponding lengths are 0. The Noise API allows ad == NULL when ad_len == 0, but C APIs commonly assume pointers are non-NULL even for zero-length inputs. Consider always passing non-NULL pointers to avoid undefined behavior if the esphome libsodium implementation doesn’t special-case NULL.

    (NoiseCipherState *state, const uint8_t *ad, size_t ad_len,
     uint8_t *data, size_t len)
{
    NoiseChaChaPolyState *st = (NoiseChaChaPolyState *)state;

src/backend/sodium/cipher-chachapoly.c:80

• In the fast path, crypto_onetimeauth_poly1305_aead_mac() is called unconditionally with ad, but the Noise API allows ad == NULL when ad_len == 0 (see src/protocol/cipherstate.c:304-305). To preserve that contract and avoid relying on libsodium accepting NULL pointers for zero-length inputs, pass a non-NULL dummy pointer when ad_len is 0.

This issue also appears on line 92 of the same file.

     uint8_t *data, size_t len)
{
    NoiseChaChaPolyState *st = (NoiseChaChaPolyState *)state;
    uint8_t block[64] __attribute__((aligned(4)));

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Suppressed comments (1)

src/backend/sodium/cipher-chachapoly.c:101

• crypto_stream_chacha20_ietf_session_block0_xor() is called with NULL input/output pointers (len=0). Unless that API explicitly documents accepting NULL pointers, this is fragile and can crash if the implementation validates pointers even when len is zero. Passing non-NULL pointers keeps the behavior identical while avoiding potential UB.

    crypto_stream_chacha20_ietf_session_block0_xor
        (&(st->chacha_st), block, NULL, NULL, 0, state->n);
    crypto_onetimeauth_poly1305_aead_mac(mac, ad, ad_len, data, len, block);

[esphbot]

Previous review — superseded by a newer review below.

[esphbot]

Warning

Important issues found.

• Versioned gate uses >= 1, so a semantics-changing port bump does NOT downgrade to the fallback

[bdraco]

Both taken: the gate is now an exact match (SODIUM_ESPHOME_NOISE_FAST_PATH == 1) so a port version bump really does downgrade this file to the fallback until it is updated deliberately, and the comment says so; the block0_xor NULL in and out with len 0 contract is now documented in the fast path header comment alongside the ad rule.

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

[esphbot]

@bdraco: Both taken: the gate is now an exact match (SODIUM_ESPHOME_NOISE_FAST_PATH == 1) so a port version b...

Confirmed at 57ff3ac. Gate reads == 1; line 38. Comment at 55-58 now matches. block0_xor NULL in/out with len 0 documented at 46-48, next to the ad rule. Both findings closed.

One nit left, non-blocking: == 1 downgrades silently. An additive port bump costs the fast path with no build signal. A #pragma message on defined-but-unmatched would surface it.

[esphbot]

PR Review — Use single pass ChaCha20 setup and session key schedule in sodium backend

Both prior findings are resolved at 57ff3ac. Merge-ready; one non-blocking suggestion.

Verified against the PR head, not just the diff:

• The gate now matches its own comment. SODIUM_ESPHOME_NOISE_FAST_PATH == 1 (line 38) is a closed bound, so a port that ships 2 with changed session-entry-point semantics compiles the fallback rather than the fast path. The comment at lines 55-58 was rewritten to describe exactly that ("the exact match above downgrades this file to the fallback until it is updated for the new semantics on purpose"), so code and prose finally agree — this was the one blocking item.

• The block0_xor NULL contract is now stated. Lines 46-48 document that NULL in/out with len == 0 derives only the key block and leaves the counter at 1 "for the decrypt payload pass" — which is precisely what makes the implicit counter position at line 91 correct. That was the reader trap in decrypt; it is now spelled out next to the ad rule.

• Everything previously confirmed still holds. Re-checked at the new head: sodium_memzero(block, 32) before the MAC branch on both paths, the failure-path tag wipe (cipherstate.c advances n only after a successful decrypt, so a leaked tag would authenticate a replay under a live nonce), the shared noise_chachapoly_new touching only parent fields, and noise_new's calloc giving the crypto_stream_chacha20_ietf_session_state member max alignment with size stamped for noise_free.

• Scope is still exactly one file. The follow-up commit is 8 insertions / 5 deletions, all in the gate and its comment. No stray pins or unrelated edits.

• 🟢 The == 1 gate now fails silently on a version bump: an additive port bump drops every device back to the fallback with no build signal, costing back the 10–27% this PR gained. A #pragma message on defined-but-unmatched (excluding the explicit opt-out) makes that visible without breaking -Werror.

Not re-raised: Copilot's wipe-all-64 and __attribute__((aligned(4))) points remain dismissed per @bdraco — bytes 32..63 are unused counter-0 keystream that encrypt nothing, and the attribute is unreachable without the port's GCC/Clang toolchain.

---

✅ Resolved since last review (1)

Previously-flagged issues verified fixed

• src/backend/sodium/cipher-chachapoly.c:37 Versioned gate uses >= 1, so a semantics-changing port bump does NOT downgrade to the fallback

---

🟢 Suggestions

1. Exact-match gate now fails silently on a port version bump — consider a visible diagnostic
src/backend/sodium/cipher-chachapoly.c:37-39

The == 1 change is the right call and closes the correctness hole from the previous round: a port that bumps SODIUM_ESPHOME_NOISE_FAST_PATH to 2 with different session-entry-point semantics now compiles the fallback instead of miscomputing keystream positions or the MAC transcript.

The residual, much smaller cost is that the downgrade is invisible. If the port bumps the version for a purely additive reason, every ESPHome device silently loses the fast path — the build still succeeds, the wire format is still correct, and the only symptom is the 10–27% per-message regression this PR just bought back. That is exactly the kind of regression nobody notices until someone re-benchmarks months later.

A one-line diagnostic makes the drop loud without changing behaviour:

#if defined(SODIUM_ESPHOME_NOISE_FAST_PATH) && \
    SODIUM_ESPHOME_NOISE_FAST_PATH != 1 && \
    !defined(NOISE_DISABLE_SODIUM_FAST_PATH)
#  pragma message("noise-c: sodium fast path disabled - unrecognised SODIUM_ESPHOME_NOISE_FAST_PATH version")
#endif

#pragma message (rather than #warning) keeps -Werror builds green, and the condition excludes the explicit NOISE_DISABLE_SODIUM_FAST_PATH opt-out so an intentional fallback stays quiet. Non-blocking — the safety property you wanted is already there.

#if defined(SODIUM_ESPHOME_NOISE_FAST_PATH) && \
    SODIUM_ESPHOME_NOISE_FAST_PATH == 1 && \
    !defined(NOISE_DISABLE_SODIUM_FAST_PATH)

---

Checklist

• Secret material wiped on all paths (including MAC failure)
• Key material zeroed on state teardown (noise_free covers the larger struct)
• Constant-time MAC comparison preserved
• Nonce not advanced on MAC failure; computed tag not left in memory
• External dependency guarded with a versioned contract and an opt-out — suggestion ＃1
• Borrowed API boundary cases documented in-file (ad NULL, block0_xor NULL/len 0)
• Keystream position correct per message; no counter/keystream reuse
• MAC transcript unchanged (wire compatible with existing peers)
• Backward compatible — stock libsodium builds byte-for-byte unaffected
• Allocation alignment and size stamping correct for the new struct member
• Fast path exercisable off target; recipe in-tree
• Diff matches PR description; no scope creep
• Buffer bounds respected (MAC space, NOISE_MAX_PAYLOAD_LEN)
• No hardcoded secrets

---

Automated review by Kōan (Claude) HEAD=57ff3ac 1 min 59s

[esphbot]

Tip

No blocking issues found — ready to merge.

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.