diff --git a/docs/DEVELOPMENT.md b/docs/DEVELOPMENT.md index 98d8c26..8506bd4 100644 --- a/docs/DEVELOPMENT.md +++ b/docs/DEVELOPMENT.md @@ -99,6 +99,12 @@ for browser/API testing: --client-interface wlx74da385d4165 ``` +From an SSH/headless shell, the portal command may ask once for scoped +NetworkManager sudo authorization before the selected board is erased. This is +host setup, not a test secret; never add a sudo value to an env file or run the +whole runner as root. See [Testing](TESTING.md#networkmanager-authorization) +for the direct/Polkit and scoped-sudo behavior. + See [Testing](TESTING.md#docker-portal-contract) for cleanup, artifacts, and optional station handoff credentials. diff --git a/docs/TESTING.md b/docs/TESTING.md index ef45351..11303a9 100644 --- a/docs/TESTING.md +++ b/docs/TESTING.md @@ -77,6 +77,27 @@ then PlatformIO's standard `~/.platformio/penv/bin/pio` installation. That makes the same command work from a non-interactive SSH shell without modifying the user's `PATH`. +### NetworkManager authorization + +The portal adapter is a host-side resource, separate from fixture credentials. +The runner never reads a sudo password from `test/.env`, an environment file, +or source control. `doctor` reports whether the current session can use +NetworkManager directly or will need scoped sudo. In a graphical desktop, +Polkit normally authorizes the selected adapter directly. In an SSH or other +headless session with no Polkit agent, the runner visibly validates `sudo -v` +before it erases or flashes the board, then uses `sudo -n nmcli` only to scan, +disconnect, join, and remove its generated connection on the named secondary +adapter. + +The normal setting is `WM_NMCLI_AUTH=auto`. Use `WM_NMCLI_AUTH=sudo` to choose +the same scoped path deliberately, or `WM_NMCLI_AUTH=direct` only when a +working Polkit policy already grants the required actions. Do not run the whole +runner under `sudo`: its state files and browser artifacts intentionally remain +owned by the invoking developer. If the sudo ticket expires during a long run, +the runner stops with an actionable message rather than silently treating an +unauthorized rescan as a missing portal SSID. `down` uses the same scoped path +to remove a retained connection. + ```bash ./tools/portal-hardware doctor --client-interface wlx74da385d4165 ./tools/portal-hardware run \ diff --git a/test/portal-harness/README.md b/test/portal-harness/README.md index 58ff17e..5f2676a 100644 --- a/test/portal-harness/README.md +++ b/test/portal-harness/README.md @@ -15,6 +15,14 @@ The ESP8266 portal SSID is `WM Contract ESP8266`; the ESP32 SSID is `WM Contract The runner cleans up only the temporary connection it creates on the named secondary interface. It refuses to run if that interface is the system default route. +NetworkManager authority is a host prerequisite, not a fixture secret. A GUI +Polkit session may authorize the adapter directly; a headless/SSH invocation +validates sudo before flashing, then elevates only the generated portal +connection actions. Leave the runner itself unprivileged so its private state +and browser artifacts remain owned by the developer. See +[`docs/TESTING.md`](../../docs/TESTING.md#networkmanager-authorization) for +the `WM_NMCLI_AUTH` options. + ## A/B portal OTA fixture The same fixture has dedicated `*_ota_a` and `*_ota_b` PlatformIO environments diff --git a/tools/lib/portal-hardware-session.sh b/tools/lib/portal-hardware-session.sh index 940756d..71dda92 100755 --- a/tools/lib/portal-hardware-session.sh +++ b/tools/lib/portal-hardware-session.sh @@ -13,6 +13,107 @@ wm_require() { } } +wm_nmcli_permission() { + local permission="$1" + awk -F: -v permission="$permission" '$1 == permission { print $2; exit }' \ + <<<"${WM_NMCLI_PERMISSIONS:-}" +} + +wm_prepare_networkmanager_authorization() { + # A GUI Polkit agent can authorize direct nmcli actions. An SSH/headless + # shell has no such agent on many Linux hosts, even for a sudo-capable + # developer. Resolve that host-side boundary before an erase/upload, then + # use one fixed command path rather than suppressing an unauthorized scan + # and later misreporting an SSID timeout. + local permission value direct=yes + case "${WM_NMCLI_AUTH:-auto}" in + auto|direct|sudo) ;; + *) + echo 'WM_NMCLI_AUTH must be auto, direct, or sudo.' >&2 + return 2 + ;; + esac + [[ "${WM_NMCLI_AUTH_READY:-no}" == yes ]] && return 0 + + WM_NMCLI_PERMISSIONS="$(nmcli -t -f PERMISSION,VALUE general permissions 2>/dev/null || true)" + for permission in \ + org.freedesktop.NetworkManager.wifi.scan \ + org.freedesktop.NetworkManager.network-control \ + org.freedesktop.NetworkManager.settings.modify.system; do + value="$(wm_nmcli_permission "$permission")" + [[ "$value" == yes ]] || direct=no + done + + case "${WM_NMCLI_AUTH:-auto}" in + direct) + WM_NMCLI_MODE=direct + ;; + sudo) + WM_NMCLI_MODE=sudo + ;; + auto) + if [[ "$direct" == yes ]]; then + WM_NMCLI_MODE=direct + elif [[ -z "${DISPLAY:-}" && -z "${WAYLAND_DISPLAY:-}" && -z "${DBUS_SESSION_BUS_ADDRESS:-}" ]]; then + WM_NMCLI_MODE=sudo + else + # Give a graphical Polkit agent the chance to authorize the + # action. A failure is reported verbatim by wm_nmcli. + WM_NMCLI_MODE=direct + fi + ;; + esac + + if [[ "$WM_NMCLI_MODE" == sudo ]]; then + command -v sudo >/dev/null 2>&1 || { + echo 'NetworkManager requires authorization, but sudo is unavailable. Use a graphical Polkit session or install/configure sudo.' >&2 + return 1 + } + echo 'NetworkManager requires scoped authorization for the named portal adapter; validating sudo before the board is flashed.' >&2 + sudo -v || { + echo 'Could not validate sudo for the scoped NetworkManager portal actions.' >&2 + return 1 + } + fi + WM_NMCLI_AUTH_READY=yes + export WM_NMCLI_MODE WM_NMCLI_AUTH_READY WM_NMCLI_PERMISSIONS +} + +wm_report_networkmanager_authorization() { + local permission value direct=yes + WM_NMCLI_PERMISSIONS="$(nmcli -t -f PERMISSION,VALUE general permissions 2>/dev/null || true)" + for permission in \ + org.freedesktop.NetworkManager.wifi.scan \ + org.freedesktop.NetworkManager.network-control \ + org.freedesktop.NetworkManager.settings.modify.system; do + value="$(wm_nmcli_permission "$permission")" + [[ "$value" == yes ]] || direct=no + done + if [[ "$direct" == yes ]]; then + echo 'NetworkManager portal authorization: direct.' + elif [[ -z "${DISPLAY:-}" && -z "${WAYLAND_DISPLAY:-}" && -z "${DBUS_SESSION_BUS_ADDRESS:-}" ]]; then + echo 'NetworkManager portal authorization: scoped sudo will be requested before a portal command flashes the board.' + else + echo 'NetworkManager portal authorization: graphical Polkit may authorize actions; set WM_NMCLI_AUTH=sudo to use scoped sudo instead.' + fi +} + +wm_nmcli() { + # Only this small set of nmcli calls is elevated when the preflight selects + # sudo. The runner, artifacts, and session record remain owned by the + # invoking developer; every mutating call still names the guarded adapter + # or a generated temporary connection. + if [[ "${WM_NMCLI_MODE:-direct}" == sudo ]]; then + sudo -n true || { + echo 'The sudo authorization for scoped NetworkManager actions expired; run sudo -v and retry the portal command.' >&2 + return 1 + } + sudo -n -- nmcli "$@" + else + nmcli "$@" + fi +} + wm_default_route_interface() { ip route show default 2>/dev/null | awk '/^default/{print $5; exit}' } @@ -47,7 +148,7 @@ wm_require_client_adapter() { echo "Refusing to use the host default-route interface: $interface" >&2 return 1 } - active_connection="$(nmcli -g GENERAL.CONNECTION device show "$interface" 2>/dev/null || true)" + active_connection="$(wm_nmcli -g GENERAL.CONNECTION device show "$interface" 2>/dev/null || true)" if [[ -n "$active_connection" && "$active_connection" != "--" && "$allow_takeover" != "yes" ]]; then echo "Client adapter $interface already has connection '$active_connection'." >&2 echo "Pass --take-over-client-adapter to replace only that adapter's connection." >&2 @@ -64,14 +165,24 @@ wm_portal_ssid() { } wm_wait_for_portal_ssid() { - local interface="$1" ssid="$2" attempt - nmcli device wifi rescan ifname "$interface" >/dev/null 2>&1 || true + local interface="$1" ssid="$2" attempt advertised + if ! wm_nmcli device wifi rescan ifname "$interface"; then + echo "NetworkManager could not scan the selected portal adapter: $interface" >&2 + return 1 + fi for attempt in $(seq 1 45); do - if nmcli -t -f SSID device wifi list ifname "$interface" | grep -Fxq "$ssid"; then + if ! advertised="$(wm_nmcli -t -f SSID device wifi list ifname "$interface")"; then + echo "NetworkManager could not read Wi-Fi scan results from $interface." >&2 + return 1 + fi + if grep -Fxq "$ssid" <<<"$advertised"; then return 0 fi sleep 1 - nmcli device wifi rescan ifname "$interface" >/dev/null 2>&1 || true + if ! wm_nmcli device wifi rescan ifname "$interface"; then + echo "NetworkManager could not refresh Wi-Fi scan results from $interface." >&2 + return 1 + fi done echo "Portal SSID not detected on $interface: $ssid" >&2 return 1 @@ -80,8 +191,8 @@ wm_wait_for_portal_ssid() { wm_remove_connection_by_name() { local name="$1" [[ -n "$name" ]] || return 0 - nmcli connection down "$name" >/dev/null 2>&1 || true - nmcli connection delete "$name" >/dev/null 2>&1 || true + wm_nmcli connection down "$name" >/dev/null 2>&1 || true + wm_nmcli connection delete "$name" >/dev/null 2>&1 || true } wm_create_portal_connection() { @@ -102,12 +213,12 @@ wm_create_portal_connection() { unset WM_PORTAL_CONNECTION_UUID WM_PORTAL_CONNECTION_NAME return 1 fi - nmcli device disconnect "$interface" >/dev/null 2>&1 || true + wm_nmcli device disconnect "$interface" >/dev/null 2>&1 || true if ! wm_wait_for_portal_ssid "$interface" "$ssid"; then wm_cleanup_created_connection return 1 fi - if ! nmcli connection add type wifi ifname "$interface" con-name "$name" ssid "$ssid" \ + if ! wm_nmcli connection add type wifi ifname "$interface" con-name "$name" ssid "$ssid" \ ipv4.method auto ipv4.never-default yes ipv6.method ignore connection.autoconnect no >/dev/null; then wm_cleanup_created_connection return 1 @@ -115,7 +226,7 @@ wm_create_portal_connection() { # NetworkManager creates the UUID at `connection add`, before any later # configuration or association step. Capture it immediately so cleanup # has an exact identifier throughout the remaining critical section. - uuid="$(nmcli -g connection.uuid connection show "$name")" + uuid="$(wm_nmcli -g connection.uuid connection show "$name")" if [[ -z "$uuid" || "$uuid" == "--" ]]; then wm_cleanup_created_connection echo "NetworkManager did not return a UUID for the portal connection." >&2 @@ -130,11 +241,11 @@ wm_create_portal_connection() { wm_cleanup_created_connection return 1 fi - if ! nmcli connection modify "$name" wifi-sec.key-mgmt wpa-psk wifi-sec.psk "$password"; then + if ! wm_nmcli connection modify "$name" wifi-sec.key-mgmt wpa-psk wifi-sec.psk "$password"; then wm_cleanup_created_connection return 1 fi - if ! nmcli connection up "$name" ifname "$interface"; then + if ! wm_nmcli connection up "$name" ifname "$interface"; then wm_cleanup_created_connection return 1 fi @@ -152,8 +263,8 @@ wm_verify_portal_route() { wm_remove_connection() { local uuid="${1:-}" name="${2:-}" if [[ -n "$uuid" ]]; then - nmcli connection down uuid "$uuid" >/dev/null 2>&1 || true - nmcli connection delete uuid "$uuid" >/dev/null 2>&1 || true + wm_nmcli connection down uuid "$uuid" >/dev/null 2>&1 || true + wm_nmcli connection delete uuid "$uuid" >/dev/null 2>&1 || true elif [[ -n "$name" ]]; then wm_remove_connection_by_name "$name" fi diff --git a/tools/portal-hardware b/tools/portal-hardware index c4573f6..50ef183 100755 --- a/tools/portal-hardware +++ b/tools/portal-hardware @@ -242,6 +242,7 @@ start_portal_session() { wm_acquire_hardware_lock wm_require_no_active_session wm_require_client_adapter "$client_interface" "$takeover" + wm_prepare_networkmanager_authorization prepare_output_dir ssid="$(wm_portal_ssid "$platform")" @@ -490,6 +491,7 @@ run_ota_contract() { finish_portal_session() { wm_load_state + wm_prepare_networkmanager_authorization wm_remove_connection "$WM_PORTAL_CONNECTION_UUID" "$WM_PORTAL_CONNECTION_NAME" wm_clear_state } @@ -517,6 +519,7 @@ case "$command_name" in require_common wm_acquire_hardware_lock wm_require_client_adapter "$client_interface" "$takeover" + wm_report_networkmanager_authorization printf 'Portal hardware prerequisites are ready. Main route is untouched; client adapter: %s\n' "$client_interface" ;; up) @@ -575,6 +578,7 @@ case "$command_name" in wm_acquire_hardware_lock wm_require_no_active_session wm_require_client_adapter "$client_interface" "$takeover" + wm_prepare_networkmanager_authorization prepare_output_dir prepare_ota_firmware prepare_ota_contract_image diff --git a/tools/tests/test-portal-hardware-cli.sh b/tools/tests/test-portal-hardware-cli.sh index 3732e61..0fe2f72 100755 --- a/tools/tests/test-portal-hardware-cli.sh +++ b/tools/tests/test-portal-hardware-cli.sh @@ -11,6 +11,9 @@ mkdir -p "$stub_bin" export CALL_LOG="$tmp/calls.log" export WM_HARDWARE_LOCK_FILE="$tmp/hardware.lock" export XDG_STATE_HOME="$tmp/state" +# Most mock cases exercise the direct desktop-Polkit path. Individual cases +# below explicitly select the headless scoped-sudo path. +export WM_NMCLI_AUTH=direct printf '%s\n' '#!/usr/bin/env bash' \ 'if [[ "$1" == "link" && "$2" == "show" ]]; then exit 0; fi' \ @@ -18,12 +21,22 @@ printf '%s\n' '#!/usr/bin/env bash' \ 'echo "192.168.4.1 dev wlan-client src 192.168.4.2"' >"$stub_bin/ip" printf '%s\n' '#!/usr/bin/env bash' \ 'printf "%s\\n" "$*" >>"$CALL_LOG"' \ +'if [[ "${NMCLI_REQUIRE_SUDO:-}" == "yes" && "${RUN_AS_SUDO:-}" != "yes" ]]; then echo "Error: Insufficient privileges" >&2; exit 7; fi' \ +'if [[ "${NMCLI_FAIL_SCAN:-}" == "yes" && "$1" == "device" && "$2" == "wifi" && "$3" == "rescan" ]]; then echo "fixture scan failure" >&2; exit 7; fi' \ 'if [[ "${NMCLI_FAIL_ADD:-}" == "yes" && "$1" == "connection" && "$2" == "add" ]]; then exit 7; fi' \ 'if [[ "${NMCLI_SIGNAL_PARENT:-}" == "yes" && "$1" == "connection" && "$2" == "add" ]]; then kill -TERM "$PPID"; exit 0; fi' \ 'if [[ "${NMCLI_FAIL_UP:-}" == "yes" && "$1" == "connection" && "$2" == "up" ]]; then exit 7; fi' \ 'if [[ "$1" == "-t" && "$2" == "-f" && "$3" == "SSID" ]]; then echo "WM Contract ESP8266"; exit 0; fi' \ 'if [[ "$1" == "-g" && "$2" == "connection.uuid" ]]; then echo "stub-uuid"; exit 0; fi' \ 'if [[ "$1" == "-g" ]]; then echo "--"; fi' >"$stub_bin/nmcli" +printf '%s\n' '#!/usr/bin/env bash' \ +'printf "sudo %s\\n" "$*" >>"$CALL_LOG"' \ +'if [[ "${SUDO_FAIL:-}" == "yes" ]]; then exit 1; fi' \ +'if [[ "$1" == "-v" ]]; then exit 0; fi' \ +'if [[ "$1" == "-n" ]]; then shift; fi' \ +'if [[ "$1" == "true" ]]; then exit 0; fi' \ +'[[ "$1" == "--" ]] && shift' \ +'RUN_AS_SUDO=yes exec "$@"' >"$stub_bin/sudo" printf '%s\n' '#!/usr/bin/env bash' 'exit 0' >"$stub_bin/pio" printf '%s\n' '#!/usr/bin/env bash' \ 'printf "docker %s\n" "$*" >>"$CALL_LOG"' \ @@ -80,6 +93,23 @@ fi # A failed association must delete the only connection it just created. source "$root/tools/lib/portal-hardware-session.sh" +# A NetworkManager failure must remain distinct from an absent portal SSID. +: >"$CALL_LOG" +export NMCLI_FAIL_SCAN=yes +if scan_output="$(wm_wait_for_portal_ssid wlan-client 'fixture portal' 2>&1)"; then + echo 'failed Wi-Fi scan was reported as an SSID result' >&2 + exit 1 +fi +unset NMCLI_FAIL_SCAN +[[ "$scan_output" == *'NetworkManager could not scan the selected portal adapter'* ]] || { + echo 'failed Wi-Fi scan did not report the NetworkManager error path' >&2 + exit 1 +} +if grep -Fq 'connection add' "$CALL_LOG"; then + echo 'failed Wi-Fi scan continued into connection creation' >&2 + exit 1 +fi + wm_wait_for_portal_ssid() { return 0; } export NMCLI_FAIL_UP=yes if wm_create_portal_connection wlan-client 'fixture portal' placeholder esp8266; then @@ -102,11 +132,50 @@ if wm_create_portal_connection wlan-client 'fixture portal' placeholder esp8266; fi unset NMCLI_FAIL_ADD grep -Eq 'connection delete wifimanager-portal-' "$CALL_LOG" +if grep -Fq 'sudo ' "$CALL_LOG"; then + echo 'generic NetworkManager failure unexpectedly retried with sudo' >&2 + exit 1 +fi [[ ! -e "$(wm_state_file)" ]] || { echo 'failed connection creation left pending recovery state behind' >&2 exit 1 } +# A non-graphical SSH shell often has no Polkit agent. Preflight the scoped +# sudo path once, then use it for only the named portal adapter actions; never +# require the developer to run the entire runner as root. +: >"$CALL_LOG" +export NMCLI_REQUIRE_SUDO=yes +export WM_NMCLI_AUTH=sudo +unset WM_NMCLI_AUTH_READY WM_NMCLI_MODE WM_NMCLI_PERMISSIONS +wm_prepare_networkmanager_authorization +wm_wait_for_portal_ssid() { return 0; } +if ! wm_create_portal_connection wlan-client 'fixture portal' placeholder esp8266; then + echo 'authorization fallback did not create the portal connection' >&2 + exit 1 +fi +wm_cleanup_created_connection +unset NMCLI_REQUIRE_SUDO +export WM_NMCLI_AUTH=direct +unset WM_NMCLI_AUTH_READY WM_NMCLI_MODE WM_NMCLI_PERMISSIONS +grep -Fq 'sudo -n -- nmcli connection add' "$CALL_LOG" +[[ ! -e "$(wm_state_file)" ]] || { + echo 'authorization fallback left portal recovery state behind' >&2 + exit 1 +} + +# A failed sudo validation must stop before the runner can erase or flash a +# board; it is not an SSID discovery failure. +export WM_NMCLI_AUTH=sudo SUDO_FAIL=yes +unset WM_NMCLI_AUTH_READY WM_NMCLI_MODE WM_NMCLI_PERMISSIONS +if wm_prepare_networkmanager_authorization >/dev/null 2>&1; then + echo 'failed scoped sudo validation was accepted' >&2 + exit 1 +fi +unset SUDO_FAIL +export WM_NMCLI_AUTH=direct +unset WM_NMCLI_AUTH_READY WM_NMCLI_MODE WM_NMCLI_PERMISSIONS + # A portal scan failure must stop before `connection add` and clear the # pre-add pending record, even though this helper is invoked in an `if`. wm_wait_for_portal_ssid() { return 1; }