From f4d1a462a7809e7abf2e5e731b07eb482e89d421 Mon Sep 17 00:00:00 2001 From: Bilal Elmoussaoui Date: Fri, 26 Jun 2026 21:41:08 +0200 Subject: [PATCH 1/4] client: Allow using None for secret/key to create an not encrypted keyring Similar to what gnome-keyring does when the user sets the password to '' --- client/src/file/api/encrypted_item.rs | 36 ++++++++++-- client/src/file/api/mod.rs | 79 ++++++++++----------------- client/src/file/locked_item.rs | 2 +- client/src/file/locked_keyring.rs | 4 +- client/src/file/mod.rs | 14 +---- client/src/file/unlocked_item.rs | 35 ++++++++---- client/src/file/unlocked_keyring.rs | 73 ++++++++++++++++--------- server/src/collection/mod.rs | 8 ++- 8 files changed, 141 insertions(+), 110 deletions(-) diff --git a/client/src/file/api/encrypted_item.rs b/client/src/file/api/encrypted_item.rs index 17ef9efe6..f34fcf992 100644 --- a/client/src/file/api/encrypted_item.rs +++ b/client/src/file/api/encrypted_item.rs @@ -5,7 +5,7 @@ use zbus::zvariant::Type; use zeroize::{Zeroize, ZeroizeOnDrop}; use super::{Error, UnlockedItem}; -use crate::{Key, Mac, crypto}; +use crate::{AsAttributes, Key, Mac, crypto}; #[derive(Deserialize, Serialize, Type, Debug, Clone, Zeroize, ZeroizeOnDrop)] pub(crate) struct EncryptedItem { @@ -20,7 +20,35 @@ impl EncryptedItem { self.hashed_attributes.get(key) == Some(value_mac) } - fn try_decrypt_inner(&self, key: &Key) -> Result { + fn has_plaintext_attribute(&self, key: &str, value: &str) -> bool { + self.hashed_attributes + .get(key) + .is_some_and(|mac| mac.as_slice() == value.as_bytes()) + } + + pub fn matches(&self, attributes: &impl AsAttributes, key: Option<&Key>) -> bool { + match key { + Some(key) => { + let hashed = attributes.hash(key); + hashed + .iter() + .all(|(k, v)| v.as_ref().is_ok_and(|v| self.has_attribute(k.as_str(), v))) + } + None => attributes + .as_attributes() + .iter() + .all(|(k, v)| self.has_plaintext_attribute(k.as_str(), v.as_str())), + } + } + + fn try_decrypt_inner(&self, key: Option<&Key>) -> Result { + match key { + Some(key) => self.try_decrypt_encrypted(key), + None => UnlockedItem::try_from(self.blob.as_slice()), + } + } + + fn try_decrypt_encrypted(&self, key: &Key) -> Result { let n = self.blob.len(); let n_mac = crypto::mac_len(); let n_iv = crypto::iv_len(); @@ -45,11 +73,11 @@ impl EncryptedItem { Ok(item) } - pub fn is_valid(&self, key: &Key) -> bool { + pub fn is_valid(&self, key: Option<&Key>) -> bool { self.try_decrypt_inner(key).is_ok() } - pub fn decrypt(self, key: &Key) -> Result { + pub fn decrypt(self, key: Option<&Key>) -> Result { self.try_decrypt_inner(key) } diff --git a/client/src/file/api/mod.rs b/client/src/file/api/mod.rs index 6e1770674..618ecc191 100644 --- a/client/src/file/api/mod.rs +++ b/client/src/file/api/mod.rs @@ -190,17 +190,11 @@ impl Keyring { pub fn search_items( &self, attributes: &impl AsAttributes, - key: &Key, + key: Option<&Key>, ) -> Result, Error> { - let hashed_search = attributes.hash(key); - self.items .iter() - .filter(|e| { - hashed_search - .iter() - .all(|(k, v)| v.as_ref().is_ok_and(|v| e.has_attribute(k.as_str(), v))) - }) + .filter(|e| e.matches(attributes, key)) .map(|e| (*e).clone().decrypt(key)) .collect() } @@ -208,53 +202,35 @@ impl Keyring { pub fn lookup_item( &self, attributes: &impl AsAttributes, - key: &Key, + key: Option<&Key>, ) -> Result, Error> { - let hashed_search = attributes.hash(key); - self.items .iter() - .find(|e| { - hashed_search - .iter() - .all(|(k, v)| v.as_ref().is_ok_and(|v| e.has_attribute(k.as_str(), v))) - }) + .find(|e| e.matches(attributes, key)) .map(|e| (*e).clone().decrypt(key)) .transpose() } - pub fn lookup_item_index(&self, attributes: &impl AsAttributes, key: &Key) -> Option { - let hashed_search = attributes.hash(key); - - self.items.iter().position(|e| { - hashed_search - .iter() - .all(|(k, v)| v.as_ref().is_ok_and(|v| e.has_attribute(k.as_str(), v))) - }) + pub fn lookup_item_index( + &self, + attributes: &impl AsAttributes, + key: Option<&Key>, + ) -> Option { + self.items.iter().position(|e| e.matches(attributes, key)) } - pub fn remove_items(&mut self, attributes: &impl AsAttributes, key: &Key) -> Result<(), Error> { - let hashed_search = attributes.hash(key); - - // Validate items to be removed before actually removing them + pub fn remove_items( + &mut self, + attributes: &impl AsAttributes, + key: Option<&Key>, + ) -> Result<(), Error> { for item in &self.items { - if hashed_search - .iter() - .all(|(k, v)| v.as_ref().is_ok_and(|v| item.has_attribute(k.as_str(), v))) - { - // Validate by checking if it can be decrypted - if !item.is_valid(key) { - return Err(Error::MacError); - } + if item.matches(attributes, key) && !item.is_valid(key) { + return Err(Error::MacError); } } - // Remove matching items - self.items.retain(|e| { - !hashed_search - .iter() - .all(|(k, v)| v.as_ref().is_ok_and(|v| e.has_attribute(k.as_str(), v))) - }); + self.items.retain(|e| !e.matches(attributes, key)); Ok(()) } @@ -328,7 +304,7 @@ impl Keyring { // Check if at least one item can be decrypted with this key // We only need to check one item to validate the password - Ok(self.items.iter().any(|item| item.is_valid(&key))) + Ok(self.items.iter().any(|item| item.is_valid(Some(&key)))) } /// Get the modification timestamp @@ -405,15 +381,15 @@ mod tests { let mut keyring = Keyring::new()?; let key = keyring.derive_key(&SECRET.to_vec().into())?; - keyring - .items - .push(UnlockedItem::new("Label", needle, Secret::blob("MyPassword")).encrypt(&key)?); + keyring.items.push( + UnlockedItem::new("Label", needle, Secret::blob("MyPassword")).encrypt(Some(&key))?, + ); - assert_eq!(keyring.search_items(needle, &key)?.len(), 1); + assert_eq!(keyring.search_items(needle, Some(&key))?.len(), 1); - keyring.remove_items(needle, &key)?; + keyring.remove_items(needle, Some(&key))?; - assert_eq!(keyring.search_items(needle, &key)?.len(), 0); + assert_eq!(keyring.search_items(needle, Some(&key))?.len(), 0); Ok(()) } @@ -427,14 +403,15 @@ mod tests { new_keyring.items.push( UnlockedItem::new("My Label", &[("my-tag", "my tag value")], "A Password") - .encrypt(&key)?, + .encrypt(Some(&key))?, ); new_keyring.dump("/tmp/test.keyring", None).await?; let blob = tokio::fs::read("/tmp/test.keyring").await?; let loaded_keyring = Keyring::try_from(blob.as_slice())?; - let loaded_items = loaded_keyring.search_items(&[("my-tag", "my tag value")], &key)?; + let loaded_items = + loaded_keyring.search_items(&[("my-tag", "my tag value")], Some(&key))?; assert_eq!(loaded_items[0].secret(), Secret::text("A Password")); assert_eq!(loaded_items[0].secret().content_type(), ContentType::Text); diff --git a/client/src/file/locked_item.rs b/client/src/file/locked_item.rs index 9db7511c4..3258e382c 100644 --- a/client/src/file/locked_item.rs +++ b/client/src/file/locked_item.rs @@ -11,7 +11,7 @@ pub struct LockedItem { impl LockedItem { /// Unlocks the item. - pub fn unlock(self, key: &Key) -> Result { + pub fn unlock(self, key: Option<&Key>) -> Result { self.inner.decrypt(key) } } diff --git a/client/src/file/locked_keyring.rs b/client/src/file/locked_keyring.rs index d2466312b..baeaa4b2d 100644 --- a/client/src/file/locked_keyring.rs +++ b/client/src/file/locked_keyring.rs @@ -99,7 +99,7 @@ impl LockedKeyring { let mut n_broken_items = 0; let mut n_valid_items = 0; for encrypted_item in &inner_keyring.items { - if encrypted_item.is_valid(&key) { + if encrypted_item.is_valid(Some(&key)) { n_valid_items += 1; } else { n_broken_items += 1; @@ -139,7 +139,7 @@ impl LockedKeyring { path: self.path, mtime: self.mtime, key: Mutex::new(key), - secret: Mutex::new(Arc::new(secret)), + secret: Mutex::new(Some(Arc::new(secret))), }) } diff --git a/client/src/file/mod.rs b/client/src/file/mod.rs index 2157828a5..65e7a7193 100644 --- a/client/src/file/mod.rs +++ b/client/src/file/mod.rs @@ -82,7 +82,7 @@ impl Item { } /// Check if this item matches the given attributes - pub fn matches_attributes(&self, attributes: &impl AsAttributes, key: &Key) -> bool { + pub fn matches_attributes(&self, attributes: &impl AsAttributes, key: Option<&Key>) -> bool { match self { Self::Unlocked(unlocked) => { let item_attrs = unlocked.attributes(); @@ -90,17 +90,7 @@ impl Item { item_attrs.get(k.as_str()).map(|v| v.as_ref()) == Some(value.as_str()) }) } - Self::Locked(locked) => { - let hashed_attrs = attributes.hash(key); - - hashed_attrs.iter().all(|(attr_key, mac_result)| { - mac_result - .as_ref() - .ok() - .map(|mac| locked.inner.has_attribute(attr_key.as_str(), mac)) - .unwrap_or(false) - }) - } + Self::Locked(locked) => locked.inner.matches(attributes, key), } } } diff --git a/client/src/file/unlocked_item.rs b/client/src/file/unlocked_item.rs index a629825c6..9cb3f50e1 100644 --- a/client/src/file/unlocked_item.rs +++ b/client/src/file/unlocked_item.rs @@ -7,7 +7,7 @@ use super::{ Error, LockedItem, api::{EncryptedItem, GVARIANT_ENCODING}, }; -use crate::{AsAttributes, CONTENT_TYPE_ATTRIBUTE, Key, Secret, crypto, secret::ContentType}; +use crate::{AsAttributes, CONTENT_TYPE_ATTRIBUTE, Key, Mac, Secret, crypto, secret::ContentType}; /// An item stored in the file backend. #[derive( @@ -160,20 +160,35 @@ impl UnlockedItem { } /// Lock the item with the given key. - pub fn lock(self, key: &Key) -> Result { + pub fn lock(self, key: Option<&Key>) -> Result { let inner = self.encrypt(key)?; Ok(LockedItem { inner }) } - pub(crate) fn encrypt(&self, key: &Key) -> Result { - key.check_strength()?; - - let iv = crypto::generate_iv()?; + pub(crate) fn encrypt(&self, key: Option<&Key>) -> Result { + match key { + Some(key) => { + key.check_strength()?; + let iv = crypto::generate_iv()?; + self.encrypt_encrypted(key, &iv) + } + None => self.encrypt_plaintext(), + } + } - self.encrypt_inner(key, &iv) + fn encrypt_plaintext(&self) -> Result { + let blob = zvariant::to_bytes(*GVARIANT_ENCODING, &self)?.to_vec(); + Ok(EncryptedItem { + hashed_attributes: self + .attributes + .iter() + .map(|(k, v)| (k.to_owned(), Mac::new(v.as_bytes().to_vec()))) + .collect(), + blob, + }) } - fn encrypt_inner(&self, key: &Key, iv: &[u8]) -> Result { + fn encrypt_encrypted(&self, key: &Key, iv: &[u8]) -> Result { let decrypted = Zeroizing::new(zvariant::to_bytes(*GVARIANT_ENCODING, &self)?.to_vec()); let mut blob = crypto::encrypt(&*decrypted, key, iv)?; @@ -339,7 +354,7 @@ mod tests { secret: b"bar".to_vec(), }; - let encrypted = item.encrypt_inner(&key, &iv).unwrap(); + let encrypted = item.encrypt_encrypted(&key, &iv).unwrap(); assert!(encrypted.has_attribute("fooness", &attribute_value_mac)); let blob = &encrypted.blob; @@ -361,7 +376,7 @@ mod tests { ] ); - let decrypted = encrypted.decrypt(&key).unwrap(); + let decrypted = encrypted.decrypt(Some(&key)).unwrap(); // The decrypted item matches the original one but with the content-type // attribute set. diff --git a/client/src/file/unlocked_keyring.rs b/client/src/file/unlocked_keyring.rs index 5bfbfdedd..ba5d588f1 100644 --- a/client/src/file/unlocked_keyring.rs +++ b/client/src/file/unlocked_keyring.rs @@ -34,7 +34,7 @@ pub struct UnlockedKeyring { /// file changes before writing pub(super) mtime: Mutex>, pub(super) key: Mutex>>, - pub(super) secret: Mutex>, + pub(super) secret: Mutex>>, } impl UnlockedKeyring { @@ -102,7 +102,19 @@ impl UnlockedKeyring { path: None, mtime: Default::default(), key: Default::default(), - secret: Mutex::new(Arc::new(secret)), + secret: Mutex::new(Some(Arc::new(secret))), + }) + } + + /// Creates a temporary unencrypted backend, that is never stored on disk. + pub async fn temporary_unencrypted() -> Result { + let keyring = api::Keyring::new()?; + Ok(Self { + keyring: Arc::new(RwLock::new(keyring)), + path: None, + mtime: Default::default(), + key: Default::default(), + secret: Mutex::new(None), }) } @@ -122,7 +134,7 @@ impl UnlockedKeyring { path: Some(path.as_ref().to_path_buf()), mtime: Default::default(), key: Default::default(), - secret: Mutex::new(Arc::new(secret)), + secret: Mutex::new(Some(Arc::new(secret))), }), Err(Error::VersionMismatch(Some(version))) if version[0] == api::LEGACY_MAJOR_VERSION => @@ -141,7 +153,7 @@ impl UnlockedKeyring { tracing::debug_span!("migrate_items", item_count = decrypted_items.len()); for item in decrypted_items { - let encrypted_item = item.encrypt(&key)?; + let encrypted_item = item.encrypt(Some(&key))?; keyring.items.push(encrypted_item); } @@ -150,7 +162,7 @@ impl UnlockedKeyring { path: Some(path.as_ref().to_path_buf()), mtime: Default::default(), key: Default::default(), - secret: Mutex::new(Arc::new(secret)), + secret: Mutex::new(Some(Arc::new(secret))), }) } Err(err) => Err(err), @@ -186,7 +198,7 @@ impl UnlockedKeyring { path: Some(v1_path), mtime: Default::default(), key: Default::default(), - secret: Mutex::new(Arc::new(secret)), + secret: Mutex::new(Some(Arc::new(secret))), }) } } @@ -262,19 +274,21 @@ impl UnlockedKeyring { #[cfg_attr(feature = "tracing", tracing::instrument(skip(self, item)))] pub async fn lock_item(&self, item: UnlockedItem) -> Result { let key = self.derive_key().await?; - item.lock(&key) + item.lock(key.as_deref()) } /// Unlock an item using the keyring's key. #[cfg_attr(feature = "tracing", tracing::instrument(skip(self, item)))] pub async fn unlock_item(&self, item: LockedItem) -> Result { let key = self.derive_key().await?; - item.unlock(&key) + item.unlock(key.as_deref()) } /// Get the encryption key for this keyring. + /// + /// Returns `None` for unencrypted keyrings. #[cfg_attr(feature = "tracing", tracing::instrument(skip(self)))] - pub async fn key(&self) -> Result, crate::crypto::Error> { + pub async fn key(&self) -> Result>, crate::crypto::Error> { self.derive_key().await } @@ -317,7 +331,7 @@ impl UnlockedKeyring { .items .iter() .map(|e| { - (*e).clone().decrypt(&key).map_err(|err| { + (*e).clone().decrypt(key.as_deref()).map_err(|err| { InvalidItemError::new( err, e.hashed_attributes.keys().map(|x| x.to_string()).collect(), @@ -345,7 +359,7 @@ impl UnlockedKeyring { ) -> Result, Error> { let key = self.derive_key().await?; let keyring = self.keyring.read().await; - let results = keyring.search_items(attributes, &key)?; + let results = keyring.search_items(attributes, key.as_deref())?; #[cfg(feature = "tracing")] tracing::debug!("Found {} matching items", results.len()); @@ -362,7 +376,7 @@ impl UnlockedKeyring { let key = self.derive_key().await?; let keyring = self.keyring.read().await; - keyring.lookup_item(attributes, &key) + keyring.lookup_item(attributes, key.as_deref()) } /// Find the index in the list of items of the first item matching the @@ -375,7 +389,7 @@ impl UnlockedKeyring { let key = self.derive_key().await?; let keyring = self.keyring.read().await; - Ok(keyring.lookup_item_index(attributes, &key)) + Ok(keyring.lookup_item_index(attributes, key.as_deref())) } /// Delete an item. @@ -387,7 +401,7 @@ impl UnlockedKeyring { { let key = self.derive_key().await?; let mut keyring = self.keyring.write().await; - keyring.remove_items(attributes, &key)?; + keyring.remove_items(attributes, key.as_deref())?; }; self.write().await?; @@ -424,10 +438,10 @@ impl UnlockedKeyring { let key = self.derive_key().await?; let mut keyring = self.keyring.write().await; if replace { - keyring.remove_items(attributes, &key)?; + keyring.remove_items(attributes, key.as_deref())?; } let item = UnlockedItem::new(label, attributes, secret); - let encrypted_item = item.encrypt(&key)?; + let encrypted_item = item.encrypt(key.as_deref())?; keyring.items.push(encrypted_item); item }; @@ -457,7 +471,7 @@ impl UnlockedKeyring { let mut keyring = self.keyring.write().await; if let Some(item_store) = keyring.items.get_mut(index) { - *item_store = item.encrypt(&key)?; + *item_store = item.encrypt(key.as_deref())?; } else { return Err(Error::InvalidItemIndex(index)); } @@ -499,10 +513,10 @@ impl UnlockedKeyring { for (label, attributes, secret, replace) in items { if replace { - keyring.remove_items(&attributes, &key)?; + keyring.remove_items(&attributes, key.as_deref())?; } let item = UnlockedItem::new(label, &attributes, secret); - let encrypted_item = item.encrypt(&key)?; + let encrypted_item = item.encrypt(key.as_deref())?; keyring.items.push(encrypted_item); } @@ -539,12 +553,17 @@ impl UnlockedKeyring { Ok(()) } - /// Return key, derive and store it first if not initialized + /// Return key, derive and store it first if not initialized. + /// + /// Returns `None` when no secret is set (unencrypted keyring). #[cfg_attr(feature = "tracing", tracing::instrument(skip(self)))] - async fn derive_key(&self) -> Result, crate::crypto::Error> { + async fn derive_key(&self) -> Result>, crate::crypto::Error> { let keyring = Arc::clone(&self.keyring); let secret_lock = self.secret.lock().await; - let secret = Arc::clone(&secret_lock); + let secret = match secret_lock.as_ref() { + Some(secret) => Arc::clone(secret), + None => return Ok(None), + }; drop(secret_lock); let mut key_lock = self.key.lock().await; @@ -564,7 +583,7 @@ impl UnlockedKeyring { *key_lock = Some(Arc::new(key)); } - Ok(Arc::clone(key_lock.as_ref().unwrap())) + Ok(key_lock.clone()) } /// Change keyring secret @@ -583,7 +602,7 @@ impl UnlockedKeyring { tracing::debug_span!("decrypt_for_reencrypt", total_items = keyring.items.len()); for item in &keyring.items { - items.push(item.clone().decrypt(&key)?); + items.push(item.clone().decrypt(key.as_deref())?); } drop(keyring); @@ -591,7 +610,7 @@ impl UnlockedKeyring { tracing::debug!("Updating secret and resetting key"); let mut secret_lock = self.secret.lock().await; - *secret_lock = Arc::new(secret); + *secret_lock = Some(Arc::new(secret)); drop(secret_lock); let mut key_lock = self.key.lock().await; @@ -612,7 +631,7 @@ impl UnlockedKeyring { let mut keyring = self.keyring.write().await; for item in items { - let encrypted_item = item.encrypt(&key)?; + let encrypted_item = item.encrypt(key.as_deref())?; keyring.items.push(encrypted_item); } drop(keyring); @@ -650,7 +669,7 @@ impl UnlockedKeyring { let _span = tracing::debug_span!("identify_broken", total_items = keyring.items.len()); for (index, encrypted_item) in keyring.items.iter().enumerate() { - if !encrypted_item.is_valid(&key) { + if !encrypted_item.is_valid(key.as_deref()) { broken_items.push(index); } } diff --git a/server/src/collection/mod.rs b/server/src/collection/mod.rs index 965734a66..e2d7b570f 100644 --- a/server/src/collection/mod.rs +++ b/server/src/collection/mod.rs @@ -307,7 +307,9 @@ impl Collection { // Remove any existing items with the same attributes if replace { - let existing_items = self.search_items_with_key(&attributes, &key).await?; + let existing_items = self + .search_items_with_key(&attributes, key.as_deref()) + .await?; if !existing_items.is_empty() { let mut items = self.items.lock().await; for existing in &existing_items { @@ -503,13 +505,13 @@ impl Collection { .map_err(|err| custom_service_error(&format!("Failed to derive key: {err}")))?; drop(keyring_guard); - self.search_items_with_key(attributes, &key).await + self.search_items_with_key(attributes, key.as_deref()).await } async fn search_items_with_key( &self, attributes: &HashMap, - key: &oo7::Key, + key: Option<&oo7::Key>, ) -> Result, ServiceError> { let mut matching_items = Vec::new(); let items = self.items.lock().await; From e8b5c8dc8d21b5bb049cca8d68cbe2246e62220f Mon Sep 17 00:00:00 2001 From: Bilal Elmoussaoui Date: Fri, 26 Jun 2026 22:54:26 +0200 Subject: [PATCH 2/4] server: Map '' to a None secret, and create an unecrypted keyring instead. --- client/examples/file_tracing.rs | 12 +++--- client/examples/schema.rs | 2 +- client/src/file/api/mod.rs | 4 ++ client/src/file/locked_keyring.rs | 28 ++++++++++++ client/src/file/mod.rs | 9 +++- client/src/file/unlocked_keyring.rs | 59 ++++++++++++++----------- client/src/keyring.rs | 2 +- client/src/migration.rs | 6 ++- client/tests/file_unlocked_keyring.rs | 58 ++++++++++++------------- server/src/collection/mod.rs | 20 +++++---- server/src/gnome/internal.rs | 16 ++++--- server/src/gnome/prompter.rs | 8 +++- server/src/gnome/secret_exchange.rs | 4 +- server/src/item/mod.rs | 9 ++-- server/src/migration.rs | 11 +++-- server/src/pam_listener/tests.rs | 5 ++- server/src/plasma/prompter.rs | 8 +++- server/src/prompt/mod.rs | 62 ++++++++++++++++++--------- server/src/service/mod.rs | 31 ++++++-------- server/src/service/tests.rs | 16 ++++--- 20 files changed, 234 insertions(+), 136 deletions(-) diff --git a/client/examples/file_tracing.rs b/client/examples/file_tracing.rs index b8581517d..2bd8a3ed9 100644 --- a/client/examples/file_tracing.rs +++ b/client/examples/file_tracing.rs @@ -59,7 +59,7 @@ async fn test_keyring_lifecycle() -> oo7::Result<()> { // Measure keyring creation let start = Instant::now(); - let keyring = UnlockedKeyring::load(&keyring_path, secret.clone()).await?; + let keyring = UnlockedKeyring::load(&keyring_path, Some(secret.clone())).await?; let create_time = start.elapsed(); info!("Fresh keyring creation: {:?}", create_time); @@ -79,7 +79,7 @@ async fn test_keyring_lifecycle() -> oo7::Result<()> { // Measure keyring reload (with existing data) drop(keyring); let start = Instant::now(); - let keyring = UnlockedKeyring::load(&keyring_path, secret).await?; + let keyring = UnlockedKeyring::load(&keyring_path, Some(secret)).await?; let reload_time = start.elapsed(); info!("Keyring reload with 1 item: {:?}", reload_time); @@ -103,7 +103,7 @@ async fn test_bulk_operations() -> oo7::Result<()> { let temp_dir = tempdir().unwrap(); let keyring_path = temp_dir.path().join("bulk_test.keyring"); let secret = oo7::Secret::from("test-secret-key-that-is-long-enough".as_bytes()); - let keyring = UnlockedKeyring::load(&keyring_path, secret).await?; + let keyring = UnlockedKeyring::load(&keyring_path, Some(secret)).await?; // Test creating multiple items individually let item_counts = [10, 50, 100]; @@ -166,7 +166,7 @@ async fn test_scaling_behavior() -> oo7::Result<()> { // Create fresh keyring with 'size' items std::fs::remove_file(&keyring_path).ok(); // Remove if exists - let keyring = UnlockedKeyring::load(&keyring_path, secret.clone()).await?; + let keyring = UnlockedKeyring::load(&keyring_path, Some(secret.clone())).await?; // Populate keyring if size > 0 { @@ -188,7 +188,7 @@ async fn test_scaling_behavior() -> oo7::Result<()> { // Test reload performance drop(keyring); let start = Instant::now(); - let keyring = UnlockedKeyring::load(&keyring_path, secret.clone()).await?; + let keyring = UnlockedKeyring::load(&keyring_path, Some(secret.clone())).await?; let reload_time = start.elapsed(); info!("Reload with {} items: {:?}", size, reload_time); @@ -227,7 +227,7 @@ async fn test_search_performance() -> oo7::Result<()> { let temp_dir = tempdir().unwrap(); let keyring_path = temp_dir.path().join("search_test.keyring"); let secret = oo7::Secret::from("test-secret-key-that-is-long-enough".as_bytes()); - let keyring = UnlockedKeyring::load(&keyring_path, secret).await?; + let keyring = UnlockedKeyring::load(&keyring_path, Some(secret)).await?; // Create diverse set of items for search testing let apps = ["browser", "email", "social", "development", "finance"]; diff --git a/client/examples/schema.rs b/client/examples/schema.rs index 5b7c301a9..efb8bd1b2 100644 --- a/client/examples/schema.rs +++ b/client/examples/schema.rs @@ -13,7 +13,7 @@ struct PasswordSchema { async fn main() -> oo7::Result<()> { let temp_dir = tempfile::tempdir().unwrap(); let keyring_path = temp_dir.path().join("test.keyring"); - let keyring = UnlockedKeyring::load(&keyring_path, Secret::text("test_password")).await?; + let keyring = UnlockedKeyring::load(&keyring_path, Some(Secret::text("test_password"))).await?; println!("=== Creating items ==="); diff --git a/client/src/file/api/mod.rs b/client/src/file/api/mod.rs index 618ecc191..08db1b035 100644 --- a/client/src/file/api/mod.rs +++ b/client/src/file/api/mod.rs @@ -307,6 +307,10 @@ impl Keyring { Ok(self.items.iter().any(|item| item.is_valid(Some(&key)))) } + pub fn validate_unencrypted(&self) -> bool { + self.items.iter().all(|item| item.is_valid(None)) + } + /// Get the modification timestamp pub fn modified_time(&self) -> std::time::Duration { std::time::Duration::from_secs(self.modified_time) diff --git a/client/src/file/locked_keyring.rs b/client/src/file/locked_keyring.rs index baeaa4b2d..e6bfbd639 100644 --- a/client/src/file/locked_keyring.rs +++ b/client/src/file/locked_keyring.rs @@ -44,6 +44,11 @@ impl LockedKeyring { Ok(keyring.validate_secret(secret)?) } + pub async fn validate_unencrypted(&self) -> Result { + let keyring = self.keyring.read().await; + Ok(keyring.validate_unencrypted()) + } + /// Return the associated file if any. pub fn path(&self) -> Option<&std::path::Path> { self.path.as_deref() @@ -143,6 +148,29 @@ impl LockedKeyring { }) } + /// Unlocks a keyring without a secret, for unencrypted keyrings. + /// + /// Validates that existing items (if any) can be read without + /// encryption. Returns [`Error::IncorrectSecret`] if encrypted items + /// are found. + pub async fn unlock_unencrypted(self) -> Result { + let inner_keyring = self.keyring.read().await; + for encrypted_item in &inner_keyring.items { + if !encrypted_item.is_valid(None) { + return Err(Error::IncorrectSecret); + } + } + drop(inner_keyring); + + Ok(UnlockedKeyring { + keyring: self.keyring, + path: self.path, + mtime: self.mtime, + key: Mutex::new(None), + secret: Mutex::new(None), + }) + } + /// Load a keyring from a file path. pub async fn load(path: impl AsRef) -> Result { let path = path.as_ref(); diff --git a/client/src/file/mod.rs b/client/src/file/mod.rs index 65e7a7193..500596a7b 100644 --- a/client/src/file/mod.rs +++ b/client/src/file/mod.rs @@ -4,7 +4,7 @@ //! use oo7::{Secret, file::UnlockedKeyring}; //! //! # async fn run() -> oo7::Result<()> { -//! let keyring = UnlockedKeyring::load("default.keyring", Secret::text("some_text")).await?; +//! let keyring = UnlockedKeyring::load("default.keyring", Some(Secret::text("some_text"))).await?; //! keyring //! .create_item("My Label", &[("account", "alice")], "My Password", true) //! .await?; @@ -123,6 +123,13 @@ impl Keyring { } } + pub async fn validate_unencrypted(&self) -> Result { + match self { + Self::Locked(keyring) => keyring.validate_unencrypted().await, + Self::Unlocked(keyring) => keyring.validate_unencrypted().await, + } + } + /// Get the modification timestamp pub async fn modified_time(&self) -> std::time::Duration { match self { diff --git a/client/src/file/unlocked_keyring.rs b/client/src/file/unlocked_keyring.rs index ba5d588f1..74d3d3106 100644 --- a/client/src/file/unlocked_keyring.rs +++ b/client/src/file/unlocked_keyring.rs @@ -44,8 +44,9 @@ impl UnlockedKeyring { /// /// * `path` - The path to the file backend. /// * `secret` - The service key, usually retrieved from the Secrets portal. + /// Pass `None` for unencrypted keyrings. #[cfg_attr(feature = "tracing", tracing::instrument(skip(secret), fields(path = ?path.as_ref())))] - pub async fn load(path: impl AsRef, secret: Secret) -> Result { + pub async fn load(path: impl AsRef, secret: Option) -> Result { Self::load_inner(path, secret, true).await } @@ -69,27 +70,27 @@ impl UnlockedKeyring { path: impl AsRef, secret: Secret, ) -> Result { - Self::load_inner(path, secret, false).await + Self::load_inner(path, Some(secret), false).await } #[cfg_attr(feature = "tracing", tracing::instrument(skip(secret), fields(path = ?path.as_ref(), validate_items = validate_items)))] async fn load_inner( path: impl AsRef, - secret: Secret, + secret: Option, validate_items: bool, ) -> Result { #[cfg(feature = "tracing")] tracing::debug!("Trying to load keyring file at {:?}", path.as_ref()); - if validate_items { - LockedKeyring::load(path).await?.unlock(secret).await - } else { - #[allow(unsafe_code)] - unsafe { - LockedKeyring::load(path) - .await? - .unlock_unchecked(secret) - .await + let locked = LockedKeyring::load(path).await?; + match secret { + Some(secret) if validate_items => locked.unlock(secret).await, + Some(secret) => { + #[allow(unsafe_code)] + unsafe { + locked.unlock_unchecked(secret).await + } } + None => locked.unlock_unencrypted().await, } } @@ -122,7 +123,7 @@ impl UnlockedKeyring { async fn migrate( file: &mut fs::File, path: impl AsRef, - secret: Secret, + secret: Option, ) -> Result { let metadata = file.metadata().await?; let mut content = Vec::with_capacity(metadata.len() as usize); @@ -134,7 +135,7 @@ impl UnlockedKeyring { path: Some(path.as_ref().to_path_buf()), mtime: Default::default(), key: Default::default(), - secret: Mutex::new(Some(Arc::new(secret))), + secret: Mutex::new(secret.map(Arc::new)), }), Err(Error::VersionMismatch(Some(version))) if version[0] == api::LEGACY_MAJOR_VERSION => @@ -144,16 +145,17 @@ impl UnlockedKeyring { let legacy_keyring = api::LegacyKeyring::try_from(content.as_slice())?; let mut keyring = api::Keyring::new()?; - let key = keyring.derive_key(&secret)?; - let decrypted_items = legacy_keyring.decrypt_items(&secret)?; + let key = secret.as_ref().map(|s| keyring.derive_key(s)).transpose()?; + let decrypted_items = legacy_keyring + .decrypt_items(&secret.clone().unwrap_or_else(|| Secret::from(vec![])))?; #[cfg(feature = "tracing")] let _migrate_span = tracing::debug_span!("migrate_items", item_count = decrypted_items.len()); for item in decrypted_items { - let encrypted_item = item.encrypt(Some(&key))?; + let encrypted_item = item.encrypt(key.as_ref())?; keyring.items.push(encrypted_item); } @@ -162,7 +164,7 @@ impl UnlockedKeyring { path: Some(path.as_ref().to_path_buf()), mtime: Default::default(), key: Default::default(), - secret: Mutex::new(Some(Arc::new(secret))), + secret: Mutex::new(secret.map(Arc::new)), }) } Err(err) => Err(err), @@ -175,7 +177,7 @@ impl UnlockedKeyring { async fn open_with_paths( v1_path: PathBuf, v0_path: PathBuf, - secret: Secret, + secret: Option, ) -> Result { if v1_path.exists() { #[cfg(feature = "tracing")] @@ -198,7 +200,7 @@ impl UnlockedKeyring { path: Some(v1_path), mtime: Default::default(), key: Default::default(), - secret: Mutex::new(Some(Arc::new(secret))), + secret: Mutex::new(secret.map(Arc::new)), }) } } @@ -213,7 +215,7 @@ impl UnlockedKeyring { /// * `name` - The name of the keyring. /// * `secret` - The service key, usually retrieved from the Secrets portal. #[cfg_attr(feature = "tracing", tracing::instrument(skip(secret)))] - pub async fn open(name: &str, secret: Secret) -> Result { + pub async fn open(name: &str, secret: Option) -> Result { let v1_path = api::Keyring::path(name, api::MAJOR_VERSION)?; let v0_path = api::Keyring::path(name, api::LEGACY_MAJOR_VERSION)?; Self::open_with_paths(v1_path, v0_path, secret).await @@ -240,8 +242,12 @@ impl UnlockedKeyring { /// # use oo7::{Secret, file::UnlockedKeyring}; /// # async fn example() -> Result<(), Box> { /// let temp_dir = tempfile::tempdir()?; - /// let keyring = - /// UnlockedKeyring::open_at(temp_dir.path(), "test-keyring", Secret::from("password")).await?; + /// let keyring = UnlockedKeyring::open_at( + /// temp_dir.path(), + /// "test-keyring", + /// Some(Secret::from("password")), + /// ) + /// .await?; /// keyring /// .create_item("item", &[("attr", "value")], Secret::text("secret"), false) /// .await?; @@ -254,7 +260,7 @@ impl UnlockedKeyring { pub async fn open_at( data_dir: impl AsRef, name: &str, - secret: Secret, + secret: Option, ) -> Result { let v1_path = api::Keyring::path_at(&data_dir, name, api::MAJOR_VERSION); let v0_path = api::Keyring::path_at(&data_dir, name, api::LEGACY_MAJOR_VERSION); @@ -653,6 +659,11 @@ impl UnlockedKeyring { Ok(keyring.validate_secret(secret)?) } + pub async fn validate_unencrypted(&self) -> Result { + let keyring = self.keyring.read().await; + Ok(keyring.validate_unencrypted()) + } + /// Delete any item that cannot be decrypted with the key associated to the /// keyring. /// diff --git a/client/src/keyring.rs b/client/src/keyring.rs index 525f4fc44..a6d3303f6 100644 --- a/client/src/keyring.rs +++ b/client/src/keyring.rs @@ -73,7 +73,7 @@ impl Keyring { #[cfg(feature = "tracing")] tracing::debug!("Using file backend with custom path"); - let file = file::UnlockedKeyring::load(path, secret.clone()).await?; + let file = file::UnlockedKeyring::load(path, Some(secret.clone())).await?; Ok(Self::File( Arc::new(RwLock::new(Some(file::Keyring::Unlocked(file)))), secret, diff --git a/client/src/migration.rs b/client/src/migration.rs index 6f6c93a27..443f91682 100644 --- a/client/src/migration.rs +++ b/client/src/migration.rs @@ -31,7 +31,7 @@ async fn migrate_inner( attributes: Vec, replace: bool, ) -> Result<()> { - let file_backend = UnlockedKeyring::load(keyring_path, secret).await?; + let file_backend = UnlockedKeyring::load(keyring_path, Some(secret)).await?; let collection = service.default_collection().await?; let mut all_items = Vec::default(); @@ -146,7 +146,9 @@ mod tests { assert_eq!(items_after.len(), 0); // Verify items exist in file backend - let file_backend = UnlockedKeyring::load(&keyring_path, secret).await.unwrap(); + let file_backend = UnlockedKeyring::load(&keyring_path, Some(secret)) + .await + .unwrap(); let migrated_items = file_backend .search_items(&[("app", "test-migration")]) .await diff --git a/client/tests/file_unlocked_keyring.rs b/client/tests/file_unlocked_keyring.rs index 69dcb414a..3bc4e2fb9 100644 --- a/client/tests/file_unlocked_keyring.rs +++ b/client/tests/file_unlocked_keyring.rs @@ -17,7 +17,7 @@ async fn repeated_write() -> Result<(), Error> { let path = temp_dir.path().join("test.keyring"); let secret = Secret::from(vec![1, 2]); - let keyring = UnlockedKeyring::load(&path, secret).await?; + let keyring = UnlockedKeyring::load(&path, Some(secret)).await?; keyring.write().await?; keyring.write().await?; @@ -30,7 +30,7 @@ async fn delete() -> Result<(), Error> { let temp_dir = tempdir()?; let path = temp_dir.path().join("test-delete.keyring"); - let keyring = UnlockedKeyring::load(&path, strong_key()).await?; + let keyring = UnlockedKeyring::load(&path, Some(strong_key())).await?; let attributes: HashMap<&str, &str> = HashMap::default(); keyring .create_item("Label", &attributes, "secret", false) @@ -51,7 +51,7 @@ async fn write_with_weak_key() -> Result<(), Error> { let path = temp_dir.path().join("write_with_weak_key.keyring"); let secret = Secret::from(vec![1, 2]); - let keyring = UnlockedKeyring::load(&path, secret).await?; + let keyring = UnlockedKeyring::load(&path, Some(secret)).await?; let attributes: HashMap<&str, &str> = HashMap::default(); let result = keyring @@ -71,7 +71,7 @@ async fn write_with_strong_key() -> Result<(), Error> { let temp_dir = tempdir()?; let path = temp_dir.path().join("write_with_strong_key.keyring"); - let keyring = UnlockedKeyring::load(&path, strong_key()).await?; + let keyring = UnlockedKeyring::load(&path, Some(strong_key())).await?; let attributes: HashMap<&str, &str> = HashMap::default(); keyring @@ -86,7 +86,7 @@ async fn concurrent_writes() -> Result<(), Error> { let temp_dir = tempdir()?; let path = temp_dir.path().join("concurrent_writes.keyring"); - let keyring = Arc::new(UnlockedKeyring::load(&path, strong_key()).await?); + let keyring = Arc::new(UnlockedKeyring::load(&path, Some(strong_key())).await?); let keyring_clone = keyring.clone(); let handle_1 = tokio::task::spawn(async move { keyring_clone.write().await }); @@ -132,7 +132,7 @@ async fn migrate_from_legacy() -> Result<(), Error> { assert!(!v1_dir.join("default.keyring").exists()); let secret = Secret::blob("test"); - let keyring = UnlockedKeyring::open_at(data_dir.path(), "default", secret).await?; + let keyring = UnlockedKeyring::open_at(data_dir.path(), "default", Some(secret)).await?; check_items(&keyring).await?; @@ -155,7 +155,7 @@ async fn migrate() -> Result<(), Error> { fs::copy(&fixture_path, &v0_dir.join("default.keyring")).await?; let secret = Secret::blob("test"); - let keyring = UnlockedKeyring::open_at(data_dir.path(), "default", secret).await?; + let keyring = UnlockedKeyring::open_at(data_dir.path(), "default", Some(secret)).await?; assert!(!v1_dir.join("default.keyring").exists()); @@ -180,13 +180,13 @@ async fn open_wrong_password() -> Result<(), Error> { fs::copy(&fixture_path, &v1_dir.join("default.keyring")).await?; let secret = Secret::blob("wrong"); - let keyring = UnlockedKeyring::open_at(data_dir.path(), "default", secret).await; + let keyring = UnlockedKeyring::open_at(data_dir.path(), "default", Some(secret)).await; assert!(keyring.is_err()); assert!(matches!(keyring.unwrap_err(), Error::IncorrectSecret)); let secret = Secret::blob("test"); - let keyring = UnlockedKeyring::open_at(data_dir.path(), "default", secret).await; + let keyring = UnlockedKeyring::open_at(data_dir.path(), "default", Some(secret)).await; assert!(keyring.is_ok()); @@ -206,7 +206,7 @@ async fn open() -> Result<(), Error> { fs::copy(&fixture_path, &v1_dir.join("default.keyring")).await?; let secret = Secret::blob("test"); - let keyring = UnlockedKeyring::open_at(data_dir.path(), "default", secret).await?; + let keyring = UnlockedKeyring::open_at(data_dir.path(), "default", Some(secret)).await?; assert!(v1_dir.join("default.keyring").exists()); @@ -226,7 +226,7 @@ async fn open_nonexistent() -> Result<(), Error> { fs::create_dir_all(&v1_dir).await?; let secret = Secret::blob("test"); - let keyring = UnlockedKeyring::open_at(data_dir.path(), "default", secret).await?; + let keyring = UnlockedKeyring::open_at(data_dir.path(), "default", Some(secret)).await?; assert!(!v1_dir.join("default.keyring").exists()); @@ -263,7 +263,7 @@ async fn delete_broken_items() -> Result<(), Error> { // 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, Secret::blob("test")).await?; + let keyring = UnlockedKeyring::load(&keyring_path, Some(Secret::blob("test"))).await?; for i in 0..VALID_TO_ADD { keyring .create_item( @@ -293,7 +293,7 @@ async fn delete_broken_items() -> Result<(), Error> { drop(keyring); // 3) Load with the correct password and run the deletion. - let keyring = UnlockedKeyring::load(&keyring_path, Secret::blob("test")).await?; + let keyring = UnlockedKeyring::load(&keyring_path, Some(Secret::blob("test"))).await?; let removed = keyring.delete_broken_items().await?; assert!( removed >= BROKEN_TO_ADD, @@ -322,7 +322,7 @@ async fn change_secret() -> Result<(), Error> { let keyring_path = v1_dir.join("default.keyring"); fs::copy(&fixture_path, &keyring_path).await?; - let keyring = UnlockedKeyring::load(&keyring_path, Secret::blob("test")).await?; + let keyring = UnlockedKeyring::load(&keyring_path, Some(Secret::blob("test"))).await?; let attributes = &[("attr", "value")]; let item_before = keyring .create_item("test", attributes, "password", false) @@ -332,7 +332,7 @@ async fn change_secret() -> Result<(), Error> { keyring.change_secret(secret).await?; let secret = Secret::blob("new_secret"); - let keyring = UnlockedKeyring::load(&keyring_path, secret).await?; + let keyring = UnlockedKeyring::load(&keyring_path, Some(secret)).await?; let item_now = keyring.lookup_item(attributes).await?.unwrap(); assert_eq!(item_before.label(), item_now.label()); @@ -389,13 +389,13 @@ async fn wrong_password_error_type() -> Result<(), Error> { let wrong_secret = Secret::from("wrong-password-that-is-long-enough".as_bytes()); // Create a keyring with the correct password - let keyring = UnlockedKeyring::load(&keyring_path, correct_secret).await?; + let keyring = UnlockedKeyring::load(&keyring_path, Some(correct_secret)).await?; keyring .create_item("Test Item", &[("app", "test")], "my-secret", false) .await?; // Try to load with wrong password - let result = UnlockedKeyring::load(&keyring_path, wrong_secret).await; + let result = UnlockedKeyring::load(&keyring_path, Some(wrong_secret)).await; // Verify this returns IncorrectSecret, not ChecksumMismatch assert!(matches!(result, Err(Error::IncorrectSecret))); @@ -407,7 +407,7 @@ async fn wrong_password_error_type() -> Result<(), Error> { async fn comprehensive_search_patterns() -> Result<(), Error> { let temp_dir = tempdir().unwrap(); let keyring_path = temp_dir.path().join("search_test.keyring"); - let keyring = UnlockedKeyring::load(&keyring_path, strong_key()).await?; + let keyring = UnlockedKeyring::load(&keyring_path, Some(strong_key())).await?; // Create diverse test data let test_items = [ @@ -485,7 +485,7 @@ async fn comprehensive_search_patterns() -> Result<(), Error> { async fn item_replacement_behavior() -> Result<(), Error> { let temp_dir = tempdir().unwrap(); let keyring_path = temp_dir.path().join("replace_test.keyring"); - let keyring = UnlockedKeyring::load(&keyring_path, strong_key()).await?; + let keyring = UnlockedKeyring::load(&keyring_path, Some(strong_key())).await?; let attrs = &[("app", "test"), ("user", "alice")]; @@ -553,7 +553,7 @@ async fn item_replacement_behavior() -> Result<(), Error> { async fn empty_keyring_operations() -> Result<(), Error> { let temp_dir = tempdir().unwrap(); let keyring_path = temp_dir.path().join("empty_test.keyring"); - let keyring = UnlockedKeyring::load(&keyring_path, strong_key()).await?; + let keyring = UnlockedKeyring::load(&keyring_path, Some(strong_key())).await?; // Test operations on empty keyring let items = keyring.items().await?; @@ -579,7 +579,7 @@ async fn empty_keyring_operations() -> Result<(), Error> { async fn secret_types_handling() -> Result<(), Error> { let temp_dir = tempdir().unwrap(); let keyring_path = temp_dir.path().join("secret_types_test.keyring"); - let keyring = UnlockedKeyring::load(&keyring_path, strong_key()).await?; + let keyring = UnlockedKeyring::load(&keyring_path, Some(strong_key())).await?; // Test text secret keyring @@ -654,7 +654,7 @@ async fn secret_types_handling() -> Result<(), Error> { async fn item_lifecycle_operations() -> Result<(), Error> { let temp_dir = tempdir().unwrap(); let keyring_path = temp_dir.path().join("lifecycle_test.keyring"); - let keyring = UnlockedKeyring::load(&keyring_path, strong_key()).await?; + let keyring = UnlockedKeyring::load(&keyring_path, Some(strong_key())).await?; // Test creating multiple items keyring @@ -702,7 +702,7 @@ async fn item_lifecycle_operations() -> Result<(), Error> { async fn item_attribute_operations() -> Result<(), Error> { let temp_dir = tempdir().unwrap(); let keyring_path = temp_dir.path().join("attr_test.keyring"); - let keyring = UnlockedKeyring::load(&keyring_path, strong_key()).await?; + let keyring = UnlockedKeyring::load(&keyring_path, Some(strong_key())).await?; // Create item with initial attributes keyring @@ -760,7 +760,7 @@ async fn item_attribute_operations() -> Result<(), Error> { async fn bulk_create_items() -> Result<(), Error> { let temp_dir = tempdir().unwrap(); let keyring_path = temp_dir.path().join("bulk_create_test.keyring"); - let keyring = UnlockedKeyring::load(&keyring_path, strong_key()).await?; + let keyring = UnlockedKeyring::load(&keyring_path, Some(strong_key())).await?; // Prepare multiple items to create at once let items_to_create = vec![ @@ -825,7 +825,7 @@ async fn partially_corrupted_keyring_error() -> Result<(), Error> { // Create keyring with correct password and add 2 valid items let correct_secret = Secret::from("correct-password-long-enough".as_bytes()); - let keyring = UnlockedKeyring::load(&keyring_path, correct_secret.clone()).await?; + let keyring = UnlockedKeyring::load(&keyring_path, Some(correct_secret.clone())).await?; keyring .create_item("valid1", &[("attr", "value1")], "password1", false) .await?; @@ -848,7 +848,7 @@ async fn partially_corrupted_keyring_error() -> Result<(), Error> { .await?; drop(keyring); - let result = UnlockedKeyring::load(&keyring_path, correct_secret).await; + let result = UnlockedKeyring::load(&keyring_path, Some(correct_secret)).await; assert!(result.is_err()); match result.unwrap_err() { Error::PartiallyCorruptedKeyring { @@ -872,7 +872,7 @@ async fn invalid_item_error_on_decrypt_failure() -> Result<(), Error> { // 1) Create keyring with correct password and add 2 items let correct_secret = Secret::from("correct-password-long-enough".as_bytes()); - let keyring = UnlockedKeyring::load(&keyring_path, correct_secret).await?; + let keyring = UnlockedKeyring::load(&keyring_path, Some(correct_secret)).await?; keyring .create_item("item1", &[("app", "test1")], "password1", false) .await?; @@ -904,7 +904,7 @@ async fn invalid_item_error_on_decrypt_failure() -> Result<(), Error> { async fn replace_item_index_invalid() -> Result<(), Error> { let temp_dir = tempdir().unwrap(); let keyring_path = temp_dir.path().join("replace_invalid_index.keyring"); - let keyring = UnlockedKeyring::load(&keyring_path, strong_key()).await?; + let keyring = UnlockedKeyring::load(&keyring_path, Some(strong_key())).await?; // Create one item keyring @@ -928,7 +928,7 @@ async fn set_attributes() -> Result<(), Error> { fs::create_dir_all(&dir).await.unwrap(); let path = dir.join("default.keyring"); - let keyring = UnlockedKeyring::load(&path, strong_key()).await?; + let keyring = UnlockedKeyring::load(&path, Some(strong_key())).await?; let items = keyring.items().await?; assert_eq!(items.len(), 0); diff --git a/server/src/collection/mod.rs b/server/src/collection/mod.rs index e2d7b570f..4dd714008 100644 --- a/server/src/collection/mod.rs +++ b/server/src/collection/mod.rs @@ -67,9 +67,9 @@ impl Collection { let collection = self.clone(); let caller_owned = caller; let action = - crate::prompt::PromptAction::new(move |unlock_secret: Secret| async move { + crate::prompt::PromptAction::new(move |unlock_secret: Option| async move { // Unlock the collection - collection.set_locked(false, Some(unlock_secret)).await?; + collection.set_locked(false, unlock_secret).await?; collection.delete_unlocked(&caller_owned).await?; @@ -201,8 +201,8 @@ impl Collection { let collection = self.clone(); let action = - crate::prompt::PromptAction::new(move |unlock_secret: Secret| async move { - collection.set_locked(false, Some(unlock_secret)).await?; + crate::prompt::PromptAction::new(move |unlock_secret: Option| async move { + collection.set_locked(false, unlock_secret).await?; let item_path = collection .create_item_unlocked(properties, secret, replace) @@ -554,13 +554,15 @@ impl Collection { Keyring::Locked(unlocked.lock()) } (Keyring::Locked(locked_kr), false) => { - let secret = secret.ok_or_else(|| { - custom_service_error("Cannot unlock collection without a secret") - })?; - let keyring_path = locked_kr.path().map(|p| p.to_path_buf()); - let unlocked = match locked_kr.unlock(secret).await { + let unlock_result = if let Some(secret) = secret { + locked_kr.unlock(secret).await + } else { + locked_kr.unlock_unencrypted().await + }; + + let unlocked = match unlock_result { Ok(unlocked) => unlocked, Err(err) => { // Reload the locked keyring from disk before returning error diff --git a/server/src/gnome/internal.rs b/server/src/gnome/internal.rs index 9528e4a21..425784911 100644 --- a/server/src/gnome/internal.rs +++ b/server/src/gnome/internal.rs @@ -65,7 +65,7 @@ impl InternalInterface { let collection_path = self .service - .create_collection_with_secret(&label, "", secret) + .create_collection_with_secret(&label, "", Some(secret)) .await?; tracing::info!( @@ -166,7 +166,7 @@ impl InternalInterface { let service = self.service.clone(); let collection_path = collection.to_owned(); - let action = PromptAction::new(move |new_secret: Secret| { + let action = PromptAction::new(move |new_secret: Option| { let service = service.clone(); let collection_path = collection_path.clone(); async move { @@ -177,9 +177,15 @@ impl InternalInterface { 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}")) - })?; + if let Some(new_secret) = new_secret { + unlocked.change_secret(new_secret).await.map_err(|err| { + custom_service_error(&format!("Failed to change secret: {err}")) + })?; + } else { + return Err(custom_service_error( + "Cannot change to empty password via this interface", + )); + } } else { return Err(custom_service_error( "Collection must be unlocked to change password", diff --git a/server/src/gnome/prompter.rs b/server/src/gnome/prompter.rs index c2f411a58..733a4891a 100644 --- a/server/src/gnome/prompter.rs +++ b/server/src/gnome/prompter.rs @@ -381,12 +381,18 @@ impl GNOMEPrompterCallback { )) })?; - let Some(secret) = secret_exchange::retrieve(exchange, &aes_key) else { + let Some(raw_secret) = secret_exchange::retrieve(exchange, &aes_key) else { return Err(custom_service_error( "Failed to retrieve keyring secret from SecretExchange.", )); }; + let secret = if raw_secret.as_bytes().is_empty() { + None + } else { + Some(raw_secret) + }; + // Handle each role differently based on what validation/preparation is needed match prompt.role() { PromptRole::Unlock => { diff --git a/server/src/gnome/secret_exchange.rs b/server/src/gnome/secret_exchange.rs index e67d73ce8..2a545658e 100644 --- a/server/src/gnome/secret_exchange.rs +++ b/server/src/gnome/secret_exchange.rs @@ -52,9 +52,7 @@ pub fn retrieve(exchange: &str, aes_key: &Key) -> Option { // AES ciphertext must be a multiple of 16 bytes (block size) // and at least 16 bytes (minimum for PKCS7 padding) if secret.is_empty() || secret.len() % 16 != 0 { - // Invalid ciphertext - return a false secret to avoid decryption errors - let false_secret = vec![0, 1]; - return Some(oo7::Secret::from(false_secret)); + return None; } let iv = decoded.get(IV)?; diff --git a/server/src/item/mod.rs b/server/src/item/mod.rs index 795cbf73f..d72a5c63e 100644 --- a/server/src/item/mod.rs +++ b/server/src/item/mod.rs @@ -56,10 +56,10 @@ impl Item { let item_self = self.clone(); let coll = collection.clone(); let caller = caller.to_owned(); - let action = - crate::prompt::PromptAction::new(move |unlock_secret: oo7::Secret| async move { + let action = crate::prompt::PromptAction::new( + move |unlock_secret: Option| async move { // Unlock the collection - coll.set_locked(false, Some(unlock_secret)).await?; + coll.set_locked(false, unlock_secret).await?; // Now delete the item item_self.delete_unlocked(&coll, &caller).await?; @@ -67,7 +67,8 @@ impl Item { Ok(zbus::zvariant::Value::new(OwnedObjectPath::default()) .try_into_owned() .unwrap()) - }); + }, + ); prompt.set_action(action).await; diff --git a/server/src/migration.rs b/server/src/migration.rs index 30491260f..92edc1e25 100644 --- a/server/src/migration.rs +++ b/server/src/migration.rs @@ -31,13 +31,13 @@ impl PendingMigration { pub async fn migrate( &self, data_dir: &PathBuf, - secret: &Secret, + secret: Option<&Secret>, ) -> Result { match self { Self::V0 { path, name, .. } => { tracing::debug!("Migrating v0 keyring: {}", name); - let unlocked = UnlockedKeyring::open_at(data_dir, name, secret.clone()).await?; + let unlocked = UnlockedKeyring::open_at(data_dir, name, secret.cloned()).await?; // Write migrated keyring unlocked.write().await?; @@ -57,6 +57,10 @@ impl PendingMigration { Self::KWallet { path, name, .. } => { tracing::debug!("Migrating KWallet keyring: {}", name); + let secret = secret.ok_or_else(|| { + Error::IO(std::io::Error::other("KWallet migration requires a secret")) + })?; + // Parse KWallet file in blocking task let path_clone = path.clone(); let password = secret.to_vec(); @@ -71,7 +75,8 @@ impl PendingMigration { tracing::info!("Parsed KWallet file '{}'", name); // Create new oo7 keyring - let unlocked = UnlockedKeyring::open_at(data_dir, name, secret.clone()).await?; + let unlocked = + UnlockedKeyring::open_at(data_dir, name, Some(secret.clone())).await?; // Convert KWallet entries to oo7 items let mut items = Vec::new(); diff --git a/server/src/pam_listener/tests.rs b/server/src/pam_listener/tests.rs index b8f77b3a3..b145b60ef 100644 --- a/server/src/pam_listener/tests.rs +++ b/server/src/pam_listener/tests.rs @@ -139,7 +139,7 @@ async fn pam_unlocks_locked_collections() -> Result<(), Box Result<(), Box> { let temp_dir = tempfile::tempdir()?; let old_secret = Secret::from("old-password"); - let keyring = UnlockedKeyring::open_at(temp_dir.path(), "work", old_secret.clone()).await?; + let keyring = + UnlockedKeyring::open_at(temp_dir.path(), "work", Some(old_secret.clone())).await?; keyring .create_item( "Work Item", diff --git a/server/src/plasma/prompter.rs b/server/src/plasma/prompter.rs index d50d8cfdd..10fad893f 100644 --- a/server/src/plasma/prompter.rs +++ b/server/src/plasma/prompter.rs @@ -111,7 +111,11 @@ impl PlasmaPrompterCallback { .await .expect("error reading secret"); tracing::debug!("Read secret from fd, length {}", buffer.len()); - oo7::Secret::from(buffer) + if buffer.is_empty() { + None + } else { + Some(oo7::Secret::from(buffer)) + } }; self.on_reply(&prompt, secret).await @@ -207,7 +211,7 @@ impl PlasmaPrompterCallback { async fn on_reply( &self, prompt: &Prompt, - secret: Secret, + secret: Option, ) -> Result { // Handle each role differently based on what validation/preparation is needed match prompt.role() { diff --git a/server/src/prompt/mod.rs b/server/src/prompt/mod.rs index 820ed3e52..42abeeae8 100644 --- a/server/src/prompt/mod.rs +++ b/server/src/prompt/mod.rs @@ -53,7 +53,7 @@ pub type PromptActionFuture = /// Represents the action to be taken when a prompt completes pub struct PromptAction { /// The async function to execute when the prompt is accepted - action: Box PromptActionFuture + Send>, + action: Box) -> PromptActionFuture + Send>, } impl PromptAction { @@ -61,7 +61,7 @@ impl PromptAction { /// and returns a future pub fn new(f: F) -> Self where - F: FnOnce(Secret) -> Fut + Send + 'static, + F: FnOnce(Option) -> Fut + Send + 'static, Fut: Future> + Send + 'static, { Self { @@ -70,7 +70,7 @@ impl PromptAction { } /// Execute the action with the provided secret - pub async fn execute(self, secret: Secret) -> Result { + pub async fn execute(self, secret: Option) -> Result { (self.action)(secret).await } } @@ -200,26 +200,44 @@ impl Prompt { self.action.lock().await.take() } - pub async fn on_unlock_collection(&self, secret: Secret) -> Result { + pub async fn on_unlock_collection(&self, secret: Option) -> Result { debug_assert_eq!(self.role, PromptRole::Unlock); // Get the collection to validate the secret let collection = self.collection().expect("Unlock requires a collection"); let label = self.label(); - // Validate the secret using the already-open keyring - let keyring_guard = collection.keyring.read().await; - let is_valid = keyring_guard - .as_ref() - .unwrap() - .validate_secret(&secret) - .await - .map_err(|err| { - custom_service_error(&format!( - "Failed to validate secret for {label} keyring: {err}." - )) - })?; - drop(keyring_guard); + let is_valid = if let Some(ref secret) = secret { + // Validate the secret using the already-open keyring + let keyring_guard = collection.keyring.read().await; + let valid = keyring_guard + .as_ref() + .unwrap() + .validate_secret(secret) + .await + .map_err(|err| { + custom_service_error(&format!( + "Failed to validate secret for {label} keyring: {err}." + )) + })?; + drop(keyring_guard); + valid + } else { + // No secret means unencrypted -> validate items are plaintext + let keyring_guard = collection.keyring.read().await; + let valid = keyring_guard + .as_ref() + .unwrap() + .validate_unencrypted() + .await + .map_err(|err| { + custom_service_error(&format!( + "Failed to validate unencrypted keyring {label}: {err}." + )) + })?; + drop(keyring_guard); + valid + }; if is_valid { tracing::debug!("Keyring secret matches for {label}."); @@ -247,7 +265,7 @@ impl Prompt { } } - pub async fn on_create_collection(&self, secret: Secret) -> Result<(), ServiceError> { + pub async fn on_create_collection(&self, secret: Option) -> Result<(), ServiceError> { debug_assert_eq!(self.role, PromptRole::CreateCollection); let Some(action) = self.take_action().await else { @@ -275,7 +293,7 @@ impl Prompt { } } - pub async fn on_change_password(&self, secret: Secret) -> Result<(), ServiceError> { + pub async fn on_change_password(&self, secret: Option) -> Result<(), ServiceError> { debug_assert_eq!(self.role, PromptRole::ChangePassword); let Some(action) = self.take_action().await else { @@ -395,7 +413,11 @@ impl Prompt { stream.read_to_string(&mut buffer).await.map_err(|e| { custom_service_error(&format!("Failed to read secret from CLI prompter: {e}")) })?; - let secret = Secret::from(buffer); + let secret = if buffer.is_empty() { + None + } else { + Some(Secret::from(buffer)) + }; match self.role { PromptRole::Unlock => { diff --git a/server/src/service/mod.rs b/server/src/service/mod.rs index 1b83669ef..9e61205fe 100644 --- a/server/src/service/mod.rs +++ b/server/src/service/mod.rs @@ -191,7 +191,7 @@ impl Service { // Create the collection creation action let service = self.clone(); let creation_prompt_path = prompt_path.clone(); - let action = PromptAction::new(move |secret: Secret| async move { + let action = PromptAction::new(move |secret: Option| async move { let collection_path = service .complete_collection_creation(&creation_prompt_path, secret) .await?; @@ -264,7 +264,7 @@ impl Service { // Create the unlock action let service = self.clone(); - let action = PromptAction::new(move |secret: Secret| async move { + let action = PromptAction::new(move |secret: Option| async move { // The prompter will handle secret validation // Here we just perform the unlock operation @@ -290,7 +290,7 @@ impl Service { ); // Attempt migration with the provided secret (no locks held) - match migration.migrate(&service.data_dir, &secret).await { + match migration.migrate(&service.data_dir, secret.as_ref()).await { Ok(unlocked_keyring) => { tracing::info!( "Successfully migrated '{}' during unlock", @@ -323,14 +323,12 @@ impl Service { migration_name, e ); - // Leave in pending_migrations, try normal unlock - let _ = - collection.set_locked(false, Some(secret.clone())).await; + let _ = collection.set_locked(false, secret.clone()).await; } } } else { // Normal unlock - let _ = collection.set_locked(false, Some(secret.clone())).await; + let _ = collection.set_locked(false, secret.clone()).await; } } else { // Try to find as item within collections @@ -350,7 +348,7 @@ impl Service { if let Some((collection, item, is_locked)) = found_collection { if is_locked { - let _ = collection.set_locked(false, Some(secret.clone())).await; + let _ = collection.set_locked(false, secret.clone()).await; } else { let keyring = collection.keyring.read().await; match keyring.as_ref() { @@ -359,9 +357,7 @@ impl Service { } _ => { drop(keyring); - let _ = collection - .set_locked(false, Some(secret.clone())) - .await; + let _ = collection.set_locked(false, secret.clone()).await; } } } @@ -850,7 +846,7 @@ impl Service { if let Some(secret) = secret { tracing::debug!("Attempting immediate migration of KWallet keyring '{name}'",); - match migration.migrate(&self.data_dir, secret).await { + match migration.migrate(&self.data_dir, Some(secret)).await { Ok(unlocked) => { tracing::info!("Successfully migrated KWallet keyring '{name}' to oo7",); discovered.push(( @@ -966,7 +962,8 @@ impl Service { if let Some(secret) = secret { tracing::debug!("Attempting immediate migration of v0 keyring '{name}'",); - match UnlockedKeyring::open_at(&self.data_dir, name, secret.clone()).await { + match UnlockedKeyring::open_at(&self.data_dir, name, Some(secret.clone())).await + { Ok(unlocked) => { tracing::info!("Successfully migrated v0 keyring '{name}' to v1",); @@ -1046,7 +1043,7 @@ impl Service { tracing::info!("No default collection found, creating 'Login' keyring"); let keyring = if let Some(secret) = secret { - UnlockedKeyring::open_at(&self.data_dir, Self::LOGIN_ALIAS, secret) + UnlockedKeyring::open_at(&self.data_dir, Self::LOGIN_ALIAS, Some(secret)) .await .map(Keyring::Unlocked) } else { @@ -1392,7 +1389,7 @@ impl Service { &self, label: &str, alias: &str, - secret: Secret, + secret: Option, ) -> Result { // Create a persistent keyring with the provided secret let keyring = UnlockedKeyring::open_at(&self.data_dir, &label.to_lowercase(), secret) @@ -1448,7 +1445,7 @@ impl Service { pub async fn complete_collection_creation( &self, prompt_path: &ObjectPath<'_>, - secret: Secret, + secret: Option, ) -> Result { let Some((label, alias)) = self.pending_collection(prompt_path).await else { return Err(ServiceError::NoSuchObject(format!( @@ -1545,7 +1542,7 @@ impl Service { for (name, migration) in pending.iter() { tracing::debug!("Attempting to migrate pending keyring: {name}"); - match migration.migrate(&self.data_dir, secret).await { + match migration.migrate(&self.data_dir, Some(secret)).await { Ok(unlocked) => { let label = migration.label(); let alias = migration.alias(); diff --git a/server/src/service/tests.rs b/server/src/service/tests.rs index 5559d6e43..51bc908d3 100644 --- a/server/src/service/tests.rs +++ b/server/src/service/tests.rs @@ -1071,7 +1071,7 @@ async fn complete_collection_creation_no_pending() -> Result<(), Box Result<(), Box> { // Create multiple keyrings with different passwords // Add items to each so password validation works let secret1 = Secret::from("password-for-work"); - let keyring1 = UnlockedKeyring::open_at(temp_dir.path(), "work", secret1.clone()).await?; + let keyring1 = UnlockedKeyring::open_at(temp_dir.path(), "work", Some(secret1.clone())).await?; keyring1 .create_item( "Work Item", @@ -1114,7 +1114,8 @@ async fn discover_v1_keyrings() -> Result<(), Box> { keyring1.write().await?; let secret2 = Secret::from("password-for-personal"); - let keyring2 = UnlockedKeyring::open_at(temp_dir.path(), "personal", secret2.clone()).await?; + let keyring2 = + UnlockedKeyring::open_at(temp_dir.path(), "personal", Some(secret2.clone())).await?; keyring2 .create_item( "Personal Item", @@ -1127,7 +1128,8 @@ async fn discover_v1_keyrings() -> Result<(), Box> { // Create a "login" keyring which should get the default alias let secret3 = Secret::from("password-for-login"); - let keyring3 = UnlockedKeyring::open_at(temp_dir.path(), "login", secret3.clone()).await?; + let keyring3 = + UnlockedKeyring::open_at(temp_dir.path(), "login", Some(secret3.clone())).await?; keyring3 .create_item( "Login Item", @@ -1326,7 +1328,8 @@ async fn discover_v0_keyrings() -> Result<(), Box> { // Create a v1 keyring for mixed scenario let v1_secret = Secret::from("v1-password"); - let v1_keyring = UnlockedKeyring::open_at(temp_dir.path(), "modern", v1_secret.clone()).await?; + let v1_keyring = + UnlockedKeyring::open_at(temp_dir.path(), "modern", Some(v1_secret.clone())).await?; v1_keyring .create_item( "V1 Item", @@ -1417,7 +1420,8 @@ async fn discover_kwallet_keyrings() -> Result<(), Box> { // Create a v1 keyring for mixed scenario let v1_secret = Secret::from("v1-password"); - let v1_keyring = UnlockedKeyring::open_at(temp_dir.path(), "modern", v1_secret.clone()).await?; + let v1_keyring = + UnlockedKeyring::open_at(temp_dir.path(), "modern", Some(v1_secret.clone())).await?; v1_keyring .create_item( "V1 Item", From a5fb35a204922683d80f224b467b70a8c2e816ba Mon Sep 17 00:00:00 2001 From: Bilal Elmoussaoui Date: Fri, 26 Jun 2026 23:06:48 +0200 Subject: [PATCH 3/4] Add tests for unencrypted keyrings --- client/tests/file_unencrypted_keyring.rs | 240 +++++++++++++++++++++++ server/src/service/mod.rs | 2 + server/src/service/unencrypted_tests.rs | 114 +++++++++++ 3 files changed, 356 insertions(+) create mode 100644 client/tests/file_unencrypted_keyring.rs create mode 100644 server/src/service/unencrypted_tests.rs diff --git a/client/tests/file_unencrypted_keyring.rs b/client/tests/file_unencrypted_keyring.rs new file mode 100644 index 000000000..5b0eebb13 --- /dev/null +++ b/client/tests/file_unencrypted_keyring.rs @@ -0,0 +1,240 @@ +use oo7::{Secret, file::*}; +use tempfile::tempdir; + +#[tokio::test] +async fn unencrypted_roundtrip() -> Result<(), Error> { + let data_dir = tempdir()?; + let keyring = UnlockedKeyring::open_at(data_dir.path(), "plain", None).await?; + + keyring + .create_item( + "Item 1", + &[("app", "test"), ("user", "alice")], + "secret-1", + false, + ) + .await?; + keyring + .create_item( + "Item 2", + &[("app", "test"), ("user", "bob")], + "secret-2", + false, + ) + .await?; + + keyring.write().await?; + + let v1_path = data_dir + .path() + .join("keyrings") + .join("v1") + .join("plain.keyring"); + assert!(v1_path.exists()); + + let reloaded = UnlockedKeyring::load(&v1_path, None).await?; + let items = reloaded.search_items(&[("app", "test")]).await?; + assert_eq!(items.len(), 2); + + let alice = items + .iter() + .find(|i| { + i.attributes() + .get("user") + .map(|v| v == "alice") + .unwrap_or(false) + }) + .expect("alice item"); + assert_eq!(alice.label(), "Item 1"); + assert_eq!(alice.secret(), Secret::text("secret-1")); + + let bob = items + .iter() + .find(|i| { + i.attributes() + .get("user") + .map(|v| v == "bob") + .unwrap_or(false) + }) + .expect("bob item"); + assert_eq!(bob.label(), "Item 2"); + assert_eq!(bob.secret(), Secret::text("secret-2")); + + Ok(()) +} + +#[tokio::test] +async fn unencrypted_search() -> Result<(), Error> { + let data_dir = tempdir()?; + let keyring = UnlockedKeyring::open_at(data_dir.path(), "search", None).await?; + + keyring + .create_item( + "A", + &[("kind", "password"), ("service", "github")], + "pw1", + false, + ) + .await?; + keyring + .create_item( + "B", + &[("kind", "password"), ("service", "gitlab")], + "pw2", + false, + ) + .await?; + keyring + .create_item( + "C", + &[("kind", "token"), ("service", "github")], + "tok1", + false, + ) + .await?; + + let passwords = keyring.search_items(&[("kind", "password")]).await?; + assert_eq!(passwords.len(), 2); + + let github = keyring.search_items(&[("service", "github")]).await?; + assert_eq!(github.len(), 2); + + let exact = keyring + .search_items(&[("kind", "password"), ("service", "github")]) + .await?; + assert_eq!(exact.len(), 1); + assert_eq!(exact[0].label(), "A"); + + let none = keyring.search_items(&[("kind", "nonexistent")]).await?; + assert!(none.is_empty()); + + Ok(()) +} + +#[tokio::test] +async fn unencrypted_delete() -> Result<(), Error> { + let data_dir = tempdir()?; + let keyring = UnlockedKeyring::open_at(data_dir.path(), "del", None).await?; + + keyring + .create_item("Keep", &[("id", "keep")], "s1", false) + .await?; + keyring + .create_item("Remove", &[("id", "remove")], "s2", false) + .await?; + + assert_eq!(keyring.n_items().await, 2); + + keyring.delete(&[("id", "remove")]).await?; + assert_eq!(keyring.n_items().await, 1); + + keyring.write().await?; + + let v1_path = data_dir + .path() + .join("keyrings") + .join("v1") + .join("del.keyring"); + let reloaded = UnlockedKeyring::load(&v1_path, None).await?; + assert_eq!(reloaded.n_items().await, 1); + + let items = reloaded.search_items(&[("id", "keep")]).await?; + assert_eq!(items.len(), 1); + assert_eq!(items[0].label(), "Keep"); + + let removed = reloaded.search_items(&[("id", "remove")]).await?; + assert!(removed.is_empty()); + + Ok(()) +} + +#[tokio::test] +async fn unencrypted_unlock_encrypted_fails() -> Result<(), Error> { + let data_dir = tempdir()?; + let secret = Secret::from([1, 2].into_iter().cycle().take(64).collect::>()); + let keyring = UnlockedKeyring::open_at(data_dir.path(), "enc", Some(secret)).await?; + + keyring + .create_item("Encrypted", &[("a", "b")], "secret", false) + .await?; + keyring.write().await?; + + let v1_path = data_dir + .path() + .join("keyrings") + .join("v1") + .join("enc.keyring"); + let locked = LockedKeyring::load(&v1_path).await?; + let result = locked.unlock_unencrypted().await; + + assert!(result.is_err()); + assert!(matches!(result.unwrap_err(), Error::IncorrectSecret)); + + Ok(()) +} + +#[tokio::test] +async fn encrypted_unlock_unencrypted_fails() -> Result<(), Error> { + let data_dir = tempdir()?; + let keyring = UnlockedKeyring::open_at(data_dir.path(), "plain", None).await?; + + keyring + .create_item("Plaintext", &[("a", "b")], "secret", false) + .await?; + keyring.write().await?; + + let v1_path = data_dir + .path() + .join("keyrings") + .join("v1") + .join("plain.keyring"); + let locked = LockedKeyring::load(&v1_path).await?; + let wrong_secret = Secret::from([9, 8].into_iter().cycle().take(64).collect::>()); + let result = locked.unlock(wrong_secret).await; + + assert!(result.is_err()); + assert!(matches!(result.unwrap_err(), Error::IncorrectSecret)); + + Ok(()) +} + +#[tokio::test] +async fn unencrypted_temporary() -> Result<(), Error> { + let keyring = UnlockedKeyring::temporary_unencrypted().await?; + + keyring + .create_item("Temp", &[("x", "y")], "temp-secret", false) + .await?; + + let items = keyring.search_items(&[("x", "y")]).await?; + assert_eq!(items.len(), 1); + assert_eq!(items[0].label(), "Temp"); + assert_eq!(items[0].secret(), Secret::text("temp-secret")); + + Ok(()) +} + +#[tokio::test] +async fn validate_unencrypted() -> Result<(), Error> { + // Empty keyring validates as unencrypted + let data_dir = tempdir()?; + let empty = UnlockedKeyring::open_at(data_dir.path(), "empty", None).await?; + assert!(empty.validate_unencrypted().await?); + + // Unencrypted keyring with items validates + let plain = UnlockedKeyring::open_at(data_dir.path(), "plain", None).await?; + plain + .create_item("Item", &[("a", "b")], "secret", false) + .await?; + assert!(plain.validate_unencrypted().await?); + + // Encrypted keyring does not validate as unencrypted + let secret = Secret::from([1, 2].into_iter().cycle().take(64).collect::>()); + let encrypted = UnlockedKeyring::open_at(data_dir.path(), "enc", Some(secret)).await?; + encrypted + .create_item("Item", &[("a", "b")], "secret", false) + .await?; + assert!(!encrypted.validate_unencrypted().await?); + + Ok(()) +} diff --git a/server/src/service/mod.rs b/server/src/service/mod.rs index 9e61205fe..64409eb41 100644 --- a/server/src/service/mod.rs +++ b/server/src/service/mod.rs @@ -1624,3 +1624,5 @@ impl Service { #[cfg(test)] mod tests; +#[cfg(test)] +mod unencrypted_tests; diff --git a/server/src/service/unencrypted_tests.rs b/server/src/service/unencrypted_tests.rs new file mode 100644 index 000000000..80f1eca4e --- /dev/null +++ b/server/src/service/unencrypted_tests.rs @@ -0,0 +1,114 @@ +use oo7::{Secret, file::UnlockedKeyring}; +use zbus::zvariant::ObjectPath; + +use crate::tests::TestServiceSetup; + +#[tokio::test] +async fn discover_unencrypted_keyrings() -> Result<(), Box> { + let temp_dir = tempfile::tempdir()?; + + let v1_dir = temp_dir.path().join("keyrings/v1"); + tokio::fs::create_dir_all(&v1_dir).await?; + + // Create an unencrypted keyring with items on disk + let keyring = UnlockedKeyring::open_at(temp_dir.path(), "nopass", None).await?; + keyring + .create_item( + "Unencrypted Item", + &[("app", "test-unenc")], + Secret::text("plain-secret"), + false, + ) + .await?; + keyring.write().await?; + + // Discover keyrings without any secret — should find it locked + let setup = + TestServiceSetup::with_disk_keyrings(temp_dir.path().to_path_buf(), None, None).await?; + + let collections = setup.server.collections.lock().await; + let mut nopass_collection = None; + for collection in collections.values() { + if collection.label().await == "Nopass" { + nopass_collection = Some(collection.clone()); + break; + } + } + let nopass_collection = nopass_collection.expect("nopass collection should be discovered"); + assert!( + nopass_collection.is_locked().await, + "Should be locked initially" + ); + drop(collections); + + // Unlock with None (unencrypted) + nopass_collection.set_locked(false, None).await?; + assert!( + !nopass_collection.is_locked().await, + "Should be unlocked after set_locked(false, None)" + ); + + // Verify items are accessible + let keyring_guard = nopass_collection.keyring.read().await; + let unlocked = keyring_guard.as_ref().unwrap().as_unlocked(); + let items = unlocked.search_items(&[("app", "test-unenc")]).await?; + assert_eq!(items.len(), 1); + assert_eq!(items[0].label(), "Unencrypted Item"); + assert_eq!(items[0].secret(), Secret::text("plain-secret")); + + Ok(()) +} + +#[tokio::test] +async fn complete_collection_creation_unencrypted() -> Result<(), Box> { + let temp_dir = tempfile::tempdir()?; + + let v1_dir = temp_dir.path().join("keyrings/v1"); + tokio::fs::create_dir_all(&v1_dir).await?; + + let setup = + TestServiceSetup::with_disk_keyrings(temp_dir.path().to_path_buf(), None, None).await?; + + // Register a pending collection creation + let prompt_path = ObjectPath::try_from("/org/freedesktop/secrets/prompt/p_unenc_test").unwrap(); + setup.server.pending_collections.lock().await.insert( + prompt_path.to_owned().into(), + ("Unenc".into(), "unenc".into()), + ); + + // Complete with None secret + let collection_path = setup + .server + .complete_collection_creation(&prompt_path, None) + .await?; + + // Verify the collection exists and is unlocked + let collection = setup + .server + .collection_from_path(&ObjectPath::try_from(collection_path.as_str()).unwrap()) + .await + .expect("collection should exist"); + + assert!( + !collection.is_locked().await, + "Newly created unencrypted collection should be unlocked" + ); + + // Add an item and verify it works + let keyring_guard = collection.keyring.read().await; + let unlocked = keyring_guard.as_ref().unwrap().as_unlocked(); + unlocked + .create_item( + "Test Item", + &[("created", "unenc-test")], + Secret::text("test-value"), + false, + ) + .await?; + + let items = unlocked.search_items(&[("created", "unenc-test")]).await?; + assert_eq!(items.len(), 1); + assert_eq!(items[0].secret(), Secret::text("test-value")); + + Ok(()) +} From 25bfd43b286e6b828a6ea480a9af17f10a2f8e16 Mon Sep 17 00:00:00 2001 From: Bilal Elmoussaoui Date: Sat, 27 Jun 2026 07:43:45 +0200 Subject: [PATCH 4/4] cli: Support unencrypted keyrings as well --- cli/src/main.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/cli/src/main.rs b/cli/src/main.rs index f1671b56c..14414c5bf 100644 --- a/cli/src/main.rs +++ b/cli/src/main.rs @@ -424,8 +424,9 @@ impl Commands { (None, keyring) } - (Some(_), None) => { - return Err(Error::new("A keyring requires a secret.")); + (Some(path), None) => { + let keyring = Keyring::File(oo7::file::UnlockedKeyring::load(path, None).await?); + (None, keyring) } (None, Some(_)) => { return Err(Error::new("A secret requires a keyring."));