From 40a31917937957daa9ded53113b2cc0720ea2983 Mon Sep 17 00:00:00 2001 From: jotaen4tinypilot <83721279+jotaen4tinypilot@users.noreply.github.com> Date: Fri, 5 Jul 2024 07:48:01 +0200 Subject: [PATCH] WiFi dialog: add scripts for enabling/disabling WiFi connection (#1804) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Related https://github.com/tiny-pilot/tinypilot/issues/131, parts (a) and (b). This PR adds two scripts for enabling and disabling the WiFi network interface on the device, so that you can also access TinyPilot through a wireless network connection. I already discussed the basic approach last week with Charles (see comment and discussion below), because of his familiarity with networking/WiFi. Since then, I did more testing in various scenarios, and also already opened two subsequent draft PRs for the [backend](https://github.com/tiny-pilot/tinypilot/pull/1805) and [frontend](https://github.com/tiny-pilot/tinypilot/pull/1810). They are WIP still, but they may give a rough impression of how the WiFi feature will look in its entirety. Overall, it seems to be looking good, so I thought it might make sense to already start the review process. A few notes on the implementation: - We write into the `wpa_supplicants.conf` file directly to configure the WiFi connection, and we use the marker-section mechanism to manage the TinyPilot-specific part of the configuration. Depending on whether the WiFi network is password-protected or not, we either use the `wpa_passphrase` command, or we generate the config section manually. - I’m not sure it’s worth to validate the `--country` input value in the `enable-wifi` script: I’d plan to do validation in the backend later on, and the network service seems to behave graceful against invalid country codes. For the `--psk` flag, we have to do a basic length validation, because otherwise the `wpa_passphrase` command would fail. - For testing on device, you can use the `ifconfig` command to check whether the `wlan` network interface is up and active. (I.e., whether `ifconfig` outputs a `wlan` block, and whether that has a `inet`/`inet6` address assigned to it.) --- ### Original draft PR comment (2024-06-13) This PR is a draft for now, because I wanted to evaluate the technical approach of the underlying WiFi-related logic first, before investing time into the implementation of the application logic (i.e., the Python part and the UI). I saw that you @cghague have been involved into [our WiFi FAQ document](https://github.com/tiny-pilot/tinypilotkvm.com/blob/46696cc1698e1eb73051616423ab3c6b09a4f5e1/content/faq/enable-wifi/index.md), so I was hoping that you are maybe familiar with the technical domain. Could you look over the privileged scripts of this PR and give me some initial, high-level feedback about whether the proposed approach seems sensible to you? At this point, I’d mainly like to avoid going into a wrong direction, or to miss any potential caveats or other issues I may have forgotten to consider. A few notes on the implementation: - For the purposes of https://github.com/tiny-pilot/tinypilot/issues/131, we need to fully automate the logic for enabling and disabling WiFi, so that we can invoke both procedures from the Python backend programmatically. The `enable-wifi` script obviously would have to parse CLI arguments to parametrize the SSID, PSK, and country code. The `disable-wifi` script wouldn’t take any arguments. - I’ve successfully tested the scripts on my device, and so far they appear to work fine. By issuing `rfkill unblock wifi` and `wpa_cli -i wlan0 reconfigure`, it seems to be possible to effectuate the changes and activate WiFi without having to reboot the device. This would make things easier for us in the UI, but it would also be more convenient and snappier for the end-user. - My thinking was that it doesn’t make sense to use the `raspi-config` tool in the scripts, because that seems to be too limited. For example, we don’t want to just keep adding new WiFi configs to the `wpa_supplicant.conf` file, but we basically want to maintain *one* “TinyPilot-owned” custom WiFi config, which we then also can delete or modify in a targeted way. For this, it seemed more sensible to me to write into the `wpa_supplicant.conf` file directly, using the [marker-section technique](https://github.com/tiny-pilot/tinypilot/blob/9bbc056c6e4dcec1804b99b3cdcbd0bcdf1ca0dc/debian-pkg/opt/tinypilot-privileged/scripts/lib/markers.sh) that we already employ elsewhere. The downside is that this requires more low-levelled logic on our side. If you’d generally agree with the proposed approach and underlying reasoning, I’d proceed to wrap up this PR and to build the surrounding application logic. Review on CodeApprove --------- Co-authored-by: Jan Heuermann --- debian-pkg/etc/sudoers.d/tinypilot | 2 + .../tinypilot-privileged/scripts/disable-wifi | 45 ++++++ .../tinypilot-privileged/scripts/enable-wifi | 145 ++++++++++++++++++ dev-scripts/mock-scripts/disable-wifi | 3 + dev-scripts/mock-scripts/enable-wifi | 3 + 5 files changed, 198 insertions(+) create mode 100755 debian-pkg/opt/tinypilot-privileged/scripts/disable-wifi create mode 100755 debian-pkg/opt/tinypilot-privileged/scripts/enable-wifi create mode 100755 dev-scripts/mock-scripts/disable-wifi create mode 100755 dev-scripts/mock-scripts/enable-wifi diff --git a/debian-pkg/etc/sudoers.d/tinypilot b/debian-pkg/etc/sudoers.d/tinypilot index 1afd13d3c..88ecea9d3 100644 --- a/debian-pkg/etc/sudoers.d/tinypilot +++ b/debian-pkg/etc/sudoers.d/tinypilot @@ -1,6 +1,8 @@ tinypilot ALL=(ALL) NOPASSWD: /opt/tinypilot-privileged/scripts/change-hostname tinypilot ALL=(ALL) NOPASSWD: /opt/tinypilot-privileged/scripts/collect-debug-logs tinypilot ALL=(ALL) NOPASSWD: /opt/tinypilot-privileged/scripts/configure-janus +tinypilot ALL=(ALL) NOPASSWD: /opt/tinypilot-privileged/scripts/disable-wifi +tinypilot ALL=(ALL) NOPASSWD: /opt/tinypilot-privileged/scripts/enable-wifi tinypilot ALL=(ALL) NOPASSWD: /opt/tinypilot-privileged/scripts/read-update-log tinypilot ALL=(ALL) NOPASSWD: /opt/tinypilot-privileged/scripts/update tinypilot ALL=(ALL) NOPASSWD: /sbin/shutdown diff --git a/debian-pkg/opt/tinypilot-privileged/scripts/disable-wifi b/debian-pkg/opt/tinypilot-privileged/scripts/disable-wifi new file mode 100755 index 000000000..2d3d71470 --- /dev/null +++ b/debian-pkg/opt/tinypilot-privileged/scripts/disable-wifi @@ -0,0 +1,45 @@ +#!/bin/bash +# +# Disable the WiFi network connection. + +# Exit on first failure. +set -e + +print_help() { + cat < 0 )); do + case "$1" in + --help) + print_help + exit + ;; + *) + >&2 echo "Unknown flag/argument: $1" + >&2 echo "Use the '--help' flag for more information" + exit 1 + ;; + esac +done + +# Echo commands before executing them, by default to stderr. +set -x + +# Exit on unset variable. +set -u + +SCRIPT_DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" &> /dev/null && pwd )" +readonly SCRIPT_DIR +readonly CONFIG_FILE='/etc/wpa_supplicant/wpa_supplicant.conf' + +# Remove any existing automated configuration. +"${SCRIPT_DIR}/strip-marker-sections" "${CONFIG_FILE}" + +# Effectuate changes. This will disable the WiFi connection instantly. +rfkill block wlan diff --git a/debian-pkg/opt/tinypilot-privileged/scripts/enable-wifi b/debian-pkg/opt/tinypilot-privileged/scripts/enable-wifi new file mode 100755 index 000000000..4b615560c --- /dev/null +++ b/debian-pkg/opt/tinypilot-privileged/scripts/enable-wifi @@ -0,0 +1,145 @@ +#!/bin/bash +# +# Enables a WiFi network connection. + +# Exit on first failure. +set -e + +print_help() { + cat < 0 )); do + case "$1" in + --help) + print_help + exit + ;; + --country) + if (( "$#" < 2 )); then + shift + break + fi + WIFI_COUNTRY="$2" + shift # For flag name. + shift # For flag value. + ;; + --ssid) + if (( "$#" < 2 )); then + shift + break + fi + WIFI_SSID="$2" + shift # For flag name. + shift # For flag value. + ;; + --psk) + if (( "$#" < 2 )); then + shift + break + fi + WIFI_PSK="$2" + shift # For flag name. + shift # For flag value. + ;; + *) + >&2 echo "Unknown flag/argument: $1" + >&2 echo "Use the '--help' flag for more information" + exit 1 + ;; + esac +done +readonly WIFI_COUNTRY +readonly WIFI_SSID +readonly WIFI_PSK="${WIFI_PSK:-}" + +if [[ -z "${WIFI_COUNTRY}" ]]; then + >&2 echo 'Missing argument: COUNTRY' + >&2 echo "Use the '--help' flag for more information" + exit 1 +fi + +# According to ISO 3166-1 alpha-2, the country code has to contain 2 letters. +COUNTRY_LENGTH="$(echo -n "${WIFI_COUNTRY}" | wc --bytes)" +readonly COUNTRY_LENGTH +if (( "${COUNTRY_LENGTH}" != 2 )); then + >&2 echo 'Invalid argument: COUNTRY' + >&2 echo "Use the '--help' flag for more information" + exit 1 +fi + +if [[ -z "${WIFI_SSID}" ]]; then + >&2 echo 'Missing argument: SSID' + >&2 echo "Use the '--help' flag for more information" + exit 1 +fi + +# If a password is specified, it has to be 8-63 characters in length. +if [[ -n "${WIFI_PSK}" ]]; then + PSK_LENGTH="$(echo -n "${WIFI_PSK}" | wc --bytes)" + readonly PSK_LENGTH + if (( "${PSK_LENGTH}" < 8 || "${PSK_LENGTH}" > 63 )); then + >&2 echo 'Invalid argument: PSK' + >&2 echo "Use the '--help' flag for more information" + exit 1 + fi +fi + +# Echo commands before executing them, by default to stderr. +set -x + +# Exit on unset variable. +set -u + +SCRIPT_DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" &> /dev/null && pwd )" +readonly SCRIPT_DIR + +# Remove any existing automated configuration. +readonly CONFIG_FILE='/etc/wpa_supplicant/wpa_supplicant.conf' +"${SCRIPT_DIR}/strip-marker-sections" "${CONFIG_FILE}" + +# Write out the new configuration. +# shellcheck source=lib/markers.sh +. "${SCRIPT_DIR}/lib/markers.sh" +{ + echo "${MARKER_START}" + echo "country=${WIFI_COUNTRY}" + + # Generate the "network" block of the config. + # - If a password is specified, we use the `wpa_passphrase` command. This + # outputs a complete "network" block, and hashes the password instead of + # storing it in clear text. Note that it still includes the original + # password as comment line in the output, so we have to strip off that line + # (which starts with `#psk=`) + # - If no password is specified, we assemble the "network" block manually. In + # this case, we also have to set `key_mgmt=NONE` to denote an open network. + if [[ -n "${WIFI_PSK}" ]]; then + wpa_passphrase "${WIFI_SSID}" "${WIFI_PSK}" | sed '/^\t#psk=.*/d' + else + echo 'network={' + echo -e "\tssid=\"${WIFI_SSID}\"" + echo -e '\tkey_mgmt=NONE' + echo '}' + fi + + echo "${MARKER_END}" +} >> "${CONFIG_FILE}" + +# Effectuate changes. +rfkill unblock wifi +wpa_cli -i wlan0 reconfigure diff --git a/dev-scripts/mock-scripts/disable-wifi b/dev-scripts/mock-scripts/disable-wifi new file mode 100755 index 000000000..636149d21 --- /dev/null +++ b/dev-scripts/mock-scripts/disable-wifi @@ -0,0 +1,3 @@ +#!/bin/bash + +# Mock version of /opt/tinypilot-privileged/scripts/disable-wifi diff --git a/dev-scripts/mock-scripts/enable-wifi b/dev-scripts/mock-scripts/enable-wifi new file mode 100755 index 000000000..f616e1f55 --- /dev/null +++ b/dev-scripts/mock-scripts/enable-wifi @@ -0,0 +1,3 @@ +#!/bin/bash + +# Mock version of /opt/tinypilot-privileged/scripts/enable-wifi