diff --git a/modalapi/wifi/ops.py b/modalapi/wifi/ops.py index 7f98c4e5f..7113019af 100644 --- a/modalapi/wifi/ops.py +++ b/modalapi/wifi/ops.py @@ -163,6 +163,9 @@ def connect_scanned(iface_name: str, ssid: str, security: str, psk: Optional[str _, del_err = nmcli(["connection", "delete", name], sudo=True, timeout=20) if del_err is not None: logging.error("failed to delete partial profile %s: %s" % (name, del_err.decode("utf-8", "replace"))) + if km == KeyMgmt.SAE: + logging.info("SAE connect to %s failed: %s" % (ssid, err.decode("utf-8", "replace"))) + return b"WPA3 connect failed. Try WPA2 if possible." return err diff --git a/modalapi/wifi/types.py b/modalapi/wifi/types.py index 61b7407ee..fb056e6c8 100644 --- a/modalapi/wifi/types.py +++ b/modalapi/wifi/types.py @@ -35,10 +35,14 @@ def from_scan_security(cls, security: str) -> "KeyMgmt": s = (security or "").upper().strip() if not s or s == "--": return cls.NONE - if "SAE" in s or "WPA3" in s: - return cls.SAE if "802.1X" in s or "EAP" in s: return cls.WPA_EAP + # Ahead of SAE on purpose: we've had more luck with WPA2 than SAE, + # so for transition-mode APs ("WPA2 WPA3") choose the former. + if "WPA2" in s or "WPA1" in s: + return cls.WPA_PSK + if "SAE" in s or "WPA3" in s: + return cls.SAE if "WPA" in s or "PSK" in s: return cls.WPA_PSK raise ValueError(f"unsupported wifi security: {security!r}") diff --git a/tests/test_wifi_manager.py b/tests/test_wifi_manager.py index efb8569c9..a1b544906 100644 --- a/tests/test_wifi_manager.py +++ b/tests/test_wifi_manager.py @@ -12,6 +12,7 @@ from modalapi.wifi import KeyMgmt, WifiManager, WifiStatus from modalapi.wifi import ops +from modalapi.wifi.types import parse_nmcli_error @pytest.fixture @@ -410,8 +411,10 @@ def fake_run(cmd, **kw): ("WPA1 WPA2", KeyMgmt.WPA_PSK), ("WPA2 802.1X", KeyMgmt.WPA_EAP), # enterprise wins over PSK keyword ("WPA3", KeyMgmt.SAE), - ("WPA2 WPA3", KeyMgmt.SAE), # presence of SAE means it's available ("SAE", KeyMgmt.SAE), + ("WPA2 WPA3", KeyMgmt.WPA_PSK), # transition mode: WPA2 is the AKM that works + ("WPA1 WPA2 WPA3", KeyMgmt.WPA_PSK), + ("WPA3 802.1X", KeyMgmt.WPA_EAP), # enterprise wins over SAE too ("802.1X", KeyMgmt.WPA_EAP), ], ) @@ -500,6 +503,40 @@ def test_connect_scanned_wpa3_uses_sae(wm): assert "sae" in add +def test_connect_scanned_sae_failure_does_not_blame_the_password(wm): + """NM reports no-secrets when SAE association times out; don't repeat that lie.""" + + def run(cmd, **kw): + if "up" in list(cmd): + return MagicMock(returncode=4, stdout="", stderr="Secrets were required, but not provided") + return MagicMock(returncode=0, stdout="", stderr="") + + with ( + patch.object(wm, "list_connections", return_value=[]), + patch("subprocess.run", side_effect=run), + ): + err = wm.connect_scanned("Net3", "WPA3", "secret") + + assert err is not None + assert b"WPA3" in err + assert "password" not in parse_nmcli_error(err) + + +def test_connect_scanned_transition_mode_uses_wpa2(wm): + """A WPA2/WPA3 AP must be joined as WPA2; SAE fails on brcmfmac against some APs.""" + calls: list[list[str]] = [] + with ( + patch.object(wm, "list_connections", return_value=[]), + patch("subprocess.run", side_effect=_ok_run(calls)), + ): + wm.connect_scanned("Net23", "WPA2 WPA3", "secret") + + add = next(c for c in calls if "add" in c) + assert "sae" not in add + assert "wpa-psk" in add + assert add[add.index("wifi-sec.pmf") + 1] == "optional" + + def test_connect_scanned_deletes_only_freshly_added_profile_on_failure(wm): """If `connection up` fails, we delete the profile *we just added* — not any pre-existing sibling profile that happens to share the SSID."""