diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a7f3a87c..ca25468e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -115,7 +115,7 @@ jobs: iw dev | awk '$1 == "Interface" { print $2 }' | sort -u ) - sudo -n modprobe mac80211_hwsim radios=2 + sudo -n modprobe mac80211_hwsim radios=3 udevadm settle --timeout=10 mapfile -t interfaces_after < <( @@ -126,8 +126,8 @@ jobs: <(printf '%s\n' "${interfaces_before[@]}") \ <(printf '%s\n' "${interfaces_after[@]}") ) - if (( ${#hwsim_interfaces[@]} != 2 )); then - echo "Expected the hwsim module to create two interfaces, found ${#hwsim_interfaces[@]}" >&2 + if (( ${#hwsim_interfaces[@]} != 3 )); then + echo "Expected the hwsim module to create three interfaces, found ${#hwsim_interfaces[@]}" >&2 iw dev >&2 || true exit 1 fi @@ -149,8 +149,8 @@ jobs: trap diagnose ERR read -r -a hwsim_interfaces <<< "${NMRS_HOST_HWSIM_INTERFACES:?missing hwsim interface list}" - if (( ${#hwsim_interfaces[@]} != 2 )); then - echo "Expected exactly two recorded hwsim interfaces, found ${#hwsim_interfaces[@]}" >&2 + if (( ${#hwsim_interfaces[@]} != 3 )); then + echo "Expected exactly three recorded hwsim interfaces, found ${#hwsim_interfaces[@]}" >&2 exit 1 fi @@ -169,8 +169,8 @@ jobs: test-integration \ -euo pipefail -c ' read -r -a hwsim_interfaces <<< "${NMRS_HOST_HWSIM_INTERFACES:?missing hwsim interface list}" - if (( ${#hwsim_interfaces[@]} != 2 )); then - echo "Expected exactly two hwsim interfaces in the helper, found ${#hwsim_interfaces[@]}" >&2 + if (( ${#hwsim_interfaces[@]} != 3 )); then + echo "Expected exactly three hwsim interfaces in the helper, found ${#hwsim_interfaces[@]}" >&2 exit 1 fi diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index fc60d54d..f0dbd2e3 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -102,21 +102,30 @@ declared, missing services and unexpected D-Bus errors fail the test. ### Deterministic WiFi integration -The WiFi contract requires two `mac80211_hwsim` radios. The container configures -one as a WPA2 access point, supplies DHCP with dnsmasq, and gives only the other -radio to its private NetworkManager. It asserts discovery, WPA authentication, -network and device callback delivery, DHCP, disconnect, saved-credential -reconnect, forget, and the missing-password error after cleanup. +The WiFi contract requires three `mac80211_hwsim` radios. The container +configures the first as a WPA2 access point and the second as an SAE-only +access point, each with its own hostapd, subnet, and dnsmasq, and gives only the +third radio to its private NetworkManager. It asserts discovery, WPA +authentication, network and device callback delivery, DHCP, disconnect, +saved-credential reconnect, forget, and the missing-password error after +cleanup. + +WPA3-Personal access points reject `key-mgmt=wpa-psk`, so the SAE-only radio is +what proves `WifiSecurity::WpaPsk` upgrades to SAE instead of failing, and that +`WifiSecurity::Sae` authenticates. It requires a `hostapd` built with SAE +support. ```bash -sudo modprobe mac80211_hwsim radios=2 +sudo modprobe mac80211_hwsim radios=3 docker compose run --build --rm test-wifi-integration sudo modprobe -r mac80211_hwsim ``` The WiFi runner sets `NMRS_REQUIRE_WIFI=1`, `NMRS_WIFI_INTERFACE`, -`NMRS_EXPECT_WIFI_SSID`, and `NMRS_WIFI_PASSWORD`. If a declared facility is -missing, the test fails rather than being reported as a pass. +`NMRS_EXPECT_WIFI_SSID`, and `NMRS_WIFI_PASSWORD`, plus +`NMRS_REQUIRE_WIFI_SAE=1`, `NMRS_EXPECT_WIFI_SAE_SSID`, and +`NMRS_WIFI_SAE_PASSWORD` for the SAE BSS. If a declared facility is missing, the +test fails rather than being reported as a pass. To run the NM-only contracts against a deliberately selected local daemon, opt in explicitly: diff --git a/docs/src/api/models.md b/docs/src/api/models.md index 32b2a6cf..0ab470f6 100644 --- a/docs/src/api/models.md +++ b/docs/src/api/models.md @@ -303,11 +303,17 @@ pub struct NetworkInfo { pub enum WifiSecurity { Open, WpaPsk { psk: String }, + Sae { psk: String }, WpaEap { opts: EapOptions }, + Wpa3Eap192bit { opts: EapOptions }, } ``` -Methods: `secured()`, `is_psk()`, `is_eap()` +Methods: `secured()`, `is_psk()`, `is_sae()`, `is_eap()` + +`WpaPsk` emits `key-mgmt=wpa-psk` and `Sae` emits `key-mgmt=sae`. SAE-only +WPA3-Personal access points reject the former; `connect()` upgrades `WpaPsk` to +`Sae` automatically when the target AP advertises SAE without PSK. ### EapOptions diff --git a/docs/src/development/contributing.md b/docs/src/development/contributing.md index 98bf5857..eb50d637 100644 --- a/docs/src/development/contributing.md +++ b/docs/src/development/contributing.md @@ -91,7 +91,7 @@ The WiFi contract uses one hwsim radio for a WPA2 access point with DHCP and a second radio as NetworkManager's station: ```bash -sudo modprobe mac80211_hwsim radios=2 +sudo modprobe mac80211_hwsim radios=3 docker compose run --build --rm test-wifi-integration sudo modprobe -r mac80211_hwsim ``` diff --git a/docs/src/development/testing.md b/docs/src/development/testing.md index 940144de..c1f6893d 100644 --- a/docs/src/development/testing.md +++ b/docs/src/development/testing.md @@ -142,7 +142,7 @@ with `mac80211_hwsim`. One radio advertises a WPA2-PSK test network using other radio. ```bash -sudo modprobe mac80211_hwsim radios=2 +sudo modprobe mac80211_hwsim radios=3 docker compose run --build --rm test-wifi-integration sudo modprobe -r mac80211_hwsim ``` @@ -160,8 +160,9 @@ It also mounts the host's `/run/udev` read-only so NetworkManager can manage the newly created hwsim links. The self-hosted runner service account needs passwordless `sudo` permission for -`modprobe mac80211_hwsim radios=2` and `modprobe -r mac80211_hwsim`; CI invokes -both commands with `sudo -n`. +`modprobe mac80211_hwsim radios=3` and `modprobe -r mac80211_hwsim`; CI invokes +both commands with `sudo -n`. The sudoers rule matches the exact argument list, +so changing the radio count means updating the rule on the runner as well. ### Approving Wi-Fi CI Runs diff --git a/docs/src/guide/wifi-wpa-psk.md b/docs/src/guide/wifi-wpa-psk.md index ada513a4..b0357faa 100644 --- a/docs/src/guide/wifi-wpa-psk.md +++ b/docs/src/guide/wifi-wpa-psk.md @@ -22,6 +22,26 @@ async fn main() -> nmrs::Result<()> { The `WifiSecurity::WpaPsk` variant works with WPA, WPA2, and WPA3 Personal networks. NetworkManager negotiates the strongest supported protocol automatically. +## WPA3-Personal (SAE) + +`WpaPsk` emits `key-mgmt=wpa-psk`, which WPA3-Personal access points reject when +they advertise SAE without PSK. `connect()` detects that case from the access +point's own security flags and switches to SAE for you, so the snippet above +works unchanged on an SAE-only network. + +Transition-mode access points advertise both SAE and PSK and accept `WpaPsk`, so +those are left as-is. To request SAE explicitly — when building settings +yourself, or to pin key management regardless of what the AP advertises — use +the variant directly: + +```rust +nm.connect("Wpa3Only", None, WifiSecurity::Sae { + psk: "my_secure_password".into(), +}).await?; +``` + +Unlike WPA-PSK, SAE has no 8-character minimum passphrase length. + ## Password Requirements - A new profile requires a non-empty password. An empty PSK requests the stored diff --git a/nmrs/CHANGELOG.md b/nmrs/CHANGELOG.md index 57d9d91b..c5f04ef5 100644 --- a/nmrs/CHANGELOG.md +++ b/nmrs/CHANGELOG.md @@ -5,6 +5,9 @@ All notable changes to the `nmrs` crate will be documented in this file. ## [Unreleased] ### Added +- `WifiSecurity::Sae` and `WifiConnectionBuilder::sae()` emit + `key-mgmt=sae` for WPA3-Personal access points, plus `WifiSecurity::is_sae()` + to distinguish them from WPA-PSK. ([#486](https://github.com/freedesktop-rs/nmrs/issues/486)) - `connect_by_uuid()` and `disconnect_by_uuid()` activate and deactivate any saved connection by UUID, not just VPN profiles. `connect_by_uuid()` takes a `ConnectByUuidConfig` so the interface stored in the profile can be @@ -19,6 +22,13 @@ All notable changes to the `nmrs` crate will be documented in this file. ### Fixed +- Connecting with `WifiSecurity::WpaPsk` to a WPA3-Personal access point that + advertises SAE without PSK no longer fails with + `802-11-wireless-security.key-mgmt: Access point does not support PSK but + setting requires it`. `connect()` reads the target access point's security + flags and emits `key-mgmt=sae` for SAE-only APs; transition-mode APs that + still advertise PSK are unchanged. + ([#486](https://github.com/freedesktop-rs/nmrs/issues/486)) - `disconnect_vpn_by_uuid()` again returns `Ok(())` when no saved connection matches the UUID. Delegating to `disconnect_by_uuid()` leaked that case out as `ConnectionError::SavedConnectionNotFound`, which callers of the diff --git a/nmrs/src/api/builders/wifi.rs b/nmrs/src/api/builders/wifi.rs index 46cdb912..fa7d3cb5 100644 --- a/nmrs/src/api/builders/wifi.rs +++ b/nmrs/src/api/builders/wifi.rs @@ -76,6 +76,7 @@ pub fn build_wifi_connection( let builder = match security { models::WifiSecurity::Open => base_wifi_builder(ssid, opts).open(), models::WifiSecurity::WpaPsk { psk } => base_wifi_builder(ssid, opts).wpa_psk(psk), + models::WifiSecurity::Sae { psk } => base_wifi_builder(ssid, opts).sae(psk), models::WifiSecurity::WpaEap { opts: eap } => { base_wifi_builder(ssid, opts).wpa_eap(eap.clone()) } @@ -120,6 +121,7 @@ pub fn try_build_wifi_connection( let builder = match security { models::WifiSecurity::Open => base.open(), models::WifiSecurity::WpaPsk { psk } => base.wpa_psk(psk), + models::WifiSecurity::Sae { psk } => base.sae(psk), models::WifiSecurity::WpaEap { opts: eap } => base.try_wpa_eap(eap.clone())?, models::WifiSecurity::Wpa3Eap192bit { opts: eap } => { base.try_wpa3_eap_192_bit(eap.clone())? @@ -227,6 +229,23 @@ mod tests { } } + #[test] + fn sae_security_maps_to_the_sae_builder() { + let conn = build_wifi_connection( + "wpa3net", + &WifiSecurity::Sae { + psk: "password123".into(), + }, + &default_opts(), + ); + + let security = conn + .get("802-11-wireless-security") + .expect("SAE must produce a security section"); + assert_eq!(security.get("key-mgmt"), Some(&Value::from("sae"))); + assert!(security.get("auth-alg").is_none()); + } + #[test] fn open_connection_has_correct_type() { let conn = build_wifi_connection("open_net", &WifiSecurity::Open, &default_opts()); diff --git a/nmrs/src/api/builders/wifi_builder.rs b/nmrs/src/api/builders/wifi_builder.rs index d9106247..ee6e1335 100644 --- a/nmrs/src/api/builders/wifi_builder.rs +++ b/nmrs/src/api/builders/wifi_builder.rs @@ -222,6 +222,31 @@ impl WifiConnectionBuilder { self } + /// Configures SAE (WPA3-Personal) security with the given passphrase. + /// + /// Emits `key-mgmt=sae`, which is what SAE-only access points require. + /// Transition-mode APs that advertise both SAE and PSK also accept + /// [`wpa_psk`](Self::wpa_psk). + /// + /// Unlike [`wpa_psk`](Self::wpa_psk) this sets no `auth-alg`; + /// NetworkManager rejects `auth-alg=open` combined with SAE. Protected + /// management frames are left unset so NetworkManager applies its own + /// SAE default. + #[must_use] + pub fn sae(mut self, psk: impl Into) -> Self { + let mut security = HashMap::new(); + security.insert("key-mgmt", Value::from("sae")); + security.insert("psk", Value::from(psk.into())); + security.insert("psk-flags", Value::from(0u32)); + + self.inner = self + .inner + .without_section("802-1x") + .with_section("802-11-wireless-security", security); + self.security_configured = true; + self + } + /// Configures WPA-EAP (Enterprise) security with 802.1X authentication. /// /// Supports PEAP, TTLS, and TLS methods with various inner authentication protocols. @@ -610,6 +635,53 @@ mod tests { ); } + #[test] + fn builds_sae_wifi() { + let settings = WifiConnectionBuilder::new("Wpa3Net") + .sae("password123") + .ipv4_auto() + .ipv6_auto() + .build(); + + let security = settings + .get("802-11-wireless-security") + .expect("SAE must produce a security section"); + assert_eq!(security.get("key-mgmt"), Some(&Value::from("sae"))); + assert_eq!( + security.get("psk"), + Some(&Value::from("password123".to_string())) + ); + assert_eq!(security.get("psk-flags"), Some(&Value::from(0u32))); + + // NetworkManager rejects a profile that pairs SAE with an auth-alg. + assert!( + security.get("auth-alg").is_none(), + "SAE must not set auth-alg" + ); + assert!(!settings.contains_key("802-1x")); + + let wireless = settings.get("802-11-wireless").unwrap(); + assert_eq!( + wireless.get("security"), + Some(&Value::from("802-11-wireless-security")) + ); + } + + #[test] + fn sae_replaces_previously_configured_psk_security() { + let settings = WifiConnectionBuilder::new("Wpa3Net") + .wpa_psk("password123") + .sae("password123") + .build(); + + let security = settings.get("802-11-wireless-security").unwrap(); + assert_eq!(security.get("key-mgmt"), Some(&Value::from("sae"))); + assert!( + security.get("auth-alg").is_none(), + "the earlier wpa_psk auth-alg must not survive" + ); + } + #[test] fn builds_wpa_eap_wifi() { let eap_opts = EapOptions { diff --git a/nmrs/src/api/models/wifi.rs b/nmrs/src/api/models/wifi.rs index 6e5eab00..99130422 100644 --- a/nmrs/src/api/models/wifi.rs +++ b/nmrs/src/api/models/wifi.rs @@ -51,7 +51,7 @@ pub struct WifiNetworkGroup { /// println!("SSID: {}", net.ssid); /// println!(" Signal: {}%", net.strength.unwrap_or(0)); /// println!(" Secured: {}", net.secured); -/// +/// /// if let Some(freq) = net.frequency { /// let band = if freq > 5000 { "5GHz" } else { "2.4GHz" }; /// println!(" Band: {}", band); @@ -118,12 +118,12 @@ pub struct Network { /// /// if let Some(network) = networks.first() { /// let info = nm.show_details(network).await?; -/// +/// /// println!("Network: {}", info.ssid); /// println!("Signal: {} {}", info.strength, info.bars); /// println!("Security: {}", info.security); /// println!("Status: {}", info.status); -/// +/// /// if let Some(rate) = info.rate_mbps { /// println!("Speed: {} Mbps", rate); /// } @@ -917,7 +917,9 @@ impl EapOptionsBuilder { /// # Variants /// /// - [`Open`](WifiSecurity::Open) - No authentication required (open network) -/// - [`WpaPsk`](WifiSecurity::WpaPsk) - WPA/WPA2/WPA3 Personal (password-based) +/// - [`WpaPsk`](WifiSecurity::WpaPsk) - WPA/WPA2 Personal, and WPA3 Personal on +/// transition-mode APs that still advertise PSK (password-based) +/// - [`Sae`](WifiSecurity::Sae) - WPA3 Personal on SAE-only APs (password-based) /// - [`WpaEap`](WifiSecurity::WpaEap) - WPA/WPA2 Enterprise (802.1X authentication) /// /// # Examples @@ -970,7 +972,12 @@ impl EapOptionsBuilder { pub enum WifiSecurity { /// Open network (no authentication) Open, - /// WPA-PSK (password-based authentication) + /// WPA-PSK (password-based authentication). + /// + /// Emits `key-mgmt=wpa-psk`, which SAE-only WPA3-Personal access points + /// reject. [`connect`](crate::NetworkManager::connect) upgrades this to + /// [`Sae`](Self::Sae) automatically when the target AP advertises SAE + /// without PSK. WpaPsk { /// Pre-shared key (password) psk: String, @@ -986,6 +993,15 @@ pub enum WifiSecurity { /// EAP configuration options opts: EapOptions, }, + /// SAE (WPA3-Personal, password-based authentication). + /// + /// Emits `key-mgmt=sae`. Use this for access points that advertise SAE + /// without PSK; transition-mode APs that advertise both accept + /// [`WpaPsk`](Self::WpaPsk). + Sae { + /// Pre-shared key (password) + psk: String, + }, } impl fmt::Debug for WifiSecurity { @@ -1004,6 +1020,10 @@ impl fmt::Debug for WifiSecurity { .debug_struct("Wpa3Eap192bit") .field("opts", opts) .finish(), + Self::Sae { .. } => formatter + .debug_struct("Sae") + .field("psk", &Redacted) + .finish(), } } } @@ -1021,6 +1041,15 @@ impl WifiSecurity { matches!(self, WifiSecurity::WpaPsk { .. }) } + /// Returns `true` if this is an SAE (WPA3-Personal) security type. + /// + /// [`is_psk`](Self::is_psk) covers only `key-mgmt=wpa-psk`; callers that + /// treat both as "needs a passphrase" should check both. + #[must_use] + pub fn is_sae(&self) -> bool { + matches!(self, WifiSecurity::Sae { .. }) + } + /// Returns `true` if this is a WPA-EAP (Enterprise/802.1X) security type. #[must_use] pub fn is_eap(&self) -> bool { diff --git a/nmrs/src/core/connection.rs b/nmrs/src/core/connection.rs index 3a8e8ff3..706a0d4c 100644 --- a/nmrs/src/core/connection.rs +++ b/nmrs/src/core/connection.rs @@ -5,6 +5,7 @@ use zbus::Connection; use zvariant::OwnedObjectPath; use crate::api::builders::wifi::{build_ethernet_connection, try_build_wifi_connection}; +use crate::api::models::access_point::{SecurityFeatures, decode_security}; use crate::api::models::{ConnectionError, ConnectionOptions, TimeoutConfig, WifiSecurity}; use crate::core::connection_settings::{delete_connection, get_saved_connection_path}; use crate::core::state_wait::{wait_for_connection_activation, wait_for_device_disconnect}; @@ -84,6 +85,7 @@ pub(crate) async fn connect( } let specific_object = scan_and_resolve_ap(conn, &wifi, ssid).await?; + let creds = upgrade_security_for_ap(conn, &specific_object, creds).await; match decision { SavedDecision::UseSaved(saved) => { @@ -749,6 +751,7 @@ pub(crate) async fn connect_to_bssid( futures_timer::Delay::new(timeouts::scan_wait()).await; let specific_object = find_ap_by_bssid(conn, &wifi, ssid, target_bssid).await?; + let creds = upgrade_security_for_ap(conn, &specific_object, creds).await; match decision { SavedDecision::UseSaved(saved) => { @@ -1040,16 +1043,11 @@ fn decide_saved_connection( creds: &WifiSecurity, ) -> Result { match saved { - Some(path) - if matches!(creds, WifiSecurity::Open) - || matches!(creds, WifiSecurity::WpaPsk { psk } if psk.is_empty()) => - { + Some(path) if matches!(creds, WifiSecurity::Open) || wants_stored_secret(creds) => { Ok(SavedDecision::UseSaved(path)) } Some(_) => Ok(SavedDecision::RebuildFresh), - None if matches!(creds, WifiSecurity::WpaPsk { psk } if psk.is_empty()) => { - Err(ConnectionError::MissingPassword) - } + None if wants_stored_secret(creds) => Err(ConnectionError::MissingPassword), None => Ok(SavedDecision::RebuildFresh), } } @@ -1057,7 +1055,73 @@ fn decide_saved_connection( /// Whether a failed saved-profile activation can be retried without relying on /// that profile's stored secret. fn can_rebuild_after_saved_failure(creds: &WifiSecurity) -> bool { - !matches!(creds, WifiSecurity::WpaPsk { psk } if psk.is_empty()) + !wants_stored_secret(creds) +} + +/// Whether the caller supplied an empty passphrase, meaning "use the secret +/// already stored in the saved profile". +fn wants_stored_secret(creds: &WifiSecurity) -> bool { + matches!( + creds, + WifiSecurity::WpaPsk { psk } | WifiSecurity::Sae { psk } if psk.is_empty() + ) +} + +/// Rewrites `WpaPsk` credentials to `Sae` when the target AP is SAE-only. +/// +/// `WifiSecurity::WpaPsk` emits `key-mgmt=wpa-psk`, which NetworkManager +/// rejects outright on WPA3-Personal APs that advertise SAE without PSK. +/// Transition-mode APs advertise both and still accept PSK, so those are left +/// alone. +/// +/// AP security flags are advisory here: if they cannot be read, the caller's +/// credentials are used unchanged and NetworkManager reports any mismatch. +async fn upgrade_security_for_ap( + conn: &Connection, + ap: &OwnedObjectPath, + creds: WifiSecurity, +) -> WifiSecurity { + let WifiSecurity::WpaPsk { psk } = &creds else { + return creds; + }; + if psk.is_empty() { + return creds; + } + + let Ok(proxy) = NMAccessPointProxy::builder(conn).path(ap.clone()) else { + return creds; + }; + let Ok(proxy) = proxy.build().await else { + return creds; + }; + let (Ok(flags), Ok(wpa), Ok(rsn)) = ( + proxy.flags().await, + proxy.wpa_flags().await, + proxy.rsn_flags().await, + ) else { + trace!("Could not read AP security flags; keeping the supplied credentials"); + return creds; + }; + + upgrade_security_for_features(creds, &decode_security(flags, wpa, rsn)) +} + +/// Decides the key management to use given what the AP advertises. +/// +/// Only a non-empty `WpaPsk` on an SAE-only AP is rewritten. Transition-mode +/// APs advertise SAE *and* PSK and accept `wpa-psk`, so they are left alone +/// rather than switched to SAE behind the caller's back. +fn upgrade_security_for_features(creds: WifiSecurity, security: &SecurityFeatures) -> WifiSecurity { + let WifiSecurity::WpaPsk { psk } = creds else { + return creds; + }; + + if psk.is_empty() || !security.sae || security.psk { + return WifiSecurity::WpaPsk { psk }; + } + + debug!("AP advertises SAE without PSK; using key-mgmt=sae instead of wpa-psk"); + WifiSecurity::Sae { psk } } /// Checks if currently connected to the specified SSID. @@ -1300,13 +1364,108 @@ mod tests { assert!(!can_rebuild_after_saved_failure(&WifiSecurity::WpaPsk { psk: String::new(), })); + assert!(!can_rebuild_after_saved_failure(&WifiSecurity::Sae { + psk: String::new(), + })); assert!(can_rebuild_after_saved_failure(&WifiSecurity::Open)); assert!(can_rebuild_after_saved_failure(&WifiSecurity::WpaPsk { psk: "password".into(), })); + assert!(can_rebuild_after_saved_failure(&WifiSecurity::Sae { + psk: "password".into(), + })); assert!(can_rebuild_after_saved_failure(&enterprise_credentials())); assert!(can_rebuild_after_saved_failure( &wpa3_enterprise_credentials() )); } + + #[test] + fn empty_sae_passphrase_requests_the_stored_secret() { + let path = saved_path(); + + assert_eq!( + decide_saved_connection( + Some(path.clone()), + &WifiSecurity::Sae { psk: String::new() } + ) + .unwrap(), + SavedDecision::UseSaved(path) + ); + assert!(matches!( + decide_saved_connection(None, &WifiSecurity::Sae { psk: String::new() }), + Err(ConnectionError::MissingPassword) + )); + } + + fn sae_only_ap() -> SecurityFeatures { + SecurityFeatures { + privacy: true, + sae: true, + ccmp: true, + ..SecurityFeatures::default() + } + } + + fn transition_mode_ap() -> SecurityFeatures { + SecurityFeatures { + psk: true, + ..sae_only_ap() + } + } + + #[test] + fn psk_credentials_become_sae_on_sae_only_access_points() { + let upgraded = upgrade_security_for_features( + WifiSecurity::WpaPsk { + psk: "password".into(), + }, + &sae_only_ap(), + ); + + assert_eq!( + upgraded, + WifiSecurity::Sae { + psk: "password".into() + } + ); + } + + #[test] + fn psk_credentials_survive_transition_mode_and_psk_only_access_points() { + let creds = WifiSecurity::WpaPsk { + psk: "password".into(), + }; + + for ap in [transition_mode_ap(), SecurityFeatures::default()] { + assert_eq!( + upgrade_security_for_features(creds.clone(), &ap), + creds, + "PSK must be preserved where the AP accepts it: {ap:?}" + ); + } + } + + #[test] + fn upgrade_leaves_every_other_credential_kind_untouched() { + let cases = [ + WifiSecurity::Open, + // Empty PSK means "use the stored secret", which the saved profile + // already carries with the right key management. + WifiSecurity::WpaPsk { psk: String::new() }, + WifiSecurity::Sae { + psk: "password".into(), + }, + enterprise_credentials(), + wpa3_enterprise_credentials(), + ]; + + for creds in cases { + assert_eq!( + upgrade_security_for_features(creds.clone(), &sae_only_ap()), + creds, + "only a non-empty WpaPsk should be rewritten: {creds:?}" + ); + } + } } diff --git a/nmrs/src/util/validation.rs b/nmrs/src/util/validation.rs index 0167e849..58b4beb6 100644 --- a/nmrs/src/util/validation.rs +++ b/nmrs/src/util/validation.rs @@ -138,6 +138,26 @@ pub fn validate_wifi_security(security: &WifiSecurity) -> Result<(), ConnectionE Ok(()) } + // SAE has no 8-character floor: WPA3-Personal passphrases may be + // shorter than WPA2-PSK requires, so only the upper bound applies. + WifiSecurity::Sae { psk } => { + // Allow empty PSK only if user wants to use saved credentials + if psk.is_empty() { + return Ok(()); + } + + let psk_len = psk.len(); + + if psk_len > MAX_WPA_PSK_LENGTH { + return Err(ConnectionError::InvalidAddress(format!( + "SAE password too long: {} characters (maximum {} characters)", + psk_len, MAX_WPA_PSK_LENGTH + ))); + } + + Ok(()) + } + WifiSecurity::WpaEap { opts } => { validate_wifi_eap(opts)?; @@ -975,6 +995,44 @@ mod tests { ); } + #[test] + fn test_validate_wifi_security_sae_valid() { + let sae = WifiSecurity::Sae { + psk: "password123".to_string(), + }; + assert!(validate_wifi_security(&sae).is_ok()); + } + + #[test] + fn test_validate_wifi_security_sae_allows_short_passphrase() { + // SAE has no 8-character minimum, unlike WPA-PSK. + let sae = WifiSecurity::Sae { + psk: "short".to_string(), + }; + assert!(validate_wifi_security(&sae).is_ok()); + } + + #[test] + fn test_validate_wifi_security_sae_empty() { + let sae = WifiSecurity::Sae { + psk: "".to_string(), + }; + // Empty passphrase is allowed (for saved credentials) + assert!(validate_wifi_security(&sae).is_ok()); + } + + #[test] + fn test_validate_wifi_security_sae_too_long() { + let sae = WifiSecurity::Sae { + psk: "a".repeat(64), + }; + assert_error_message!( + validate_wifi_security(&sae), + InvalidAddress, + "SAE password too long: 64 characters (maximum 63 characters)" + ); + } + #[test] fn test_validate_wifi_security_eap_valid() { let eap = WifiSecurity::WpaEap { diff --git a/nmrs/tests/integration_test.rs b/nmrs/tests/integration_test.rs index 6feb3542..cf6ef91e 100644 --- a/nmrs/tests/integration_test.rs +++ b/nmrs/tests/integration_test.rs @@ -10,8 +10,8 @@ use nmrs::agent::{SecretAgent, SecretAgentFlags, SecretAgentHandle, SecretSettin use nmrs::builders::WireGuardBuilder; use nmrs::raw::zvariant::{OwnedObjectPath, OwnedValue, Value}; use nmrs::{ - ActiveConnection, ActiveConnectionState, ConnectByUuidConfig, ConnectionError, DeviceState, - MonitorHandle, NetworkEvent, NetworkEventStream, NetworkManager, SettingsChange, + ActiveConnection, ActiveConnectionState, ConnectByUuidConfig, ConnectType, ConnectionError, + DeviceState, MonitorHandle, NetworkEvent, NetworkEventStream, NetworkManager, SettingsChange, SettingsEventStream, SettingsPatch, SettingsSummary, TimeoutConfig, WifiKeyMgmt, WifiScope, WifiSecurity, WireGuardPeer, }; @@ -119,6 +119,36 @@ async fn cleanup_saved_profile(nm: &NetworkManager, uuid: &str) -> Vec { } } +/// Reads back the key management NetworkManager actually stored for `ssid`. +/// +/// Asserting on the stored profile rather than on connectivity alone is what +/// distinguishes an SAE profile from a PSK profile that happened to associate. +async fn saved_wifi_key_mgmt(nm: &NetworkManager, ssid: &str) -> WifiKeyMgmt { + let uuid = bounded( + "resolve the saved WiFi UUID", + DBUS_TIMEOUT, + nm.get_saved_connection_uuid(ssid), + ) + .await + .expect("failed to resolve the saved WiFi UUID") + .unwrap_or_else(|| panic!("a successful connection to {ssid:?} created no saved profile")); + let saved = bounded( + "decode the saved WiFi profile", + DBUS_TIMEOUT, + nm.get_saved_connection(&uuid), + ) + .await + .expect("failed to decode the saved WiFi profile"); + + match saved.summary { + SettingsSummary::Wifi { + security: Some(security), + .. + } => security.key_mgmt, + other => panic!("expected a secured WiFi settings summary, got {other:?}"), + } +} + async fn cleanup_wifi_profile(wifi: &WifiScope, ssid: &str) -> Vec { let mut failures = Vec::new(); @@ -1120,6 +1150,215 @@ async fn wired_connection_lifecycle() { finish_after_cleanup(outcome, cleanup_failures); } +/// Proves WPA3-Personal (SAE) authentication against the SAE-only hwsim BSS. +/// +/// The regression this guards (issue #486): `WifiSecurity::WpaPsk` emits +/// `key-mgmt=wpa-psk`, which NetworkManager rejects outright on an access point +/// advertising SAE without PSK, with +/// `802-11-wireless-security.key-mgmt: Access point does not support PSK but +/// setting requires it`. Connecting must succeed and must leave behind a +/// profile NetworkManager stored as SAE, not merely a profile that connected. +#[tokio::test] +#[serial] +#[ignore = "requires the isolated mac80211_hwsim WiFi harness"] +async fn wifi_sae_only_access_point_accepts_psk_and_sae_credentials() { + required_capability("NMRS_REQUIRE_WIFI_SAE"); + let interface = required_env("NMRS_WIFI_INTERFACE"); + let ssid = required_env("NMRS_EXPECT_WIFI_SAE_SSID"); + let password = required_env("NMRS_WIFI_SAE_PASSWORD"); + + let nm = network_manager().await; + let initial_wifi_enabled = bounded( + "read the initial WiFi radio state", + DBUS_TIMEOUT, + nm.wifi_state(), + ) + .await + .expect("failed to capture the WiFi radio state before the test") + .enabled; + let wifi = nm.wifi(&interface); + + let outcome = AssertUnwindSafe(async { + bounded( + "enable the WiFi radio", + DBUS_TIMEOUT, + nm.set_wireless_enabled(true), + ) + .await + .expect("the harness declared WiFi available, but enabling it failed"); + bounded( + "wait for the WiFi device to become ready", + DBUS_TIMEOUT, + nm.wait_for_wifi_ready(), + ) + .await + .expect("the harness WiFi device did not become ready"); + bounded( + "remove any stale SAE test profile", + DBUS_TIMEOUT, + wifi.forget(&ssid), + ) + .await + .expect("failed to remove a stale SAE test profile"); + + // Pin the harness precondition: without this, a PSK-capable AP would + // let the WpaPsk connect below pass for the wrong reason. + bounded("scan for the SAE network", WIFI_TIMEOUT, wifi.scan()) + .await + .expect("failed to scan for the SAE network"); + // A single scan often reports nothing yet, so poll as the WPA test does. + let network = timeout(Duration::from_secs(15), async { + loop { + let networks = wifi + .list_networks() + .await + .expect("listing WiFi scan results failed"); + if let Some(network) = networks.into_iter().find(|network| network.ssid == ssid) { + return network; + } + sleep(Duration::from_millis(500)).await; + } + }) + .await + .unwrap_or_else(|_| panic!("the SAE harness network {ssid:?} was not discovered")); + assert_eq!(network.device, interface); + assert!( + network.security_features.sae, + "the harness AP must advertise SAE: {:?}", + network.security_features + ); + assert!( + !network.security_features.psk, + "the harness AP must not advertise PSK, or this test proves nothing: {:?}", + network.security_features + ); + assert_eq!( + network.security_features.preferred_connect_type(), + ConnectType::Sae + ); + + // 1. PSK credentials against an SAE-only AP: the reported failure. + bounded( + "connect to the SAE-only AP with WpaPsk credentials", + WIFI_TIMEOUT, + wifi.connect( + &ssid, + WifiSecurity::WpaPsk { + psk: password.clone(), + }, + ), + ) + .await + .expect("WpaPsk credentials must reach an SAE-only AP, not be rejected by NetworkManager"); + assert!( + bounded( + "check the SAE connection state", + DBUS_TIMEOUT, + nm.is_connected(&ssid), + ) + .await + .expect("failed to query the SAE connection state") + ); + assert_eq!( + saved_wifi_key_mgmt(&nm, &ssid).await, + WifiKeyMgmt::Sae, + "PSK credentials on an SAE-only AP must be stored as SAE, not wpa-psk" + ); + + // 2. The same network requested explicitly as SAE. + bounded( + "forget the upgraded SAE profile", + WIFI_TIMEOUT, + wifi.forget(&ssid), + ) + .await + .expect("failed to forget the upgraded SAE profile"); + bounded( + "connect with explicit SAE credentials", + WIFI_TIMEOUT, + wifi.connect( + &ssid, + WifiSecurity::Sae { + psk: password.clone(), + }, + ), + ) + .await + .expect("explicit SAE credentials failed against the SAE-only AP"); + assert_eq!(saved_wifi_key_mgmt(&nm, &ssid).await, WifiKeyMgmt::Sae); + + let saved_uuid = bounded( + "resolve the saved SAE UUID", + DBUS_TIMEOUT, + nm.get_saved_connection_uuid(&ssid), + ) + .await + .expect("failed to resolve the saved SAE UUID") + .expect("a successful SAE connection had no saved UUID"); + let active = active_connections(&nm).await; + let typed_wifi = active + .iter() + .find_map(|connection| match connection { + ActiveConnection::Wifi(wifi) if wifi.ssid == ssid => Some(wifi.clone()), + _ => None, + }) + .unwrap_or_else(|| { + panic!("typed active connections omitted the SAE network: {active:?}") + }); + assert_eq!(typed_wifi.uuid, saved_uuid); + assert_eq!(typed_wifi.interface.as_deref(), Some(interface.as_str())); + assert_eq!(typed_wifi.state, ActiveConnectionState::Activated); + assert!( + typed_wifi + .ip4_address + .as_deref() + .is_some_and(|address| address.starts_with("192.168.252.")), + "the SAE connection did not complete DHCP on the SAE BSS: {:?}", + typed_wifi.ip4_address + ); + + // 3. An empty SAE passphrase reuses the secret NetworkManager stored. + bounded( + "disconnect from the SAE network", + DBUS_TIMEOUT, + wifi.disconnect(), + ) + .await + .expect("failed to disconnect from the SAE network"); + bounded( + "reconnect with the stored SAE secret", + WIFI_TIMEOUT, + wifi.connect(&ssid, WifiSecurity::Sae { psk: String::new() }), + ) + .await + .expect("stored-secret SAE reconnect failed"); + assert!( + bounded( + "check the stored-secret SAE reconnect", + DBUS_TIMEOUT, + nm.is_connected(&ssid), + ) + .await + .expect("failed to query the stored-secret SAE reconnect") + ); + assert_eq!(saved_wifi_key_mgmt(&nm, &ssid).await, WifiKeyMgmt::Sae); + }) + .catch_unwind() + .await; + + let mut cleanup_failures = cleanup_wifi_profile(&wifi, &ssid).await; + match timeout(DBUS_TIMEOUT, nm.set_wireless_enabled(initial_wifi_enabled)).await { + Ok(Ok(())) => {} + Ok(Err(error)) => cleanup_failures.push(format!( + "restore WiFi radio enabled={initial_wifi_enabled}: {error}" + )), + Err(_) => cleanup_failures.push(format!( + "restore WiFi radio enabled={initial_wifi_enabled}: timed out" + )), + } + finish_after_cleanup(outcome, cleanup_failures); +} + /// Proves discovery, WPA authentication, DHCP, saved-secret reuse, and cleanup /// against the deterministic mac80211_hwsim access point. #[tokio::test] diff --git a/scripts/README.md b/scripts/README.md index 2992dc63..85fdd064 100644 --- a/scripts/README.md +++ b/scripts/README.md @@ -89,8 +89,8 @@ touches the host's NetworkManager profiles. It starts a private D-Bus system bus, `udevd`, and NetworkManager, then provisions whichever fixtures the selected mode needs: -- a `mac80211_hwsim` virtual Wi-Fi network (`hostapd` + `wpa_supplicant` + - `dnsmasq`) on SSID `nmrs-hwsim` +- two `mac80211_hwsim` virtual Wi-Fi networks (`hostapd` + `wpa_supplicant` + + `dnsmasq`): WPA2-PSK on SSID `nmrs-hwsim` and SAE-only on `nmrs-hwsim-sae` - a wired `veth` pair (`nmrs-client` / `nmrs-server`) with its own `dnsmasq` for DHCP - a WireGuard interface for native-tunnel activation @@ -118,7 +118,7 @@ The `wifi-integration` mode needs two `mac80211_hwsim` radios on the host and runs with `network_mode: host`: ```bash -sudo modprobe mac80211_hwsim radios=2 +sudo modprobe mac80211_hwsim radios=3 docker compose run --build --rm test-wifi-integration sudo modprobe -r mac80211_hwsim ``` diff --git a/scripts/ci/run-networkmanager-tests.sh b/scripts/ci/run-networkmanager-tests.sh index ab03947e..1d8fc095 100755 --- a/scripts/ci/run-networkmanager-tests.sh +++ b/scripts/ci/run-networkmanager-tests.sh @@ -10,16 +10,26 @@ readonly dbus_log="${runtime_dir}/dbus.log" readonly udev_log="${runtime_dir}/udev.log" readonly networkmanager_log="${runtime_dir}/networkmanager.log" readonly hostapd_log="${runtime_dir}/hostapd.log" +readonly sae_hostapd_log="${runtime_dir}/sae-hostapd.log" readonly dnsmasq_log="${runtime_dir}/dnsmasq.log" +readonly sae_dnsmasq_log="${runtime_dir}/sae-dnsmasq.log" readonly wired_dnsmasq_log="${runtime_dir}/wired-dnsmasq.log" readonly wpa_supplicant_log="${runtime_dir}/wpa_supplicant.log" readonly hostapd_config="${runtime_dir}/hostapd.conf" +readonly sae_hostapd_config="${runtime_dir}/sae-hostapd.conf" readonly networkmanager_config="${runtime_dir}/NetworkManager.conf" readonly dnsmasq_leases="${runtime_dir}/dnsmasq.leases" +readonly sae_dnsmasq_leases="${runtime_dir}/sae-dnsmasq.leases" readonly wired_dnsmasq_leases="${runtime_dir}/wired-dnsmasq.leases" readonly hwsim_ssid="nmrs-hwsim" readonly hwsim_password="nmrs-hwsim-password" readonly hwsim_gateway="192.168.250.1" +# Second access point on its own radio, advertising SAE without PSK. +# WPA3-Personal rejects `key-mgmt=wpa-psk`, which is what the SAE integration +# test exercises. +readonly hwsim_sae_ssid="nmrs-hwsim-sae" +readonly hwsim_sae_password="nmrs-hwsim-sae-password" +readonly hwsim_sae_gateway="192.168.252.1" readonly wired_client_interface="nmrs-client" readonly wired_server_interface="nmrs-server" readonly wired_gateway="192.168.251.1" @@ -28,10 +38,13 @@ dbus_pid="" udev_pid="" networkmanager_pid="" hostapd_pid="" +sae_hostapd_pid="" dnsmasq_pid="" +sae_dnsmasq_pid="" wired_dnsmasq_pid="" wpa_supplicant_pid="" hwsim_station_interface="" +hwsim_sae_ap_interface="" wired_veth_created=false stop_process() { @@ -49,8 +62,10 @@ cleanup() { stop_process "${networkmanager_pid}" stop_process "${wpa_supplicant_pid}" stop_process "${dnsmasq_pid}" + stop_process "${sae_dnsmasq_pid}" stop_process "${wired_dnsmasq_pid}" stop_process "${hostapd_pid}" + stop_process "${sae_hostapd_pid}" if [[ "${wired_veth_created}" == true ]] && ip link show "${wired_client_interface}" >/dev/null 2>&1; then ip link delete "${wired_client_interface}" || true fi @@ -65,7 +80,9 @@ cleanup() { "${networkmanager_log}" \ "${wpa_supplicant_log}" \ "${hostapd_log}" \ + "${sae_hostapd_log}" \ "${dnsmasq_log}" \ + "${sae_dnsmasq_log}" \ "${wired_dnsmasq_log}"; do if [[ -s "${log_file}" ]]; then printf '\n===== %s =====\n' "$(basename "${log_file}")" >&2 @@ -87,6 +104,37 @@ print_hostapd_log() { cat "${hostapd_log}" >&2 || true } +print_sae_hostapd_log() { + echo "The SAE hostapd did not become ready. Its log follows:" >&2 + cat "${sae_hostapd_log}" >&2 || true +} + +# Starts one hostapd per access point and waits for its beacon. +start_hostapd() { + local config="$1" log="$2" pid + + hostapd "${config}" >"${log}" 2>&1 & + pid=$! + + for _ in $(seq 1 15); do + if grep --quiet 'AP-ENABLED' "${log}"; then + break + fi + + if ! kill -0 "${pid}" 2>/dev/null; then + return 1 + fi + + sleep 1 + done + + if ! grep --quiet 'AP-ENABLED' "${log}"; then + return 1 + fi + + printf '%s\n' "${pid}" +} + print_wpa_supplicant_log() { echo "wpa_supplicant did not become ready. Its log follows:" >&2 cat "${wpa_supplicant_log}" >&2 || true @@ -97,6 +145,11 @@ print_dnsmasq_log() { cat "${dnsmasq_log}" >&2 || true } +print_sae_dnsmasq_log() { + echo "The SAE dnsmasq did not become ready. Its log follows:" >&2 + cat "${sae_dnsmasq_log}" >&2 || true +} + start_dbus() { mkdir -p /run/dbus rm -f /run/dbus/system_bus_socket @@ -211,14 +264,15 @@ setup_hwsim_access_point() { fi done | sort ) - if (( ${#wifi_interfaces[@]} != 2 )); then - echo "Expected exactly two mac80211_hwsim interfaces, found ${#wifi_interfaces[@]}" >&2 + if (( ${#wifi_interfaces[@]} != 3 )); then + echo "Expected exactly three mac80211_hwsim interfaces, found ${#wifi_interfaces[@]}" >&2 iw dev >&2 || true exit 1 fi ap_interface="${wifi_interfaces[0]}" - hwsim_station_interface="${wifi_interfaces[1]}" + hwsim_sae_ap_interface="${wifi_interfaces[1]}" + hwsim_station_interface="${wifi_interfaces[2]}" printf '%s\n' \ "interface=${ap_interface}" \ @@ -231,29 +285,36 @@ setup_hwsim_access_point() { 'wpa_key_mgmt=WPA-PSK' \ 'rsn_pairwise=CCMP' >"${hostapd_config}" - hostapd "${hostapd_config}" >"${hostapd_log}" 2>&1 & - hostapd_pid=$! - - for _ in $(seq 1 15); do - if grep --quiet 'AP-ENABLED' "${hostapd_log}"; then - break - fi - - if ! kill -0 "${hostapd_pid}" 2>/dev/null; then - print_hostapd_log - exit 1 - fi - - sleep 1 - done + # SAE-only: no WPA-PSK in wpa_key_mgmt, and PMF required, as WPA3-Personal + # mandates. NetworkManager rejects a wpa-psk profile against it, which is + # what makes it a useful fixture. It gets its own radio and hostapd + # process; a second BSS on the PSK radio did not initialize under hwsim. + printf '%s\n' \ + "interface=${hwsim_sae_ap_interface}" \ + 'driver=nl80211' \ + "ssid=${hwsim_sae_ssid}" \ + 'hw_mode=g' \ + 'channel=1' \ + 'wpa=2' \ + "sae_password=${hwsim_sae_password}" \ + 'wpa_key_mgmt=SAE' \ + 'rsn_pairwise=CCMP' \ + 'ieee80211w=2' \ + 'sae_require_mfp=1' >"${sae_hostapd_config}" - if ! grep --quiet 'AP-ENABLED' "${hostapd_log}"; then + if ! hostapd_pid="$(start_hostapd "${hostapd_config}" "${hostapd_log}")"; then print_hostapd_log exit 1 fi + if ! sae_hostapd_pid="$(start_hostapd "${sae_hostapd_config}" "${sae_hostapd_log}")"; then + print_sae_hostapd_log + exit 1 + fi ip link set "${ap_interface}" up ip address replace "${hwsim_gateway}/24" dev "${ap_interface}" + ip link set "${hwsim_sae_ap_interface}" up + ip address replace "${hwsim_sae_gateway}/24" dev "${hwsim_sae_ap_interface}" # NetworkManager's activation does not complete until DHCP succeeds. Run a # DHCP-only dnsmasq bound to the hwsim AP interface; port=0 avoids exposing @@ -289,6 +350,39 @@ setup_hwsim_access_point() { exit 1 fi + # The SAE radio is a separate L2 segment, so it needs its own DHCP server + # on its own subnet. A second instance keeps the PSK setup untouched. + dnsmasq \ + --no-daemon \ + --conf-file=/dev/null \ + --interface="${hwsim_sae_ap_interface}" \ + --bind-interfaces \ + --port=0 \ + --dhcp-authoritative \ + --dhcp-range=192.168.252.10,192.168.252.50,255.255.255.0,1h \ + --dhcp-option=3,"${hwsim_sae_gateway}" \ + --dhcp-leasefile="${sae_dnsmasq_leases}" \ + --log-dhcp >"${sae_dnsmasq_log}" 2>&1 & + sae_dnsmasq_pid=$! + + for _ in $(seq 1 10); do + if grep --quiet 'DHCP, IP range' "${sae_dnsmasq_log}"; then + break + fi + + if ! kill -0 "${sae_dnsmasq_pid}" 2>/dev/null; then + print_sae_dnsmasq_log + exit 1 + fi + + sleep 1 + done + + if ! kill -0 "${sae_dnsmasq_pid}" 2>/dev/null; then + print_sae_dnsmasq_log + exit 1 + fi + # Keep NetworkManager away from the runner's interfaces and AP radio. printf '%s\n' \ '[main]' \ @@ -457,6 +551,9 @@ if [[ "${mode}" == "wifi-integration" ]]; then export NMRS_EXPECT_WIFI_SSID="${hwsim_ssid}" export NMRS_WIFI_PASSWORD="${hwsim_password}" export NMRS_WIFI_INTERFACE="${hwsim_station_interface}" + export NMRS_REQUIRE_WIFI_SAE=1 + export NMRS_EXPECT_WIFI_SAE_SSID="${hwsim_sae_ssid}" + export NMRS_WIFI_SAE_PASSWORD="${hwsim_sae_password}" elif [[ "${mode}" == "all" || "${mode}" == "integration" ]]; then nmcli device set "${wired_client_interface}" managed yes