Files
keyrelay/tests/test_conn.cpp
T
ducoterra e0780f58e0 phase: 02_tls_control_channel
**Verified / implemented (phase 02 final pass)**
- All 6 tasks already in `complete/` → verification pass. Device measurement **found a real defect (README bug 11)**: the cert carried its SAN *value* under the OID written from the ASCII text `"2.5.29.17"` instead of DER content bytes `55 1D 11` (`openssl x509 -text` showed `1.10.46.53…`, hex `30 2f 06 09 32 2e 35…`), i.e. **no SAN** → hostname validation impossible while every host check stayed green (they walked the value, never the label).
- Fixed in the pure module: `kSanOidTlv`/`sanOidDer()`, `sanExtensionFullDer()` (pinned to the 43 bytes OpenSSL writes for these names), `derCarriesSan()` (OID *and* value); `tls.cpp` uses them + 3 `static_assert`s; `cert` now warns and names the remedy. 33 new checks in `test_tls_pem.cpp`, incl. the pre-fix extension byte-for-byte (`derContains` yes / `derCarriesSan` no).
- Corrected a false finding: the earlier "fingerprint rotated once" was an **OpenSSL `-ext` artifact** (`C8:76…` vs the real `61:54:19:30…`, same DER). Re-measured: serial `cert` == live handshake DER hash; page 51 626 B, `sha256 dd16e195…f04e9` == locally assembled page.

**Tests / lint (100 % green)**
- `bash .agents/validate.sh` → **GATE_EXIT=0**: `RESULT: all suites passed` in the plain *and* `SANITIZE=1` run, **4826 checks / 0 failures** each; `bridge`/`console`/`rollback` **SUCCESS**; `page bytes: 51626`; `README/AGENTS claims: OK`; `repo hygiene: OK (71 tracked files)`.
- Footprint: bridge **RAM 30.4 % (99 616 B) / Flash 39.6 % (1 246 145 B)** vs baseline 26.7 %/87 592 & 36.1 %/1 135 293 → **+12 024 B RAM / +110 852 B flash**; TLS alone +6 160 B / +100 008 B (rollback image byte-identical); session heap 187 936 → ~145.5 k (1 session) → ~107.9 k (2).
- `ruff check --select F,E9 tools/ tests/` → All checks passed; `bash -n` clean (gate, runner, probe); `-Wall -Wextra` warning-free.

**Completion criteria**: 1 ✓ (gate green + RAM/Flash + session heap quoted); 2 ✓ as recorded (boot prints no fingerprint on the NVS-load path, `cert` prints on demand, identity survived reflashes, nothing about the key is printed anywhere — `nvs erase`→first-boot leg **not re-run**); 3 ✓ re-measured (51 626 == gate; `:80` → 301, no page bytes); 4 partial — 426/401/404 + pinning handshake measured, but arming/typing needs the operator's passphrase (`dflt:0`) and the target is this machine, so no input was sent; 5 ✓ (no `.pem/.key/.crt` tracked or on disk; hygiene block ran); 6 ✓ (`[env:rollback]` built every run + README section); 7 ✓.

**Documentation**: README — check count 4793→4826 (intro + Quick start), `test_tls_pem` 829→862 with new pins, footprint/TLS-cost rows re-measured, "Ten bugs"→"Eleven bugs" + new entry + both anchors, `cert` row, Troubleshooting names-warning row, `tls_pem.h` architecture entry, nine/ten pure-module wording. AGENTS.md (staleness only): "Ten bugs"→"Eleven bugs" ×2, 4826, and the certificate-names row extended with the DER-OID rule. `.agents/HW_VERIFICATION.md` — 426 body/headers re-measured, fingerprint row correction + `-ext` rule, new SAN row (`failed → fixed in tree, device leg needs nvs erase`) with closing commands; phase report written to `.agents/reports/02_tls_control_channel/02_tls_control_channel__00_phase.a1.md`.

**Deviations / live-infra**: made **no** reflash and **no** `nvs erase` (erase also destroys the token and Wi-Fi credentials; flashing puts a live HID keyboard into ROM download mode on this machine). Live steps were read-only `curl`/`openssl` plus one capped `console.py cert` (opening the CDC resets the board — permitted, recorded). No phase/PLAN files touched; everything left in the working tree for the harness to commit.

**Next pending phase**: `03_session_takeover` — `01_pending_claim_socket.md`.
2026-11-09 05:51:57 -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 = 51626;
/** 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");
// 51 626 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 (51 626 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);
// 51 626 / 1460 = 36 chunks; / 5840 per tick = 9 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, 9);
CHECK_EQ((sizeof(big) + kPageChunkBytes - 1) / kPageChunkBytes, 36);
}
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");
}