diff --git a/docs/src/api/builders.md b/docs/src/api/builders.md index 7cbc71a6..551f7497 100644 --- a/docs/src/api/builders.md +++ b/docs/src/api/builders.md @@ -134,7 +134,11 @@ use nmrs::builders::{build_wifi_connection, build_ethernet_connection}; use nmrs::{WifiSecurity, ConnectionOptions}; // Wi-Fi -let wifi = build_wifi_connection("MyNetwork", &WifiSecurity::Open, &ConnectionOptions::default()); +let wifi = build_wifi_connection( + "MyNetwork", + &WifiSecurity::Open, + &ConnectionOptions::default(), +)?; // Ethernet let eth = build_ethernet_connection("eth0", &ConnectionOptions::default()); @@ -195,7 +199,7 @@ let settings = build_wifi_connection( "GuestWiFi", &WifiSecurity::WpaPsk { psk: "password".into() }, &ConnectionOptions::new(true), -); +)?; let profile = nm.add_connection(settings).await?; ``` diff --git a/docs/src/api/network-manager.md b/docs/src/api/network-manager.md index 0765bf3a..07fc27f9 100644 --- a/docs/src/api/network-manager.md +++ b/docs/src/api/network-manager.md @@ -32,7 +32,7 @@ let settings = build_wifi_connection( "GuestWiFi", &WifiSecurity::WpaPsk { psk: "password".into() }, &ConnectionOptions::new(true), -); +)?; let profile = nm.add_connection(settings).await?; ``` diff --git a/nmrs/CHANGELOG.md b/nmrs/CHANGELOG.md index 44ae5b76..cf701800 100644 --- a/nmrs/CHANGELOG.md +++ b/nmrs/CHANGELOG.md @@ -4,6 +4,13 @@ All notable changes to the `nmrs` crate will be documented in this file. ## [Unreleased] +### Changed + +- **Breaking:** WPA-EAP connection builders and `build_wifi_connection()` now + return `Result`, reporting conflicting certificate path/blob inputs as + `ConnectionError::InvalidInput` instead of panicking. + ([#478](https://github.com/freedesktop-rs/nmrs/issues/478)) + ## [3.4.2] - 2026-07-27 ### Changed diff --git a/nmrs/src/api/builders/mod.rs b/nmrs/src/api/builders/mod.rs index 3e651ec5..92bb1070 100644 --- a/nmrs/src/api/builders/mod.rs +++ b/nmrs/src/api/builders/mod.rs @@ -54,7 +54,8 @@ //! "MyNetwork", //! &WifiSecurity::WpaPsk { psk: "password".into() }, //! &opts, -//! ); +//! ) +//! .expect("valid Wi-Fi settings"); //! let eth = build_ethernet_connection("eth0", &opts); //! ``` //! diff --git a/nmrs/src/api/builders/wifi.rs b/nmrs/src/api/builders/wifi.rs index 83941fa1..59da53bc 100644 --- a/nmrs/src/api/builders/wifi.rs +++ b/nmrs/src/api/builders/wifi.rs @@ -33,7 +33,7 @@ use zvariant::Value; use super::connection_builder::ConnectionBuilder; use super::wifi_builder::WifiConnectionBuilder; -use crate::api::models::{self, ConnectionOptions}; +use crate::api::models::{self, ConnectionError, ConnectionOptions}; /// Builds a complete Wi-Fi connection settings dictionary. /// @@ -57,12 +57,17 @@ use crate::api::models::{self, ConnectionOptions}; /// /// This function is maintained for backward compatibility. For new code, /// consider using `WifiConnectionBuilder` for a more ergonomic API. -#[must_use] +/// +/// # Errors +/// +/// Returns [`ConnectionError::InvalidInput`] when an EAP certificate or key +/// is supplied as both a path and a blob. +#[must_use = "handle invalid Wi-Fi EAP inputs before using the settings"] pub fn build_wifi_connection( ssid: &str, security: &models::WifiSecurity, opts: &ConnectionOptions, -) -> HashMap<&'static str, HashMap<&'static str, Value<'static>>> { +) -> Result>>, ConnectionError> { let mut builder = WifiConnectionBuilder::new(ssid) .options(opts) .ipv4_auto() @@ -71,11 +76,11 @@ pub fn build_wifi_connection( builder = match security { models::WifiSecurity::Open => builder.open(), models::WifiSecurity::WpaPsk { psk } => builder.wpa_psk(psk), - models::WifiSecurity::WpaEap { opts } => builder.wpa_eap(opts.clone()), - models::WifiSecurity::Wpa3Eap192bit { opts } => builder.wpa3_eap_192_bit(opts.clone()), + models::WifiSecurity::WpaEap { opts } => builder.wpa_eap(opts.clone())?, + models::WifiSecurity::Wpa3Eap192bit { opts } => builder.wpa3_eap_192_bit(opts.clone())?, }; - builder.build() + Ok(builder.build()) } /// Builds a complete Ethernet connection settings dictionary. @@ -114,6 +119,14 @@ mod tests { use crate::models::{ConnectionOptions, EapMethod, EapOptions, Phase2, WifiSecurity}; use zvariant::Value; + fn build_wifi_connection( + ssid: &str, + security: &WifiSecurity, + opts: &ConnectionOptions, + ) -> HashMap<&'static str, HashMap<&'static str, Value<'static>>> { + super::build_wifi_connection(ssid, security, opts).expect("valid Wi-Fi settings") + } + fn default_opts() -> ConnectionOptions { ConnectionOptions { autoconnect: true, @@ -141,6 +154,26 @@ mod tests { assert!(!conn.contains_key("802-11-wireless-security")); } + #[test] + fn conflicting_eap_ca_cert_path_and_blob_returns_invalid_input() { + let mut opts = EapOptions::new("user@example.com", "secret"); + opts.ca_cert_path = Some("file:///etc/ssl/certs/ca.pem".into()); + opts.ca_cert_blob = Some(vec![1, 2, 3]); + + match super::build_wifi_connection( + "enterprise", + &WifiSecurity::WpaEap { opts }, + &default_opts(), + ) { + Err(ConnectionError::InvalidInput { field, reason }) => { + assert_eq!(field, "ca_cert"); + assert_eq!(reason, "cannot specify both ca_cert_path and ca_cert_blob"); + } + Ok(_) => panic!("conflicting EAP certificate inputs should be rejected"), + Err(error) => panic!("expected InvalidInput, got {error:?}"), + } + } + #[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 97cb8991..95c60f58 100644 --- a/nmrs/src/api/builders/wifi_builder.rs +++ b/nmrs/src/api/builders/wifi_builder.rs @@ -7,7 +7,7 @@ use std::collections::HashMap; use zvariant::Value; use super::connection_builder::ConnectionBuilder; -use crate::api::models::{self, ConnectionOptions, EapMethod}; +use crate::api::models::{self, ConnectionError, ConnectionOptions, EapMethod}; /// WiFi band selection. #[non_exhaustive] @@ -92,6 +92,7 @@ impl WifiMode { /// /// let settings = WifiConnectionBuilder::new("CorpNetwork") /// .wpa_eap(eap_opts) +/// .expect("valid EAP options") /// .autoconnect(false) /// .build(); /// ``` @@ -176,21 +177,34 @@ impl WifiConnectionBuilder { /// Configures WPA-EAP (Enterprise) security with 802.1X authentication. /// /// Supports PEAP, TTLS, and TLS methods with various inner authentication protocols. - #[must_use] - pub fn wpa_eap(self, opts: models::EapOptions) -> Self { + /// + /// # Errors + /// + /// Returns [`ConnectionError::InvalidInput`] when a certificate or private + /// key is supplied as both a path and a blob. + #[must_use = "handle the invalid EAP configuration before continuing the builder chain"] + pub fn wpa_eap(self, opts: models::EapOptions) -> Result { self.wpa_eap_shared("wpa-eap", opts) } /// Configures WPA3-EAP (Enterprise) with 192bit security with 802.1X authentication. /// /// Supports only EAP-TLS. - #[must_use] - pub fn wpa3_eap_192_bit(self, opts: models::EapOptions) -> Self { + /// + /// # Errors + /// + /// Returns [`ConnectionError::InvalidInput`] when a certificate or private + /// key is supplied as both a path and a blob. + #[must_use = "handle the invalid EAP configuration before continuing the builder chain"] + pub fn wpa3_eap_192_bit(self, opts: models::EapOptions) -> Result { self.wpa_eap_shared("wpa-eap-suite-b-192", opts) } - #[must_use] - fn wpa_eap_shared(mut self, key_mgmt: &'static str, opts: models::EapOptions) -> Self { + fn wpa_eap_shared( + mut self, + key_mgmt: &'static str, + opts: models::EapOptions, + ) -> Result { let mut security = HashMap::new(); security.insert("key-mgmt", Value::from(key_mgmt)); security.insert("auth-alg", Value::from("open")); @@ -226,7 +240,7 @@ impl WifiConnectionBuilder { } EapMethod::Tls => { if let Some(cert) = - Self::path_or_blob("private_key", opts.private_key_path, opts.private_key_blob) + Self::path_or_blob("private_key", opts.private_key_path, opts.private_key_blob)? { e1x.insert("private-key", cert); } @@ -236,7 +250,7 @@ impl WifiConnectionBuilder { } if let Some(cert) = - Self::path_or_blob("client_cert", opts.client_cert_path, opts.client_cert_blob) + Self::path_or_blob("client_cert", opts.client_cert_path, opts.client_cert_blob)? { e1x.insert("client-cert", cert); } @@ -246,7 +260,7 @@ impl WifiConnectionBuilder { if opts.system_ca_certs { e1x.insert("system-ca-certs", Value::from(true)); } - if let Some(cert) = Self::path_or_blob("ca_cert", opts.ca_cert_path, opts.ca_cert_blob) { + if let Some(cert) = Self::path_or_blob("ca_cert", opts.ca_cert_path, opts.ca_cert_blob)? { e1x.insert("ca-cert", cert); } if let Some(dom) = opts.domain_suffix_match { @@ -255,7 +269,7 @@ impl WifiConnectionBuilder { self.inner = self.inner.with_section("802-1x", e1x); self.security_configured = true; - self + Ok(self) } /// Marks this network as hidden (doesn't broadcast SSID). @@ -412,14 +426,15 @@ impl WifiConnectionBuilder { attribute: &str, path: Option, blob: Option>, - ) -> Option> { + ) -> Result>, ConnectionError> { match (path, blob) { - (None, None) => None, - (Some(path), None) => Some(Self::path(path)), - (None, Some(blob)) => Some(Self::blob(blob)), - (Some(_), Some(_)) => { - panic!("Cannot specify both {attribute}_path and {attribute}_blob."); - } + (None, None) => Ok(None), + (Some(path), None) => Ok(Some(Self::path(path))), + (None, Some(blob)) => Ok(Some(Self::blob(blob))), + (Some(_), Some(_)) => Err(ConnectionError::InvalidInput { + field: attribute.to_string(), + reason: format!("cannot specify both {attribute}_path and {attribute}_blob"), + }), } } @@ -436,6 +451,7 @@ impl WifiConnectionBuilder { #[cfg(test)] mod tests { use super::*; + use crate::ConnectionError; use crate::models::{EapOptions, Phase2}; #[test] @@ -532,6 +548,7 @@ mod tests { let settings = WifiConnectionBuilder::new("Enterprise") .wpa_eap(eap_opts) + .expect("valid EAP options") .autoconnect(false) .ipv4_auto() .ipv6_auto() @@ -551,6 +568,22 @@ mod tests { assert_eq!(e1x.get("phase2-auth"), Some(&Value::from("mschapv2"))); } + #[test] + fn rejects_conflicting_eap_ca_cert_path_and_blob() { + let mut eap_opts = EapOptions::new("user@example.com", "secret"); + eap_opts.ca_cert_path = Some("file:///etc/ssl/certs/ca.pem".into()); + eap_opts.ca_cert_blob = Some(vec![1, 2, 3]); + + match WifiConnectionBuilder::new("Enterprise").wpa_eap(eap_opts) { + Err(ConnectionError::InvalidInput { field, reason }) => { + assert_eq!(field, "ca_cert"); + assert_eq!(reason, "cannot specify both ca_cert_path and ca_cert_blob"); + } + Ok(_) => panic!("conflicting EAP certificate inputs should be rejected"), + Err(error) => panic!("expected InvalidInput, got {error:?}"), + } + } + #[test] fn configures_hidden_network() { let settings = WifiConnectionBuilder::new("HiddenSSID") diff --git a/nmrs/src/api/network_manager.rs b/nmrs/src/api/network_manager.rs index 9b88df48..2bb20a3f 100644 --- a/nmrs/src/api/network_manager.rs +++ b/nmrs/src/api/network_manager.rs @@ -295,7 +295,7 @@ impl NetworkManager { /// "GuestWiFi", /// &WifiSecurity::WpaPsk { psk: "password".into() }, /// &opts, - /// ); + /// )?; /// let profile = nm.add_connection(settings).await?; /// println!("Saved profile at {}", profile.as_str()); /// # Ok(()) diff --git a/nmrs/src/core/connection.rs b/nmrs/src/core/connection.rs index a15daed9..5a1c8e66 100644 --- a/nmrs/src/core/connection.rs +++ b/nmrs/src/core/connection.rs @@ -776,7 +776,7 @@ async fn connect_via_saved( autoconnect_retries: None, }; - let settings = build_wifi_connection(ssid, creds, &opts); + let settings = build_wifi_connection(ssid, creds, &opts)?; debug!("Creating fresh connection with corrected settings"); let (new_connection, new_active_conn) = nm @@ -816,7 +816,7 @@ async fn connect_via_saved( autoconnect_retries: None, }; - let settings = build_wifi_connection(ssid, creds, &opts); + let settings = build_wifi_connection(ssid, creds, &opts)?; let (new_connection, active_conn) = nm .add_and_activate_connection(settings, wifi_device.clone(), ap.clone()) @@ -880,7 +880,7 @@ async fn build_and_activate_new( autoconnect_priority: None, }; - let settings = build_wifi_connection(ssid, &creds, &opts); + let settings = build_wifi_connection(ssid, &creds, &opts)?; trace!( "Creating new connection with {} settings sections", diff --git a/nmrs/src/lib.rs b/nmrs/src/lib.rs index 4323ee5d..07473fe5 100644 --- a/nmrs/src/lib.rs +++ b/nmrs/src/lib.rs @@ -363,7 +363,8 @@ pub mod raw { /// use nmrs::{ConnectionOptions, WifiSecurity}; /// /// let opts = ConnectionOptions::new(true); -/// let settings = build_wifi_connection("MyNetwork", &WifiSecurity::Open, &opts); +/// let settings = build_wifi_connection("MyNetwork", &WifiSecurity::Open, &opts) +/// .expect("valid Wi-Fi settings"); /// // `settings` can be passed straight to NetworkManager via D-Bus. /// ``` pub mod builders {