From 35e3bdcbbd9916c971f64b852657eda7f11cea0c Mon Sep 17 00:00:00 2001 From: ekklesia Date: Mon, 21 Sep 2026 21:11:19 -0500 Subject: [PATCH] fix: give a Wi-Fi the installer set up to NetworkManager, by hand MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Bible study laptop had working internet and a panel that said there was no connection. Both were true, and one thing caused both. Measured on the machine: nmcli device status wlp2s0 wifi unmanaged nmcli networking connectivity none while curl fetched fine nmcli device wifi list nothing, with the radio enabled /run/network/ifstate wlp2s0=wlp2s0 a second wpa_supplicant, -i wlp2s0, with no -c config file dhcpcd holding the lease /etc/network/interfaces 0600 root:root Debian's installer had been given the Wi-Fi, so it wrote it into /etc/network/interfaces and ifupdown brings it up. NetworkManager ships with plugins=ifupdown and [ifupdown] managed=false, so an interface named in that file is UNMANAGED — which is why the applet had nothing to report and why scanning found nothing. An unmanaged device is not a radio NetworkManager will scan with. rfkill was ruled out separately: the radio is enabled. managed=false stays; it is what keeps a tower's static Ethernet where an administrator put it. Only a wireless interface is moved. classroom-wifi grows --check, --adopt (with --dry-run) and --rollback. --adopt builds a NetworkManager SYSTEM connection from the SSID and key already in that root-only file, so the password never leaves the machine and never reaches a human or a log; it lands in a 0600 root file with psk-flags=0, the same shape 0.2.4 established, and the polkit rules protecting it are untouched. Deliberately NOT run by the upgrade. It moves a machine's only link between managers and the machine it could strand is a laptop in a locked room, so classroom-migrate only looks and prints what it found. An administrator runs it over SSH while watching. Two things this learned from testing the rollback rather than from reading the code, and both are why the rollback is worth testing: * ifdown was missing in the lab, subprocess raised FileNotFoundError, and the exception escaped AFTER the stanza was commented out. No rollback ran. Every hand-over command now goes through a runner that tolerates a missing binary, and the whole hand-over is inside a guard that restores the file on any failure at all. * it now refuses to start when ifdown or ifup are absent, because without them the hand-over would be a one-way door. Success also requires the interface itself to be carrying the connection, not merely that the machine is online — a machine with a second link would otherwise report a failed migration as a success. Verified in the lab against real NetworkManager with a real ifupdown file: --check and --dry-run leave it byte-identical, a failed --adopt rolls back byte-identically, and --rollback restores a half-migrated machine byte-identically and exits 0. The one thing no lab can do is the hand-over succeeding, which needs the laptop's radio. Co-Authored-By: Claude Opus 5 --- docs/networking.md | 62 ++++++ scripts/classroom-migrate | 39 ++++ scripts/classroom-wifi | 261 +++++++++++++++++++++----- tests/test-classroom-network-adopt.sh | 33 +++- tools/build-release.sh | 4 + tools/validate-trixie-release.sh | 11 ++ 6 files changed, 358 insertions(+), 52 deletions(-) diff --git a/docs/networking.md b/docs/networking.md index dc27343..41a034b 100644 --- a/docs/networking.md +++ b/docs/networking.md @@ -96,6 +96,68 @@ An administrator's own account and root are untouched. To lift any of it for maintenance, drop a file of the same name into `/etc/polkit-1/rules.d/`, which polkit reads first. +## A Wi-Fi the Debian installer set up + +Symptom: the internet works, Chrome loads pages, and the panel insists +there is no connection. Scanning finds nothing either. + +Both are true at once, and they have one cause. If the installer was +given the Wi-Fi, it wrote the network into `/etc/network/interfaces`. +ifupdown brings that up with its own `wpa_supplicant` and `dhcpcd`, which +is the working internet. NetworkManager ships here with +`plugins=ifupdown` and `[ifupdown] managed=false`, so an interface named +in that file is marked **unmanaged** — and an unmanaged device is not a +radio NetworkManager will report on or scan with. + +Measured on the Bible study laptop: + +``` +$ nmcli device status +wlp2s0 wifi unmanaged -- +$ nmcli networking connectivity +none # while curl fetched fine +$ nmcli device wifi list +SSID SIGNAL # nothing, and the radio is enabled +``` + +`managed=false` stays. It is what keeps a tower's static Ethernet under +ifupdown's control where an administrator put it. Only a **wireless** +interface is moved. + +### Moving it, while watching + +This is never done by an upgrade. It moves a machine's only link between +two managers, and the machine it could strand is a laptop in a locked +room. `classroom-migrate` only looks and says what it found. An +administrator does the rest, over SSH, watching: + +``` +classroom-wifi --check # who owns it; changes nothing +classroom-wifi --adopt --dry-run # what would change; changes nothing +classroom-wifi --adopt # do it +classroom-wifi --rollback # undo it, at any time afterwards +``` + +`--adopt` creates a NetworkManager **system** connection from the SSID +and key already in that file — so the password never leaves the machine, +is never printed, and lands in a 0600 root-only file with `psk-flags=0`, +exactly as `classroom-wifi` provisions any other network. The stanza is +then commented out, the interface handed over, and the result checked: +the connection must be active **on that interface** and the machine must +really fetch something. Checking only "is the machine online" would pass +on a machine with a second link while the Wi-Fi sat dead. + +If any of that fails — including a command that is not installed, or an +interrupt — the original file is put back and ifupdown gets the interface +again. It refuses to start at all if `ifdown`/`ifup` are missing, because +that would be a one-way door. + +The original is kept at `/etc/network/interfaces.before-ekklesia-classroom`. + +After the migration the panel shows the connection at the next login, the +room can scan, and the credential protection below applies to it like any +other system connection. + ### Recovery when there is no network at all There is deliberately no teacher-facing "join a network" dialog in this diff --git a/scripts/classroom-migrate b/scripts/classroom-migrate index 2bc39d6..5edad33 100755 --- a/scripts/classroom-migrate +++ b/scripts/classroom-migrate @@ -308,6 +308,45 @@ else say " of settings this release added" fi +# ------------------------------------------- a Wi-Fi the installer set up + +# The Bible study laptop came up with working internet and a Plasma applet +# that insisted there was no connection. Both were true. Debian's +# installer had been given the Wi-Fi, so it wrote the network into +# /etc/network/interfaces; ifupdown brings that up with its own +# wpa_supplicant and dhcpcd, and NetworkManager — which ships here with +# plugins=ifupdown and [ifupdown] managed=false — marks an interface named +# in that file UNMANAGED. Measured on the laptop: +# +# wlp2s0 wifi unmanaged +# nmcli networking connectivity -> none while curl fetched fine +# +# An unmanaged device is also not a radio NetworkManager will scan with, +# which is the same reason the room could not find any networks. +# +# managed=false stays: it is what keeps a tower's static Ethernet under +# ifupdown's control where an administrator put it. Only a WIRELESS +# interface is moved, and classroom-wifi refuses to guess — it does +# nothing at all on a machine whose Wi-Fi is already NetworkManager's, or +# that has none, which is every existing classroom. +# +# DELIBERATELY NOT DONE HERE. +# +# Moving a machine's only network link from one manager to another during +# an unattended package upgrade is the kind of thing that is fine until +# the once it is not, and the machine it would strand is a laptop in a +# room with no keyboard on it. So this only LOOKS, and says what it found. +# An administrator runs the migration when they are watching: +# +# classroom-wifi --adopt --dry-run +# classroom-wifi --adopt +# classroom-wifi --rollback if anything about it is wrong +# +# Reading is safe and changes nothing. +if [[ -x /usr/local/bin/classroom-wifi ]]; then + /usr/local/bin/classroom-wifi --check 2>&1 | sed "s/^/ /" || true +fi + # ------------------------------------------------------- the old session # Plasma reads the classroom's settings out of a directory that has to be diff --git a/scripts/classroom-wifi b/scripts/classroom-wifi index 534803e..181bcd4 100755 --- a/scripts/classroom-wifi +++ b/scripts/classroom-wifi @@ -148,6 +148,22 @@ def add(ssid, password, name=None): return 0 +def run(*command): + """Run a command, tolerating one that is not installed. + + Every call in the hand-over goes through this. An uncaught + FileNotFoundError here once left a machine with its stanza already + commented out and no rollback run, which is the exact state this + whole command exists to avoid. + """ + try: + return subprocess.run(list(command), capture_output=True, text=True, + timeout=60, check=False) + except (OSError, subprocess.SubprocessError) as error: + say(f"{command[0]}: {error}") + return None + + def is_wireless(device): return os.path.isdir(f"/sys/class/net/{device}/wireless") @@ -159,12 +175,7 @@ def connectivity(): def online(): - """Whether the machine can actually reach anything. - - NetworkManager's own answer first, because that is the thing the room - was complaining about; a real fetch second, because "full" is a claim - and this is the moment to be sure of it. - """ + """Whether the machine can actually reach anything.""" if connectivity() in ("full", "limited"): return True @@ -175,6 +186,106 @@ def online(): ).returncode == 0 +def carrying(name, device): + """Whether NetworkManager has this connection up on this device. + + Asked as well as "is the machine online", and not instead of it. A + machine with a second link would answer yes to connectivity while the + interface being migrated sat dead, and this would then call a failed + migration a success and leave the Wi-Fi behind. + """ + result = nmcli("-t", "-f", "NAME,DEVICE", "connection", "show", + "--active", check=False) + + for line in (result.stdout or "").splitlines(): + parts = line.rsplit(":", 1) + + if len(parts) == 2 and parts[0] == name and parts[1] == device: + return True + + return False + + +def find_stanza(): + """The one wireless stanza ifupdown holds, or None, plus the file.""" + try: + with open(INTERFACES, "r", encoding="utf-8") as handle: + text = handle.read() + except OSError as error: + say(f"cannot read {INTERFACES}: {error}") + return None, None + + found = ifupdown.wireless(ifupdown.parse(text), is_wireless) + + if not found: + return None, text + + if len(found) > 1: + say(f"{len(found)} wireless stanzas found; refusing to guess") + return None, text + + return found[0], text + + +def check(): + """Say who owns the Wi-Fi. Reads only; changes nothing, ever. + + This is what the migration runs instead of migrating, because moving + a machine's only link during an unattended upgrade is not a risk a + room in a locked building should carry. + """ + stanza, text = find_stanza() + + if text is None: + return 0 + + if stanza is None: + say("no wireless interface is under ifupdown's control here") + return 0 + + device = stanza["name"] + ssid, _ = ifupdown.credentials(stanza) + + say(f"NOTE: {device} is configured in {INTERFACES}, so ifupdown owns " + "it and") + say(" NetworkManager will report it unmanaged — the room's panel") + say(" will say there is no connection and will not find networks.") + + if ssid: + say(" To move it, while watching, over SSH:") + say(" classroom-wifi --adopt --dry-run") + say(" classroom-wifi --adopt") + else: + say(" Its credentials are not in that file, so it has to be " + "moved by hand.") + + return 0 + + +def rollback(): + """Give the Wi-Fi back to ifupdown, exactly as it was. + + The migration does this by itself if the network does not come back. + This is the same path with a person deciding, for the case where the + network came back and something else about it is wrong. + """ + backup = f"{INTERFACES}.before-ekklesia-classroom" + + if not os.path.exists(backup): + say(f"there is no {backup}; nothing was migrated from this file") + return 1 + + with open(backup, "r", encoding="utf-8") as handle: + original = handle.read() + + found = ifupdown.wireless(ifupdown.parse(original), is_wireless) + device = found[0]["name"] if found else None + + ssid, _ = ifupdown.credentials(found[0]) if found else (None, None) + + return 0 if restore(device, ssid) else 1 + + def adopt(dry_run=False): """Move an installer-configured Wi-Fi from ifupdown to NetworkManager. @@ -184,26 +295,17 @@ def adopt(dry_run=False): handed over, and if the room is not back on the network within ADOPT_TIMEOUT the file is restored and ifupdown brought back. """ - try: - with open(INTERFACES, "r", encoding="utf-8") as handle: - text = handle.read() - except OSError as error: - say(f"cannot read {INTERFACES}: {error}") + stanza, text = find_stanza() + + if text is None: return 1 - stanzas = ifupdown.wireless(ifupdown.parse(text), is_wireless) - - if not stanzas: + if stanza is None: say("no wireless interface is configured in /etc/network/interfaces") say("nothing to adopt; this machine's Wi-Fi is already " "NetworkManager's or it has none") return 0 - if len(stanzas) > 1: - say(f"{len(stanzas)} wireless stanzas found; refusing to guess") - return 1 - - stanza = stanzas[0] device = stanza["name"] ssid, psk = ifupdown.credentials(stanza) @@ -217,7 +319,23 @@ def adopt(dry_run=False): say(f"{device}: ifupdown holds a wireless network " f"({'with' if psk else 'without'} a saved password)") + # Refuse to start a hand-over that cannot be undone. ifdown releases + # the interface and ifup is the whole of the rollback; without them + # this would be a one-way door. + missing = [tool for tool in ("ifdown", "ifup") + if shutil.which(tool) is None] + + if missing and not dry_run: + say(f"{' and '.join(missing)} not installed, so this could not be " + "undone") + say("leaving the network exactly as it is") + return 1 + if dry_run: + if missing: + say(f"NOTE: {' and '.join(missing)} are missing here, so a real " + "run would refuse") + say(f"would create a NetworkManager system connection for {ssid!r}") say(f"would comment the {device} stanza out of {INTERFACES}") say("would hand the interface over, and put it back if that failed") @@ -238,48 +356,81 @@ def adopt(dry_run=False): shutil.copy2(INTERFACES, backup) say(f"kept the original at {backup}") - with open(INTERFACES, "w", encoding="utf-8") as handle: - handle.write(ifupdown.comment_out(text, stanza)) + # From here the machine is mid-migration, so everything is inside a + # guard. Whatever goes wrong — a command that is not installed, a + # timeout, an interrupt — the file goes back and ifupdown gets the + # interface again. Nothing below may raise past this. + try: + with open(INTERFACES, "w", encoding="utf-8") as handle: + handle.write(ifupdown.comment_out(text, stanza)) - say(f"took {device} out of ifupdown's hands") + say(f"took {device} out of ifupdown's hands") + + # NetworkManager only re-reads which interfaces ifupdown claims + # when its configuration is reloaded. The link is still + # ifupdown's at this point, and NetworkManager does not touch a + # device it considers unmanaged, so this cannot drop it. + nmcli("general", "reload", "conf", check=False) + + run("ifdown", device) + nmcli("device", "set", device, "managed", "yes", check=False) + nmcli("connection", "up", ssid, check=False) + + deadline = time.monotonic() + ADOPT_TIMEOUT + + while time.monotonic() < deadline: + if carrying(ssid, device) and online(): + say(f"{device} is NetworkManager's now, and the room is " + "online") + say("the panel will show the connection at the next login") + return 0 + + time.sleep(3) + + say("NetworkManager did not get the network back") + except BaseException as error: # noqa: BLE001 - see the comment above + say(f"the migration failed part way through: {error}") + + say("rolling back") + restore(device, ssid) + + # The migration did not happen, whatever the rollback managed. + return 1 + + +def restore(device, ssid=None): + """Put the interface back under ifupdown. + + Returns True when the room is on the network again. The callers + decide what that means for an exit code: for a failed migration it is + still a failure, and for `--rollback` it is a success. + """ + backup = f"{INTERFACES}.before-ekklesia-classroom" + + if os.path.exists(backup): + shutil.copy2(backup, INTERFACES) + say(f"restored {INTERFACES} from {backup}") - # NetworkManager only re-reads which interfaces ifupdown claims when - # its configuration is reloaded. The link is still ifupdown's at this - # point, and NetworkManager does not touch a device it considers - # unmanaged, so this cannot drop it. nmcli("general", "reload", "conf", check=False) - subprocess.run(["ifdown", device], capture_output=True, check=False) - nmcli("device", "set", device, "managed", "yes", check=False) - nmcli("connection", "up", ssid, check=False) + if ssid: + nmcli("connection", "down", ssid, check=False) - deadline = time.monotonic() + ADOPT_TIMEOUT - - while time.monotonic() < deadline: - if online(): - say(f"{device} is NetworkManager's now, and the room is online") - say("the panel will show the connection at the next login") - return 0 - - time.sleep(3) - - # It did not come back. Put the machine exactly as it was. - say("NetworkManager did not get the network back; rolling back") - shutil.copy2(backup, INTERFACES) - nmcli("general", "reload", "conf", check=False) - nmcli("device", "set", device, "managed", "no", check=False) - subprocess.run(["ifup", device], capture_output=True, check=False) + if device: + nmcli("device", "set", device, "managed", "no", check=False) + run("ifup", device) + say(f"ifupdown has {device} again") for _ in range(10): if online(): - say("ifupdown has the network back; nothing was lost") - return 1 + say("the room is on the network") + return True time.sleep(3) - say("WARNING: the machine may be offline. ifupdown was restored; try " - f"`ifup {device}` or a reboot") - return 1 + say("WARNING: the machine may be offline. The original " + f"{INTERFACES} is restored; try `ifup {device}` or a reboot") + return False def main(): @@ -298,6 +449,10 @@ def main(): "ifupdown into NetworkManager") parser.add_argument("--dry-run", action="store_true", help="with --adopt, say what would change") + parser.add_argument("--check", action="store_true", + help="say who owns the Wi-Fi; changes nothing") + parser.add_argument("--rollback", action="store_true", + help="give the Wi-Fi back to ifupdown") arguments = parser.parse_args() if arguments.list: @@ -307,6 +462,12 @@ def main(): say("run this as root; it creates a connection the machine owns") return 1 + if arguments.check: + return check() + + if arguments.rollback: + return rollback() + if arguments.adopt: return adopt(dry_run=arguments.dry_run) diff --git a/tests/test-classroom-network-adopt.sh b/tests/test-classroom-network-adopt.sh index f27f67f..02f0755 100755 --- a/tests/test-classroom-network-adopt.sh +++ b/tests/test-classroom-network-adopt.sh @@ -136,17 +136,46 @@ check("a wpa-ssid alone is enough", print("7. the command itself") wifi = (root / "scripts/classroom-wifi").read_text() check("--adopt exists and has a dry run", "--adopt" in wifi and "--dry-run" in wifi) +check("--check reads without changing anything", "--check" in wifi) +check("--rollback is a command a person can run, not only an automatic path", + "--rollback" in wifi) check("it keeps the original file before changing it", "before-ekklesia-classroom" in wifi) check("it verifies the room is really online, not just that NM says so", "curl" in wifi and "connectivity" in wifi) -check("it rolls back to ifupdown if the network does not come back", - "rolling back" in wifi and "ifup" in wifi) +# A machine with a second link would answer "online" while the interface +# being migrated sat dead, and a migration that failed would be called a +# success. +check("and that the interface itself is carrying the connection", + "def carrying(" in wifi) check("it never prints the password", "psk}" not in wifi and "{psk" not in wifi) check("the connection it creates is still a system one", '"connection.permissions"' not in wifi) +print("8. it cannot leave a machine half migrated") +# Found by testing the rollback rather than by reading the code: ifdown +# was missing in the lab, subprocess raised, and the exception escaped +# AFTER the stanza had been commented out — no rollback, no network. Both +# of these are the fix. +check("every hand-over command tolerates not being installed", + "def run(" in wifi and "OSError" in wifi) +check("and the whole hand-over is guarded, so any failure restores it", + "except BaseException" in wifi and "restore(device, ssid)" in wifi) +check("it refuses to start when it could not undo itself", + "shutil.which" in wifi + and "leaving the network exactly as it is" in wifi) + +print("9. the upgrade does not do any of this on its own") +# Moving a machine's only link during an unattended upgrade is fine until +# the once it is not, and the machine it would strand is a laptop in a +# locked room. +migrate = (root / "scripts/classroom-migrate").read_text() +code = "\n".join(line for line in migrate.splitlines() + if not line.lstrip().startswith("#")) +check("the migration never runs --adopt", "--adopt" not in code) +check("it only looks, and says what it found", "classroom-wifi --check" in code) + print() print(f"{passed} passed, {failed} failed") raise SystemExit(1 if failed else 0) diff --git a/tools/build-release.sh b/tools/build-release.sh index 7c13644..8743acd 100755 --- a/tools/build-release.sh +++ b/tools/build-release.sh @@ -147,6 +147,10 @@ install -Dm644 \ "$ROOT/lib/classroom_display.py" \ "$STAGE/usr/local/lib/ekklesia-classroom/classroom_display.py" +install -Dm644 \ + "$ROOT/lib/classroom_network.py" \ + "$STAGE/usr/local/lib/ekklesia-classroom/classroom_network.py" + install -Dm644 \ diff --git a/tools/validate-trixie-release.sh b/tools/validate-trixie-release.sh index e7ef885..57653b6 100755 --- a/tools/validate-trixie-release.sh +++ b/tools/validate-trixie-release.sh @@ -132,6 +132,17 @@ grep -q "wifi.share.protected" "$network_rules" grep -q "kcm_networkmanagement=false" /etc/ekklesia-classroom/xdg/kdeglobals command -v classroom-wifi >/dev/null +# Adopting an installer-configured Wi-Fi is an administrator's command, +# never something an upgrade does on its own: it moves a machine's only +# link between managers, and the machine it would strand is a laptop in a +# locked room. The migration may only LOOK. +classroom-wifi --check 2>&1 | grep -q "no wireless interface" +classroom-wifi --adopt 2>&1 | grep -q "nothing to adopt" +! grep -qE '^[^#]*classroom-wifi --adopt' /usr/local/bin/classroom-migrate +grep -q "classroom-wifi --check" /usr/local/bin/classroom-migrate +PYTHONPATH=/usr/local/lib/ekklesia-classroom python3 -c \ + 'import classroom_network; classroom_network.parse("")' + # The policy module and the daemon agree on what they import, on a machine # rather than in a checkout. PYTHONPATH=/usr/local/lib/ekklesia-classroom python3 -c \