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