[kahrendt]

Summary

A pairing message's plaintext could still reach the heap unwiped. The session's message buffer (scratch_) and the reassembly buffer freed their old block unwiped when they grew. A session that ended with a pairing fragment pending kept it in scratch_. On the host's AES-GCM backend, which decrypts in place before the tag check, a client/pair-finalize whose tag failed left its long_term_psk in scratch_.

Change

• secure_reserve() (in crypto/crypto.h, next to secure_zero_storage()) reserves the capacity asked for by hand and wipes the block it leaves, as far as its size or the bytes written, whichever is further. Reassembler::accept() calls it only when a fragment outgrows the buffer, at least doubling it within the cap as insert() did. The session clears scratch_ before reserving, so a growth copies nothing.
• Session::finish() wipes scratch_ and the reassembly buffer, which covers every end: a decrypt failure, a failed fragment sequence (so the session's separate wipes there are gone), a host close. ~Session() wipes scratch_, for a connection dropped without an end; the reassembler already wipes in its destructor.
• docs/conventions.md "Pairing secrets the same way" describes the wipes.

Tests

• SessionTest.WipesAPairingMessageWhoseTagFails: a pair-finalize with one tag bit flipped ends DECRYPT_FAILED with no PSK in scratch_. It can fail only under AES-GCM, since ChaChaPoly checks the tag before it decrypts.
• SessionTest.WipesAPendingPairingFragmentAtTheEnd: with a pair-finalize's first fragment pending, a host close and a message that fails to decrypt each leave the PSK in neither buffer. Control: the scan finds it before the end.
• tests/freed_blocks.h: FreedBlockScan counts the blocks the test thread frees still holding a secret, through the sanitizer's free hook. A test that uses it skips itself in a build without ASan or TSan.
• SecureReserve.WipesTheBlockItLeaves: the scan shows that secure_reserve() frees its old block wiped (past the size as far as written, and to the size when nothing was counted as written), keeps the contents, reserves exactly what it is asked for, and moves nothing when the room is already there. Control: a vector grown by its own insert() frees the block with the secret still in it.
• SessionTest.GrowsItsBuffersWithoutFreeingAPairingSecret: a pairing fragment followed by a longer one grows both buffers, and neither frees a block holding the PSK.
• SessionTest.WipesAPendingPairingFragmentWhenDestroyed: a session destroyed with a pairing fragment pending and no end frees neither buffer holding the PSK.
• Known gap: GCC installs no header for the hook, so tests/freed_blocks.h declares the two functions itself there. That was compiled locally but not run; CI's Linux ASan and TSan jobs are its first run.

Closes #77

[Copilot]

🟡 Changes recommended

The test-only allocator parameters violate the repository’s explicit prohibition on production test seams.

1 open finding

• Avoid production allocator template parameters added solely for tests · New

What changed in this PR

Protects pairing plaintext in session buffers during growth and cleanup, addressing #77.

Changes:

• Adds secure buffer growth that wipes replaced storage.
• Wipes plaintext buffers on session termination and destruction.
• Adds regression tests and updates secret-handling documentation.

File	Description
tests/​test_session.cpp	Tests cleanup after authentication failure and pending fragments.
tests/​test_crypto.cpp	Tests wiping storage during buffer growth.
src/​session.h	Declares destructor and disables copying and moving.
src/​session.cpp	Adds secure growth and lifecycle wipes.
src/​fragmentation.cpp	Wipes replaced reassembly storage.
src/​crypto/​crypto.h	Adds allocator-aware secure reservation.
docs/​conventions.md	Documents expanded plaintext-wiping requirements.

🧠 Review effort: Balanced

---

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

[Copilot]

🔵 Needs a closer look

Security-sensitive memory cleanup needs human validation, particularly the GCC sanitizer-hook execution noted as unverified.

0 open findings

1 resolved since last review

• Avoid production allocator template parameters added solely for tests

🧠 Review effort: Balanced