Files
keyrelay/tests/test_conn.cpp
T
ducoterra c43a088ce8 phase: 04_controller_page_responsive
**Final verification pass** (all three task reports PASS; tasks already in `complete/`). One real defect found and fixed.

**Commands / results.** `bash tests/run_tests.sh` → `RESULT: all suites passed`, **5 104** checks (page **220**). `SANITIZE=1 bash tests/run_tests.sh` → passed. `bash .agents/validate.sh` → `validation OK`; bridge/console/rollback `SUCCESS`; `page bytes: 59262`; `README/AGENTS claims: OK`; `repo hygiene: OK`. Real-browser leg (headless Chromium, served assembled bytes): no horizontal scroll at 360/390/768/1440, ARM first-in-`main` and sticky, log height-capped, all 5 folds openable, all 3 changed page statements execute.

**Fix.** The folded notes' `<summary>` — the only route to five pinned safety sentences — rendered **17 px** on coarse pointers, under the page's own 44 px thumb floor, while every byte-level assertion stayed green. `src/web_page.h`: `button.k,.pill,input,summary{min-height:44px}` + `summary{padding:13px 0}`; `tests/test_page.js` +1 check (219→220); mutation-tested (reverting the CSS fails host *and* browser legs, then restored byte-for-byte).

**Criteria.** 1 ✓ browser-measured (physical phone still unobserved). 2 ✓ 220 checks + prose untouched. 3 ✓ page 59 286→**59 262 B** (−24 vs phase start), README = gate figure. 4 ✓ one `test_page` row at 220, intro 5 104 = measured. 5 ✓ gate green; phases 01–03 assertions unchanged apart from the re-pinned byte constant.

**Docs.** Re-pinned 59 262 (README, `config.h`, `test_conn.cpp`), totals 5 103→5 104, README Tests row 220; re-measured footprint rows (flash 1 257 901 / 1 178 141 / 1 157 593, TLS +6 176/+100 308); fixed stale "48 KB"/"~50 KB" (×4 src comments, ×2 README) and the 51 626-byte page-delivery row; added **bug 14** with root cause and renamed *Thirteen*→*Fourteen bugs* in README + AGENTS.md's two stale references; added the Phase 04 browser+device row to `.agents/HW_VERIFICATION.md`.

**Deviations.** Device leg stays open: live board serves 51 626 B (pre-phase image); no reflash attempted, per the standing no-unattended-reflash rule — no byte-equality claim made. `ruff check tools tests`: 29 findings, identical at HEAD and `--isolated`, `E9,F` clean, no repo lint config or gate step → left untouched. `.agents/validate.sh` is **untracked and has never been committed** (`git log --all --` empty) though README/AGENTS/PLAN cite it 13× — harness needs `git add` it; I did not stage anything.
2026-11-09 11:00:27 -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 = 59262;
/** 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 262 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 262 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 262 / 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");
}