From c34d20ca14faa75b90580124790956f5c061db82 Mon Sep 17 00:00:00 2001 From: Afonso Oliveira Date: Tue, 25 Aug 2026 18:39:00 +0100 Subject: [PATCH 1/5] [Security] Pin Windows VM mounts behind a root boundary --- bin/omarchy-windows-vm | 360 +++++++++++++++++++----- manual/28-windows-vm.md | 4 +- test/shell.d/windows-vm-compose-test.sh | 82 +++--- 3 files changed, 343 insertions(+), 103 deletions(-) diff --git a/bin/omarchy-windows-vm b/bin/omarchy-windows-vm index 1487cb21..c22c4815 100755 --- a/bin/omarchy-windows-vm +++ b/bin/omarchy-windows-vm @@ -67,9 +67,14 @@ priv_target() { priv() { local action="$1" shift - if [[ $action != write_compose ]] && ! docker_needs_sudo; then - "__priv_$action" "$@" - return + if [[ $action != write_compose && $action != remove ]] && ! docker_needs_sudo; then + # Existing installs used bind sources in $HOME. One privileged run is + # needed to atomically move those directories below the root-owned mount + # boundary, even when the caller can otherwise reach Docker directly. + if [[ $action != up && $action != up_wait ]] || ! compose_needs_mount_migration; then + "__priv_$action" "$@" + return + fi fi local target target=$(priv_target) || { @@ -88,16 +93,6 @@ valid_cores() { [[ $1 =~ ^[0-9]{1,2}$ ]] && ((10#$1 >= 1)); } valid_disk() { [[ $1 =~ ^[0-9]{1,4}G$ ]]; } valid_username() { [[ $1 =~ ^[A-Za-z0-9_-]{1,20}$ ]]; } valid_tz() { [[ $1 =~ ^[A-Za-z0-9_/.+-]{1,64}$ ]]; } -valid_path() { - [[ $1 =~ ^/[A-Za-z0-9._/-]+$ ]] || return 1 - # Reject non-normalized paths: a . or .. component canonicalizes at mount time - # (e.g. /./ or /a/../ -> /), which would bind-mount a sensitive directory — - # host / included — into the guest. Volumes must be given already-normalized. - case "$1" in - *//* | */./* | */../* | */. | */..) return 1 ;; - esac - return 0 -} valid_password() { [[ $1 =~ ^[[:print:]]{1,64}$ ]]; } # The only privileged sub-actions __priv may dispatch. A bash command name @@ -113,13 +108,207 @@ valid_priv_action() { # --- privileged actions (run as root via pkexec, or directly when sudoless) --- +# Resolve the account that authorized pkexec. Never trust HOME or a caller- +# supplied mount path in the privileged process: pkexec can reset HOME, and the +# old path arguments were the source of an arbitrary host bind-mount primitive. +resolve_caller() { + local entry canonical parent owner mode + + if ((EUID == 0)); then + [[ ${PKEXEC_UID:-} =~ ^[0-9]+$ ]] && ((10#$PKEXEC_UID > 0)) || { + echo "omarchy-windows-vm: cannot identify the user who authorized this action" >&2 + return 1 + } + CALLER_UID=$((10#$PKEXEC_UID)) + else + CALLER_UID=$(id -u) + fi + + entry=$(getent passwd "$CALLER_UID") || { + echo "omarchy-windows-vm: no account exists for uid $CALLER_UID" >&2 + return 1 + } + IFS=: read -r _ _ _ CALLER_GID _ CALLER_HOME _ <<<"$entry" + [[ $CALLER_GID =~ ^[0-9]+$ && $CALLER_HOME == /* && -d $CALLER_HOME ]] || { + echo "omarchy-windows-vm: invalid home directory for uid $CALLER_UID" >&2 + return 1 + } + + # A direct, non-root development invocation with a non-standard runtime has + # no privilege boundary and may use its current HOME (which also keeps these + # functions testable). Production always uses the account database value. + if ((EUID != 0)) && [[ $RUNTIME_DIR != /var/lib/omarchy/windows ]]; then + CALLER_HOME=${HOME:-$CALLER_HOME} + fi + canonical=$(realpath -e -- "$CALLER_HOME" 2>/dev/null) || return 1 + [[ $canonical == "$CALLER_HOME" ]] || { + echo "omarchy-windows-vm: refusing a home directory reached through a symlink" >&2 + return 1 + } + + if ((EUID == 0)); then + owner=$(stat -Lc '%u' "$CALLER_HOME") || return 1 + [[ $owner == "$CALLER_UID" ]] || { + echo "omarchy-windows-vm: caller does not own $CALLER_HOME" >&2 + return 1 + } + # The user must not be able to rename or replace their home while root is + # moving the legacy data entry out of it. + parent=$(dirname -- "$CALLER_HOME") + while :; do + owner=$(stat -Lc '%u' "$parent") || return 1 + mode=$(stat -Lc '%a' "$parent") || return 1 + [[ $owner == 0 ]] && ! ((8#$mode & 022)) || { + echo "omarchy-windows-vm: unsafe writable parent in home path: $parent" >&2 + return 1 + } + [[ $parent == / ]] && break + parent=$(dirname -- "$parent") + done + fi + + # Keep the potentially large disk on the same filesystem as the user's home, + # but outside that user-writable directory. The already-validated home parent + # is root-owned, so this sibling tree provides a stable rename boundary. + MOUNT_ROOT="$(dirname -- "$CALLER_HOME")/.omarchy-windows" + USERS_DIR="$MOUNT_ROOT/users" + CALLER_DATA_ROOT="$USERS_DIR/$CALLER_UID" + EXPECTED_STORAGE="$CALLER_DATA_ROOT/storage" + EXPECTED_SHARED="$CALLER_DATA_ROOT/shared" + LEGACY_STORAGE="$CALLER_HOME/.windows" + LEGACY_SHARED="$CALLER_HOME/Windows" +} + +boundary_owner() { + # Production boundaries remain root-owned even when a docker-group user runs + # the read-only bring-up checks directly. A non-standard runtime is supported + # only for unprivileged tests/development and is owned by that caller. + if ((EUID == 0)) || [[ $RUNTIME_DIR == /var/lib/omarchy/windows ]]; then + printf '0' + else + printf '%s' "$CALLER_UID" + fi +} + +assert_boundary_dir() { + local path="$1" expected_owner="$2" owner mode canonical + [[ -d $path && ! -L $path ]] || return 1 + canonical=$(realpath -e -- "$path" 2>/dev/null) || return 1 + [[ $canonical == "$path" ]] || return 1 + owner=$(stat -Lc '%u' "$path") || return 1 + mode=$(stat -Lc '%a' "$path") || return 1 + [[ $owner == "$expected_owner" ]] && ! ((8#$mode & 022)) +} + +prepare_runtime_tree() { + local owner probe + owner=$(boundary_owner) + if ((EUID == 0)); then + [[ $RUNTIME_DIR == /var/lib/omarchy/windows ]] || { + echo "omarchy-windows-vm: refusing a non-standard privileged runtime path" >&2 + return 1 + } + # Check the nearest existing ancestor before mkdir can follow anything. + # Every new component is then created by root and checked again below. + probe=$RUNTIME_DIR + while [[ ! -e $probe && ! -L $probe ]]; do probe=$(dirname -- "$probe"); done + while :; do + assert_boundary_dir "$probe" 0 || { + echo "omarchy-windows-vm: unsafe runtime parent: $probe" >&2 + return 1 + } + [[ $probe == / ]] && break + probe=$(dirname -- "$probe") + done + if [[ -e $MOUNT_ROOT || -L $MOUNT_ROOT ]]; then + assert_boundary_dir "$MOUNT_ROOT" 0 || { + echo "omarchy-windows-vm: unsafe mount root: $MOUNT_ROOT" >&2 + return 1 + } + fi + fi + mkdir -p -- "$RUNTIME_DIR" "$MOUNT_ROOT" "$USERS_DIR" "$CALLER_DATA_ROOT" + chmod 0755 "$RUNTIME_DIR" "$MOUNT_ROOT" "$USERS_DIR" "$CALLER_DATA_ROOT" + if ((EUID == 0)); then + chown root:root "$RUNTIME_DIR" "$MOUNT_ROOT" "$USERS_DIR" "$CALLER_DATA_ROOT" + fi + assert_boundary_dir "$RUNTIME_DIR" "$owner" && + assert_boundary_dir "$MOUNT_ROOT" "$owner" && + assert_boundary_dir "$USERS_DIR" "$owner" && + assert_boundary_dir "$CALLER_DATA_ROOT" "$owner" || { + echo "omarchy-windows-vm: unsafe VM mount boundary" >&2 + return 1 + } +} + + # Move an existing home entry first, then inspect the pinned object below the +# root-owned parent. This closes the check/use gap where an attacker could swap +# a checked home directory for a symlink before Docker resolved it. +prepare_mount_leaf() { + local legacy="$1" stable="$2" rejected source_dev target_dev + + if [[ ! -e $stable && ! -L $stable ]]; then + if [[ -e $legacy || -L $legacy ]]; then + # rename(2) pins the exact directory entry the caller presented. GNU mv + # falls back to a privileged recursive copy across filesystems, which + # would reopen the source path and reintroduce the race, so fail closed in + # that uncommon layout instead of copying as root. + source_dev=$(stat -c '%d' -- "$legacy") || return 1 + target_dev=$(stat -Lc '%d' -- "$CALLER_DATA_ROOT") || return 1 + [[ $source_dev == "$target_dev" ]] || { + echo "omarchy-windows-vm: cannot safely migrate $legacy across filesystems" >&2 + echo "Move it onto the filesystem containing $MOUNT_ROOT, then retry." >&2 + return 1 + } + mv --no-copy -T -- "$legacy" "$stable" || return 1 + if [[ ! -d $stable || -L $stable ]]; then + rejected="$CALLER_DATA_ROOT/rejected-$(basename -- "$stable")-$$" + mv --no-copy -T -- "$stable" "$rejected" 2>/dev/null || true + echo "omarchy-windows-vm: refusing non-directory VM data entry at $legacy" >&2 + return 1 + fi + else + install -d -m 0700 "$stable" + fi + fi + + [[ -d $stable && ! -L $stable ]] && [[ $(realpath -e -- "$stable" 2>/dev/null) == "$stable" ]] || { + echo "omarchy-windows-vm: unsafe VM data directory: $stable" >&2 + return 1 + } + if ((EUID == 0)); then + chown "$CALLER_UID:$CALLER_GID" "$stable" + fi + + if [[ -L $legacy ]]; then + [[ $(realpath -e -- "$legacy" 2>/dev/null) == "$stable" ]] || { + echo "omarchy-windows-vm: $legacy does not point to its protected mount anchor" >&2 + return 1 + } + elif [[ -e $legacy ]]; then + echo "omarchy-windows-vm: refusing to replace existing data at $legacy" >&2 + return 1 + else + ln -s -- "$stable" "$legacy" || return 1 + if ((EUID == 0)); then + chown -h "$CALLER_UID:$CALLER_GID" "$legacy" + fi + fi +} + +prepare_caller_mounts() { + resolve_caller && prepare_runtime_tree && + prepare_mount_leaf "$LEGACY_STORAGE" "$EXPECTED_STORAGE" && + prepare_mount_leaf "$LEGACY_SHARED" "$EXPECTED_SHARED" +} + # Reads KEY=VALUE lines on stdin, re-validates every field, and writes the # compose atomically as root. Re-validation here is the security boundary: the # writer refuses rather than emit a compose an attacker could have influenced. # Only these fixed keys are honored; image, container name, devices, caps, and # port bindings are hard-coded and never taken from input. __priv_write_compose() { - local ram cores disk username password tz storage shared key value + local ram cores disk username password tz key value while IFS='=' read -r key value; do case "$key" in @@ -129,8 +318,6 @@ __priv_write_compose() { USERNAME) username="$value" ;; PASSWORD) password="$value" ;; TZ) tz="$value" ;; - STORAGE) storage="$value" ;; - SHARED) shared="$value" ;; esac done @@ -140,8 +327,7 @@ __priv_write_compose() { valid_username "$username" || { echo "invalid username: $username" >&2; exit 2; } valid_password "$password" || { echo "invalid password" >&2; exit 2; } valid_tz "$tz" || tz="UTC" - valid_path "$storage" || { echo "invalid storage path: $storage" >&2; exit 2; } - valid_path "$shared" || { echo "invalid shared path: $shared" >&2; exit 2; } + prepare_caller_mounts || exit 2 # Neutralize anything in the password that could be misread when the compose # is parsed. Two layers apply, in this order at parse time: docker compose @@ -154,10 +340,6 @@ __priv_write_compose() { esc_password=${esc_password//\"/\\\"} esc_password=${esc_password//\$/\$\$} - mkdir -p "$RUNTIME_DIR" - chmod 0755 "$RUNTIME_DIR" 2>/dev/null || true - chown root:root "$RUNTIME_DIR" 2>/dev/null || true - local tmp tmp=$(mktemp "$RUNTIME_DIR/.compose.XXXXXX") cat >"$tmp" <"$tmp" || { rm -f "$tmp"; return 1; } + chmod 0640 "$tmp" + if ((EUID == 0)); then + chown root:docker "$tmp" 2>/dev/null || chown root:root "$tmp" + fi + mv -f -- "$tmp" "$COMPOSE_FILE" +} + +assert_compose_trusted() { + local owner expected mode + [[ -f $COMPOSE_FILE && ! -L $COMPOSE_FILE ]] || return 1 + owner=$(stat -Lc '%u' "$COMPOSE_FILE") || return 1 + mode=$(stat -Lc '%a' "$COMPOSE_FILE") || return 1 + expected=$(boundary_owner) + [[ $owner == "$expected" ]] && ! ((8#$mode & 022)) +} + assert_mounts_safe() { - local mnt src real - for mnt in /storage /shared; do - src=$(get_mount_source "$mnt") - [[ -n $src ]] || { - echo "omarchy-windows-vm: missing $mnt mount source in the compose" >&2 + local storage shared owner + resolve_caller || return 1 + assert_compose_trusted || { + echo "omarchy-windows-vm: refusing an untrusted compose file" >&2 + return 1 + } + storage=$(get_mount_source /storage) + shared=$(get_mount_source /shared) + + if [[ $storage == "$LEGACY_STORAGE" && $shared == "$LEGACY_SHARED" ]]; then + ((EUID == 0)) || { + echo "omarchy-windows-vm: legacy VM data needs an authorized migration" >&2 return 1 } - if [[ -L $src ]]; then - echo "omarchy-windows-vm: refusing to start — $src is a symlink; the VM mount source must be a real directory" >&2 - return 1 - fi - if [[ -e $src ]]; then - [[ -d $src ]] || { - echo "omarchy-windows-vm: refusing to start — $src is not a directory" >&2 - return 1 - } - real=$(realpath "$src" 2>/dev/null) - [[ $real == "$src" ]] || { - echo "omarchy-windows-vm: refusing to start — $src resolves through a symlink to $real" >&2 - return 1 - } - fi - done + prepare_caller_mounts || return 1 + rewrite_compose_mounts || return 1 + storage=$EXPECTED_STORAGE + shared=$EXPECTED_SHARED + fi + + [[ $storage == "$EXPECTED_STORAGE" && $shared == "$EXPECTED_SHARED" ]] || { + echo "omarchy-windows-vm: refusing unexpected host paths in the compose" >&2 + return 1 + } + owner=$(boundary_owner) + assert_boundary_dir "$RUNTIME_DIR" "$owner" && + assert_boundary_dir "$MOUNT_ROOT" "$owner" && + assert_boundary_dir "$USERS_DIR" "$owner" && + assert_boundary_dir "$CALLER_DATA_ROOT" "$owner" && + [[ -d $EXPECTED_STORAGE && ! -L $EXPECTED_STORAGE ]] && + [[ -d $EXPECTED_SHARED && ! -L $EXPECTED_SHARED ]] && + [[ $(realpath -e -- "$EXPECTED_STORAGE" 2>/dev/null) == "$EXPECTED_STORAGE" ]] && + [[ $(realpath -e -- "$EXPECTED_SHARED" 2>/dev/null) == "$EXPECTED_SHARED" ]] || { + echo "omarchy-windows-vm: refusing an unsafe VM mount anchor" >&2 + return 1 + } } __priv_up() { assert_mounts_safe && dc up -d; } @@ -273,19 +495,24 @@ __priv_up_wait() { __priv_status() { docker inspect --format='{{.State.Status}}' "$CONTAINER" 2>/dev/null || true; } __priv_remove() { + resolve_caller || return 1 dc down 2>/dev/null || true docker rmi "$IMAGE" 2>/dev/null || true rm -f "$COMPOSE_FILE" - rmdir "$RUNTIME_DIR" 2>/dev/null || true + # Shared files intentionally survive removal. The storage leaf cannot be + # swapped by the user because its parent is the protected boundary. + if [[ $EXPECTED_STORAGE == "$USERS_DIR/$CALLER_UID/storage" && -d $EXPECTED_STORAGE && ! -L $EXPECTED_STORAGE ]]; then + rm -rf --one-file-system -- "$EXPECTED_STORAGE" + fi } # --- config helpers ---------------------------------------------------------- # Feed the collected settings to the elevated writer. write_compose() { - local ram="$1" cores="$2" disk="$3" username="$4" password="$5" tz="$6" storage="$7" shared="$8" - printf 'RAM=%s\nCORES=%s\nDISK=%s\nUSERNAME=%s\nPASSWORD=%s\nTZ=%s\nSTORAGE=%s\nSHARED=%s\n' \ - "$ram" "$cores" "$disk" "$username" "$password" "$tz" "$storage" "$shared" | + local ram="$1" cores="$2" disk="$3" username="$4" password="$5" tz="$6" + printf 'RAM=%s\nCORES=%s\nDISK=%s\nUSERNAME=%s\nPASSWORD=%s\nTZ=%s\n' \ + "$ram" "$cores" "$disk" "$username" "$password" "$tz" | priv write_compose } @@ -340,22 +567,17 @@ migrate_legacy_compose() { [[ -f $LEGACY_COMPOSE_FILE ]] || return 1 echo "Migrating Windows VM configuration to $COMPOSE_FILE ..." - local ram cores disk username password tz storage shared + local ram cores disk username password tz ram=$(read_compose_value RAM_SIZE "$LEGACY_COMPOSE_FILE") cores=$(read_compose_value CPU_CORES "$LEGACY_COMPOSE_FILE") disk=$(read_compose_value DISK_SIZE "$LEGACY_COMPOSE_FILE") username=$(read_compose_value USERNAME "$LEGACY_COMPOSE_FILE") password=$(read_compose_value PASSWORD "$LEGACY_COMPOSE_FILE") tz=$(read_compose_value TZ "$LEGACY_COMPOSE_FILE") - # The VM's data always lived in the user's own ~/.windows and ~/Windows; the - # old compose only ever recorded those. Reconstruct them from $HOME (trusted — - # this runs as the user) rather than reading host paths back from a file a - # rogue process could have rewritten to bind-mount, say, / into the guest. - storage="$HOME/.windows" - shared="$HOME/Windows" - [[ -z $tz ]] && tz="UTC" - if ! write_compose "$ram" "$cores" "$disk" "$username" "$password" "$tz" "$storage" "$shared"; then + # The elevated writer derives both mount anchors from the authenticated uid; + # it never consumes volume paths from this user-owned legacy file. + if ! write_compose "$ram" "$cores" "$disk" "$username" "$password" "$tz"; then echo "Could not migrate the existing configuration automatically." >&2 echo "Re-run: omarchy-windows-vm install" >&2 return 1 @@ -404,7 +626,6 @@ install_windows() { omarchy-pkg-add freerdp openbsd-netcat gum - mkdir -p "$HOME/.windows" mkdir -p "$HOME/.local/share/applications" cat </dev/null @@ -540,15 +761,14 @@ EOF exit 1 fi - mkdir -p "$HOME/Windows" - local tz tz=$(timedatectl show -p Timezone --value 2>/dev/null || echo UTC) # Write the root-owned compose from the validated settings (one prompt if - # sudoless Docker is off), then bring the stack up. + # sudoless Docker is off). The writer creates protected storage/shared mount + # anchors and leaves the familiar home entries as symlinks to them. write_compose "$SELECTED_RAM" "$SELECTED_CORES" "$SELECTED_DISK" \ - "$USERNAME" "$PASSWORD" "$tz" "$HOME/.windows" "$HOME/Windows" || { + "$USERNAME" "$PASSWORD" "$tz" || { echo "❌ Failed to write the Windows VM configuration." exit 1 } diff --git a/manual/28-windows-vm.md b/manual/28-windows-vm.md index 9da7786f..e12a74d8 100644 --- a/manual/28-windows-vm.md +++ b/manual/28-windows-vm.md @@ -26,7 +26,9 @@ omarchy windows vm launch # start and connect ## Sharing files -The directory `~/Windows` in your home directory is automatically shared with the VM. Put files there if you want them accessible to Windows. The VM has no access to any other part of your file system, so you're safe from anything nasty on the Windows side. Its own virtual disk lives in `~/.windows`. +The directory `~/Windows` in your home directory is automatically shared with the VM. Put files there if you want them accessible to Windows. The VM has no access to any other part of your file system, so you're safe from anything nasty on the Windows side. Its own virtual disk is available at `~/.windows`. + +Those familiar home paths are links to per-user mount anchors in a root-owned `.omarchy-windows` directory beside your home directory. Keeping the anchors on the home filesystem preserves the expected disk location, while their protected parent prevents another process running as you from swapping a checked directory for a symlink while the privileged VM is starting. The VM's ports are bound to localhost only, so nothing on your network can reach the Windows machine. diff --git a/test/shell.d/windows-vm-compose-test.sh b/test/shell.d/windows-vm-compose-test.sh index 0aa353f2..953f3403 100644 --- a/test/shell.d/windows-vm-compose-test.sh +++ b/test/shell.d/windows-vm-compose-test.sh @@ -13,39 +13,47 @@ source "$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")" && pwd)/base-test.sh" TMPDIR=$(mktemp -d) trap 'rm -rf "$TMPDIR"' EXIT export OMARCHY_WINDOWS_DIR="$TMPDIR/win" +export HOME="$TMPDIR/home" +mkdir -p "$HOME" # Source the command's functions; the dispatcher just prints usage for "help". set -- help source "$ROOT/bin/omarchy-windows-vm" >/dev/null 2>&1 COMPOSE="$OMARCHY_WINDOWS_DIR/docker-compose.yml" -write() { # RAM CORES DISK USER PASS TZ STORAGE SHARED - printf 'RAM=%s\nCORES=%s\nDISK=%s\nUSERNAME=%s\nPASSWORD=%s\nTZ=%s\nSTORAGE=%s\nSHARED=%s\n' \ +write() { # RAM CORES DISK USER PASS TZ + printf 'RAM=%s\nCORES=%s\nDISK=%s\nUSERNAME=%s\nPASSWORD=%s\nTZ=%s\n' \ "$@" | __priv_write_compose } # --- valid compose, with the dangerous bits pinned and unreachable by input --- rm -f "$COMPOSE" -write 4G 2 64G alice 's3cret' Europe/Copenhagen /home/alice/.windows /home/alice/Windows +write 4G 2 64G alice 's3cret' Europe/Copenhagen +resolve_caller [[ -f $COMPOSE ]] || fail "writer produced a compose file" grep -q 'image: dockurr/windows' "$COMPOSE" || fail "image is pinned" grep -q -- '- NET_ADMIN' "$COMPOSE" || fail "cap_add is pinned" -grep -q -- '- /home/alice/.windows:/storage' "$COMPOSE" || fail "storage volume uses the given path" +grep -q -- "- $EXPECTED_STORAGE:/storage" "$COMPOSE" || fail "storage uses the per-uid protected anchor" +grep -q -- "- $EXPECTED_SHARED:/shared" "$COMPOSE" || fail "shared files use the per-uid protected anchor" +[[ -L $HOME/.windows && $(realpath "$HOME/.windows") == "$EXPECTED_STORAGE" ]] || fail "home storage link targets the protected anchor" +[[ -L $HOME/Windows && $(realpath "$HOME/Windows") == "$EXPECTED_SHARED" ]] || fail "home shared link targets the protected anchor" grep -q -- '- /:/' "$COMPOSE" && fail "compose must never contain a host-root bind mount" -pass "writer emits a pinned compose with no host-root mount" +pass "writer derives protected per-uid anchors and emits no host-root mount" # --- injection attempts are rejected, no file written --- rm -f "$COMPOSE" -write 4G 2 64G 'x -v /:/h' p UTC /a /b 2>/dev/null && fail "malicious username was accepted" +write 4G 2 64G 'x -v /:/h' p UTC 2>/dev/null && fail "malicious username was accepted" [[ ! -f $COMPOSE ]] || fail "no compose written for a bad username" -write 4G 2 64G ok p UTC '/a -v /etc:/etc' /b 2>/dev/null && fail "malicious storage path was accepted" -write '4G; rm -rf /' 2 64G ok p UTC /a /b 2>/dev/null && fail "malicious RAM was accepted" -pass "injection attempts in username, path, and RAM are rejected" +printf 'RAM=4G\nCORES=2\nDISK=64G\nUSERNAME=ok\nPASSWORD=p\nTZ=UTC\nSTORAGE=/\nSHARED=/etc\n' | __priv_write_compose +grep -q -- "- $EXPECTED_STORAGE:/storage" "$COMPOSE" || fail "caller-supplied storage path affected the compose" +grep -q -- '- /:/storage' "$COMPOSE" && fail "host root was accepted as storage" +write '4G; rm -rf /' 2 64G ok p UTC 2>/dev/null && fail "malicious RAM was accepted" +pass "injection attempts are rejected and caller-supplied paths are ignored" # --- password survives YAML (" \) and compose interpolation ($) --- rm -f "$COMPOSE" tricky='p@$$w:rd$HOME"x\y' -write 8G 4 64G bob "$tricky" UTC /h/.windows /h/Windows +write 8G 4 64G bob "$tricky" UTC grep -q 'PASSWORD: ".*\$\$.*"' "$COMPOSE" || fail "\$ is escaped as \$\$ for compose interpolation" recovered=$(unescape "$(read_compose_value PASSWORD "$COMPOSE")") [[ $recovered == "$tricky" ]] || fail "password round-trips through write/unescape" @@ -65,8 +73,10 @@ pass "privileged action whitelist accepts known actions and rejects the rest" # mount host / into the guest, so migration must ignore its volume paths and # reconstruct them from the current user's $HOME. rm -rf "$OMARCHY_WINDOWS_DIR" -export HOME="$TMPDIR/home" -mkdir -p "$HOME/.config/windows" +rm -rf "$MOUNT_ROOT" +rm -f "$HOME/.windows" "$HOME/Windows" +mkdir -p "$HOME/.config/windows" "$HOME/.windows" "$HOME/Windows" +touch "$HOME/.windows/existing-disk" "$HOME/Windows/existing-shared-file" LEGACY_COMPOSE_FILE="$HOME/.config/windows/docker-compose.yml" COMPOSE_FILE="$COMPOSE" cat >"$LEGACY_COMPOSE_FILE" <<'LEG' @@ -88,32 +98,40 @@ priv() { local a=$1; shift; "__priv_$a" "$@"; } migrate_legacy_compose [[ -f $COMPOSE_FILE ]] || fail "migration wrote the root-owned compose" grep -q 'USERNAME: "legacyuser"' "$COMPOSE_FILE" || fail "migration preserves settings" -grep -q -- "- $HOME/.windows:/storage" "$COMPOSE_FILE" || fail "migration uses the user's home for the data volume" +resolve_caller +grep -q -- "- $EXPECTED_STORAGE:/storage" "$COMPOSE_FILE" || fail "migration uses the protected storage anchor" +grep -q -- "- $EXPECTED_SHARED:/shared" "$COMPOSE_FILE" || fail "migration uses the protected shared anchor" +[[ -f $EXPECTED_STORAGE/existing-disk ]] || fail "migration preserves the existing disk data" +[[ -f $EXPECTED_SHARED/existing-shared-file ]] || fail "migration preserves existing shared files" +[[ -L $HOME/.windows && -L $HOME/Windows ]] || fail "migration replaces home entries with compatibility links" grep -q -- '- /:/' "$COMPOSE_FILE" && fail "migration must not carry a host-root bind mount from a tampered legacy file" grep -q -- '- /etc:/shared' "$COMPOSE_FILE" && fail "migration must not carry a tampered legacy volume path" [[ ! -f $LEGACY_COMPOSE_FILE ]] || fail "migration removes the legacy compose" -pass "migration reconstructs data paths from \$HOME and ignores tampered legacy volumes" +pass "legacy migration pins existing data and ignores tampered legacy volumes" -# --- bring-up refuses a symlinked mount source (a symlink redirects the -# privileged bind mount the same way traversal would; the string check on -# the stored path cannot see it) --- -rm -f "$COMPOSE" -mkdir -p "$TMPDIR/realstore" "$TMPDIR/realshare" -write 4G 2 64G dave pw UTC "$TMPDIR/realstore" "$TMPDIR/realshare" +# --- bring-up accepts only the derived pair in a trusted compose --- assert_mounts_safe || fail "real directory mount sources are accepted" -ln -sfn / "$TMPDIR/evilshare" -write 4G 2 64G dave pw UTC "$TMPDIR/realstore" "$TMPDIR/evilshare" -assert_mounts_safe && fail "a symlinked mount source must be refused" -pass "bring-up refuses a symlinked mount source" +sed -i "s|$EXPECTED_SHARED:/shared|/etc:/shared|" "$COMPOSE" +assert_mounts_safe 2>/dev/null && fail "a tampered host path must be refused" +sed -i "s|/etc:/shared|$EXPECTED_SHARED:/shared|" "$COMPOSE" +chmod 0666 "$COMPOSE" +assert_mounts_safe 2>/dev/null && fail "a user-writable compose must be refused" +chmod 0640 "$COMPOSE" +pass "bring-up rejects unexpected mounts and a writable compose" -# --- valid_path rejects traversal and non-normalized paths --- -for p in /home/u/.windows /var/lib/omarchy/windows; do - valid_path "$p" || fail "valid_path rejected a normal path: $p" -done -for p in / /./ // /tmp/../etc /home/u/. '/home/u/../root' '/a//b'; do - valid_path "$p" && fail "valid_path accepted a traversal/non-normalized path: $p" -done -pass "valid_path accepts normalized paths and rejects traversal" +# --- a symlink supplied as legacy data is renamed below the protected parent +# before inspection, then quarantined rather than followed --- +rm -rf "$OMARCHY_WINDOWS_DIR" +rm -rf "$MOUNT_ROOT" +rm -f "$HOME/.windows" "$HOME/Windows" +ln -s / "$HOME/.windows" +mkdir -p "$HOME/Windows" +rm -f "$COMPOSE" +write 4G 2 64G dave pw UTC 2>/dev/null && fail "a symlinked legacy data entry was accepted" +[[ ! -f $COMPOSE ]] || fail "no compose is written for a symlinked legacy entry" +[[ ! -L $MOUNT_ROOT/users/$(id -u)/storage ]] || fail "the mount anchor must not remain a symlink" +find "$MOUNT_ROOT/users/$(id -u)" -maxdepth 1 -type l -name 'rejected-storage-*' | grep -q . || fail "the rejected symlink was not quarantined" +pass "migration pins and rejects a symlinked legacy data entry" # --- credentials are stored privately and round-trip (incl. = in password) --- export CREDENTIALS_FILE="$TMPDIR/creds" From a165185a3f48b2344f89d2e5f14a8f3e9e02ffdd Mon Sep 17 00:00:00 2001 From: Afonso Oliveira Date: Thu, 27 Aug 2026 23:53:44 +0100 Subject: [PATCH 2/5] [Security] Complete Windows VM mount hardening --- bin/omarchy-windows-vm | 835 ++++++++++++++---- manual/28-windows-vm.md | 6 +- test/shell.d/windows-vm-compose-test.sh | 426 +++++++-- .../shell.d/windows-vm-mount-boundary-test.sh | 168 ++++ 4 files changed, 1194 insertions(+), 241 deletions(-) create mode 100644 test/shell.d/windows-vm-mount-boundary-test.sh diff --git a/bin/omarchy-windows-vm b/bin/omarchy-windows-vm index c22c4815..0676b770 100755 --- a/bin/omarchy-windows-vm +++ b/bin/omarchy-windows-vm @@ -28,6 +28,23 @@ LEGACY_COMPOSE_FILE="$HOME/.config/windows/docker-compose.yml" CREDENTIALS_FILE="$HOME/.config/windows/credentials" IMAGE="dockurr/windows" CONTAINER="omarchy-windows" +VM_LOCK_DIR=/run/lock/omarchy-windows-vm +# Removal is the only path that recursively proves there are no bind aliases. +# Bound every metadata walk so a large or hostile caller tree fails closed +# instead of hanging a privileged action indefinitely. +TREE_SCAN_TIMEOUT_SECONDS=5 +TREE_SCAN_KILL_AFTER_SECONDS=1 +TREE_SCAN_TIMEOUT=/usr/bin/timeout +TREE_SCAN_FIND=/usr/bin/find + +# The privileged process must not resolve mount, stat, Docker, or any other +# helper from a caller-controlled PATH, and validation must not change with an +# inherited locale. pkexec normally sanitizes both; pin them here as defense in +# depth for every direct __priv entry as well. +if ((EUID == 0)); then + export PATH=/usr/bin:/usr/sbin:/bin:/sbin + export LC_ALL=C.UTF-8 +fi # --- privilege helpers ------------------------------------------------------- @@ -45,17 +62,19 @@ docker_needs_sudo() { omarchy-sudo-docker; } # prompt the user grants for the trusted helper could run an attacker's binary # as root. Fails closed (empty output) when no trustworthy target is found. priv_target() { - local candidate owner mode - for candidate in /usr/bin/omarchy-windows-vm "$(type -P omarchy-windows-vm 2>/dev/null)"; do - [[ -n $candidate && -x $candidate ]] || continue - owner=$(stat -Lc '%u' "$candidate" 2>/dev/null) || continue - mode=$(stat -Lc '%a' "$candidate" 2>/dev/null) || continue - [[ $owner == "0" ]] || continue - ((8#$mode & 022)) && continue # writable by group or other -> reject - printf '%s\n' "$candidate" - return 0 + local candidate=/usr/bin/omarchy-windows-vm canonical probe owner mode + [[ -f $candidate && ! -L $candidate && -x $candidate ]] || return 1 + canonical=$(realpath -e -- "$candidate" 2>/dev/null) || return 1 + [[ $canonical == "$candidate" ]] || return 1 + probe=$candidate + while :; do + owner=$(stat -Lc '%u' "$probe" 2>/dev/null) || return 1 + mode=$(stat -Lc '%a' "$probe" 2>/dev/null) || return 1 + [[ $owner == 0 ]] && ! ((8#$mode & 022)) || return 1 + [[ $probe == / ]] && break + probe=$(dirname -- "$probe") done - return 1 + printf '%s\n' "$candidate" } # Run a privileged VM action. write_compose always elevates (the compose is @@ -68,11 +87,15 @@ priv() { local action="$1" shift if [[ $action != write_compose && $action != remove ]] && ! docker_needs_sudo; then - # Existing installs used bind sources in $HOME. One privileged run is - # needed to atomically move those directories below the root-owned mount - # boundary, even when the caller can otherwise reach Docker directly. - if [[ $action != up && $action != up_wait ]] || ! compose_needs_mount_migration; then - "__priv_$action" "$@" + # Bring-up normally runs directly for a docker-group user, but recreating + # the protected bind anchors after reboot (or migrating an old compose) + # still needs one privileged invocation. + if [[ -d $VM_LOCK_DIR && ! -L $VM_LOCK_DIR && -r $VM_LOCK_DIR && -x $VM_LOCK_DIR ]] && { + [[ $action != up && $action != up_wait ]] || { + ! compose_needs_mount_migration && mounts_ready >/dev/null 2>&1 + } + }; then + with_vm_lock "__priv_$action" "$@" return fi fi @@ -86,6 +109,28 @@ priv() { dc() { docker-compose -f "$COMPOSE_FILE" "$@"; } +with_vm_lock() { + local fd rc=0 + if ((EUID == 0)); then + assert_boundary_dir /run 0 && assert_boundary_dir /run/lock 0 || return 1 + if getent group docker >/dev/null 2>&1; then + install -d -o root -g docker -m 0750 -- "$VM_LOCK_DIR" || return 1 + else + install -d -o root -g root -m 0700 -- "$VM_LOCK_DIR" || return 1 + fi + fi + assert_boundary_dir "$VM_LOCK_DIR" 0 || return 1 + # Flock the directory inode itself. Concurrent first callers may both run + # install -d, but mkdir is atomic and they necessarily open the same stable + # inode below the root-owned /run/lock parent. + exec {fd}<"$VM_LOCK_DIR/." || return 1 + flock -x "$fd" || { exec {fd}<&-; return 1; } + "$@" || rc=$? + flock -u "$fd" || rc=1 + exec {fd}<&- + return "$rc" +} + # --- validation (shared by the user-side prompts and the root-side writer) ---- valid_ram() { [[ $1 =~ ^[0-9]{1,3}G$ ]]; } @@ -115,7 +160,7 @@ resolve_caller() { local entry canonical parent owner mode if ((EUID == 0)); then - [[ ${PKEXEC_UID:-} =~ ^[0-9]+$ ]] && ((10#$PKEXEC_UID > 0)) || { + [[ ${PKEXEC_UID:-} =~ ^[0-9]{1,10}$ ]] && ((10#$PKEXEC_UID > 0)) || { echo "omarchy-windows-vm: cannot identify the user who authorized this action" >&2 return 1 } @@ -153,7 +198,7 @@ resolve_caller() { return 1 } # The user must not be able to rename or replace their home while root is - # moving the legacy data entry out of it. + # opening and pinning the familiar data entries below it. parent=$(dirname -- "$CALLER_HOME") while :; do owner=$(stat -Lc '%u' "$parent") || return 1 @@ -167,16 +212,23 @@ resolve_caller() { done fi - # Keep the potentially large disk on the same filesystem as the user's home, - # but outside that user-writable directory. The already-validated home parent - # is root-owned, so this sibling tree provides a stable rename boundary. - MOUNT_ROOT="$(dirname -- "$CALLER_HOME")/.omarchy-windows" + # Docker only ever sees fixed paths below the root-owned runtime tree. The + # user's real data stays wherever ~/.windows and ~/Windows resolve (including + # separately mounted homes and legitimate symlinks); those sources are pinned + # into these anchors with bind mounts before Docker is allowed to start. + MOUNT_ROOT="$RUNTIME_DIR/mounts" USERS_DIR="$MOUNT_ROOT/users" CALLER_DATA_ROOT="$USERS_DIR/$CALLER_UID" EXPECTED_STORAGE="$CALLER_DATA_ROOT/storage" EXPECTED_SHARED="$CALLER_DATA_ROOT/shared" LEGACY_STORAGE="$CALLER_HOME/.windows" LEGACY_SHARED="$CALLER_HOME/Windows" + # The first protected-anchor implementation used a root-owned sibling of + # home. Recognize that exact derived pair during upgrade, but never accept a + # path read from user input. + OLD_MOUNT_ROOT="$(dirname -- "$CALLER_HOME")/.omarchy-windows" + OLD_EXPECTED_STORAGE="$OLD_MOUNT_ROOT/users/$CALLER_UID/storage" + OLD_EXPECTED_SHARED="$OLD_MOUNT_ROOT/users/$CALLER_UID/shared" } boundary_owner() { @@ -200,8 +252,25 @@ assert_boundary_dir() { [[ $owner == "$expected_owner" ]] && ! ((8#$mode & 022)) } +prepare_boundary_component() { + local path="$1" parent="$2" owner="$3" mode="$4" + assert_boundary_dir "$parent" "$owner" || return 1 + if [[ -e $path || -L $path ]]; then + assert_boundary_dir "$path" "$owner" || return 1 + else + if ((EUID == 0)); then + install -d -o root -g root -m "$mode" -- "$path" || return 1 + else + install -d -m "$mode" -- "$path" || return 1 + fi + fi + chmod "$mode" -- "$path" || return 1 + if ((EUID == 0)); then chown root:root -- "$path" || return 1; fi + assert_boundary_dir "$path" "$owner" +} + prepare_runtime_tree() { - local owner probe + local owner probe runtime_parent owner=$(boundary_owner) if ((EUID == 0)); then [[ $RUNTIME_DIR == /var/lib/omarchy/windows ]] || { @@ -220,114 +289,395 @@ prepare_runtime_tree() { [[ $probe == / ]] && break probe=$(dirname -- "$probe") done - if [[ -e $MOUNT_ROOT || -L $MOUNT_ROOT ]]; then - assert_boundary_dir "$MOUNT_ROOT" 0 || { - echo "omarchy-windows-vm: unsafe mount root: $MOUNT_ROOT" >&2 - return 1 - } - fi fi - mkdir -p -- "$RUNTIME_DIR" "$MOUNT_ROOT" "$USERS_DIR" "$CALLER_DATA_ROOT" - chmod 0755 "$RUNTIME_DIR" "$MOUNT_ROOT" "$USERS_DIR" "$CALLER_DATA_ROOT" - if ((EUID == 0)); then - chown root:root "$RUNTIME_DIR" "$MOUNT_ROOT" "$USERS_DIR" "$CALLER_DATA_ROOT" + + runtime_parent=$(dirname -- "$RUNTIME_DIR") + if ((EUID == 0)) && [[ ! -e $runtime_parent && ! -L $runtime_parent ]]; then + [[ $runtime_parent == /var/lib/omarchy ]] || return 1 + assert_boundary_dir /var/lib 0 || return 1 + install -d -o root -g root -m 0755 -- "$runtime_parent" || return 1 fi - assert_boundary_dir "$RUNTIME_DIR" "$owner" && - assert_boundary_dir "$MOUNT_ROOT" "$owner" && - assert_boundary_dir "$USERS_DIR" "$owner" && - assert_boundary_dir "$CALLER_DATA_ROOT" "$owner" || { + prepare_boundary_component "$RUNTIME_DIR" "$runtime_parent" "$owner" 0755 && + prepare_boundary_component "$MOUNT_ROOT" "$RUNTIME_DIR" "$owner" 0711 && + prepare_boundary_component "$USERS_DIR" "$MOUNT_ROOT" "$owner" 0711 && + prepare_boundary_component "$CALLER_DATA_ROOT" "$USERS_DIR" "$owner" 0711 || { echo "omarchy-windows-vm: unsafe VM mount boundary" >&2 return 1 } } - # Move an existing home entry first, then inspect the pinned object below the -# root-owned parent. This closes the check/use gap where an attacker could swap -# a checked home directory for a symlink before Docker resolved it. -prepare_mount_leaf() { - local legacy="$1" stable="$2" rejected source_dev target_dev - - if [[ ! -e $stable && ! -L $stable ]]; then - if [[ -e $legacy || -L $legacy ]]; then - # rename(2) pins the exact directory entry the caller presented. GNU mv - # falls back to a privileged recursive copy across filesystems, which - # would reopen the source path and reintroduce the race, so fail closed in - # that uncommon layout instead of copying as root. - source_dev=$(stat -c '%d' -- "$legacy") || return 1 - target_dev=$(stat -Lc '%d' -- "$CALLER_DATA_ROOT") || return 1 - [[ $source_dev == "$target_dev" ]] || { - echo "omarchy-windows-vm: cannot safely migrate $legacy across filesystems" >&2 - echo "Move it onto the filesystem containing $MOUNT_ROOT, then retry." >&2 - return 1 - } - mv --no-copy -T -- "$legacy" "$stable" || return 1 - if [[ ! -d $stable || -L $stable ]]; then - rejected="$CALLER_DATA_ROOT/rejected-$(basename -- "$stable")-$$" - mv --no-copy -T -- "$stable" "$rejected" 2>/dev/null || true - echo "omarchy-windows-vm: refusing non-directory VM data entry at $legacy" >&2 - return 1 - fi - else - install -d -m 0700 "$stable" - fi - fi - - [[ -d $stable && ! -L $stable ]] && [[ $(realpath -e -- "$stable" 2>/dev/null) == "$stable" ]] || { - echo "omarchy-windows-vm: unsafe VM data directory: $stable" >&2 +# Open the source directory itself and keep the descriptor alive until after the +# bind. /proc/$BASHPID/fd refers to this exact shell process (including when a +# function runs in a pipeline subshell), not the short-lived mount subprocess, +# so a rename or symlink swap after open cannot change which inode is mounted. +open_mount_source() { + local path="$1" label="$2" fd record uid identity + [[ -d $path ]] || { + echo "omarchy-windows-vm: $label source is not a directory: $path" >&2 return 1 } - if ((EUID == 0)); then - chown "$CALLER_UID:$CALLER_GID" "$stable" - fi + # Appending /. makes a directory-to-FIFO swap fail with ENOTDIR instead of + # leaving the privileged helper blocked while opening an attacker-held pipe. + exec {fd}<"$path/." || { + echo "omarchy-windows-vm: cannot open $label source: $path" >&2 + return 1 + } + [[ -d /proc/$BASHPID/fd/$fd ]] || { + exec {fd}<&- + return 1 + } + record=$(stat -Lc '%u|%d:%i' "/proc/$BASHPID/fd/$fd" 2>/dev/null) || { + exec {fd}<&- + return 1 + } + IFS='|' read -r uid identity <<<"$record" + [[ $uid == "$CALLER_UID" ]] || { + exec {fd}<&- + echo "omarchy-windows-vm: $label source must be a directory owned by uid $CALLER_UID" >&2 + return 1 + } + OPENED_MOUNT_FD=$fd + OPENED_MOUNT_ID=$identity +} - if [[ -L $legacy ]]; then - [[ $(realpath -e -- "$legacy" 2>/dev/null) == "$stable" ]] || { - echo "omarchy-windows-vm: $legacy does not point to its protected mount anchor" >&2 - return 1 +# Return 0 when ancestor_id contains the already-open descendant directory, 1 +# when the walk reaches the namespace root without finding it, and 2 on any +# error or an implausibly deep walk. Every hop is opened relative to a pinned +# directory FD; no caller-mutable pathname is re-resolved. +pinned_dir_contains() { + local ancestor_id="$1" descendant_fd="$2" walk_fd parent_fd current_id parent_id depth + exec {walk_fd}<"/proc/$BASHPID/fd/$descendant_fd/." || return 2 + for ((depth = 0; depth < 256; depth++)); do + current_id=$(stat -Lc '%d:%i' "/proc/$BASHPID/fd/$walk_fd" 2>/dev/null) || { + exec {walk_fd}<&- + return 2 } - elif [[ -e $legacy ]]; then - echo "omarchy-windows-vm: refusing to replace existing data at $legacy" >&2 + if [[ $current_id == "$ancestor_id" ]]; then + exec {walk_fd}<&- + return 0 + fi + exec {parent_fd}<"/proc/$BASHPID/fd/$walk_fd/.." || { + exec {walk_fd}<&- + return 2 + } + parent_id=$(stat -Lc '%d:%i' "/proc/$BASHPID/fd/$parent_fd" 2>/dev/null) || { + exec {parent_fd}<&- + exec {walk_fd}<&- + return 2 + } + if [[ $parent_id == "$current_id" ]]; then + exec {parent_fd}<&- + exec {walk_fd}<&- + return 1 + fi + exec {walk_fd}<&- + walk_fd=$parent_fd + done + exec {walk_fd}<&- + return 2 +} + +# A bind alias can give the same directory inode a second parent chain, so an +# upward walk alone is insufficient for destructive removal. Search from a +# pinned tree root for the other pinned inode without following symlinks or +# crossing the removal traversal's filesystem boundary. Return 0 when found, 1 +# when absent, and 2 on timeout, traversal error, or unexpected output. +pinned_tree_contains() { + local root_fd="$1" needle_fd="$2" found rc + # -xdev still evaluates a nested mountpoint itself before pruning its + # children, so a direct different-filesystem alias of the needle is found too. + # This matches removal's traversal boundary without skipping mount aliases. + if found=$("$TREE_SCAN_TIMEOUT" --signal=TERM --kill-after="${TREE_SCAN_KILL_AFTER_SECONDS}s" \ + "${TREE_SCAN_TIMEOUT_SECONDS}s" "$TREE_SCAN_FIND" -P "/proc/$BASHPID/fd/$root_fd/." \ + -xdev -type d -samefile "/proc/$BASHPID/fd/$needle_fd/." \ + -printf 'found\n' -quit 2>/dev/null); then + rc=0 + else + rc=$? + fi + ((rc == 0)) || return 2 + case "$found" in + found) return 0 ;; + "") return 1 ;; + *) return 2 ;; + esac +} + +validate_pinned_sources_disjoint() { + local storage_fd="$1" storage_id="$2" shared_fd="$3" shared_id="$4" rc + if [[ $storage_id == "$shared_id" ]]; then + echo "omarchy-windows-vm: storage and shared must be different directories" >&2 + return 1 + fi + if pinned_dir_contains "$storage_id" "$shared_fd"; then + echo "omarchy-windows-vm: shared directory must not be inside storage" >&2 return 1 else - ln -s -- "$stable" "$legacy" || return 1 - if ((EUID == 0)); then - chown -h "$CALLER_UID:$CALLER_GID" "$legacy" - fi + rc=$? + ((rc == 1)) || { + echo "omarchy-windows-vm: could not verify storage/shared ancestry" >&2 + return 1 + } fi + if pinned_dir_contains "$shared_id" "$storage_fd"; then + echo "omarchy-windows-vm: storage directory must not be inside shared" >&2 + return 1 + else + rc=$? + ((rc == 1)) || { + echo "omarchy-windows-vm: could not verify storage/shared ancestry" >&2 + return 1 + } + fi +} + +removal_trees_disjoint() { + local storage_fd shared_fd anchor_storage_fd anchor_shared_fd storage_id shared_id rc=1 scan_rc + local scan scan_root_fd scan_needle_fd scan_label + open_mount_source "$LEGACY_STORAGE" storage || return 1 + storage_fd=$OPENED_MOUNT_FD + storage_id=$OPENED_MOUNT_ID + if ! open_mount_source "$LEGACY_SHARED" shared; then + exec {storage_fd}<&- + return 1 + fi + shared_fd=$OPENED_MOUNT_FD + shared_id=$OPENED_MOUNT_ID + if ! validate_pinned_sources_disjoint "$storage_fd" "$storage_id" "$shared_fd" "$shared_id" || + ! mounted_leaf_matches "$EXPECTED_STORAGE" "$storage_id" || + ! mounted_leaf_matches "$EXPECTED_SHARED" "$shared_id"; then + exec {storage_fd}<&- + exec {shared_fd}<&- + return 1 + fi + if ! exec {anchor_storage_fd}<"$EXPECTED_STORAGE/."; then + exec {storage_fd}<&- + exec {shared_fd}<&- + return 1 + fi + if ! exec {anchor_shared_fd}<"$EXPECTED_SHARED/."; then + exec {anchor_storage_fd}<&- + exec {storage_fd}<&- + exec {shared_fd}<&- + return 1 + fi + + # Scan the protected anchor views first. Also scan the pinned caller views: + # a submount attached after the original non-recursive bind is intentionally + # absent from the anchor view, but removal must still refuse that alias. + rc=0 + for scan in \ + "$anchor_storage_fd:$anchor_shared_fd:shared below protected storage" \ + "$anchor_shared_fd:$anchor_storage_fd:storage below protected shared" \ + "$storage_fd:$shared_fd:shared below storage source" \ + "$shared_fd:$storage_fd:storage below shared source"; do + IFS=: read -r scan_root_fd scan_needle_fd scan_label <<<"$scan" + if pinned_tree_contains "$scan_root_fd" "$scan_needle_fd"; then + echo "omarchy-windows-vm: refusing removal: $scan_label" >&2 + rc=1 + break + else + scan_rc=$? + if ((scan_rc != 1)); then + echo "omarchy-windows-vm: removal containment scan failed or timed out" >&2 + rc=1 + break + fi + fi + done + + exec {anchor_storage_fd}<&- + exec {anchor_shared_fd}<&- + exec {storage_fd}<&- + exec {shared_fd}<&- + return "$rc" +} + +prepare_mount_anchor() { + local path="$1" owner + owner=$(boundary_owner) + [[ ! -L $path ]] || return 1 + if mountpoint -q -- "$path" 2>/dev/null; then return 0; fi + prepare_boundary_component "$path" "$CALLER_DATA_ROOT" "$owner" 0700 || return 1 + [[ -z $(find "$path" -mindepth 1 -print -quit) ]] || { + echo "omarchy-windows-vm: refusing to hide data below mount anchor: $path" >&2 + return 1 + } +} + +mounted_leaf_matches() { + local stable="$1" identity="$2" actual owner mode canonical + [[ -d $stable && ! -L $stable ]] || return 1 + canonical=$(realpath -e -- "$stable" 2>/dev/null) || return 1 + [[ $canonical == "$stable" ]] || return 1 + mountpoint -q -- "$stable" 2>/dev/null || return 1 + [[ $(mount_layer_count "$stable") == 1 ]] || return 1 + actual=$(stat -Lc '%d:%i' "$stable" 2>/dev/null) || return 1 + owner=$(stat -Lc '%u' "$stable" 2>/dev/null) || return 1 + mode=$(stat -Lc '%a' "$stable" 2>/dev/null) || return 1 + [[ $actual == "$identity" && $owner == "$CALLER_UID" && $mode == 700 ]] +} + +bind_mount_leaf() { + local fd="$1" identity="$2" stable="$3" actual owner + MOUNT_LEAF_NEW=0 + if mountpoint -q -- "$stable" 2>/dev/null; then + mounted_leaf_matches "$stable" "$identity" || { + echo "omarchy-windows-vm: protected mount at $stable no longer matches its home source" >&2 + return 1 + } + return 0 + fi + + # util-linux normally canonicalizes a /proc//fd link back to a pathname, + # which would throw away the FD pin. Pass the procfd to mount(2) unchanged. + mount --no-canonicalize --bind "/proc/$BASHPID/fd/$fd" "$stable" || return 1 + MOUNT_LEAF_NEW=1 + actual=$(stat -Lc '%d:%i' "$stable" 2>/dev/null) || actual="" + owner=$(stat -Lc '%u' "$stable" 2>/dev/null) || owner="" + if [[ $actual != "$identity" || $owner != "$CALLER_UID" ]]; then + if umount -- "$stable"; then + MOUNT_LEAF_NEW=0 + else + echo "omarchy-windows-vm: could not roll back unverified bind at $stable" >&2 + fi + echo "omarchy-windows-vm: bind verification failed for $stable" >&2 + return 1 + fi + mounted_leaf_matches "$stable" "$identity" || { + if umount -- "$stable"; then + MOUNT_LEAF_NEW=0 + else + echo "omarchy-windows-vm: could not roll back invalid bind at $stable" >&2 + fi + return 1 + } } prepare_caller_mounts() { - resolve_caller && prepare_runtime_tree && - prepare_mount_leaf "$LEGACY_STORAGE" "$EXPECTED_STORAGE" && - prepare_mount_leaf "$LEGACY_SHARED" "$EXPECTED_SHARED" + local storage_fd storage_id shared_fd shared_id storage_mode shared_mode + CALLER_MOUNTS_NEW_STORAGE=0 + CALLER_MOUNTS_NEW_SHARED=0 + resolve_caller && prepare_runtime_tree || return 1 + prepare_mount_anchor "$EXPECTED_STORAGE" && prepare_mount_anchor "$EXPECTED_SHARED" || return 1 + + # Pre-open and validate both sources before changing either mount anchor. + open_mount_source "$LEGACY_STORAGE" storage || return 1 + storage_fd=$OPENED_MOUNT_FD + storage_id=$OPENED_MOUNT_ID + if ! open_mount_source "$LEGACY_SHARED" shared; then + exec {storage_fd}<&- + return 1 + fi + shared_fd=$OPENED_MOUNT_FD + shared_id=$OPENED_MOUNT_ID + if ! validate_pinned_sources_disjoint "$storage_fd" "$storage_id" "$shared_fd" "$shared_id"; then + exec {storage_fd}<&- + exec {shared_fd}<&- + return 1 + fi + + # Privacy is an explicit preflight step for both already-pinned sources, not + # a side effect halfway through the two-mount transaction. Old umask-022 + # installs are hardened together before either Docker-facing anchor changes. + chmod 0700 -- "/proc/$BASHPID/fd/$storage_fd" "/proc/$BASHPID/fd/$shared_fd" || { + exec {storage_fd}<&- + exec {shared_fd}<&- + return 1 + } + storage_mode=$(stat -Lc '%a' "/proc/$BASHPID/fd/$storage_fd" 2>/dev/null) || storage_mode="" + shared_mode=$(stat -Lc '%a' "/proc/$BASHPID/fd/$shared_fd" 2>/dev/null) || shared_mode="" + if [[ $storage_mode != 700 || $shared_mode != 700 ]]; then + exec {storage_fd}<&- + exec {shared_fd}<&- + return 1 + fi + + if bind_mount_leaf "$storage_fd" "$storage_id" "$EXPECTED_STORAGE"; then + CALLER_MOUNTS_NEW_STORAGE=$MOUNT_LEAF_NEW + else + CALLER_MOUNTS_NEW_STORAGE=$MOUNT_LEAF_NEW + rollback_new_caller_mounts || true + exec {storage_fd}<&- + exec {shared_fd}<&- + return 1 + fi + if ! bind_mount_leaf "$shared_fd" "$shared_id" "$EXPECTED_SHARED"; then + CALLER_MOUNTS_NEW_SHARED=$MOUNT_LEAF_NEW + rollback_new_caller_mounts || true + exec {storage_fd}<&- + exec {shared_fd}<&- + return 1 + fi + CALLER_MOUNTS_NEW_SHARED=$MOUNT_LEAF_NEW + + exec {storage_fd}<&- + exec {shared_fd}<&- } -# Reads KEY=VALUE lines on stdin, re-validates every field, and writes the -# compose atomically as root. Re-validation here is the security boundary: the -# writer refuses rather than emit a compose an attacker could have influenced. -# Only these fixed keys are honored; image, container name, devices, caps, and -# port bindings are hard-coded and never taken from input. -__priv_write_compose() { - local ram cores disk username password tz key value +mounts_ready() { + local storage_fd storage_id shared_fd shared_id rc=1 + resolve_caller || return 1 + open_mount_source "$LEGACY_STORAGE" storage || return 1 + storage_fd=$OPENED_MOUNT_FD + storage_id=$OPENED_MOUNT_ID + if ! open_mount_source "$LEGACY_SHARED" shared; then + exec {storage_fd}<&- + return 1 + fi + shared_fd=$OPENED_MOUNT_FD + shared_id=$OPENED_MOUNT_ID + if ! validate_pinned_sources_disjoint "$storage_fd" "$storage_id" "$shared_fd" "$shared_id"; then + exec {storage_fd}<&- + exec {shared_fd}<&- + return 1 + fi + if assert_boundary_dir "$RUNTIME_DIR" "$(boundary_owner)" && + assert_boundary_dir "$MOUNT_ROOT" "$(boundary_owner)" && + assert_boundary_dir "$USERS_DIR" "$(boundary_owner)" && + assert_boundary_dir "$CALLER_DATA_ROOT" "$(boundary_owner)" && + mounted_leaf_matches "$EXPECTED_STORAGE" "$storage_id" && + mounted_leaf_matches "$EXPECTED_SHARED" "$shared_id"; then + rc=0 + fi + exec {storage_fd}<&- + exec {shared_fd}<&- + return "$rc" +} - while IFS='=' read -r key value; do - case "$key" in - RAM) ram="$value" ;; - CORES) cores="$value" ;; - DISK) disk="$value" ;; - USERNAME) username="$value" ;; - PASSWORD) password="$value" ;; - TZ) tz="$value" ;; - esac - done +mount_layer_count() { + local path="$1" + awk -v path="$path" '$5 == path { count++ } END { print count + 0 }' /proc/self/mountinfo +} - valid_ram "$ram" || { echo "invalid RAM: $ram" >&2; exit 2; } - valid_cores "$cores" || { echo "invalid CPU cores: $cores" >&2; exit 2; } - valid_disk "$disk" || { echo "invalid disk size: $disk" >&2; exit 2; } - valid_username "$username" || { echo "invalid username: $username" >&2; exit 2; } - valid_password "$password" || { echo "invalid password" >&2; exit 2; } - valid_tz "$tz" || tz="UTC" - prepare_caller_mounts || exit 2 +mount_descendant_count() { + local path="$1" + awk -v prefix="$path/" 'index($5, prefix) == 1 { count++ } END { print count + 0 }' /proc/self/mountinfo +} + +rollback_new_caller_mounts() { + local failed=0 + if ((CALLER_MOUNTS_NEW_SHARED)); then + if umount -- "$EXPECTED_SHARED"; then CALLER_MOUNTS_NEW_SHARED=0; else failed=1; fi + fi + if ((CALLER_MOUNTS_NEW_STORAGE)); then + if umount -- "$EXPECTED_STORAGE"; then CALLER_MOUNTS_NEW_STORAGE=0; else failed=1; fi + fi + ((failed == 0)) || echo "omarchy-windows-vm: could not roll back newly created VM mounts" >&2 + return "$failed" +} + +write_compose_atomically() ( + local ram="$1" cores="$2" disk="$3" username="$4" password="$5" tz="$6" + local tmp="" rc esc_password + cleanup_writer() { + rc=$? + trap - EXIT + [[ -z $tmp ]] || rm -f -- "$tmp" || true + if ((rc != 0)) && ! rollback_new_caller_mounts; then rc=1; fi + exit "$rc" + } + trap cleanup_writer EXIT # Neutralize anything in the password that could be misread when the compose # is parsed. Two layers apply, in this order at parse time: docker compose @@ -336,13 +686,12 @@ __priv_write_compose() { # double-quote) and the interpolation layer last ($ -> $$), so a password # containing " \ or $ reaches the guest verbatim. unescape() reverses this in # the opposite order for the RDP credentials. - local esc_password=${password//\\/\\\\} + esc_password=${password//\\/\\\\} esc_password=${esc_password//\"/\\\"} esc_password=${esc_password//\$/\$\$} - local tmp - tmp=$(mktemp "$RUNTIME_DIR/.compose.XXXXXX") - cat >"$tmp" <"$tmp" </dev/null || true - chown root:docker "$tmp" 2>/dev/null || chown root:root "$tmp" 2>/dev/null || true - mv -f "$tmp" "$COMPOSE_FILE" + chmod 0640 "$tmp" || exit 1 + if ((EUID == 0)); then + chown root:docker "$tmp" 2>/dev/null || chown root:root "$tmp" || exit 1 + fi + mv -fT -- "$tmp" "$COMPOSE_FILE" || exit 1 + tmp="" + trap - EXIT +) + +# Reads KEY=VALUE lines on stdin, re-validates every field, and writes the +# compose atomically as root. Re-validation here is the security boundary: the +# writer refuses rather than emit a compose an attacker could have influenced. +# Only these fixed keys are honored; image, container name, devices, caps, and +# port bindings are hard-coded and never taken from input. +__priv_write_compose() { + local ram cores disk username password tz key value + + while IFS='=' read -r key value; do + case "$key" in + RAM) ram="$value" ;; + CORES) cores="$value" ;; + DISK) disk="$value" ;; + USERNAME) username="$value" ;; + PASSWORD) password="$value" ;; + TZ) tz="$value" ;; + esac + done + + valid_ram "$ram" || { echo "invalid RAM: $ram" >&2; exit 2; } + valid_cores "$cores" || { echo "invalid CPU cores: $cores" >&2; exit 2; } + valid_disk "$disk" || { echo "invalid disk size: $disk" >&2; exit 2; } + valid_username "$username" || { echo "invalid username: $username" >&2; exit 2; } + valid_password "$password" || { echo "invalid password" >&2; exit 2; } + valid_tz "$tz" || tz="UTC" + prepare_caller_mounts || exit 2 + # Readable by root and the docker group only. Any failure after mounting rolls + # back just the binds this writer created and leaves an old compose untouched. + write_compose_atomically "$ram" "$cores" "$disk" "$username" "$password" "$tz" || exit 2 } # Read the host source of a bind mount out of the compose (e.g. /storage). @@ -392,8 +771,19 @@ get_mount_source() { compose_needs_mount_migration() { [[ -f $COMPOSE_FILE ]] || return 1 resolve_caller || return 1 - [[ $(get_mount_source /storage) == "$LEGACY_STORAGE" && - $(get_mount_source /shared) == "$LEGACY_SHARED" ]] + [[ $(mount_source_count /storage) == 1 && $(mount_source_count /shared) == 1 ]] || return 1 + compose_mount_pair_is_migratable "$(get_mount_source /storage)" "$(get_mount_source /shared)" +} + +compose_mount_pair_is_migratable() { + local storage="$1" shared="$2" + [[ $storage == "$LEGACY_STORAGE" && $shared == "$LEGACY_SHARED" ]] || + [[ $storage == "$OLD_EXPECTED_STORAGE" && $shared == "$OLD_EXPECTED_SHARED" ]] +} + +mount_source_count() { + local destination="$1" + sed -n "s|^[[:space:]]*-[[:space:]]*\(/[^:]*\):$destination\$|x|p" "$COMPOSE_FILE" | wc -l } rewrite_compose_mounts() { @@ -404,11 +794,14 @@ rewrite_compose_mounts() { /^[[:space:]]*-[[:space:]]*\/[^:]*:\/shared$/ { print " - " shared ":/shared"; next } { print } ' "$COMPOSE_FILE" >"$tmp" || { rm -f "$tmp"; return 1; } - chmod 0640 "$tmp" + chmod 0640 "$tmp" || { rm -f "$tmp"; return 1; } if ((EUID == 0)); then - chown root:docker "$tmp" 2>/dev/null || chown root:root "$tmp" + chown root:docker "$tmp" 2>/dev/null || chown root:root "$tmp" || { + rm -f "$tmp" + return 1 + } fi - mv -f -- "$tmp" "$COMPOSE_FILE" + mv -fT -- "$tmp" "$COMPOSE_FILE" || { rm -f "$tmp"; return 1; } } assert_compose_trusted() { @@ -421,7 +814,7 @@ assert_compose_trusted() { } assert_mounts_safe() { - local storage shared owner + local storage shared resolve_caller || return 1 assert_compose_trusted || { echo "omarchy-windows-vm: refusing an untrusted compose file" >&2 @@ -430,13 +823,21 @@ assert_mounts_safe() { storage=$(get_mount_source /storage) shared=$(get_mount_source /shared) - if [[ $storage == "$LEGACY_STORAGE" && $shared == "$LEGACY_SHARED" ]]; then + [[ $(mount_source_count /storage) == 1 && $(mount_source_count /shared) == 1 ]] || { + echo "omarchy-windows-vm: refusing duplicate or missing VM mounts in the compose" >&2 + return 1 + } + + if compose_mount_pair_is_migratable "$storage" "$shared"; then ((EUID == 0)) || { echo "omarchy-windows-vm: legacy VM data needs an authorized migration" >&2 return 1 } prepare_caller_mounts || return 1 - rewrite_compose_mounts || return 1 + if ! rewrite_compose_mounts; then + rollback_new_caller_mounts || true + return 1 + fi storage=$EXPECTED_STORAGE shared=$EXPECTED_SHARED fi @@ -445,15 +846,14 @@ assert_mounts_safe() { echo "omarchy-windows-vm: refusing unexpected host paths in the compose" >&2 return 1 } - owner=$(boundary_owner) - assert_boundary_dir "$RUNTIME_DIR" "$owner" && - assert_boundary_dir "$MOUNT_ROOT" "$owner" && - assert_boundary_dir "$USERS_DIR" "$owner" && - assert_boundary_dir "$CALLER_DATA_ROOT" "$owner" && - [[ -d $EXPECTED_STORAGE && ! -L $EXPECTED_STORAGE ]] && - [[ -d $EXPECTED_SHARED && ! -L $EXPECTED_SHARED ]] && - [[ $(realpath -e -- "$EXPECTED_STORAGE" 2>/dev/null) == "$EXPECTED_STORAGE" ]] && - [[ $(realpath -e -- "$EXPECTED_SHARED" 2>/dev/null) == "$EXPECTED_SHARED" ]] || { + + # Mounts disappear at reboot. Root recreates them from the already-opened, + # caller-owned sources; a docker-group invocation may proceed directly only + # while the exact pinned pair is still present. + if ((EUID == 0)); then + prepare_caller_mounts || return 1 + fi + mounts_ready || { echo "omarchy-windows-vm: refusing an unsafe VM mount anchor" >&2 return 1 } @@ -495,19 +895,109 @@ __priv_up_wait() { __priv_status() { docker inspect --format='{{.State.Status}}' "$CONTAINER" 2>/dev/null || true; } __priv_remove() { - resolve_caller || return 1 - dc down 2>/dev/null || true + # Rebuild/verify both pinned binds before deleting through the storage anchor. + # In particular, a legitimate ~/.windows symlink means deleting only the link + # from the user side would strand the virtual disk in its external target. + assert_mounts_safe || return 1 + [[ $EXPECTED_STORAGE == "$USERS_DIR/$CALLER_UID/storage" ]] || return 1 + [[ $EXPECTED_SHARED == "$USERS_DIR/$CALLER_UID/shared" ]] || return 1 + [[ $(mount_layer_count "$EXPECTED_STORAGE") == 1 && + $(mount_layer_count "$EXPECTED_SHARED") == 1 && + $(mount_descendant_count "$EXPECTED_STORAGE") == 0 && + $(mount_descendant_count "$EXPECTED_SHARED") == 0 ]] || { + echo "omarchy-windows-vm: refusing removal with unknown or stacked VM mounts" >&2 + return 1 + } + + dc down || { + echo "omarchy-windows-vm: could not stop the Windows VM; storage was not deleted" >&2 + return 1 + } + if docker inspect "$CONTAINER" >/dev/null 2>&1; then + echo "omarchy-windows-vm: Windows container still exists; storage was not deleted" >&2 + return 1 + fi + docker info >/dev/null 2>&1 || { + echo "omarchy-windows-vm: cannot verify Docker state; storage was not deleted" >&2 + return 1 + } + mounts_ready || { + echo "omarchy-windows-vm: VM mount identity changed during removal" >&2 + return 1 + } + removal_trees_disjoint || return 1 + + # find -xdev deliberately empties the verified disk source without crossing + # into another mounted filesystem. Shared files are never traversed. + find "$EXPECTED_STORAGE" -xdev -mindepth 1 -delete || return 1 + [[ -z $(find "$EXPECTED_STORAGE" -mindepth 1 -print -quit) ]] || return 1 + + # Release only the single known top mounts checked above. Unmount shared first + # so a storage-unmount failure cannot expose shared data to deletion. + umount -- "$EXPECTED_SHARED" || return 1 + umount -- "$EXPECTED_STORAGE" || return 1 docker rmi "$IMAGE" 2>/dev/null || true rm -f "$COMPOSE_FILE" - # Shared files intentionally survive removal. The storage leaf cannot be - # swapped by the user because its parent is the protected boundary. - if [[ $EXPECTED_STORAGE == "$USERS_DIR/$CALLER_UID/storage" && -d $EXPECTED_STORAGE && ! -L $EXPECTED_STORAGE ]]; then - rm -rf --one-file-system -- "$EXPECTED_STORAGE" - fi + rmdir -- "$EXPECTED_STORAGE" "$EXPECTED_SHARED" "$CALLER_DATA_ROOT" 2>/dev/null || true } # --- config helpers ---------------------------------------------------------- +# Validate both familiar home entries before creating or changing either. A +# legitimate symlink is kept exactly as-is; only its caller-owned directory +# target is used. The privileged half repeats the ownership check on pinned FDs. +preflight_user_mount_source() { + local path="$1" label="$2" uid owner + uid=$(id -u) + if [[ -L $path ]]; then + [[ -d $path ]] || { + echo "omarchy-windows-vm: $label is a broken or non-directory symlink: $path" >&2 + return 1 + } + elif [[ -e $path ]]; then + [[ -d $path ]] || { + echo "omarchy-windows-vm: $label is not a directory: $path" >&2 + return 1 + } + else + return 0 + fi + owner=$(stat -Lc '%u' -- "$path") || return 1 + [[ $owner == "$uid" ]] || { + echo "omarchy-windows-vm: $label must be owned by uid $uid: $path" >&2 + return 1 + } +} + +prepare_user_mount_sources() { + local storage="$HOME/.windows" shared="$HOME/Windows" storage_id shared_id + preflight_user_mount_source "$storage" storage && + preflight_user_mount_source "$shared" shared || return 1 + [[ -e $storage || -L $storage ]] || install -d -m 0700 -- "$storage" || return 1 + [[ -e $shared || -L $shared ]] || install -d -m 0700 -- "$shared" || return 1 + storage_id=$(stat -Lc '%d:%i' -- "$storage") || return 1 + shared_id=$(stat -Lc '%d:%i' -- "$shared") || return 1 + [[ $storage_id != "$shared_id" ]] || { + echo "omarchy-windows-vm: storage and shared must be different directories" >&2 + return 1 + } + chmod 0700 -- "$storage" "$shared" +} + +storage_space_path() { + if [[ -d $HOME/.windows ]]; then + realpath -e -- "$HOME/.windows" + else + printf '%s\n' "$HOME" + fi +} + +available_storage_gb() { + local path + path=$(storage_space_path) || return 1 + df -P -- "$path" | awk 'NR==2 {print int($4/1024/1024)}' +} + # Feed the collected settings to the elevated writer. write_compose() { local ram="$1" cores="$2" disk="$3" username="$4" password="$5" tz="$6" @@ -531,12 +1021,19 @@ unescape() { # password is not world-readable. The password is one validated printable line # (no newline), so plain KEY=VALUE is safe. write_credentials() { - local username="$1" password="$2" old_umask - mkdir -p "$(dirname "$CREDENTIALS_FILE")" + local username="$1" password="$2" old_umask dir tmp + dir=$(dirname -- "$CREDENTIALS_FILE") + mkdir -p "$dir" || return 1 + chmod 0700 "$dir" || return 1 old_umask=$(umask) umask 077 - printf 'USERNAME=%s\nPASSWORD=%s\n' "$username" "$password" >"$CREDENTIALS_FILE" - chmod 600 "$CREDENTIALS_FILE" 2>/dev/null || true + tmp=$(mktemp "$dir/.credentials.XXXXXX") || { umask "$old_umask"; return 1; } + if ! printf 'USERNAME=%s\nPASSWORD=%s\n' "$username" "$password" >"$tmp" || + ! chmod 0600 "$tmp" || ! mv -fT -- "$tmp" "$CREDENTIALS_FILE"; then + rm -f -- "$tmp" + umask "$old_umask" + return 1 + fi umask "$old_umask" } @@ -575,6 +1072,11 @@ migrate_legacy_compose() { password=$(read_compose_value PASSWORD "$LEGACY_COMPOSE_FILE") tz=$(read_compose_value TZ "$LEGACY_COMPOSE_FILE") [[ -z $tz ]] && tz="UTC" + prepare_user_mount_sources || { + echo "Could not validate the existing Windows VM data directories." >&2 + return 1 + } + write_credentials "$username" "$password" || return 1 # The elevated writer derives both mount anchors from the authenticated uid; # it never consumes volume paths from this user-owned legacy file. if ! write_compose "$ram" "$cores" "$disk" "$username" "$password" "$tz"; then @@ -582,7 +1084,6 @@ migrate_legacy_compose() { echo "Re-run: omarchy-windows-vm install" >&2 return 1 fi - write_credentials "$username" "$password" rm -f "$LEGACY_COMPOSE_FILE" } @@ -607,7 +1108,10 @@ check_prerequisites() { fi # Check disk space - AVAILABLE_SPACE=$(df "$HOME" | awk 'NR==2 {print int($4/1024/1024)}') + AVAILABLE_SPACE=$(available_storage_gb) || { + echo "❌ Could not determine available space for $HOME/.windows" >&2 + exit 1 + } if ((AVAILABLE_SPACE < REQUIRED_SPACE)); then echo "❌ Insufficient disk space!" echo " Available: ${AVAILABLE_SPACE}GB" @@ -622,6 +1126,7 @@ install_windows() { # Set up trap to handle Ctrl+C trap "echo ''; echo 'Installation cancelled by user'; exit 1" INT + prepare_user_mount_sources || exit 1 check_prerequisites omarchy-pkg-add freerdp openbsd-netcat gum @@ -678,7 +1183,10 @@ EOF SELECTED_CORES=2 fi - AVAILABLE_SPACE=$(df "$HOME" | awk 'NR==2 {print int($4/1024/1024)}') + AVAILABLE_SPACE=$(available_storage_gb) || { + echo "❌ Could not determine available space for $HOME/.windows" >&2 + exit 1 + } MAX_DISK_GB=$((AVAILABLE_SPACE - 10)) # Leave 10GB for Windows image # Check if we have enough space for minimum @@ -765,14 +1273,17 @@ EOF tz=$(timedatectl show -p Timezone --value 2>/dev/null || echo UTC) # Write the root-owned compose from the validated settings (one prompt if - # sudoless Docker is off). The writer creates protected storage/shared mount - # anchors and leaves the familiar home entries as symlinks to them. + # sudoless Docker is off). The writer pins the familiar home directories (or + # their legitimate symlink targets) into root-protected bind anchors. write_compose "$SELECTED_RAM" "$SELECTED_CORES" "$SELECTED_DISK" \ "$USERNAME" "$PASSWORD" "$tz" || { echo "❌ Failed to write the Windows VM configuration." exit 1 } - write_credentials "$USERNAME" "$PASSWORD" + write_credentials "$USERNAME" "$PASSWORD" || { + echo "❌ Failed to store private RDP credentials." >&2 + exit 1 + } echo "" echo "Starting Windows VM installation..." @@ -817,9 +1328,17 @@ remove_windows() { echo "Removing Windows VM..." - migrate_legacy_compose 2>/dev/null || true + if [[ ! -f $COMPOSE_FILE && -f $LEGACY_COMPOSE_FILE ]]; then + migrate_legacy_compose || { + echo "❌ Could not safely migrate the VM before removal." >&2 + exit 1 + } + fi if [[ -f $COMPOSE_FILE ]]; then - priv remove || true + priv remove || { + echo "❌ Windows VM removal stopped before user-side cleanup; inspect the VM data before retrying." >&2 + exit 1 + } fi rm -f "$HOME/.local/share/applications/windows-vm.desktop" @@ -1009,7 +1528,7 @@ __priv) echo "omarchy-windows-vm: unknown privileged action" >&2 exit 1 } - "__priv_${action}" "$@" + with_vm_lock "__priv_${action}" "$@" ;; install) install_windows diff --git a/manual/28-windows-vm.md b/manual/28-windows-vm.md index e12a74d8..29946466 100644 --- a/manual/28-windows-vm.md +++ b/manual/28-windows-vm.md @@ -28,7 +28,11 @@ omarchy windows vm launch # start and connect The directory `~/Windows` in your home directory is automatically shared with the VM. Put files there if you want them accessible to Windows. The VM has no access to any other part of your file system, so you're safe from anything nasty on the Windows side. Its own virtual disk is available at `~/.windows`. -Those familiar home paths are links to per-user mount anchors in a root-owned `.omarchy-windows` directory beside your home directory. Keeping the anchors on the home filesystem preserves the expected disk location, while their protected parent prevents another process running as you from swapping a checked directory for a symlink while the privileged VM is starting. +Those familiar home paths stay on their own filesystems. They can also be symlinks to directories you own, which is useful when the virtual disk lives on a larger drive. The installer measures free space on the filesystem that actually contains `~/.windows`, not necessarily the filesystem containing your home directory. + +Keep the disk and shared paths as separate, non-overlapping directories. Removal deliberately empties the disk directory but preserves the shared directory. Immediately before deletion, Omarchy performs a bounded containment check and refuses to remove anything if that check times out or cannot prove the two trees are separate. + +Before the VM starts, Omarchy opens and pins those two directories, then bind-mounts the exact directory inodes onto private per-user anchors below `/var/lib/omarchy/windows/mounts`. Docker only sees those root-protected anchors. This preserves custom disk locations while preventing another process running as you from swapping a checked path before the privileged container consumes it. Existing disk and shared directories are tightened to mode `0700` during migration so other local accounts cannot browse their contents. The VM's ports are bound to localhost only, so nothing on your network can reach the Windows machine. diff --git a/test/shell.d/windows-vm-compose-test.sh b/test/shell.d/windows-vm-compose-test.sh index 953f3403..77c298a8 100644 --- a/test/shell.d/windows-vm-compose-test.sh +++ b/test/shell.d/windows-vm-compose-test.sh @@ -1,82 +1,118 @@ #!/bin/bash -# -# The Windows VM compose file is written by an elevated, input-validated writer -# into a root-owned directory. These tests pin the security-critical behavior: -# no input can inject a host-root bind mount or a privileged flag, the password -# survives both the YAML and the compose-interpolation layer, only known -# privileged actions dispatch, and legacy configs migrate without redownloading. +# Security regression coverage for the Windows VM compose/mount boundary. set -euo pipefail - source "$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")" && pwd)/base-test.sh" +# Bind mounts need CAP_SYS_ADMIN in a private mount namespace. Keep the +# caller's uid so the non-root development path is exercised. +if [[ ${OMARCHY_WINDOWS_TEST_NAMESPACE:-0} != 1 ]]; then + if unshare --user --map-current-user --keep-caps --mount true 2>/dev/null; then + exec env OMARCHY_WINDOWS_TEST_NAMESPACE=1 \ + unshare --user --map-current-user --keep-caps --mount --propagation private bash "$0" + fi + pass "unprivileged mount namespaces unavailable; skipping Windows VM mount runtime tests" + exit 0 +fi + TMPDIR=$(mktemp -d) -trap 'rm -rf "$TMPDIR"' EXIT export OMARCHY_WINDOWS_DIR="$TMPDIR/win" export HOME="$TMPDIR/home" mkdir -p "$HOME" -# Source the command's functions; the dispatcher just prints usage for "help". set -- help source "$ROOT/bin/omarchy-windows-vm" >/dev/null 2>&1 COMPOSE="$OMARCHY_WINDOWS_DIR/docker-compose.yml" +unmount_all() { + local path + resolve_caller >/dev/null 2>&1 || return 0 + for path in "$EXPECTED_SHARED" "$EXPECTED_STORAGE"; do + while mountpoint -q -- "$path" 2>/dev/null; do umount -- "$path" || break; done + done +} + +cleanup() { + set +e + unmount_all + rm -rf "$TMPDIR" +} +trap cleanup EXIT + write() { # RAM CORES DISK USER PASS TZ printf 'RAM=%s\nCORES=%s\nDISK=%s\nUSERNAME=%s\nPASSWORD=%s\nTZ=%s\n' \ "$@" | __priv_write_compose } -# --- valid compose, with the dangerous bits pinned and unreachable by input --- -rm -f "$COMPOSE" -write 4G 2 64G alice 's3cret' Europe/Copenhagen +fd_count() { find "/proc/$$/fd" -mindepth 1 -maxdepth 1 -printf x | wc -c; } + +reset_case() { + unmount_all + rm -rf "$OMARCHY_WINDOWS_DIR" "$HOME/.windows" "$HOME/Windows" + mkdir -p "$HOME" +} + +# Fixed protected anchors consume the pinned source inodes. +prepare_user_mount_sources +write 4G 2 64G alice s3cret Europe/Copenhagen resolve_caller [[ -f $COMPOSE ]] || fail "writer produced a compose file" grep -q 'image: dockurr/windows' "$COMPOSE" || fail "image is pinned" grep -q -- '- NET_ADMIN' "$COMPOSE" || fail "cap_add is pinned" -grep -q -- "- $EXPECTED_STORAGE:/storage" "$COMPOSE" || fail "storage uses the per-uid protected anchor" -grep -q -- "- $EXPECTED_SHARED:/shared" "$COMPOSE" || fail "shared files use the per-uid protected anchor" -[[ -L $HOME/.windows && $(realpath "$HOME/.windows") == "$EXPECTED_STORAGE" ]] || fail "home storage link targets the protected anchor" -[[ -L $HOME/Windows && $(realpath "$HOME/Windows") == "$EXPECTED_SHARED" ]] || fail "home shared link targets the protected anchor" -grep -q -- '- /:/' "$COMPOSE" && fail "compose must never contain a host-root bind mount" -pass "writer derives protected per-uid anchors and emits no host-root mount" +grep -q -- "- $EXPECTED_STORAGE:/storage" "$COMPOSE" || fail "storage uses the protected anchor" +grep -q -- "- $EXPECTED_SHARED:/shared" "$COMPOSE" || fail "shared uses the protected anchor" +[[ ! -L $HOME/.windows && ! -L $HOME/Windows ]] || fail "fresh sources stay real directories" +[[ $(stat -Lc '%d:%i' "$HOME/.windows") == $(stat -Lc '%d:%i' "$EXPECTED_STORAGE") ]] || fail "storage bind did not pin source" +[[ $(stat -Lc '%d:%i' "$HOME/Windows") == $(stat -Lc '%d:%i' "$EXPECTED_SHARED") ]] || fail "shared bind did not pin source" +[[ $(stat -Lc '%a' "$EXPECTED_STORAGE") == 700 && $(stat -Lc '%a' "$EXPECTED_SHARED") == 700 ]] || fail "mount leaves are not private" +grep -q -- '- /:/' "$COMPOSE" && fail "compose contains host-root bind" +pass "writer emits fixed anchors bound to exact private source inodes" -# --- injection attempts are rejected, no file written --- +# Input cannot widen a mount or compose field. rm -f "$COMPOSE" -write 4G 2 64G 'x -v /:/h' p UTC 2>/dev/null && fail "malicious username was accepted" -[[ ! -f $COMPOSE ]] || fail "no compose written for a bad username" +write 4G 2 64G 'x -v /:/h' p UTC 2>/dev/null && fail "malicious username accepted" +[[ ! -f $COMPOSE ]] || fail "bad input wrote compose" printf 'RAM=4G\nCORES=2\nDISK=64G\nUSERNAME=ok\nPASSWORD=p\nTZ=UTC\nSTORAGE=/\nSHARED=/etc\n' | __priv_write_compose -grep -q -- "- $EXPECTED_STORAGE:/storage" "$COMPOSE" || fail "caller-supplied storage path affected the compose" -grep -q -- '- /:/storage' "$COMPOSE" && fail "host root was accepted as storage" -write '4G; rm -rf /' 2 64G ok p UTC 2>/dev/null && fail "malicious RAM was accepted" -pass "injection attempts are rejected and caller-supplied paths are ignored" +grep -q -- "- $EXPECTED_STORAGE:/storage" "$COMPOSE" || fail "caller storage affected compose" +grep -q -- '- /:/storage' "$COMPOSE" && fail "host root accepted as storage" +write '4G; rm -rf /' 2 64G ok p UTC 2>/dev/null && fail "malicious RAM accepted" +pass "input cannot inject a host path or compose field" -# --- password survives YAML (" \) and compose interpolation ($) --- -rm -f "$COMPOSE" tricky='p@$$w:rd$HOME"x\y' write 8G 4 64G bob "$tricky" UTC -grep -q 'PASSWORD: ".*\$\$.*"' "$COMPOSE" || fail "\$ is escaped as \$\$ for compose interpolation" -recovered=$(unescape "$(read_compose_value PASSWORD "$COMPOSE")") -[[ $recovered == "$tricky" ]] || fail "password round-trips through write/unescape" -pass "password with \" \\ and \$ round-trips" +grep -q 'PASSWORD: ".*\$\$.*"' "$COMPOSE" || fail "dollar not escaped" +[[ $(unescape "$(read_compose_value PASSWORD "$COMPOSE")") == "$tricky" ]] || fail "password did not round-trip" +pass "password with quote, backslash, and dollar round-trips" -# --- only known privileged actions may dispatch --- for action in write_compose up up_wait down status remove; do - valid_priv_action "$action" || fail "known privileged action rejected: $action" + valid_priv_action "$action" || fail "known action rejected: $action" done for action in '/../evil/x' bogus 'up;rm' '' '__priv_up'; do - valid_priv_action "$action" && fail "privileged action whitelist accepted: [$action]" + valid_priv_action "$action" && fail "action whitelist accepted: [$action]" done -pass "privileged action whitelist accepts known actions and rejects the rest" +pass "privileged action dispatch is allowlisted" -# --- legacy per-user compose migrates into the root-owned location --- -# A rogue process could have rewritten the user-owned legacy compose to bind -# mount host / into the guest, so migration must ignore its volume paths and -# reconstruct them from the current user's $HOME. -rm -rf "$OMARCHY_WINDOWS_DIR" -rm -rf "$MOUNT_ROOT" -rm -f "$HOME/.windows" "$HOME/Windows" -mkdir -p "$HOME/.config/windows" "$HOME/.windows" "$HOME/Windows" -touch "$HOME/.windows/existing-disk" "$HOME/Windows/existing-shared-file" +# A PATH symlink to bash must never become the pkexec target. Hide the packaged +# file from priv_target's stat checks to exercise the historical fallback. +attack_bin="$TMPDIR/attack-bin" +mkdir -p "$attack_bin" +ln -s /bin/bash "$attack_bin/omarchy-windows-vm" +printf 'printf exploited >"$TMPDIR/exploited"\n' >"$TMPDIR/__priv" +stat() { + [[ ${!#} == /usr/bin/omarchy-windows-vm ]] && return 1 + command stat "$@" +} +PATH="$attack_bin:$PATH" priv_target >/dev/null 2>&1 && fail "PATH symlink became a privileged target" +unset -f stat +[[ ! -e $TMPDIR/exploited ]] || fail "attacker __priv script executed" +pass "pkexec target is only the canonical packaged regular file, never a PATH symlink" + +# Legacy migration keeps directories and legitimate symlinks in place. +reset_case +external_shared="$TMPDIR/external-shared" +mkdir -m 0755 -p "$HOME/.windows" "$external_shared" "$HOME/.config/windows" +ln -s "$external_shared" "$HOME/Windows" +touch "$HOME/.windows/existing-disk" "$external_shared/existing-shared-file" LEGACY_COMPOSE_FILE="$HOME/.config/windows/docker-compose.yml" COMPOSE_FILE="$COMPOSE" cat >"$LEGACY_COMPOSE_FILE" <<'LEG' @@ -93,50 +129,276 @@ services: - /./:/storage - /etc:/shared LEG -# In production the write elevates via pkexec; here run it in-process. -priv() { local a=$1; shift; "__priv_$a" "$@"; } +priv() { local action=$1; shift; "__priv_$action" "$@"; } migrate_legacy_compose -[[ -f $COMPOSE_FILE ]] || fail "migration wrote the root-owned compose" -grep -q 'USERNAME: "legacyuser"' "$COMPOSE_FILE" || fail "migration preserves settings" resolve_caller -grep -q -- "- $EXPECTED_STORAGE:/storage" "$COMPOSE_FILE" || fail "migration uses the protected storage anchor" -grep -q -- "- $EXPECTED_SHARED:/shared" "$COMPOSE_FILE" || fail "migration uses the protected shared anchor" -[[ -f $EXPECTED_STORAGE/existing-disk ]] || fail "migration preserves the existing disk data" -[[ -f $EXPECTED_SHARED/existing-shared-file ]] || fail "migration preserves existing shared files" -[[ -L $HOME/.windows && -L $HOME/Windows ]] || fail "migration replaces home entries with compatibility links" -grep -q -- '- /:/' "$COMPOSE_FILE" && fail "migration must not carry a host-root bind mount from a tampered legacy file" -grep -q -- '- /etc:/shared' "$COMPOSE_FILE" && fail "migration must not carry a tampered legacy volume path" -[[ ! -f $LEGACY_COMPOSE_FILE ]] || fail "migration removes the legacy compose" -pass "legacy migration pins existing data and ignores tampered legacy volumes" +[[ -f $COMPOSE ]] || fail "migration did not write compose" +grep -q 'USERNAME: "legacyuser"' "$COMPOSE" || fail "migration lost settings" +[[ -f $HOME/.windows/existing-disk && -f $external_shared/existing-shared-file ]] || fail "migration lost data" +[[ ! -L $HOME/.windows && $(readlink "$HOME/Windows") == "$external_shared" ]] || fail "migration consumed source path" +[[ $(stat -Lc '%a' "$HOME/.windows") == 700 && $(stat -Lc '%a' "$external_shared") == 700 ]] || fail "migration did not harden legacy directories" +grep -q -- '- /:/' "$COMPOSE" && fail "migration copied malicious storage" +grep -q -- '- /etc:/shared' "$COMPOSE" && fail "migration copied malicious share" +[[ ! -f $LEGACY_COMPOSE_FILE ]] || fail "migration left legacy compose" +pass "migration preserves data and symlinks while hardening permissions" -# --- bring-up accepts only the derived pair in a trusted compose --- -assert_mounts_safe || fail "real directory mount sources are accepted" +# Bring-up re-proves compose trust, cardinality, and mounted identities. +assert_mounts_safe || fail "verified sources rejected" sed -i "s|$EXPECTED_SHARED:/shared|/etc:/shared|" "$COMPOSE" -assert_mounts_safe 2>/dev/null && fail "a tampered host path must be refused" +assert_mounts_safe 2>/dev/null && fail "tampered host path accepted" sed -i "s|/etc:/shared|$EXPECTED_SHARED:/shared|" "$COMPOSE" +printf ' - %s:/storage\n' "$EXPECTED_STORAGE" >>"$COMPOSE" +assert_mounts_safe 2>/dev/null && fail "duplicate destination accepted" +write 16G 6 128G legacyuser legacypass America/New_York chmod 0666 "$COMPOSE" -assert_mounts_safe 2>/dev/null && fail "a user-writable compose must be refused" +assert_mounts_safe 2>/dev/null && fail "writable compose accepted" chmod 0640 "$COMPOSE" -pass "bring-up rejects unexpected mounts and a writable compose" +pass "bring-up rejects tampered, duplicate, and writable compose inputs" -# --- a symlink supplied as legacy data is renamed below the protected parent -# before inspection, then quarantined rather than followed --- -rm -rf "$OMARCHY_WINDOWS_DIR" -rm -rf "$MOUNT_ROOT" -rm -f "$HOME/.windows" "$HOME/Windows" -ln -s / "$HOME/.windows" -mkdir -p "$HOME/Windows" -rm -f "$COMPOSE" -write 4G 2 64G dave pw UTC 2>/dev/null && fail "a symlinked legacy data entry was accepted" -[[ ! -f $COMPOSE ]] || fail "no compose is written for a symlinked legacy entry" -[[ ! -L $MOUNT_ROOT/users/$(id -u)/storage ]] || fail "the mount anchor must not remain a symlink" -find "$MOUNT_ROOT/users/$(id -u)" -maxdepth 1 -type l -name 'rejected-storage-*' | grep -q . || fail "the rejected symlink was not quarantined" -pass "migration pins and rejects a symlinked legacy data entry" +# Both sources are pinned before a bind; bad symlinks stay untouched. +reset_case +mkdir -p "$HOME/.windows" +ln -s / "$HOME/Windows" +before_fds=$(fd_count) +prepare_user_mount_sources 2>/dev/null && fail "root symlink passed user preflight" +[[ -L $HOME/Windows && $(readlink "$HOME/Windows") == / ]] || fail "rejected symlink consumed" +printf 'RAM=4G\nCORES=2\nDISK=64G\nUSERNAME=x\nPASSWORD=p\nTZ=UTC\n' | __priv_write_compose 2>/dev/null && fail "root symlink passed privileged preflight" +resolve_caller +[[ $(mount_layer_count "$EXPECTED_STORAGE") == 0 && $(mount_layer_count "$EXPECTED_SHARED") == 0 ]] || fail "one source mounted before other failed" +[[ $(fd_count) == "$before_fds" ]] || fail "source preflight leaked FD" +find "$CALLER_DATA_ROOT" -name 'rejected-*' -print -quit | grep -q . && fail "source was quarantined" +pass "invalid second source leaves paths and anchors untouched and leaks no FD" -# --- credentials are stored privately and round-trip (incl. = in password) --- -export CREDENTIALS_FILE="$TMPDIR/creds" -write_credentials 'carol' 'p=a$$w"x' -[[ $(stat -c '%a' "$CREDENTIALS_FILE") == "600" ]] || fail "credentials file is 0600" -[[ $(read_credential USERNAME) == "carol" ]] || fail "username round-trips" -[[ $(read_credential PASSWORD) == 'p=a$$w"x' ]] || fail "password (with =) round-trips" -pass "credentials are written 0600 and round-trip" +# Distinct caller-owned symlink targets are supported and remain links. +reset_case +external_storage="$TMPDIR/external-storage" +external_shared2="$TMPDIR/external-shared-2" +mkdir -p "$external_storage" "$external_shared2" +ln -s "$external_storage" "$HOME/.windows" +ln -s "$external_shared2" "$HOME/Windows" +prepare_user_mount_sources +write 4G 2 64G symlinked pw UTC +resolve_caller +[[ $(readlink "$HOME/.windows") == "$external_storage" && $(readlink "$HOME/Windows") == "$external_shared2" ]] || fail "writer replaced symlinks" +[[ $(stat -Lc '%d:%i' "$EXPECTED_STORAGE") == $(stat -Lc '%d:%i' "$external_storage") ]] || fail "symlink target not pinned" +pass "legitimate caller-owned symlinks remain in place" + +# Same-inode sources fail before mounting and close both descriptors. +reset_case +same="$TMPDIR/same-source" +mkdir -p "$same" +ln -s "$same" "$HOME/.windows" +ln -s "$same" "$HOME/Windows" +before_fds=$(fd_count) +prepare_user_mount_sources 2>/dev/null && fail "same source passed user preflight" +printf 'RAM=4G\nCORES=2\nDISK=64G\nUSERNAME=x\nPASSWORD=p\nTZ=UTC\n' | __priv_write_compose 2>/dev/null && fail "same source passed root preflight" +resolve_caller +[[ $(mount_layer_count "$EXPECTED_STORAGE") == 0 && $(mount_layer_count "$EXPECTED_SHARED") == 0 ]] || fail "same source left mount" +[[ $(fd_count) == "$before_fds" ]] || fail "same source leaked FDs" +pass "storage and shared must differ and failure closes FDs" + +# Ancestor/descendant aliases are just as destructive as same-inode aliases: +# removal must never recurse from storage into shared (or accept the inverse). +reset_case +shared_inside="$TMPDIR/shared-inside-storage" +mkdir -p "$shared_inside/storage/shared" +ln -s "$shared_inside/storage" "$HOME/.windows" +ln -s "$shared_inside/storage/shared" "$HOME/Windows" +prepare_user_mount_sources +before_fds=$(fd_count) +write 4G 2 64G nested pw UTC 2>/dev/null && fail "shared-inside-storage sources were accepted" +resolve_caller +[[ $(mount_layer_count "$EXPECTED_STORAGE") == 0 && $(mount_layer_count "$EXPECTED_SHARED") == 0 ]] || fail "shared-inside-storage failure left a mount" +[[ $(fd_count) == "$before_fds" ]] || fail "shared-inside-storage failure leaked FDs" + +reset_case +storage_inside="$TMPDIR/storage-inside-shared" +mkdir -p "$storage_inside/shared/storage" +ln -s "$storage_inside/shared/storage" "$HOME/.windows" +ln -s "$storage_inside/shared" "$HOME/Windows" +prepare_user_mount_sources +before_fds=$(fd_count) +write 4G 2 64G nested pw UTC 2>/dev/null && fail "storage-inside-shared sources were accepted" +resolve_caller +[[ $(mount_layer_count "$EXPECTED_STORAGE") == 0 && $(mount_layer_count "$EXPECTED_SHARED") == 0 ]] || fail "storage-inside-shared failure left a mount" +[[ $(fd_count) == "$before_fds" ]] || fail "storage-inside-shared failure leaked FDs" +pass "pinned-FD ancestry checks reject overlap in both directions before mounting" + +# Exact bind-alias bypass regression: the shared FD's visible parent is the +# alias directory, but its inode is still reachable below storage. +reset_case +alias_under="$TMPDIR/bind-alias-under" +alias_shared="$TMPDIR/bind-alias-shared" +mkdir -p "$alias_under/storage/shared" "$alias_shared" +mount --no-canonicalize --bind "$alias_under/storage/shared" "$alias_shared" +ln -s "$alias_under/storage" "$HOME/.windows" +ln -s "$alias_shared" "$HOME/Windows" +prepare_user_mount_sources +before_fds=$(fd_count) +resolve_caller +open_mount_source "$LEGACY_STORAGE" storage +alias_storage_fd=$OPENED_MOUNT_FD +alias_storage_id=$OPENED_MOUNT_ID +open_mount_source "$LEGACY_SHARED" shared +alias_shared_fd=$OPENED_MOUNT_FD +pinned_dir_contains "$alias_storage_id" "$alias_shared_fd" && fail "bind-alias repro unexpectedly shared the underlying parent walk" +pinned_tree_contains "$alias_storage_fd" "$alias_shared_fd" || fail "tree-rooted discovery missed the bind-alias inode" +exec {alias_storage_fd}<&- +exec {alias_shared_fd}<&- +write 4G 2 64G alias pw UTC +resolve_caller +touch "$HOME/.windows/disk.img" "$HOME/Windows/keep.txt" +dc() { :; } +docker() { [[ $1 == inspect ]] && return 1; :; } +__priv_remove 2>/dev/null && fail "removal accepted a shared bind alias into storage" +[[ -f $HOME/.windows/disk.img && -f $HOME/Windows/keep.txt && -f $COMPOSE ]] || fail "bind-alias removal refusal changed state" +[[ $(mount_layer_count "$EXPECTED_STORAGE") == 1 && $(mount_layer_count "$EXPECTED_SHARED") == 1 ]] || fail "bind-alias removal refusal changed mounts" +[[ $(fd_count) == "$before_fds" ]] || fail "bind-alias removal refusal leaked FDs" +unmount_all +umount -- "$alias_shared" +pass "cheap startup permits a bind alias, but bounded removal discovery refuses it" + +# A late writer failure rolls back both newly-created binds. +reset_case +prepare_user_mount_sources +mv() { return 1; } +write 4G 2 64G rollback pw UTC 2>/dev/null && fail "forced writer failure succeeded" +unset -f mv +resolve_caller +[[ $(mount_layer_count "$EXPECTED_STORAGE") == 0 && $(mount_layer_count "$EXPECTED_SHARED") == 0 ]] || fail "writer failure left binds" +[[ ! -f $COMPOSE ]] || fail "writer failure replaced compose" +pass "atomic writer failure rolls back both new bind mounts" + +# Revalidate ancestry during removal: move the already-bound shared inode below +# storage, keep its familiar path as a symlink, and prove nothing is deleted. +reset_case +prepare_user_mount_sources +write 4G 2 64G moved pw UTC +touch "$HOME/.windows/disk.img" "$HOME/Windows/keep.txt" +mv "$HOME/Windows" "$HOME/.windows/moved-shared" +ln -s "$HOME/.windows/moved-shared" "$HOME/Windows" +dc() { :; } +docker() { [[ $1 == inspect ]] && return 1; :; } +__priv_remove 2>/dev/null && fail "removal accepted a shared inode moved below storage" +[[ -f $HOME/.windows/disk.img && -f $HOME/.windows/moved-shared/keep.txt && -f $COMPOSE ]] || fail "overlap rejection changed disk, shared data, or compose" +pass "removal revalidates pinned ancestry and leaves moved shared data untouched" + +# Even when both familiar paths remain disjoint, a same-filesystem bind of the +# pinned shared inode introduced below storage must stop removal before change. +reset_case +prepare_user_mount_sources +write 4G 2 64G removal-alias pw UTC +touch "$HOME/.windows/disk.img" "$HOME/Windows/keep.txt" +mkdir "$HOME/.windows/shared-bind-alias" +mount --no-canonicalize --bind "$HOME/Windows" "$HOME/.windows/shared-bind-alias" +__priv_remove 2>/dev/null && fail "removal missed a shared bind alias introduced below storage" +[[ -f $HOME/.windows/disk.img && -f $HOME/Windows/keep.txt && -f $COMPOSE ]] || fail "removal bind-alias rejection changed state" +umount -- "$HOME/.windows/shared-bind-alias" +pass "removal tree discovery catches a shared alias not used by either home path" + +# A direct alias on another filesystem is still visited by find -xdev at its +# mountpoint and must be rejected, while unrelated separate filesystems remain +# supported by the root suite. +reset_case +prepare_user_mount_sources +mount -t tmpfs -o uid="$(id -u)",gid="$(id -g)",mode=0700,size=8m crossdev-shared "$HOME/Windows" +touch "$HOME/Windows/keep.txt" +write 4G 2 64G crossdev-alias pw UTC +touch "$HOME/.windows/disk.img" +mkdir "$HOME/.windows/crossdev-shared-alias" +mount --no-canonicalize --bind "$HOME/Windows" "$HOME/.windows/crossdev-shared-alias" +__priv_remove 2>/dev/null && fail "removal missed a different-device shared alias below storage" +[[ -f $HOME/.windows/disk.img && -f $HOME/Windows/keep.txt && -f $COMPOSE ]] || fail "cross-device alias rejection changed state" +umount -- "$HOME/.windows/crossdev-shared-alias" +unmount_all +umount -- "$HOME/Windows" +pass "removal catches a direct different-filesystem shared alias at the xdev boundary" + +# Recursive alias discovery is destructive-removal-only and bounded. A hung or +# failing scanner must fail closed before the disk, share, compose, or mounts +# are changed. +reset_case +prepare_user_mount_sources +write 4G 2 64G scan-failure pw UTC +touch "$HOME/.windows/disk.img" "$HOME/Windows/keep.txt" +scan_helper="$TMPDIR/tree-scan-helper" +saved_tree_scan_find=$TREE_SCAN_FIND +saved_tree_scan_timeout=$TREE_SCAN_TIMEOUT_SECONDS +saved_tree_scan_kill_after=$TREE_SCAN_KILL_AFTER_SECONDS +printf '#!/bin/bash\n/bin/sleep 10\n' >"$scan_helper" +chmod 0700 "$scan_helper" +TREE_SCAN_FIND=$scan_helper +TREE_SCAN_TIMEOUT_SECONDS=0.05 +TREE_SCAN_KILL_AFTER_SECONDS=0.05 +__priv_remove 2>/dev/null && fail "removal continued after its containment scan timed out" +[[ -f $HOME/.windows/disk.img && -f $HOME/Windows/keep.txt && -f $COMPOSE ]] || fail "timed-out containment scan changed state" +[[ $(mount_layer_count "$EXPECTED_STORAGE") == 1 && $(mount_layer_count "$EXPECTED_SHARED") == 1 ]] || fail "timed-out containment scan changed mounts" + +printf '#!/bin/bash\nexit 42\n' >"$scan_helper" +__priv_remove 2>/dev/null && fail "removal continued after its containment scanner failed" +[[ -f $HOME/.windows/disk.img && -f $HOME/Windows/keep.txt && -f $COMPOSE ]] || fail "failed containment scan changed state" +[[ $(mount_layer_count "$EXPECTED_STORAGE") == 1 && $(mount_layer_count "$EXPECTED_SHARED") == 1 ]] || fail "failed containment scan changed mounts" +TREE_SCAN_FIND=$saved_tree_scan_find +TREE_SCAN_TIMEOUT_SECONDS=$saved_tree_scan_timeout +TREE_SCAN_KILL_AFTER_SECONDS=$saved_tree_scan_kill_after +pass "removal scan timeout and errors fail closed without changing VM state" + +# Removal rejects stacks, then deletes disk only through verified binds. +reset_case +prepare_user_mount_sources +write 4G 2 64G remove pw UTC +resolve_caller +touch "$HOME/.windows/disk.img" "$HOME/Windows/keep.txt" +mount --no-canonicalize --bind "$HOME/.windows" "$EXPECTED_STORAGE" +dc() { :; } +docker() { [[ $1 == inspect ]] && return 1; :; } +__priv_remove 2>/dev/null && fail "removal accepted stacked storage mount" +[[ -f $HOME/.windows/disk.img && -f $HOME/Windows/keep.txt && -f $COMPOSE ]] || fail "rejected removal changed state" +umount -- "$EXPECTED_STORAGE" +dc() { return 1; } +__priv_remove 2>/dev/null && fail "removal deleted data after docker-compose down failed" +[[ -f $HOME/.windows/disk.img && -f $HOME/Windows/keep.txt && -f $COMPOSE ]] || fail "failed down changed data or compose" +dc() { :; } +__priv_remove +[[ ! -e $HOME/.windows/disk.img ]] || fail "removal preserved disk data" +[[ -e $HOME/Windows/keep.txt ]] || fail "removal deleted shared data" +[[ ! -f $COMPOSE ]] || fail "removal left compose" +resolve_caller +[[ $(mount_layer_count "$EXPECTED_STORAGE") == 0 && $(mount_layer_count "$EXPECTED_SHARED") == 0 ]] || fail "removal left binds" +pass "removal rejects stacks, deletes disk, and preserves shared files" + +# Credentials replace a planted link rather than following it, and a failed +# atomic rename preserves the last complete private file. +credentials_dir="$TMPDIR/credentials" +CREDENTIALS_FILE="$credentials_dir/credentials" +credentials_victim="$TMPDIR/credentials-victim" +mkdir -m 0755 -p "$credentials_dir" +printf 'victim\n' >"$credentials_victim" +ln -s "$credentials_victim" "$CREDENTIALS_FILE" +write_credentials carol 'p=a$$w"x' +[[ -f $CREDENTIALS_FILE && ! -L $CREDENTIALS_FILE ]] || fail "credentials did not replace a planted symlink" +[[ $(stat -c '%a' "$credentials_dir") == 700 && $(stat -c '%a' "$CREDENTIALS_FILE") == 600 ]] || fail "credentials path is not private" +[[ $(cat "$credentials_victim") == victim ]] || fail "credentials write changed a symlink victim" +[[ $(read_credential USERNAME) == carol && $(read_credential PASSWORD) == 'p=a$$w"x' ]] || fail "credentials did not round-trip" +credentials_before=$(cat "$CREDENTIALS_FILE") +mv() { return 1; } +write_credentials changed replacement 2>/dev/null && fail "forced credentials rename failure succeeded" +unset -f mv +[[ $(cat "$CREDENTIALS_FILE") == "$credentials_before" ]] || fail "failed credentials rename replaced the live file" +! find "$credentials_dir" -name '.credentials.*' -print -quit | grep -q . || fail "failed credentials write left a temporary file" +pass "credentials are atomically replaced as a private regular file" + +# Free-space accounting follows the real storage target. +reset_case +mkdir -p "$external_storage" "$HOME/Windows" +ln -s "$external_storage" "$HOME/.windows" +prepare_user_mount_sources +df_log="$TMPDIR/df-path" +df() { + printf '%s\n' "${!#}" >"$df_log" + printf 'Filesystem 1024-blocks Used Available Capacity Mounted on\nmock 104857600 0 94371840 0%% /mock\n' +} +[[ $(available_storage_gb) == 90 ]] || fail "free-space parsed wrong value" +unset -f df +[[ $(cat "$df_log") == "$external_storage" ]] || fail "free-space used home filesystem" +pass "disk-space checks follow the storage symlink target" diff --git a/test/shell.d/windows-vm-mount-boundary-test.sh b/test/shell.d/windows-vm-mount-boundary-test.sh new file mode 100644 index 00000000..509143b2 --- /dev/null +++ b/test/shell.d/windows-vm-mount-boundary-test.sh @@ -0,0 +1,168 @@ +#!/bin/bash +# Exercise the real EUID-0/PKEXEC_UID boundary in an isolated user+mount namespace. + +set -euo pipefail +source "$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")" && pwd)/base-test.sh" + +if ((EUID != 0)); then + if unshare --user --map-auto --map-root-user --mount true 2>/dev/null; then + exec unshare --user --map-auto --map-root-user --mount --propagation private bash "$0" + fi + pass "automatic subordinate-id namespace unavailable; skipping root Windows VM boundary probe" + exit 0 +fi + +test_tmp=$(mktemp -d) +trap 'rm -rf "$test_tmp"' EXIT + +# Hide host state before creating the production paths used by the root helper. +mount -t tmpfs -o mode=0755,size=8m run-test /run +mkdir -p /run/lock +mount -t tmpfs -o mode=0755,size=16m var-test /var +mkdir -p /var/lib/omarchy +mount -t tmpfs -o mode=0755,size=16m home-parent /home +mkdir /home/alice +mount -t tmpfs -o uid=0,gid=0,mode=0710,size=1g home-alice /home/alice + +export HOME=/home/alice +unset OMARCHY_WINDOWS_DIR +set -- help +source "$ROOT/bin/omarchy-windows-vm" >/dev/null 2>&1 + +# The namespace maps the host filesystem's uid 0 to nobody. Only / remains on +# that filesystem; all paths the helper mutates are isolated tmpfs mounts. +stat() { + if [[ ${!#} == / && $* == *"%u"* ]]; then printf '0\n'; return; fi + command stat "$@" +} + +TEST_PASSWD_HOME=/home/alice +getent() { + if [[ $1 == passwd && ${2:-} == 1000 ]]; then + printf 'alice:x:1000:1000::%s:/bin/bash\n' "$TEST_PASSWD_HOME" + return 0 + fi + return 2 +} + +assert_no_runtime_mutation() { + [[ ! -e /var/lib/omarchy/windows && ! -L /var/lib/omarchy/windows ]] || + fail "$1 mutated the production runtime" +} + +unset PKEXEC_UID +resolve_caller 2>/dev/null && fail "root accepted missing PKEXEC_UID" +assert_no_runtime_mutation "missing PKEXEC_UID" +PKEXEC_UID=0 +resolve_caller 2>/dev/null && fail "root accepted PKEXEC_UID=0" +assert_no_runtime_mutation "zero PKEXEC_UID" +PKEXEC_UID=not-a-number +resolve_caller 2>/dev/null && fail "root accepted nonnumeric PKEXEC_UID" +assert_no_runtime_mutation "nonnumeric PKEXEC_UID" +PKEXEC_UID=1001 +resolve_caller 2>/dev/null && fail "root accepted uid absent from passwd" +assert_no_runtime_mutation "missing passwd entry" + +PKEXEC_UID=1000 +resolve_caller 2>/dev/null && fail "root accepted a home not owned by caller" +assert_no_runtime_mutation "wrong-owned home" +chown 1000:1000 /home/alice + +chmod 0777 /home +resolve_caller 2>/dev/null && fail "root accepted writable home parent" +assert_no_runtime_mutation "writable parent" +chmod 0755 /home + +mkdir /home/real-alice +chown 1000:1000 /home/real-alice +ln -s /home/real-alice /home/link-alice +TEST_PASSWD_HOME=/home/link-alice +resolve_caller 2>/dev/null && fail "root accepted symlinked passwd home" +assert_no_runtime_mutation "symlinked home" +TEST_PASSWD_HOME=/home/alice +resolve_caller || fail "valid root PKEXEC_UID/home boundary was rejected" +pass "root dispatch rejects missing/invalid uid, passwd, owner, symlink, and writable-parent boundaries without mutation" + +# Put each familiar source on its own filesystem. Both start with legacy 0755 +# permissions and world-readable payloads to prove migration hardens the leaves. +mkdir /home/storage-target /home/shared-target +mount -t tmpfs -o uid=1000,gid=1000,mode=0755,size=3g storage-test /home/storage-target +mount -t tmpfs -o uid=1000,gid=1000,mode=0755,size=64m shared-test /home/shared-target +ln -s /home/storage-target /home/alice/.windows +ln -s /home/shared-target /home/alice/Windows +chown -h 1000:1000 /home/alice/.windows /home/alice/Windows +printf disk >/home/storage-target/disk.img +printf shared >/home/shared-target/shared.txt +chown 1000:1000 /home/storage-target/disk.img /home/shared-target/shared.txt +chmod 0644 /home/storage-target/disk.img /home/shared-target/shared.txt + +home_dev=$(command stat -Lc '%d' /home/alice) +storage_dev=$(command stat -Lc '%d' /home/storage-target) +[[ $home_dev != "$storage_dev" ]] || fail "storage target did not land on a separate filesystem" + +with_vm_lock prepare_caller_mounts || fail "root could not create verified production bind anchors" +resolve_caller +[[ $(readlink /home/alice/.windows) == /home/storage-target && + $(readlink /home/alice/Windows) == /home/shared-target ]] || fail "root consumed legitimate symlinks" +[[ $(command stat -Lc '%d:%i' "$EXPECTED_STORAGE") == $(command stat -Lc '%d:%i' /home/storage-target) ]] || fail "storage bind identity differs from pinned source" +[[ $(command stat -Lc '%d:%i' "$EXPECTED_SHARED") == $(command stat -Lc '%d:%i' /home/shared-target) ]] || fail "shared bind identity differs from pinned source" +[[ $(command stat -Lc '%d' "$CALLER_DATA_ROOT") != "$storage_dev" ]] || fail "Docker boundary unexpectedly shares the storage filesystem" +[[ $(command stat -Lc '%u:%a' "$MOUNT_ROOT") == 0:711 && + $(command stat -Lc '%u:%a' "$CALLER_DATA_ROOT") == 0:711 ]] || fail "production ancestors are not root-owned/private-boundary modes" +[[ $(command stat -Lc '%u:%a' "$EXPECTED_STORAGE") == 1000:700 && + $(command stat -Lc '%u:%a' "$EXPECTED_SHARED") == 1000:700 ]] || fail "migrated leaves are not caller-owned 0700" +if setpriv --reuid=1001 --regid=1001 --clear-groups cat "$EXPECTED_STORAGE/disk.img" >/dev/null 2>&1; then + fail "another local account read the VM disk through its anchor" +fi +if setpriv --reuid=1001 --regid=1001 --clear-groups cat "$EXPECTED_SHARED/shared.txt" >/dev/null 2>&1; then + fail "another local account read shared files through their anchor" +fi +pass "cross-filesystem symlink sources bind by identity and migrated 0700 leaves deny another account" + +expected_space=$(command df -P -- /home/storage-target | awk 'NR==2 {print int($4/1024/1024)}') +actual_space=$(available_storage_gb) +[[ $actual_space == "$expected_space" ]] || fail "disk-space helper did not measure the storage target filesystem" +[[ $(command df -P -- /home/alice | awk 'NR==2 {print int($4/1024/1024)}') != "$actual_space" ]] || fail "test filesystems do not distinguish home from storage" +pass "disk-space accounting measures the actual storage filesystem, not home" + +# Exercise the real root writer and final guard against the production paths. +printf 'RAM=4G\nCORES=2\nDISK=64G\nUSERNAME=alice\nPASSWORD=pw\nTZ=UTC\n' | + with_vm_lock __priv_write_compose +[[ $(command stat -Lc '%u:%a' "$COMPOSE_FILE") == 0:640 ]] || fail "root compose ownership/mode is wrong" +with_vm_lock assert_mounts_safe || fail "final root mount/compose assertion rejected the verified pair" +pass "root writer and final pre-Docker guard revalidate the pinned production mounts" + +# Upgrade the exact sibling-anchor pair emitted by the earlier fix without +# moving or replacing either familiar home symlink. +sed -i "s|$EXPECTED_STORAGE:/storage|$OLD_EXPECTED_STORAGE:/storage|" "$COMPOSE_FILE" +sed -i "s|$EXPECTED_SHARED:/shared|$OLD_EXPECTED_SHARED:/shared|" "$COMPOSE_FILE" +compose_needs_mount_migration || fail "previous protected anchor pair was not recognized for upgrade" +with_vm_lock assert_mounts_safe || fail "root could not upgrade previous protected anchors" +grep -q -- "- $EXPECTED_STORAGE:/storage" "$COMPOSE_FILE" || fail "upgrade did not rewrite storage anchor" +grep -q -- "- $EXPECTED_SHARED:/shared" "$COMPOSE_FILE" || fail "upgrade did not rewrite shared anchor" +[[ $(readlink /home/alice/.windows) == /home/storage-target ]] || fail "protected-anchor upgrade replaced home storage link" +pass "previous sibling-anchor installs upgrade in place to the fixed /var/lib boundary" + +# Preflight both sources before either bind on a clean anchor pair. +umount "$EXPECTED_SHARED" +umount "$EXPECTED_STORAGE" +rm /home/alice/Windows +ln -s / /home/alice/Windows +chown -h 1000:1000 /home/alice/Windows +with_vm_lock prepare_caller_mounts 2>/dev/null && fail "root accepted a non-caller-owned second source" +[[ $(mount_layer_count "$EXPECTED_STORAGE") == 0 && $(mount_layer_count "$EXPECTED_SHARED") == 0 ]] || fail "failed second-source preflight left a partial bind" +[[ $(readlink /home/alice/Windows) == / ]] || fail "failed preflight consumed or quarantined symlink" +pass "root preflights both sources before mounting either and preserves rejection evidence" + +# mountpoint(1) follows symlinks, so explicitly pin the invariant that even a +# root-planted anchor symlink to the expected mounted source is rejected. +rm /home/alice/Windows +ln -s /home/shared-target /home/alice/Windows +chown -h 1000:1000 /home/alice/Windows +rmdir "$EXPECTED_STORAGE" +ln -s /home/storage-target "$EXPECTED_STORAGE" +storage_id=$(command stat -Lc '%d:%i' /home/storage-target) +mounted_leaf_matches "$EXPECTED_STORAGE" "$storage_id" && fail "symlink mount anchor passed final identity check" +with_vm_lock prepare_caller_mounts 2>/dev/null && fail "root followed a symlink mount anchor" +[[ -L $EXPECTED_STORAGE ]] || fail "rejected anchor symlink was consumed" +pass "final guard rejects a symlink even when it resolves to the expected mounted source" From 4fc14173b79ad42b6f67550248c1810f71648398 Mon Sep 17 00:00:00 2001 From: Afonso Oliveira Date: Fri, 28 Aug 2026 19:37:04 +0100 Subject: [PATCH 3/5] [Security] Add Windows VM boundary race regressions --- test/shell.d/windows-vm-compose-test.sh | 23 +++++++++++++++++++ .../shell.d/windows-vm-mount-boundary-test.sh | 20 ++++++++++++++++ 2 files changed, 43 insertions(+) diff --git a/test/shell.d/windows-vm-compose-test.sh b/test/shell.d/windows-vm-compose-test.sh index 77c298a8..0495e1e0 100644 --- a/test/shell.d/windows-vm-compose-test.sh +++ b/test/shell.d/windows-vm-compose-test.sh @@ -183,6 +183,29 @@ resolve_caller [[ $(stat -Lc '%d:%i' "$EXPECTED_STORAGE") == $(stat -Lc '%d:%i' "$external_storage") ]] || fail "symlink target not pinned" pass "legitimate caller-owned symlinks remain in place" +# Reproduce the original post-validation race at the last possible moment: +# replace the familiar shared path with / only after the final guard returns, +# inside the mocked Docker Compose invocation. Compose must still consume the +# protected anchor bound to the inode that was validated earlier. +raced_shared="$HOME/Windows.before-race" +shared_id_before_race=$(stat -Lc '%d:%i' "$external_shared2") +race_ran=0 +dc() { + [[ $1 == up && ${2:-} == -d ]] || return 1 + mv -T -- "$HOME/Windows" "$raced_shared" + ln -s / "$HOME/Windows" + race_ran=1 + [[ $(get_mount_source /shared) == "$EXPECTED_SHARED" ]] || return 1 + [[ $(stat -Lc '%d:%i' "$EXPECTED_SHARED") == "$shared_id_before_race" ]] || return 1 +} +__priv_up || fail "post-validation home-path swap changed the Docker mount source" +(( race_ran == 1 )) || fail "post-validation race hook did not run" +[[ -L $HOME/Windows && $(readlink "$HOME/Windows") == / ]] || fail "race did not replace the familiar shared path" +rm "$HOME/Windows" +mv -T -- "$raced_shared" "$HOME/Windows" +unset -f dc +pass "a post-validation path swap cannot redirect Docker away from the pinned shared inode" + # Same-inode sources fail before mounting and close both descriptors. reset_case same="$TMPDIR/same-source" diff --git a/test/shell.d/windows-vm-mount-boundary-test.sh b/test/shell.d/windows-vm-mount-boundary-test.sh index 509143b2..a49e5652 100644 --- a/test/shell.d/windows-vm-mount-boundary-test.sh +++ b/test/shell.d/windows-vm-mount-boundary-test.sh @@ -119,6 +119,26 @@ if setpriv --reuid=1001 --regid=1001 --clear-groups cat "$EXPECTED_SHARED/shared fi pass "cross-filesystem symlink sources bind by identity and migrated 0700 leaves deny another account" +# Existing production boundary components are never repaired in place when +# their ownership or write permissions are unsafe. Both the preparation path +# and the final pre-Docker guard must fail closed without disturbing the binds. +chmod 0731 "$MOUNT_ROOT" +with_vm_lock prepare_caller_mounts 2>/dev/null && fail "root repaired a group-writable mount boundary instead of rejecting it" +mounts_ready 2>/dev/null && fail "final guard accepted a group-writable mount boundary" +[[ $(command stat -Lc '%a' "$MOUNT_ROOT") == 731 ]] || fail "rejection unexpectedly changed the writable boundary" +chmod 0711 "$MOUNT_ROOT" + +chown 1000:1000 "$USERS_DIR" +with_vm_lock prepare_caller_mounts 2>/dev/null && fail "root repaired a caller-owned mount boundary instead of rejecting it" +mounts_ready 2>/dev/null && fail "final guard accepted a caller-owned mount boundary" +[[ $(command stat -Lc '%u' "$USERS_DIR") == 1000 ]] || fail "rejection unexpectedly changed the boundary owner" +chown root:root "$USERS_DIR" + +[[ $(mount_layer_count "$EXPECTED_STORAGE") == 1 && + $(mount_layer_count "$EXPECTED_SHARED") == 1 ]] || fail "boundary rejection changed the verified mount pair" +mounts_ready || fail "restored production boundaries were rejected" +pass "root rejects wrong-owned and group-writable production mount boundaries without mutation" + expected_space=$(command df -P -- /home/storage-target | awk 'NR==2 {print int($4/1024/1024)}') actual_space=$(available_storage_gb) [[ $actual_space == "$expected_space" ]] || fail "disk-space helper did not measure the storage target filesystem" From bf10b75150f7cd89d7ab42b0f56818e6a7792525 Mon Sep 17 00:00:00 2001 From: Erik Melton <798237+ErikMelton@users.noreply.github.com> Date: Sat, 29 Aug 2026 16:00:10 +0200 Subject: [PATCH 4/5] Protect the Windows VM web console --- bin/omarchy-windows-vm | 54 ++++++++++++++----- manual/28-windows-vm.md | 4 +- test/shell.d/windows-vm-compose-test.sh | 9 +++- .../shell.d/windows-vm-mount-boundary-test.sh | 12 ++++- 4 files changed, 63 insertions(+), 16 deletions(-) diff --git a/bin/omarchy-windows-vm b/bin/omarchy-windows-vm index 0676b770..f1582e2d 100755 --- a/bin/omarchy-windows-vm +++ b/bin/omarchy-windows-vm @@ -92,7 +92,7 @@ priv() { # still needs one privileged invocation. if [[ -d $VM_LOCK_DIR && ! -L $VM_LOCK_DIR && -r $VM_LOCK_DIR && -x $VM_LOCK_DIR ]] && { [[ $action != up && $action != up_wait ]] || { - ! compose_needs_mount_migration && mounts_ready >/dev/null 2>&1 + ! compose_needs_security_migration && mounts_ready >/dev/null 2>&1 } }; then with_vm_lock "__priv_$action" "$@" @@ -703,6 +703,7 @@ services: DISK_SIZE: "$disk" USERNAME: "$username" PASSWORD: "$esc_password" + PROTECT: "Y" TZ: "$tz" ARGUMENTS: "-rtc base=localtime,clock=host,driftfix=slew" devices: @@ -765,14 +766,20 @@ get_mount_source() { sed -n "s|^[[:space:]]*-[[:space:]]*\(/[^:]*\):$1\$|\1|p" "$COMPOSE_FILE" | head -n1 } -# True only for the exact pair emitted by older Omarchy releases. The result is -# used by priv() to force a one-time elevated migration for sudoless-Docker -# users; arbitrary or mixed bind sources are never classified as migratable. -compose_needs_mount_migration() { +# True only when a trusted compose has an exact security upgrade path. The +# result forces a one-time elevated migration for sudoless-Docker users. An +# arbitrary or mixed bind pair is never classified as migratable. +compose_needs_security_migration() { + local storage shared [[ -f $COMPOSE_FILE ]] || return 1 resolve_caller || return 1 [[ $(mount_source_count /storage) == 1 && $(mount_source_count /shared) == 1 ]] || return 1 - compose_mount_pair_is_migratable "$(get_mount_source /storage)" "$(get_mount_source /shared)" + storage=$(get_mount_source /storage) + shared=$(get_mount_source /shared) + if compose_mount_pair_is_migratable "$storage" "$shared"; then + return 0 + fi + [[ $storage == "$EXPECTED_STORAGE" && $shared == "$EXPECTED_SHARED" ]] && ! compose_web_protected } compose_mount_pair_is_migratable() { @@ -786,14 +793,26 @@ mount_source_count() { sed -n "s|^[[:space:]]*-[[:space:]]*\(/[^:]*\):$destination\$|x|p" "$COMPOSE_FILE" | wc -l } -rewrite_compose_mounts() { +compose_web_protected() { + [[ $(sed -n 's/^[[:space:]]*PROTECT:.*$/x/p' "$COMPOSE_FILE" | wc -l) == 1 && + $(sed -n 's/^[[:space:]]*PROTECT:[[:space:]]*"Y"[[:space:]]*$/x/p' "$COMPOSE_FILE" | wc -l) == 1 ]] +} + +rewrite_compose_security() { local tmp tmp=$(mktemp "$RUNTIME_DIR/.compose.XXXXXX") || return 1 awk -v storage="$EXPECTED_STORAGE" -v shared="$EXPECTED_SHARED" ' + /^ environment:$/ { print; print " PROTECT: \"Y\""; next } + /^[[:space:]]+PROTECT:/ { next } /^[[:space:]]*-[[:space:]]*\/[^:]*:\/storage$/ { print " - " storage ":/storage"; next } /^[[:space:]]*-[[:space:]]*\/[^:]*:\/shared$/ { print " - " shared ":/shared"; next } { print } ' "$COMPOSE_FILE" >"$tmp" || { rm -f "$tmp"; return 1; } + [[ $(sed -n 's/^[[:space:]]*PROTECT:.*$/x/p' "$tmp" | wc -l) == 1 && + $(sed -n 's/^[[:space:]]*PROTECT:[[:space:]]*"Y"[[:space:]]*$/x/p' "$tmp" | wc -l) == 1 ]] || { + rm -f "$tmp" + return 1 + } chmod 0640 "$tmp" || { rm -f "$tmp"; return 1; } if ((EUID == 0)); then chown root:docker "$tmp" 2>/dev/null || chown root:root "$tmp" || { @@ -814,7 +833,7 @@ assert_compose_trusted() { } assert_mounts_safe() { - local storage shared + local storage shared needs_rewrite=0 mounts_prepared=0 resolve_caller || return 1 assert_compose_trusted || { echo "omarchy-windows-vm: refusing an untrusted compose file" >&2 @@ -834,10 +853,8 @@ assert_mounts_safe() { return 1 } prepare_caller_mounts || return 1 - if ! rewrite_compose_mounts; then - rollback_new_caller_mounts || true - return 1 - fi + mounts_prepared=1 + needs_rewrite=1 storage=$EXPECTED_STORAGE shared=$EXPECTED_SHARED fi @@ -847,6 +864,19 @@ assert_mounts_safe() { return 1 } + if ! compose_web_protected; then + ((EUID == 0)) || { + echo "omarchy-windows-vm: web-console protection needs an authorized migration" >&2 + return 1 + } + needs_rewrite=1 + fi + + if ((needs_rewrite)) && ! rewrite_compose_security; then + if ((mounts_prepared)); then rollback_new_caller_mounts || true; fi + return 1 + fi + # Mounts disappear at reboot. Root recreates them from the already-opened, # caller-owned sources; a docker-group invocation may proceed directly only # while the exact pinned pair is still present. diff --git a/manual/28-windows-vm.md b/manual/28-windows-vm.md index 29946466..0551b23f 100644 --- a/manual/28-windows-vm.md +++ b/manual/28-windows-vm.md @@ -4,7 +4,7 @@ Omarchy offers an easy way to run Windows through a Docker VM. You can install i Your machine needs KVM virtualization for this, which most do — but it's sometimes switched off in the BIOS, and the installer will tell you if that's the case. You'll also want the disk space: whatever you give Windows, plus about 10GB for the image itself. -The installer asks how much RAM, how many CPU cores, and how much disk to hand over (64GB or more is the sensible floor), then for a Windows username and password. Leave those blank and you get `docker` / `admin`. The download takes a while — 10-15 minutes is normal — and you can follow the progress in the browser at `http://127.0.0.1:8006`. +The installer asks how much RAM, how many CPU cores, and how much disk to hand over (64GB or more is the sensible floor), then for a Windows username and password. Leave those blank and you get `docker` / `admin`. The download takes a while — 10-15 minutes is normal — and you can follow the progress in the browser at `http://127.0.0.1:8006`. The browser prompts for the same username and password before opening the console. ![windows-vm](images/windows-vm.webp) @@ -34,7 +34,7 @@ Keep the disk and shared paths as separate, non-overlapping directories. Removal Before the VM starts, Omarchy opens and pins those two directories, then bind-mounts the exact directory inodes onto private per-user anchors below `/var/lib/omarchy/windows/mounts`. Docker only sees those root-protected anchors. This preserves custom disk locations while preventing another process running as you from swapping a checked path before the privileged container consumes it. Existing disk and shared directories are tightened to mode `0700` during migration so other local accounts cannot browse their contents. -The VM's ports are bound to localhost only, so nothing on your network can reach the Windows machine. +The VM's ports are bound to localhost only, so nothing on your network can reach the Windows machine. The web console also requires the configured Windows username and password, preventing another local account from driving the VM through port 8006. ## Limits and licensing diff --git a/test/shell.d/windows-vm-compose-test.sh b/test/shell.d/windows-vm-compose-test.sh index 0495e1e0..64825526 100644 --- a/test/shell.d/windows-vm-compose-test.sh +++ b/test/shell.d/windows-vm-compose-test.sh @@ -61,6 +61,7 @@ grep -q 'image: dockurr/windows' "$COMPOSE" || fail "image is pinned" grep -q -- '- NET_ADMIN' "$COMPOSE" || fail "cap_add is pinned" grep -q -- "- $EXPECTED_STORAGE:/storage" "$COMPOSE" || fail "storage uses the protected anchor" grep -q -- "- $EXPECTED_SHARED:/shared" "$COMPOSE" || fail "shared uses the protected anchor" +grep -q 'PROTECT: "Y"' "$COMPOSE" || fail "web console is not password protected" [[ ! -L $HOME/.windows && ! -L $HOME/Windows ]] || fail "fresh sources stay real directories" [[ $(stat -Lc '%d:%i' "$HOME/.windows") == $(stat -Lc '%d:%i' "$EXPECTED_STORAGE") ]] || fail "storage bind did not pin source" [[ $(stat -Lc '%d:%i' "$HOME/Windows") == $(stat -Lc '%d:%i' "$EXPECTED_SHARED") ]] || fail "shared bind did not pin source" @@ -150,10 +151,16 @@ sed -i "s|/etc:/shared|$EXPECTED_SHARED:/shared|" "$COMPOSE" printf ' - %s:/storage\n' "$EXPECTED_STORAGE" >>"$COMPOSE" assert_mounts_safe 2>/dev/null && fail "duplicate destination accepted" write 16G 6 128G legacyuser legacypass America/New_York +sed -i 's/PROTECT: "Y"/PROTECT: "N"/' "$COMPOSE" +assert_mounts_safe 2>/dev/null && fail "unprotected web console accepted" +sed -i 's/PROTECT: "N"/PROTECT: "Y"/' "$COMPOSE" +printf ' PROTECT: "N"\n' >>"$COMPOSE" +assert_mounts_safe 2>/dev/null && fail "duplicate web protection setting accepted" +sed -i '$d' "$COMPOSE" chmod 0666 "$COMPOSE" assert_mounts_safe 2>/dev/null && fail "writable compose accepted" chmod 0640 "$COMPOSE" -pass "bring-up rejects tampered, duplicate, and writable compose inputs" +pass "bring-up rejects tampered, duplicate, unprotected, and writable compose inputs" # Both sources are pinned before a bind; bad symlinks stay untouched. reset_case diff --git a/test/shell.d/windows-vm-mount-boundary-test.sh b/test/shell.d/windows-vm-mount-boundary-test.sh index a49e5652..34f36cc2 100644 --- a/test/shell.d/windows-vm-mount-boundary-test.sh +++ b/test/shell.d/windows-vm-mount-boundary-test.sh @@ -156,13 +156,23 @@ pass "root writer and final pre-Docker guard revalidate the pinned production mo # moving or replacing either familiar home symlink. sed -i "s|$EXPECTED_STORAGE:/storage|$OLD_EXPECTED_STORAGE:/storage|" "$COMPOSE_FILE" sed -i "s|$EXPECTED_SHARED:/shared|$OLD_EXPECTED_SHARED:/shared|" "$COMPOSE_FILE" -compose_needs_mount_migration || fail "previous protected anchor pair was not recognized for upgrade" +sed -i '/PROTECT: "Y"/d' "$COMPOSE_FILE" +compose_needs_security_migration || fail "previous protected compose was not recognized for upgrade" with_vm_lock assert_mounts_safe || fail "root could not upgrade previous protected anchors" grep -q -- "- $EXPECTED_STORAGE:/storage" "$COMPOSE_FILE" || fail "upgrade did not rewrite storage anchor" grep -q -- "- $EXPECTED_SHARED:/shared" "$COMPOSE_FILE" || fail "upgrade did not rewrite shared anchor" +grep -q 'PROTECT: "Y"' "$COMPOSE_FILE" || fail "upgrade did not protect the web console" [[ $(readlink /home/alice/.windows) == /home/storage-target ]] || fail "protected-anchor upgrade replaced home storage link" pass "previous sibling-anchor installs upgrade in place to the fixed /var/lib boundary" +# A compose that already uses the fixed anchors still needs an authorized +# upgrade when it predates web-console authentication. +sed -i '/PROTECT: "Y"/d' "$COMPOSE_FILE" +compose_needs_security_migration || fail "unprotected fixed-anchor compose was not recognized for upgrade" +with_vm_lock assert_mounts_safe || fail "root could not protect an existing fixed-anchor compose" +grep -q 'PROTECT: "Y"' "$COMPOSE_FILE" || fail "fixed-anchor upgrade did not protect the web console" +pass "existing fixed-anchor compose gains web-console authentication" + # Preflight both sources before either bind on a clean anchor pair. umount "$EXPECTED_SHARED" umount "$EXPECTED_STORAGE" From 11fa6b9809094e347e47a67d9c0962aa4b37e3b9 Mon Sep 17 00:00:00 2001 From: Erik Melton <798237+ErikMelton@users.noreply.github.com> Date: Sat, 29 Aug 2026 16:02:38 +0200 Subject: [PATCH 5/5] Race Windows VM mount sources concurrently --- test/shell.d/windows-vm-compose-test.sh | 59 +++++++++++++++++++++++++ 1 file changed, 59 insertions(+) diff --git a/test/shell.d/windows-vm-compose-test.sh b/test/shell.d/windows-vm-compose-test.sh index 64825526..189536f4 100644 --- a/test/shell.d/windows-vm-compose-test.sh +++ b/test/shell.d/windows-vm-compose-test.sh @@ -213,6 +213,65 @@ mv -T -- "$raced_shared" "$HOME/Windows" unset -f dc pass "a post-validation path swap cannot redirect Docker away from the pinned shared inode" +# Run the same attack as a genuinely concurrent process. A successful bring-up +# deliberately waits inside the Docker boundary until the attacker has replaced +# the familiar path with /, then verifies that the real bind anchor still names +# the caller-owned directory that was pinned before the race. +reset_case +prepare_user_mount_sources +touch "$HOME/Windows/safe-marker" +write 4G 2 64G concurrent pw UTC +resolve_caller +concurrent_shared_id=$(stat -Lc '%d:%i' "$HOME/Windows") +host_root_id=$(stat -Lc '%d:%i' /) +race_source="$HOME/Windows.race-source" +race_stop="$TMPDIR/stop-concurrent-race" +race_swaps="$TMPDIR/concurrent-race-swaps" +( + set +e + while [[ ! -e $race_stop ]]; do + if [[ -d $HOME/Windows && ! -L $HOME/Windows ]] && mv -T -- "$HOME/Windows" "$race_source" 2>/dev/null; then + ln -s / "$HOME/Windows" 2>/dev/null || true + printf x >>"$race_swaps" + sleep 0.002 + fi + if [[ -L $HOME/Windows ]]; then + rm -f -- "$HOME/Windows" + mv -T -- "$race_source" "$HOME/Windows" 2>/dev/null || true + sleep 0.005 + fi + done +) & +racer_pid=$! +concurrent_dc_calls=0 +dc() { + local attempt + [[ $1 == up && ${2:-} == -d ]] || return 1 + for ((attempt = 0; attempt < 20000; attempt++)); do + if [[ -L $HOME/Windows && $(readlink "$HOME/Windows" 2>/dev/null) == / ]]; then + break + fi + done + [[ -L $HOME/Windows && $(readlink "$HOME/Windows" 2>/dev/null) == / ]] || return 1 + ((concurrent_dc_calls++)) + [[ $(get_mount_source /shared) == "$EXPECTED_SHARED" ]] || return 1 + [[ $(stat -Lc '%d:%i' "$EXPECTED_SHARED") == "$concurrent_shared_id" ]] || return 1 + [[ $(stat -Lc '%d:%i' "$EXPECTED_SHARED") != "$host_root_id" ]] || return 1 + [[ -f $EXPECTED_SHARED/safe-marker ]] +} +for ((attempt = 0; attempt < 200; attempt++)); do + if __priv_up 2>/dev/null; then break; fi +done +touch "$race_stop" +wait "$racer_pid" +unset -f dc +if [[ -L $HOME/Windows ]]; then rm -f -- "$HOME/Windows"; fi +if [[ ! -e $HOME/Windows && -d $race_source ]]; then mv -T -- "$race_source" "$HOME/Windows"; fi +[[ -s $race_swaps ]] || fail "concurrent attacker never swapped the shared path" +((concurrent_dc_calls > 0)) || fail "concurrent race never reached Docker while the familiar path named host root" +[[ $(stat -Lc '%d:%i' "$EXPECTED_SHARED") == "$concurrent_shared_id" ]] || fail "concurrent race changed the protected shared inode" +pass "a concurrent home-path swap cannot redirect Docker away from the pinned shared inode" + # Same-inode sources fail before mounting and close both descriptors. reset_case same="$TMPDIR/same-source"