leyoda/cosmic-settings
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

fix(networking): stop storing the VPN username as a secret (#2116)

Browse source 
This PRs diff fixes #2109 such that a VPN username now gets written to
`vpn.data`, where NetworkManager and the VPN plugins actually read it
from. The existing `vpn.data` dict is read and merged rather than
replaced, so unrelated keys like remote and ca survive the update.

Profiles saved by earlier versions already have the username sitting in
the keyring, so the secret agent filters it out on read as well. Without
that, the stale entry keeps getting replayed to NetworkManager and
affected connections stay broken after upgrading.
This commit is contained in:
akrm al-hakimi 2026-07-28 18:29:19 -04:00 • committed by GitHub
parent 25cc9ef31b
commit 7287257ec9
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 109 additions and 20 deletions
Download patch file Download diff file Expand all files Collapse all files

4
Cargo.lock generated
View file

@ -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",

2
cosmic-settings/Cargo.toml
View file

@ -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"

83
cosmic-settings/src/pages/networking/backend.rs
View file

@ -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<String>,
),
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::<String, String>::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<String, String> = 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();

40
cosmic-settings/src/pages/networking/vpn/mod.rs
View file

@ -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
})