diff --git a/client/src/dbus/api/secret.rs b/client/src/dbus/api/secret.rs index 669cc8b30..ea473923a 100644 --- a/client/src/dbus/api/secret.rs +++ b/client/src/dbus/api/secret.rs @@ -7,14 +7,14 @@ use zeroize::{Zeroize, ZeroizeOnDrop}; use super::Session; use crate::{Key, Secret, crypto, dbus::Error, secret::ContentType}; -#[derive(Debug, Serialize, Deserialize, Type)] +#[derive(Debug, Serialize, Deserialize, Type, Zeroize, ZeroizeOnDrop)] #[zvariant(signature = "(oayays)")] /// Same as [`DBusSecret`] without tying the session path to a [`Session`] type. pub struct DBusSecretInner( - pub OwnedObjectPath, + #[zeroize(skip)] pub OwnedObjectPath, #[serde(with = "serde_bytes")] pub Vec, #[serde(with = "serde_bytes")] pub Vec, - pub ContentType, + #[zeroize(skip)] pub ContentType, ); #[derive(Debug, Type, Zeroize, ZeroizeOnDrop)] @@ -58,9 +58,9 @@ impl DBusSecret { 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, - value: inner.2, + session: Arc::new(Session::new(cnx, inner.0.clone()).await?), + parameters: inner.1.clone(), + value: inner.2.clone(), content_type: inner.3, }) } diff --git a/client/src/file/api/encrypted_item.rs b/client/src/file/api/encrypted_item.rs index f235ffd27..17ef9efe6 100644 --- a/client/src/file/api/encrypted_item.rs +++ b/client/src/file/api/encrypted_item.rs @@ -2,12 +2,14 @@ use std::collections::HashMap; use serde::{Deserialize, Serialize}; use zbus::zvariant::Type; +use zeroize::{Zeroize, ZeroizeOnDrop}; use super::{Error, UnlockedItem}; use crate::{Key, Mac, crypto}; -#[derive(Deserialize, Serialize, Type, Debug, Clone)] +#[derive(Deserialize, Serialize, Type, Debug, Clone, Zeroize, ZeroizeOnDrop)] pub(crate) struct EncryptedItem { + #[zeroize(skip)] pub(crate) hashed_attributes: HashMap, #[serde(with = "serde_bytes")] pub(crate) blob: Vec, diff --git a/client/src/file/api/legacy_keyring.rs b/client/src/file/api/legacy_keyring.rs index c0f9dd9de..c458a75a9 100644 --- a/client/src/file/api/legacy_keyring.rs +++ b/client/src/file/api/legacy_keyring.rs @@ -6,6 +6,7 @@ use std::{ }; use endi::{Endian, ReadBytes}; +use zeroize::{Zeroize, ZeroizeOnDrop}; use super::{Secret, UnlockedItem}; use crate::{ @@ -19,7 +20,7 @@ const FILE_HEADER_LEN: usize = FILE_HEADER.len(); pub const MAJOR_VERSION: u8 = 0; pub const MINOR_VERSION: u8 = 0; -#[derive(Debug)] +#[derive(Debug, Zeroize, ZeroizeOnDrop)] pub struct Keyring { salt: Vec, iteration_count: u32, diff --git a/client/src/file/api/mod.rs b/client/src/file/api/mod.rs index 86c2fd29d..6e1770674 100644 --- a/client/src/file/api/mod.rs +++ b/client/src/file/api/mod.rs @@ -44,6 +44,7 @@ mod legacy_keyring; pub(super) use encrypted_item::EncryptedItem; pub(super) use legacy_keyring::{Keyring as LegacyKeyring, MAJOR_VERSION as LEGACY_MAJOR_VERSION}; +use zeroize::{Zeroize, ZeroizeOnDrop}; use crate::{ AsAttributes, Key, Secret, crypto, @@ -67,7 +68,7 @@ pub(crate) static GVARIANT_ENCODING: LazyLock = LazyLock::new(|| Context::new_gvariant(Endian::Little, 0)); /// Logical contents of a keyring file -#[derive(Deserialize, Serialize, Type, Debug)] +#[derive(Deserialize, Serialize, Type, Debug, Zeroize, ZeroizeOnDrop)] pub struct Keyring { salt_size: u32, #[serde(with = "serde_bytes")] @@ -75,6 +76,7 @@ pub struct Keyring { iteration_count: u32, modified_time: u64, usage_count: u32, + #[zeroize(skip)] pub(in crate::file) items: Vec, } diff --git a/client/src/mac.rs b/client/src/mac.rs index ebd7fb06d..76f15784f 100644 --- a/client/src/mac.rs +++ b/client/src/mac.rs @@ -2,11 +2,12 @@ use serde::{Deserialize, Serialize}; #[cfg(feature = "native_crypto")] use subtle::ConstantTimeEq; use zbus::zvariant::Type; +use zeroize::{Zeroize, ZeroizeOnDrop}; // There is no constructor to avoid performing sanity checks, e.g. length. /// A message authentication code. It provides constant-time comparison when /// compared against another mac or against a slice of bytes. -#[derive(Deserialize, Serialize, Type, Clone)] +#[derive(Deserialize, Serialize, Type, Clone, Zeroize, ZeroizeOnDrop)] pub struct Mac(#[serde(with = "serde_bytes")] Vec); impl std::fmt::Debug for Mac { diff --git a/server/src/capability.rs b/server/src/capability.rs index 3f7e68ea7..8f20db365 100644 --- a/server/src/capability.rs +++ b/server/src/capability.rs @@ -58,7 +58,7 @@ fn set_bounding_set(caps: CapabilitySet) -> Result<(), rustix::io::Errno> { Ok(()) } -#[derive(Debug, PartialEq)] +#[derive(PartialEq)] enum CapabilityState { Full, // setuid root or root user Partial, // filesystem-based capabilities diff --git a/server/src/collection/mod.rs b/server/src/collection/mod.rs index 936e7a32e..afb07c135 100644 --- a/server/src/collection/mod.rs +++ b/server/src/collection/mod.rs @@ -24,7 +24,7 @@ use crate::{ item, }; -#[derive(Debug, Clone)] +#[derive(Clone)] pub struct Collection { // Properties items: Arc>>, @@ -225,12 +225,12 @@ impl Collection { let keyring = self.keyring.read().await; let keyring = keyring.as_ref().unwrap().as_unlocked(); - let DBusSecretInner(session_path, iv, secret_bytes, content_type) = secret; + let DBusSecretInner(ref session_path, ref iv, ref secret_bytes, ref content_type) = secret; let label = properties.label(); // Safe to unwrap as an item always has attributes let mut attributes = properties.attributes().unwrap().to_owned(); - let Some(session) = self.service.session(&session_path).await else { + let Some(session) = self.service.session(session_path).await else { tracing::error!("The session `{}` does not exist.", session_path); return Err(ServiceError::NoSession(format!( "The session `{session_path}` does not exist." @@ -238,9 +238,9 @@ impl Collection { }; let secret = match session.aes_key() { - Some(key) => oo7::crypto::decrypt(secret_bytes, &key, &iv) + Some(key) => oo7::crypto::decrypt(secret_bytes, &key, iv) .map_err(|err| custom_service_error(&format!("Failed to decrypt secret {err}.")))?, - None => zeroize::Zeroizing::new(secret_bytes), + None => zeroize::Zeroizing::new(secret_bytes.clone()), }; // Ensure content-type attribute is stored diff --git a/server/src/gnome/internal.rs b/server/src/gnome/internal.rs index 5c4d26d52..9528e4a21 100644 --- a/server/src/gnome/internal.rs +++ b/server/src/gnome/internal.rs @@ -20,7 +20,7 @@ use crate::{ pub const INTERNAL_INTERFACE_PATH: &str = "/org/gnome/keyring/InternalUnsupportedGuiltRiddenInterface"; -#[derive(Debug, Clone)] +#[derive(Clone)] pub struct InternalInterface { service: Service, } diff --git a/server/src/gnome/prompter.rs b/server/src/gnome/prompter.rs index 4d034b380..9df238d2a 100644 --- a/server/src/gnome/prompter.rs +++ b/server/src/gnome/prompter.rs @@ -43,7 +43,7 @@ mod double_value_optional { } } -#[derive(Debug, Serialize, Deserialize, Type, Default)] +#[derive(Serialize, Deserialize, Type, Default)] #[zvariant(signature = "dict")] #[serde(rename_all = "kebab-case")] // GcrPrompt properties @@ -226,7 +226,7 @@ impl TryFrom for Reply { } } -#[derive(Deserialize, Serialize, Debug, Type, PartialEq, Eq, PartialOrd, Ord)] +#[derive(Debug, Deserialize, Serialize, Type, PartialEq, Eq, PartialOrd, Ord)] #[serde(rename_all = "lowercase")] #[zvariant(signature = "s")] pub enum PromptType { @@ -254,7 +254,7 @@ pub trait GNOMEPrompter { fn stop_prompting(&self, callback: &ObjectPath<'_>) -> Result<(), ServiceError>; } -#[derive(Debug, Clone)] +#[derive(Clone)] pub struct GNOMEPrompterCallback { window_id: Option, private_key: Arc, diff --git a/server/src/item/mod.rs b/server/src/item/mod.rs index eefdd584b..729558b11 100644 --- a/server/src/item/mod.rs +++ b/server/src/item/mod.rs @@ -8,7 +8,7 @@ use zbus::zvariant::{ObjectPath, OwnedObjectPath}; use crate::{Service, collection::Collection, error::custom_service_error}; -#[derive(Debug, Clone)] +#[derive(Clone)] pub struct Item { // Properties pub(super) inner: Arc>>, @@ -139,9 +139,9 @@ impl Item { } pub async fn set_secret(&self, secret: DBusSecretInner) -> Result<(), ServiceError> { - let DBusSecretInner(session, iv, secret, content_type) = secret; + let DBusSecretInner(ref session, ref iv, ref secret, ref content_type) = secret; - let Some(session) = self.service.session(&session).await else { + let Some(session) = self.service.session(session).await else { tracing::error!("The session `{}` does not exist.", session); return Err(ServiceError::NoSession(format!( "The session `{session}` does not exist." @@ -162,7 +162,7 @@ impl Item { match session.aes_key() { Some(key) => { - let decrypted = oo7::crypto::decrypt(secret, &key, &iv).map_err(|err| { + let decrypted = oo7::crypto::decrypt(secret, &key, iv).map_err(|err| { custom_service_error(&format!("Failed to decrypt secret {err}.")) })?; inner.as_mut_unlocked().set_secret(decrypted); diff --git a/server/src/migration.rs b/server/src/migration.rs index cbb933186..30491260f 100644 --- a/server/src/migration.rs +++ b/server/src/migration.rs @@ -7,7 +7,7 @@ use oo7::{Secret, file::UnlockedKeyring}; use crate::error::Error; /// Pending keyring migration -#[derive(Clone, Debug)] +#[derive(Clone)] pub enum PendingMigration { /// Legacy v0 keyring format V0 { diff --git a/server/src/pam_listener/mod.rs b/server/src/pam_listener/mod.rs index 0e5adce3b..59529e9e9 100644 --- a/server/src/pam_listener/mod.rs +++ b/server/src/pam_listener/mod.rs @@ -25,7 +25,7 @@ enum PamOperation { ChangePassword = 1, } -#[derive(Debug, Serialize, Deserialize, Type, Zeroize, ZeroizeOnDrop)] +#[derive(Serialize, Deserialize, Type, Zeroize, ZeroizeOnDrop)] struct PamMessage { #[zeroize(skip)] operation: PamOperation, diff --git a/server/src/plasma/prompter.rs b/server/src/plasma/prompter.rs index 1ac56ebdc..d50d8cfdd 100644 --- a/server/src/plasma/prompter.rs +++ b/server/src/plasma/prompter.rs @@ -19,7 +19,7 @@ use crate::{ }; #[repr(i32)] -#[derive(Debug, Type, Serialize)] +#[derive(Type, Serialize)] pub enum CallbackAction { Dismiss = 0, Keep = 1, @@ -77,7 +77,7 @@ pub trait PlasmaPrompter { ) -> Result<(), ServiceError>; } -#[derive(Debug, Clone)] +#[derive(Clone)] pub struct PlasmaPrompterCallback { service: Service, prompt_path: OwnedObjectPath, diff --git a/server/src/prompt/mod.rs b/server/src/prompt/mod.rs index c801bd87d..ae0f38459 100644 --- a/server/src/prompt/mod.rs +++ b/server/src/prompt/mod.rs @@ -75,19 +75,6 @@ pub struct Prompt { action: Arc>>, } -// Manual impl because OnceCell doesn't impl Debug -impl std::fmt::Debug for Prompt { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - f.debug_struct("Prompt") - .field("service", &self.service) - .field("role", &self.role) - .field("path", &self.path) - .field("label", &self.label) - .field("collection", &self.collection) - .finish() - } -} - #[cfg(any( feature = "gnome_openssl_crypto", feature = "gnome_native_crypto", @@ -114,9 +101,9 @@ impl Prompt { PlasmaPrompterCallback::new(self.service.clone(), self.path.clone()).await; let path = OwnedObjectPath::from(callback.path().clone()); - self.plasma_callback - .set(callback.clone()) - .expect("A prompt callback is only set once"); + // We are sure the callback is not set at this point, so it is fine to ignore + // the result of set + let _ = self.plasma_callback.set(callback.clone()); self.service .object_server() .at(&path, callback.clone()) @@ -147,10 +134,9 @@ impl Prompt { let path = OwnedObjectPath::from(callback.path().clone()); - self.gnome_callback - .set(callback.clone()) - .expect("A prompt callback is only set once"); - + // We are sure the callback is not set at this point, so it is fine to ignore + // the result of set + let _ = self.gnome_callback.set(callback.clone()); self.service.object_server().at(&path, callback).await?; tracing::debug!("Prompt `{}` created.", self.path); diff --git a/server/src/service/mod.rs b/server/src/service/mod.rs index 30c9eee10..65098e4bf 100644 --- a/server/src/service/mod.rs +++ b/server/src/service/mod.rs @@ -38,14 +38,14 @@ const DEFAULT_COLLECTION_ALIAS_PATH: ObjectPath<'static> = ObjectPath::from_static_str_unchecked("/org/freedesktop/secrets/aliases/default"); /// Prompter type -#[derive(Debug, Clone, Copy, PartialEq, Eq)] +#[derive(Clone, Copy, PartialEq, Eq)] pub enum PrompterType { #[allow(clippy::upper_case_acronyms)] GNOME, Plasma, } -#[derive(Debug, Clone)] +#[derive(Clone)] pub struct Service { // Properties pub(crate) collections: Arc>>, diff --git a/server/src/session.rs b/server/src/session.rs index 7db909162..1f4b8c7ec 100644 --- a/server/src/session.rs +++ b/server/src/session.rs @@ -11,7 +11,7 @@ use zbus::{ use crate::Service; -#[derive(Debug, Clone)] +#[derive(Clone)] pub struct Session { aes_key: Option>, service: Service, diff --git a/server/src/tests.rs b/server/src/tests.rs index 8d7372445..459843b90 100644 --- a/server/src/tests.rs +++ b/server/src/tests.rs @@ -401,7 +401,7 @@ impl TestServiceSetup { /// This simulates the GNOME System Prompter for testing without requiring /// the actual GNOME keyring prompter service to be running. #[cfg(any(feature = "gnome_native_crypto", feature = "gnome_openssl_crypto"))] -#[derive(Debug, Clone)] +#[derive(Clone)] pub(crate) struct MockPrompterService { /// The password to use for unlock prompts (simulates user input) unlock_password: Arc>>, @@ -634,7 +634,7 @@ impl MockPrompterService { /// This simulates the Plasma System Prompter for testing without requiring /// the actual service to be running. #[cfg(any(feature = "plasma_native_crypto", feature = "plasma_openssl_crypto"))] -#[derive(Debug, Clone)] +#[derive(Clone)] pub(crate) struct MockPrompterServicePlasma { /// The password to use for unlock prompts (simulates user input) unlock_password: Arc>>,