From d6bf080f9a72f6be58139b643d8c153059eb7033 Mon Sep 17 00:00:00 2001 From: Tom Elliott Date: Wed, 9 Sep 2026 08:19:48 -0500 Subject: [PATCH] Refuse a multicast stream that is not the one this partition asked for udpcast carries no metadata, so a stream is bound to a partition by nothing but its position in the sequence: the server chains one udp-sender per image file on a shared portbase, FOS opens one udp-receiver per file it expects, and the Nth receiver gets the Nth stream. A client that reboots mid-session -- or whose receiver opens after a sender has already stopped waiting for it -- therefore lands one stream out of step, and every partition after that is restored from the wrong image. partclone objects only when the target partition is SMALLER than the source; when it is larger the wrong filesystem is written and the deploy reports success. fogproject #1742 records a run where partition 3's image was caught on its way onto partition 2 (105 MB target, 135 MB source) only after partition 2's had already gone silently onto partition 1 (300 MB target). The server now introduces every stream with a fixed 128-byte record naming the image file it carries. writeImage reads it off the receiver before the decompressor is started -- so a refusal leaves the target untouched -- and compares it against the file it was asked for. That argument was already being passed in and thrown away on the multicast path; it is now the assertion. Three details are forced rather than chosen. The record is fixed width because a count of bytes is the only thing a pipe lets you read without consuming payload. It is read with bs=1 because a pipe may return a short read and over-reading eats the front of the image. And the header is only stripped when getversion.php?caps=1 advertises mcstreamid, because taking 128 bytes off a server that does not prepend them would corrupt every deploy -- the call is checked before its answer is read, so an unreachable server does not read as "no header" (the GH-1266 shape, same as the mclvm probe beside it). The name is the basename, except that a stream carrying several files is named by the stem they share, which is what the client's own glob collapses to. So d1p1.img* and sys.img.* both become the stem while rec.img.000 and rec.img.001 keep their names -- the sys/rec asymmetry is FOS's own and the server already mirrors it when deciding what to concatenate. Both sides reach the same string from their own layout; neither counts the other's streams. tests/checks/multicast-stream-identity.sh greps the wiring, because the ordering of "open the receiver, read the header, only then restore" is what makes a refusal safe, and executes the real function against synthetic headers for the naming rule, because that rule is not uniform. Seven mutations -- the call removed, the check moved after the payload starts flowing, each of the two suffix strips dropped, the comparison neutered, one byte short on the read, the capability guard removed -- each fail it. Writing that harness found a defect in the check itself: the right side of [[ != ]] is a pattern, so an unstripped d1p1.img* matched the stem it was meant to be compared against. Quoted. Verified against real udpcast on a loopback, driving the server's own emitted command and this function: a flat partition asked for by name and a split one asked for by glob both accepted with the payload byte-identical by sha256 after the header, and partition 3's stream offered to partition 2 refused. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01A767exFmz6sQUcuZofqE1V --- .../rootfs_overlay/usr/share/fog/lib/funcs.sh | 88 +++++++++- ...8-multicast-streams-identify-themselves.md | 111 +++++++++++++ tests/checks/multicast-stream-identity.sh | 156 ++++++++++++++++++ 3 files changed, 351 insertions(+), 4 deletions(-) create mode 100644 docs/adr/0018-multicast-streams-identify-themselves.md create mode 100755 tests/checks/multicast-stream-identity.sh diff --git a/Buildroot/board/FOG/FOS/rootfs_overlay/usr/share/fog/lib/funcs.sh b/Buildroot/board/FOG/FOS/rootfs_overlay/usr/share/fog/lib/funcs.sh index 61c899c..dcf3477 100644 --- a/Buildroot/board/FOG/FOS/rootfs_overlay/usr/share/fog/lib/funcs.sh +++ b/Buildroot/board/FOG/FOS/rootfs_overlay/usr/share/fog/lib/funcs.sh @@ -865,6 +865,73 @@ countPartTypes() { # $1 = Source File # $2 = Target # $3 = mc task or not (not required) +# Answers whether this server prefixes each multicast stream with an +# identity header, caching the probe in $mcstreamidcap. A server that +# does not must not have 128 bytes stripped off its payload, so the +# question has to be settled before the first receiver's bytes are read. +# +# The CALL is checked before its answer is interpreted: reading the body +# straight out of $( ) would turn an unreachable server into "no header", +# which is a claim about the server's VERSION drawn from a dead network. +# Same shape as fogproject GH-1266, same handling as the mclvm probe. +mcStreamIdSupported() { + [[ -n ${mcstreamidcap:-} ]] && return 0 + local servercaps="" + if ! callServer "${web}service/getversion.php?caps=1"; then + handleError "Cannot confirm how the FOG server frames multicast streams: ${serverReason} (${FUNCNAME[0]})\n Args Passed: $*" + fi + servercaps="$serverBody" + if [[ $servercaps == *mcstreamid* ]]; then + mcstreamidcap="yes" + else + mcstreamidcap="no" + fi + return 0 +} +# Reads the identity header off the front of a multicast stream and refuses +# the stream if it is not the image file this partition asked for. +# +# udpcast carries no metadata, so a stream is bound to a partition by +# nothing but its position in the sequence. A client that reboots +# mid-session, or whose receiver opens after a sender has already given up +# waiting for it, lands one stream out of step and every partition after +# that is restored from the wrong image. partclone objects only when the +# target partition is smaller than the source; when it is larger the wrong +# filesystem is written and the deploy reports success. fogproject #1742. +# +# Reads on fd 9, which the caller has open on the receiver, and reads a +# byte at a time: a pipe may return a short read, and over-reading would +# eat payload. +# +# $1 = the image file this partition expects (may be a glob) +checkStreamIdentity() { + local wanted="$1" + [[ -z $wanted ]] && handleError "No expected image file passed (${FUNCNAME[0]})\n Args Passed: $*" + # The server names a concatenated stream by the stem its chunks share, + # which is what the client's own glob collapses to: d1p4.img* and + # sys.img.* both become the stem. A plain filename is used verbatim, + # so rec.img.000 and rec.img.001 stay distinguishable -- under the + # legacy layouts those are whole partitions, not chunks. + wanted="$(basename "$wanted")" + wanted="${wanted%\*}" + wanted="${wanted%.}" + local hdr="" + hdr=$(dd bs=1 count=128 status=none <&9) + local tag="${hdr%% *}" + if [[ $tag != FOGMC1 ]]; then + handleError "Multicast stream for $wanted did not start with a stream header; the server and this client disagree about the stream format (${FUNCNAME[0]})\n Args Passed: $*" + fi + local got="${hdr#FOGMC1 }" + got="${got%%[[:space:]]*}" + # Quoted: an unquoted right side of [[ != ]] is a PATTERN, so a $wanted + # still carrying its glob would match the stem it was supposed to be + # compared against, and the star-strip above would be doing the work + # by accident. Names with [ or ? would be worse. + if [[ $got != "$wanted" ]]; then + handleError "Multicast stream mismatch: expected $wanted but the server is sending $got. The receiver is out of step with the sender, which would restore this image onto the wrong partition (${FUNCNAME[0]})\n Args Passed: $*" + fi + echo " * Multicast stream $got" +} writeImage() { local file="$1" local target="$2" @@ -873,10 +940,23 @@ writeImage() { mkfifo /tmp/pigz1 case $mc in yes) - if [[ -z $mcastrdv ]]; then - udp-receiver --nokbd --portbase $port --ttl 32 --mcast-rdv-address $storageip 2>/dev/null >/tmp/pigz1 & + [[ -z $file ]] && handleError "No source file passed (${FUNCNAME[0]})\n Args Passed: $*" + local rdvaddress="$storageip" + [[ -n $mcastrdv ]] && rdvaddress="$mcastrdv" + mcStreamIdSupported + if [[ $mcstreamidcap == yes ]]; then + # The receiver's own output is read here first, so that the + # header can be checked before anything reaches partclone + # and the target partition is still untouched on a refusal. + rm -f /tmp/mcraw + mkfifo /tmp/mcraw + udp-receiver --nokbd --portbase $port --ttl 32 --mcast-rdv-address $rdvaddress 2>/dev/null >/tmp/mcraw & + exec 9/tmp/pigz1 & + exec 9<&- else - udp-receiver --nokbd --portbase $port --ttl 32 --mcast-rdv-address $mcastrdv 2>/dev/null >/tmp/pigz1 & + udp-receiver --nokbd --portbase $port --ttl 32 --mcast-rdv-address $rdvaddress 2>/dev/null >/tmp/pigz1 & fi ;; *) @@ -918,7 +998,7 @@ writeImage() { exitcode=$? set +o pipefail [[ ! $exitcode -eq 0 ]] && handleError "Image failed to restore and exited with exit code $exitcode (${FUNCNAME[0]})\n Info: $(cat /tmp/partclone.log)\n Args Passed: $*" - rm -rf /tmp/pigz1 >/dev/null 2>&1 + rm -rf /tmp/pigz1 /tmp/mcraw >/dev/null 2>&1 } # Gets the valid restore parts. They're only # valid if the partition data exists for diff --git a/docs/adr/0018-multicast-streams-identify-themselves.md b/docs/adr/0018-multicast-streams-identify-themselves.md new file mode 100644 index 0000000..455a9a1 --- /dev/null +++ b/docs/adr/0018-multicast-streams-identify-themselves.md @@ -0,0 +1,111 @@ +# Multicast streams identify themselves + +udpcast carries no metadata. The server chains one `udp-sender` per image +file on a shared portbase, FOS opens one `udp-receiver` per file it expects, +and the Nth receiver gets the Nth stream. [ADR-0007](0007-multicast-lvm-sidecar-order-contract.md) +recorded that property and handled one of its consequences — version skew +across per-LV image files — with a capability probe. It did not address the +other: **position is the only thing binding a stream to a partition, and +nothing detects when the two sides stop agreeing on it.** + +We decided **every stream is prefixed with a fixed 128-byte record naming the +image file it carries, and a receiver refuses a stream that is not the one it +asked for, before any of it reaches partclone.** + +## The failure this closes + +fogproject issue #1742. A host was reset while its multicast task kept +running; the sender chain does not rewind, so the rebooted client opened its +first receiver against the sender already on the second file and stayed one +stream behind. Partition 2's image was written to partition 1 and partition +3's was offered to partition 2. + +Only the second of those was noticed, and only by accident: partclone +compares the incoming source size against the target device and refuses when +the target is smaller (105 MB EFI partition, 135 MB MSR source). The first +went the other way — a 100 MiB image onto a 300 MiB partition — and produced +no error at all. **A larger target means the wrong filesystem is written and +the deploy reports success.** That is the half worth fixing; the noisy half +was already survivable. + +partclone's own check cannot be tightened into a fix, because for a resizable +image a target larger than the source is legitimate and routine. The +comparison that would mean something is against the size the partition had in +the *image*, and nothing on the wire says which partition that is. + +## Why a header and not per-stream ports + +The obvious structural fix is to give sender *i* and receiver *i* their own +portbase so no other pair can meet. It was rejected on two counts. + +FOS would have to derive the same stream index the server used, and for split +and per-LV images that index is not the partition number. ADR-0007 refused +exactly this re-derivation for exactly this reason: a divergence is not a +crash but a silent data-placement bug. Naming the file instead means both +sides reach the same string from their own layout, and neither counts the +other's streams. + +The port budget also does not survive it. Sessions currently reserve two +ports each out of a window (`FOG_UDPCAST_STARTINGPORT` plus twice +`FOG_MULTICAST_MAX_SESSIONS`) that the installer opens in the firewall as a +matching range in `lib/common/config.sh`. Per-stream ports multiply that by +the partition count, and the two definitions have to move together or +multicast breaks on any firewalled server — a pairing nothing enforces. + +## The naming rule, and why it is not uniform + +The id is the image file basename, except that a stream carrying several +files is named by the stem its files share: + +| On the wire | Layout | Client asks for | Id | +|---|---|---|---| +| `d1p2.img` | flat partition | the name | `d1p2.img` | +| `d1p1.img.000`, `.001` | split chunks | `d1p1.img*` | `d1p1.img` | +| `d1p2.img.000` | split, one chunk | `d1p2.img*` | `d1p2.img` | +| `sys.img.000`, `.001` | **one** partition | `sys.img.*` | `sys.img` | +| `rec.img.000` / `.001` | **two** partitions | each by name | `rec.img.000` | + +The `sys`/`rec` asymmetry is FOS's own and predates this: `sys.img.*` is one +partition spread over several files, `rec.img.NNN` is one partition each. +The server already mirrors it when it decides what to concatenate (#897), so +the id is decided at the same place, in the branch that already knows the +answer, rather than re-derived afterward from a filename. + +The client reaches the same string by taking the basename of what it was +about to restore and dropping a trailing `*` and `.`. That argument was +already being passed to `writeImage` and discarded on the multicast path — +it is now the assertion. + +## Fixed width, read a byte at a time + +The client reads the header with a single `dd bs=1 count=128` on a file +descriptor it holds open on the receiver. A count of bytes is the only thing +a pipe lets it read without consuming payload, so the record cannot be +variable-length or newline-delimited. `bs=1` because a pipe may return a +short read, and over-reading would eat the front of the image. + +The read happens before the decompressor is started, so a refusal leaves the +target partition untouched. + +## Skew + +FOS probes `getversion.php?caps=1` for an `mcstreamid` token and strips the +header only when the server advertises it — a server that does not prepend +one must not have 128 bytes taken off its payload. The call is checked before +its answer is interpreted, so an unreachable server does not read as "no +header" (the GH-1266 shape, same as the `mclvm` probe beside it). + +The reverse pairing, a new server against an old FOS, feeds 128 bytes of +header into the decompressor and fails cleanly there. It is not silent +corruption, which is the property that matters, and in practice FOS is served +by the server it is talking to. + +## Known gaps + +- **This detects, it does not prevent.** A desynced client still ends its + deploy with an error rather than a correct image. Refusing a client that + tries to join a session already past its first stream would remove the + common trigger, and is not done here. +- **The 128-byte width is duplicated**, as a constant on the server and a + `dd` count here. Both are pinned by their own repo's checks, but nothing + compares them across the two. diff --git a/tests/checks/multicast-stream-identity.sh b/tests/checks/multicast-stream-identity.sh new file mode 100755 index 0000000..1820eab --- /dev/null +++ b/tests/checks/multicast-stream-identity.sh @@ -0,0 +1,156 @@ +#!/bin/bash +# +# Assertion harness for the multicast stream identity check. +# +# tests/checks/multicast-stream-identity.sh +# +# Why this exists. udpcast carries no metadata. The server chains one +# udp-sender per image file on a shared portbase, FOS opens one udp-receiver +# per file it expects, and the Nth receiver gets the Nth stream. Position is +# the only thing binding a stream to a partition. +# +# So a client that reboots mid-session -- or whose receiver opens after a +# sender has already stopped waiting for it -- lands one stream out of step, +# and every partition after that point is restored from the wrong image. +# partclone objects only when the target partition is SMALLER than the +# source; when it is larger the wrong filesystem is written and the deploy +# reports success. fogproject issue #1742 records a run where partition 3's +# image went onto partition 2 (caught, 105 MB target vs 135 MB source) after +# partition 2's had already gone onto partition 1 (not caught, 300 MB target). +# +# The server now prefixes every stream with a fixed 128-byte record naming +# the image file it carries, and checkStreamIdentity() refuses a stream that +# is not the one this partition asked for -- before the receiver's bytes +# reach partclone, so a refusal leaves the target untouched. +# +# Two halves are pinned here. The wiring is grepped, because the ordering of +# "open the receiver, read the header, only then start the restore" is what +# makes a refusal safe and there is nothing to execute that would show it. +# The naming rule is EXECUTED against the real function, because it is not +# uniform and the asymmetry is the whole point: +# +# d1p2.img flat partition asked for by name -> verbatim +# d1p1.img* split chunks asked for by glob -> stem +# sys.img.* ONE partition, N files -> stem +# rec.img.000 ONE partition, one file each -> verbatim +# +# Both sides reach the same string from their own layout, so neither has to +# re-derive the other's ordinal -- and any divergence is a refusal, never a +# misplaced filesystem. + +HERE="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +FUNCS="$HERE/../../Buildroot/board/FOG/FOS/rootfs_overlay/usr/share/fog/lib/funcs.sh" + +fails=0 +checked=0 + +ck() { + checked=$((checked + 1)) + if [[ $2 != 1 ]]; then + echo "FAIL $1" + fails=$((fails + 1)) + fi +} + +# --- the wiring ------------------------------------------------------- + +grep -q 'exec 9/tmp/pigz1' "$FUNCS" | head -1 | cut -d: -f1) ]]; then + ck "the header is checked before any payload is forwarded" 1 +else + ck "the header is checked before any payload is forwarded" 0 +fi + +grep -q 'mcstreamidcap == yes' "$FUNCS" +ck "the header is only stripped when the server advertises it" "$([[ $? -eq 0 ]] && echo 1 || echo 0)" + +# A server that does not prepend a header must keep its old, unstripped +# path, or 128 bytes of payload go missing. +grep -q 'udp-receiver --nokbd --portbase \$port --ttl 32 --mcast-rdv-address \$rdvaddress 2>/dev/null >/tmp/pigz1 &' "$FUNCS" +ck "an unadvertised server still gets the direct receiver" "$([[ $? -eq 0 ]] && echo 1 || echo 0)" + +# Same shape as the mclvm probe and fogproject GH-1266: an unreachable +# server must not read as "no header", which is a claim about the server's +# version drawn from a dead network. +grep -q 'if ! callServer "${web}service/getversion.php?caps=1"; then' "$FUNCS" +ck "the capability call is checked before its answer is read" "$([[ $? -eq 0 ]] && echo 1 || echo 0)" + +# --- the naming rule, executed ---------------------------------------- + +# The real function, lifted from the shipped file rather than restated. +tmp="$(mktemp -d)" +trap 'rm -rf "$tmp"' EXIT +awk '/^checkStreamIdentity\(\) \{/,/^\}/' "$FUNCS" > "$tmp/fn.sh" +grep -q 'FOGMC1' "$tmp/fn.sh" +ck "the function was extracted from the shipped file" "$([[ $? -eq 0 ]] && echo 1 || echo 0)" + +# $1 = name the server put in the header, $2 = what the client asks for, +# $3 = expected outcome (accept|refuse) +identity_case() { + local sent="$1" wanted="$2" expect="$3" rc=0 + printf 'FOGMC1 %-120s\n' "$sent" > "$tmp/stream" + printf 'PAYLOAD' >> "$tmp/stream" + ( + handleError() { exit 42; } + . "$tmp/fn.sh" + exec 9<"$tmp/stream" + checkStreamIdentity "$wanted" + ) >/dev/null 2>&1 || rc=$? + if [[ $expect == accept ]]; then + ck "$sent accepted for $wanted" "$([[ $rc -eq 0 ]] && echo 1 || echo 0)" + else + ck "$sent refused for $wanted" "$([[ $rc -eq 42 ]] && echo 1 || echo 0)" + fi +} + +identity_case 'd1p2.img' 'd1p2.img' accept +identity_case 'd1p2.img' '/net/dev/foo/d1p2.img' accept +identity_case 'd1p1.img' 'd1p1.img*' accept +identity_case 'sys.img' 'sys.img.*' accept +identity_case 'rec.img.000' 'rec.img.000' accept +# The desync from #1742: partition 3's stream offered to partition 2. +identity_case 'd1p3.img' 'd1p2.img' refuse +identity_case 'rec.img.001' 'rec.img.000' refuse +# A stem must not satisfy a request for one specific chunked partition of +# the legacy layout, or rec.img.000 and rec.img.001 become interchangeable. +identity_case 'rec.img' 'rec.img.000' refuse + +# An unheadered stream must be refused outright rather than treated as +# payload -- this is the new-client/old-server pairing. +printf 'partclone-image and then some binary rubbish' > "$tmp/stream" +rc=0 +( + handleError() { exit 42; } + . "$tmp/fn.sh" + exec 9<"$tmp/stream" + checkStreamIdentity 'd1p1.img' +) >/dev/null 2>&1 || rc=$? +ck "a stream with no header is refused" "$([[ $rc -eq 42 ]] && echo 1 || echo 0)" + +# Exactly 128 bytes come off the front: one byte more or less and the +# payload handed to partclone is corrupt. +printf 'FOGMC1 %-120s\n' 'd1p1.img' > "$tmp/stream" +printf 'PAYLOADSTARTSHERE' >> "$tmp/stream" +rest=$( + handleError() { exit 42; } + . "$tmp/fn.sh" + exec 9<"$tmp/stream" + checkStreamIdentity 'd1p1.img' >/dev/null 2>&1 + cat <&9 +) +ck "the header consumes exactly 128 bytes" "$([[ $rest == PAYLOADSTARTSHERE ]] && echo 1 || echo 0)" + +if [[ $fails -gt 0 ]]; then + echo "multicast-stream-identity: $fails of $checked checks failed" + exit 1 +fi +echo "multicast-stream-identity: $checked checks passed" +exit 0