From 389e5beb64df3521b276dd63a31242be1bc093d6 Mon Sep 17 00:00:00 2001 From: Rage Lopez Date: Wed, 29 Jul 2026 19:55:48 -0500 Subject: [PATCH 1/3] fix(#478): return errors for conflicting EAP cert inputs --- docs/src/api/builders.md | 8 +++- docs/src/api/network-manager.md | 2 +- nmrs/CHANGELOG.md | 7 +++ nmrs/src/api/builders/mod.rs | 3 +- nmrs/src/api/builders/wifi.rs | 45 ++++++++++++++--- nmrs/src/api/builders/wifi_builder.rs | 69 ++++++++++++++++++++------- nmrs/src/api/network_manager.rs | 2 +- nmrs/src/core/connection.rs | 6 +-- nmrs/src/lib.rs | 3 +- 9 files changed, 112 insertions(+), 33 deletions(-) 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 { From f644fff6c3a6901cc5407053e246ad8fbc0abcb0 Mon Sep 17 00:00:00 2001 From: Rage Lopez Date: Sat, 1 Aug 2026 00:06:55 -0500 Subject: [PATCH 2/3] fix(#478): make EAP cert sources exclusive --- docs/src/api/builders.md | 8 +- docs/src/api/models.md | 18 ++- docs/src/api/network-manager.md | 2 +- nmrs/CHANGELOG.md | 6 +- nmrs/src/api/builders/mod.rs | 3 +- nmrs/src/api/builders/wifi.rs | 89 ++++--------- nmrs/src/api/builders/wifi_builder.rs | 90 ++++--------- nmrs/src/api/models/tests.rs | 59 ++++----- nmrs/src/api/models/wifi.rs | 159 ++++++++--------------- nmrs/src/api/network_manager.rs | 2 +- nmrs/src/core/connection.rs | 6 +- nmrs/src/lib.rs | 6 +- nmrs/src/util/validation.rs | 174 ++++++++++---------------- 13 files changed, 222 insertions(+), 400 deletions(-) diff --git a/docs/src/api/builders.md b/docs/src/api/builders.md index 551f7497..7cbc71a6 100644 --- a/docs/src/api/builders.md +++ b/docs/src/api/builders.md @@ -134,11 +134,7 @@ 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()); @@ -199,7 +195,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/models.md b/docs/src/api/models.md index 6491c501..ce25cf58 100644 --- a/docs/src/api/models.md +++ b/docs/src/api/models.md @@ -319,19 +319,31 @@ pub struct EapOptions { pub password: String, pub anonymous_identity: Option, pub domain_suffix_match: Option, - pub ca_cert_path: Option, + pub ca_cert: Option, pub system_ca_certs: bool, pub method: EapMethod, pub phase2: Phase2, + pub private_key: Option, + pub private_key_password: Option, + pub client_cert: Option, +} + +pub enum EapCertSource { + Path(String), + Blob(Vec), } ``` -Constructors: `new(identity, password)`, `builder()` +Each certificate or private key has exactly one source representation. Use the +existing `*_path` or `*_blob` builder methods to select it. + +Constructors: `new(identity, password)`, `new_tls_path(...)`, +`new_tls_blob(...)`, `builder()` ### EapMethod / Phase2 ```rust -pub enum EapMethod { Peap, Ttls } +pub enum EapMethod { Peap, Ttls, Tls } pub enum Phase2 { Mschapv2, Pap } ``` diff --git a/docs/src/api/network-manager.md b/docs/src/api/network-manager.md index 07fc27f9..0765bf3a 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 cf701800..7d395fa8 100644 --- a/nmrs/CHANGELOG.md +++ b/nmrs/CHANGELOG.md @@ -6,9 +6,9 @@ All notable changes to the `nmrs` crate will be documented in this file. ### 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. +- **Breaking:** `EapOptions` now represents each certificate or private key as + an `Option`, making path/blob conflicts unrepresentable while + preserving infallible `build_wifi_connection()` and fluent WPA-EAP chaining. ([#478](https://github.com/freedesktop-rs/nmrs/issues/478)) ## [3.4.2] - 2026-07-27 diff --git a/nmrs/src/api/builders/mod.rs b/nmrs/src/api/builders/mod.rs index 92bb1070..3e651ec5 100644 --- a/nmrs/src/api/builders/mod.rs +++ b/nmrs/src/api/builders/mod.rs @@ -54,8 +54,7 @@ //! "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 59da53bc..725e8136 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, ConnectionError, ConnectionOptions}; +use crate::api::models::{self, ConnectionOptions}; /// Builds a complete Wi-Fi connection settings dictionary. /// @@ -57,17 +57,12 @@ use crate::api::models::{self, ConnectionError, ConnectionOptions}; /// /// This function is maintained for backward compatibility. For new code, /// consider using `WifiConnectionBuilder` for a more ergonomic API. -/// -/// # 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"] +#[must_use] pub fn build_wifi_connection( ssid: &str, security: &models::WifiSecurity, opts: &ConnectionOptions, -) -> Result>>, ConnectionError> { +) -> HashMap<&'static str, HashMap<&'static str, Value<'static>>> { let mut builder = WifiConnectionBuilder::new(ssid) .options(opts) .ipv4_auto() @@ -76,11 +71,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()), }; - Ok(builder.build()) + builder.build() } /// Builds a complete Ethernet connection settings dictionary. @@ -116,17 +111,11 @@ pub fn build_ethernet_connection( #[cfg(test)] mod tests { use super::*; - use crate::models::{ConnectionOptions, EapMethod, EapOptions, Phase2, WifiSecurity}; + use crate::models::{ + ConnectionOptions, EapCertSource, 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, @@ -154,26 +143,6 @@ 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()); @@ -223,16 +192,13 @@ mod tests { password: "secret123".into(), anonymous_identity: Some("anonymous@example.com".into()), domain_suffix_match: Some("example.com".into()), - ca_cert_path: None, - ca_cert_blob: None, + ca_cert: None, system_ca_certs: true, method: EapMethod::Peap, phase2: Phase2::Mschapv2, - private_key_path: None, - private_key_blob: None, + private_key: None, private_key_password: None, - client_cert_path: None, - client_cert_blob: None, + client_cert: None, }; let conn = build_wifi_connection( "enterprise", @@ -266,16 +232,13 @@ mod tests { password: "campus123".into(), anonymous_identity: None, domain_suffix_match: None, - ca_cert_path: Some("file:///etc/ssl/certs/ca.pem".into()), - ca_cert_blob: None, + ca_cert: Some(EapCertSource::Path("file:///etc/ssl/certs/ca.pem".into())), system_ca_certs: false, method: EapMethod::Ttls, phase2: Phase2::Pap, - private_key_path: None, - private_key_blob: None, + private_key: None, private_key_password: None, - client_cert_path: None, - client_cert_blob: None, + client_cert: None, }; let conn = build_wifi_connection( "eduroam", @@ -300,16 +263,17 @@ mod tests { password: String::new(), anonymous_identity: None, domain_suffix_match: None, - ca_cert_path: Some("file:///etc/ssl/certs/ca.pem".into()), - ca_cert_blob: None, + ca_cert: Some(EapCertSource::Path("file:///etc/ssl/certs/ca.pem".into())), system_ca_certs: false, method: EapMethod::Tls, phase2: Phase2::Mschapv2, - private_key_path: Some("file:///etc/ssl/private/client.key".into()), - private_key_blob: None, + private_key: Some(EapCertSource::Path( + "file:///etc/ssl/private/client.key".into(), + )), private_key_password: Some("password".into()), - client_cert_path: Some("file:///etc/ssl/certs/client.crt".into()), - client_cert_blob: None, + client_cert: Some(EapCertSource::Path( + "file:///etc/ssl/certs/client.crt".into(), + )), }; let conn = build_wifi_connection( "eduroam", @@ -351,16 +315,13 @@ mod tests { password: String::new(), anonymous_identity: None, domain_suffix_match: None, - ca_cert_path: None, - ca_cert_blob: Some(b"ca_cert_blob".into()), + ca_cert: Some(EapCertSource::Blob(b"ca_cert_blob".into())), system_ca_certs: false, method: EapMethod::Tls, phase2: Phase2::Mschapv2, - private_key_path: None, - private_key_blob: Some(b"private_key_blob".into()), + private_key: Some(EapCertSource::Blob(b"private_key_blob".into())), private_key_password: Some("password".into()), - client_cert_path: None, - client_cert_blob: Some(b"client_cert_blob".into()), + client_cert: Some(EapCertSource::Blob(b"client_cert_blob".into())), }; let conn = build_wifi_connection( "eduroam", diff --git a/nmrs/src/api/builders/wifi_builder.rs b/nmrs/src/api/builders/wifi_builder.rs index 95c60f58..6ed5a461 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, ConnectionError, ConnectionOptions, EapMethod}; +use crate::api::models::{self, ConnectionOptions, EapCertSource, EapMethod}; /// WiFi band selection. #[non_exhaustive] @@ -92,7 +92,6 @@ impl WifiMode { /// /// let settings = WifiConnectionBuilder::new("CorpNetwork") /// .wpa_eap(eap_opts) -/// .expect("valid EAP options") /// .autoconnect(false) /// .build(); /// ``` @@ -177,34 +176,21 @@ impl WifiConnectionBuilder { /// Configures WPA-EAP (Enterprise) security with 802.1X authentication. /// /// Supports PEAP, TTLS, and TLS methods with various inner authentication protocols. - /// - /// # 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 { + #[must_use] + pub fn wpa_eap(self, opts: models::EapOptions) -> Self { self.wpa_eap_shared("wpa-eap", opts) } /// Configures WPA3-EAP (Enterprise) with 192bit security with 802.1X authentication. /// /// Supports only EAP-TLS. - /// - /// # 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 { + #[must_use] + pub fn wpa3_eap_192_bit(self, opts: models::EapOptions) -> Self { self.wpa_eap_shared("wpa-eap-suite-b-192", opts) } - fn wpa_eap_shared( - mut self, - key_mgmt: &'static str, - opts: models::EapOptions, - ) -> Result { + #[must_use] + fn wpa_eap_shared(mut self, key_mgmt: &'static str, opts: models::EapOptions) -> Self { let mut security = HashMap::new(); security.insert("key-mgmt", Value::from(key_mgmt)); security.insert("auth-alg", Value::from("open")); @@ -239,20 +225,16 @@ impl WifiConnectionBuilder { e1x.insert("phase2-auth", Value::from(p2)); } EapMethod::Tls => { - if let Some(cert) = - Self::path_or_blob("private_key", opts.private_key_path, opts.private_key_blob)? - { - e1x.insert("private-key", cert); + if let Some(source) = opts.private_key { + e1x.insert("private-key", Self::cert_source(source)); } if let Some(password) = opts.private_key_password { e1x.insert("private-key-password", Value::from(password)); } - if let Some(cert) = - Self::path_or_blob("client_cert", opts.client_cert_path, opts.client_cert_blob)? - { - e1x.insert("client-cert", cert); + if let Some(source) = opts.client_cert { + e1x.insert("client-cert", Self::cert_source(source)); } } } @@ -260,8 +242,8 @@ 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)? { - e1x.insert("ca-cert", cert); + if let Some(source) = opts.ca_cert { + e1x.insert("ca-cert", Self::cert_source(source)); } if let Some(dom) = opts.domain_suffix_match { e1x.insert("domain-suffix-match", Value::from(dom)); @@ -269,7 +251,7 @@ impl WifiConnectionBuilder { self.inner = self.inner.with_section("802-1x", e1x); self.security_configured = true; - Ok(self) + self } /// Marks this network as hidden (doesn't broadcast SSID). @@ -422,19 +404,10 @@ impl WifiConnectionBuilder { Value::from(vals) } - fn path_or_blob( - attribute: &str, - path: Option, - blob: Option>, - ) -> Result>, ConnectionError> { - match (path, 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"), - }), + fn cert_source(source: EapCertSource) -> Value<'static> { + match source { + EapCertSource::Path(path) => Self::path(path), + EapCertSource::Blob(blob) => Self::blob(blob), } } @@ -451,7 +424,6 @@ impl WifiConnectionBuilder { #[cfg(test)] mod tests { use super::*; - use crate::ConnectionError; use crate::models::{EapOptions, Phase2}; #[test] @@ -534,21 +506,17 @@ mod tests { password: "secret".into(), anonymous_identity: Some("anon@example.com".into()), domain_suffix_match: Some("example.com".into()), - ca_cert_path: None, - ca_cert_blob: None, + ca_cert: None, system_ca_certs: true, method: EapMethod::Peap, phase2: Phase2::Mschapv2, - private_key_path: None, - private_key_blob: None, + private_key: None, private_key_password: None, - client_cert_path: None, - client_cert_blob: None, + client_cert: None, }; let settings = WifiConnectionBuilder::new("Enterprise") .wpa_eap(eap_opts) - .expect("valid EAP options") .autoconnect(false) .ipv4_auto() .ipv6_auto() @@ -568,22 +536,6 @@ 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/models/tests.rs b/nmrs/src/api/models/tests.rs index f542600d..4316c62b 100644 --- a/nmrs/src/api/models/tests.rs +++ b/nmrs/src/api/models/tests.rs @@ -211,16 +211,13 @@ fn wifi_security_eap() { password: "secret".into(), anonymous_identity: None, domain_suffix_match: None, - ca_cert_path: None, - ca_cert_blob: None, + ca_cert: None, system_ca_certs: false, method: EapMethod::Peap, phase2: Phase2::Mschapv2, - private_key_path: None, - private_key_blob: None, + private_key: None, private_key_password: None, - client_cert_path: None, - client_cert_blob: None, + client_cert: None, }, }; assert!(eap.secured()); @@ -236,16 +233,17 @@ fn wifi_security_eap_192bit() { password: "".into(), anonymous_identity: None, domain_suffix_match: None, - ca_cert_path: Some("file:///etc/ssl/certs/ca.crt".into()), - ca_cert_blob: None, + ca_cert: Some(EapCertSource::Path("file:///etc/ssl/certs/ca.crt".into())), system_ca_certs: false, method: EapMethod::Tls, phase2: Phase2::Mschapv2, - private_key_path: Some("file:///etc/ssl/private/client.key".into()), - private_key_blob: None, + private_key: Some(EapCertSource::Path( + "file:///etc/ssl/private/client.key".into(), + )), private_key_password: Some("password".into()), - client_cert_path: Some("file:///etc/ssl/certs/client.crt".into()), - client_cert_blob: None, + client_cert: Some(EapCertSource::Path( + "file:///etc/ssl/certs/client.crt".into(), + )), }, }; assert!(eap.secured()); @@ -1016,7 +1014,7 @@ fn test_eap_options_builder_basic() { assert_eq!(opts.phase2, Phase2::Mschapv2); assert!(opts.anonymous_identity.is_none()); assert!(opts.domain_suffix_match.is_none()); - assert!(opts.ca_cert_path.is_none()); + assert!(opts.ca_cert.is_none()); assert!(!opts.system_ca_certs); } @@ -1044,8 +1042,8 @@ fn test_eap_options_builder_with_optionals() { ); assert_eq!(opts.domain_suffix_match, Some("company.com".into())); assert_eq!( - opts.ca_cert_path, - Some("file:///etc/ssl/certs/ca.pem".into()) + opts.ca_cert, + Some(EapCertSource::Path("file:///etc/ssl/certs/ca.pem".into())) ); assert!(opts.system_ca_certs); } @@ -1080,8 +1078,10 @@ fn test_eap_options_builder_ttls_pap() { assert_eq!(opts.method, EapMethod::Ttls); assert_eq!(opts.phase2, Phase2::Pap); assert_eq!( - opts.ca_cert_path, - Some("file:///etc/ssl/certs/university.pem".into()) + opts.ca_cert, + Some(EapCertSource::Path( + "file:///etc/ssl/certs/university.pem".into() + )) ); } @@ -1099,17 +1099,21 @@ fn test_eap_options_builder_tls() { assert_eq!(opts.method, EapMethod::Tls); assert_eq!( - opts.ca_cert_path, - Some("file:///etc/ssl/certs/ca.pem".into()) + opts.ca_cert, + Some(EapCertSource::Path("file:///etc/ssl/certs/ca.pem".into())) ); assert_eq!( - opts.private_key_path, - Some("file:///etc/ssl/private/client.key".into()) + opts.private_key, + Some(EapCertSource::Path( + "file:///etc/ssl/private/client.key".into() + )) ); assert_eq!(opts.private_key_password, Some("password".into())); assert_eq!( - opts.client_cert_path, - Some("file:///etc/ssl/certs/client.pem".into()) + opts.client_cert, + Some(EapCertSource::Path( + "file:///etc/ssl/certs/client.pem".into() + )) ); } @@ -1159,8 +1163,7 @@ fn test_eap_options_builder_ca_cert_blob_overrides_path() { .unwrap(); assert_eq!(opts.method, EapMethod::Tls); - assert_eq!(opts.ca_cert_path, None); - assert_eq!(opts.ca_cert_blob, Some(vec![1])); + assert_eq!(opts.ca_cert, Some(EapCertSource::Blob(vec![1]))); } #[test] @@ -1177,8 +1180,7 @@ fn test_eap_options_builder_path_blob_private_key() { .unwrap(); assert_eq!(opts.method, EapMethod::Tls); - assert_eq!(opts.private_key_path, None); - assert_eq!(opts.private_key_blob, Some(vec![1])); + assert_eq!(opts.private_key, Some(EapCertSource::Blob(vec![1]))); } #[test] @@ -1195,8 +1197,7 @@ fn test_eap_options_builder_path_blob_client_cert() { .unwrap(); assert_eq!(opts.method, EapMethod::Tls); - assert_eq!(opts.client_cert_path, None); - assert_eq!(opts.client_cert_blob, Some(vec![1])); + assert_eq!(opts.client_cert, Some(EapCertSource::Blob(vec![1]))); } #[test] diff --git a/nmrs/src/api/models/wifi.rs b/nmrs/src/api/models/wifi.rs index 6e5eab00..3cc91eec 100644 --- a/nmrs/src/api/models/wifi.rs +++ b/nmrs/src/api/models/wifi.rs @@ -192,6 +192,20 @@ pub enum Phase2 { Pap, } +/// Source for an EAP certificate or private key. +/// +/// NetworkManager accepts these values either as a `file://` path or as the +/// encoded bytes. Using one enum makes those representations mutually +/// exclusive at the type level. +#[non_exhaustive] +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum EapCertSource { + /// Path to a certificate or private-key file (`file://` URL). + Path(String), + /// Encoded certificate or private-key bytes. + Blob(Vec), +} + /// EAP options for WPA-EAP (Enterprise) Wi-Fi connections. /// /// Configuration for 802.1X authentication, commonly used in corporate @@ -233,26 +247,20 @@ pub struct EapOptions { pub anonymous_identity: Option, /// Domain to match against server certificate pub domain_suffix_match: Option, - /// Path to CA certificate file (file:// URL), mutually exclusive with `ca_cert_blob` - pub ca_cert_path: Option, - /// CA certificate encoded as DER, mutually exclusive with `ca_cert_path` - pub ca_cert_blob: Option>, + /// CA certificate source. + pub ca_cert: Option, /// Use system CA certificate store pub system_ca_certs: bool, /// EAP method (PEAP or TTLS) pub method: EapMethod, /// PEAP/TTLS: Phase 2 inner authentication method pub phase2: Phase2, - /// TLS: Path to the private key file of the client certificate (file:// URL), mutually exclusive with `private_key_blob` - pub private_key_path: Option, - /// TLS: Private key of the client certificate encoded as PEM or PKCS#12, mutually exclusive with `private_key_path` - pub private_key_blob: Option>, + /// TLS: Private key source for the client certificate. + pub private_key: Option, /// TLS: Password for the private key file pub private_key_password: Option, - /// TLS: Path to the client certificate file (file:// URL), mutually exclusive with `client_cert_blob` - pub client_cert_path: Option, - /// TLS: Client certificate encoded as DER or PKCS#12, mutually exclusive with `client_cert_path` - pub client_cert_blob: Option>, + /// TLS: Client certificate source. + pub client_cert: Option, } impl fmt::Debug for EapOptions { @@ -263,19 +271,16 @@ impl fmt::Debug for EapOptions { .field("password", &Redacted) .field("anonymous_identity", &self.anonymous_identity) .field("domain_suffix_match", &self.domain_suffix_match) - .field("ca_cert_path", &self.ca_cert_path) - .field("ca_cert_blob", &self.ca_cert_blob) + .field("ca_cert", &self.ca_cert) .field("system_ca_certs", &self.system_ca_certs) .field("method", &self.method) .field("phase2", &self.phase2) - .field("private_key_path", &self.private_key_path) - .field("private_key_blob", &redact_option(&self.private_key_blob)) + .field("private_key", &redact_option(&self.private_key)) .field( "private_key_password", &redact_option(&self.private_key_password), ) - .field("client_cert_path", &self.client_cert_path) - .field("client_cert_blob", &self.client_cert_blob) + .field("client_cert", &self.client_cert) .finish() } } @@ -287,16 +292,13 @@ impl Default for EapOptions { password: String::new(), anonymous_identity: None, domain_suffix_match: None, - ca_cert_path: None, - ca_cert_blob: None, + ca_cert: None, system_ca_certs: false, method: EapMethod::Peap, phase2: Phase2::Mschapv2, - private_key_path: None, - private_key_blob: None, + private_key: None, private_key_password: None, - client_cert_path: None, - client_cert_blob: None, + client_cert: None, } } } @@ -340,8 +342,8 @@ impl EapOptions { Self { identity: identity.into(), method: EapMethod::Tls, - private_key_path: Some(private_key_path.into()), - client_cert_path: Some(client_cert_path.into()), + private_key: Some(EapCertSource::Path(private_key_path.into())), + client_cert: Some(EapCertSource::Path(client_cert_path.into())), ..Default::default() } } @@ -369,8 +371,8 @@ impl EapOptions { Self { identity: identity.into(), method: EapMethod::Tls, - private_key_blob: Some(private_key_blob.into()), - client_cert_blob: Some(client_cert_blob.into()), + private_key: Some(EapCertSource::Blob(private_key_blob.into())), + client_cert: Some(EapCertSource::Blob(client_cert_blob.into())), ..Default::default() } } @@ -415,22 +417,16 @@ impl EapOptions { } /// Sets the path to the CA certificate file (must start with `file://`). - /// - /// Clears `ca_cert_blob` because they are mutually exclusive. #[must_use] pub fn with_ca_cert_path(mut self, path: impl Into) -> Self { - self.ca_cert_blob = None; - self.ca_cert_path = Some(path.into()); + self.ca_cert = Some(EapCertSource::Path(path.into())); self } /// Sets the CA certificate encoded as DER. - /// - /// Clears `ca_cert_path` because they are mutually exclusive. #[must_use] pub fn with_ca_cert_blob(mut self, data: impl Into>) -> Self { - self.ca_cert_path = None; - self.ca_cert_blob = Some(data.into()); + self.ca_cert = Some(EapCertSource::Blob(data.into())); self } @@ -523,16 +519,13 @@ pub struct EapOptionsBuilder { password: Option, anonymous_identity: Option, domain_suffix_match: Option, - ca_cert_path: Option, - ca_cert_blob: Option>, + ca_cert: Option, system_ca_certs: bool, method: Option, phase2: Option, - private_key_path: Option, - private_key_blob: Option>, + private_key: Option, private_key_password: Option, - client_cert_path: Option, - client_cert_blob: Option>, + client_cert: Option, } impl fmt::Debug for EapOptionsBuilder { @@ -543,19 +536,16 @@ impl fmt::Debug for EapOptionsBuilder { .field("password", &redact_option(&self.password)) .field("anonymous_identity", &self.anonymous_identity) .field("domain_suffix_match", &self.domain_suffix_match) - .field("ca_cert_path", &self.ca_cert_path) - .field("ca_cert_blob", &self.ca_cert_blob) + .field("ca_cert", &self.ca_cert) .field("system_ca_certs", &self.system_ca_certs) .field("method", &self.method) .field("phase2", &self.phase2) - .field("private_key_path", &self.private_key_path) - .field("private_key_blob", &redact_option(&self.private_key_blob)) + .field("private_key", &redact_option(&self.private_key)) .field( "private_key_password", &redact_option(&self.private_key_password), ) - .field("client_cert_path", &self.client_cert_path) - .field("client_cert_blob", &self.client_cert_blob) + .field("client_cert", &self.client_cert) .finish() } } @@ -621,8 +611,6 @@ impl EapOptionsBuilder { /// /// The path must start with `file://` (e.g., "file:///etc/ssl/certs/ca.pem"). /// - /// Clears `ca_cert_blob` because they are mutually exclusive. - /// /// # Examples /// /// ```rust @@ -633,15 +621,12 @@ impl EapOptionsBuilder { /// ``` #[must_use] pub fn ca_cert_path(mut self, path: impl Into) -> Self { - self.ca_cert_blob = None; - self.ca_cert_path = Some(path.into()); + self.ca_cert = Some(EapCertSource::Path(path.into())); self } /// Sets the CA certificate encoded as DER. /// - /// Clears `ca_cert_path` because they are mutually exclusive. - /// /// # Examples /// /// ```rust @@ -652,8 +637,7 @@ impl EapOptionsBuilder { /// ``` #[must_use] pub fn ca_cert_blob(mut self, data: impl Into>) -> Self { - self.ca_cert_path = None; - self.ca_cert_blob = Some(data.into()); + self.ca_cert = Some(EapCertSource::Blob(data.into())); self } @@ -718,8 +702,6 @@ impl EapOptionsBuilder { /// /// The path must start with `file://` (e.g., "file:///etc/ssl/private/client.key"). /// - /// Clears `private_key_blob` because they are mutually exclusive. - /// /// # Examples /// /// ```rust @@ -730,15 +712,12 @@ impl EapOptionsBuilder { /// ``` #[must_use] pub fn private_key_path(mut self, path: impl Into) -> Self { - self.private_key_blob = None; - self.private_key_path = Some(path.into()); + self.private_key = Some(EapCertSource::Path(path.into())); self } /// Sets the private key of the client certificate encoded as PEM or PKCS#12. /// - /// Clears `private_key_path` because they are mutually exclusive. - /// /// # Examples /// /// ```rust @@ -749,8 +728,7 @@ impl EapOptionsBuilder { /// ``` #[must_use] pub fn private_key_blob(mut self, data: impl Into>) -> Self { - self.private_key_path = None; - self.private_key_blob = Some(data.into()); + self.private_key = Some(EapCertSource::Blob(data.into())); self } @@ -774,8 +752,6 @@ impl EapOptionsBuilder { /// /// The path must start with `file://` (e.g., "file:///etc/ssl/certs/client.crt"). /// - /// Clears `client_cert_blob` because they are mutually exclusive. - /// /// # Examples /// /// ```rust @@ -786,15 +762,12 @@ impl EapOptionsBuilder { /// ``` #[must_use] pub fn client_cert_path(mut self, path: impl Into) -> Self { - self.client_cert_blob = None; - self.client_cert_path = Some(path.into()); + self.client_cert = Some(EapCertSource::Path(path.into())); self } /// Sets the client certificate encoded as DER or PKCS#12. /// - /// Clears `client_cert_path` because they are mutually exclusive. - /// /// # Examples /// /// ```rust @@ -805,8 +778,7 @@ impl EapOptionsBuilder { /// ``` #[must_use] pub fn client_cert_blob(mut self, data: impl Into>) -> Self { - self.client_cert_path = None; - self.client_cert_blob = Some(data.into()); + self.client_cert = Some(EapCertSource::Blob(data.into())); self } @@ -835,38 +807,16 @@ impl EapOptionsBuilder { let is_peap_or_ttls = self.method == Some(EapMethod::Peap) || self.method == Some(EapMethod::Ttls); - if let (Some(_), Some(_)) = (&self.ca_cert_path, &self.ca_cert_blob) { + if let (Some(EapMethod::Tls), None) = (&self.method, &self.private_key) { return Err(ConnectionError::IncompleteBuilder( - "EAP CA certificate cannot be specified both as a path and blob".into(), + "EAP private key is required for TLS (use .private_key_path() or .private_key_blob())".into(), )); } - match (&self.method, &self.private_key_path, &self.private_key_blob) { - (_, Some(_), Some(_)) => { - return Err(ConnectionError::IncompleteBuilder( - "EAP private key cannot be specified both as a path and blob".into(), - )); - } - (Some(EapMethod::Tls), None, None) => { - return Err(ConnectionError::IncompleteBuilder( - "EAP private key is required for TLS (use .private_key_path() or .private_key_blob())".into(), - )); - } - _ => {} - } - - match (&self.method, &self.client_cert_path, &self.client_cert_blob) { - (_, Some(_), Some(_)) => { - return Err(ConnectionError::IncompleteBuilder( - "EAP client certificate cannot be specified both as a path and blob".into(), - )); - } - (Some(EapMethod::Tls), None, None) => { - return Err(ConnectionError::IncompleteBuilder( - "EAP client certificate is required for TLS (use .client_cert_path() or .client_cert_blob())".into(), - )); - } - _ => {} + if let (Some(EapMethod::Tls), None) = (&self.method, &self.client_cert) { + return Err(ConnectionError::IncompleteBuilder( + "EAP client certificate is required for TLS (use .client_cert_path() or .client_cert_blob())".into(), + )); } Ok(EapOptions { @@ -886,8 +836,7 @@ impl EapOptionsBuilder { }, anonymous_identity: self.anonymous_identity, domain_suffix_match: self.domain_suffix_match, - ca_cert_path: self.ca_cert_path, - ca_cert_blob: self.ca_cert_blob, + ca_cert: self.ca_cert, system_ca_certs: self.system_ca_certs, method: self.method.ok_or_else(|| { ConnectionError::IncompleteBuilder("EAP method is required (use .method())".into()) @@ -901,11 +850,9 @@ impl EapOptionsBuilder { } else { Phase2::Mschapv2 }, - private_key_path: self.private_key_path, - private_key_blob: self.private_key_blob, + private_key: self.private_key, private_key_password: self.private_key_password, - client_cert_path: self.client_cert_path, - client_cert_blob: self.client_cert_blob, + client_cert: self.client_cert, }) } } diff --git a/nmrs/src/api/network_manager.rs b/nmrs/src/api/network_manager.rs index 2bb20a3f..9b88df48 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 5a1c8e66..a15daed9 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 07473fe5..6468c6a7 100644 --- a/nmrs/src/lib.rs +++ b/nmrs/src/lib.rs @@ -112,7 +112,7 @@ //! - [`Device`] / [`DeviceType`] / [`DeviceState`] — network devices and their state //! - [`Network`] / [`AccessPoint`] / [`NetworkInfo`] — discovered Wi-Fi data //! - [`WifiDevice`] — per-Wi-Fi-device summary -//! - [`WifiSecurity`] / [`EapOptions`] / [`EapMethod`] / [`Phase2`] — Wi-Fi security +//! - [`WifiSecurity`] / [`EapOptions`] / [`EapCertSource`] / [`EapMethod`] / [`Phase2`] — Wi-Fi security //! - [`ConnectionOptions`] / [`TimeoutConfig`] — connection knobs //! - [`WireGuardConfig`] / [`WireGuardPeer`] — WireGuard configuration //! - [`OpenVpnConfig`] / [`OpenVpnAuthType`] / [`OpenVpnProxy`] — OpenVPN configuration @@ -363,8 +363,7 @@ pub mod raw { /// use nmrs::{ConnectionOptions, WifiSecurity}; /// /// let opts = ConnectionOptions::new(true); -/// let settings = build_wifi_connection("MyNetwork", &WifiSecurity::Open, &opts) -/// .expect("valid Wi-Fi settings"); +/// let settings = build_wifi_connection("MyNetwork", &WifiSecurity::Open, &opts); /// // `settings` can be passed straight to NetworkManager via D-Bus. /// ``` pub mod builders { @@ -435,6 +434,7 @@ pub mod models { } // Re-export commonly used types at crate root for convenience +pub use api::models::EapCertSource; #[allow(deprecated)] pub use api::models::{ AccessPoint, ActiveConnection, ActiveConnectionState, ActiveOtherConnection, diff --git a/nmrs/src/util/validation.rs b/nmrs/src/util/validation.rs index 0167e849..76745acc 100644 --- a/nmrs/src/util/validation.rs +++ b/nmrs/src/util/validation.rs @@ -9,7 +9,7 @@ use crate::api::models::{ ConnectionError, OpenVpnAuthType, OpenVpnConfig, OpenVpnProxy, VpnCredentials, WifiSecurity, WireGuardPeer, }; -use crate::{EapMethod, EapOptions}; +use crate::{EapCertSource, EapMethod, EapOptions}; /// Maximum SSID length in bytes (802.11 standard). const MAX_SSID_BYTES: usize = 32; @@ -194,21 +194,13 @@ fn validate_wifi_eap(opts: &EapOptions) -> Result<(), ConnectionError> { } } EapMethod::Tls => { - if !validate_path_or_blob( - "EAP private key", - &opts.private_key_path, - &opts.private_key_blob, - )? { + if !validate_cert_source("EAP private key", &opts.private_key)? { return Err(ConnectionError::InvalidAddress( "EAP private key must be provided".to_string(), )); } - if !validate_path_or_blob( - "EAP client certificate", - &opts.client_cert_path, - &opts.client_cert_blob, - )? { + if !validate_cert_source("EAP client certificate", &opts.client_cert)? { return Err(ConnectionError::InvalidAddress( "EAP client certificate must be provided".to_string(), )); @@ -216,20 +208,18 @@ fn validate_wifi_eap(opts: &EapOptions) -> Result<(), ConnectionError> { } } - validate_path_or_blob("EAP CA certificate", &opts.ca_cert_path, &opts.ca_cert_blob)?; + validate_cert_source("EAP CA certificate", &opts.ca_cert)?; Ok(()) } -fn validate_path_or_blob( +fn validate_cert_source( field: &str, - path: &Option, - blob: &Option>, + source: &Option, ) -> Result { - // Validate CA cert path if provided - match (path, blob) { - (None, None) => Ok(false), - (Some(path), None) => { + match source { + None => Ok(false), + Some(EapCertSource::Path(path)) => { if path.trim().is_empty() { return Err(ConnectionError::InvalidAddress(format!( "{field} path cannot be empty if provided" @@ -243,10 +233,7 @@ fn validate_path_or_blob( } Ok(true) } - (None, Some(_)) => Ok(true), - (Some(_), Some(_)) => Err(ConnectionError::InvalidAddress(format!( - "{field} path and blob cannot be provided at the same time" - ))), + Some(EapCertSource::Blob(_)) => Ok(true), } } @@ -983,16 +970,13 @@ mod tests { password: "password".to_string(), anonymous_identity: None, domain_suffix_match: Some("example.com".to_string()), - ca_cert_path: Some("file:///etc/ssl/cert.pem".to_string()), - ca_cert_blob: None, + ca_cert: Some(EapCertSource::Path("file:///etc/ssl/cert.pem".to_string())), system_ca_certs: false, method: EapMethod::Peap, phase2: Phase2::Mschapv2, - private_key_path: None, - private_key_blob: None, + private_key: None, private_key_password: None, - client_cert_path: None, - client_cert_blob: None, + client_cert: None, }, }; assert!(validate_wifi_security(&eap).is_ok()); @@ -1006,16 +990,13 @@ mod tests { password: "password".to_string(), anonymous_identity: None, domain_suffix_match: None, - ca_cert_path: None, - ca_cert_blob: None, + ca_cert: None, system_ca_certs: true, method: EapMethod::Peap, phase2: Phase2::Mschapv2, - private_key_path: None, - private_key_blob: None, + private_key: None, private_key_password: None, - client_cert_path: None, - client_cert_blob: None, + client_cert: None, }, }; assert_error_message!( @@ -1033,16 +1014,13 @@ mod tests { password: "password".to_string(), anonymous_identity: None, domain_suffix_match: None, - ca_cert_path: Some("/etc/ssl/cert.pem".to_string()), // Missing file:// - ca_cert_blob: None, + ca_cert: Some(EapCertSource::Path("/etc/ssl/cert.pem".to_string())), // Missing file:// system_ca_certs: false, method: EapMethod::Peap, phase2: Phase2::Mschapv2, - private_key_path: None, - private_key_blob: None, + private_key: None, private_key_password: None, - client_cert_path: None, - client_cert_blob: None, + client_cert: None, }, }; assert_error_message!( @@ -1060,16 +1038,13 @@ mod tests { password: "password".to_string(), anonymous_identity: None, domain_suffix_match: None, - ca_cert_path: None, - ca_cert_blob: None, + ca_cert: None, system_ca_certs: true, method: EapMethod::Peap, phase2: Phase2::Mschapv2, - private_key_path: None, - private_key_blob: None, + private_key: None, private_key_password: None, - client_cert_path: None, - client_cert_blob: None, + client_cert: None, }, }; assert_error_message!( @@ -1087,16 +1062,19 @@ mod tests { password: String::new(), anonymous_identity: None, domain_suffix_match: Some("example.com".to_string()), - ca_cert_path: Some("file:///etc/ssl/certs/ca.pem".to_string()), - ca_cert_blob: None, + ca_cert: Some(EapCertSource::Path( + "file:///etc/ssl/certs/ca.pem".to_string(), + )), system_ca_certs: false, method: EapMethod::Tls, phase2: Phase2::Mschapv2, - private_key_path: Some("file:///etc/ssl/private/client.pem".to_string()), - private_key_blob: None, + private_key: Some(EapCertSource::Path( + "file:///etc/ssl/private/client.pem".to_string(), + )), private_key_password: None, - client_cert_path: Some("file:///etc/ssl/certs/client.pem".to_string()), - client_cert_blob: None, + client_cert: Some(EapCertSource::Path( + "file:///etc/ssl/certs/client.pem".to_string(), + )), }, }; assert!(validate_wifi_security(&eap).is_ok()); @@ -1110,48 +1088,18 @@ mod tests { password: String::new(), anonymous_identity: None, domain_suffix_match: Some("example.com".to_string()), - ca_cert_path: None, - ca_cert_blob: Some(b"ca_cert_blob".to_vec()), + ca_cert: Some(EapCertSource::Blob(b"ca_cert_blob".to_vec())), system_ca_certs: false, method: EapMethod::Tls, phase2: Phase2::Mschapv2, - private_key_path: None, - private_key_blob: Some(b"private_key_blob".to_vec()), + private_key: Some(EapCertSource::Blob(b"private_key_blob".to_vec())), private_key_password: None, - client_cert_path: None, - client_cert_blob: Some(b"client_cert_blob".to_vec()), + client_cert: Some(EapCertSource::Blob(b"client_cert_blob".to_vec())), }, }; assert!(validate_wifi_security(&eap).is_ok()); } - #[test] - fn test_validate_wifi_security_eap_192bit_path_blob() { - let eap = WifiSecurity::Wpa3Eap192bit { - opts: EapOptions { - identity: "user@example.com".to_string(), - password: String::new(), - anonymous_identity: None, - domain_suffix_match: Some("example.com".to_string()), - ca_cert_path: Some("file:///etc/ssl/certs/ca.pem".to_string()), - ca_cert_blob: Some(b"ca_cert_blob".to_vec()), - system_ca_certs: false, - method: EapMethod::Tls, - phase2: Phase2::Mschapv2, - private_key_path: Some("file:///etc/ssl/private/client.pem".to_string()), - private_key_blob: Some(b"private_key_blob".to_vec()), - private_key_password: None, - client_cert_path: Some("file:///etc/ssl/certs/client.pem".to_string()), - client_cert_blob: Some(b"client_cert_blob".to_vec()), - }, - }; - assert_error_message!( - validate_wifi_security(&eap), - InvalidAddress, - "EAP private key path and blob cannot be provided at the same time" - ); - } - #[test] fn test_validate_wifi_security_eap_tls_invalid_private_key() { let eap = WifiSecurity::WpaEap { @@ -1160,16 +1108,19 @@ mod tests { password: String::new(), anonymous_identity: None, domain_suffix_match: Some("example.com".to_string()), - ca_cert_path: Some("file:///etc/ssl/certs/ca.pem".to_string()), - ca_cert_blob: None, + ca_cert: Some(EapCertSource::Path( + "file:///etc/ssl/certs/ca.pem".to_string(), + )), system_ca_certs: false, method: EapMethod::Tls, phase2: Phase2::Mschapv2, - private_key_path: Some("/etc/ssl/private/client.pem".to_string()), - private_key_blob: None, + private_key: Some(EapCertSource::Path( + "/etc/ssl/private/client.pem".to_string(), + )), private_key_password: None, - client_cert_path: Some("file:///etc/ssl/certs/client.pem".to_string()), - client_cert_blob: None, + client_cert: Some(EapCertSource::Path( + "file:///etc/ssl/certs/client.pem".to_string(), + )), }, }; assert_error_message!( @@ -1187,16 +1138,17 @@ mod tests { password: String::new(), anonymous_identity: None, domain_suffix_match: Some("example.com".to_string()), - ca_cert_path: Some("file:///etc/ssl/certs/ca.pem".to_string()), - ca_cert_blob: None, + ca_cert: Some(EapCertSource::Path( + "file:///etc/ssl/certs/ca.pem".to_string(), + )), system_ca_certs: false, method: EapMethod::Tls, phase2: Phase2::Mschapv2, - private_key_path: Some("file:///etc/ssl/private/client.pem".to_string()), - private_key_blob: None, + private_key: Some(EapCertSource::Path( + "file:///etc/ssl/private/client.pem".to_string(), + )), private_key_password: None, - client_cert_path: Some("/etc/ssl/certs/client.pem".to_string()), - client_cert_blob: None, + client_cert: Some(EapCertSource::Path("/etc/ssl/certs/client.pem".to_string())), }, }; assert_error_message!( @@ -1214,16 +1166,17 @@ mod tests { password: String::new(), anonymous_identity: None, domain_suffix_match: Some("example.com".to_string()), - ca_cert_path: Some("file:///etc/ssl/certs/ca.pem".to_string()), - ca_cert_blob: None, + ca_cert: Some(EapCertSource::Path( + "file:///etc/ssl/certs/ca.pem".to_string(), + )), system_ca_certs: false, method: EapMethod::Tls, phase2: Phase2::Mschapv2, - private_key_path: None, - private_key_blob: None, + private_key: None, private_key_password: None, - client_cert_path: Some("file:///etc/ssl/certs/client.pem".to_string()), - client_cert_blob: None, + client_cert: Some(EapCertSource::Path( + "file:///etc/ssl/certs/client.pem".to_string(), + )), }, }; assert_error_message!( @@ -1241,16 +1194,17 @@ mod tests { password: String::new(), anonymous_identity: None, domain_suffix_match: Some("example.com".to_string()), - ca_cert_path: Some("file:///etc/ssl/certs/ca.pem".to_string()), - ca_cert_blob: None, + ca_cert: Some(EapCertSource::Path( + "file:///etc/ssl/certs/ca.pem".to_string(), + )), system_ca_certs: false, method: EapMethod::Tls, phase2: Phase2::Mschapv2, - private_key_path: Some("file:///etc/ssl/private/client.pem".to_string()), - private_key_blob: None, + private_key: Some(EapCertSource::Path( + "file:///etc/ssl/private/client.pem".to_string(), + )), private_key_password: None, - client_cert_path: None, - client_cert_blob: None, + client_cert: None, }, }; assert_error_message!( From c9b052c1efb70f49712d729ebb3384f678ebbdf7 Mon Sep 17 00:00:00 2001 From: Akrm Al-Hakimi Date: Mon, 3 Aug 2026 21:14:09 -0400 Subject: [PATCH 3/3] fix(#478): return InvalidInput for conflicting EAP certs --- docs/src/api/builders.md | 8 +- docs/src/api/models.md | 18 +-- docs/src/api/network-manager.md | 2 +- nmrs/CHANGELOG.md | 6 +- nmrs/src/api/builders/mod.rs | 3 +- nmrs/src/api/builders/wifi.rs | 89 +++++++++---- nmrs/src/api/builders/wifi_builder.rs | 90 +++++++++---- nmrs/src/api/models/tests.rs | 59 +++++---- nmrs/src/api/models/wifi.rs | 159 +++++++++++++++-------- nmrs/src/api/network_manager.rs | 2 +- nmrs/src/core/connection.rs | 6 +- nmrs/src/lib.rs | 6 +- nmrs/src/util/validation.rs | 174 ++++++++++++++++---------- 13 files changed, 400 insertions(+), 222 deletions(-) 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/models.md b/docs/src/api/models.md index ce25cf58..6491c501 100644 --- a/docs/src/api/models.md +++ b/docs/src/api/models.md @@ -319,31 +319,19 @@ pub struct EapOptions { pub password: String, pub anonymous_identity: Option, pub domain_suffix_match: Option, - pub ca_cert: Option, + pub ca_cert_path: Option, pub system_ca_certs: bool, pub method: EapMethod, pub phase2: Phase2, - pub private_key: Option, - pub private_key_password: Option, - pub client_cert: Option, -} - -pub enum EapCertSource { - Path(String), - Blob(Vec), } ``` -Each certificate or private key has exactly one source representation. Use the -existing `*_path` or `*_blob` builder methods to select it. - -Constructors: `new(identity, password)`, `new_tls_path(...)`, -`new_tls_blob(...)`, `builder()` +Constructors: `new(identity, password)`, `builder()` ### EapMethod / Phase2 ```rust -pub enum EapMethod { Peap, Ttls, Tls } +pub enum EapMethod { Peap, Ttls } pub enum Phase2 { Mschapv2, Pap } ``` 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 7d395fa8..cf701800 100644 --- a/nmrs/CHANGELOG.md +++ b/nmrs/CHANGELOG.md @@ -6,9 +6,9 @@ All notable changes to the `nmrs` crate will be documented in this file. ### Changed -- **Breaking:** `EapOptions` now represents each certificate or private key as - an `Option`, making path/blob conflicts unrepresentable while - preserving infallible `build_wifi_connection()` and fluent WPA-EAP chaining. +- **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 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 725e8136..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. @@ -111,11 +116,17 @@ pub fn build_ethernet_connection( #[cfg(test)] mod tests { use super::*; - use crate::models::{ - ConnectionOptions, EapCertSource, EapMethod, EapOptions, Phase2, WifiSecurity, - }; + 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, @@ -143,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()); @@ -192,13 +223,16 @@ mod tests { password: "secret123".into(), anonymous_identity: Some("anonymous@example.com".into()), domain_suffix_match: Some("example.com".into()), - ca_cert: None, + ca_cert_path: None, + ca_cert_blob: None, system_ca_certs: true, method: EapMethod::Peap, phase2: Phase2::Mschapv2, - private_key: None, + private_key_path: None, + private_key_blob: None, private_key_password: None, - client_cert: None, + client_cert_path: None, + client_cert_blob: None, }; let conn = build_wifi_connection( "enterprise", @@ -232,13 +266,16 @@ mod tests { password: "campus123".into(), anonymous_identity: None, domain_suffix_match: None, - ca_cert: Some(EapCertSource::Path("file:///etc/ssl/certs/ca.pem".into())), + ca_cert_path: Some("file:///etc/ssl/certs/ca.pem".into()), + ca_cert_blob: None, system_ca_certs: false, method: EapMethod::Ttls, phase2: Phase2::Pap, - private_key: None, + private_key_path: None, + private_key_blob: None, private_key_password: None, - client_cert: None, + client_cert_path: None, + client_cert_blob: None, }; let conn = build_wifi_connection( "eduroam", @@ -263,17 +300,16 @@ mod tests { password: String::new(), anonymous_identity: None, domain_suffix_match: None, - ca_cert: Some(EapCertSource::Path("file:///etc/ssl/certs/ca.pem".into())), + ca_cert_path: Some("file:///etc/ssl/certs/ca.pem".into()), + ca_cert_blob: None, system_ca_certs: false, method: EapMethod::Tls, phase2: Phase2::Mschapv2, - private_key: Some(EapCertSource::Path( - "file:///etc/ssl/private/client.key".into(), - )), + private_key_path: Some("file:///etc/ssl/private/client.key".into()), + private_key_blob: None, private_key_password: Some("password".into()), - client_cert: Some(EapCertSource::Path( - "file:///etc/ssl/certs/client.crt".into(), - )), + client_cert_path: Some("file:///etc/ssl/certs/client.crt".into()), + client_cert_blob: None, }; let conn = build_wifi_connection( "eduroam", @@ -315,13 +351,16 @@ mod tests { password: String::new(), anonymous_identity: None, domain_suffix_match: None, - ca_cert: Some(EapCertSource::Blob(b"ca_cert_blob".into())), + ca_cert_path: None, + ca_cert_blob: Some(b"ca_cert_blob".into()), system_ca_certs: false, method: EapMethod::Tls, phase2: Phase2::Mschapv2, - private_key: Some(EapCertSource::Blob(b"private_key_blob".into())), + private_key_path: None, + private_key_blob: Some(b"private_key_blob".into()), private_key_password: Some("password".into()), - client_cert: Some(EapCertSource::Blob(b"client_cert_blob".into())), + client_cert_path: None, + client_cert_blob: Some(b"client_cert_blob".into()), }; let conn = build_wifi_connection( "eduroam", diff --git a/nmrs/src/api/builders/wifi_builder.rs b/nmrs/src/api/builders/wifi_builder.rs index 6ed5a461..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, EapCertSource, 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")); @@ -225,16 +239,20 @@ impl WifiConnectionBuilder { e1x.insert("phase2-auth", Value::from(p2)); } EapMethod::Tls => { - if let Some(source) = opts.private_key { - e1x.insert("private-key", Self::cert_source(source)); + if let Some(cert) = + Self::path_or_blob("private_key", opts.private_key_path, opts.private_key_blob)? + { + e1x.insert("private-key", cert); } if let Some(password) = opts.private_key_password { e1x.insert("private-key-password", Value::from(password)); } - if let Some(source) = opts.client_cert { - e1x.insert("client-cert", Self::cert_source(source)); + if let Some(cert) = + Self::path_or_blob("client_cert", opts.client_cert_path, opts.client_cert_blob)? + { + e1x.insert("client-cert", cert); } } } @@ -242,8 +260,8 @@ impl WifiConnectionBuilder { if opts.system_ca_certs { e1x.insert("system-ca-certs", Value::from(true)); } - if let Some(source) = opts.ca_cert { - e1x.insert("ca-cert", Self::cert_source(source)); + 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 { e1x.insert("domain-suffix-match", Value::from(dom)); @@ -251,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). @@ -404,10 +422,19 @@ impl WifiConnectionBuilder { Value::from(vals) } - fn cert_source(source: EapCertSource) -> Value<'static> { - match source { - EapCertSource::Path(path) => Self::path(path), - EapCertSource::Blob(blob) => Self::blob(blob), + fn path_or_blob( + attribute: &str, + path: Option, + blob: Option>, + ) -> Result>, ConnectionError> { + match (path, 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"), + }), } } @@ -424,6 +451,7 @@ impl WifiConnectionBuilder { #[cfg(test)] mod tests { use super::*; + use crate::ConnectionError; use crate::models::{EapOptions, Phase2}; #[test] @@ -506,17 +534,21 @@ mod tests { password: "secret".into(), anonymous_identity: Some("anon@example.com".into()), domain_suffix_match: Some("example.com".into()), - ca_cert: None, + ca_cert_path: None, + ca_cert_blob: None, system_ca_certs: true, method: EapMethod::Peap, phase2: Phase2::Mschapv2, - private_key: None, + private_key_path: None, + private_key_blob: None, private_key_password: None, - client_cert: None, + client_cert_path: None, + client_cert_blob: None, }; let settings = WifiConnectionBuilder::new("Enterprise") .wpa_eap(eap_opts) + .expect("valid EAP options") .autoconnect(false) .ipv4_auto() .ipv6_auto() @@ -536,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/models/tests.rs b/nmrs/src/api/models/tests.rs index 4316c62b..f542600d 100644 --- a/nmrs/src/api/models/tests.rs +++ b/nmrs/src/api/models/tests.rs @@ -211,13 +211,16 @@ fn wifi_security_eap() { password: "secret".into(), anonymous_identity: None, domain_suffix_match: None, - ca_cert: None, + ca_cert_path: None, + ca_cert_blob: None, system_ca_certs: false, method: EapMethod::Peap, phase2: Phase2::Mschapv2, - private_key: None, + private_key_path: None, + private_key_blob: None, private_key_password: None, - client_cert: None, + client_cert_path: None, + client_cert_blob: None, }, }; assert!(eap.secured()); @@ -233,17 +236,16 @@ fn wifi_security_eap_192bit() { password: "".into(), anonymous_identity: None, domain_suffix_match: None, - ca_cert: Some(EapCertSource::Path("file:///etc/ssl/certs/ca.crt".into())), + ca_cert_path: Some("file:///etc/ssl/certs/ca.crt".into()), + ca_cert_blob: None, system_ca_certs: false, method: EapMethod::Tls, phase2: Phase2::Mschapv2, - private_key: Some(EapCertSource::Path( - "file:///etc/ssl/private/client.key".into(), - )), + private_key_path: Some("file:///etc/ssl/private/client.key".into()), + private_key_blob: None, private_key_password: Some("password".into()), - client_cert: Some(EapCertSource::Path( - "file:///etc/ssl/certs/client.crt".into(), - )), + client_cert_path: Some("file:///etc/ssl/certs/client.crt".into()), + client_cert_blob: None, }, }; assert!(eap.secured()); @@ -1014,7 +1016,7 @@ fn test_eap_options_builder_basic() { assert_eq!(opts.phase2, Phase2::Mschapv2); assert!(opts.anonymous_identity.is_none()); assert!(opts.domain_suffix_match.is_none()); - assert!(opts.ca_cert.is_none()); + assert!(opts.ca_cert_path.is_none()); assert!(!opts.system_ca_certs); } @@ -1042,8 +1044,8 @@ fn test_eap_options_builder_with_optionals() { ); assert_eq!(opts.domain_suffix_match, Some("company.com".into())); assert_eq!( - opts.ca_cert, - Some(EapCertSource::Path("file:///etc/ssl/certs/ca.pem".into())) + opts.ca_cert_path, + Some("file:///etc/ssl/certs/ca.pem".into()) ); assert!(opts.system_ca_certs); } @@ -1078,10 +1080,8 @@ fn test_eap_options_builder_ttls_pap() { assert_eq!(opts.method, EapMethod::Ttls); assert_eq!(opts.phase2, Phase2::Pap); assert_eq!( - opts.ca_cert, - Some(EapCertSource::Path( - "file:///etc/ssl/certs/university.pem".into() - )) + opts.ca_cert_path, + Some("file:///etc/ssl/certs/university.pem".into()) ); } @@ -1099,21 +1099,17 @@ fn test_eap_options_builder_tls() { assert_eq!(opts.method, EapMethod::Tls); assert_eq!( - opts.ca_cert, - Some(EapCertSource::Path("file:///etc/ssl/certs/ca.pem".into())) + opts.ca_cert_path, + Some("file:///etc/ssl/certs/ca.pem".into()) ); assert_eq!( - opts.private_key, - Some(EapCertSource::Path( - "file:///etc/ssl/private/client.key".into() - )) + opts.private_key_path, + Some("file:///etc/ssl/private/client.key".into()) ); assert_eq!(opts.private_key_password, Some("password".into())); assert_eq!( - opts.client_cert, - Some(EapCertSource::Path( - "file:///etc/ssl/certs/client.pem".into() - )) + opts.client_cert_path, + Some("file:///etc/ssl/certs/client.pem".into()) ); } @@ -1163,7 +1159,8 @@ fn test_eap_options_builder_ca_cert_blob_overrides_path() { .unwrap(); assert_eq!(opts.method, EapMethod::Tls); - assert_eq!(opts.ca_cert, Some(EapCertSource::Blob(vec![1]))); + assert_eq!(opts.ca_cert_path, None); + assert_eq!(opts.ca_cert_blob, Some(vec![1])); } #[test] @@ -1180,7 +1177,8 @@ fn test_eap_options_builder_path_blob_private_key() { .unwrap(); assert_eq!(opts.method, EapMethod::Tls); - assert_eq!(opts.private_key, Some(EapCertSource::Blob(vec![1]))); + assert_eq!(opts.private_key_path, None); + assert_eq!(opts.private_key_blob, Some(vec![1])); } #[test] @@ -1197,7 +1195,8 @@ fn test_eap_options_builder_path_blob_client_cert() { .unwrap(); assert_eq!(opts.method, EapMethod::Tls); - assert_eq!(opts.client_cert, Some(EapCertSource::Blob(vec![1]))); + assert_eq!(opts.client_cert_path, None); + assert_eq!(opts.client_cert_blob, Some(vec![1])); } #[test] diff --git a/nmrs/src/api/models/wifi.rs b/nmrs/src/api/models/wifi.rs index 3cc91eec..6e5eab00 100644 --- a/nmrs/src/api/models/wifi.rs +++ b/nmrs/src/api/models/wifi.rs @@ -192,20 +192,6 @@ pub enum Phase2 { Pap, } -/// Source for an EAP certificate or private key. -/// -/// NetworkManager accepts these values either as a `file://` path or as the -/// encoded bytes. Using one enum makes those representations mutually -/// exclusive at the type level. -#[non_exhaustive] -#[derive(Debug, Clone, PartialEq, Eq)] -pub enum EapCertSource { - /// Path to a certificate or private-key file (`file://` URL). - Path(String), - /// Encoded certificate or private-key bytes. - Blob(Vec), -} - /// EAP options for WPA-EAP (Enterprise) Wi-Fi connections. /// /// Configuration for 802.1X authentication, commonly used in corporate @@ -247,20 +233,26 @@ pub struct EapOptions { pub anonymous_identity: Option, /// Domain to match against server certificate pub domain_suffix_match: Option, - /// CA certificate source. - pub ca_cert: Option, + /// Path to CA certificate file (file:// URL), mutually exclusive with `ca_cert_blob` + pub ca_cert_path: Option, + /// CA certificate encoded as DER, mutually exclusive with `ca_cert_path` + pub ca_cert_blob: Option>, /// Use system CA certificate store pub system_ca_certs: bool, /// EAP method (PEAP or TTLS) pub method: EapMethod, /// PEAP/TTLS: Phase 2 inner authentication method pub phase2: Phase2, - /// TLS: Private key source for the client certificate. - pub private_key: Option, + /// TLS: Path to the private key file of the client certificate (file:// URL), mutually exclusive with `private_key_blob` + pub private_key_path: Option, + /// TLS: Private key of the client certificate encoded as PEM or PKCS#12, mutually exclusive with `private_key_path` + pub private_key_blob: Option>, /// TLS: Password for the private key file pub private_key_password: Option, - /// TLS: Client certificate source. - pub client_cert: Option, + /// TLS: Path to the client certificate file (file:// URL), mutually exclusive with `client_cert_blob` + pub client_cert_path: Option, + /// TLS: Client certificate encoded as DER or PKCS#12, mutually exclusive with `client_cert_path` + pub client_cert_blob: Option>, } impl fmt::Debug for EapOptions { @@ -271,16 +263,19 @@ impl fmt::Debug for EapOptions { .field("password", &Redacted) .field("anonymous_identity", &self.anonymous_identity) .field("domain_suffix_match", &self.domain_suffix_match) - .field("ca_cert", &self.ca_cert) + .field("ca_cert_path", &self.ca_cert_path) + .field("ca_cert_blob", &self.ca_cert_blob) .field("system_ca_certs", &self.system_ca_certs) .field("method", &self.method) .field("phase2", &self.phase2) - .field("private_key", &redact_option(&self.private_key)) + .field("private_key_path", &self.private_key_path) + .field("private_key_blob", &redact_option(&self.private_key_blob)) .field( "private_key_password", &redact_option(&self.private_key_password), ) - .field("client_cert", &self.client_cert) + .field("client_cert_path", &self.client_cert_path) + .field("client_cert_blob", &self.client_cert_blob) .finish() } } @@ -292,13 +287,16 @@ impl Default for EapOptions { password: String::new(), anonymous_identity: None, domain_suffix_match: None, - ca_cert: None, + ca_cert_path: None, + ca_cert_blob: None, system_ca_certs: false, method: EapMethod::Peap, phase2: Phase2::Mschapv2, - private_key: None, + private_key_path: None, + private_key_blob: None, private_key_password: None, - client_cert: None, + client_cert_path: None, + client_cert_blob: None, } } } @@ -342,8 +340,8 @@ impl EapOptions { Self { identity: identity.into(), method: EapMethod::Tls, - private_key: Some(EapCertSource::Path(private_key_path.into())), - client_cert: Some(EapCertSource::Path(client_cert_path.into())), + private_key_path: Some(private_key_path.into()), + client_cert_path: Some(client_cert_path.into()), ..Default::default() } } @@ -371,8 +369,8 @@ impl EapOptions { Self { identity: identity.into(), method: EapMethod::Tls, - private_key: Some(EapCertSource::Blob(private_key_blob.into())), - client_cert: Some(EapCertSource::Blob(client_cert_blob.into())), + private_key_blob: Some(private_key_blob.into()), + client_cert_blob: Some(client_cert_blob.into()), ..Default::default() } } @@ -417,16 +415,22 @@ impl EapOptions { } /// Sets the path to the CA certificate file (must start with `file://`). + /// + /// Clears `ca_cert_blob` because they are mutually exclusive. #[must_use] pub fn with_ca_cert_path(mut self, path: impl Into) -> Self { - self.ca_cert = Some(EapCertSource::Path(path.into())); + self.ca_cert_blob = None; + self.ca_cert_path = Some(path.into()); self } /// Sets the CA certificate encoded as DER. + /// + /// Clears `ca_cert_path` because they are mutually exclusive. #[must_use] pub fn with_ca_cert_blob(mut self, data: impl Into>) -> Self { - self.ca_cert = Some(EapCertSource::Blob(data.into())); + self.ca_cert_path = None; + self.ca_cert_blob = Some(data.into()); self } @@ -519,13 +523,16 @@ pub struct EapOptionsBuilder { password: Option, anonymous_identity: Option, domain_suffix_match: Option, - ca_cert: Option, + ca_cert_path: Option, + ca_cert_blob: Option>, system_ca_certs: bool, method: Option, phase2: Option, - private_key: Option, + private_key_path: Option, + private_key_blob: Option>, private_key_password: Option, - client_cert: Option, + client_cert_path: Option, + client_cert_blob: Option>, } impl fmt::Debug for EapOptionsBuilder { @@ -536,16 +543,19 @@ impl fmt::Debug for EapOptionsBuilder { .field("password", &redact_option(&self.password)) .field("anonymous_identity", &self.anonymous_identity) .field("domain_suffix_match", &self.domain_suffix_match) - .field("ca_cert", &self.ca_cert) + .field("ca_cert_path", &self.ca_cert_path) + .field("ca_cert_blob", &self.ca_cert_blob) .field("system_ca_certs", &self.system_ca_certs) .field("method", &self.method) .field("phase2", &self.phase2) - .field("private_key", &redact_option(&self.private_key)) + .field("private_key_path", &self.private_key_path) + .field("private_key_blob", &redact_option(&self.private_key_blob)) .field( "private_key_password", &redact_option(&self.private_key_password), ) - .field("client_cert", &self.client_cert) + .field("client_cert_path", &self.client_cert_path) + .field("client_cert_blob", &self.client_cert_blob) .finish() } } @@ -611,6 +621,8 @@ impl EapOptionsBuilder { /// /// The path must start with `file://` (e.g., "file:///etc/ssl/certs/ca.pem"). /// + /// Clears `ca_cert_blob` because they are mutually exclusive. + /// /// # Examples /// /// ```rust @@ -621,12 +633,15 @@ impl EapOptionsBuilder { /// ``` #[must_use] pub fn ca_cert_path(mut self, path: impl Into) -> Self { - self.ca_cert = Some(EapCertSource::Path(path.into())); + self.ca_cert_blob = None; + self.ca_cert_path = Some(path.into()); self } /// Sets the CA certificate encoded as DER. /// + /// Clears `ca_cert_path` because they are mutually exclusive. + /// /// # Examples /// /// ```rust @@ -637,7 +652,8 @@ impl EapOptionsBuilder { /// ``` #[must_use] pub fn ca_cert_blob(mut self, data: impl Into>) -> Self { - self.ca_cert = Some(EapCertSource::Blob(data.into())); + self.ca_cert_path = None; + self.ca_cert_blob = Some(data.into()); self } @@ -702,6 +718,8 @@ impl EapOptionsBuilder { /// /// The path must start with `file://` (e.g., "file:///etc/ssl/private/client.key"). /// + /// Clears `private_key_blob` because they are mutually exclusive. + /// /// # Examples /// /// ```rust @@ -712,12 +730,15 @@ impl EapOptionsBuilder { /// ``` #[must_use] pub fn private_key_path(mut self, path: impl Into) -> Self { - self.private_key = Some(EapCertSource::Path(path.into())); + self.private_key_blob = None; + self.private_key_path = Some(path.into()); self } /// Sets the private key of the client certificate encoded as PEM or PKCS#12. /// + /// Clears `private_key_path` because they are mutually exclusive. + /// /// # Examples /// /// ```rust @@ -728,7 +749,8 @@ impl EapOptionsBuilder { /// ``` #[must_use] pub fn private_key_blob(mut self, data: impl Into>) -> Self { - self.private_key = Some(EapCertSource::Blob(data.into())); + self.private_key_path = None; + self.private_key_blob = Some(data.into()); self } @@ -752,6 +774,8 @@ impl EapOptionsBuilder { /// /// The path must start with `file://` (e.g., "file:///etc/ssl/certs/client.crt"). /// + /// Clears `client_cert_blob` because they are mutually exclusive. + /// /// # Examples /// /// ```rust @@ -762,12 +786,15 @@ impl EapOptionsBuilder { /// ``` #[must_use] pub fn client_cert_path(mut self, path: impl Into) -> Self { - self.client_cert = Some(EapCertSource::Path(path.into())); + self.client_cert_blob = None; + self.client_cert_path = Some(path.into()); self } /// Sets the client certificate encoded as DER or PKCS#12. /// + /// Clears `client_cert_path` because they are mutually exclusive. + /// /// # Examples /// /// ```rust @@ -778,7 +805,8 @@ impl EapOptionsBuilder { /// ``` #[must_use] pub fn client_cert_blob(mut self, data: impl Into>) -> Self { - self.client_cert = Some(EapCertSource::Blob(data.into())); + self.client_cert_path = None; + self.client_cert_blob = Some(data.into()); self } @@ -807,16 +835,38 @@ impl EapOptionsBuilder { let is_peap_or_ttls = self.method == Some(EapMethod::Peap) || self.method == Some(EapMethod::Ttls); - if let (Some(EapMethod::Tls), None) = (&self.method, &self.private_key) { + if let (Some(_), Some(_)) = (&self.ca_cert_path, &self.ca_cert_blob) { return Err(ConnectionError::IncompleteBuilder( - "EAP private key is required for TLS (use .private_key_path() or .private_key_blob())".into(), + "EAP CA certificate cannot be specified both as a path and blob".into(), )); } - if let (Some(EapMethod::Tls), None) = (&self.method, &self.client_cert) { - return Err(ConnectionError::IncompleteBuilder( - "EAP client certificate is required for TLS (use .client_cert_path() or .client_cert_blob())".into(), - )); + match (&self.method, &self.private_key_path, &self.private_key_blob) { + (_, Some(_), Some(_)) => { + return Err(ConnectionError::IncompleteBuilder( + "EAP private key cannot be specified both as a path and blob".into(), + )); + } + (Some(EapMethod::Tls), None, None) => { + return Err(ConnectionError::IncompleteBuilder( + "EAP private key is required for TLS (use .private_key_path() or .private_key_blob())".into(), + )); + } + _ => {} + } + + match (&self.method, &self.client_cert_path, &self.client_cert_blob) { + (_, Some(_), Some(_)) => { + return Err(ConnectionError::IncompleteBuilder( + "EAP client certificate cannot be specified both as a path and blob".into(), + )); + } + (Some(EapMethod::Tls), None, None) => { + return Err(ConnectionError::IncompleteBuilder( + "EAP client certificate is required for TLS (use .client_cert_path() or .client_cert_blob())".into(), + )); + } + _ => {} } Ok(EapOptions { @@ -836,7 +886,8 @@ impl EapOptionsBuilder { }, anonymous_identity: self.anonymous_identity, domain_suffix_match: self.domain_suffix_match, - ca_cert: self.ca_cert, + ca_cert_path: self.ca_cert_path, + ca_cert_blob: self.ca_cert_blob, system_ca_certs: self.system_ca_certs, method: self.method.ok_or_else(|| { ConnectionError::IncompleteBuilder("EAP method is required (use .method())".into()) @@ -850,9 +901,11 @@ impl EapOptionsBuilder { } else { Phase2::Mschapv2 }, - private_key: self.private_key, + private_key_path: self.private_key_path, + private_key_blob: self.private_key_blob, private_key_password: self.private_key_password, - client_cert: self.client_cert, + client_cert_path: self.client_cert_path, + client_cert_blob: self.client_cert_blob, }) } } 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 6468c6a7..07473fe5 100644 --- a/nmrs/src/lib.rs +++ b/nmrs/src/lib.rs @@ -112,7 +112,7 @@ //! - [`Device`] / [`DeviceType`] / [`DeviceState`] — network devices and their state //! - [`Network`] / [`AccessPoint`] / [`NetworkInfo`] — discovered Wi-Fi data //! - [`WifiDevice`] — per-Wi-Fi-device summary -//! - [`WifiSecurity`] / [`EapOptions`] / [`EapCertSource`] / [`EapMethod`] / [`Phase2`] — Wi-Fi security +//! - [`WifiSecurity`] / [`EapOptions`] / [`EapMethod`] / [`Phase2`] — Wi-Fi security //! - [`ConnectionOptions`] / [`TimeoutConfig`] — connection knobs //! - [`WireGuardConfig`] / [`WireGuardPeer`] — WireGuard configuration //! - [`OpenVpnConfig`] / [`OpenVpnAuthType`] / [`OpenVpnProxy`] — OpenVPN configuration @@ -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 { @@ -434,7 +435,6 @@ pub mod models { } // Re-export commonly used types at crate root for convenience -pub use api::models::EapCertSource; #[allow(deprecated)] pub use api::models::{ AccessPoint, ActiveConnection, ActiveConnectionState, ActiveOtherConnection, diff --git a/nmrs/src/util/validation.rs b/nmrs/src/util/validation.rs index 76745acc..0167e849 100644 --- a/nmrs/src/util/validation.rs +++ b/nmrs/src/util/validation.rs @@ -9,7 +9,7 @@ use crate::api::models::{ ConnectionError, OpenVpnAuthType, OpenVpnConfig, OpenVpnProxy, VpnCredentials, WifiSecurity, WireGuardPeer, }; -use crate::{EapCertSource, EapMethod, EapOptions}; +use crate::{EapMethod, EapOptions}; /// Maximum SSID length in bytes (802.11 standard). const MAX_SSID_BYTES: usize = 32; @@ -194,13 +194,21 @@ fn validate_wifi_eap(opts: &EapOptions) -> Result<(), ConnectionError> { } } EapMethod::Tls => { - if !validate_cert_source("EAP private key", &opts.private_key)? { + if !validate_path_or_blob( + "EAP private key", + &opts.private_key_path, + &opts.private_key_blob, + )? { return Err(ConnectionError::InvalidAddress( "EAP private key must be provided".to_string(), )); } - if !validate_cert_source("EAP client certificate", &opts.client_cert)? { + if !validate_path_or_blob( + "EAP client certificate", + &opts.client_cert_path, + &opts.client_cert_blob, + )? { return Err(ConnectionError::InvalidAddress( "EAP client certificate must be provided".to_string(), )); @@ -208,18 +216,20 @@ fn validate_wifi_eap(opts: &EapOptions) -> Result<(), ConnectionError> { } } - validate_cert_source("EAP CA certificate", &opts.ca_cert)?; + validate_path_or_blob("EAP CA certificate", &opts.ca_cert_path, &opts.ca_cert_blob)?; Ok(()) } -fn validate_cert_source( +fn validate_path_or_blob( field: &str, - source: &Option, + path: &Option, + blob: &Option>, ) -> Result { - match source { - None => Ok(false), - Some(EapCertSource::Path(path)) => { + // Validate CA cert path if provided + match (path, blob) { + (None, None) => Ok(false), + (Some(path), None) => { if path.trim().is_empty() { return Err(ConnectionError::InvalidAddress(format!( "{field} path cannot be empty if provided" @@ -233,7 +243,10 @@ fn validate_cert_source( } Ok(true) } - Some(EapCertSource::Blob(_)) => Ok(true), + (None, Some(_)) => Ok(true), + (Some(_), Some(_)) => Err(ConnectionError::InvalidAddress(format!( + "{field} path and blob cannot be provided at the same time" + ))), } } @@ -970,13 +983,16 @@ mod tests { password: "password".to_string(), anonymous_identity: None, domain_suffix_match: Some("example.com".to_string()), - ca_cert: Some(EapCertSource::Path("file:///etc/ssl/cert.pem".to_string())), + ca_cert_path: Some("file:///etc/ssl/cert.pem".to_string()), + ca_cert_blob: None, system_ca_certs: false, method: EapMethod::Peap, phase2: Phase2::Mschapv2, - private_key: None, + private_key_path: None, + private_key_blob: None, private_key_password: None, - client_cert: None, + client_cert_path: None, + client_cert_blob: None, }, }; assert!(validate_wifi_security(&eap).is_ok()); @@ -990,13 +1006,16 @@ mod tests { password: "password".to_string(), anonymous_identity: None, domain_suffix_match: None, - ca_cert: None, + ca_cert_path: None, + ca_cert_blob: None, system_ca_certs: true, method: EapMethod::Peap, phase2: Phase2::Mschapv2, - private_key: None, + private_key_path: None, + private_key_blob: None, private_key_password: None, - client_cert: None, + client_cert_path: None, + client_cert_blob: None, }, }; assert_error_message!( @@ -1014,13 +1033,16 @@ mod tests { password: "password".to_string(), anonymous_identity: None, domain_suffix_match: None, - ca_cert: Some(EapCertSource::Path("/etc/ssl/cert.pem".to_string())), // Missing file:// + ca_cert_path: Some("/etc/ssl/cert.pem".to_string()), // Missing file:// + ca_cert_blob: None, system_ca_certs: false, method: EapMethod::Peap, phase2: Phase2::Mschapv2, - private_key: None, + private_key_path: None, + private_key_blob: None, private_key_password: None, - client_cert: None, + client_cert_path: None, + client_cert_blob: None, }, }; assert_error_message!( @@ -1038,13 +1060,16 @@ mod tests { password: "password".to_string(), anonymous_identity: None, domain_suffix_match: None, - ca_cert: None, + ca_cert_path: None, + ca_cert_blob: None, system_ca_certs: true, method: EapMethod::Peap, phase2: Phase2::Mschapv2, - private_key: None, + private_key_path: None, + private_key_blob: None, private_key_password: None, - client_cert: None, + client_cert_path: None, + client_cert_blob: None, }, }; assert_error_message!( @@ -1062,19 +1087,16 @@ mod tests { password: String::new(), anonymous_identity: None, domain_suffix_match: Some("example.com".to_string()), - ca_cert: Some(EapCertSource::Path( - "file:///etc/ssl/certs/ca.pem".to_string(), - )), + ca_cert_path: Some("file:///etc/ssl/certs/ca.pem".to_string()), + ca_cert_blob: None, system_ca_certs: false, method: EapMethod::Tls, phase2: Phase2::Mschapv2, - private_key: Some(EapCertSource::Path( - "file:///etc/ssl/private/client.pem".to_string(), - )), + private_key_path: Some("file:///etc/ssl/private/client.pem".to_string()), + private_key_blob: None, private_key_password: None, - client_cert: Some(EapCertSource::Path( - "file:///etc/ssl/certs/client.pem".to_string(), - )), + client_cert_path: Some("file:///etc/ssl/certs/client.pem".to_string()), + client_cert_blob: None, }, }; assert!(validate_wifi_security(&eap).is_ok()); @@ -1088,18 +1110,48 @@ mod tests { password: String::new(), anonymous_identity: None, domain_suffix_match: Some("example.com".to_string()), - ca_cert: Some(EapCertSource::Blob(b"ca_cert_blob".to_vec())), + ca_cert_path: None, + ca_cert_blob: Some(b"ca_cert_blob".to_vec()), system_ca_certs: false, method: EapMethod::Tls, phase2: Phase2::Mschapv2, - private_key: Some(EapCertSource::Blob(b"private_key_blob".to_vec())), + private_key_path: None, + private_key_blob: Some(b"private_key_blob".to_vec()), private_key_password: None, - client_cert: Some(EapCertSource::Blob(b"client_cert_blob".to_vec())), + client_cert_path: None, + client_cert_blob: Some(b"client_cert_blob".to_vec()), }, }; assert!(validate_wifi_security(&eap).is_ok()); } + #[test] + fn test_validate_wifi_security_eap_192bit_path_blob() { + let eap = WifiSecurity::Wpa3Eap192bit { + opts: EapOptions { + identity: "user@example.com".to_string(), + password: String::new(), + anonymous_identity: None, + domain_suffix_match: Some("example.com".to_string()), + ca_cert_path: Some("file:///etc/ssl/certs/ca.pem".to_string()), + ca_cert_blob: Some(b"ca_cert_blob".to_vec()), + system_ca_certs: false, + method: EapMethod::Tls, + phase2: Phase2::Mschapv2, + private_key_path: Some("file:///etc/ssl/private/client.pem".to_string()), + private_key_blob: Some(b"private_key_blob".to_vec()), + private_key_password: None, + client_cert_path: Some("file:///etc/ssl/certs/client.pem".to_string()), + client_cert_blob: Some(b"client_cert_blob".to_vec()), + }, + }; + assert_error_message!( + validate_wifi_security(&eap), + InvalidAddress, + "EAP private key path and blob cannot be provided at the same time" + ); + } + #[test] fn test_validate_wifi_security_eap_tls_invalid_private_key() { let eap = WifiSecurity::WpaEap { @@ -1108,19 +1160,16 @@ mod tests { password: String::new(), anonymous_identity: None, domain_suffix_match: Some("example.com".to_string()), - ca_cert: Some(EapCertSource::Path( - "file:///etc/ssl/certs/ca.pem".to_string(), - )), + ca_cert_path: Some("file:///etc/ssl/certs/ca.pem".to_string()), + ca_cert_blob: None, system_ca_certs: false, method: EapMethod::Tls, phase2: Phase2::Mschapv2, - private_key: Some(EapCertSource::Path( - "/etc/ssl/private/client.pem".to_string(), - )), + private_key_path: Some("/etc/ssl/private/client.pem".to_string()), + private_key_blob: None, private_key_password: None, - client_cert: Some(EapCertSource::Path( - "file:///etc/ssl/certs/client.pem".to_string(), - )), + client_cert_path: Some("file:///etc/ssl/certs/client.pem".to_string()), + client_cert_blob: None, }, }; assert_error_message!( @@ -1138,17 +1187,16 @@ mod tests { password: String::new(), anonymous_identity: None, domain_suffix_match: Some("example.com".to_string()), - ca_cert: Some(EapCertSource::Path( - "file:///etc/ssl/certs/ca.pem".to_string(), - )), + ca_cert_path: Some("file:///etc/ssl/certs/ca.pem".to_string()), + ca_cert_blob: None, system_ca_certs: false, method: EapMethod::Tls, phase2: Phase2::Mschapv2, - private_key: Some(EapCertSource::Path( - "file:///etc/ssl/private/client.pem".to_string(), - )), + private_key_path: Some("file:///etc/ssl/private/client.pem".to_string()), + private_key_blob: None, private_key_password: None, - client_cert: Some(EapCertSource::Path("/etc/ssl/certs/client.pem".to_string())), + client_cert_path: Some("/etc/ssl/certs/client.pem".to_string()), + client_cert_blob: None, }, }; assert_error_message!( @@ -1166,17 +1214,16 @@ mod tests { password: String::new(), anonymous_identity: None, domain_suffix_match: Some("example.com".to_string()), - ca_cert: Some(EapCertSource::Path( - "file:///etc/ssl/certs/ca.pem".to_string(), - )), + ca_cert_path: Some("file:///etc/ssl/certs/ca.pem".to_string()), + ca_cert_blob: None, system_ca_certs: false, method: EapMethod::Tls, phase2: Phase2::Mschapv2, - private_key: None, + private_key_path: None, + private_key_blob: None, private_key_password: None, - client_cert: Some(EapCertSource::Path( - "file:///etc/ssl/certs/client.pem".to_string(), - )), + client_cert_path: Some("file:///etc/ssl/certs/client.pem".to_string()), + client_cert_blob: None, }, }; assert_error_message!( @@ -1194,17 +1241,16 @@ mod tests { password: String::new(), anonymous_identity: None, domain_suffix_match: Some("example.com".to_string()), - ca_cert: Some(EapCertSource::Path( - "file:///etc/ssl/certs/ca.pem".to_string(), - )), + ca_cert_path: Some("file:///etc/ssl/certs/ca.pem".to_string()), + ca_cert_blob: None, system_ca_certs: false, method: EapMethod::Tls, phase2: Phase2::Mschapv2, - private_key: Some(EapCertSource::Path( - "file:///etc/ssl/private/client.pem".to_string(), - )), + private_key_path: Some("file:///etc/ssl/private/client.pem".to_string()), + private_key_blob: None, private_key_password: None, - client_cert: None, + client_cert_path: None, + client_cert_blob: None, }, }; assert_error_message!(