From f48bcde63bb54134e21246ca0edb5bbefc453c3e Mon Sep 17 00:00:00 2001 From: Ashley Wulber Date: Wed, 28 Jan 2026 20:35:01 -0500 Subject: [PATCH] improv: network authentication --- cosmic-settings/src/pages/networking/wifi.rs | 23 ++------- subscriptions/network-manager/src/lib.rs | 49 ++++++------------- .../network-manager/src/nm_secret_agent.rs | 14 ++++-- 3 files changed, 31 insertions(+), 55 deletions(-) diff --git a/cosmic-settings/src/pages/networking/wifi.rs b/cosmic-settings/src/pages/networking/wifi.rs index 7a8a09a..e7fe4a3 100644 --- a/cosmic-settings/src/pages/networking/wifi.rs +++ b/cosmic-settings/src/pages/networking/wifi.rs @@ -20,7 +20,6 @@ use cosmic_settings_network_manager_subscription::{ self as network_manager, NetworkManagerState, available_wifi::{AccessPoint, NetworkType}, current_networks::ActiveConnectionInfo, - hw_address::HwAddress, nm_secret_agent, }; use cosmic_settings_page::{self as page, Section, section}; @@ -103,7 +102,6 @@ enum WiFiDialog { Forget(network_manager::SSID), Password { ssid: network_manager::SSID, - hw_address: HwAddress, identity: Option, password: SecureString, password_hidden: bool, @@ -353,12 +351,7 @@ impl Page { } match req { - network_manager::Request::Authenticate { - ssid, - identity, - hw_address, - .. - } => { + network_manager::Request::Authenticate { ssid, identity, .. } => { if success { self.connecting.remove(ssid.as_str()); } else { @@ -366,7 +359,6 @@ impl Page { self.dialog = Some(WiFiDialog::Password { ssid: ssid.into(), identity, - hw_address, password: SecureString::from(""), password_hidden: true, tx: Arc::new(Mutex::new(None)), @@ -377,9 +369,9 @@ impl Page { network_manager::Request::SelectAccessPoint( ssid, - hw_address, network_type, _tx, + interface, ) => { if success || matches!(network_type, NetworkType::Open) { self.connecting.remove(ssid.as_ref()); @@ -388,7 +380,6 @@ impl Page { ssid, identity: matches!(network_type, NetworkType::EAP) .then(String::new), - hw_address, password: SecureString::from(""), password_hidden: true, tx: Arc::new(Mutex::new(None)), @@ -499,9 +490,9 @@ impl Page { .sender .unbounded_send(network_manager::Request::SelectAccessPoint( ssid, - ap.hw_address, ap.network_type, self.secret_tx.clone(), + self.active_device.as_ref().map(|d| d.interface.clone()), )); } } @@ -528,7 +519,6 @@ impl Page { self.dialog = Some(WiFiDialog::Password { ssid, identity: matches!(ap.network_type, NetworkType::EAP).then(String::new), - hw_address: ap.hw_address, password: SecureString::from(""), password_hidden: true, tx: Arc::new(Mutex::new(None)), @@ -553,7 +543,6 @@ impl Page { ssid, identity, password, - hw_address, tx, .. } = dialog @@ -562,6 +551,7 @@ impl Page { self.connecting.insert(ssid.clone()); let nm_sender = nm.sender.clone(); let secret_tx = self.secret_tx.clone(); + let interface = self.active_device.as_ref().map(|d| d.interface.clone()); return Task::future(async move { let mut guard = tx.lock().await; if let Some(tx) = guard.take() { @@ -570,9 +560,9 @@ impl Page { _ = nm_sender.unbounded_send(network_manager::Request::Authenticate { ssid: ssid.to_string(), identity, - hw_address, password, secret_tx, + interface, }); } }) @@ -718,7 +708,6 @@ impl Page { ssid, password: previous, password_hidden: true, - hw_address: ap.hw_address, identity: matches!(ap.network_type, NetworkType::EAP).then(String::new), tx, }); @@ -736,7 +725,6 @@ impl Page { ssid, password, identity, - hw_address, .. }) = self.dialog.take() { @@ -746,7 +734,6 @@ impl Page { tx: Arc::new(Mutex::new(None)), ssid, identity, - hw_address, }); return task::message(Message::FocusSecureInput); } diff --git a/subscriptions/network-manager/src/lib.rs b/subscriptions/network-manager/src/lib.rs index 21fb8c9..33165b6 100644 --- a/subscriptions/network-manager/src/lib.rs +++ b/subscriptions/network-manager/src/lib.rs @@ -29,7 +29,6 @@ use futures::{ FutureExt, SinkExt, StreamExt, channel::mpsc::{UnboundedReceiver, UnboundedSender, unbounded}, }; -use hw_address::HwAddress; use iced_futures::{Subscription, stream}; use secure_string::SecureString; use tokio::process::Command; @@ -45,8 +44,6 @@ pub type UUID = Arc; #[derive(thiserror::Error, Debug)] pub enum Error { - #[error("access point not found")] - AccessPointNotFound, #[error("failed to list bluetooth devices with rfkill")] BluetoothRfkillList(std::io::Error), #[error("failed to activate connection")] @@ -299,8 +296,8 @@ async fn start_listening( ssid, identity, password, - hw_address, secret_tx, + interface, }) => { let nm_state = NetworkManagerState::new(&conn).await.unwrap_or_default(); @@ -310,13 +307,13 @@ async fn start_listening( &ssid, identity.as_deref(), Some(password.unsecure()), - hw_address, secret_tx.clone(), if identity.is_some() { NetworkType::EAP } else { NetworkType::PSK }, + interface.clone(), ) .await .is_ok(); @@ -327,8 +324,8 @@ async fn start_listening( ssid: ssid.clone(), identity: identity.clone(), password: password.clone(), - hw_address, secret_tx: secret_tx.clone(), + interface, }, success, state: NetworkManagerState::new(&conn).await.unwrap_or_default(), @@ -336,17 +333,10 @@ async fn start_listening( .await; } - Some(Request::SelectAccessPoint(ssid, hw_address, network_type, secret_tx)) => { + Some(Request::SelectAccessPoint(ssid, network_type, secret_tx, interface)) => { if matches!(network_type, NetworkType::Open) { - attempt_wifi_connection( - &conn, - ssid, - hw_address, - network_type, - output, - None, - ) - .await; + attempt_wifi_connection(&conn, ssid, network_type, output, None, interface) + .await; } else { // For secured networks, check if we have saved credentials let has_saved = has_saved_wifi_credentials(&conn, &ssid).await; @@ -359,10 +349,10 @@ async fn start_listening( attempt_wifi_connection( &conn, ssid, - hw_address, network_type, output, secret_tx, + interface, ) .await; } @@ -640,10 +630,10 @@ async fn has_saved_wifi_credentials(conn: &zbus::Connection, ssid: &str) -> bool async fn attempt_wifi_connection( conn: &zbus::Connection, ssid: SSID, - hw_address: HwAddress, network_type: NetworkType, output: &mut futures::channel::mpsc::Sender, secret_tx: Option>, + interface: Option, ) { let state = NetworkManagerState::new(conn).await.unwrap_or_default(); let success = if let Err(err) = state @@ -652,9 +642,9 @@ async fn attempt_wifi_connection( ssid.as_ref(), None, None, - hw_address, secret_tx, network_type, + interface.clone(), ) .await { @@ -666,7 +656,7 @@ async fn attempt_wifi_connection( _ = request_response( conn, - Request::SelectAccessPoint(ssid, hw_address, network_type, None), + Request::SelectAccessPoint(ssid, network_type, None, interface), success, ) .then(|event| output.send(event)) @@ -688,8 +678,8 @@ pub enum Request { ssid: String, identity: Option, password: SecureString, - hw_address: HwAddress, secret_tx: Option>, + interface: Option, }, /// Get WiFi credentials for a known access point. GetWiFiCredentials( @@ -705,9 +695,9 @@ pub enum Request { /// Connect to a known access point. SelectAccessPoint( SSID, - HwAddress, NetworkType, Option>, + Option, ), /// Toggle airplaine mode. SetAirplaneMode(bool), @@ -907,9 +897,9 @@ impl NetworkManagerState { ssid: &str, identity: Option<&str>, password: Option<&str>, - hw_address: HwAddress, mut secret_tx: Option>, network_type: NetworkType, + interface: Option, ) -> Result<(), Error> { secret_tx = secret_tx.filter(|tx| !tx.is_closed()); let nm = NetworkManager::new(conn).await?; @@ -925,14 +915,6 @@ impl NetworkManagerState { } } - let Some(ap) = self - .wireless_access_points - .iter() - .find(|ap| ap.ssid.as_ref() == ssid && ap.hw_address == hw_address) - else { - return Err(Error::AccessPointNotFound); - }; - let mut conn_settings: HashMap<&str, HashMap<&str, zvariant::Value>> = HashMap::from([ ( "802-11-wireless", @@ -984,7 +966,8 @@ impl NetworkManagerState { if !matches!( device.device_type().await.unwrap_or(DeviceType::Other), DeviceType::Wifi - ) { + ) || (interface.is_some() && interface != device.interface().await.ok()) + { continue; } @@ -1010,7 +993,7 @@ impl NetworkManagerState { let known_conn = if let Some(known_conn) = known_conn { if secret_tx.is_none() || identity.is_some() { - known_conn.update(conn_settings).await?; + known_conn.update(conn_settings).await.unwrap(); } known_conn } else { diff --git a/subscriptions/network-manager/src/nm_secret_agent.rs b/subscriptions/network-manager/src/nm_secret_agent.rs index f7ee0a7..f67b892 100644 --- a/subscriptions/network-manager/src/nm_secret_agent.rs +++ b/subscriptions/network-manager/src/nm_secret_agent.rs @@ -147,6 +147,16 @@ async fn secret_agent_stream_impl( msg_tx: futures::channel::mpsc::Sender, mut rx: tokio::sync::mpsc::Receiver, ) -> Result<(), Error> { + // fail early if we can't connect, closing the channel + { + let ss = secret_service::SecretService::connect(secret_service::EncryptionType::Dh) + .await + .map_err(|e| Arc::new(e))?; + let collection = ss.get_default_collection().await.map_err(|e| Arc::new(e))?; + if collection.is_locked().await.map_err(|e| Arc::new(e))? { + _ = collection.unlock().await.map_err(|e| Arc::new(e))?; + } + } // register the secret agent with NetworkManager let proxy = nm_secret_agent_manager::AgentManagerProxy::builder(&zbus::Connection::system().await?) @@ -162,10 +172,6 @@ async fn secret_agent_stream_impl( .await?; proxy.register_with_capabilities(identifier, 1).await?; - // fail early if we can't connect, closing the channel - let _ = secret_service::SecretService::connect(secret_service::EncryptionType::Dh) - .await - .map_err(|e| Arc::new(e))?; while let Some(request) = rx.recv().await { match request {