commit e233607aa644e0431ee41a6b3f1ecff807ce0072 from: Isaac Meerleo date: Thu Jul 16 12:38:57 2026 UTC harden typed worker startup exchange commit - 7ae9fd34087e74f14a7cb1d230cf495ab6819245 commit + e233607aa644e0431ee41a6b3f1ecff807ce0072 blob - 6bfe7612d0ca06dc171951787945560e50a14bbd blob + cd0e6e1e852cc3acca7ffa676c534dfcc90ee5f0 --- regress/sandbox/harness.c +++ regress/sandbox/harness.c @@ -53,6 +53,11 @@ #include "priv.h" #include "proto.h" +#define TEST_BAD_HELLO (-1) +#define TEST_BAD_HELLO_ID (-2) +#define TEST_BAD_PING (-3) +#define TEST_BAD_PING_ID (-4) + /* Blocking imsg read of one message of the expected type. */ static int expect_msg(struct imsgbuf *ibuf, uint32_t want, struct imsg *out) @@ -80,15 +85,22 @@ expect_msg(struct imsgbuf *ibuf, uint32_t want, struct } static void -send_msg(struct imsgbuf *ibuf, uint32_t type, const void *data, - size_t len) +send_msg_id(struct imsgbuf *ibuf, uint32_t type, uint32_t id, + const void *data, size_t len) { - if (imsg_compose(ibuf, type, 0, -1, -1, data, len) == -1) + if (imsg_compose(ibuf, type, id, -1, -1, data, len) == -1) errx(1, "imsg_compose"); if (imsgbuf_flush(ibuf) == -1) errx(1, "imsgbuf_flush"); } +static void +send_msg(struct imsgbuf *ibuf, uint32_t type, const void *data, + size_t len) +{ + send_msg_id(ibuf, type, 0, data, len); +} + static enum fugu_role role_of(const char *s) { @@ -120,6 +132,14 @@ action_of(const char *s) return (SELFTEST_WRITE_OOT); if (strcmp(s, "write_intree") == 0) return (SELFTEST_WRITE_INTREE); + if (strcmp(s, "bad_hello") == 0) + return (TEST_BAD_HELLO); + if (strcmp(s, "bad_hello_id") == 0) + return (TEST_BAD_HELLO_ID); + if (strcmp(s, "bad_ping") == 0) + return (TEST_BAD_PING); + if (strcmp(s, "bad_ping_id") == 0) + return (TEST_BAD_PING_ID); if (strcmp(s, "none") == 0) return (0); errx(1, "unknown action %s", s); @@ -142,6 +162,7 @@ main(int argc, char *argv[]) pid_t wpid; int action, readonly = 0, nowrite = 0, status, alive; const char *expect, *rolename; + char byte = 'x'; log_init(LOG_TO_STDERR, 0, 0); if (argc != 5) @@ -163,9 +184,26 @@ main(int argc, char *argv[]) errx(2, "spawn %s", rolename); if (imsgbuf_init(&ibuf, kid.fd) == -1) errx(2, "imsgbuf_init"); + if (action == TEST_BAD_HELLO || action == TEST_BAD_HELLO_ID) { + if (action == TEST_BAD_HELLO) + send_msg(&ibuf, FUGU_IMSG_HELLO, &byte, sizeof(byte)); + else + send_msg_id(&ibuf, FUGU_IMSG_HELLO, 7, NULL, 0); + alive = (expect_msg(&ibuf, FUGU_IMSG_READY, &imsg) == 0); + if (alive) { + imsg_free(&imsg); + send_msg(&ibuf, FUGU_IMSG_SHUTDOWN, NULL, 0); + } + if (waitpid(kid.pid, &status, 0) == -1) + errx(2, "waitpid bad HELLO"); + return (!alive && WIFEXITED(status) && + WEXITSTATUS(status) == 1 ? 0 : 1); + } /* drive the worker to its steady pledge/unveil */ memset(&hello, 0, sizeof(hello)); + hello.cols = 80; + hello.rows = 24; if (role == ROLE_TTY) { send_msg(&ibuf, FUGU_IMSG_HELLO, &hello, sizeof(hello)); } else if (role == ROLE_API) { @@ -199,13 +237,33 @@ main(int argc, char *argv[]) } if (expect_msg(&ibuf, FUGU_IMSG_READY, &imsg) == -1) errx(2, "%s never became ready", rolename); + if (imsg_get_len(&imsg) != 0 || imsg_get_id(&imsg) != 0) + errx(2, "%s sent malformed READY", rolename); imsg_free(&imsg); + if (action == TEST_BAD_PING || action == TEST_BAD_PING_ID) { + if (action == TEST_BAD_PING) + send_msg(&ibuf, FUGU_IMSG_PING, &byte, sizeof(byte)); + else + send_msg_id(&ibuf, FUGU_IMSG_PING, 7, NULL, 0); + alive = (expect_msg(&ibuf, FUGU_IMSG_PONG, &imsg) == 0); + if (alive) { + imsg_free(&imsg); + send_msg(&ibuf, FUGU_IMSG_SHUTDOWN, NULL, 0); + } + if (waitpid(kid.pid, &status, 0) == -1) + errx(2, "waitpid bad PING"); + return (!alive && WIFEXITED(status) && + WEXITSTATUS(status) == 1 ? 0 : 1); + } + if (action == 0) { /* handshake-only: verify liveness then shut down cleanly */ send_msg(&ibuf, FUGU_IMSG_PING, NULL, 0); if (expect_msg(&ibuf, FUGU_IMSG_PONG, &imsg) == -1) errx(2, "%s did not pong", rolename); + if (imsg_get_len(&imsg) != 0 || imsg_get_id(&imsg) != 0) + errx(2, "%s sent malformed PONG", rolename); imsg_free(&imsg); send_msg(&ibuf, FUGU_IMSG_SHUTDOWN, NULL, 0); waitpid(kid.pid, &status, 0); blob - c52938907c4fabb5d87b143f56237b40c20c673e blob + f85a09d29f9352c3f7a43139b226c91d8b268187 --- regress/sandbox/run.sh +++ regress/sandbox/run.sh @@ -90,6 +90,16 @@ run_case fugu-web none ok - run_case fugu-tool none ok - run_case fugu-editor none ok - +# I10/I13: startup and liveness frames are exact typed exchanges. The TTY +# and editor historically accepted malformed HELLO payloads; the common +# worker runtime must also reject nonzero control correlation IDs and malformed +# PING frames rather than replying as though they were valid. +run_case fugu-tty bad_hello reject - +run_case fugu-editor bad_hello reject - +run_case fugu-editor bad_hello_id reject - +run_case fugu-editor bad_ping reject - +run_case fugu-editor bad_ping_id reject - + # I1: the credential custodians cannot exec or open files. run_case fugu-api exec dead execve run_case fugu-api open_etc dead open blob - 8e670dea289657fa382a40a2f9c6d272da945fd6 blob + 468df9fb27bfb87b4a1b0a5814de22a4fbe39a96 --- regress/turn/Makefile +++ regress/turn/Makefile @@ -6,7 +6,7 @@ LDADD= -ltls DPADD= ${LIBTLS} REGRESS_TARGETS= run-turn run-agent -CLEANFILES+= reqcheck +CLEANFILES+= reqcheck handshake REQCHECK_SRCS= ${.CURDIR}/reqcheck.c \ ${.CURDIR}/../../src/common/json.c \ @@ -17,11 +17,15 @@ REQCHECK_SRCS= ${.CURDIR}/reqcheck.c \ reqcheck: ${REQCHECK_SRCS} ${CC} ${CFLAGS} -o ${.OBJDIR}/reqcheck ${REQCHECK_SRCS} +handshake: ${.CURDIR}/handshake.c + ${CC} ${CFLAGS} -I${.CURDIR}/../../src/fugu \ + -o ${.OBJDIR}/handshake ${.CURDIR}/handshake.c -lutil + # a pty driver to exercise the curses front end end to end (2.1) ptydrive: ${.CURDIR}/ptydrive.c ${CC} ${CFLAGS} -o ${.OBJDIR}/ptydrive ${.CURDIR}/ptydrive.c -lutil -run-turn: stub ptydrive reqcheck +run-turn: stub ptydrive reqcheck handshake sh ${.CURDIR}/run.sh ${.OBJDIR} ${.CURDIR}/../.. run-agent: stub blob - /dev/null blob + 93c0d1aef1b93dccd380c6ecd7d28e647bc1471f (mode 644) --- /dev/null +++ regress/turn/handshake.c @@ -0,0 +1,139 @@ +/* + * Copyright (c) 2026 Isaac + * + * Permission to use, copy, modify, and distribute this software for any + * purpose with or without fee is hereby granted, provided that the above + * copyright notice and this permission notice appear in all copies. + * + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR + * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN + * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF + * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. + */ + +/* + * Hostile fugu-editor replacement for the coordinator handshake regressions. + * It receives HELLO, emits the selected malformed startup sequence, then + * remains cooperative for SHUTDOWN so a vulnerable coordinator can finish a + * normal print Turn instead of making the test pass through an unrelated EOF. + */ + +#include + +#include +#include +#include +#include +#include +#include + +#include "proto.h" + +static int +next_imsg(struct imsgbuf *ibuf, struct imsg *imsg) +{ + struct pollfd pfd; + ssize_t n; + + for (;;) { + if ((n = imsg_get(ibuf, imsg)) == -1) + return (-1); + if (n != 0) + return (1); + pfd.fd = ibuf->fd; + pfd.events = POLLIN; + pfd.revents = 0; + if (poll(&pfd, 1, 5000) <= 0) + return (-1); + if ((n = imsgbuf_read(ibuf)) <= 0) + return (0); + } +} + +static void +send_msg_id(struct imsgbuf *ibuf, uint32_t type, uint32_t id, + const void *data, size_t len) +{ + if (imsg_compose(ibuf, type, id, -1, -1, data, len) == -1) + err(2, "imsg_compose"); +} + +static void +send_msg(struct imsgbuf *ibuf, uint32_t type, const void *data, size_t len) +{ + send_msg_id(ibuf, type, 0, data, len); +} + +int +main(int argc, char *argv[]) +{ + struct imsgbuf ibuf; + struct imsg imsg; + const char *attack; + char byte = 'x'; + int n; + + (void)argc; + (void)argv; + if ((attack = getenv("FUGU_HANDSHAKE_ATTACK")) == NULL) + err(2, "FUGU_HANDSHAKE_ATTACK"); + if (imsgbuf_init(&ibuf, FUGU_CTL_FD) == -1) + err(2, "imsgbuf_init"); + if ((n = next_imsg(&ibuf, &imsg)) != 1 || + imsg_get_type(&imsg) != FUGU_IMSG_HELLO) + errx(2, "expected HELLO"); + imsg_free(&imsg); + + if (strcmp(attack, "duplicate") == 0) { + send_msg(&ibuf, FUGU_IMSG_READY, NULL, 0); + send_msg(&ibuf, FUGU_IMSG_READY, NULL, 0); + } else if (strcmp(attack, "payload") == 0) + send_msg(&ibuf, FUGU_IMSG_READY, &byte, sizeof(byte)); + else if (strcmp(attack, "id") == 0) + send_msg_id(&ibuf, FUGU_IMSG_READY, 7, NULL, 0); + else if (strcmp(attack, "pong") == 0) { + send_msg(&ibuf, FUGU_IMSG_PONG, NULL, 0); + send_msg(&ibuf, FUGU_IMSG_READY, NULL, 0); + } else if (strcmp(attack, "pong_duplicate") == 0 || + strcmp(attack, "pong_payload") == 0 || + strcmp(attack, "pong_id") == 0) { + /* A fixed coordinator sends PING only after all unique READYs. + * An old coordinator instead reaches the Turn and eventually + * sends SHUTDOWN; returning cleanly keeps that RED outcome tied + * to the missing typed PONG validation. */ + send_msg(&ibuf, FUGU_IMSG_READY, NULL, 0); + if (imsgbuf_flush(&ibuf) == -1) + err(2, "imsgbuf_flush READY"); + if ((n = next_imsg(&ibuf, &imsg)) != 1) + return (n == 0 ? 0 : 2); + if (imsg_get_type(&imsg) == FUGU_IMSG_SHUTDOWN) { + imsg_free(&imsg); + return (0); + } + if (imsg_get_type(&imsg) != FUGU_IMSG_PING) + errx(2, "expected PING"); + imsg_free(&imsg); + if (strcmp(attack, "pong_duplicate") == 0) { + send_msg(&ibuf, FUGU_IMSG_PONG, NULL, 0); + send_msg(&ibuf, FUGU_IMSG_PONG, NULL, 0); + } else if (strcmp(attack, "pong_payload") == 0) + send_msg(&ibuf, FUGU_IMSG_PONG, &byte, sizeof(byte)); + else + send_msg_id(&ibuf, FUGU_IMSG_PONG, 7, NULL, 0); + } else + errx(2, "unknown handshake attack: %s", attack); + if (imsgbuf_flush(&ibuf) == -1) + err(2, "imsgbuf_flush"); + + while ((n = next_imsg(&ibuf, &imsg)) == 1) { + if (imsg_get_type(&imsg) == FUGU_IMSG_SHUTDOWN) { + imsg_free(&imsg); + return (0); + } + imsg_free(&imsg); + } + return (n == 0 ? 0 : 2); +} blob - ad425bd327f6fed1b27f8091d0960f111bd718cf blob + fad71189bc0121f1a51aae272ac685ff851074a8 --- regress/turn/run.sh +++ regress/turn/run.sh @@ -111,6 +111,36 @@ run_fugu_in() { # $1 = stdin file, rest = args sessions=$HOME/.fugu/sessions +# --- 0: startup replies are per-Role, empty, and exactly once (I10/I13) --- +# Replace only the otherwise-unused print-mode editor. It remains alive for +# SHUTDOWN, so an unhardened coordinator completes the OK Turn; only rejecting +# the hostile handshake can make these checks pass. +start_stub "OK OK OK OK OK OK OK" +write_conf "$stubport" +cp "$lx/fugu-editor" "$dir/fugu-editor.real" +cp "$obj/handshake" "$lx/fugu-editor" +for attack in duplicate payload id pong pong_duplicate pong_payload pong_id; do + rm -rf "$HOME/.fugu" + env FUGU_HANDSHAKE_ATTACK="$attack" FUGU_CONF="$dir/fugu.conf" \ + FUGU_LIBEXEC="$dir/libexec/fugu" FUGU_CA_FILE="$dir/cert.pem" \ + FUGU_RETRY_BASE_MS=50 "$fugu" -p "handshake probe" \ + "$dir/out" 2>"$dir/errout" + rc=$? + ok "handshake $attack: coordinator fails closed" \ + $([ "$rc" -ne 0 ]; echo $?) + case "$attack" in + duplicate|payload|id) + grep -q "bad READY from fugu-editor" "$dir/errout" + ;; + pong|pong_duplicate|pong_payload|pong_id) + grep -q "bad PONG from fugu-editor" "$dir/errout" + ;; + esac + ok "handshake $attack: typed violation is reported" $? +done +cp "$dir/fugu-editor.real" "$lx/fugu-editor" +stop_stub + # --- 1: a clean OK turn --------------------------------------------- start_stub "OK" write_conf "$stubport" blob - 3bec9f667fc5ff3e063fd8573fc16846cb499c22 blob + 60341b8c5acd1592c87a55c9e428785eba8683f2 --- src/common/worker.c +++ src/common/worker.c @@ -174,12 +174,17 @@ worker_dispatch(int fd, short event, void *arg) case FUGU_IMSG_HELLO: if (w->ready) fatalx("%s: duplicate HELLO", w->title); + if (imsg_get_id(&imsg) != 0) + fatalx("%s: bad HELLO correlation", w->title); if (w->lockdown(w, &imsg) == -1) fatalx("%s: lockdown failed", w->title); w->ready = 1; imsgev_send(&w->iev, FUGU_IMSG_READY, 0, NULL, 0); break; case FUGU_IMSG_PING: + if (imsg_get_len(&imsg) != 0 || + imsg_get_id(&imsg) != 0) + fatalx("%s: bad PING", w->title); imsgev_send(&w->iev, FUGU_IMSG_PONG, 0, NULL, 0); break; case FUGU_IMSG_LOG_SYSLOG: blob - 4472b2143282d3cdebf4abed38f4f8e1e66f820a blob + 8abfa6306d29944cac454e3872a47d78c06a6dbe --- src/fugu/coord.c +++ src/fugu/coord.c @@ -1985,29 +1985,56 @@ coord_dispatch(int fd, short event, void *arg) if (!allowed_from(role, type)) fatalx("coordinator: imsg %u from %s", type, coord->kids[role].title); + if (type != FUGU_IMSG_READY && type != FUGU_IMSG_PONG && + !coord->kids[role].ready) + fatalx("coordinator: imsg %u before READY from %s", type, + coord->kids[role].title); switch (type) { - case FUGU_IMSG_READY: + case FUGU_IMSG_READY: { + struct imsgev_payload payload; + int r; + + if (imsgev_get_payload(&imsg, &payload) == -1 || + payload.len != 0 || imsg_get_id(&imsg) != 0 || + coord->kids[role].ready) + fatalx("bad READY from %s", + coord->kids[role].title); + coord->kids[role].ready = 1; /* READY from fugu-tty means ui_start has completed and * curses now owns the terminal. This is the exact boundary * where the Coordinator and every Role leave startup stderr; * -d suppresses the switch for the whole process mesh. */ if (role == ROLE_TTY && coord->ui && !coord->debug) { - int r; - log_init(LOG_TO_SYSLOG, coord->verbose, LOG_DAEMON); for (r = ROLE_TTY; r < ROLE_MAX; r++) imsgev_send(&coord->iev[r], FUGU_IMSG_LOG_SYSLOG, 0, NULL, 0); } - if (++coord->nready == coord->nkids) - event_loopexit(NULL); + if (++coord->nready == coord->nkids) { + /* Every Role proved steady confinement. One typed + * liveness round completes the startup exchange. */ + for (r = ROLE_TTY; r < ROLE_MAX; r++) + imsgev_send(&coord->iev[r], FUGU_IMSG_PING, + 0, NULL, 0); + } break; - case FUGU_IMSG_PONG: + } + case FUGU_IMSG_PONG: { + struct imsgev_payload payload; + + if (imsgev_get_payload(&imsg, &payload) == -1 || + payload.len != 0 || imsg_get_id(&imsg) != 0 || + !coord->kids[role].ready || + coord->nready != coord->nkids || + coord->kids[role].pong) + fatalx("bad PONG from %s", coord->kids[role].title); + coord->kids[role].pong = 1; if (++coord->npong == coord->nkids) event_loopexit(NULL); break; + } case FUGU_IMSG_A_TEXT: case FUGU_IMSG_A_USAGE: case FUGU_IMSG_A_TOOL_BEGIN: @@ -4326,7 +4353,7 @@ coord_run(struct fugu_conf *cf, const char *prompt, si send_hello(&c, role); conf_wipe_secrets(cf); - event_dispatch(); /* until every worker is READY */ + event_dispatch(); /* until every worker is READY and PONGed */ /* the in-tree context file is read via fugu-tool (no rpath here) */ context_post_bringup(&c, resume_id != NULL); blob - 2b0f2f6a9fcc6155df7323c5b8a4e22eb67e1a5d blob + ee24d4ffa816365de7f2aec47743ffc9455c8891 --- src/fugu/priv.c +++ src/fugu/priv.c @@ -149,6 +149,7 @@ priv_spawn(struct child *c, const char *libexec, enum c->role = role; c->title = title; c->ready = 0; + c->pong = 0; c->fd = fds[0]; return (0); } blob - 7273ba385a212b7b8456fc0b3d81199d3570d442 blob + 5dd12213c55119b6f7308b081d008c2b4e55d85a --- src/fugu/priv.h +++ src/fugu/priv.h @@ -37,6 +37,7 @@ struct child { const char *title; int fd; /* parent end of the imsg channel */ int ready; + int pong; /* answered the startup liveness round */ }; /* blob - 069e63f2ee77a067e43d2b3a5a003715b89581dd blob + 64cb697e7ff1393b7924d2f6dbc0ff4360892ec7 --- src/fugu-editor/main.c +++ src/fugu-editor/main.c @@ -320,8 +320,13 @@ edit_dispatch(struct worker *w, struct imsg *imsg) static int edit_lockdown(struct worker *w, struct imsg *imsg) { + struct imsgev_payload payload; + (void)w; - (void)imsg; /* HELLO carries nothing for this room */ + if (imsgev_get_payload(imsg, &payload) == -1) + fatal("get editor HELLO payload"); + if (payload.len != 0) + fatalx("fugu-editor: malformed HELLO"); /* * Steady state: the temp file (rpath wpath cpath), the shared blob - ab37d11aefcdb8201041780c97c9a7fe699c1f0a blob + 28ed2b97652a5bf29e1654f4695b10fae98b8498 --- src/fugu-tty/main.c +++ src/fugu-tty/main.c @@ -40,10 +40,15 @@ static int tty_lockdown(struct worker *w, struct imsg *imsg) { struct hello_tty h; + struct imsgev_payload payload; - memset(&h, 0, sizeof(h)); - if (imsg_get_len(imsg) == sizeof(h)) - (void)imsg_get_data(imsg, &h, sizeof(h)); + if (imsgev_get_payload(imsg, &payload) == -1) + fatal("get TTY HELLO payload"); + if (payload.data == NULL || payload.len != sizeof(h)) + fatalx("fugu-tty: malformed HELLO"); + memcpy(&h, payload.data, sizeof(h)); + if ((h.curses != 0 && h.curses != 1) || h.cols <= 0 || h.rows <= 0) + fatalx("fugu-tty: malformed HELLO policy"); /* * In print/line mode the front end is idle and must not touch the