mirror of
https://github.com/openglow-org/forgefirm.git
synced 2026-09-27 16:51:12 -07:00
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.
This commit is contained in:
+21
-2
@@ -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
|
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
|
level again (it was the only kernel-log trace of the fault), and
|
||||||
`image.health` asserts the SDMA clock enable count directly. The ecspi2
|
`image.health` asserts the SDMA clock enable count directly. The ecspi2
|
||||||
`dmas` stay deleted. Left: build, flash, the item-16 drill and the
|
`dmas` stay deleted. Bench-proven on dev 20260824215906: the clock count
|
||||||
campaign.
|
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
|
**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
|
stale origin unless homing is required (GRBL mode permits unhomed cutting; the
|
||||||
|
|||||||
@@ -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,
|
bench `clk_enable_count` reading 1, `MOTION OK` from the probe,
|
||||||
`cnc/free` at 33521664 idle, a GRBL job, and the campaign.
|
`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
|
## Superseded status notes
|
||||||
|
|
||||||
### Shared machine services — remaining polish, as listed 2026-08-13
|
### Shared machine services — remaining polish, as listed 2026-08-13
|
||||||
|
|||||||
@@ -6,7 +6,7 @@ import time
|
|||||||
from ..catalog import test
|
from ..catalog import test
|
||||||
from .. import hw
|
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():
|
def lan_ip():
|
||||||
@@ -29,7 +29,8 @@ def lan_ip():
|
|||||||
covers=_COVERS_AUTH,
|
covers=_COVERS_AUTH,
|
||||||
description="Every state-changing endpoint refuses an unauthenticated write; a non-literal "
|
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 "
|
"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; "
|
"(token and the physical button) and refused without either; "
|
||||||
"the flash and factory-restore chain is refused unauthenticated.")
|
"the flash and factory-restore chain is refused unauthenticated.")
|
||||||
def auth(ctx):
|
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)",
|
"GET /fuse-identity with the token but no button -> %s %r (expected the two-factor refusal)",
|
||||||
st, body)
|
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()
|
ip = lan_ip()
|
||||||
ev["lan_ip"] = ip
|
ev["lan_ip"] = ip
|
||||||
ctx.check(ip, "cannot determine the board's LAN address")
|
ctx.check(ip, "cannot determine the board's LAN address")
|
||||||
|
|||||||
+65
@@ -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 <s.e.wiederhold@gmail.com>
|
||||||
|
|
||||||
|
--- a/src/ulfius.c
|
||||||
|
+++ b/src/ulfius.c
|
||||||
|
@@ -30,6 +30,7 @@
|
||||||
|
#endif
|
||||||
|
|
||||||
|
#include <ctype.h>
|
||||||
|
+#include <netinet/in.h>
|
||||||
|
#include <stdlib.h>
|
||||||
|
#include <string.h>
|
||||||
|
|
||||||
|
@@ -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 <ctype.h>
|
||||||
|
+#include <netinet/in.h>
|
||||||
|
#include <stdarg.h>
|
||||||
|
#include <stdlib.h>
|
||||||
|
#include <string.h>
|
||||||
|
#include <u_private.h>
|
||||||
|
@@ -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;
|
||||||
@@ -12,6 +12,12 @@ LIC_FILES_CHKSUM = "file://LICENSE;md5=40d2542b8c43a3ec2b7f5da31a697b88"
|
|||||||
SRC_URI = "git://github.com/babelouest/ulfius;protocol=https;branch=master"
|
SRC_URI = "git://github.com/babelouest/ulfius;protocol=https;branch=master"
|
||||||
SRCREV = "a0603447d3ed63c0880db396b9c395fb4bf6b559"
|
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"
|
S = "${WORKDIR}/git"
|
||||||
|
|
||||||
inherit cmake pkgconfig
|
inherit cmake pkgconfig
|
||||||
|
|||||||
Reference in New Issue
Block a user