Repository navigation
Run the pairing exchange without heap allocation - #253
Merged
Merged
Conversation
CPace built a std::vector for every length-value piece, transcript and copied sid/AD on each round; the PRS, the emitted code and the QR token were heap-backed; the AEAD for the wrapped PSK allocated; and the pairing note slot lost its capacity to the main loop at every drain. CPace now takes spans, holds sid and AD in fixed arrays, and hashes the generator string, ISK transcript and MAC input piece by piece into SHA-512 (hmac_sha512_streamed), checked against the draft-21 vectors and the existing KATs. The PRS and the emitted codes are inline, one bit-stream base32 formatter serves both tokens, aead_oneshot_encrypt writes into the caller's buffer, and the note slot and the drain trade two buffers reserved at construction. A new alloc window drives the digits and QR code flows with a test-side CPace server: only the listener's std::string arguments remain. The protocol task's ESP stack bounds are unchanged.
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Pairing-note handling can retain oversized buffers after peer-driven bursts and allocate unnecessarily during reentrant shutdown, risking sustained RAM pressure and shutdown aborts on constrained devices.
8 open issues
Avoid reserving a pairing buffer when no exchange is pending · New Bound pairing-note backlog or release oversized buffers · New Document reentrant drain allocation in the inventory · New Describe the notes buffer as reusable, not empty between drains · New Test reentrant pairing drains across stop and restart · New Test reserved-vector exchange and retained-storage lifecycle · New Test caller-buffer rejection, wiping, and successful round-trip · New Add the required Control: marker to the accepting row · New
What changed in this PR
Reduces C++ heap allocations during Sendspin pairing, continuing #252’s record-store work without changing the public API.
Changes:
- Streams cryptographic inputs and stores pairing state, codes, and tokens inline.
- Writes encrypted values into caller-owned buffers and reuses pairing-note storage.
- Extends allocation and size-limit tests and updates memory documentation.
| File | Description |
|---|---|
| tests/wrap_test_helpers.h | Accepts session identifiers as spans. |
| tests/test_psk_wrap.cpp | Supplies CPace sessions to wrapping tests. |
| tests/test_pairing_token.cpp | Adapts assertions to inline token results. |
| tests/test_pairing_state_machine.cpp | Updates fixtures for spans and inline codes. |
| tests/test_pairing_code.cpp | Checks inline decimal-code results. |
| tests/test_cpace.cpp | Tests streamed encodings and storage limits. |
| tests/test_allocations.cpp | Measures digits and QR pairing allocation windows. |
| tests/lifecycle_test_fixtures.h | Exposes the handshake hash for pairing tests. |
| src/platform/crypto.h | Adds streamed HMAC and caller-buffer encryption. |
| src/inbox.h | Supports reserved storage and buffer exchange. |
| src/crypto/psk_wrap.h | Updates wrapping interfaces for spans and CPace. |
| src/crypto/psk_wrap.cpp | Encrypts directly into fixed-size output. |
| src/crypto/pairing_token.h | Defines inline token types. |
| src/crypto/pairing_token.cpp | Shares an allocation-free base32 formatter. |
| src/crypto/pairing_code.h | Defines inline codes and PRS views. |
| src/crypto/pairing_code.cpp | Formats decimal codes into inline storage. |
| src/crypto/cpace.h | Bounds retained state and defines streaming encoders. |
| src/crypto/cpace.cpp | Hashes generator, transcript, and MAC inputs incrementally. |
| src/connection.h | Holds the pairing PRS in a fixed buffer. |
| src/connection_manager_pairing.cpp | Integrates inline pairing helpers and session identifiers. |
| src/client.cpp | Reuses note buffers and preserves public string results. |
| docs/internals.md | Updates the pairing memory-allocation model. |
| CLAUDE.md | Documents expanded allocation-test coverage. |
Review effort: Max
…ntrant pairing drains
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.


Summary
After #252 the pairing exchange itself still allocated on the protocol task: CPace built a
std::vectorfor every length-value piece and transcript and copied the sid and AD on each round, the PRS and the emitted code (and the 43-character QR token) were heap-backed, the AEAD that wraps the new long-term PSK allocated, and the pairing-note slot handed its capacity to the main loop at every drain. A digits-code pairing made 68 library allocations, a QR-code pairing 71. Part of Sendspin/backlog#92.No public API change. The listener's
std::stringcallback arguments on the main loop remain, as the public callbacks require.Change
src/crypto/cpace.*) takes spans, holds the sid and AD in fixed arrays (size-checked, failing closed), and hashes the generator string, the ISK transcript and the MAC input piece by piece into SHA-512 instead of concatenating;hmac_sha512_streamed()feeds the HMAC's inner hash and wipes the key-derived buffers.build_pake_sid()returns a fixed array; the ADs are views of the constants.pairing_code_digits()and the QR/pairing tokens returnInlineTextfrom one bit-stream base32 formatter; the publicformat_pairing_token()converts at its boundary and its output is unchanged.aead_oneshot_encrypt()writes into the caller's buffer and wipes it on every failure;wrap_value()reads the sid from the CPace run.InboxSlot::exchange(), so a drain no longer takes the slot's capacity away. A drain reserves only when it takes notes, so a nested drain inside a listener'sstop()allocates only when it delivers that stop's dismissals (listed in internals.md "Memory").Tests
Allocations.CodePairingThroughItsRecordFlushplays the server side with the library's CPace for both the digits and qr_code flows: 68 -> 2 and 71 -> 3 (the listener's server-id strings and the QR token). Thepairing_pskwindow drops from 4 to 2.StartRejectsAnOversizedSidOrAdcovers the size checks.ClientLifecycle.StopFromInsideAPairingCallbackDismissesThePromptOnce:stop(), andstop()thenstart(), fromon_display_pairing_code()deliver each owed dismissal once, abandon the rest of the batch, and drain a fresh batch after the restart (mutants: no batch abandonment, no dismissals instop()).InboxSlot.ExchangeAndEditKeepTheSlotsStoragepinsexchange()andedit()keeping the buffers' storage (mutant:exchange()moving instead of swapping).EventStateis private toclient.cpp, so checking it would need a production seam; the allocation windows cover the normal drain.Stack (ESP-IDF 5.5.4, esp32), re-measured on the final tree: the protocol task is unchanged at 6,544 B -Os / 6,688 B -Og bounded and 5,920 / 6,144 reachable, under the 7,168 B default.