From 0aa92e128e31110387214427b386bbe1dbac15bb Mon Sep 17 00:00:00 2001 From: librelad Date: Fri, 28 Aug 2026 08:48:03 +0100 Subject: [PATCH] webui: a one-shot channel for secrets, instead of the command line MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A password typed in the WebUI has to reach the host, and both existing routes leak it. As part of a task's command string it lands in frontend/data/tasks/*.json — 0644, inside a world-readable directory — and is visible in `ps` while the task runs; as a plain file there it is either world-readable at 0644 or unreadable by the manager at 0640. Verified still true on a clean install. A backup repository password sent that way is the key to every backup the user has, readable by any local account. libreportal-ownership gains `secret-dir`: the mirror of _webui_bind_access. That one makes manager-owned config readable by the container; this makes a container-written file readable by the MANAGER. The directory is : mode 2730 — setgid so each file inherits the manager's group, the container writes it 0640, and 0730 leaves the directory unlistable because the manager is handed a filename rather than going looking. Group rwx is what lets it unlink after reading. The WebUI then sends a REFERENCE ("secret:") wherever it used to send the value, and configUpdateBatch redeems it at the last moment before the write. That is the single point every config write from the WebUI passes through, so this covers every password field rather than only the backup ones — which is what docs/roadmap/first-run-restore.md §4.1 asked for. A reference that cannot be redeemed leaves the field unchanged rather than blanking it. Verified on a live install: the container drops a secret, the manager applies it by reference, the file is unlinked, `nobody` can neither read nor list it, and a second redemption of the same reference fails. footprint_version 9 -> 10 (root-owned helper changed). Also fixes a block of constructor initialisations I spliced into the middle of renderStorageChoices in aa44e0b: on a single-drive box — the case in the screenshot that prompted this — rendering the Storage step silently reset backupDest and cleared the import selections. Co-Authored-By: Claude Opus 5 --- .../frontend/core/setup/js/setup-wizard.js | 7 -- init.sh | 2 +- scripts/config/config_update.sh | 25 ++++++ scripts/dev/lp-secret-channel-test | 86 +++++++++++++++++++ .../source/files/arrays/function_manifest.sh | 15 ++++ scripts/system/libreportal-ownership | 28 +++++- scripts/webui/webui_secret.sh | 86 +++++++++++++++++++ 7 files changed, 240 insertions(+), 9 deletions(-) create mode 100755 scripts/dev/lp-secret-channel-test create mode 100644 scripts/webui/webui_secret.sh diff --git a/containers/libreportal/frontend/core/setup/js/setup-wizard.js b/containers/libreportal/frontend/core/setup/js/setup-wizard.js index e71fd78..4fd92c0 100755 --- a/containers/libreportal/frontend/core/setup/js/setup-wizard.js +++ b/containers/libreportal/frontend/core/setup/js/setup-wizard.js @@ -613,13 +613,6 @@ class SetupWizard { if (opts.length < 2) { box.innerHTML = ''; this.storageDefault = 'primary'; - // Where LibrePortal's own tree should live (root-only to change post-install). - this.storageSystemChoice = 'primary'; - // Backup destination: '' = none, 'primary' = system disk, else a drive path. - this.backupDest = ''; - // .lpapp exports found at the path the user gave, and which to import. - this.importResults = []; - this.importSelected = []; this.storageSystemChoice = 'primary'; return; } diff --git a/init.sh b/init.sh index 5e429f5..61a3e95 100755 --- a/init.sh +++ b/init.sh @@ -134,7 +134,7 @@ command_symlink="/usr/local/bin/libreportal" # `update apply` runs as the manager and CANNOT rewrite root-owned files, so a bump # tells the updater the new release needs a root re-install (which re-bakes them). # Recorded at install in $lp_lib_dir/.footprint_version. See docs/contributing/development.md. -footprint_version=9 +footprint_version=10 footprint_marker="$lp_lib_dir/.footprint_version" # Directories — three independently-relocatable roots (see scripts/source/paths.sh diff --git a/scripts/config/config_update.sh b/scripts/config/config_update.sh index 6aa2e17..84aae0b 100755 --- a/scripts/config/config_update.sh +++ b/scripts/config/config_update.sh @@ -22,6 +22,31 @@ configUpdateBatch() if [[ "$pair" =~ ^(CFG_[A-Z0-9_]+)=(.*)$ ]]; then local key="${BASH_REMATCH[1]}" local value="${BASH_REMATCH[2]//%7C/|}" + + # A value the WebUI collected as a secret arrives here as a + # REFERENCE, never the secret itself. Redeem it at the last moment + # before the write. + # + # This is the whole point of the channel: the pair string reaching + # this function came in as part of a task's command, and tasks are + # recorded in frontend/data/tasks/*.json — 0644, inside a + # world-readable directory — as well as being visible in `ps` while + # they run. A backup repository password sent that way is the key to + # every backup the user has, readable by any local account. + # + # Done here rather than per caller because this is the single point + # every config write from the WebUI passes through, so every + # password field benefits at once. + if declare -f webuiSecretIsRef >/dev/null 2>&1 && webuiSecretIsRef "$value"; then + local _secret + if ! _secret=$(webuiSecretConsume "${value#secret:}"); then + isError "Could not read the submitted value for $key — leaving it unchanged." + ((failed++)) + continue + fi + value="$_secret" + fi + [[ "$key" == CFG_CATALOG_* ]] && catalog_changed=true if updateConfigOption "$key" "$value"; then ((applied++)) diff --git a/scripts/dev/lp-secret-channel-test b/scripts/dev/lp-secret-channel-test new file mode 100755 index 0000000..13b69d0 --- /dev/null +++ b/scripts/dev/lp-secret-channel-test @@ -0,0 +1,86 @@ +#!/bin/bash +# The one-shot channel for a secret the WebUI collected. +# +# scripts/dev/lp-secret-channel-test +# +# The browser cannot run restic, so a repository password typed in the WebUI has +# to reach the host. Both channels that already existed leak it: as part of a +# task's command string it lands in frontend/data/tasks/*.json — 0644, inside a +# world-readable directory, and visible in `ps` while the task runs — and as a +# plain file there it is either world-readable at 0644 or unreadable by the +# manager at 0640. That password is the key to every backup the user has. +# +# So the container writes it into a directory root prepares for this: owned +# :, mode 2730. The setgid bit gives the file the manager's +# group, the container writes it 0640, and the directory is unlistable. The +# manager redeems it once and unlinks it, and the value never enters argv. +# +# The ownership half needs two real accounts and root, so it is checked only +# when those are present; the redemption rules are checked always. + +REPO="$(cd "$(dirname "$0")/../.." && pwd)" +BASE="$(mktemp -d "${TMPDIR:-/tmp}/lp-secret-test-XXXXXX")" +trap 'rm -rf "$BASE"' EXIT + +fail=0 +chk(){ if [[ "$2" == "$3" ]]; then echo " ok $1"; else echo " FAIL $1: got '$2' want '$3'"; fail=1; fi; } + +mkdir -p "$BASE/webui/frontend/data/.secrets" +webuiDir(){ printf '%s/webui' "$BASE"; } +isError(){ LAST_ERR="$*"; } +source "$REPO/scripts/webui/webui_secret.sh" + +printf 'hunter2-repo-password' > "$BASE/webui/frontend/data/.secrets/abcd1234efgh" + +echo "--- a reference is redeemed once ---" +chk "first read" "$(webuiSecretConsume abcd1234efgh)" "hunter2-repo-password" +chk "second read" "$(webuiSecretConsume abcd1234efgh 2>/dev/null)" "" +chk "file is gone" "$([[ -e "$BASE/webui/frontend/data/.secrets/abcd1234efgh" ]] && echo yes || echo no)" "no" + +echo "--- ids that could escape the directory are refused ---" +for bad in '../../../etc/passwd' 'a/b' 'x' '' '../secrets2/leak' 'name with space' "$(printf 'a%.0s' {1..80})"; do + if webuiSecretConsume "$bad" >/dev/null 2>&1; then + echo " FAIL accepted '$bad'"; fail=1 + fi +done +echo " ok all rejected" +# Nothing outside the drop may be touched even when the name resolves to a file. +printf 'do-not-read' > "$BASE/outside" +webuiSecretConsume "../outside" >/dev/null 2>&1 +chk "file outside untouched" "$([[ -f "$BASE/outside" ]] && echo yes || echo no)" "yes" + +echo "--- resolve: references redeemed, plain values passed through ---" +printf 'sekrit' > "$BASE/webui/frontend/data/.secrets/refref1234ab" +chk "reference" "$(webuiSecretResolve 'secret:refref1234ab')" "sekrit" +chk "plain value" "$(webuiSecretResolve 'just-a-password')" "just-a-password" +chk "empty stays" "$(webuiSecretResolve '')" "" + +echo "--- abandoned drops are swept ---" +printf 'stale' > "$BASE/webui/frontend/data/.secrets/staleaaaa111" +touch -d '2 hours ago' "$BASE/webui/frontend/data/.secrets/staleaaaa111" +printf 'fresh' > "$BASE/webui/frontend/data/.secrets/freshbbbb222" +webuiSecretSweep +chk "old removed" "$([[ -e "$BASE/webui/frontend/data/.secrets/staleaaaa111" ]] && echo yes || echo no)" "no" +chk "fresh kept" "$([[ -e "$BASE/webui/frontend/data/.secrets/freshbbbb222" ]] && echo yes || echo no)" "yes" + +echo "--- the ownership boundary (needs root + both accounts) ---" +if [[ $EUID -eq 0 ]] && id libreportal >/dev/null 2>&1 && id dockerinstall >/dev/null 2>&1 \ + && [[ -d /libreportal-containers/libreportal/frontend/data ]]; then + /usr/local/lib/libreportal/libreportal-ownership secret-dir + d=/libreportal-containers/libreportal/frontend/data/.secrets + chk "dir mode" "$(stat -c '%a' "$d")" "2730" + chk "dir owner" "$(stat -c '%U:%G' "$d")" "dockerinstall:libreportal" + su -s /bin/bash -c "umask 027; printf 'x' > $d/testtesttest" dockerinstall + chk "file group inherited" "$(stat -c '%G' "$d/testtesttest")" "libreportal" + su -s /bin/bash -c "test -r $d/testtesttest" libreportal \ + && echo " ok manager can read" || { echo " FAIL manager cannot read"; fail=1; } + su -s /bin/bash -c "test -r $d/testtesttest" nobody 2>/dev/null \ + && { echo " FAIL another account can read"; fail=1; } || echo " ok others cannot read" + rm -f "$d/testtesttest" +else + echo " SKIP not root, or the install is not present" +fi + +echo "" +if (( fail )); then echo "FAILED"; exit 1; fi +echo "All secret-channel checks passed." diff --git a/scripts/source/files/arrays/function_manifest.sh b/scripts/source/files/arrays/function_manifest.sh index 8010f70..c0fd568 100644 --- a/scripts/source/files/arrays/function_manifest.sh +++ b/scripts/source/files/arrays/function_manifest.sh @@ -1226,6 +1226,11 @@ declare -gA LP_FN_MAP=( [webuiRemoveSetupLock]="webui/data/lock/webui_remove_setup_lock.sh" [webuiRemoveUpdateLock]="webui/data/lock/webui_remove_update_lock.sh" [webuiRunUpdate]="update/check_update.sh" + [webuiSecretConsume]="webui/webui_secret.sh" + [webuiSecretDir]="webui/webui_secret.sh" + [webuiSecretIsRef]="webui/webui_secret.sh" + [webuiSecretResolve]="webui/webui_secret.sh" + [webuiSecretSweep]="webui/webui_secret.sh" [webuiSetConfigOptions]="webui/data/generators/config/webui_cli_config_set.sh" [webuiSyncAppIcon]="webui/data/utils/webui_app_icons.sh" [webuiSyncAppIcons]="webui/data/utils/webui_app_icons.sh" @@ -2472,6 +2477,11 @@ declare -gA LP_FN_ROOT=( [webuiRemoveSetupLock]="scripts" [webuiRemoveUpdateLock]="scripts" [webuiRunUpdate]="scripts" + [webuiSecretConsume]="scripts" + [webuiSecretDir]="scripts" + [webuiSecretIsRef]="scripts" + [webuiSecretResolve]="scripts" + [webuiSecretSweep]="scripts" [webuiSetConfigOptions]="scripts" [webuiSyncAppIcon]="scripts" [webuiSyncAppIcons]="scripts" @@ -3756,6 +3766,11 @@ webuiRegistryCatalogScan() { unset -f webuiRegistryCatalogScan; __lpAutoload "${ webuiRemoveSetupLock() { unset -f webuiRemoveSetupLock; __lpAutoload "${install_scripts_dir}webui/data/lock/webui_remove_setup_lock.sh"; webuiRemoveSetupLock "$@"; } webuiRemoveUpdateLock() { unset -f webuiRemoveUpdateLock; __lpAutoload "${install_scripts_dir}webui/data/lock/webui_remove_update_lock.sh"; webuiRemoveUpdateLock "$@"; } webuiRunUpdate() { unset -f webuiRunUpdate; __lpAutoload "${install_scripts_dir}update/check_update.sh"; webuiRunUpdate "$@"; } +webuiSecretConsume() { unset -f webuiSecretConsume; __lpAutoload "${install_scripts_dir}webui/webui_secret.sh"; webuiSecretConsume "$@"; } +webuiSecretDir() { unset -f webuiSecretDir; __lpAutoload "${install_scripts_dir}webui/webui_secret.sh"; webuiSecretDir "$@"; } +webuiSecretIsRef() { unset -f webuiSecretIsRef; __lpAutoload "${install_scripts_dir}webui/webui_secret.sh"; webuiSecretIsRef "$@"; } +webuiSecretResolve() { unset -f webuiSecretResolve; __lpAutoload "${install_scripts_dir}webui/webui_secret.sh"; webuiSecretResolve "$@"; } +webuiSecretSweep() { unset -f webuiSecretSweep; __lpAutoload "${install_scripts_dir}webui/webui_secret.sh"; webuiSecretSweep "$@"; } webuiSetConfigOptions() { unset -f webuiSetConfigOptions; __lpAutoload "${install_scripts_dir}webui/data/generators/config/webui_cli_config_set.sh"; webuiSetConfigOptions "$@"; } webuiSyncAppIcon() { unset -f webuiSyncAppIcon; __lpAutoload "${install_scripts_dir}webui/data/utils/webui_app_icons.sh"; webuiSyncAppIcon "$@"; } webuiSyncAppIcons() { unset -f webuiSyncAppIcons; __lpAutoload "${install_scripts_dir}webui/data/utils/webui_app_icons.sh"; webuiSyncAppIcons "$@"; } diff --git a/scripts/system/libreportal-ownership b/scripts/system/libreportal-ownership index f1d7c62..6fcce7a 100644 --- a/scripts/system/libreportal-ownership +++ b/scripts/system/libreportal-ownership @@ -119,6 +119,31 @@ _webui_bind_access() { fi } +# A one-shot channel for a secret typed in the browser. +# +# The mirror of _webui_bind_access above. That one makes manager-owned config +# READABLE by the container; this makes a container-written file readable by the +# MANAGER — so a password entered in the WebUI never has to travel as part of a +# task command string. Those land in frontend/data/tasks/*.json, which is 0644 +# inside a world-readable directory, so every local account can read them; a +# backup repository password is the key to every backup the user has. +# +# cowner:MANAGER with the setgid bit: the container owns the directory and can +# create in it, setgid gives each new file the manager's group, and the container +# writes the file 0640 — readable by the manager, by nobody else. Mode 0730 +# leaves the directory unlistable on purpose: the manager is handed a filename, +# it never enumerates. Group rwx is what lets the manager unlink after reading. +secret_dir() { + local mode cowner; mode="$(_mode)"; cowner="$(_container_owner "$mode")" + local parent="$WEBUI_DIR/frontend/data" + [[ -d "$parent" ]] || { echo "libreportal-ownership: no WebUI data dir at $parent" >&2; return 1; } + local d="$parent/.secrets" + mkdir -p -- "$d" || return 1 + chown "$cowner:$MANAGER" -- "$d" || return 1 + chmod 2730 -- "$d" || return 1 + return 0 +} + # Control plane -> manager; container + backup roots -> container owner. reconcile() { local mode="${1:-$(_mode)}" @@ -468,5 +493,6 @@ case "$action" in app-adopt) app_adopt "${1:-}" "${2:-}" "${3:-}";; restore-stage) restore_stage "${1:-}";; restore-unstage) restore_unstage "${1:-}";; - *) echo "usage: libreportal-ownership {reconcile [mode]|traversal|containers-top|backups-top|db-own|app-perms|webui|webui-bind|taskdir|app-data-nobody |app-data-remove |app-file |app-move |app-adopt |restore-stage |restore-unstage }" >&2; exit 2;; + secret-dir) secret_dir;; + *) echo "usage: libreportal-ownership {reconcile [mode]|traversal|containers-top|backups-top|db-own|app-perms|webui|webui-bind|taskdir|app-data-nobody |app-data-remove |app-file |app-move |app-adopt |restore-stage |restore-unstage |secret-dir}" >&2; exit 2;; esac diff --git a/scripts/webui/webui_secret.sh b/scripts/webui/webui_secret.sh new file mode 100644 index 0000000..cdf6131 --- /dev/null +++ b/scripts/webui/webui_secret.sh @@ -0,0 +1,86 @@ +#!/bin/bash + +# Reading a secret the WebUI collected, without it ever touching a command line. +# +# The problem this exists for: the browser cannot run restic, so a repository +# password typed in the WebUI has to reach the host. The two channels that +# already existed both leak it — +# +# * as part of a task's command string, which lands in +# frontend/data/tasks/*.json (0644, in a world-readable directory) and is +# visible in `ps` while the task runs +# * as a plain file in that same directory, which is either 0644 and +# world-readable, or 0640 and unreadable by the manager +# +# and a backup repository password is the key to every backup the user has. +# +# So the container writes it into a directory root prepared for exactly this: +# owned :, mode 2730. Setgid gives the file the manager's +# group, the container writes it 0640, and the directory is unlistable. The +# manager reads it once and unlinks it; the value never appears in argv. +# +# Callers pass a REFERENCE (the id) wherever they would have passed the secret, +# and resolve it here at the last moment. + +# Where the drop lives. Created by `libreportal-ownership secret-dir`. +webuiSecretDir() +{ + printf '%s' "$(webuiDir)/frontend/data/.secrets" +} + +# Read a secret and consume it. Prints the value on stdout; nothing on failure. +# +# Single use by construction: a reference that has already been redeemed, or was +# never written, is indistinguishable from a wrong one, which is what we want. +webuiSecretConsume() +{ + local id="$1" + # Names come from the browser, so nothing that could escape the directory. + if [[ ! "$id" =~ ^[A-Za-z0-9_-]{8,64}$ ]]; then + isError "Invalid secret reference." + return 1 + fi + + local f; f="$(webuiSecretDir)/$id" + if [[ ! -f "$f" ]]; then + isError "That secret is not available (already used, or expired)." + return 1 + fi + + cat -- "$f" 2>/dev/null + local rc=$? + # Unlink even if the read failed — a secret that could not be delivered must + # not sit around waiting to be delivered to someone else. + rm -f -- "$f" 2>/dev/null + return $rc +} + +# Drop anything left behind. A secret is written immediately before the action +# that redeems it, so anything older than a few minutes belongs to a flow that +# was abandoned — a browser tab closed mid-form — and should not linger. +webuiSecretSweep() +{ + local d; d="$(webuiSecretDir)" + [[ -d "$d" ]] || return 0 + find "$d" -maxdepth 1 -type f -mmin +15 -delete 2>/dev/null + return 0 +} + +# True when a value is a reference rather than the secret itself. Lets a caller +# accept either during the transition without guessing. +webuiSecretIsRef() +{ + [[ "$1" == secret:* ]] +} + +# Resolve a value that may be a reference. Anything else is returned unchanged, +# so call sites stay readable. +webuiSecretResolve() +{ + local v="$1" + if webuiSecretIsRef "$v"; then + webuiSecretConsume "${v#secret:}" + return $? + fi + printf '%s' "$v" +}