From 243320f65debab0aaddec4c48e0c98807fcf9c22 Mon Sep 17 00:00:00 2001 From: ScottW514 Date: Mon, 24 Aug 2026 19:02:52 -0400 Subject: [PATCH] ulfius: client_address carries the whole peer sockaddr; forgectrl.auth asserts the loopback report is accepted The dual-stack listener reports every peer as a sockaddr_in6; ulfius 2.7.15 copied sixteen bytes of it, so forgectrl's loopback-only cooling channel refused the controller's every report (403 loopback only) and the engine never saw a run or an armed window. The recipe carries the patch: a sockaddr_storage allocation and a copy of the family's length, in the dispatcher and in ulfius_copy_request. forgetest: forgectrl.auth asserts POST /cool/state from loopback -> 200 beside the LAN 403, and covers src/peer.*. BRINGUP item 21 and the campaign log record how the campaign on dev 20260824215906 found it. --- docs/BRINGUP.md | 23 ++++++- docs/CAMPAIGN-LOG.md | 41 ++++++++++++ forgetest/forgetest/suite/forgectrl.py | 17 ++++- ...ress-carries-the-whole-peer-sockaddr.patch | 65 +++++++++++++++++++ .../recipes-extended/ulfius/ulfius_2.7.15.bb | 6 ++ 5 files changed, 147 insertions(+), 5 deletions(-) create mode 100644 meta-forgefirm/recipes-extended/ulfius/ulfius/0001-client_address-carries-the-whole-peer-sockaddr.patch diff --git a/docs/BRINGUP.md b/docs/BRINGUP.md index 6eeb2f7..712ad27 100644 --- a/docs/BRINGUP.md +++ b/docs/BRINGUP.md @@ -1586,8 +1586,27 @@ Open items only. Anything closed is in `CAMPAIGN-LOG.md`. remove and on the probe unwind, the empty-ring run request logs at ERR level again (it was the only kernel-log trace of the fault), and `image.health` asserts the SDMA clock enable count directly. The ecspi2 - `dmas` stay deleted. Left: build, flash, the item-16 drill and the - campaign. + `dmas` stay deleted. Bench-proven on dev 20260824215906: the clock count + reads 1, the probe reports MOTION OK, `cnc/free` reads the ring less its + gap, and the campaign ran every kernel, forgectrl, logs and motion test + green. + + That campaign then stopped on `cooling.fans-quiet-after-motion`: M8 + raised no fan duty because forgectrl had accepted no cooling report + from the controller at all (`report_age_s` -1). The dual-stack listener + of the second round reports every peer as a `sockaddr_in6`, and ulfius + 2.7.15 copies the peer into `client_address` as `sizeof(struct + sockaddr)`, 16 bytes; the mapped-loopback bytes the check reads lie + beyond the copy, so `POST /cool/state` from 127.0.0.1 got `403 loopback + only` (fail-safe: the engine treats silence as a stand-down, so nothing + fired, but no run profile and no armed window either). The fix: the + image patches ulfius to allocate a `sockaddr_storage` and copy the + family's length (`meta-forgefirm/recipes-extended/ulfius`), the peer + check lives in `src/peer.c` with a host unit test (`auth_peer_test`, + including the truncated-copy case, which fails closed), and + `forgectrl.auth` asserts that the loopback peer is accepted as well as + that a LAN peer is refused. Left: build, flash, the item-16 drill and + the campaign. **Deliberately not gated:** an armed GRBL job after an underrun cuts at the stale origin unless homing is required (GRBL mode permits unhomed cutting; the diff --git a/docs/CAMPAIGN-LOG.md b/docs/CAMPAIGN-LOG.md index 2c07d39..9202b05 100644 --- a/docs/CAMPAIGN-LOG.md +++ b/docs/CAMPAIGN-LOG.md @@ -3934,6 +3934,47 @@ tests and the 270 forgetest tests pass. Owed: the image, then on the bench `clk_enable_count` reading 1, `MOTION OK` from the probe, `cnc/free` at 33521664 idle, a GRBL job, and the campaign. +## 2026-08-24: the clocks proven, and the listener that heard nobody + +Dev 20260824215906, built with the SDMA clock fix, on the bench: `sdma` +`clk_enable_count` 1, the supervisor's probe `MOTION OK` (p2p x=3390 +y=1720), `/mode` verified, `cnc/free` 33521664 at idle, position 0. +Campaign `c-20260824223050-0356` (36 unattended, the fixture in the loop): +`image.health` passed in seconds with its new clock assertion, and every +kernel, forgectrl, logs and motion test passed, `motion.liveness-probe` +and `motion.button-hold-resume` among them. `cooling.flow-verify` passed. +`cooling.fans-quiet-after-motion` failed: `M8 did not raise the fan duty +off idle`. + +The engine had heard nothing. `/cool/status` showed `report_age_s` -1 for +the controller the supervisor had just respawned, and a hand-sent +`POST /cool/state?mode=idle` from 127.0.0.1 answered `403 loopback only`. +The listener is dual-stack since the second kernel round (`:::8080`), so +every peer arrives as a `sockaddr_in6`, the IPv4 client as +`::ffff:127.0.0.1`. forgectrl's check handles that spelling; ulfius 2.7.15 +does not hand it over: `src/ulfius.c` allocates and copies +`client_address` as `sizeof(struct sockaddr)`, 16 bytes, which holds the +family, the port, the flow label and eight address bytes. The mapped +prefix and the 127 sit at bytes 10 to 12 of the address, past the copy, +in heap the check should never have read. Every report since dev +20260824200726 was refused the same way; nothing ran the cooling tests on +those images until now. The direction was safe: the engine treats silence +as a stand-down, so no run profile, no armed window, no fire. + +The fix and its proof so far: the image carries a ulfius patch (a +`sockaddr_storage` allocation, a copy of the family's length, in the +dispatcher and in `ulfius_copy_request`); the recipe builds it clean under +ulfius's own `-Werror -Wconversion`. The peer check moved into +`forgectrl/src/peer.c` unchanged in meaning, with `tests/auth_peer_test.c` +in CI: 127/8, `::1` and mapped 127/8 pass; LAN addresses in both families, +a mapped LAN address, link-local, unspecified, `AF_UNIX`, NULL and a +`::ffff:127.0.0.1` cut to sixteen bytes are refused. forgectrl cross-builds +under `-Werror`. `forgectrl.auth` now asserts the loopback acceptance +(200) next to the LAN refusal (403), so a listener that truncates the +peer fails the catalog on the first forgectrl test rather than the first +cooling one. Owed: the image, the loopback report accepted on the bench, +the campaign. + ## Superseded status notes ### Shared machine services — remaining polish, as listed 2026-08-13 diff --git a/forgetest/forgetest/suite/forgectrl.py b/forgetest/forgetest/suite/forgectrl.py index 24f5a47..c625a92 100644 --- a/forgetest/forgetest/suite/forgectrl.py +++ b/forgetest/forgetest/suite/forgectrl.py @@ -6,7 +6,7 @@ import time from ..catalog import test from .. import hw -_COVERS_AUTH = [("forgectrl", "src/auth.*"), ("forgectrl", "src/main.c")] +_COVERS_AUTH = [("forgectrl", "src/auth.*"), ("forgectrl", "src/peer.*"), ("forgectrl", "src/main.c")] def lan_ip(): @@ -29,7 +29,8 @@ def lan_ip(): covers=_COVERS_AUTH, description="Every state-changing endpoint refuses an unauthenticated write; a non-literal " "Host, a non-literal Origin and a cross-site Sec-Fetch-Site are refused; the " - "cooling report channel refuses a non-loopback peer; the fuse view is two-factor " + "cooling report channel accepts the loopback peer and refuses a non-loopback " + "one; the fuse view is two-factor " "(token and the physical button) and refused without either; " "the flash and factory-restore chain is refused unauthenticated.") def auth(ctx): @@ -93,7 +94,17 @@ def auth(ctx): "GET /fuse-identity with the token but no button -> %s %r (expected the two-factor refusal)", st, body) - # the cooling report channel: loopback only, even with a token + # the cooling report channel: the loopback peer is accepted. An idle + # report is what the controller sends every period; the engine is idle + # here, so it changes nothing. A dual-stack listener reports this peer + # as ::ffff:127.0.0.1, which the check must recognize in full. + st, body = fc.post("/cool/state", params={"mode": "idle", "armed": "0"}) + ev["cool_state_from_loopback"] = st + ctx.log("POST /cool/state from loopback -> %s %s", st, body if isinstance(body, dict) else "") + ctx.check(st == 200, "/cool/state refused the loopback peer (%s %r): the controller's " + "reports never reach the engine", st, body) + + # ...and a non-loopback peer is refused, even with a token ip = lan_ip() ev["lan_ip"] = ip ctx.check(ip, "cannot determine the board's LAN address") diff --git a/meta-forgefirm/recipes-extended/ulfius/ulfius/0001-client_address-carries-the-whole-peer-sockaddr.patch b/meta-forgefirm/recipes-extended/ulfius/ulfius/0001-client_address-carries-the-whole-peer-sockaddr.patch new file mode 100644 index 0000000..780ebd8 --- /dev/null +++ b/meta-forgefirm/recipes-extended/ulfius/ulfius/0001-client_address-carries-the-whole-peer-sockaddr.patch @@ -0,0 +1,65 @@ +The client address carries the whole peer sockaddr + +The request's client_address was allocated and copied as +sizeof(struct sockaddr), 16 bytes. A dual-stack listener reports every +peer as a sockaddr_in6 (28 bytes), so the address bytes a consumer needs +to recognize ::1 or a v4-mapped ::ffff:127.0.0.1 lay beyond the copy. +Allocate a sockaddr_storage and copy the length the family calls for, +here and in ulfius_copy_request(). + +Upstream-Status: Pending +Signed-off-by: Scott Wiederhold + +--- a/src/ulfius.c ++++ b/src/ulfius.c +@@ -30,6 +30,7 @@ + #endif + + #include ++#include + #include + #include + +@@ -515,12 +516,15 @@ + con_info->max_post_param_size = ((struct _u_instance *)cls)->max_post_param_size; + con_info->request->http_protocol = o_strdup(version); + con_info->request->http_verb = o_strdup(method); +- con_info->request->client_address = o_malloc(sizeof(struct sockaddr)); ++ con_info->request->client_address = o_malloc(sizeof(struct sockaddr_storage)); + if (con_info->request->client_address == NULL || con_info->request->http_verb == NULL) { + y_log_message(Y_LOG_LEVEL_ERROR, "Ulfius - Error allocating client_address or http_verb"); + return MHD_NO; + } +- memcpy(con_info->request->client_address, so_client, sizeof(struct sockaddr)); ++ memset(con_info->request->client_address, 0, sizeof(struct sockaddr_storage)); ++ memcpy(con_info->request->client_address, so_client, ++ so_client->sa_family == AF_INET6 ? sizeof(struct sockaddr_in6) : ++ so_client->sa_family == AF_INET ? sizeof(struct sockaddr_in) : sizeof(struct sockaddr)); + if (con_info->u_instance->check_utf8) { + MHD_get_connection_values (connection, MHD_HEADER_KIND, ulfius_fill_map_check_utf8, con_info->request->map_header); + MHD_get_connection_values (connection, MHD_GET_ARGUMENT_KIND, ulfius_fill_map_check_utf8, &con_info->map_url_initial); +--- a/src/u_request.c ++++ b/src/u_request.c +@@ -24,6 +24,7 @@ + */ + #include ++#include + #include + #include + #include + #include +@@ -400,9 +401,12 @@ + dest->callback_position = source->callback_position; + + if (source->client_address != NULL) { +- dest->client_address = o_malloc(sizeof(struct sockaddr)); ++ dest->client_address = o_malloc(sizeof(struct sockaddr_storage)); + if (dest->client_address != NULL) { +- memcpy(dest->client_address, source->client_address, sizeof(struct sockaddr)); ++ memset(dest->client_address, 0, sizeof(struct sockaddr_storage)); ++ memcpy(dest->client_address, source->client_address, ++ source->client_address->sa_family == AF_INET6 ? sizeof(struct sockaddr_in6) : ++ source->client_address->sa_family == AF_INET ? sizeof(struct sockaddr_in) : sizeof(struct sockaddr)); + } else { + y_log_message(Y_LOG_LEVEL_ERROR, "Ulfius - Error allocating resources for dest->client_address"); + ret = U_ERROR_MEMORY; diff --git a/meta-forgefirm/recipes-extended/ulfius/ulfius_2.7.15.bb b/meta-forgefirm/recipes-extended/ulfius/ulfius_2.7.15.bb index 49b454d..a8dfebb 100644 --- a/meta-forgefirm/recipes-extended/ulfius/ulfius_2.7.15.bb +++ b/meta-forgefirm/recipes-extended/ulfius/ulfius_2.7.15.bb @@ -12,6 +12,12 @@ LIC_FILES_CHKSUM = "file://LICENSE;md5=40d2542b8c43a3ec2b7f5da31a697b88" SRC_URI = "git://github.com/babelouest/ulfius;protocol=https;branch=master" SRCREV = "a0603447d3ed63c0880db396b9c395fb4bf6b559" +# The request's client_address is the whole peer sockaddr. Upstream copies +# sizeof(struct sockaddr), 16 bytes, which truncates the sockaddr_in6 a +# dual-stack listener reports for every peer; forgectrl's loopback-only +# report channel reads the mapped address bytes that lie beyond it. +SRC_URI += "file://0001-client_address-carries-the-whole-peer-sockaddr.patch" + S = "${WORKDIR}/git" inherit cmake pkgconfig