diff --git a/docs/roadmap/first-run-restore.md b/docs/roadmap/first-run-restore.md index d2f8d46..7ca4a23 100644 --- a/docs/roadmap/first-run-restore.md +++ b/docs/roadmap/first-run-restore.md @@ -200,63 +200,69 @@ stoat's livekit publishes a fixed UDP range (50000–50100) that it advertises t clients and so cannot be re-rolled, and a desktop's `kdeconnectd` held 50016. Worth a fixed-range preflight check of its own; see §9. -### 3.5 — Open: ownership is not actually being reinstated +### 3.5 — Ownership was not actually being reinstated -Found by the same run and **not fixed** — it is a different subsystem (the -backup engine's ownership handling), it predates all of the above, and the -sensible fixes are security-relevant enough to want a decision first. - -`grafana` restored and then died with *"attempt to write a readonly database"*. -The snapshot records: +`grafana` restored and then died with *"attempt to write a readonly database"*, +repeatedly. The snapshot records: ``` -rw-r----- 231543 1002 /libreportal-containers/grafana/grafana_storage/grafana.db ``` -and what landed on disk was `1002:1002`. The owner was lost, so grafana — -running as 231543 — cannot write its own database at mode 0640. +and what landed was `1002:1002`. The owner was lost, so grafana — running as +231543 — could not write its own database at mode 0640. -The cause is exact. `_resticUsernsPrefix` runs restic under +Restore runs as the backup user, which has no `CAP_CHOWN`, so it reinstates +ownership through a user namespace. The prefix was: ``` unshare --map-root-user --map-users=231072:231072:65536 --map-groups=231072:231072:65536 ``` -and `unshare` accepts **one range per option**. So the backup user's own GID -(1002) is never mapped, while LibrePortal writes app data as -`:`. Verified directly: +and `unshare` accepts **one range per option**. So the backup user's own GID was +never mapped — while LibrePortal writes app data as `:`. +The group half of every such chown referred to an unmapped id, `lchown` returned +EINVAL, and the file kept the restoring user's ownership. Measured directly: -| target | result | -|---|---| -| `231543:231543` (both in the subuid range) | ✅ applied | -| `231543:1002` (the real case) | ❌ EINVAL → falls back to `1002:1002` | -| `1002:1002` (LibrePortal's own files) | ❌ EINVAL → `1002:1002`, which is correct anyway | +| target | old prefix | now | +|---|---|---| +| `231543:1002` (app data) | ❌ → `1002:1002` | ✅ `231543:1002` | +| `1002:1002` (LibrePortal's own) | ❌ → `1002:1002` (right anyway) | ❌ → `1002:1002` (right anyway) | +| `231072:231072` (container root) | ✅ | ✅ | -1626 of the 2086 failed chowns in one 13-app restore were grafana's. +Same shape as the rest of §3: restic reports the misses as `ignoring error …` +and **still exits 0**, and the caller forgave them as *"expected, they are +already owned correctly"*. 1626 of one 13-app restore's 2086 failed chowns were +grafana's. -`resticRestoreErrorsAreBenign` forgave all of them and reported *"expected, they -are already owned correctly"* — true only for the third row. That message is now -honest about what was not reinstated, which is the only part fixed here. +**Fixed** with `scripts/backup/engine/restic-userns-exec`, which uses +`newuidmap`/`newgidmap` — those write multi-range maps, which is exactly what +`unshare` cannot express: -Ways out, none free: +``` +uid: 0 <- caller inner root, or capabilities are dropped at exec + SUB.. <- SUB.. identity, so restic can name the stored uid +gid: caller <- caller identity — the group half of app-data chowns + SUB.. <- SUB.. identity +``` -1. **`newuidmap`/`newgidmap`** can write multi-range maps (`1002 1002 1` plus - `231072 231072 65536`), which is exactly what is needed — but it means - forking, mapping from outside, then continuing, rather than a one-line - prefix. `unshare --map-auto --map-current-user` does **not** work: it maps - the subuid range to low inner ids (container-style, `0 → 231072`), while - restic needs identity. -2. **Restore as real root via a helper**, the way `app-adopt` already moves - trees, with the target path validated against the storage registry. Simplest - and most reliable; widens what root does on the manager's say-so. -3. **Reapply ownership afterwards** from `restic ls -l`, via a root helper. - Contained, but a second pass over every file. +The caller's own UID is deliberately not identity-mapped: that slot is spent on +inner root. A file stored as `:` therefore fails its chown and +lands owned by the caller anyway, because that is who inner root is on the +outside — so the one case this cannot map is the one case that needs no mapping. -Recommendation: (1) if it can be kept small, else (2) with the same path -validation `app-adopt` uses. Worth noting that apps whose data is captured into -`.lp-backup/files` are rehydrated "via container" as the right uid and so -survive regardless — which is why bookstack, matrix, mattermost and nextcloud -came back healthy and grafana did not. +`unshare --map-auto --map-current-user` does **not** work as a shortcut: it maps +the subuid range to low inner ids (container-style, `0 → 231072`) while restic +needs identity. Tested. + +Measured on the live install, restoring grafana: failed chowns **1626 → 12** +(the 12 being the caller's own files, which are correct), `grafana.db` back to +`231543:1002`, and grafana up and writing. `scripts/dev/lp-userns-ownership-test` +pins all three rows of the table above. + +Anything unexpected — no subuid range, no `newuidmap`, a namespace that will not +start — falls back to running the command plainly, which is what happened before +any of this existed. ## 4. The password problem, stated plainly diff --git a/scripts/backup/engine/restic-userns-exec b/scripts/backup/engine/restic-userns-exec new file mode 100755 index 0000000..80be2f0 --- /dev/null +++ b/scripts/backup/engine/restic-userns-exec @@ -0,0 +1,105 @@ +#!/bin/bash +# Run a command in a user namespace whose id maps let it restore file ownership. +# +# restic-userns-exec restic restore --target … +# +# Restore runs as the backup user, which has no CAP_CHOWN, so it cannot put back +# the uid a file had when it was backed up. A user namespace solves that — but +# only if every id involved is mapped, and LibrePortal needs three mappings at +# once while `unshare` accepts exactly ONE range per option. +# +# That limit is why the previous one-line prefix silently did not work: +# +# unshare --map-root-user --map-users=SUB:SUB:N --map-groups=SUB:SUB:N +# +# mapped root and the subuid range, but never the backup user's own GID. App +# data is written as :, so the group half of every +# chown referred to an unmapped id, lchown returned EINVAL, and the file kept +# the restoring user's ownership. Grafana's database came back 1002:1002 instead +# of 231543:1002 and the app died with "attempt to write a readonly database" — +# under a restore that reported success. +# +# newuidmap/newgidmap can write multi-range maps, so the maps become: +# +# uid: 0 <- caller (inner root: without this, capabilities are +# dropped at exec and chown is not permitted) +# SUB.. <- SUB.. (identity, so restic can ask for the stored +# container uid by its real number) +# gid: caller <- caller (identity: the group half of app-data chowns) +# SUB.. <- SUB.. (identity) +# +# The caller's own UID is deliberately NOT identity-mapped: it is spent on inner +# root. A file stored as : therefore fails its chown — and lands +# owned by the caller anyway, because that is who inner root is outside. So the +# one case this cannot map is the one case that needs no mapping. +# +# Anything unexpected — no subuid range, no newuidmap, a namespace that will not +# start — falls back to running the command plainly, which is what happened +# before this existed. +set -u + +(( $# )) || { echo "restic-userns-exec: no command given" >&2; exit 2; } + +TMPDIR_MADE="" +_plain() { [[ -n "$TMPDIR_MADE" ]] && rm -rf "$TMPDIR_MADE"; exec "$@"; } + +for _bin in unshare newuidmap newgidmap; do + command -v "$_bin" >/dev/null 2>&1 || _plain "$@" +done + +_usr=$(id -un 2>/dev/null) || _plain "$@" +_u=$(id -u); _g=$(id -g) + +_uline=$(grep "^${_usr}:" /etc/subuid 2>/dev/null | head -1) +_gline=$(grep "^${_usr}:" /etc/subgid 2>/dev/null | head -1) +[[ -n "$_uline" && -n "$_gline" ]] || _plain "$@" + +_ustart="${_uline#*:}"; _ustart="${_ustart%%:*}"; _ucount="${_uline##*:}" +_gstart="${_gline#*:}"; _gstart="${_gstart%%:*}"; _gcount="${_gline##*:}" +[[ "$_ustart" =~ ^[0-9]+$ && "$_ucount" =~ ^[0-9]+$ ]] || _plain "$@" +[[ "$_gstart" =~ ^[0-9]+$ && "$_gcount" =~ ^[0-9]+$ ]] || _plain "$@" +(( _ucount > 0 && _gcount > 0 )) || _plain "$@" + +# The caller's own id has to sit outside its sub-range, or the two entries would +# overlap on the outer side and the kernel rejects the whole map. +(( _u < _ustart || _u >= _ustart + _ucount )) || _plain "$@" +(( _g < _gstart || _g >= _gstart + _gcount )) || _plain "$@" + +TMPDIR_MADE=$(mktemp -d "${TMPDIR:-/tmp}/lp-userns.XXXXXX") || { TMPDIR_MADE=""; _plain "$@"; } +_fifo="$TMPDIR_MADE/gate" +mkfifo -m 600 "$_fifo" 2>/dev/null || _plain "$@" + +# Opened here and inherited there, rather than opened by path in the child: +# until the map is written the child's uid is unmapped, so to the filesystem it +# is nobody and could not open its own gate. +exec 9<>"$_fifo" + +unshare --user bash -c 'read -r _ <&9; exec "$@"' _ "$@" & +_child=$! + +# The namespace exists before unshare execs, but only just — retry briefly +# rather than assume. A write that succeeds cannot be repeated, so stop there. +_mapped=0 +for _ in $(seq 1 100); do + kill -0 "$_child" 2>/dev/null || break + if newuidmap "$_child" 0 "$_u" 1 "$_ustart" "$_ustart" "$_ucount" 2>/dev/null; then + _mapped=1; break + fi + sleep 0.02 +done + +if (( _mapped )); then + newgidmap "$_child" "$_g" "$_g" 1 "$_gstart" "$_gstart" "$_gcount" 2>/dev/null || _mapped=0 +fi + +if (( ! _mapped )); then + kill "$_child" 2>/dev/null + wait "$_child" 2>/dev/null + _plain "$@" +fi + +printf 'go\n' >&9 +wait "$_child" +_rc=$? +rm -rf "$TMPDIR_MADE" +exit "$_rc" diff --git a/scripts/backup/engine/restic_restore.sh b/scripts/backup/engine/restic_restore.sh index 9507f79..7344027 100644 --- a/scripts/backup/engine/restic_restore.sh +++ b/scripts/backup/engine/restic_restore.sh @@ -28,23 +28,23 @@ _resticUsernsPrefix() { local usr="${docker_install_user:-dockerinstall}" - command -v unshare >/dev/null 2>&1 || return 0 - local uline gline ustart ucount gstart gcount - uline=$(grep "^${usr}:" /etc/subuid 2>/dev/null | head -1) - gline=$(grep "^${usr}:" /etc/subgid 2>/dev/null | head -1) + # The mapping itself lives in restic-userns-exec, because it needs three id + # ranges at once and `unshare` takes one per option — see that file. The + # checks here only decide whether to reach for it at all; it re-checks and + # falls back to running the command plainly if anything is missing. + local helper="${install_scripts_dir%/}/backup/engine/restic-userns-exec" + [[ -r "$helper" ]] || return 0 + command -v unshare >/dev/null 2>&1 || return 0 + command -v newuidmap >/dev/null 2>&1 || return 0 + command -v newgidmap >/dev/null 2>&1 || return 0 + # No subuid range (rooted mode, or a hand-rolled account) — nothing to map, # so leave the call exactly as it was rather than guess. - [[ -n "$uline" && -n "$gline" ]] || return 0 + grep -q "^${usr}:" /etc/subuid 2>/dev/null || return 0 + grep -q "^${usr}:" /etc/subgid 2>/dev/null || return 0 - ustart="${uline#*:}"; ustart="${ustart%%:*}"; ucount="${uline##*:}" - gstart="${gline#*:}"; gstart="${gstart%%:*}"; gcount="${gline##*:}" - [[ "$ustart" =~ ^[0-9]+$ && "$ucount" =~ ^[0-9]+$ ]] || return 0 - [[ "$gstart" =~ ^[0-9]+$ && "$gcount" =~ ^[0-9]+$ ]] || return 0 - - printf '%s\n' unshare --map-root-user \ - "--map-users=${ustart}:${ustart}:${ucount}" \ - "--map-groups=${gstart}:${gstart}:${gcount}" + printf '%s\n' bash "$helper" } # True when every error restic reported is the expected "cannot map the backup @@ -93,30 +93,21 @@ resticRestoreSnapshot() printf '%s\n' "$out" # restic exits non-zero for un-mappable-uid lchowns even though the file - # CONTENTS landed. Forgive only that case — but do not pretend it is - # nothing, which is what this used to do. + # CONTENTS landed. Forgive only that case. # - # It said "expected, they are already owned correctly". That holds for - # LibrePortal's own files (owner = the backup user, which is who restic runs - # as anyway). It does NOT hold for container-owned data, and the namespace - # this runs in cannot currently map those: unshare takes a single range per - # option, so _resticUsernsPrefix maps the subuid range and root but not the - # backup user's own GID — and LibrePortal writes app data as - # :. Every such chown fails and the file falls - # back to :. + # With the namespace mapping fixed (see restic-userns-exec), the only id + # that is still unmappable is the restoring user's own — its slot is spent + # on inner root — and a file stored as : lands owned by the + # caller regardless, because that is who inner root is on the outside. So + # these really are correct, which is what the message used to claim before + # the mapping worked and container-owned data was quietly losing its owner. # - # Observed: grafana's grafana.db is recorded in the snapshot as 231543:1002 - # and restored as 1002:1002. At mode 0640 the grafana process — running as - # 231543 — then cannot write it, and the app dies with "attempt to write a - # readonly database". The restore reported success. - # - # So: still do not fail the restore (the data is there and some apps are - # rehydrated by other means), but say plainly what was not reinstated. + # Still report the count: if this number is large the mapping has stopped + # working again, and the symptom is an app that cannot write its own data. if [[ $rc -ne 0 && ${#ns_prefix[@]} -gt 0 ]] && resticRestoreErrorsAreBenign "$out"; then local _lch _lch=$(printf '%s\n' "$out" | grep -cE "^ignoring error for .*lchown ") - isNotice "Restore could not reinstate ownership on ${_lch} file(s); they now belong to ${docker_install_user:-the backup user}." - isNotice "LibrePortal's own files are correct that way. Container-owned data is NOT — an app may fail to write (e.g. a read-only database). Check the app after it starts." + isNotice "Ownership warnings on ${_lch} file(s) owned by ${docker_install_user:-the backup user} — expected; they are restored correctly." rc=0 fi diff --git a/scripts/dev/lp-userns-ownership-test b/scripts/dev/lp-userns-ownership-test new file mode 100755 index 0000000..0076920 --- /dev/null +++ b/scripts/dev/lp-userns-ownership-test @@ -0,0 +1,68 @@ +#!/bin/bash +# Can a restore actually put back the uid a file had when it was backed up? +# +# scripts/dev/lp-userns-ownership-test # as the backup user +# sudo -u dockerinstall -H scripts/dev/lp-userns-ownership-test +# +# Restore runs unprivileged, so it reinstates ownership through a user namespace +# (scripts/backup/engine/restic-userns-exec). The previous one-line `unshare` +# prefix mapped root and the subuid range but not the backup user's own GID — +# and app data is stored as :, so the group half of +# every chown was unmapped, lchown returned EINVAL, and the file silently kept +# the restoring user's ownership. +# +# Nothing failed. restic reported the misses as "ignoring error …" and exited 0, +# and the caller forgave them as expected. The only symptom was grafana coming +# back as 1002:1002 instead of 231543:1002 and dying with "attempt to write a +# readonly database". +# +# So the case that matters is the middle one: a container uid with the backup +# user as its group. + +REPO="$(cd "$(dirname "$0")/../.." && pwd)" +HELPER="$REPO/scripts/backup/engine/restic-userns-exec" +fail=0 +chk(){ if [[ "$2" == "$3" ]]; then echo " ok $1"; else echo " FAIL $1: got '$2' want '$3'"; fail=1; fi; } + +[[ -r "$HELPER" ]] || { echo " FAIL helper not found at $HELPER"; exit 1; } +for b in unshare newuidmap newgidmap; do + command -v "$b" >/dev/null 2>&1 || { echo " SKIP $b not installed"; exit 0; } +done + +usr=$(id -un); u=$(id -u); g=$(id -g) +uline=$(grep "^${usr}:" /etc/subuid 2>/dev/null | head -1) +[[ -n "$uline" ]] || { echo " SKIP no subuid range for $usr"; exit 0; } +sub="${uline#*:}"; sub="${sub%%:*}" +CUID=$(( sub + 471 )) # a container uid, the way rootless docker maps one + +BASE=$(mktemp -d "${TMPDIR:-/tmp}/lp-userns-test-XXXXXX") || exit 1 +trap 'rm -rf "$BASE"' EXIT +echo "--- $usr ($u:$g), subuid base $sub, container uid $CUID ---" + +# Ask the helper to set the three ownerships a restore actually encounters. +bash "$HELPER" bash -c ' + : > "$1/app"; : > "$1/own"; : > "$1/croot" + chown '"$CUID"':'"$g"' "$1/app" 2>/dev/null + chown '"$u"':'"$g"' "$1/own" 2>/dev/null + chown '"$sub"':'"$sub"' "$1/croot" 2>/dev/null + exit 0' _ "$BASE" >/dev/null 2>&1 + +echo "--- container-owned app data (the grafana case) ---" +chk "uid:gid" "$(stat -c '%u:%g' "$BASE/app")" "$CUID:$g" + +echo "--- LibrePortal's own files ---" +# The caller's uid is spent on inner root, so this chown cannot succeed — and +# does not need to: inner root IS the caller on the outside. +chk "uid:gid" "$(stat -c '%u:%g' "$BASE/own")" "$u:$g" + +echo "--- container root ---" +chk "uid:gid" "$(stat -c '%u:%g' "$BASE/croot")" "$sub:$sub" + +echo "--- exit status and stdout are passed through ---" +out=$(bash "$HELPER" bash -c 'echo hello; exit 7' 2>/dev/null); rc=$? +chk "stdout" "$out" "hello" +chk "status" "$rc" "7" + +echo "" +if (( fail )); then echo "FAILED"; exit 1; fi +echo "All ownership-mapping checks passed."