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 \