diff --git a/Cargo.lock b/Cargo.lock index ce47a4a..6c11fe1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5042,9 +5042,9 @@ checksum = "650eef8c711430f1a879fdd01d4745a7deea475becfb90269c06775983bbf086" [[package]] name = "nmrs" -version = "3.4.1" +version = "3.4.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8c58e7c48ffdccfa5164000de751cbdf5c9cb2d79a33e75b9e2618ab72ea86f7" +checksum = "33e42c05118e18f29f55eefa2fbfd98a06b0c7a1a829bdb0f328137d358c6013" dependencies = [ "async-trait", "base64", diff --git a/cosmic-settings/Cargo.toml b/cosmic-settings/Cargo.toml index 93683e2..226cbf0 100644 --- a/cosmic-settings/Cargo.toml +++ b/cosmic-settings/Cargo.toml @@ -54,7 +54,7 @@ locale1 = { git = "https://github.com/pop-os/dbus-settings-bindings", optional = sysinfo = { version = "=0.38.0", optional = true } mime-apps = { package = "cosmic-mime-apps", git = "https://github.com/pop-os/cosmic-mime-apps", features = ["tokio"], optional = true } notify = "8.2.0" -nmrs = { version = "3.4.1", optional = true } +nmrs = { version = "3.4.2", optional = true } regex = "1.13.1" ron = "0.12" rust-embed = "8.12.0" diff --git a/cosmic-settings/src/pages/networking/backend.rs b/cosmic-settings/src/pages/networking/backend.rs index eec2e4e..db0e3be 100644 --- a/cosmic-settings/src/pages/networking/backend.rs +++ b/cosmic-settings/src/pages/networking/backend.rs @@ -12,7 +12,7 @@ use futures::channel::mpsc::{UnboundedReceiver, UnboundedSender, unbounded}; use futures::{FutureExt, SinkExt, StreamExt}; use nmrs::agent::{SecretAgent, SecretAgentFlags, SecretRequest, SecretResponder, SecretSetting}; use nmrs::raw::zbus; -use nmrs::raw::zvariant::{OwnedObjectPath, OwnedValue, Str, Value}; +use nmrs::raw::zvariant::{Dict, OwnedObjectPath, OwnedValue, Str, Value}; use nmrs::{ ActiveConnection, ConnectType, ConnectionOptions, EapOptions, NetworkEvent, NetworkManager, SavedConnection, SettingsSummary, WifiKeyMgmt, WifiSecurity, @@ -514,6 +514,10 @@ pub enum Request { Option, ), SetAirplaneMode(bool), + SetVpnUsername { + uuid: UUID, + username: String, + }, SetWiFi(bool), } @@ -720,12 +724,64 @@ async fn handle_request(nm: &NetworkManager, req: Request) -> Event { result.is_ok() } Request::SetAirplaneMode(enabled) => nm.set_airplane_mode(*enabled).await.is_ok(), + Request::SetVpnUsername { uuid, username } => { + let result = set_vpn_username(nm, uuid, username).await; + if let Err(err) = &result { + tracing::error!(%err, %uuid, "failed to store VPN username"); + } + result.is_ok() + } Request::SetWiFi(enabled) => nm.set_wireless_enabled(*enabled).await.is_ok(), }; request_response(nm, req, success).await } +/// Stores a VPN username in `vpn.data`, which is where NetworkManager and the VPN +/// plugins read it from. +/// +/// The username must not be sent through the secret agent: openvpn fails to +/// reconnect when it receives a `username` key in the VPN secrets dictionary. +async fn set_vpn_username( + nm: &NetworkManager, + uuid: &str, + username: &str, +) -> Result<(), nmrs::ConnectionError> { + let settings = nm.get_saved_connection_raw(uuid).await?; + + // The whole dict is rewritten, so the keys already in it have to be carried over. + let mut data = HashMap::::new(); + if let Some(stored) = settings.get("vpn").and_then(|vpn| vpn.get("data")) + && let Ok(dict) = Dict::try_from(stored.clone()) + { + for (key, value) in dict.iter() { + if let (Ok(key), Ok(value)) = (Str::try_from(key.clone()), Str::try_from(value.clone())) + { + data.insert(key.to_string(), value.to_string()); + } + } + } + + if data + .get("username") + .is_some_and(|stored| stored == username) + { + return Ok(()); + } + + data.insert("username".to_string(), username.to_string()); + let data = OwnedValue::try_from(Value::from(data)) + .map_err(|err| nmrs::ConnectionError::VpnFailed(err.to_string()))?; + + let mut patch = nmrs::SettingsPatch::default(); + patch.raw_overlay = Some(HashMap::from([( + "vpn".to_string(), + HashMap::from([("data".to_string(), data)]), + )])); + + nm.update_saved_connection(uuid, patch).await +} + async fn request_response(nm: &NetworkManager, req: Request, success: bool) -> Event { Event::RequestResponse { req, @@ -1649,15 +1705,28 @@ pub mod nm_secret_agent { } } SecretSetting::Vpn { .. } => { - let secrets = stored + // Earlier versions stored the username next to the password. openvpn + // fails to reconnect when one is present in the secrets dictionary, so + // profiles saved by those versions have to be filtered on read too. + let secrets: HashMap = stored .into_iter() + .filter(|(key, _)| key != "username") .map(|(key, value)| (key, value.unsecure().to_string())) .collect(); - request - .responder - .vpn_secrets(secrets) - .await - .map_err(|err| Error(err.to_string()))?; + + if secrets.is_empty() { + request + .responder + .no_secrets() + .await + .map_err(|err| Error(err.to_string()))?; + } else { + request + .responder + .vpn_secrets(secrets) + .await + .map_err(|err| Error(err.to_string()))?; + } } _ => { let setting_name = setting_name(&request.setting).to_string(); diff --git a/cosmic-settings/src/pages/networking/vpn/mod.rs b/cosmic-settings/src/pages/networking/vpn/mod.rs index 6c2c9e7..7323d84 100644 --- a/cosmic-settings/src/pages/networking/vpn/mod.rs +++ b/cosmic-settings/src/pages/networking/vpn/mod.rs @@ -612,7 +612,7 @@ impl Page { } = dialog && let Some(NmState { ref sender, .. }) = self.nm_state { - let username_unwrapped = username.clone().unwrap_or_default(); + let username = username.clone().unwrap_or_default(); let sec_tx = self.secret_tx.clone(); let nm_sender = sender.clone(); return Task::future(async move { @@ -626,10 +626,10 @@ impl Page { .send(nm_secret_agent::Request::SetSecrets { setting_name: "vpn".to_string(), uuid: uuid.to_string(), - secrets: HashMap::from_iter([ - ("username".to_string(), username_unwrapped.into()), - ("password".to_string(), password), - ]), + secrets: HashMap::from_iter([( + "password".to_string(), + password, + )]), applied_tx, }) .await @@ -645,6 +645,16 @@ impl Page { tracing::error!(%err, "failed to apply secret"); } } + + if !username.is_empty() { + _ = nm_sender.unbounded_send( + network_manager::Request::SetVpnUsername { + uuid: uuid.clone(), + username, + }, + ); + } + _ = nm_sender .unbounded_send(network_manager::Request::ActivateVpn(uuid)); } @@ -668,7 +678,7 @@ impl Page { } = dialog && let Some(NmState { ref sender, .. }) = self.nm_state { - let username_unwrapped = username.unwrap_or_default(); + let username = username.unwrap_or_default(); let sec_tx = self.secret_tx.clone(); let nm_sender = sender.clone(); return Task::future(async move { @@ -678,10 +688,10 @@ impl Page { .send(nm_secret_agent::Request::SetSecrets { setting_name: "vpn".to_string(), uuid: uuid.to_string(), - secrets: HashMap::from_iter([ - ("username".to_string(), username_unwrapped.into()), - ("password".to_string(), password), - ]), + secrets: HashMap::from_iter([( + "password".to_string(), + password, + )]), applied_tx, }) .await; @@ -689,6 +699,16 @@ impl Page { tokio::time::timeout(std::time::Duration::from_secs(1), applied_rx) .await; } + + if !username.is_empty() { + _ = nm_sender.unbounded_send( + network_manager::Request::SetVpnUsername { + uuid: uuid.clone(), + username, + }, + ); + } + _ = nm_sender.unbounded_send(network_manager::Request::ActivateVpn(uuid)); Message::Refresh })