From 828f18115bad666e311e751ba0910904d955960f Mon Sep 17 00:00:00 2001 From: Michael Aaron Murphy Date: Tue, 8 Oct 2024 17:14:12 +0200 Subject: [PATCH] fix(vpn): improve support for importing WireGuard configs --- cosmic-settings/src/pages/networking/mod.rs | 20 +- .../src/pages/networking/vpn/mod.rs | 239 +++++++++++++++--- .../src/pages/networking/vpn/nmcli.rs | 32 ++- cosmic-settings/src/pages/networking/wired.rs | 1 - cosmic-settings/src/utils.rs | 13 +- i18n/en/cosmic_settings.ftl | 21 ++ 6 files changed, 260 insertions(+), 66 deletions(-) diff --git a/cosmic-settings/src/pages/networking/mod.rs b/cosmic-settings/src/pages/networking/mod.rs index 8714793d..e088bcd6 100644 --- a/cosmic-settings/src/pages/networking/mod.rs +++ b/cosmic-settings/src/pages/networking/mod.rs @@ -5,7 +5,7 @@ pub mod vpn; pub mod wifi; pub mod wired; -use std::{ffi::OsStr, io, process::ExitStatus, sync::Arc}; +use std::{ffi::OsStr, process::Stdio, sync::Arc}; use anyhow::Context; use cosmic::{widget, Apply, Command, Element}; @@ -354,29 +354,33 @@ impl Page { } } -async fn nm_add_vpn_file>(type_: &str, path: P) -> io::Result { +async fn nm_add_vpn_file>(type_: &str, path: P) -> Result<(), String> { tokio::process::Command::new("nmcli") .args(["connection", "import", "type", type_, "file"]) .arg(path) - .status() + .stderr(Stdio::piped()) + .output() .await + .apply(crate::utils::map_stderr_output) } -async fn nm_add_wired() -> io::Result { +async fn nm_add_wired() -> Result<(), String> { nm_connection_editor(&["--type=802-3-ethernet", "-c"]).await } -async fn nm_add_wifi() -> io::Result { +async fn nm_add_wifi() -> Result<(), String> { nm_connection_editor(&["--type=802-11-wireless", "-c"]).await } -async fn nm_edit_connection(uuid: &str) -> io::Result { +async fn nm_edit_connection(uuid: &str) -> Result<(), String> { nm_connection_editor(&[&["--edit=", uuid].concat()]).await } -async fn nm_connection_editor(args: &[&str]) -> io::Result { +async fn nm_connection_editor(args: &[&str]) -> Result<(), String> { tokio::process::Command::new(NM_CONNECTION_EDITOR) .args(args) - .status() + .stderr(Stdio::piped()) + .output() .await + .apply(crate::utils::map_stderr_output) } diff --git a/cosmic-settings/src/pages/networking/vpn/mod.rs b/cosmic-settings/src/pages/networking/vpn/mod.rs index 38b3d37b..17dc755a 100644 --- a/cosmic-settings/src/pages/networking/vpn/mod.rs +++ b/cosmic-settings/src/pages/networking/vpn/mod.rs @@ -31,6 +31,8 @@ pub enum Message { Activate(ConnectionId), /// Add a network connection AddNetwork, + /// Show a dialog requesting a name for the WireGuard device + AddWireGuardDevice(String, String, String), /// Cancels an active dialog. CancelDialog, /// Connect to a VPN with the given username and password @@ -38,9 +40,9 @@ pub enum Message { /// Deactivate a connection. Deactivate(ConnectionId), /// An error occurred. - Error(String), + Error(ErrorKind, String), /// Update the list of known connections. - KnownConnections(IndexMap), + KnownConnections(IndexMap), /// An update from the network manager daemon NetworkManager(network_manager::Event), /// Successfully connected to the system dbus. @@ -70,6 +72,39 @@ pub enum Message { UsernameUpdate(String), /// Display more options for an access point ViewMore(Option), + /// Create a new wireguard connection + WireGuardConfig, + /// Update the text input for the wireguard device name + WireGuardDeviceInput(String), +} + +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub enum ErrorKind { + Config, + Connect, + ConnectionEditor, + ConnectionSettings, + DbusConnection, + UpdatingState, + WireGuardConfigPath, + WireGuardDevice, + WithPassword(&'static str), +} + +impl ErrorKind { + pub fn localized(self) -> String { + match self { + ErrorKind::Config => fl!("vpn-error", "config"), + ErrorKind::Connect => fl!("vpn-error", "connect"), + ErrorKind::ConnectionEditor => fl!("vpn-error", "connection-editor"), + ErrorKind::ConnectionSettings => fl!("vpn-error", "connection-settings"), + ErrorKind::DbusConnection => fl!("dbus-connection-error"), + ErrorKind::UpdatingState => fl!("vpn-error", "updating-state"), + ErrorKind::WireGuardConfigPath => fl!("vpn-error", "wireguard-config-path"), + ErrorKind::WireGuardDevice => fl!("vpn-error", "wireguard-device"), + ErrorKind::WithPassword(field) => fl!("vpn-error", "with-password", field = field), + } + } } impl From for crate::app::Message { @@ -84,6 +119,12 @@ impl From for crate::pages::Message { } } +#[derive(Clone, Debug)] +pub enum ConnectionSettings { + Vpn(VpnConnectionSettings), + Wireguard { id: String }, +} + #[derive(Clone, Debug, Default)] pub struct VpnConnectionSettings { id: String, @@ -127,6 +168,7 @@ enum PasswordFlag { #[derive(Clone, Debug, Eq, PartialEq)] enum VpnDialog { + Error(ErrorKind, String), Password { id: String, uuid: Arc, @@ -135,6 +177,7 @@ enum VpnDialog { password_hidden: bool, }, RemoveProfile(ConnectionId), + WireGuardName(String, String, String), } #[derive(Debug)] @@ -151,7 +194,8 @@ pub struct Page { nm_state: Option, dialog: Option, view_more_popup: Option, - known_connections: IndexMap, + known_connections: IndexMap, + wireguard_connections: IndexMap, /// Withhold device update if the view more popup is shown. withheld_devices: Option>, /// Withhold active connections update if the view more popup is shown. @@ -176,6 +220,21 @@ impl page::Page for Page { fn dialog(&self) -> Option> { self.dialog.as_ref().map(|dialog| match dialog { + VpnDialog::Error(error_kind, message) => { + let reason = widget::text::body(message.as_str()).wrap(Wrap::Word); + + let primary_action = + widget::button::standard(fl!("ok")).on_press(Message::CancelDialog); + + widget::dialog(fl!("vpn-error")) + .icon(icon::from_name("dialog-error-symbolic").size(64)) + .body(error_kind.localized()) + .control(reason) + .primary_action(primary_action) + .apply(Element::from) + .map(crate::pages::Message::Vpn) + } + VpnDialog::Password { username, password, @@ -216,6 +275,27 @@ impl page::Page for Page { .map(crate::pages::Message::Vpn) } + VpnDialog::WireGuardName(device, ..) => { + let input = widget::text_input("", device.as_str()).on_input(|input| { + Message::WireGuardDeviceInput(input.replace(|c: char| !c.is_alphanumeric(), "")) + }); + + let primary_action = + widget::button::suggested(fl!("connect")).on_press(Message::WireGuardConfig); + + let secondary_action = + widget::button::standard(fl!("cancel")).on_press(Message::CancelDialog); + + widget::dialog(fl!("wireguard-dialog")) + .icon(icon::from_name("network-vpn-symbolic").size(64)) + .body(fl!("wireguard-dialog", "description")) + .control(input) + .primary_action(primary_action) + .secondary_action(secondary_action) + .apply(Element::from) + .map(crate::pages::Message::Vpn) + } + VpnDialog::RemoveProfile(uuid) => { let primary_action = widget::button::destructive(fl!("remove")) .on_press(Message::RemoveProfile(uuid.clone())); @@ -258,7 +338,7 @@ impl page::Page for Page { .await .context("failed to create system dbus connection") .map_or_else( - |why| Message::Error(why.to_string()), + |why| Message::Error(ErrorKind::DbusConnection, why.to_string()), |conn| Message::NetworkManagerConnect((conn, sender.clone())), ) }); @@ -357,10 +437,50 @@ impl Page { Message::AddNetwork => return add_network(), + Message::AddWireGuardDevice(device, filename, path) => { + self.dialog = Some(VpnDialog::WireGuardName(device, filename, path)); + } + + Message::WireGuardDeviceInput(input) => { + if let Some(VpnDialog::WireGuardName(ref mut device, ..)) = self.dialog { + *device = input + } + } + + Message::WireGuardConfig => { + if let Some(VpnDialog::WireGuardName(device, filename, path)) = self.dialog.take() { + return cosmic::command::future(async move { + let new_path = path.replace(&filename, &device); + _ = std::fs::rename(&path, &new_path); + match super::nm_add_vpn_file("wireguard", new_path).await { + Ok(_) => Message::Refresh, + Err(why) => Message::Error(ErrorKind::Config, why.to_string()), + } + }); + } + } + Message::Activate(uuid) => { self.close_popup_and_apply_updates(); if let Some(settings) = self.known_connections.get(&uuid) { + let settings = match settings { + ConnectionSettings::Vpn(ref settings) => settings, + ConnectionSettings::Wireguard { id } => { + let connection_name = id.clone(); + return cosmic::command::future(async move { + if let Err(why) = nmcli::connect(&connection_name).await { + return Message::Error( + ErrorKind::Connect, + format!("failed to connect to WireGuard VPN: {why}"), + ); + } + + Message::Refresh + }); + } + }; + match settings.password_flag() { Some(PasswordFlag::NotSaved | PasswordFlag::AgentOwned) => { self.view_more_popup = None; @@ -377,9 +497,10 @@ impl Page { let connection_name = settings.id.clone(); return cosmic::command::future(async move { if let Err(why) = nmcli::connect(&connection_name).await { - return Message::Error(format!( - "failed to connect to VPN: {why}" - )); + return Message::Error( + ErrorKind::Connect, + format!("failed to connect to VPN: {why}"), + ); } Message::Refresh @@ -422,9 +543,11 @@ impl Page { return cosmic::command::future(async move { super::nm_edit_connection(uuid.as_ref()) .then(|res| async move { - match res.context("failed to open connection editor") { + match res { Ok(_) => Message::Refresh, - Err(why) => Message::Error(why.to_string()), + Err(why) => { + Message::Error(ErrorKind::ConnectionEditor, why.to_string()) + } } }) .await @@ -491,8 +614,9 @@ impl Page { } } - Message::Error(why) => { - tracing::error!(why); + Message::Error(error_kind, why) => { + tracing::error!(?error_kind, why); + self.dialog = Some(VpnDialog::Error(error_kind, why)) } Message::NetworkManagerConnect((conn, output)) => { @@ -511,21 +635,19 @@ impl Page { ) -> Command { cosmic::command::future(async move { if let Err(why) = nmcli::set_username(&connection_name, &username).await { - return Message::Error(format!("failed to set VPN username: {why}")); + return Message::Error(ErrorKind::WithPassword("username"), why.to_string()); } if let Err(why) = nmcli::set_password_flags_none(&connection_name).await { - return Message::Error(format!( - "failed to call nmcli to set VPN password-flags parameter: {why}" - )); + return Message::Error(ErrorKind::WithPassword("password-flags"), why.to_string()); } if let Err(why) = nmcli::set_password(&connection_name, password.unsecure()).await { - return Message::Error(format!("failed to call nmcli to set VPN password: {why}")); + return Message::Error(ErrorKind::WithPassword("password"), why.to_string()); } if let Err(why) = nmcli::connect(&connection_name).await { - return Message::Error(format!("failed to connect to VPN: {why}")); + return Message::Error(ErrorKind::Connect, why.to_string()); } Message::Refresh @@ -631,10 +753,13 @@ fn devices_view() -> Section { let known_networks = page.known_connections.iter().fold( vpn_connections, |networks, (uuid, connection)| { + let id = match connection { + ConnectionSettings::Vpn(connection) => connection.id.as_str(), + ConnectionSettings::Wireguard { id } => id.as_str(), + }; + let is_connected = active_conns.iter().any(|conn| match conn { - ActiveConnectionInfo::Vpn { name, .. } => { - name.as_str() == connection.id.as_str() - } + ActiveConnectionInfo::Vpn { name, .. } => name.as_str() == id, _ => false, }); @@ -648,8 +773,7 @@ fn devices_view() -> Section { ) }; - let identifier = - widget::text::body(connection.id.as_str()).wrap(Wrap::Glyph); + let identifier = widget::text::body(id).wrap(Wrap::Glyph); let connect: Element<'_, Message> = if let Some(msg) = connect_msg { widget::button::text(connect_txt).on_press(msg).into() @@ -739,7 +863,7 @@ fn update_state(conn: zbus::Connection) -> Command { cosmic::command::future(async move { match NetworkManagerState::new(&conn).await { Ok(state) => Message::UpdateState(state), - Err(why) => Message::Error(why.to_string()), + Err(why) => Message::Error(ErrorKind::UpdatingState, why.to_string()), } }) } @@ -751,7 +875,7 @@ fn update_devices(conn: zbus::Connection) -> Command { match network_manager::devices::list(&conn, filter).await { Ok(devices) => Message::UpdateDevices(devices), - Err(why) => Message::Error(why.to_string()), + Err(why) => Message::Error(ErrorKind::UpdatingState, why.to_string()), } }) } @@ -774,17 +898,37 @@ fn add_network() -> Command { .then(|result| async move { match result { Ok(response) => { - let vpn_type = if response.url().as_str().ends_with(".conf") { - "wireguard" + let response_str = response.url().as_str(); + let result = if let Some(device) = response_str.strip_suffix(".conf") { + let Ok(path) = response.url().to_file_path() else { + return Message::Error( + ErrorKind::WireGuardConfigPath, + fl!("vpn-error", "wireguard-config-path-desc"), + ); + }; + + let path = path.to_string_lossy().to_string(); + + let filename = device.rsplit_once("/").unwrap_or_default().1; + + let mut device = filename + .replace(|c: char| !c.is_alphanumeric(), "") + .to_ascii_lowercase(); + + device.truncate(15); + + return Message::AddWireGuardDevice(device, filename.to_owned(), path); } else { - "openvpn" + super::nm_add_vpn_file("openvpn", response.url().path()).await }; - _ = super::nm_add_vpn_file(vpn_type, response.url().path()).await; - Message::Refresh + match result { + Ok(_) => Message::Refresh, + Err(why) => Message::Error(ErrorKind::Config, why.to_string()), + } } Err(why) => { - return Message::Error(why.to_string()); + return Message::Error(ErrorKind::Config, why.to_string()); } } }) @@ -810,12 +954,26 @@ fn connection_settings(conn: zbus::Connection) -> Command { .filter_map(|conn| async move { let settings = conn.get_settings().await.ok()?; - let (connection, vpn) = settings.get("connection").zip(settings.get("vpn"))?; + let connection = settings.get("connection")?; + + match connection + .get("type")? + .downcast_ref::() + .ok()? + .as_str() + { + "vpn" => (), + + "wireguard" => { + let id = connection.get("id")?.downcast_ref::().ok()?; + let uuid = connection.get("uuid")?.downcast_ref::().ok()?; + return Some((Arc::from(uuid), ConnectionSettings::Wireguard { id })); + } - if connection.get("type")?.downcast_ref::().ok()? != "vpn" { - return None; + _ => return None, } + let vpn = settings.get("vpn")?; let id = connection.get("id")?.downcast_ref::().ok()?; let uuid = connection.get("uuid")?.downcast_ref::().ok()?; @@ -858,12 +1016,12 @@ fn connection_settings(conn: zbus::Connection) -> Command { Some(( Arc::from(uuid), - VpnConnectionSettings { + ConnectionSettings::Vpn(VpnConnectionSettings { id, connection_type, password_flag, username, - }, + }), )) }) // Reduce the settings list into @@ -877,12 +1035,9 @@ fn connection_settings(conn: zbus::Connection) -> Command { }; cosmic::command::future(async move { - settings - .await - .context("failed to get connection settings") - .map_or_else( - |why| Message::Error(why.to_string()), - Message::KnownConnections, - ) + settings.await.map_or_else( + |why| Message::Error(ErrorKind::ConnectionSettings, why.to_string()), + Message::KnownConnections, + ) }) } diff --git a/cosmic-settings/src/pages/networking/vpn/nmcli.rs b/cosmic-settings/src/pages/networking/vpn/nmcli.rs index cde91d33..8ab2dd6a 100644 --- a/cosmic-settings/src/pages/networking/vpn/nmcli.rs +++ b/cosmic-settings/src/pages/networking/vpn/nmcli.rs @@ -1,18 +1,19 @@ // Copyright 2024 System76 // SPDX-License-Identifier: GPL-3.0-only -use as_result::IntoResult; -use std::io; +use cosmic::Apply; +use std::process::Stdio; -pub async fn set_username(connection_name: &str, username: &str) -> io::Result<()> { +pub async fn set_username(connection_name: &str, username: &str) -> Result<(), String> { tokio::process::Command::new("nmcli") .args(&["con", "mod", connection_name, "vpn.user-name", username]) - .status() + .stderr(Stdio::piped()) + .output() .await - .and_then(IntoResult::into_result) + .apply(crate::utils::map_stderr_output) } -pub async fn set_password_flags_none(connection_name: &str) -> io::Result<()> { +pub async fn set_password_flags_none(connection_name: &str) -> Result<(), String> { tokio::process::Command::new("nmcli") .args(&[ "con", @@ -21,12 +22,13 @@ pub async fn set_password_flags_none(connection_name: &str) -> io::Result<()> { "+vpn.data", "password-flags=0", ]) - .status() + .stderr(Stdio::piped()) + .output() .await - .and_then(IntoResult::into_result) + .apply(crate::utils::map_stderr_output) } -pub async fn set_password(connection_name: &str, password: &str) -> io::Result<()> { +pub async fn set_password(connection_name: &str, password: &str) -> Result<(), String> { tokio::process::Command::new("nmcli") .args(&[ "con", @@ -35,15 +37,17 @@ pub async fn set_password(connection_name: &str, password: &str) -> io::Result<( "vpn.secrets", &format!("password={password}"), ]) - .status() + .stderr(Stdio::piped()) + .output() .await - .and_then(IntoResult::into_result) + .apply(crate::utils::map_stderr_output) } -pub async fn connect(connection_name: &str) -> io::Result<()> { +pub async fn connect(connection_name: &str) -> Result<(), String> { tokio::process::Command::new("nmcli") .args(&["con", "up", &connection_name]) - .status() + .stderr(Stdio::piped()) + .output() .await - .and_then(IntoResult::into_result) + .apply(crate::utils::map_stderr_output) } diff --git a/cosmic-settings/src/pages/networking/wired.rs b/cosmic-settings/src/pages/networking/wired.rs index 441584e3..4672fc1d 100644 --- a/cosmic-settings/src/pages/networking/wired.rs +++ b/cosmic-settings/src/pages/networking/wired.rs @@ -551,7 +551,6 @@ impl Page { fn devices_view() -> Section { crate::slab!(descriptions { wired_conns_txt = fl!("wired", "connections"); - wired_devices_txt = fl!("wired", "devices"); remove_txt = fl!("wired", "remove"); connect_txt = fl!("connect"); connected_txt = fl!("connected"); diff --git a/cosmic-settings/src/utils.rs b/cosmic-settings/src/utils.rs index 7b542b10..7fe5c186 100644 --- a/cosmic-settings/src/utils.rs +++ b/cosmic-settings/src/utils.rs @@ -1,4 +1,4 @@ -use std::future::Future; +use std::{future::Future, io, process}; use futures::{future::select, StreamExt}; @@ -47,6 +47,17 @@ pub fn forward_event_loop + Send + 'st cancel_tx } +/// On process failure, return stderr as `String`. +pub fn map_stderr_output(result: io::Result) -> Result<(), String> { + result.map_err(|why| why.to_string()).and_then(|output| { + if !output.status.success() { + Err(String::from_utf8(output.stderr).unwrap_or_default()) + } else { + Ok(()) + } + }) +} + /// Creates a slab with predefined items #[macro_export] macro_rules! slab { diff --git a/i18n/en/cosmic_settings.ftl b/i18n/en/cosmic_settings.ftl index 09ce0c95..a984c89c 100644 --- a/i18n/en/cosmic_settings.ftl +++ b/i18n/en/cosmic_settings.ftl @@ -1,5 +1,7 @@ app = COSMIC Settings +dbus-connection-error = Failed to connect to DBus +ok = OK unknown = Unknown number = { $number } @@ -62,9 +64,25 @@ remove-connection-dialog = Remove Connection Profile? vpn = VPN .connections = VPN Connections + .error = Failed to add VPN config .remove = Remove connection profile .select-file = Select a VPN configuration file +vpn-error = VPN Error + .config = Failed to add VPN config + .connect = Failed to connect to VPN + .connection-editor = Connection editor failed + .connection-settings = Failed to get settings for active connections + .updating-state = Failed to update network manager state + .wireguard-config-path = Invalid file path for WireGuard config + .wireguard-config-path-desc = Chosen file must be on a local file system. + .wireguard-device = Failed to create WireGuard device + .with-password = Failed to set VPN { $field -> + *[username] username + [password] password + [password-flags] password-flags + } with nmcli + wired = Wired .adapter = Wired adapter { $id } .connections = Wired Connections @@ -75,6 +93,9 @@ wifi = Wi-Fi .adapter = Wi-Fi adapter { $id } .forget = Forget this network +wireguard-dialog = Add WireGuard device + .description = Choose a device name for the WireGuard config. + ## Networking: Online Accounts online-accounts = Online Accounts