Files
keyrelay/tests/test_conn.cpp
T
ducoterra d629a0ff51 phase: 03_session_takeover
**Phase 03 `session_takeover` — final verification pass. All green.**

All four task files were already in `.agents/phases/complete/03_session_takeover/` with per-task reports, so this was the phase-level close, not new implementation.

**Completion criteria**
1. *2nd controller authenticates + takes the bridge; a 3rd is refused* — met host-side: `test_claim` 102 (whitelist matrix, `acceptsUpgrade` one-claim-at-a-time, both deadline clocks across `millis()` rollover, 64-byte cap through the real codec), `test_relayctl` 308 (`--take-over` = `T`,`X`,`H`; default refusal names the flag and sends no `X`), `test_page` 142 (claim→button→one click→`T` then `X` only). Refusal mechanism stated precisely: a second claim gets `503`/`E3`, a third *socket* gets the slot refusal (`slotrefused`).
2. *Handover releases then adopts, lands disarmed* — pinned: `closeLink("taken over")` before adoption (inverting it fails exactly 1 check, reverted); no `arm`/`setArmed`/`armed = true` in the `X` path (inserting one fails exactly 1 check, reverted); un-authenticated `X` admitted in `claim.h` fails `test_claim`+`test_relayctl` (reverted). Also verified by reading: `serveClaim` never writes `_lastInboundMs`/`_controllerSeen`, so a parked claim cannot hold the board armed past 8 s.
3. *Incumbent page says taken over, disarmed, no auto-reclaim* — `onE12()` + `test_page` (disarms, releases mouse, ends typing run, pill `taken over`, reconnect loop off).
4. *Three source pins fail on inversion* — re-demonstrated in this pass, all reverted (`src/claim.h`, `src/control_server.cpp` byte-identical to pre-inversion).
5. *Gate green + README/AGENTS say takeover cannot arm* — yes.

**Validation** — `bash .agents/validate.sh` → **exit 0**: 12 suites **5026 checks, 0 failures** (normal and `SANITIZE=1`), `bridge`/`console`/`rollback` all SUCCESS, page bytes 59 286, README/AGENTS claims OK, hygiene OK. Extra lint: `ruff --select F,E9` clean, `bash -n` clean, `py_compile` clean. Coverage on new/changed logic: `claim.h` 100 % (11 lines), `status.h` 100 % (29), `proto.h` 94.8 % (229), relayctl take-over lines 14/15 (the miss is a `def` continuation trace never records).

**Defects fixed (docs only — no code defect found)**: bug 6 claimed a 2nd `/ws` upgrade "is still refused"; Measured-behaviour samples lacked `taken=`/`busy`/`tkov`; footprint + TLS-cost rows quoted phase-02 numbers while claiming to be what the gate prints (now 99 744/1 257 933, 67 352/1 178 173, 93 568/1 157 609 → TLS = +6 176 B RAM/+100 324 B flash); page-delivery/handshake rows dated; `AGENTS.md` pure-module list was missing `claim.h`.

**Hardware**: no device run. The board still runs pre-phase-03 firmware, reflash/arming mutates a live device (the target is this machine), and both controllers need the operator's passphrase — guessing feeds the shared 5-fail lockout. The 3 blank rows (third-controller refusal, chord labels, nothing-outlives-the-link) are now recorded `skipped (no device run)` with the exact closing commands in `.agents/HW_VERIFICATION.md`.

**Tree left for the harness**: index untouched (0 staged), no history changes; `todo/03_session_takeover/00_phase.md` moved to `complete/` (matches the phase-02 rename pattern); untracked files the commit must include: `.agents/validate.sh`, `src/claim.h`, `tests/test_claim.cpp`, `.agents/phases/complete/03_session_takeover/`, `.agents/reports/03_session_takeover/`. Next phase: `todo/04_controller_page_responsive/`.
2026-11-09 08:57:35 -05:00

368 lines
18 KiB
C++

