Merge TASK-068: connection_state hardening — secure_zero arena clear (CWE-14/CWE-226)

Author: etr

configure.ac

@@ -114,6 +114,24 @@ AC_CHECK_HEADER([signal.h],[],[AC_MSG_ERROR("signal.h not found")])
 
 AC_CHECK_HEADER([gnutls/gnutls.h],[have_gnutls="yes"],[AC_MSG_WARN("gnutls/gnutls.h not found. TLS will be disabled"); have_gnutls="no"])
 
+# TASK-068: portable secure-zero primitive selection. Probe for
+# explicit_bzero (glibc / musl / FreeBSD / OpenBSD / *BSD). The header
+# httpserver/detail/secure_zero.hpp dispatches at compile time based on
+# whether this is available. Windows takes the RtlSecureZeroMemory
+# branch; macOS and other platforms without explicit_bzero take the
+# portable volatile + asm-clobber fallback (which has the same security
+# guarantee without the __STDC_WANT_LIB_EXT1__ include-order coupling
+# that memset_s would impose on a transitively-included header).
+#
+# The HAVE_EXPLICIT_BZERO macro is pushed into AM_CXXFLAGS / AM_CFLAGS
+# (matching the established libhttpserver pattern for HAVE_GNUTLS /
+# HAVE_BAUTH / etc.) so it reaches the header without requiring an
+# explicit `#include "config.h"` from secure_zero.hpp.
+AC_CHECK_DECLS([explicit_bzero], [], [], [[#include <string.h>]])
+AS_IF([test "x$ac_cv_have_decl_explicit_bzero" = "xyes"],
+    [AM_CXXFLAGS="$AM_CXXFLAGS -DHAVE_EXPLICIT_BZERO"
+     AM_CFLAGS="$AM_CFLAGS -DHAVE_EXPLICIT_BZERO"])
+
 # Checks for libmicrohttpd
 if test x"$host" = x"$build"; then
     AC_CHECK_HEADER([microhttpd.h],

specs/tasks/M7-v2-cleanup/TASK-068.md

@@ -10,10 +10,12 @@ Two acknowledged residual gaps at `connection_state.hpp:122, 130`:
 2. CWE-14 — the current clear path uses `memset(…, 0, …)`, which an optimizer may dead-code-eliminate. Replace with `explicit_bzero` (or a portable equivalent) so the zeroing is observable.
 
 **Action Items:**
-- [ ] In `connection_state::reset()` (or whatever clears the arena between requests), zero the entire used-bytes prefix unconditionally. Document the trade-off (cycles vs. CWE-226 mitigation) in the header.
-- [ ] Replace the `memset` call with `explicit_bzero` on glibc/musl/macOS, `SecureZeroMemory` on Windows, or a hand-rolled `volatile`-pointer loop where neither is available. Centralize the helper in `src/httpserver/detail/secure_zero.hpp`.
-- [ ] Update the inline comments at lines 122 and 130 to reference the fix.
-- [ ] Add a unit test (compile-time + runtime) that pins the helper is not dead-code-eliminated under `-O2` (using a memory barrier and a `volatile` sink to observe the write).
+- [x] In `connection_state::reset_arena()`, zero the entire `initial_buffer_` unconditionally via the new `httpserver::detail::secure_zero` helper. The trade-off (a few thousand cycles per keep-alive request for a non-elidable byte-wise clear vs. the CWE-14 + CWE-226 mitigation) is documented in the rewritten comment block at lines 106-148.
+- [x] Replace the `memset` call with the `secure_zero` helper centralized in `src/httpserver/detail/secure_zero.hpp`. The helper dispatches at compile time to `explicit_bzero` (glibc/musl/BSD), `RtlSecureZeroMemory` (Windows), or a portable `volatile`-pointer loop + `asm __volatile__("" ::: "memory")` clobber fallback (macOS + any lane without `explicit_bzero`). Note: `memset_s` was evaluated and skipped -- its `__STDC_WANT_LIB_EXT1__` include-order requirement is incompatible with a transitively-included internal header, so macOS takes the portable fallback (same security guarantee without the preprocessor-order coupling).
+- [x] Updated the inline comments at the formerly-line-122 / formerly-line-130 sites (now lines 106-148 of `connection_state.hpp`) to reference the `secure_zero` helper and the CWE-14 / CWE-226 posture. Also updated the cross-reference comment at `src/detail/webserver_callbacks.cpp:124`.
+- [x] Added `test/unit/secure_zero_dce_test.cpp` -- compiled at `-O2 -DNDEBUG` (per-target `CXXFLAGS` override), prefills a 256-byte buffer with 0xA5, calls `secure_zero`, then reads every byte through a `volatile unsigned char sink` and asserts each is zero. Also pinned `secure_zero(nullptr, 0)` and `secure_zero(p, 0)` no-op contracts.
+- [x] Added `test/unit/connection_state_sentinel_test.cpp` -- prefills the full 8 KiB arena with 0xDE, calls `reset_arena()`, and asserts every byte is zero (CWE-226 unit pin). A companion test confirms that a sentinel pattern written into a 512-byte arena allocation is scrubbed after `reset_arena()` + same-size re-allocation.
+- [x] Added `test/integ/connection_state_body_residue_test.cpp` -- the acceptance-criterion integ test. Two `GET /peek` requests over a single curl-keep-alive connection; the first carries a `DEADBEEFCRED_SENTINEL_USERNAME_FROM_REQUEST_1` basic-auth username (decoded into the arena-backed `http_request_impl::username` pmr::string), the second carries no Authorization header. The handler peeks `connection_state::initial_buffer_` via a new `HTTPSERVER_COMPILATION`-gated `http_request::underlying_connection_for_testing()` accessor. Sanity: request 1's handler observes the sentinel; headline: request 2's handler does not. Belt-and-braces: a server-wide `connection_opened` hook fires exactly once across both requests.
 
 **Dependencies:**
 - Blocked by: None
@@ -29,4 +31,4 @@ Two acknowledged residual gaps at `connection_state.hpp:122, 130`:
 **Related Requirements:** PRD §2 security NFR (CWE-226, CWE-14)
 **Related Decisions:** None new
 
-**Status:** Backlog
+**Status:** Completed

specs/tasks/M7-v2-cleanup/_index.md

@@ -32,7 +32,7 @@ TASK-093).
 | TASK-065 | RFC 5952 IPv6 zero-compression in `peer_address` | HIGH | S | Done |
 | TASK-066 | Runtime setter for hook alias slots | MED | M | Backlog |
 | TASK-067 | Remove v1 `registered_resources*` maps and `namespace compat` shim | MED | L | Done |
-| TASK-068 | `connection_state` hardening — CWE-226 / CWE-14 | MED | S | Backlog |
+| TASK-068 | `connection_state` hardening — CWE-226 / CWE-14 | MED | S | Done |
 | TASK-069 | Remove transitional two-arg `http_request_impl` constructor | MED | S | Backlog |
 | TASK-070 | Migrate `hook_table_` to `std::atomic<std::shared_ptr<T>>` | MED | M | Backlog |
 | TASK-071 | Wire `install_not_found_alias_` stub and remove dead `lambda_handler` arm | MED | S | Backlog |