From 30c4022c946aa14e0e72854b55091b20ba0e439d Mon Sep 17 00:00:00 2001 From: Vijit Singh Date: Sun, 2 Aug 2026 20:15:31 -0500 Subject: [PATCH] fix(security): mask notification secrets in the control surface + confirm any shell-gaining OS update MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #848: CONTROL_SECRET_PATHS (pithead) and SECRET_PATHS (control_service.py) omitted notifications.ntfy.token/.url and notifications.webhooks[] — the whole webhook URL is a bearer secret. render_masked_config wrote them live into the dashboard-container-readable masked config. Added all three to both lists (webhooks array-masked like workers.list[].token). Also consolidated data_service.py's duplicate masking pass onto a shared mask_secrets() so the third copy can't drift. #854: os_update_needs_confirmation only fired debug->non-debug (losing a shell). A release box taking a debug bundle silently BAKES a root SSH backdoor — now confirmed too (gaining a shell), plus unstamped->unknown. Closes #848 Closes #854 Co-Authored-By: Claude Fable 5 --- .../service/control_service.py | 27 +++++++-- .../mining_dashboard/service/data_service.py | 12 +--- .../tests/service/test_control_service.py | 26 +++++++++ .../tests/service/test_data_service.py | 22 +++++++ pithead | 43 +++++++++++--- tests/stack/run.sh | 57 +++++++++++++++++-- 6 files changed, 162 insertions(+), 25 deletions(-) diff --git a/build/dashboard/mining_dashboard/service/control_service.py b/build/dashboard/mining_dashboard/service/control_service.py index 9311be6f..bbc3d89e 100644 --- a/build/dashboard/mining_dashboard/service/control_service.py +++ b/build/dashboard/mining_dashboard/service/control_service.py @@ -38,11 +38,18 @@ # A capability secret: pithead's describe_change already refuses to echo it, but read_config # was serving it in cleartext to the browser. Mask it too (#33 hardening). ("healthchecks", "ping_url"), + # ntfy topic URL + access token (#848): the URL can carry a topic token in its path/query and + # the token is a Bearer credential — both masked like the ping URL above. + ("notifications", "ntfy", "url"), + ("notifications", "ntfy", "token"), # The backup's primary-dashboard URL (#249) can carry the primary's dashboard basic-auth as # userinfo — a capability secret, masked like the ping URL above. ("xvb", "standby", "source"), ] SECRET_SENTINEL = {"__secret__": True} +# notifications.webhooks[] (#848): a list of bare URL strings, each one a bearer secret (query +# strings carry tokens). No fixed leaf path reaches an array element, so it is masked separately. +WEBHOOKS_PATH = ("notifications", "webhooks") _RESULT_POLL_S = 0.5 @@ -72,6 +79,21 @@ def _set(cfg, path, value): node[path[-1]] = value +def mask_secrets(cfg): + """Replace every set secret leaf (``SECRET_PATHS``) and each set ``notifications.webhooks[]`` + entry with the sentinel, in place. Mirrors pithead's ``render_masked_config``; shared by + ``read_config`` and ``data_service`` so the fixed-path walk and the webhooks array mask (#848) + never drift between the two defense-in-depth passes. An empty secret stays empty.""" + for path in SECRET_PATHS: + found, value = _get(cfg, path) + if found and value: + _set(cfg, path, dict(SECRET_SENTINEL)) + found, hooks = _get(cfg, WEBHOOKS_PATH) + if found and isinstance(hooks, list): + _set(cfg, WEBHOOKS_PATH, [dict(SECRET_SENTINEL) if h else h for h in hooks]) + return cfg + + def _deep_merge(base, override): """Recursively lay ``override`` over ``base`` (dicts merge; any other value replaces).""" merged = dict(base) @@ -251,10 +273,7 @@ def read_config(): cfg = _deep_merge(reference, cfg) except (OSError, ValueError): logger.warning("config.reference.json unavailable — serving the host config alone.") - for path in SECRET_PATHS: - found, value = _get(cfg, path) - if found and value: - _set(cfg, path, dict(SECRET_SENTINEL)) + mask_secrets(cfg) cfg["_core_keys"] = _load_core_keys() cfg["_editable_keys"] = _editable_paths() cfg["_confirm_keys"] = _confirm_paths() diff --git a/build/dashboard/mining_dashboard/service/data_service.py b/build/dashboard/mining_dashboard/service/data_service.py index 0a97182c..6adfcc1b 100644 --- a/build/dashboard/mining_dashboard/service/data_service.py +++ b/build/dashboard/mining_dashboard/service/data_service.py @@ -78,11 +78,9 @@ from mining_dashboard.service.alert_service import AlertService from mining_dashboard.service.clearnet_sync import ClearnetSyncSupervisor from mining_dashboard.service.control_service import ( - SECRET_PATHS, SECRET_SENTINEL, - _get, - _set, env_key_config_paths, + mask_secrets, ) from mining_dashboard.service.degradation import DegradationMonitor from mining_dashboard.service.healthchecks import HealthchecksClient @@ -426,7 +424,7 @@ def _read_host_config(): The mount is the host's PRE-MASKED copy already (docker-compose bind-mounts ``control/masked/config.json``; the raw config.json never enters the container). We still - re-apply the SECRET_PATHS mask here — exactly the defense-in-depth pass ``control_service. + re-apply ``mask_secrets`` here — exactly the defense-in-depth pass ``control_service. read_config`` runs — so a host-side masking regression can never leave a raw secret VALUE resident in ``self._last_host_config`` across polls. The diff only ever compares/names keys, but this keeps the one long-lived config dict secret-free regardless.""" @@ -435,11 +433,7 @@ def _read_host_config(): cfg = json.load(f) except (OSError, ValueError): return None - for path in SECRET_PATHS: - found, value = _get(cfg, path) - if found and value: - _set(cfg, path, dict(SECRET_SENTINEL)) - return cfg + return mask_secrets(cfg) class WorkerLifecycle: diff --git a/build/dashboard/tests/service/test_control_service.py b/build/dashboard/tests/service/test_control_service.py index 21f31f3f..836c8b93 100644 --- a/build/dashboard/tests/service/test_control_service.py +++ b/build/dashboard/tests/service/test_control_service.py @@ -24,6 +24,15 @@ "telegram": {"bot_token": "123:abc"}, "workers": {"api_token": ""}, "healthchecks": {"ping_url": "https://hc-ping.com/SECRET-UUID"}, + "notifications": { + "webhooks": [ + "https://hooks.example/SECRET-HOOKA", + "", + "https://hooks.example/SECRET-HOOKB", + ], + "ntfy": {"url": "https://ntfy.example/SECRET-NTFYURL", "token": "SECRET-NTFYTOKEN"}, + "tor": True, + }, } @@ -61,6 +70,23 @@ def test_set_secrets_masked_to_sentinel(self, spool): assert "correct horse" not in json.dumps(cfg) assert "SECRET-UUID" not in json.dumps(cfg) + def test_notification_secrets_masked(self, spool): + # ntfy url/token and each set notifications.webhooks[] entry are bearer credentials (#848): + # the whole webhook URL is the secret (query strings carry tokens), so mask entry by entry. + cfg = control_service.read_config() + assert cfg["notifications"]["ntfy"]["url"] == {"__secret__": True} + assert cfg["notifications"]["ntfy"]["token"] == {"__secret__": True} + assert cfg["notifications"]["webhooks"][0] == {"__secret__": True} + assert cfg["notifications"]["webhooks"][2] == {"__secret__": True} + # A blank webhook entry stays blank; the non-secret tor flag survives. + assert cfg["notifications"]["webhooks"][1] == "" + assert cfg["notifications"]["tor"] is True + # No raw notification secret survives anywhere in the served payload. + assert "SECRET-HOOKA" not in json.dumps(cfg) + assert "SECRET-HOOKB" not in json.dumps(cfg) + assert "SECRET-NTFYURL" not in json.dumps(cfg) + assert "SECRET-NTFYTOKEN" not in json.dumps(cfg) + def test_empty_secret_stays_empty(self, spool): # An UNSET secret is served as-is so the UI can tell "set" from "not set". cfg = control_service.read_config() diff --git a/build/dashboard/tests/service/test_data_service.py b/build/dashboard/tests/service/test_data_service.py index 2b3d7aa3..97ca9a76 100644 --- a/build/dashboard/tests/service/test_data_service.py +++ b/build/dashboard/tests/service/test_data_service.py @@ -2855,6 +2855,28 @@ def test_secret_value_is_remasked(self, tmp_path, monkeypatch): out = _read_host_config() assert out["dashboard"]["auth"]["password"] == {"__secret__": True} + def test_notification_secrets_are_remasked(self, tmp_path, monkeypatch): + # Same shared mask_secrets pass covers ntfy + the webhooks array here (#848), so a host-side + # regression can't leave a raw notification credential resident in the config snapshot. + cfg = tmp_path / "config.json" + cfg.write_text( + json.dumps( + { + "notifications": { + "webhooks": ["https://hooks.example/leaked", ""], + "ntfy": {"url": "https://ntfy.example/leaked", "token": "leaked"}, + } + } + ) + ) + monkeypatch.setattr(ds_mod.config, "HOST_CONFIG_PATH", str(cfg)) + out = _read_host_config() + assert out["notifications"]["ntfy"]["url"] == {"__secret__": True} + assert out["notifications"]["ntfy"]["token"] == {"__secret__": True} + assert out["notifications"]["webhooks"][0] == {"__secret__": True} + assert out["notifications"]["webhooks"][1] == "" + assert "leaked" not in json.dumps(out) + def test_missing_or_bad_file_is_none(self, tmp_path, monkeypatch): monkeypatch.setattr(ds_mod.config, "HOST_CONFIG_PATH", "/nonexistent/config.json") assert _read_host_config() is None diff --git a/pithead b/pithead index f2129358..cf28056e 100755 --- a/pithead +++ b/pithead @@ -2272,9 +2272,20 @@ os_bundle_variant() { # $1: bundle path — echoes debug|release|unknown, never } os_update_needs_confirmation() { # $1: running variant, $2: bundle variant — rc 0 = confirm first - # Only the debug->non-debug transition loses the management channel. An unstamped bundle is - # treated as release: assuming "debug" here is how a box gets stranded. - [ "$1" = "debug" ] && [ "$2" != "debug" ] + # Consent is needed whenever an install flips the box's shell/SSH posture, in EITHER direction, + # or when the bundle's posture can't be verified. A debug image bakes a standing root + # authorized_keys + sshd; a release image is shell-less by design. + # - GAIN a shell (#854): a debug bundle onto a box that isn't already debug enables root SSH. + # "About to gain a management channel" needs consent as much as losing one — more so. + # - LOSE the shell (#819): a release/unstamped bundle onto a debug box removes the channel + # that is probably driving this very update; recovery then needs a console on the box. + # - UNVERIFIED: an unstamped bundle degrades to "unknown" and could silently BE a debug build + # (unparseable stamp), so never wave one through unprompted. + local running="$1" bundle="$2" + [ "$bundle" = "debug" ] && [ "$running" != "debug" ] && return 0 # gaining a shell + [ "$running" = "debug" ] && [ "$bundle" != "debug" ] && return 0 # losing the shell + [ "$bundle" = "unknown" ] && return 0 # unverified bundle + return 1 } os_update() { @@ -2298,11 +2309,21 @@ os_update() { running=$(os_running_variant) target=$(os_bundle_variant "$bundle") if os_update_needs_confirmation "$running" "$target"; then - warn "This system is a debug build: SSH is baked in, and it is probably the channel driving this update." - if [ "$target" = "release" ]; then - warn "The bundle is a release build — shell-less by design. Installing it removes SSH; recovery then needs a console on the box." + if [ "$target" = "debug" ]; then + # Gaining a shell — only reached when the running variant is not already debug (#854). + warn "This bundle is a DEBUG build: it bakes in a standing root SSH key and enables sshd." + warn "Installing it turns this shell-less box into one with a permanent root SSH backdoor." + elif [ "$running" = "debug" ]; then + # Losing the shell (#819): a release/unstamped bundle onto a debug box. + warn "This system is a debug build: SSH is baked in, and it is probably the channel driving this update." + if [ "$target" = "release" ]; then + warn "The bundle is a release build — shell-less by design. Installing it removes SSH; recovery then needs a console on the box." + else + warn "The bundle carries no variant stamp — treat it as a shell-less release build. Installing it can remove SSH; recovery then needs a console on the box." + fi else - warn "The bundle carries no variant stamp — treat it as a shell-less release build. Installing it can remove SSH; recovery then needs a console on the box." + # Unverified bundle onto a non-debug box: an unparseable stamp could hide a debug build. + warn "The bundle carries no variant stamp — its shell/SSH posture can't be verified, and it may enable root SSH. Recovery may need a console on the box." fi if [ "$assume_yes" -eq 0 ]; then read -r -p "Install it anyway? (y/N): " CONFIRM || true @@ -4727,6 +4748,8 @@ readonly CONTROL_SECRET_PATHS='[ ["tari","view_key"], ["p2pool","stratum_password"], ["healthchecks","ping_url"], + ["notifications","ntfy","url"], + ["notifications","ntfy","token"], ["xvb","standby","source"]]' # #690: bound every host-runner curl so a hostile/MITM'd rig or release response can't stream an @@ -4768,6 +4791,12 @@ render_masked_config() { # | if (.dashboard | type) == "object" and (.dashboard.workers | type) == "array" then .dashboard.workers |= map( if (.token // "") == "" then . else .token = {"__secret__": true} end) + else . end + # notifications.webhooks[] (#848): the whole URL is the bearer secret (query strings carry + # tokens), and there is no fixed leaf path — mask each set entry, like the worker tokens. + | if (.notifications | type) == "object" and (.notifications.webhooks | type) == "array" + then .notifications.webhooks |= map( + if (. // "") == "" then . else {"__secret__": true} end) else . end' "$CONFIG_FILE" >"$tmp" 2>/dev/null; then chmod 644 "$tmp" 2>/dev/null || true mv "$tmp" "$mdir/config.json" 2>/dev/null || diff --git a/tests/stack/run.sh b/tests/stack/run.sh index a8cb2c3b..e4202bff 100755 --- a/tests/stack/run.sh +++ b/tests/stack/run.sh @@ -6067,6 +6067,27 @@ assert_eq "workers.list-sentinel commit applies" "$(jq -r '.status' "$RESULTS/$U assert_eq "committed config keeps the live workers.list token" "$(jq -r '.workers.list[0].token' "$C/config.json")" "tok_rig1secret" assert_eq "committed config carries no sentinel dict" "$(jq -r '[.. | objects | select(.__secret__?)] | length' "$C/config.json")" "0" +echo "== black-box: notification secrets masked in the prefill copy (#848) ==" +# The ntfy topic URL + token are bearer credentials, and each notifications.webhooks[] entry IS a +# bearer URL (query strings carry tokens). All must be sentineled in the world-readable masked copy +# — one LEAK- marker across every set secret proves the whole set at once; a blank webhook entry and +# the non-secret notifications.tor flag must survive so the editor can still render the form. +jq '.notifications = { + webhooks: ["https://hooks.example/LEAK-hookA", "", "https://hooks.example/LEAK-hookB"], + ntfy: {url: "https://ntfy.example/LEAK-ntfyurl", token: "LEAK-ntfytoken"}, + tor: true}' "$C/config.json" >"$C/config.json.tmp" && mv "$C/config.json.tmp" "$C/config.json" +run_sourced "$C" render_masked_config "$C/data/control" >/dev/null 2>&1 +assert_eq "ntfy url masked to the sentinel" "$(jq -c '.notifications.ntfy.url' "$MASKED" 2>/dev/null)" '{"__secret__":true}' +assert_eq "ntfy token masked to the sentinel" "$(jq -c '.notifications.ntfy.token' "$MASKED" 2>/dev/null)" '{"__secret__":true}' +assert_eq "first webhook entry masked to the sentinel" "$(jq -c '.notifications.webhooks[0]' "$MASKED" 2>/dev/null)" '{"__secret__":true}' +assert_eq "third webhook entry masked to the sentinel" "$(jq -c '.notifications.webhooks[2]' "$MASKED" 2>/dev/null)" '{"__secret__":true}' +assert_eq "a blank webhook entry stays blank in the masked copy" "$(jq -r '.notifications.webhooks[1]' "$MASKED" 2>/dev/null)" "" +assert_eq "the non-secret notifications.tor flag survives" "$(jq -r '.notifications.tor' "$MASKED" 2>/dev/null)" "true" +case "$(cat "$MASKED")" in +*LEAK-*) bad "masked copy holds no notification secret" "a notification secret leaked into $MASKED" ;; +*) ok "masked copy holds no notification secret" ;; +esac + echo "== black-box: audit log growth is bounded (#349) ==" # Seed the log past the 512 KiB cap, then let the runner audit one more event: the writer trims # to the newest 2000 lines BEFORE appending, so the file shrinks instead of growing forever and @@ -8526,20 +8547,33 @@ unset -f okrun rm -rf "$OKSB" unset OKSB -echo "== unit: os-update variant gate — a debug box never silently loses its SSH ==" -# The trap this guards: a debug image's SSH key is often the only management channel, and a -# release bundle removes it BY DESIGN. The gate must fire on debug->release, on debug->unstamped -# (an old bundle without the stamp is shell-less too), and nowhere else. +echo "== unit: os-update variant gate — SSH posture flips in EITHER direction need consent ==" +# The trap this guards, both ways: a debug image's SSH key is often the only management channel and +# a release bundle removes it BY DESIGN (losing a shell); a debug bundle onto a hardened release box +# bakes a standing root authorized_keys + sshd (GAINING a shell, #854). Either flip, and any bundle +# whose stamp can't be verified, must confirm; a same-variant install must not. +# Losing the shell (a KNOWN debug box installing something non-debug): run_sourced "$SANDBOX" os_update_needs_confirmation debug release assert_rc "debug system + release bundle -> confirmation required" "$?" "0" run_sourced "$SANDBOX" os_update_needs_confirmation debug unknown assert_rc "debug system + unstamped bundle -> confirmation required" "$?" "0" +# Gaining a shell (a non-debug box installing a debug bundle) — the #854 direction: +run_sourced "$SANDBOX" os_update_needs_confirmation release debug +assert_rc "release system + debug bundle -> confirmation required (gains root SSH)" "$?" "0" +run_sourced "$SANDBOX" os_update_needs_confirmation unknown debug +assert_rc "unstamped system + debug bundle -> confirmation required (gains root SSH)" "$?" "0" +# Unverified bundle onto a non-debug box: the stamp could hide a debug build, so confirm. +run_sourced "$SANDBOX" os_update_needs_confirmation release unknown +assert_rc "release system + unstamped bundle -> confirmation required (posture unverifiable)" "$?" "0" +run_sourced "$SANDBOX" os_update_needs_confirmation unknown unknown +assert_rc "unstamped system + unstamped bundle -> confirmation required (posture unverifiable)" "$?" "0" +# Same-posture installs pass without ceremony: run_sourced "$SANDBOX" os_update_needs_confirmation debug debug assert_rc "debug -> debug passes without ceremony" "$?" "1" run_sourced "$SANDBOX" os_update_needs_confirmation release release assert_rc "release -> release passes (the fleet's normal update)" "$?" "1" run_sourced "$SANDBOX" os_update_needs_confirmation unknown release -assert_rc "unstamped running system passes — only a KNOWN debug box has a channel to lose" "$?" "1" +assert_rc "unstamped system + release bundle passes — stays shell-less, no channel flips" "$?" "1" OUSB=$(mktemp -d) mkdir -p "$OUSB/bin" @@ -8600,6 +8634,19 @@ assert_not_contains "rauc install was NOT reached" "$(cat "$RAUC_LOG")" "install ourun "$OUSB/variant-debug" "$OUSB/info-release.json" bundle.raucb --yes >/dev/null 2>&1 assert_rc "--yes acknowledges the warning and proceeds" "$?" "0" assert_contains "rauc install ran with the bundle" "$(cat "$RAUC_LOG")" "install bundle.raucb" +# The #854 direction: a hardened release box taking a debug bundle GAINS a root SSH backdoor. Non- +# interactive stdin reads EOF -> refused, and rauc install must never be reached — the silent +# install is exactly the backdoor this guards. +: >"$RAUC_LOG" +out=$(ourun "$OUSB/variant-release" "$OUSB/info-debug.json" bundle.raucb 2>&1) +rc=$? +assert_rc "release box + debug bundle, no --yes -> refused" "$rc" "1" +assert_contains "the refusal names the root SSH it would gain" "$out" "root SSH" +assert_not_contains "rauc install was NOT reached on the gain-a-shell refusal" "$(cat "$RAUC_LOG")" "install" +: >"$RAUC_LOG" +ourun "$OUSB/variant-release" "$OUSB/info-debug.json" bundle.raucb --yes >/dev/null 2>&1 +assert_rc "--yes acknowledges the backdoor warning and proceeds" "$?" "0" +assert_contains "rauc install ran with the debug bundle after --yes" "$(cat "$RAUC_LOG")" "install bundle.raucb" : >"$RAUC_LOG" ourun "$OUSB/variant-release" "$OUSB/info-release.json" bundle.raucb >/dev/null 2>&1 assert_rc "release -> release installs with no prompt" "$?" "0"