/**
* tests/test_conn.cpp -- the pure half of src/conn.h, walked on the host. No Arduino, no
* WiFiClient, no socket: exactly like tls_pem.h and the policy half of tls_session.h, the
* header keeps its deciding logic free of library includes so the identical code runs here
* under -fsanitize=address,undefined. `kr::Conn` itself hides behind ESP_PLATFORM and is
* proven the other way (both firmware environments link, plus the device rows in
* .agents/HW_VERIFICATION.md: page byte-exact over TLS, :80 redirecting, plaintext /ws
* refused with 426).
*
* What each check pins, and the failure it is defending:
*
* renderHttpResponseHead -- one header formatter for every route, port and transport.
* snprintf truncation used to become `headLen = 0`, which became "the
* board answered nothing". Here: the exact bytes, the Content-Length
* agreeing with the body, an extra header landing in the right place,
* and the widest legal response PROVABLY fitting kHttpResponseHeadCap.
* queryCarriesToken -- the :80 rule "a URL carrying the passphrase is never redirected".
* The boundary matters: a check that matched the substring would turn
* /status?mytoken=x into a 426, and one that missed `&token=` would
* redirect the real thing and leak the passphrase a second time.
* plaintextRoute -- the whole :80 answer table, including that the -D KR_TLS=0
* rollback answers everything (the rollback must stay buildable and
* must stay today's server).
* redirectAuthority -- an attacker-controlled `Host:` becoming a `Location:` a browser
* navigates to. `http://keyrelay.local@evil.com/` is the shape that
* makes this a real open redirect, not a theoretical one.
* renderRedirectTarget -- the request path is client text too, and a CR in it would
* inject headers into the board's own response.
* FlashSend -- the page's send cursor. An off-by-one here is a page that arrives
* a byte short and parses as garbage; a next() that ignores the budget
* is one loop() iteration spending an unbounded time writing a socket.
*
* Two defects in this new code were found by the SANITISED run of this file, not by its
* assertions, and both are pinned by the case that triggered them:
* - queryCarriesToken() memcmp()ed its 6-byte key against the tail of any shorter query,
* reading past the end of `"a=1"` (global-buffer-overflow);
* - dottedQuadPrivate() checked the octet count when a dot arrived instead of before each
* digit, so `"192.168.1.1.1"` wrote octets[4] (stack-buffer-overflow).
* Neither changes an answer on a healthy input, and neither is visible on the board until
* it isn't. That is the argument for `SANITIZE=1` being part of the gate rather than an
* option (AGENTS.md: the sanitiser run found five of the ten bugs the README documents).
*/
#include <string.h>
#include "test.h"
#include "conn.h"
using kr::FlashSend;
using kr::kConnTxCap;
using kr::kConnViewTxCap;
using kr::kConnWriteChunk;
using kr::kHttpResponseHeadCap;
using kr::kPageBytesPerTick;
using kr::kPageChunkBytes;
using kr::kStatusJsonCap;
using kr::plaintextRoute;
using kr::PlainRoute;
using kr::queryCarriesToken;
using kr::redirectAuthority;
using kr::renderHttpResponseHead;
using kr::renderRedirectTarget;
namespace {
// The served page's length, in bytes. Cross-checked against the real raw strings by
// tests/test_relayctl.py ("the page the transport sends"). The page carries one
// 19-byte #if KR_TLS build marker and both spellings are the same length, so this
// number is build-independent -- a fact test_relayctl pins, not a hope.
constexpr size_t kServedPageBytes = 59286;
/** The pump's shape, without a socket: hand out chunks, accept what a peer takes. */
struct DrainLog {
size_t pieces = 0;
size_t bytes = 0;
size_t maxPiece = 0;
};
} // namespace
int main() {
char head[kHttpResponseHeadCap];
// ------------------------------------------------------ the response head
CASE("head: the exact bytes of a plain 200");
const size_t n = renderHttpResponseHead(head, sizeof(head), "200 OK",
"text/plain; charset=utf-8", 5);
CHECK_STR(head,
"HTTP/1.1 200 OK\r\n"
"Content-Type: text/plain; charset=utf-8\r\n"
"Content-Length: 5\r\n"
"Cache-Control: no-store\r\n"
"X-Content-Type-Options: nosniff\r\n"
"Connection: close\r\n\r\n");
CHECK_EQ(n, strlen(head));
CHECK(n > 0 && n < kHttpResponseHeadCap);
CASE("head: the extra header sits before Connection");
CHECK(renderHttpResponseHead(head, sizeof(head), "301 Moved Permanently",
"text/html; charset=utf-8", 0,
"Location: https://keyrelay.local/\r\n") > 0);
CHECK(strstr(head, "Location: https://keyrelay.local/\r\nConnection: close") != nullptr);
CASE("head: 426 carries the Upgrade field RFC 4266 asks for");
CHECK(renderHttpResponseHead(head, sizeof(head), "426 Upgrade Required",
"text/plain; charset=utf-8", 40,
"Upgrade: TLS/1.2, HTTP/1.1\r\n") > 0);
CHECK(strstr(head, "Upgrade: TLS/1.2, HTTP/1.1\r\n") != nullptr);
CHECK(strstr(head, "Content-Length: 40\r\n") != nullptr);
CASE("head: too small a buffer is refused, never truncated");
char tiny[16] = "AAAAAAAAAAAAAAA";
CHECK_EQ(renderHttpResponseHead(tiny, sizeof(tiny), "200 OK", "text/plain", 1), 0);
CHECK_EQ(tiny[0], 'A'); // nothing written at all
CHECK_EQ(renderHttpResponseHead(nullptr, 64, "200 OK", "text/plain", 1), 0);
// The formatter's contract is the SHIPPED cap, so a caller passing anything else gets a
// refusal rather than a head built to a size nobody sized.
char almost[kHttpResponseHeadCap];
CHECK_EQ(renderHttpResponseHead(almost, kHttpResponseHeadCap - 1, "200 OK", "text/plain",
1), 0);
CASE("head: the widest legal response fits, with margin to spare");
// The widest authority redirectAuthority() will actually accept is a dotted quad
// (kCertCommonName is longer than any IP, and nothing else passes the name test); the
// widest path it carries over is /index.html. So this is not a hypothetical worst case,
// it is reachable -- and it must fit anyway, with room left for the next header.
char authority[80];
CHECK(redirectAuthority("192.168.123.234", authority, sizeof(authority)));
char loc[128];
const size_t locLen = renderRedirectTarget("192.168.123.234", "/index.html", loc,
sizeof(loc));
CHECK(locLen > 0);
char extra[160];
const int extraLen = snprintf(extra, sizeof(extra), "Location: %s\r\n", loc);
CHECK(extraLen > 0);
const size_t wide = renderHttpResponseHead(head, sizeof(head), "301 Moved Permanently",
"text/plain; charset=utf-8", 128, extra);
CHECK(wide > 0);
CHECK(wide < kHttpResponseHeadCap);
CHECK(kHttpResponseHeadCap - wide >= 32); // room for one more header, not slack
// -------------------------------------------------- the token-in-URL test
CASE("query: token= is found where a key can start");
CHECK(queryCarriesToken("token=secret"));
CHECK(queryCarriesToken("a=1&token=secret"));
CHECK(queryCarriesToken("a=1&b=2&token=x"));
CHECK(queryCarriesToken("token=")); // an empty token is still a token carried
CASE("query: look-alikes are not token-carrying");
CHECK(!queryCarriesToken(""));
CHECK(!queryCarriesToken(nullptr));
CHECK(!queryCarriesToken("mytoken=secret")); // not a key boundary
CHECK(!queryCarriesToken("a=mytoken"));
CHECK(!queryCarriesToken("status?token")); // no '='
CHECK(!queryCarriesToken("xtoken=secret"));
CHECK(!queryCarriesToken("a=1token=secret")); // '=' is not a key boundary
CASE("query: every short prefix is scanned without reading past the string");
// ASan caught the first version doing exactly this: comparing a 6-byte key against the
// tail of "a=1" reads four bytes that are not there. Nothing about the answer changes;
// what changes is whether the board walks off the end of a buffer on a router's URL.
static const char *const kShort[] = {"t", "to", "tok", "toke", "token", "=", "&", "a=1",
"a=1&", "a=1&b", "x&token=", "&&token=x"};
for (const char *const shortq : kShort) {
const bool want = std::strcmp(shortq, "x&token=") == 0 ||
std::strcmp(shortq, "&&token=x") == 0;
CHECK_EQ(queryCarriesToken(shortq) ? 1 : 0, want ? 1 : 0);
}
CASE("route: the :80 table, in one place, for both builds");
// tlsBuild == false is the -D KR_TLS=0 rollback: every shape is served, exactly as
// before phase 02. That is the rollback criterion, and it is a row of this table.
CHECK(plaintextRoute(false, "token=x", false) == PlainRoute::Serve);
CHECK(plaintextRoute(true, nullptr, false) == PlainRoute::Serve);
CHECK(plaintextRoute(false, nullptr, false) == PlainRoute::Serve);
// tlsBuild == true: nothing on :80 is served; an upgrade and a token both get 426; the
// rest get the redirect. 426 before redirect, for the token, always.
CHECK(plaintextRoute(false, nullptr, true) == PlainRoute::Redirect);
CHECK(plaintextRoute(false, "a=1", true) == PlainRoute::Redirect);
CHECK(plaintextRoute(false, "token=secret", true) == PlainRoute::UpgradeRequired);
CHECK(plaintextRoute(true, nullptr, true) == PlainRoute::UpgradeRequired);
// A /ws upgrade carrying a token is still an upgrade: the transport answer wins, and it
// is the more fundamental one ("this port does not speak that at all").
CHECK(plaintextRoute(true, "token=secret", true) == PlainRoute::UpgradeRequired);
// -------------------------------------------------- the redirect authority
CASE("authority: the names this board is reached at survive");
char auth[80];
CHECK(redirectAuthority("keyrelay.local", auth, sizeof(auth)));
CHECK_STR(auth, "keyrelay.local");
CHECK(redirectAuthority("KEYRELAY.LOCAL", auth, sizeof(auth)));
CHECK_STR(auth, "keyrelay.local");
CHECK(redirectAuthority("keyrelay", auth, sizeof(auth)));
CHECK_STR(auth, "keyrelay");
// "IP if the name fails" is in the README; a redirect that threw the IP away would
// break the documented fallback path.
CHECK(redirectAuthority("192.168.4.1", auth, sizeof(auth)));
CHECK_STR(auth, "192.168.4.1");
CHECK(redirectAuthority("10.2.0.22:80", auth, sizeof(auth)));
CHECK_STR(auth, "10.2.0.22"); // :port stripped, the scheme's own is implied
CHECK(redirectAuthority("172.20.1.9", auth, sizeof(auth)));
CHECK_STR(auth, "172.20.1.9");
CASE("authority: everything else falls back to the certificate's name");
// Each of these would otherwise put attacker-chosen text into a Location a browser
// follows. The @ one is the shape of a real open-redirect URL.
static const char *const kRejected[] = {
"evil.com", "keyrelay.local@evil.com", "keyrelay.local.evil.com", "evil.local",
"1.2.3.4", "8.8.8.8:443", "keyrela", "keyrelay.too", "-bad", "bad-", ".local", "",
"a b", "a\re", "::1", "192.168.1", "192.168.1.1.1", "300.1.1.1", "172.32.0.1",
"host:80:90",
};
for (const char *const bad : kRejected) {
CHECK(redirectAuthority(bad, auth, sizeof(auth)));
CHECK_STR(auth, kr::kCertCommonName);
}
CHECK(redirectAuthority(nullptr, auth, sizeof(auth)));
CHECK_STR(auth, kr::kCertCommonName);
char longish[300];
memset(longish, 'a', sizeof(longish) - 1);
longish[sizeof(longish) - 1] = '\0';
CHECK(redirectAuthority(longish, auth, sizeof(auth)));
CHECK_STR(auth, kr::kCertCommonName); // truncated, then refused: never echoed
CHECK(!redirectAuthority("keyrelay.local", auth, 4)); // cannot even hold the fallback
CASE("redirect target: only the two content paths travel");
CHECK(renderRedirectTarget("keyrelay.local", "/", loc, sizeof(loc)) > 0);
CHECK_STR(loc, "https://keyrelay.local/");
CHECK(renderRedirectTarget("keyrelay.local", "/index.html", loc, sizeof(loc)) > 0);
CHECK_STR(loc, "https://keyrelay.local/index.html");
CHECK(renderRedirectTarget("keyrelay.local", "/nope", loc, sizeof(loc)) > 0);
CHECK_STR(loc, "https://keyrelay.local/"); // 404 awaits on the encrypted port
// Header injection: the request target is client bytes, and Location is a header.
CHECK(renderRedirectTarget("keyrelay.local", "/a\r\nX-Evil: 1", loc, sizeof(loc)) > 0);
CHECK_STR(loc, "https://keyrelay.local/");
CHECK(renderRedirectTarget("keyrelay.local", "/a\nX-Evil: 1", loc, sizeof(loc)) > 0);
CHECK_STR(loc, "https://keyrelay.local/");
CHECK(renderRedirectTarget("keyrelay.local", "", loc, sizeof(loc)) > 0);
CHECK_STR(loc, "https://keyrelay.local/");
CHECK(renderRedirectTarget("keyrelay.local", nullptr, loc, sizeof(loc)) > 0);
CHECK_STR(loc, "https://keyrelay.local/");
CHECK_EQ(renderRedirectTarget("keyrelay.local", "/", loc, 4), 0); // would not fit
CHECK_EQ(renderRedirectTarget("keyrelay.local", "/", nullptr, 16), 0);
// and no redirect body ever contains a query
CHECK(renderRedirectTarget("192.168.4.1", "/index.html", loc, sizeof(loc)) > 0);
CHECK(strchr(loc, '?') == nullptr);
// ------------------------------------------------------ the flash send
CASE("flash send: empty and null are inert");
FlashSend fs;
CHECK(!fs.armed());
CHECK(fs.done());
CHECK_EQ(fs.sent(), 0);
CHECK_EQ(fs.total(), 0);
CHECK_EQ(fs.next(100, 100), 0);
fs.start(nullptr, 10);
CHECK(!fs.armed());
CHECK_EQ(fs.total(), 0);
fs.start("abc", 0); // a zero-length body is nothing to send
CHECK(!fs.armed());
CHECK(fs.done());
CASE("flash send: one chunk at a time, in order, clamped");
static const char page[] = "0123456789ABCDEFGHIJ"; // 20 bytes
fs.start(page, sizeof(page) - 1);
CHECK(fs.armed());
CHECK(!fs.done());
CHECK_EQ(fs.total(), 20);
CHECK_EQ(fs.remaining(), 20);
CHECK_EQ(fs.next(8, 100), 8); // chunk cap
CHECK_EQ(fs.next(8, 5), 5); // budget cap, whichever is smaller
CHECK_EQ(fs.next(8, 0), 0); // no budget: nothing to hand out
CHECK_EQ(fs.next(0, 100), 0);
char out[32];
memset(out, '.', sizeof(out));
fs.copyTo(out, 8);
CHECK(memcmp(out, "01234567", 8) == 0);
fs.copyTo(out, 999); // clamped to what is left, no overrun
CHECK_EQ(out[20], '.');
fs.advance(8);
CHECK_EQ(fs.sent(), 8);
CHECK_EQ(fs.remaining(), 12);
fs.advance(0); // the retry path: identical bytes again
CHECK_EQ(fs.sent(), 8);
fs.copyTo(out, 4);
CHECK(memcmp(out, "89AB", 4) == 0);
CASE("flash send: completing keeps the totals readable");
DrainLog log;
fs.start(page, sizeof(page) - 1);
while (fs.armed()) {
const size_t take = fs.next(8, 100);
if (take == 0) break;
fs.copyTo(out, take);
// every byte of the piece is the page's, at the cursor: the whole claim of the pump
for (size_t i = 0; i < take; i++)
CHECK(out[i] == page[log.bytes + i]);
log.bytes += take;
log.pieces++;
if (take > log.maxPiece) log.maxPiece = take;
fs.advance(take);
}
CHECK(fs.done());
CHECK(!fs.armed());
CHECK_EQ(log.bytes, 20);
CHECK_EQ(log.pieces, 3); // 8 + 8 + 4
CHECK_EQ(log.maxPiece, 8);
CHECK_EQ(fs.sent(), 20); // sent()/total() still describe the send
CHECK_EQ(fs.remaining(), 0);
fs.advance(5); // past the end: clamped, no wrap
CHECK_EQ(fs.sent(), 20);
CASE("flash send: the real page, on the real budget");
// 59 286 bytes (the count .agents/validate.sh prints) through kPageChunkBytes per write
// and kPageBytesPerTick per tick: the exact numbers the device check compares against.
{
// A stand-in of the served page's size (59 286 B, the `page bytes:` line the gate
// prints). tests/test_relayctl.py pins this constant against the raw string in
// src/web_page.h, so if the page changes and this number does not, a check fails --
// the same "test the table" rule as the mouse bits and the evdev codes.
static const char big[kServedPageBytes] = {0};
FlashSend p;
p.start(big, sizeof(big));
size_t ticks = 0, total = 0, worstTick = 0, worstPiece = 0;
while (p.armed() && ticks < 1000) {
size_t thisTick = 0;
while (p.armed()) {
const size_t take = p.next(kPageChunkBytes, kPageBytesPerTick - thisTick);
if (take == 0) break;
CHECK(take <= kPageChunkBytes);
thisTick += take;
CHECK(thisTick <= kPageBytesPerTick);
total += take;
if (take > worstPiece) worstPiece = take;
p.advance(take);
}
CHECK(thisTick <= kPageBytesPerTick);
if (thisTick > worstTick) worstTick = thisTick;
ticks++;
}
CHECK(p.done());
CHECK_EQ(total, sizeof(big));
CHECK(worstPiece <= kPageChunkBytes);
CHECK(worstTick <= kPageBytesPerTick);
// 59 286 / 1460 = 41 chunks; / 5840 per tick = 11 ticks. If either number changes in
// config.h this line moves, and the report has to move with it.
CHECK_EQ((sizeof(big) + kPageBytesPerTick - 1) / kPageBytesPerTick, 11);
CHECK_EQ((sizeof(big) + kPageChunkBytes - 1) / kPageChunkBytes, 41);
}
CASE("config: the sizing this layer promises");
{
// One transport write is never bigger than the transport's own record cap, and the
// per-tick budget is a whole number of chunks -- otherwise the last write of a tick
// would be a runt and the arithmetic above would have a special case in it.
CHECK(kPageChunkBytes <= kConnWriteChunk);
CHECK_EQ(kPageBytesPerTick % kPageChunkBytes, 0);
CHECK(kPageBytesPerTick >= kPageChunkBytes);
// The spare slot must be able to hold the widest response it can be asked to send:
// a head and its body are queued as a PAIR, so the pair has to fit.
CHECK_EQ(kConnViewTxCap, kHttpResponseHeadCap + kStatusJsonCap);
CHECK(kConnTxCap >= kHttpResponseHeadCap + kStatusJsonCap);
CHECK(kConnTxCap >= kStatusJsonCap + 16); // a whole status frame, as before
}
return report("test_conn");
}