From 541e427b8ed6da48d8b6c015c8c21bb2101e997f Mon Sep 17 00:00:00 2001 From: Bilal Elmoussaoui Date: Sat, 7 Mar 2026 22:09:09 +0100 Subject: [PATCH 1/2] ci: Fix coverage script so it includes also the integrations tests --- coverage.sh | 2 -- 1 file changed, 2 deletions(-) diff --git a/coverage.sh b/coverage.sh index 655b934d..b87075bb 100755 --- a/coverage.sh +++ b/coverage.sh @@ -5,7 +5,6 @@ echo "🧪 Generating coverage for oo7::native_crypto/tokio..." mkdir -p coverage-raw cargo tarpaulin \ --package oo7 \ - --lib \ --no-default-features \ --features "tracing,tokio,native_crypto" \ --ignore-panics \ @@ -17,7 +16,6 @@ echo "" echo "🧪 Generating coverage for oo7::openssl_crypto/tokio..." cargo tarpaulin \ --package oo7 \ - --lib \ --no-default-features \ --features "tracing,tokio,openssl_crypto" \ --ignore-panics \ From 585995657d5ef781a01c4b6a5bda1ffb06db3b37 Mon Sep 17 00:00:00 2001 From: Bilal Elmoussaoui Date: Sun, 8 Mar 2026 00:11:09 +0100 Subject: [PATCH 2/2] server: Add support of InternalUnsupportedGuiltRiddenInterface As it is used seahorse, so support it till we have a better way of handling this in the spec itself. --- client/src/dbus/api/secret.rs | 16 +- server/src/gnome/internal.rs | 503 ++++++++++++++++++++++++++++++++++ server/src/gnome/mod.rs | 1 + server/src/gnome/prompter.rs | 32 +++ server/src/plasma/prompter.rs | 11 + server/src/prompt/mod.rs | 29 ++ server/src/service/mod.rs | 57 +++- 7 files changed, 632 insertions(+), 17 deletions(-) create mode 100644 server/src/gnome/internal.rs diff --git a/client/src/dbus/api/secret.rs b/client/src/dbus/api/secret.rs index 49becab0..669cc8b3 100644 --- a/client/src/dbus/api/secret.rs +++ b/client/src/dbus/api/secret.rs @@ -56,10 +56,7 @@ impl DBusSecret { }) } - pub(crate) async fn from_inner( - cnx: &zbus::Connection, - inner: DBusSecretInner, - ) -> Result { + pub async fn from_inner(cnx: &zbus::Connection, inner: DBusSecretInner) -> Result { Ok(Self { session: Arc::new(Session::new(cnx, inner.0).await?), parameters: inner.1, @@ -97,6 +94,17 @@ impl DBusSecret { } } +impl From for DBusSecretInner { + fn from(secret: DBusSecret) -> Self { + Self( + secret.session().inner().path().to_owned().into(), + secret.parameters().to_vec(), + secret.value().to_vec(), + secret.content_type(), + ) + } +} + impl Serialize for DBusSecret { fn serialize(&self, serializer: S) -> std::result::Result where diff --git a/server/src/gnome/internal.rs b/server/src/gnome/internal.rs new file mode 100644 index 00000000..f924914e --- /dev/null +++ b/server/src/gnome/internal.rs @@ -0,0 +1,503 @@ +// Backward compatibility interface for GNOME Keyring. +// This allows creating/unlocking collections without user prompts. + +use oo7::{ + Secret, + dbus::{ + ServiceError, + api::{DBusSecret, DBusSecretInner, Properties}, + }, + file::Keyring, +}; +use zbus::zvariant::{ObjectPath, OwnedObjectPath, OwnedValue}; + +use crate::{ + error::custom_service_error, + prompt::{Prompt, PromptAction, PromptRole}, + service::Service, +}; + +pub const INTERNAL_INTERFACE_PATH: &str = + "/org/gnome/keyring/InternalUnsupportedGuiltRiddenInterface"; + +#[derive(Debug, Clone)] +pub struct InternalInterface { + service: Service, +} + +impl InternalInterface { + pub fn new(service: Service) -> Self { + Self { service } + } + + async fn decrypt_secret(&self, secret: DBusSecretInner) -> Result { + let session_path = &secret.0; + + let Some(session) = self.service.session(session_path).await else { + return Err(ServiceError::NoSession(format!( + "The session `{session_path}` does not exist." + ))); + }; + + let secret = DBusSecret::from_inner(self.service.connection(), secret) + .await + .map_err(|err| { + custom_service_error(&format!("Failed to create session object {err}")) + })?; + + secret + .decrypt(session.aes_key().as_ref()) + .map_err(|err| custom_service_error(&format!("Failed to decrypt secret {err}"))) + } +} + +#[zbus::interface(name = "org.gnome.keyring.InternalUnsupportedGuiltRiddenInterface")] +impl InternalInterface { + /// Create a collection with a master password without prompting the user. + #[zbus(name = "CreateWithMasterPassword")] + async fn create_with_master_password( + &self, + properties: Properties, + master: DBusSecretInner, + ) -> Result { + let label = properties.label().to_owned(); + let secret = self.decrypt_secret(master).await?; + + let collection_path = self + .service + .create_collection_with_secret(&label, "", secret) + .await?; + + tracing::info!( + "Collection `{}` created with label '{}' via InternalUnsupportedGuiltRiddenInterface", + collection_path, + label + ); + + Ok(collection_path) + } + + /// Unlock a collection with a master password. + #[zbus(name = "UnlockWithMasterPassword")] + async fn unlock_with_master_password( + &self, + collection: ObjectPath<'_>, + master: DBusSecretInner, + ) -> Result<(), ServiceError> { + let secret = self.decrypt_secret(master).await?; + + let collection_obj = self + .service + .collection_from_path(&collection) + .await + .ok_or_else(|| ServiceError::NoSuchObject(collection.to_string()))?; + + collection_obj.set_locked(false, Some(secret)).await?; + + tracing::info!( + "Collection `{}` unlocked via InternalUnsupportedGuiltRiddenInterface", + collection + ); + + Ok(()) + } + + /// Change collection password with a master password. + #[zbus(name = "ChangeWithMasterPassword")] + async fn change_with_master_password( + &self, + collection: ObjectPath<'_>, + original: DBusSecretInner, + master: DBusSecretInner, + ) -> Result<(), ServiceError> { + let original_secret = self.decrypt_secret(original).await?; + let new_secret = self.decrypt_secret(master).await?; + + let collection_obj = self + .service + .collection_from_path(&collection) + .await + .ok_or_else(|| ServiceError::NoSuchObject(collection.to_string()))?; + + collection_obj + .set_locked(false, Some(original_secret)) + .await?; + + let keyring_guard = collection_obj.keyring.read().await; + if let Some(Keyring::Unlocked(unlocked)) = keyring_guard.as_ref() { + unlocked + .change_secret(new_secret) + .await + .map_err(|err| custom_service_error(&format!("Failed to change secret: {err}")))?; + } else { + return Err(custom_service_error("Collection is not unlocked")); + } + + tracing::info!( + "Collection `{}` password changed via InternalUnsupportedGuiltRiddenInterface", + collection + ); + + Ok(()) + } + + /// Change collection password with a prompt. + #[zbus(name = "ChangeWithPrompt")] + async fn change_with_prompt( + &self, + collection: ObjectPath<'_>, + ) -> Result { + let collection_obj = self + .service + .collection_from_path(&collection) + .await + .ok_or_else(|| ServiceError::NoSuchObject(collection.to_string()))?; + + let label = collection_obj.label().await; + + let prompt = Prompt::new( + self.service.clone(), + PromptRole::ChangePassword, + label, + None, + ) + .await; + let prompt_path: OwnedObjectPath = prompt.path().to_owned().into(); + + let service = self.service.clone(); + let collection_path = collection.to_owned(); + let action = PromptAction::new(move |new_secret: Secret| { + let service = service.clone(); + let collection_path = collection_path.clone(); + async move { + let collection = service + .collection_from_path(&collection_path) + .await + .ok_or_else(|| ServiceError::NoSuchObject(collection_path.to_string()))?; + + let keyring_guard = collection.keyring.read().await; + if let Some(Keyring::Unlocked(unlocked)) = keyring_guard.as_ref() { + unlocked.change_secret(new_secret).await.map_err(|err| { + custom_service_error(&format!("Failed to change secret: {err}")) + })?; + } else { + return Err(custom_service_error( + "Collection must be unlocked to change password", + )); + } + + tracing::info!( + "Collection `{}` password changed via prompt", + collection_path + ); + + Ok(OwnedValue::from(ObjectPath::from_str_unchecked("/"))) + } + }); + + prompt.set_action(action).await; + + self.service + .object_server() + .at(prompt.path(), prompt.clone()) + .await?; + + self.service + .register_prompt(prompt_path.clone(), prompt) + .await; + + tracing::info!( + "Created password change prompt for collection `{}`", + collection + ); + + Ok(prompt_path) + } +} + +#[cfg(test)] +mod tests { + use std::sync::Arc; + + use oo7::{Secret, dbus}; + use zbus::zvariant::{ObjectPath, OwnedObjectPath}; + + use crate::tests::TestServiceSetup; + + /// Proxy for the InternalUnsupportedGuiltRiddenInterface + #[zbus::proxy( + interface = "org.gnome.keyring.InternalUnsupportedGuiltRiddenInterface", + default_service = "org.freedesktop.secrets", + default_path = "/org/gnome/keyring/InternalUnsupportedGuiltRiddenInterface", + gen_blocking = false + )] + trait InternalInterfaceProxy { + #[zbus(name = "CreateWithMasterPassword")] + fn create_with_master_password( + &self, + properties: dbus::api::Properties, + master: dbus::api::DBusSecretInner, + ) -> zbus::Result; + + #[zbus(name = "UnlockWithMasterPassword")] + fn unlock_with_master_password( + &self, + collection: &ObjectPath<'_>, + master: dbus::api::DBusSecretInner, + ) -> zbus::Result<()>; + + #[zbus(name = "ChangeWithMasterPassword")] + fn change_with_master_password( + &self, + collection: &ObjectPath<'_>, + original: dbus::api::DBusSecretInner, + master: dbus::api::DBusSecretInner, + ) -> zbus::Result<()>; + + #[zbus(name = "ChangeWithPrompt")] + fn change_with_prompt(&self, collection: &ObjectPath<'_>) -> zbus::Result; + } + + #[tokio::test] + async fn test_create_with_master_password() -> Result<(), Box> { + let setup = TestServiceSetup::encrypted_session(false).await?; + + // Create proxy to the InternalInterface + let internal_proxy = InternalInterfaceProxyProxy::builder(&setup.client_conn) + .build() + .await?; + + // Prepare properties for collection creation + let label = "TestCollection"; + let properties = oo7::dbus::api::Properties::for_collection(label); + + // Prepare the master password secret + let master_secret = Secret::text("my-master-password"); + let aes_key = setup.aes_key.as_ref().unwrap(); + let dbus_secret = oo7::dbus::api::DBusSecret::new_encrypted( + Arc::clone(&setup.session), + master_secret, + aes_key, + )?; + let dbus_secret_inner = dbus_secret.into(); + + // Call CreateWithMasterPassword via D-Bus + let collection_path = internal_proxy + .create_with_master_password(properties, dbus_secret_inner) + .await?; + + // Verify the collection was created + assert!( + !collection_path.as_str().is_empty(), + "Collection path should not be empty" + ); + + // Verify we can access the newly created collection via D-Bus + let collection = + oo7::dbus::api::Collection::new(&setup.client_conn, collection_path.clone()).await?; + let label = collection.label().await?; + assert_eq!( + label, "TestCollection", + "Collection should have the correct label" + ); + + Ok(()) + } + + #[tokio::test] + async fn test_unlock_with_master_password() -> Result<(), Box> { + let setup = TestServiceSetup::encrypted_session(true).await?; + let internal_proxy = InternalInterfaceProxyProxy::builder(&setup.client_conn) + .build() + .await?; + + // Get the default collection + let default_collection = setup.default_collection().await?; + let collection_path: zbus::zvariant::OwnedObjectPath = + default_collection.inner().path().to_owned().into(); + + // Lock the collection + setup + .service_api + .lock(&[collection_path.clone()], None) + .await?; + + // Verify it's locked + assert!( + default_collection.is_locked().await?, + "Collection should be locked" + ); + + // Prepare the unlock secret (use the keyring secret) + let unlock_secret = setup.keyring_secret.clone().unwrap(); + let aes_key = setup.aes_key.as_ref().unwrap(); + let dbus_secret = oo7::dbus::api::DBusSecret::new_encrypted( + Arc::clone(&setup.session), + unlock_secret, + aes_key, + )?; + let dbus_secret_inner = dbus_secret.into(); + + // Call UnlockWithMasterPassword via D-Bus + internal_proxy + .unlock_with_master_password(&collection_path.as_ref(), dbus_secret_inner) + .await?; + + // Verify it's unlocked + assert!( + !default_collection.is_locked().await?, + "Collection should be unlocked" + ); + + Ok(()) + } + + #[tokio::test] + async fn test_change_with_master_password() -> Result<(), Box> { + let setup = TestServiceSetup::encrypted_session(true).await?; + let internal_proxy = InternalInterfaceProxyProxy::builder(&setup.client_conn) + .build() + .await?; + + let default_collection = setup.default_collection().await?; + let collection_path: zbus::zvariant::OwnedObjectPath = + default_collection.inner().path().to_owned().into(); + + // Prepare original and new secrets + let original_secret = setup.keyring_secret.clone().unwrap(); + let new_secret = Secret::text("new-master-password"); + + let aes_key = setup.aes_key.as_ref().unwrap(); + let original_dbus = dbus::api::DBusSecret::new_encrypted( + Arc::clone(&setup.session), + original_secret, + aes_key, + )?; + let new_dbus = dbus::api::DBusSecret::new_encrypted( + Arc::clone(&setup.session), + new_secret.clone(), + aes_key, + )?; + + // Call ChangeWithMasterPassword via D-Bus + internal_proxy + .change_with_master_password( + &collection_path.as_ref(), + original_dbus.into(), + new_dbus.into(), + ) + .await?; + + // Verify the password was changed by locking and unlocking with new password + setup + .service_api + .lock(&[collection_path.clone()], None) + .await?; + assert!( + default_collection.is_locked().await?, + "Collection should be locked" + ); + + // Unlock with new password via D-Bus + let unlock_dbus = + dbus::api::DBusSecret::new_encrypted(Arc::clone(&setup.session), new_secret, aes_key)?; + internal_proxy + .unlock_with_master_password(&collection_path.as_ref(), unlock_dbus.into()) + .await?; + + assert!( + !default_collection.is_locked().await?, + "Collection should be unlocked with new password" + ); + + Ok(()) + } + + #[tokio::test] + async fn test_change_with_prompt() -> Result<(), Box> { + let setup = TestServiceSetup::encrypted_session(true).await?; + let internal_proxy = InternalInterfaceProxyProxy::builder(&setup.client_conn) + .build() + .await?; + + let default_collection = setup.default_collection().await?; + let collection_path: zbus::zvariant::OwnedObjectPath = + default_collection.inner().path().to_owned().into(); + + // Call ChangeWithPrompt via D-Bus + let prompt_path = internal_proxy + .change_with_prompt(&collection_path.as_ref()) + .await?; + + // Verify prompt was created + assert!( + !prompt_path.as_str().is_empty(), + "Prompt path should not be empty" + ); + + // Verify the prompt exists and is accessible via D-Bus + let _prompt_proxy = dbus::api::Prompt::new(&setup.client_conn, prompt_path).await?; + + Ok(()) + } + + #[tokio::test] + async fn test_unlock_with_wrong_password() -> Result<(), Box> { + let setup = TestServiceSetup::encrypted_session(true).await?; + let internal_proxy = InternalInterfaceProxyProxy::builder(&setup.client_conn) + .build() + .await?; + + let default_collection = setup.default_collection().await?; + let collection_path: zbus::zvariant::OwnedObjectPath = + default_collection.inner().path().to_owned().into(); + + // Create an item first so that the unlock validation has something to validate + let aes_key = setup.aes_key.as_ref().unwrap(); + let item_secret = Secret::text("item-secret"); + let dbus_secret = + dbus::api::DBusSecret::new_encrypted(Arc::clone(&setup.session), item_secret, aes_key)?; + + let mut attributes = std::collections::HashMap::new(); + attributes.insert("test".to_string(), "value".to_string()); + + default_collection + .create_item("Test Item", &attributes, &dbus_secret, false, None) + .await?; + + // Lock the collection + setup + .service_api + .lock(&[collection_path.clone()], None) + .await?; + + // Verify it's locked before attempting unlock + assert!( + default_collection.is_locked().await?, + "Collection should be locked before unlock attempt" + ); + + // Try to unlock with wrong password via D-Bus + let wrong_secret = Secret::text("wrong-password"); + let wrong_dbus_secret = dbus::api::DBusSecret::new_encrypted( + Arc::clone(&setup.session), + wrong_secret, + aes_key, + )?; + + let result = internal_proxy + .unlock_with_master_password(&collection_path.as_ref(), wrong_dbus_secret.into()) + .await; + + // Should fail + assert!(result.is_err(), "Unlocking with wrong password should fail"); + + // Collection should remain locked + assert!( + default_collection.is_locked().await?, + "Collection should remain locked after failed unlock" + ); + + Ok(()) + } +} diff --git a/server/src/gnome/mod.rs b/server/src/gnome/mod.rs index 3e6f1f26..a1a1d87a 100644 --- a/server/src/gnome/mod.rs +++ b/server/src/gnome/mod.rs @@ -1,3 +1,4 @@ pub mod crypto; +pub mod internal; pub mod prompter; pub mod secret_exchange; diff --git a/server/src/gnome/prompter.rs b/server/src/gnome/prompter.rs index 7f8b8cd9..20103b53 100644 --- a/server/src/gnome/prompter.rs +++ b/server/src/gnome/prompter.rs @@ -127,6 +127,28 @@ pub struct Properties { } impl Properties { + fn for_change_password(keyring: &str, window_id: Option<&WindowIdentifierType>) -> Self { + Self { + title: Some(gettext("Change Keyring Password")), + message: Some(gettext("Authentication required")), + description: Some( + formatx!( + gettext("An application wants to change the keyring password '{}'"), + keyring, + ) + .expect("Wrong format in translatable string"), + ), + warning: Some(gettext("This operation cannot be reverted")), + password_new: None, + password_strength: None, + choice_label: None, + choice_chosen: None, + caller_window: window_id.map(ToOwned::to_owned), + continue_label: Some(gettext("Continue")), + cancel_label: Some(gettext("Cancel")), + } + } + fn for_unlock( keyring: &str, warning: Option<&str>, @@ -334,6 +356,10 @@ impl PrompterCallback { Properties::for_create_collection(label, self.window_id.as_ref()), PromptType::Password, ), + PromptRole::ChangePassword => ( + Properties::for_change_password(label, self.window_id.as_ref()), + PromptType::Password, + ), }; let prompter = PrompterProxy::new(connection).await?; @@ -395,6 +421,12 @@ impl PrompterCallback { PromptRole::CreateCollection => { prompt.on_create_collection(secret).await?; + let path = self.path.clone(); + tokio::spawn(async move { prompter.stop_prompting(&path).await }); + } + PromptRole::ChangePassword => { + prompt.on_change_password(secret).await?; + let path = self.path.clone(); tokio::spawn(async move { prompter.stop_prompting(&path).await }); } diff --git a/server/src/plasma/prompter.rs b/server/src/plasma/prompter.rs index b5b0fa36..576c21c1 100644 --- a/server/src/plasma/prompter.rs +++ b/server/src/plasma/prompter.rs @@ -202,6 +202,13 @@ impl PlasmaPrompterCallback { .await }); } + PromptRole::ChangePassword => { + tokio::spawn(async move { + prompter + .unlock_collection_prompt(&path, &window_id, "", collection_name.as_str()) + .await + }); + } } Ok(()) @@ -236,6 +243,10 @@ impl PlasmaPrompterCallback { prompt.on_create_collection(secret).await?; Ok(CallbackAction::Dismiss) } + PromptRole::ChangePassword => { + prompt.on_change_password(secret).await?; + Ok(CallbackAction::Dismiss) + } } } diff --git a/server/src/prompt/mod.rs b/server/src/prompt/mod.rs index 5ed9837b..c08a80d3 100644 --- a/server/src/prompt/mod.rs +++ b/server/src/prompt/mod.rs @@ -20,6 +20,7 @@ use crate::{error::custom_service_error, service::Service}; pub enum PromptRole { Unlock, CreateCollection, + ChangePassword, } /// A boxed future that represents the action to be taken when a prompt @@ -323,6 +324,34 @@ impl Prompt { ))), } } + + pub async fn on_change_password(&self, secret: Secret) -> Result<(), ServiceError> { + debug_assert_eq!(self.role, PromptRole::ChangePassword); + + let Some(action) = self.take_action().await else { + return Err(custom_service_error( + "Prompt action was already executed or not set", + )); + }; + + // Execute the change password action with the new secret + match action.execute(secret).await { + Ok(result) => { + tracing::info!("ChangePassword action completed successfully"); + + let signal_emitter = self.service.signal_emitter(self.path().to_owned())?; + + tokio::spawn(async move { + tracing::debug!("ChangePassword prompt completed."); + let _ = Prompt::completed(&signal_emitter, false, result).await; + }); + Ok(()) + } + Err(err) => Err(custom_service_error(&format!( + "Failed to change password: {err}." + ))), + } + } } #[cfg(test)] diff --git a/server/src/service/mod.rs b/server/src/service/mod.rs index 2ab4583a..bfbe16db 100644 --- a/server/src/service/mod.rs +++ b/server/src/service/mod.rs @@ -22,6 +22,8 @@ use zbus::{ zvariant::{ObjectPath, Optional, OwnedObjectPath, OwnedValue, Value}, }; +#[cfg(any(feature = "gnome_native_crypto", feature = "gnome_openssl_crypto"))] +pub use crate::gnome::internal::{INTERNAL_INTERFACE_PATH, InternalInterface}; use crate::{ collection::Collection, error::{Error, custom_service_error}, @@ -403,6 +405,15 @@ impl Service { .build() .await?; + #[cfg(any(feature = "gnome_native_crypto", feature = "gnome_openssl_crypto"))] + connection + .object_server() + .at( + INTERNAL_INTERFACE_PATH, + InternalInterface::new(service.clone()), + ) + .await?; + // Discover existing keyrings let discovered_keyrings = service.discover_keyrings(secret).await?; @@ -438,6 +449,15 @@ impl Service { ) .await?; + #[cfg(any(feature = "gnome_native_crypto", feature = "gnome_openssl_crypto"))] + connection + .object_server() + .at( + INTERNAL_INTERFACE_PATH, + InternalInterface::new(service.clone()), + ) + .await?; + let default_keyring = if let Some(secret) = secret { vec![( "Login".to_owned(), @@ -903,18 +923,12 @@ impl Service { .cloned() } - pub async fn complete_collection_creation( + pub async fn create_collection_with_secret( &self, - prompt_path: &ObjectPath<'_>, + label: &str, + alias: &str, secret: Secret, ) -> Result { - // Retrieve the pending collection metadata - let Some((label, alias)) = self.pending_collection(prompt_path).await else { - return Err(ServiceError::NoSuchObject(format!( - "No pending collection for prompt `{prompt_path}`" - ))); - }; - // Create a persistent keyring with the provided secret let keyring = UnlockedKeyring::open(&label.to_lowercase(), secret) .await @@ -929,7 +943,7 @@ impl Service { let keyring = Keyring::Unlocked(keyring); // Create the collection - let collection = Collection::new(&label, &alias, self.clone(), keyring).await; + let collection = Collection::new(label, alias, self.clone(), keyring).await; let collection_path: OwnedObjectPath = collection.path().to_owned().into(); // Register with object server @@ -943,9 +957,6 @@ impl Service { .await .insert(collection_path.clone(), collection); - // Clean up pending collection - self.pending_collections.lock().await.remove(prompt_path); - // Emit CollectionCreated signal let service_path = oo7::dbus::api::Service::PATH.as_ref().unwrap(); let signal_emitter = self.signal_emitter(service_path)?; @@ -963,6 +974,26 @@ impl Service { Ok(collection_path) } + pub async fn complete_collection_creation( + &self, + prompt_path: &ObjectPath<'_>, + secret: Secret, + ) -> Result { + let Some((label, alias)) = self.pending_collection(prompt_path).await else { + return Err(ServiceError::NoSuchObject(format!( + "No pending collection for prompt `{prompt_path}`" + ))); + }; + + let collection_path = self + .create_collection_with_secret(&label, &alias, secret) + .await?; + + self.pending_collections.lock().await.remove(prompt_path); + + Ok(collection_path) + } + pub fn signal_emitter<'a, P>( &self, path: P,