From 8a2cad01be8153996e14a69a77ed03ee02a204ea Mon Sep 17 00:00:00 2001 From: Bilal Elmoussaoui Date: Sat, 26 Sep 2026 14:32:48 +0200 Subject: [PATCH 1/9] client: Support gnome-keyring plain keyrings gnome-keyring stores keyrings with an empty password as plain text. --- Cargo.lock | 164 +++------- client/Cargo.toml | 1 + client/fixtures/plain.keyring | 74 +++++ .../encrypted.rs} | 11 +- client/src/file/api/legacy_keyring/mod.rs | 49 +++ client/src/file/api/legacy_keyring/plain.rs | 308 ++++++++++++++++++ 6 files changed, 482 insertions(+), 125 deletions(-) create mode 100644 client/fixtures/plain.keyring rename client/src/file/api/{legacy_keyring.rs => legacy_keyring/encrypted.rs} (97%) create mode 100644 client/src/file/api/legacy_keyring/mod.rs create mode 100644 client/src/file/api/legacy_keyring/plain.rs diff --git a/Cargo.lock b/Cargo.lock index 5ca172ee4..e89e7e5fc 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -59,7 +59,7 @@ version = "1.1.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "40c48f72fd53cd289104fc64099abca73db4166ad86ea0b4341abe65af83dadc" dependencies = [ - "windows-sys 0.61.2", + "windows-sys", ] [[package]] @@ -70,7 +70,7 @@ checksum = "291e6a250ff86cd4a820112fb8898808a366d8f9f58ce16d1f538353ad55747d" dependencies = [ "anstyle", "once_cell_polyfill", - "windows-sys 0.61.2", + "windows-sys", ] [[package]] @@ -159,7 +159,7 @@ dependencies = [ "polling", "rustix", "slab", - "windows-sys 0.61.2", + "windows-sys", ] [[package]] @@ -217,7 +217,7 @@ dependencies = [ "rustix", "signal-hook-registry", "slab", - "windows-sys 0.61.2", + "windows-sys", ] [[package]] @@ -257,9 +257,9 @@ checksum = "ac07cdecf99051d9a5238b80f35af32cdeba5b336e55d957b318b50137e18da5" [[package]] name = "bitflags" -version = "2.13.1" +version = "2.13.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b588b76d00fde79687d7646a9b5bdf3cc0f655e0bbd080335a95d7e96f3587da" +checksum = "3ded4057c258ba199e2d26386d3af3780957ecaee6c4ef4041c6b4b8b97c0b06" [[package]] name = "block" @@ -339,7 +339,7 @@ dependencies = [ "serde_json", "thiserror", "time", - "windows-sys 0.61.2", + "windows-sys", ] [[package]] @@ -372,9 +372,9 @@ dependencies = [ [[package]] name = "cfg-if" -version = "1.0.4" +version = "1.0.5" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9330f8b2ff13f34540b44e946ef35111825727b38d33286ef986142615121801" +checksum = "4e7648175b45a9a48536d676f68d918270699102aa8dab5496df06904c914600" [[package]] name = "cipher" @@ -424,9 +424,9 @@ dependencies = [ [[package]] name = "clap_lex" -version = "1.1.0" +version = "1.1.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c8d4a3bb8b1e0c1050499d1815f5ab16d04f0959b233085fb31653fbfc9d98f9" +checksum = "1c133bc6a41be0d194c306b5506d15e6feeea7b1d6604bd3f8310dfb2ca96486" [[package]] name = "cmov" @@ -463,18 +463,18 @@ checksum = "15b85f9c39137c3a891689859392b1bd49812121d0d61c9caf00d46ed5ce06ae" [[package]] name = "cpufeatures" -version = "0.3.0" +version = "0.3.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8b2a41393f66f16b0823bb79094d54ac5fbd34ab292ddafb9a0456ac9f87d201" +checksum = "5ca28b0ae3115b884660db4118d803791fd6756b6e88f39c0f3f7859060d7566" dependencies = [ "libc", ] [[package]] name = "crossbeam-utils" -version = "0.8.22" +version = "0.8.23" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "61803da095bee82a81bb1a452ecc25d3b2f1416d1897eb86430c6159ef717c17" +checksum = "a31eee39dddec8330830986fcd7625edb5a24ec90ea038215273bbc3adb08ac6" [[package]] name = "crypto-common" @@ -564,7 +564,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "39cab71617ae0d63f51a36d69f866391735b51691dbda63cf6f96d042b63efeb" dependencies = [ "libc", - "windows-sys 0.59.0", + "windows-sys", ] [[package]] @@ -751,9 +751,9 @@ checksum = "2304e00983f87ffb38b55b444b5e3b60a884b5d30c0fca7d82fe33449bbe55ea" [[package]] name = "hermit-abi" -version = "0.5.2" +version = "0.5.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "fc0fef456e4baa96da950455cd02c081ca953b141298e41db3fc7e36b1da849c" +checksum = "e17592d60ebacc7d5e169f4663c5f84f9161cc90328abcfe8456f41e4dfcb284" [[package]] name = "hex" @@ -781,9 +781,9 @@ dependencies = [ [[package]] name = "hybrid-array" -version = "0.4.14" +version = "0.4.15" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "707114b52a152fa7bdb290cd7cd5912d9467273b6d74e21b8d81aca1f8533f6b" +checksum = "27f864f10dfb56725ce5ce5472bc52252c8f93a4ab86327122cebf62c5f59a17" dependencies = [ "typenum", "zeroize", @@ -791,9 +791,9 @@ dependencies = [ [[package]] name = "indexmap" -version = "2.14.0" +version = "2.14.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d466e9454f08e4a911e14806c24e16fba1b4c121d1ea474396f396069cf949d9" +checksum = "cc4e190f5d26ca7051642629da2c52fc03bde85a03197c99408dcd291734c855" dependencies = [ "equivalent", "hashbrown", @@ -912,9 +912,9 @@ dependencies = [ [[package]] name = "log" -version = "0.4.33" +version = "0.4.34" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0ceec5bc11778974d1bcb055b18002eba7f4b3518b6a0081b3af5f21666da9ad" +checksum = "f9f8bd3e56ce4dfc153cf470fffbfa98c7620958b312ca5c3a4b8d5181fd13c6" [[package]] name = "malloc_buf" @@ -961,13 +961,13 @@ dependencies = [ [[package]] name = "mio" -version = "1.2.2" +version = "1.2.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "30d65c71f1ce40ab09135ce117d742b9f8a19ff91a41a8b57ed50bc2de59c427" +checksum = "4b18443e9c262bfe8fa82f51666e2642c53393f7e5c27b3e1aeab922cff5b9d8" dependencies = [ "libc", "wasi", - "windows-sys 0.61.2", + "windows-sys", ] [[package]] @@ -976,7 +976,7 @@ version = "0.50.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7957b9740744892f114936ab4a57b3f487491bbeafaf8083688b16841a4240e5" dependencies = [ - "windows-sys 0.59.0", + "windows-sys", ] [[package]] @@ -1139,6 +1139,7 @@ dependencies = [ "futures-lite", "futures-util", "getrandom 0.4.3", + "hex", "hkdf", "md-5", "num", @@ -1379,7 +1380,7 @@ dependencies = [ "hermit-abi", "pin-project-lite", "rustix", - "windows-sys 0.61.2", + "windows-sys", ] [[package]] @@ -1588,17 +1589,17 @@ checksum = "2da316a15f47e3d053de9cb2c439650bd8fa4aaeb9365f2e5f27f492ff73c196" dependencies = [ "libc", "rtoolbox", - "windows-sys 0.61.2", + "windows-sys", ] [[package]] name = "rtoolbox" -version = "0.0.5" +version = "0.0.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "50a0e551c1e27e1731aba276dbeaeac73f53c7cd34d1bda485d02bd1e0f36844" +checksum = "9a1efe12a1469752d0e6ff5ebec0b6ef4924cc5c4c71046b0ec730040535819d" dependencies = [ "libc", - "windows-sys 0.59.0", + "windows-sys", ] [[package]] @@ -1611,7 +1612,7 @@ dependencies = [ "errno", "libc", "linux-raw-sys", - "windows-sys 0.59.0", + "windows-sys", ] [[package]] @@ -1756,7 +1757,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c3d1e2c7f27f8d4cb10542a02c49005dbd6e93095799d6f3be745fae9f8fedd4" dependencies = [ "libc", - "windows-sys 0.61.2", + "windows-sys", ] [[package]] @@ -1815,7 +1816,7 @@ dependencies = [ "getrandom 0.4.3", "once_cell", "rustix", - "windows-sys 0.59.0", + "windows-sys", ] [[package]] @@ -1894,7 +1895,7 @@ dependencies = [ "socket2", "tokio-macros", "tracing", - "windows-sys 0.61.2", + "windows-sys", ] [[package]] @@ -1930,9 +1931,9 @@ dependencies = [ [[package]] name = "toml_edit" -version = "0.25.13+spec-1.1.0" +version = "0.25.15+spec-1.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6975367e4d2ef766d86af01ffad14b622fecc8d4357a998fbc4deb6e9bacaf9b" +checksum = "1340ea94a5856333492c9064b02c778b191dd2c853778d9609debdcdfea3a614" dependencies = [ "indexmap", "toml_datetime", @@ -2035,14 +2036,14 @@ checksum = "f2f6fb2847f6742cd76af783a2a2c49e9375d0a111c7bef6f71cd9e738c72d6e" dependencies = [ "memoffset", "tempfile", - "windows-sys 0.61.2", + "windows-sys", ] [[package]] name = "unicode-ident" -version = "1.0.24" +version = "1.0.26" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e6e4313cd5fcd3dad5cafa179702e2b244f760991f45397d14d4ebf38247da75" +checksum = "d245f478577f809a851594d02313b640fb437e0bb33866753cff937863096954" [[package]] name = "utf8parse" @@ -2052,9 +2053,9 @@ checksum = "06abde3611657adf66d383f00b093d7faecc7fa57071cce2578660c9f1010821" [[package]] name = "uuid" -version = "1.24.1" +version = "1.26.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2cefc03fd367c0c6d4305de1b312cf00248c4114f4a0418ce6a6af769e3b0bd9" +checksum = "2ef6dac1e96601b4fb3acccccff2139741fcb757cb9a36089bf5be91cfb285ce" dependencies = [ "getrandom 0.4.3", "js-sys", @@ -2162,15 +2163,6 @@ version = "0.2.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f0805222e57f7521d6a62e36fa9163bc891acd422f971defe97d64e70d0a4fe5" -[[package]] -name = "windows-sys" -version = "0.59.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1e38bc4d79ed67fd075bcc251a1c39b32a1776bbe92e5bef1f0bf1f8c531853b" -dependencies = [ - "windows-targets", -] - [[package]] name = "windows-sys" version = "0.61.2" @@ -2180,70 +2172,6 @@ dependencies = [ "windows-link", ] -[[package]] -name = "windows-targets" -version = "0.52.6" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9b724f72796e036ab90c1021d4780d4d3d648aca59e491e6b98e725b84e99973" -dependencies = [ - "windows_aarch64_gnullvm", - "windows_aarch64_msvc", - "windows_i686_gnu", - "windows_i686_gnullvm", - "windows_i686_msvc", - "windows_x86_64_gnu", - "windows_x86_64_gnullvm", - "windows_x86_64_msvc", -] - -[[package]] -name = "windows_aarch64_gnullvm" -version = "0.52.6" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "32a4622180e7a0ec044bb555404c800bc9fd9ec262ec147edd5989ccd0c02cd3" - -[[package]] -name = "windows_aarch64_msvc" -version = "0.52.6" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "09ec2a7bb152e2252b53fa7803150007879548bc709c039df7627cabbd05d469" - -[[package]] -name = "windows_i686_gnu" -version = "0.52.6" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8e9b5ad5ab802e97eb8e295ac6720e509ee4c243f69d781394014ebfe8bbfa0b" - -[[package]] -name = "windows_i686_gnullvm" -version = "0.52.6" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0eee52d38c090b3caa76c563b86c3a4bd71ef1a819287c19d586d7334ae8ed66" - -[[package]] -name = "windows_i686_msvc" -version = "0.52.6" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "240948bc05c5e7c6dabba28bf89d89ffce3e303022809e73deaefe4f6ec56c66" - -[[package]] -name = "windows_x86_64_gnu" -version = "0.52.6" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "147a5c80aabfbf0c7d901cb5895d1de30ef2907eb21fbbab29ca94c5b08b1a78" - -[[package]] -name = "windows_x86_64_gnullvm" -version = "0.52.6" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "24d5b23dc417412679681396f2b49f3de8c1473deb516bd34410872eff51ed0d" - -[[package]] -name = "windows_x86_64_msvc" -version = "0.52.6" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "589f6da84c646204747d1270a2a5661ea66ed1cced2631d546fdfb155959f9ec" - [[package]] name = "winnow" version = "1.0.4" @@ -2288,7 +2216,7 @@ dependencies = [ "tracing", "uds_windows", "uuid", - "windows-sys 0.61.2", + "windows-sys", "winnow", "zbus_macros", "zbus_names", diff --git a/client/Cargo.toml b/client/Cargo.toml index 34b90d5a7..9623b7cbb 100644 --- a/client/Cargo.toml +++ b/client/Cargo.toml @@ -24,6 +24,7 @@ endi.workspace = true futures-lite = { workspace = true, optional = true } futures-util.workspace = true getrandom = "0.4" +hex = "0.4" hkdf = { workspace = true, optional = true } md-5 = { workspace = true, optional = true } num.workspace = true diff --git a/client/fixtures/plain.keyring b/client/fixtures/plain.keyring new file mode 100644 index 000000000..c83f48b77 --- /dev/null +++ b/client/fixtures/plain.keyring @@ -0,0 +1,74 @@ +[keyring] +display-name=Login +ctime=0 +mtime=1441960049 +lock-on-idle=false +lock-after=false + +[917] +item-type=0 +display-name=Remmina: Shore - password +secret=some password +mtime=1768377640 +ctime=1768377640 + +[917:attribute0] +name=filename +type=string +value=/home/user/.local/share/remmina/group_rdp_shore_shore-sym.remmina + +[917:attribute1] +name=key +type=string +value=password + +[917:attribute2] +name=xdg:schema +type=string +value=org.remmina.Password + +[918] +item-type=2 +display-name=Binary +binary-secret=ff00ab +mtime=1198027852 +ctime=1198027852 + +[918:attribute0] +name=num +type=uint32 +value=3 + +[918:attribute1] +name=empty +type=string +value= + +[918:attribute2] +name=escaped +type=string +value=\sa\tb\\c\n + +[918:attribute3] +name=bad-number +type=uint32 +value=not-a-number + +[918:attribute4] +name=missing-number +type=uint32 + +[918:attribute5] +name=bad-escape +type=string +value=\q + +[918:acl0] +display-name=some-app +path=/usr/bin/some-app +read-access=true +write-access=true +remove-access=true + +[919] +item-type=0 diff --git a/client/src/file/api/legacy_keyring.rs b/client/src/file/api/legacy_keyring/encrypted.rs similarity index 97% rename from client/src/file/api/legacy_keyring.rs rename to client/src/file/api/legacy_keyring/encrypted.rs index 9f25f381b..dac2950cf 100644 --- a/client/src/file/api/legacy_keyring.rs +++ b/client/src/file/api/legacy_keyring/encrypted.rs @@ -1,4 +1,4 @@ -//! Legacy GNOME Keyring file format low level API. +//! Encrypted binary keyring format. use std::{ collections::HashMap, @@ -8,15 +8,12 @@ use std::{ use endi::{Endian, ReadBytes}; use zeroize::{Zeroize, ZeroizeOnDrop}; -use super::{Secret, UnlockedItem}; +use super::{FILE_HEADER, FILE_HEADER_LEN}; use crate::{ - AsAttributes, crypto, - file::{Error, WeakKeyError}, + AsAttributes, Secret, crypto, + file::{Error, UnlockedItem, WeakKeyError}, }; -const FILE_HEADER: &[u8] = b"GnomeKeyring\n\r\0\n"; -const FILE_HEADER_LEN: usize = FILE_HEADER.len(); - pub const MAJOR_VERSION: u8 = 0; pub const MINOR_VERSION: u8 = 0; diff --git a/client/src/file/api/legacy_keyring/mod.rs b/client/src/file/api/legacy_keyring/mod.rs new file mode 100644 index 000000000..e746a9f56 --- /dev/null +++ b/client/src/file/api/legacy_keyring/mod.rs @@ -0,0 +1,49 @@ +//! Legacy GNOME Keyring file format low level API. +//! +//! gnome-keyring stores a keyring either in an encrypted binary format, or as a +//! plain-text key file when the keyring password is empty. + +mod encrypted; +mod plain; + +pub use encrypted::MAJOR_VERSION; + +use crate::{ + Secret, + file::{Error, UnlockedItem}, +}; + +const FILE_HEADER: &[u8] = b"GnomeKeyring\n\r\0\n"; +const FILE_HEADER_LEN: usize = FILE_HEADER.len(); + +#[derive(Debug)] +pub enum Keyring { + Encrypted(encrypted::Keyring), + Plain(plain::Keyring), +} + +impl Keyring { + /// Retrieve the keyring items. + /// + /// The secret is ignored for plain keyrings. + pub fn decrypt_items(self, secret: &Secret) -> Result, Error> { + match self { + Self::Encrypted(keyring) => keyring.decrypt_items(secret), + Self::Plain(keyring) => Ok(keyring.into_items()), + } + } +} + +impl TryFrom<&[u8]> for Keyring { + type Error = Error; + + fn try_from(value: &[u8]) -> Result { + // Same as gnome-keyring, anything without the binary header is + // attempted as a plain keyring. + if value.starts_with(FILE_HEADER) { + encrypted::Keyring::try_from(value).map(Self::Encrypted) + } else { + plain::Keyring::try_from(value).map(Self::Plain) + } + } +} diff --git a/client/src/file/api/legacy_keyring/plain.rs b/client/src/file/api/legacy_keyring/plain.rs new file mode 100644 index 000000000..d226df048 --- /dev/null +++ b/client/src/file/api/legacy_keyring/plain.rs @@ -0,0 +1,308 @@ +//! Plain-text keyring format. +//! +//! gnome-keyring writes this format instead of the encrypted one when the +//! keyring password is empty. It is a GLib key file: +//! +//! ```ini +//! [keyring] +//! display-name=Login +//! +//! [1] +//! display-name=Item label +//! secret=the secret +//! +//! [1:attribute0] +//! name=user +//! type=string +//! value=alice +//! ``` +//! +//! Items are the groups whose name contains no `:`, their attributes are the +//! `:attribute` groups. Non UTF-8 secrets are stored hex encoded in +//! `binary-secret` instead of `secret`. + +use std::{collections::HashMap, io}; + +use zeroize::Zeroizing; + +use super::FILE_HEADER_LEN; +use crate::{ + Secret, + file::{Error, UnlockedItem}, +}; + +const KEYRING_GROUP: &str = "keyring"; + +#[derive(Debug)] +pub struct Keyring { + items: Vec, +} + +impl Keyring { + pub fn into_items(self) -> Vec { + self.items + } + + fn read_item(id: &str, group: &Group, attribute_groups: &[&Group]) -> UnlockedItem { + let label = group.string("display-name").unwrap_or_else(|| { + #[cfg(feature = "tracing")] + tracing::warn!("Item '{id}' has no label, defaulting to empty"); + String::new() + }); + + let secret = if let Some(secret) = group.string("secret").map(Zeroizing::new) { + Secret::text(&*secret) + } else if let Some(encoded) = group.string("binary-secret").map(Zeroizing::new) { + let decoded = Zeroizing::new(hex::decode(&*encoded).unwrap_or_else(|_err| { + #[cfg(feature = "tracing")] + tracing::warn!("Item '{id}' has an invalid binary secret, defaulting to empty"); + Vec::new() + })); + Secret::blob(&*decoded) + } else { + #[cfg(feature = "tracing")] + tracing::warn!("Item '{id}' has no secret, defaulting to empty"); + Secret::blob([]) + }; + #[cfg(not(feature = "tracing"))] + let _ = id; + + let mut attributes = HashMap::new(); + for group in attribute_groups { + let Some(name) = group.string("name") else { + continue; + }; + let value = if group.string("type").as_deref() == Some("uint32") { + // gnome-keyring reads the number as a u64 and truncates it. + group + .value("value") + .and_then(|v| v.parse::().ok()) + .map(|v| (v as u32).to_string()) + } else { + group.string("value") + }; + if let Some(value) = value { + attributes.insert(name, value); + } + } + + UnlockedItem::new(label, &attributes, secret) + } +} + +impl TryFrom<&[u8]> for Keyring { + type Error = Error; + + fn try_from(value: &[u8]) -> Result { + let header_mismatch = || { + Error::FileHeaderMismatch( + value + .get(..FILE_HEADER_LEN) + .map(|x| String::from_utf8_lossy(x).to_string()), + ) + }; + + let content = std::str::from_utf8(value).map_err(|_| header_mismatch())?; + let groups = parse_key_file(content)?.ok_or_else(header_mismatch)?; + + let mut attribute_groups = HashMap::<&str, Vec<&Group>>::new(); + for (name, group) in &groups { + if let Some((id, rest)) = name.split_once(':') + && rest.starts_with("attribute") + { + attribute_groups.entry(id).or_default().push(group); + } + } + + let items = groups + .iter() + .filter(|(name, _)| *name != KEYRING_GROUP && !name.contains(':')) + .map(|(id, group)| { + let attributes = attribute_groups.get(id).map_or(&[][..], Vec::as_slice); + Self::read_item(id, group, attributes) + }) + .collect(); + + Ok(Self { items }) + } +} + +#[derive(Debug, Default)] +struct Group<'a>(HashMap<&'a str, &'a str>); + +impl Group<'_> { + /// The raw value, like `g_key_file_get_value`. + fn value(&self, key: &str) -> Option<&str> { + self.0.get(key).copied() + } + + /// The unescaped value, like `g_key_file_get_string`. + fn string(&self, key: &str) -> Option { + unescape(self.value(key)?) + } +} + +/// Parse a GLib key file, keeping the groups in file order. +/// +/// Returns `None` if the first group is not `[keyring]`, i.e. it is not a +/// plain keyring. +fn parse_key_file(content: &str) -> Result)>>, Error> { + let mut groups: Vec<(&str, Group)> = Vec::new(); + let mut current = None; + + for (index, line) in content.split('\n').enumerate() { + let line = line.strip_suffix('\r').unwrap_or(line); + let line = line.trim_start_matches(|c: char| c.is_ascii_whitespace()); + if line.is_empty() || line.starts_with('#') { + continue; + } + + if let Some(name) = line + .strip_prefix('[') + .and_then(|l| l.split_once(']')) + .filter(|(_, rest)| rest.trim_matches([' ', '\t']).is_empty()) + .map(|(name, _)| name) + { + if groups.is_empty() && name != KEYRING_GROUP { + return Ok(None); + } + // Duplicated groups are merged + current = Some(match groups.iter().position(|(n, _)| *n == name) { + Some(position) => position, + None => { + groups.push((name, Group::default())); + groups.len() - 1 + } + }); + continue; + } + + let Some(current) = current else { + return Ok(None); + }; + let Some((key, value)) = line.split_once('=') else { + return Err(io::Error::new( + io::ErrorKind::InvalidData, + format!("invalid key file line {}", index + 1), + ) + .into()); + }; + let key = key.trim_end_matches(|c: char| c.is_ascii_whitespace()); + let value = value.trim_start_matches(|c: char| c.is_ascii_whitespace()); + groups[current].1.0.insert(key, value); + } + + Ok(current.map(|_| groups)) +} + +fn unescape(value: &str) -> Option { + let mut result = String::with_capacity(value.len()); + let mut chars = value.chars(); + while let Some(c) = chars.next() { + if c != '\\' { + result.push(c); + continue; + } + result.push(match chars.next()? { + 's' => ' ', + 'n' => '\n', + 't' => '\t', + 'r' => '\r', + '\\' => '\\', + _ => return None, + }); + } + Some(result) +} + +#[cfg(test)] +mod tests { + use std::path::PathBuf; + + use super::*; + use crate::{CONTENT_TYPE_ATTRIBUTE, XDG_SCHEMA_ATTRIBUTE}; + + fn load(name: &str) -> Result, Error> { + let path = PathBuf::from(env!("CARGO_MANIFEST_DIR")) + .join("fixtures") + .join(name); + let blob = std::fs::read(path)?; + Ok(Keyring::try_from(blob.as_slice())?.into_items()) + } + + #[test] + fn plain() -> Result<(), Error> { + let items = load("plain.keyring")?; + assert_eq!(items.len(), 3); + + assert_eq!(items[0].label(), "Remmina: Shore - password"); + assert_eq!(items[0].secret(), Secret::text("some password")); + let attributes = items[0].attributes(); + assert_eq!(attributes.len(), 4); // also content-type + assert_eq!( + attributes.get(XDG_SCHEMA_ATTRIBUTE).map(|v| v.as_ref()), + Some("org.remmina.Password") + ); + assert_eq!(attributes.get("key").map(|v| v.as_ref()), Some("password")); + assert_eq!( + attributes.get(CONTENT_TYPE_ATTRIBUTE).map(|v| v.as_ref()), + Some("text/plain") + ); + + assert_eq!(items[1].label(), "Binary"); + assert_eq!(items[1].secret(), Secret::blob([0xff, 0x00, 0xab])); + let attributes = items[1].attributes(); + assert_eq!(attributes.get("num").map(|v| v.as_ref()), Some("3")); + assert_eq!(attributes.get("empty").map(|v| v.as_ref()), Some("")); + assert_eq!( + attributes.get("escaped").map(|v| v.as_ref()), + Some(" a\tb\\c\n") + ); + assert!(!attributes.contains_key("bad-number")); + assert!(!attributes.contains_key("missing-number")); + assert!(!attributes.contains_key("bad-escape")); + + assert_eq!(items[2].label(), ""); + assert_eq!(items[2].secret(), Secret::blob([])); + + Ok(()) + } + + #[test] + fn not_plain() { + assert!(matches!( + Keyring::try_from(&b"[other]\nfoo=bar\n"[..]), + Err(Error::FileHeaderMismatch(_)) + )); + assert!(matches!( + Keyring::try_from(&b"random data"[..]), + Err(Error::FileHeaderMismatch(_)) + )); + assert!(matches!( + Keyring::try_from(&b"\xff\xfe"[..]), + Err(Error::FileHeaderMismatch(_)) + )); + assert!(matches!( + Keyring::try_from(&b""[..]), + Err(Error::FileHeaderMismatch(_)) + )); + assert!(matches!( + Keyring::try_from(&b"[keyring]\nnot a key value\n"[..]), + Err(Error::Io(_)) + )); + } + + #[test] + fn legacy_keyring_dispatch() -> Result<(), Error> { + let path = PathBuf::from(env!("CARGO_MANIFEST_DIR")) + .join("fixtures") + .join("plain.keyring"); + let blob = std::fs::read(path)?; + let keyring = super::super::Keyring::try_from(blob.as_slice())?; + assert!(matches!(keyring, super::super::Keyring::Plain(_))); + // The secret is ignored + let items = keyring.decrypt_items(&Secret::blob("whatever"))?; + assert_eq!(items.len(), 3); + Ok(()) + } +} From 8fd27c8ce42d902ad469d9c4cdc7bbdf9ac241b2 Mon Sep 17 00:00:00 2001 From: Bilal Elmoussaoui Date: Sat, 26 Sep 2026 14:44:00 +0200 Subject: [PATCH 2/9] server/tests: Use UnixStream helper instead of rustix No need to pull the extra dep there. --- server/Cargo.toml | 1 - server/src/tests.rs | 21 ++++++++------------- 2 files changed, 8 insertions(+), 14 deletions(-) diff --git a/server/Cargo.toml b/server/Cargo.toml index 674b110c9..bd5fd05f1 100644 --- a/server/Cargo.toml +++ b/server/Cargo.toml @@ -76,5 +76,4 @@ plasma_openssl_crypto = [ ] [dev-dependencies] -rustix = { version = "1.1", default-features = false, features = ["net"] } tempfile.workspace = true diff --git a/server/src/tests.rs b/server/src/tests.rs index 6ed6a0e8a..e79310491 100644 --- a/server/src/tests.rs +++ b/server/src/tests.rs @@ -1,9 +1,8 @@ -use std::{collections::HashMap, fs::File, io::Write, sync::Arc}; +use std::{collections::HashMap, io::Write, os::unix::net::UnixStream, sync::Arc}; #[cfg(any(feature = "gnome_native_crypto", feature = "gnome_openssl_crypto"))] use base64::Engine; use oo7::{Secret, crypto, dbus}; -use rustix::net::{AddressFamily, SocketFlags, SocketType, socketpair}; use tokio_stream::StreamExt; use zbus::zvariant::{Fd, ObjectPath, Optional, Value}; @@ -684,16 +683,12 @@ impl MockPrompterServicePlasma { callback_path ); - let (read_fd, write_fd) = socketpair( - AddressFamily::UNIX, - SocketType::STREAM, - SocketFlags::CLOEXEC | SocketFlags::NONBLOCK, - None, - ) - .expect("Failed to create socketpair"); - let mut file = File::from(write_fd); - file.write_all(secret.as_bytes()).unwrap(); - drop(file); // Close write end to signal EOF + let (read_end, mut write_end) = + UnixStream::pair().expect("Failed to create socketpair"); + read_end.set_nonblocking(true).unwrap(); + write_end.set_nonblocking(true).unwrap(); + write_end.write_all(secret.as_bytes()).unwrap(); + drop(write_end); // Close write end to signal EOF connection .call_method( @@ -701,7 +696,7 @@ impl MockPrompterServicePlasma { &callback_path, Some("org.kde.secretprompter.request"), "Accepted", - &(Fd::Owned(read_fd)), + &(Fd::Owned(read_end.into())), ) .await?; From 3fdf5cace2e9ba3aa53cccee17c53647ee653b9f Mon Sep 17 00:00:00 2001 From: Bilal Elmoussaoui Date: Sat, 26 Sep 2026 14:44:37 +0200 Subject: [PATCH 3/9] Update deps --- Cargo.lock | 50 +++++++++++++++++++++++++------------------------- 1 file changed, 25 insertions(+), 25 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index e89e7e5fc..ad565ce4b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -362,9 +362,9 @@ dependencies = [ [[package]] name = "cc" -version = "1.4.3" +version = "1.5.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "509591b7bcd67f4ef775afad7662703b4935daaa6ec0e5605cfb1090b32a2b6d" +checksum = "f360145194ee8e21db5ee7f3fcd4fe52210864c75c985dae33218202c8bbe040" dependencies = [ "find-msvc-tools", "shlex", @@ -595,9 +595,9 @@ checksum = "da7c62ceae207dd37ea5b845da6a0696c799f85e97da1ab5b7910be3c1c80223" [[package]] name = "find-msvc-tools" -version = "0.1.11" +version = "0.1.14" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d45db016d36b838f563236e9193d0ee6ce38f3f68b6c94e914b4929c96bbb890" +checksum = "aedcfb3409746eddb02b9e19ebda1c3394f759a152e48ee875a0844d1b955484" [[package]] name = "foreign-types" @@ -823,9 +823,9 @@ checksum = "8f42a60cbdf9a97f5d2305f08a87dc4e09308d1276d28c869c684d7777685682" [[package]] name = "js-sys" -version = "0.3.104" +version = "0.3.106" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0e0c1080212aad755ea003d18543e8768dd432c48819efd73a7bf1e39b7a5a3a" +checksum = "7883d941dae510fb2d978fc3fe018c71c9e2892fd38854de3e8b92c2e5ad9cc5" dependencies = [ "cfg-if", "futures-util", @@ -1746,9 +1746,9 @@ checksum = "0c790de23124f9ab44544d7ac05d60440adc586479ce501c1d6d7da3cd8c9cf5" [[package]] name = "smallvec" -version = "1.15.2" +version = "1.16.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8ed6a63f02c8539c91a8685a86f4099661ba3da017932f6ebbea6de3f0fa7c90" +checksum = "f9395f0f0eee849a9b707b2f06bb92a6a422090e2123bb2ef8e87a0e61892a8e" [[package]] name = "socket2" @@ -1821,18 +1821,18 @@ dependencies = [ [[package]] name = "thiserror" -version = "2.0.20" +version = "2.0.21" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ec86235f5fcc2a73650310756d2ac5b138a5780bbbdfae3eeccec992c435ba4f" +checksum = "09e52cb86a36cede5cb101bf8908837b3e4c6e5e59fe7fd85c23fb56200d189e" dependencies = [ "thiserror-impl", ] [[package]] name = "thiserror-impl" -version = "2.0.20" +version = "2.0.21" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "bc04cd3e1236dd4a98afca4569f2deb3f120e5422a4023be2cb683f8486292af" +checksum = "fe5197923287db20a58125f0bc85c062f7f2c892de97b18c356f9efb14b28524" dependencies = [ "proc-macro2", "quote", @@ -2092,9 +2092,9 @@ dependencies = [ [[package]] name = "wasm-bindgen" -version = "0.2.127" +version = "0.2.129" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1b70935747edd64d89de3efa29d73789b806c15798f8e7dca4d8ac356b50ce70" +checksum = "9bb54f33acc68fd454578d9820b0bde1a1a3d17aa17bb7b6595806d02886d409" dependencies = [ "cfg-if", "once_cell", @@ -2105,9 +2105,9 @@ dependencies = [ [[package]] name = "wasm-bindgen-macro" -version = "0.2.127" +version = "0.2.129" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "77775f8f3f7217702089053b94958f8f54061a3f663417df76e19cbdcca29bc1" +checksum = "2e29d0c35b16e224a7eeb5cd2d25e3e1968fbd65604117b44d3b789d00ee8535" dependencies = [ "quote", "wasm-bindgen-macro-support", @@ -2115,22 +2115,22 @@ dependencies = [ [[package]] name = "wasm-bindgen-macro-support" -version = "0.2.127" +version = "0.2.129" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e11d33f857dc2fb11b8bc75aee111aa9cbeb12cd9f25efd3d4c2a3dd4e235284" +checksum = "6f501a8bc3719dba86ef8ae4728879c08001bea749eb1333ac5b91e040e2a6b7" dependencies = [ "bumpalo", "proc-macro2", "quote", - "syn 2.0.119", + "syn 3.0.6", "wasm-bindgen-shared", ] [[package]] name = "wasm-bindgen-shared" -version = "0.2.127" +version = "0.2.129" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7ef64dbcc55df09c7e5a46182d181c2cfa3e925f3da937ea764728b4bbb9dcbf" +checksum = "23f0c9c52aa7cd7d77769a4cfe2a9adb1b331f489a41d912ce14513d5ab995c6" dependencies = [ "unicode-ident", ] @@ -2260,18 +2260,18 @@ dependencies = [ [[package]] name = "zerocopy" -version = "0.8.56" +version = "0.8.59" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "556764e583adb45a9f8d413c2a147fa7e8d821e48e12b14fd560b607998b75eb" +checksum = "6df92bf3d9227be3d53173901ddbffac2babc27ae50f397776ffd6dc33f800cb" dependencies = [ "zerocopy-derive", ] [[package]] name = "zerocopy-derive" -version = "0.8.56" +version = "0.8.59" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f2ab42fc20575779bd240faa45f94a74256f755c0fa9e89f0ede20d91d0cdfc1" +checksum = "ac4f328cf2f05d084e496c3e9c3f33ed0a183656a16e1fcec4d464d8373aec82" dependencies = [ "proc-macro2", "quote", From b9696221ae99dc6f130de66b44311e9ef52149fd Mon Sep 17 00:00:00 2001 From: Bilal Elmoussaoui Date: Sat, 26 Sep 2026 15:09:31 +0200 Subject: [PATCH 4/9] server: Guard linux only bits Not that server would be usable in any other platform, but at least it would compile and most of the tests pass. --- server/src/login.rs | 9 +++++++++ server/src/main.rs | 2 ++ 2 files changed, 11 insertions(+) diff --git a/server/src/login.rs b/server/src/login.rs index d71029d29..c6ab03f81 100644 --- a/server/src/login.rs +++ b/server/src/login.rs @@ -1,3 +1,4 @@ +#[cfg(target_os = "linux")] use std::{ io::{self, IoSlice, Read}, mem::MaybeUninit, @@ -6,19 +7,24 @@ use std::{ sync::LazyLock, }; +#[cfg(target_os = "linux")] use rustix::{ fs::{MemfdFlags, SealFlags}, net::{SendAncillaryBuffer, SendAncillaryMessage, SendFlags}, }; +#[cfg(target_os = "linux")] const HELPER_TIMEOUT_SECS: u64 = 120; +#[cfg(target_os = "linux")] const BINARY_NAME: &str = env!("CARGO_BIN_NAME"); +#[cfg(target_os = "linux")] pub static SOCKET_PATH: LazyLock = LazyLock::new(|| { let uid = rustix::process::getuid().as_raw(); PathBuf::from(format!("/run/user/{uid}/oo7-daemon-login.sock")) }); +#[cfg(target_os = "linux")] fn main() { tracing_subscriber::fmt::init(); @@ -151,3 +157,6 @@ fn main() { tracing::info!("Secret delivered to daemon"); let _ = std::fs::remove_file(socket_path); } + +#[cfg(not(target_os = "linux"))] +fn main() {} diff --git a/server/src/main.rs b/server/src/main.rs index ed5364c09..4b7bba00c 100644 --- a/server/src/main.rs +++ b/server/src/main.rs @@ -1,4 +1,5 @@ #![deny(unsafe_code)] +#[cfg(target_os = "linux")] mod capability; mod collection; mod error; @@ -126,6 +127,7 @@ async fn read_secret_from_credentials_directory() -> Option { } async fn inner_main(args: Args) -> Result<(), Error> { + #[cfg(target_os = "linux")] capability::drop_unnecessary_capabilities()?; let secret = if args.login { From 3567e51ef1bd561c54c755ec116cc6c2e9e77fcb Mon Sep 17 00:00:00 2001 From: Bilal Elmoussaoui Date: Sat, 26 Sep 2026 15:36:54 +0200 Subject: [PATCH 5/9] server/tests: Don't leak connection --- server/src/gnome/internal.rs | 2 +- server/src/gnome/prompter.rs | 6 +++--- server/src/plasma/prompter.rs | 2 +- server/src/prompt/mod.rs | 4 ++-- server/src/service/mod.rs | 38 ++++++++++++++++++++++++----------- server/src/service/tests.rs | 4 ++-- server/src/session.rs | 4 ++++ server/src/tests.rs | 6 ++++++ 8 files changed, 45 insertions(+), 21 deletions(-) diff --git a/server/src/gnome/internal.rs b/server/src/gnome/internal.rs index 5ab57cfa1..2da7266b8 100644 --- a/server/src/gnome/internal.rs +++ b/server/src/gnome/internal.rs @@ -36,7 +36,7 @@ impl InternalInterface { ))); }; - let secret = DBusSecret::from_inner(self.service.connection(), secret) + let secret = DBusSecret::from_inner(&self.service.connection(), secret) .await .map_err(|err| { custom_service_error(&format!("Failed to create session object {err}")) diff --git a/server/src/gnome/prompter.rs b/server/src/gnome/prompter.rs index e8feca4d7..edeeb9e86 100644 --- a/server/src/gnome/prompter.rs +++ b/server/src/gnome/prompter.rs @@ -381,7 +381,7 @@ impl GNOMEPrompterCallback { ), }; - let prompter = GNOMEPrompterProxy::new(connection).await?; + let prompter = GNOMEPrompterProxy::new(&connection).await?; let path = self.path.clone(); let exchange = self.exchange.get().unwrap().clone(); tokio::spawn(async move { @@ -393,7 +393,7 @@ impl GNOMEPrompterCallback { } async fn prompter_done(&self, prompt: &Prompt, exchange: &str) -> Result<(), ServiceError> { - let prompter = GNOMEPrompterProxy::new(self.service.connection()).await?; + let prompter = GNOMEPrompterProxy::new(&self.service.connection()).await?; let aes_key = secret_exchange::handshake(&self.private_key, exchange).map_err(|err| { custom_service_error(&format!( "Failed to generate AES key for SecretExchange {err}." @@ -462,7 +462,7 @@ impl GNOMEPrompterCallback { async fn prompter_dismissed(&self, prompt_path: OwnedObjectPath) -> Result<(), ServiceError> { let path = self.path.clone(); - let prompter = GNOMEPrompterProxy::new(self.service.connection()).await?; + let prompter = GNOMEPrompterProxy::new(&self.service.connection()).await?; tokio::spawn(async move { prompter.stop_prompting(&path).await }); let signal_emitter = self.service.signal_emitter(prompt_path)?; diff --git a/server/src/plasma/prompter.rs b/server/src/plasma/prompter.rs index 8da89f2af..5fc6775bd 100644 --- a/server/src/plasma/prompter.rs +++ b/server/src/plasma/prompter.rs @@ -174,7 +174,7 @@ impl PlasmaPrompterCallback { collection_name: &str, ) -> Result<(), ServiceError> { let path = self.path.clone(); - let prompter = PlasmaPrompterProxy::new(self.service.connection()).await?; + let prompter = PlasmaPrompterProxy::new(&self.service.connection()).await?; let window_id = match window_id { Some(id) => id.to_string(), None => String::new(), diff --git a/server/src/prompt/mod.rs b/server/src/prompt/mod.rs index cf61ab39d..fb563d87c 100644 --- a/server/src/prompt/mod.rs +++ b/server/src/prompt/mod.rs @@ -381,14 +381,14 @@ impl Prompt { self.service.object_server().at(&path, callback).await?; tracing::debug!("Prompt `{}` created.", self.path); - let prompter = GNOMEPrompterProxy::new(self.service.connection()).await?; + let prompter = GNOMEPrompterProxy::new(&self.service.connection()).await?; tokio::spawn(async move { prompter.begin_prompting(&path).await }); Ok(()) } async fn prompt_cli(&self) -> Result<(), ServiceError> { - let proxy = CliPrompterProxy::new(self.service.connection()) + let proxy = CliPrompterProxy::new(&self.service.connection()) .await .map_err(|e| custom_service_error(&format!("CLI prompter not available: {e}")))?; diff --git a/server/src/service/mod.rs b/server/src/service/mod.rs index c2c7b08fb..1724f59a6 100644 --- a/server/src/service/mod.rs +++ b/server/src/service/mod.rs @@ -3,7 +3,7 @@ use std::{ collections::HashMap, sync::{ - Arc, OnceLock, + Arc, RwLock, atomic::{AtomicU32, Ordering}, }, }; @@ -54,7 +54,7 @@ pub struct Service { // Properties pub(crate) collections: Arc>>, // Other attributes - connection: Arc>, + connection: Arc>>, // sessions mapped to their corresponding object path on the bus sessions: Arc>>, session_index: Arc, @@ -124,7 +124,7 @@ impl Service { }; let peer_info = async { - let proxy = zbus::fdo::DBusProxy::new(self.connection()).await.ok()?; + let proxy = zbus::fdo::DBusProxy::new(&self.connection()).await.ok()?; let pid = proxy .get_connection_unix_process_id(sender.as_ref().into()) .await @@ -544,7 +544,7 @@ impl Service { if is_graphical { #[cfg(any(feature = "plasma_native_crypto", feature = "plasma_openssl_crypto"))] { - if in_plasma_environment(self.connection()).await { + if in_plasma_environment(&self.connection()).await { return PrompterType::Plasma; } } @@ -561,7 +561,7 @@ impl Service { ) -> Self { Self { collections: Arc::new(Mutex::new(HashMap::new())), - connection: Arc::new(OnceLock::new()), + connection: Default::default(), sessions: Arc::new(Mutex::new(HashMap::new())), session_index: Arc::new(AtomicU32::new(0)), prompts: Arc::new(Mutex::new(HashMap::new())), @@ -1029,7 +1029,7 @@ impl Service { secret: Option, auto_create_default: bool, ) -> Result<(), Error> { - self.connection.set(connection.clone()).unwrap(); + *self.connection.write().unwrap() = Some(connection.clone()); let object_server = connection.object_server(); let mut collections = self.collections.lock().await; @@ -1137,7 +1137,8 @@ impl Service { .member("NameOwnerChanged")? .arg(2, "")? .build(); - let mut stream = zbus::MessageStream::for_match_rule(rule, self.connection(), None).await?; + let mut stream = + zbus::MessageStream::for_match_rule(rule, &self.connection(), None).await?; while let Some(message) = stream.try_next().await? { let body = message.body(); let Ok((_name, old_owner, new_owner)) = @@ -1279,12 +1280,25 @@ impl Service { Ok((without_prompt, with_prompt)) } - pub fn connection(&self) -> &zbus::Connection { - self.connection.get().unwrap() + pub fn connection(&self) -> zbus::Connection { + self.connection + .read() + .unwrap() + .clone() + .expect("service is not initialized") + } + + pub fn object_server(&self) -> zbus::ObjectServer { + self.connection().object_server().clone() } - pub fn object_server(&self) -> &zbus::ObjectServer { - self.connection().object_server() + /// Drop the service's reference to its connection. + /// + /// The connection's object server holds the service, which in turn holds + /// the connection, so neither is ever freed otherwise. + #[cfg(any(test, feature = "test-util"))] + pub(crate) fn release_connection(&self) { + self.connection.write().unwrap().take(); } async fn resolve_alias( @@ -1469,7 +1483,7 @@ impl Service { P: TryInto>, P::Error: Into, { - let signal_emitter = zbus::object_server::SignalEmitter::new(self.connection(), path)?; + let signal_emitter = zbus::object_server::SignalEmitter::new(&self.connection(), path)?; Ok(signal_emitter) } diff --git a/server/src/service/tests.rs b/server/src/service/tests.rs index 393310597..c5505c90d 100644 --- a/server/src/service/tests.rs +++ b/server/src/service/tests.rs @@ -28,8 +28,8 @@ async fn open_session_encrypted() -> Result<(), Box> { setup.server_public_key.is_some(), "Encrypted session should have server public key" ); - let key = setup.aes_key.unwrap().clone(); - assert_eq!((*key).as_ref().len(), 16, "AES key should be 16 bytes"); + let key = setup.aes_key.as_deref().unwrap(); + assert_eq!(key.as_ref().len(), 16, "AES key should be 16 bytes"); Ok(()) } diff --git a/server/src/session.rs b/server/src/session.rs index 862586119..6a633a2f7 100644 --- a/server/src/session.rs +++ b/server/src/session.rs @@ -267,6 +267,8 @@ mod tests { command.spawn().expect("failed to spawn test child process") } + // Reads `/proc//environ` + #[cfg(target_os = "linux")] #[tokio::test] async fn from_environ_detects_wayland() { let mut child = spawn_with_env(&[("WAYLAND_DISPLAY", "wayland-test")]); @@ -279,6 +281,8 @@ mod tests { assert_eq!(session_type, Some(SessionType::Wayland)); } + // Reads `/proc//environ` + #[cfg(target_os = "linux")] #[tokio::test] async fn from_environ_detects_x11() { let mut child = spawn_with_env(&[("DISPLAY", ":0")]); diff --git a/server/src/tests.rs b/server/src/tests.rs index e79310491..12dbe6062 100644 --- a/server/src/tests.rs +++ b/server/src/tests.rs @@ -49,6 +49,12 @@ pub struct TestServiceSetup { _temp_dir: tempfile::TempDir, } +impl Drop for TestServiceSetup { + fn drop(&mut self) { + self.server.release_connection(); + } +} + impl TestServiceSetup { /// Get the default/Login collection pub async fn default_collection( From e8d5cd5f8449b48788eceb65f0b755e1927bd473 Mon Sep 17 00:00:00 2001 From: Bilal Elmoussaoui Date: Sat, 26 Sep 2026 15:37:28 +0200 Subject: [PATCH 6/9] Fix nightly formatting issue --- client/tests/file_unlocked_keyring.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/client/tests/file_unlocked_keyring.rs b/client/tests/file_unlocked_keyring.rs index 7760cb458..512da9f46 100644 --- a/client/tests/file_unlocked_keyring.rs +++ b/client/tests/file_unlocked_keyring.rs @@ -535,8 +535,8 @@ async fn delete_broken_items() -> Result<(), Error> { let keyring_path = v1_dir.join("default.keyring"); fs::copy(&fixture_path, &keyring_path).await?; - // 1) Load with the correct password and add several valid items. This - // ensures valid_items > broken_items that we'll add later. + // 1) Load with the correct password and add several valid items. + // This ensures valid_items > broken_items that we'll add later. let keyring = UnlockedKeyring::load(&keyring_path, Some(Secret::blob("test"))).await?; for i in 0..VALID_TO_ADD { keyring From 488e2633e513e31b44734e32ed809657fa4b8750 Mon Sep 17 00:00:00 2001 From: Bilal Elmoussaoui Date: Sat, 26 Sep 2026 15:48:57 +0200 Subject: [PATCH 7/9] Fallback to plain v0 keyrings when there is a header mismatch --- client/src/file/unlocked_keyring.rs | 62 ++++++++++++++++----------- client/tests/file_unlocked_keyring.rs | 37 ++++++++++++++++ server/src/service/mod.rs | 18 ++++++++ server/src/service/tests.rs | 31 ++++++++++++++ 4 files changed, 122 insertions(+), 26 deletions(-) diff --git a/client/src/file/unlocked_keyring.rs b/client/src/file/unlocked_keyring.rs index 84f8d55e6..e435bd8fc 100644 --- a/client/src/file/unlocked_keyring.rs +++ b/client/src/file/unlocked_keyring.rs @@ -162,41 +162,51 @@ impl UnlockedKeyring { key: Default::default(), secret: Mutex::new(secret.map(Arc::new)), }), + // Plain legacy keyrings don't have the binary file header + Err(Error::FileHeaderMismatch(_)) => Self::migrate_legacy(&content, path, secret), Err(Error::VersionMismatch(Some(version))) if version[0] == api::LEGACY_MAJOR_VERSION => { - #[cfg(feature = "tracing")] - tracing::debug!("Migrating from legacy keyring format"); + Self::migrate_legacy(&content, path, secret) + } + Err(err) => Err(err), + } + } - let legacy_keyring = api::LegacyKeyring::try_from(content.as_slice())?; - let mut keyring = api::Keyring::new()?; + fn migrate_legacy( + content: &[u8], + path: impl AsRef, + secret: Option, + ) -> Result { + #[cfg(feature = "tracing")] + tracing::debug!("Migrating from legacy keyring format"); - let key = secret - .as_ref() - .map(|s| keyring.derive_key_unchecked(s)) - .transpose()?; - let decrypted_items = legacy_keyring - .decrypt_items(&secret.clone().unwrap_or_else(|| Secret::from(vec![])))?; + let legacy_keyring = api::LegacyKeyring::try_from(content)?; + let mut keyring = api::Keyring::new()?; - #[cfg(feature = "tracing")] - let _migrate_span = - tracing::debug_span!("migrate_items", item_count = decrypted_items.len()); + let key = secret + .as_ref() + .map(|s| keyring.derive_key_unchecked(s)) + .transpose()?; + let decrypted_items = legacy_keyring + .decrypt_items(&secret.clone().unwrap_or_else(|| Secret::from(vec![])))?; - for item in decrypted_items { - let encrypted_item = item.encrypt(key.as_ref())?; - keyring.items.push(encrypted_item); - } + #[cfg(feature = "tracing")] + let _migrate_span = + tracing::debug_span!("migrate_items", item_count = decrypted_items.len()); - Ok(Self { - keyring: Arc::new(RwLock::new(keyring)), - path: Some(path.as_ref().to_path_buf()), - mtime: Default::default(), - key: Default::default(), - secret: Mutex::new(secret.map(Arc::new)), - }) - } - Err(err) => Err(err), + for item in decrypted_items { + let encrypted_item = item.encrypt(key.as_ref())?; + keyring.items.push(encrypted_item); } + + Ok(Self { + keyring: Arc::new(RwLock::new(keyring)), + path: Some(path.as_ref().to_path_buf()), + mtime: Default::default(), + key: Default::default(), + secret: Mutex::new(secret.map(Arc::new)), + }) } /// Helper for opening/creating keyrings with explicit paths. diff --git a/client/tests/file_unlocked_keyring.rs b/client/tests/file_unlocked_keyring.rs index 512da9f46..f247f7986 100644 --- a/client/tests/file_unlocked_keyring.rs +++ b/client/tests/file_unlocked_keyring.rs @@ -416,6 +416,43 @@ async fn migrate_from_legacy() -> Result<(), Error> { Ok(()) } +async fn migrate_from_plain_legacy(secret: Option) -> Result<(), Error> { + let data_dir = tempdir()?; + let v0_dir = data_dir.path().join("keyrings"); + let v1_dir = v0_dir.join("v1"); + fs::create_dir_all(&v1_dir).await?; + + let fixture_path = PathBuf::from(env!("CARGO_MANIFEST_DIR")) + .join("fixtures") + .join("plain.keyring"); + fs::copy(&fixture_path, &v0_dir.join("login.keyring")).await?; + + let keyring = UnlockedKeyring::open_at(data_dir.path(), "login", secret.clone()).await?; + keyring.write().await?; + assert!(v1_dir.join("login.keyring").exists()); + + let keyring = UnlockedKeyring::open_at(data_dir.path(), "login", secret).await?; + let items = keyring + .search_items(&[(XDG_SCHEMA_ATTRIBUTE, "org.remmina.Password")]) + .await?; + assert_eq!(items.len(), 1); + assert_eq!(items[0].label(), "Remmina: Shore - password"); + assert_eq!(items[0].secret(), Secret::text("some password")); + assert_eq!(keyring.n_items().await, 3); + + Ok(()) +} + +#[tokio::test] +async fn migrate_from_plain_legacy_without_secret() -> Result<(), Error> { + migrate_from_plain_legacy(None).await +} + +#[tokio::test] +async fn migrate_from_plain_legacy_with_secret() -> Result<(), Error> { + migrate_from_plain_legacy(Some(Secret::blob("test"))).await +} + #[tokio::test] async fn migrate() -> Result<(), Error> { let data_dir = tempdir()?; diff --git a/server/src/service/mod.rs b/server/src/service/mod.rs index 1724f59a6..663c35738 100644 --- a/server/src/service/mod.rs +++ b/server/src/service/mod.rs @@ -1011,6 +1011,23 @@ impl Service { Keyring::Locked(locked) } + // Plain legacy keyrings, written by gnome-keyring for an empty + // password, don't have the binary file header. Their migration + // doesn't need a secret, so if it fails this isn't a keyring. + Err(oo7::file::Error::FileHeaderMismatch(_)) => { + tracing::info!( + "Found legacy plain keyring '{name}' at {}, migrating", + path.display() + ); + + let migration = PendingMigration::V0 { + name: name.to_owned(), + path: path.to_path_buf(), + label: label.clone(), + alias: alias.clone(), + }; + Keyring::Unlocked(migration.migrate(&self.data_dir, secret).await?) + } Err(e) => { return Err(e.into()); } @@ -1297,6 +1314,7 @@ impl Service { /// The connection's object server holds the service, which in turn holds /// the connection, so neither is ever freed otherwise. #[cfg(any(test, feature = "test-util"))] + #[allow(dead_code)] pub(crate) fn release_connection(&self) { self.connection.write().unwrap().take(); } diff --git a/server/src/service/tests.rs b/server/src/service/tests.rs index c5505c90d..9743db13a 100644 --- a/server/src/service/tests.rs +++ b/server/src/service/tests.rs @@ -1398,6 +1398,37 @@ async fn discover_v0_keyrings() -> Result<(), Box> { Ok(()) } +#[tokio::test] +async fn discover_plain_v0_keyrings() -> Result<(), Box> { + let temp_dir = tempfile::tempdir()?; + let service = Service::new(temp_dir.path().to_path_buf(), None); + + let keyrings_dir = temp_dir.path().join("keyrings"); + tokio::fs::create_dir_all(keyrings_dir.join("v1")).await?; + + let fixture_path = std::path::PathBuf::from(env!("CARGO_MANIFEST_DIR")) + .parent() + .unwrap() + .join("client/fixtures/plain.keyring"); + let v0_path = keyrings_dir.join("plain.keyring"); + tokio::fs::copy(&fixture_path, &v0_path).await?; + // Not a keyring, should be skipped + tokio::fs::write(keyrings_dir.join("junk.keyring"), b"junk").await?; + + // Plain keyrings are migrated even without a secret + let discovered = service.discover_keyrings(None).await?; + assert_eq!(discovered.len(), 1, "Only the plain keyring is discovered"); + + let (_, label, _, keyring) = &discovered[0]; + assert_eq!(label, "Plain"); + assert!(!keyring.is_locked(), "Plain keyring should be migrated"); + assert!(service.pending_migrations.lock().await.is_empty()); + assert!(keyrings_dir.join("v1/plain.keyring").exists()); + assert!(crate::migration::stamp_path(&v0_path).exists()); + + Ok(()) +} + #[cfg(feature = "kwallet_migration")] #[cfg(target_endian = "little")] #[tokio::test] From 63f92177af107768cb9fe30f6e370d881d9f1c02 Mon Sep 17 00:00:00 2001 From: Bilal Elmoussaoui Date: Sat, 26 Sep 2026 15:51:18 +0200 Subject: [PATCH 8/9] pam: Don't hardcode uid in tests --- pam/src/socket.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pam/src/socket.rs b/pam/src/socket.rs index f3ed17ad6..905791ffb 100644 --- a/pam/src/socket.rs +++ b/pam/src/socket.rs @@ -418,7 +418,9 @@ mod tests { let message = PamMessage::unlock("testuser".to_string(), b"testpassword".to_vec()); - let result = send_secret_to_daemon_async(message, 1000, false, Some(socket_path)).await; + // The socket must be owned by the given UID + let uid = unsafe { libc::getuid() }; + let result = send_secret_to_daemon_async(message, uid, false, Some(socket_path)).await; assert!(result.is_ok()); server.await?; From 2d612233037227a38ddd2f102878be496dd96ea6 Mon Sep 17 00:00:00 2001 From: Bilal Elmoussaoui Date: Sat, 26 Sep 2026 16:11:54 +0200 Subject: [PATCH 9/9] python: Make it buildable on non-linux platforms For local testing only. --- Cargo.lock | 1 + python/Cargo.toml | 3 +++ python/build.rs | 5 +++++ 3 files changed, 9 insertions(+) create mode 100644 python/build.rs diff --git a/Cargo.lock b/Cargo.lock index ad565ce4b..d606bcbd1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1257,6 +1257,7 @@ dependencies = [ "oo7", "pyo3", "pyo3-async-runtimes", + "pyo3-build-config", "tokio", ] diff --git a/python/Cargo.toml b/python/Cargo.toml index b67de2fe5..927b3620c 100644 --- a/python/Cargo.toml +++ b/python/Cargo.toml @@ -20,3 +20,6 @@ oo7_rs = { package = "oo7", path = "../client", version = "0.7.0-alpha" } pyo3 = { version = "0.29", features = ["extension-module", "abi3-py38"] } pyo3-async-runtimes = { version = "0.29", features = ["tokio-runtime"] } tokio = { workspace = true, features = ["rt-multi-thread"] } + +[build-dependencies] +pyo3-build-config = "0.29" diff --git a/python/build.rs b/python/build.rs new file mode 100644 index 000000000..abb0250ec --- /dev/null +++ b/python/build.rs @@ -0,0 +1,5 @@ +fn main() { + // Allow the Python symbols to be resolved at import time on macOS, like + // maturin does. + pyo3_build_config::add_extension_module_link_args(); +}