From cce6436733749e32719d0897b7bf5c977abb8af9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20DOUIN?= Date: Fri, 11 Oct 2024 11:03:57 +0200 Subject: [PATCH 01/18] make secret service use keyutils when available --- src/secret_service.rs | 88 +++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 84 insertions(+), 4 deletions(-) diff --git a/src/secret_service.rs b/src/secret_service.rs index a27870e..07b4234 100644 --- a/src/secret_service.rs +++ b/src/secret_service.rs @@ -89,7 +89,7 @@ issue for more details and possible workarounds. */ use std::collections::HashMap; -#[cfg(not(feature = "async-secret-service"))] +#[cfg(feature = "sync-secret-service")] use dbus_secret_service::{Collection, EncryptionType, Error, Item, SecretService}; #[cfg(feature = "async-secret-service")] use secret_service::{ @@ -97,6 +97,9 @@ use secret_service::{ EncryptionType, Error, }; +#[cfg(all(target_os = "linux", feature = "linux-native"))] +use crate::keyutils::KeyutilsCredential; + use super::credential::{Credential, CredentialApi, CredentialBuilder, CredentialBuilderApi}; use super::error::{decode_password, Error as ErrorCode, Result}; @@ -113,6 +116,8 @@ pub struct SsCredential { pub attributes: HashMap, pub label: String, target: Option, + #[cfg(all(target_os = "linux", feature = "linux-native"))] + keyutils: Option, } impl CredentialApi for SsCredential { @@ -144,7 +149,14 @@ impl CredentialApi for SsCredential { let ss = SecretService::connect(session_type).map_err(platform_failure)?; // first try to find a unique, existing, matching item and set its password match self.map_matching_items(|i| set_item_secret(i, secret), true) { - Ok(_) => return Ok(()), + Ok(_) => { + #[cfg(all(target_os = "linux", feature = "linux-native"))] + if let Some(keyutils) = &self.keyutils { + let _ = keyutils.set_secret(secret); + } + + return Ok(()); + } Err(ErrorCode::NoEntry) => {} Err(err) => return Err(err), } @@ -163,6 +175,12 @@ impl CredentialApi for SsCredential { "text/plain", ) .map_err(platform_failure)?; + + #[cfg(all(target_os = "linux", feature = "linux-native"))] + if let Some(keyutils) = &self.keyutils { + let _ = keyutils.set_secret(secret); + } + Ok(()) } @@ -174,6 +192,13 @@ impl CredentialApi for SsCredential { /// returns an [Ambiguous](ErrorCode::Ambiguous) /// error with a credential for each matching item. fn get_password(&self) -> Result { + #[cfg(all(target_os = "linux", feature = "linux-native"))] + if let Some(keyutils) = &self.keyutils { + if let Ok(password) = keyutils.get_password() { + return Ok(password); + } + } + let passwords: Vec = self.map_matching_items(get_item_password, true)?; Ok(passwords[0].clone()) } @@ -186,7 +211,15 @@ impl CredentialApi for SsCredential { /// returns an [Ambiguous](ErrorCode::Ambiguous) /// error with a credential for each matching item. fn get_secret(&self) -> Result> { + #[cfg(all(target_os = "linux", feature = "linux-native"))] + if let Some(keyutils) = &self.keyutils { + if let Ok(secret) = keyutils.get_secret() { + return Ok(secret); + } + } + let secrets: Vec> = self.map_matching_items(get_item_secret, true)?; + Ok(secrets[0].clone()) } @@ -212,6 +245,12 @@ impl CredentialApi for SsCredential { /// error with a credential for each matching item. fn delete_credential(&self) -> Result<()> { self.map_matching_items(delete_item, true)?; + + #[cfg(all(target_os = "linux", feature = "linux-native"))] + if let Some(keyutils) = &self.keyutils { + let _ = keyutils.delete_credential(); + } + Ok(()) } @@ -240,6 +279,10 @@ impl SsCredential { if let Some("") = target { return Err(empty_target()); } + + #[cfg(all(target_os = "linux", feature = "linux-native"))] + let keyutils = KeyutilsCredential::new_with_target(target, service, user).ok(); + let target = target.unwrap_or("default"); let attributes = HashMap::from([ @@ -248,6 +291,7 @@ impl SsCredential { ("target".to_string(), target.to_string()), ("application".to_string(), "rust-keyring".to_string()), ]); + Ok(Self { attributes, label: format!( @@ -255,6 +299,8 @@ impl SsCredential { env!("CARGO_PKG_VERSION"), ), target: Some(target.to_string()), + #[cfg(all(target_os = "linux", feature = "linux-native"))] + keyutils, }) } @@ -263,6 +309,9 @@ impl SsCredential { /// This emulates what keyring v1 did, and can be very handy when you need to /// access an old v1 credential that's in your secret service default collection. pub fn new_with_no_target(service: &str, user: &str) -> Result { + #[cfg(all(target_os = "linux", feature = "linux-native"))] + let keyutils = KeyutilsCredential::new_with_target(None, service, user).ok(); + let attributes = HashMap::from([ ("service".to_string(), service.to_string()), ("username".to_string(), user.to_string()), @@ -275,20 +324,51 @@ impl SsCredential { env!("CARGO_PKG_VERSION"), ), target: None, + #[cfg(all(target_os = "linux", feature = "linux-native"))] + keyutils, }) } + /// Create a keyutils credential from an underlying item's attributes. + /// + /// The created credential will have all the attributes and label + /// of the underlying item, so you can examine them. + #[cfg(all(target_os = "linux", feature = "linux-native"))] + pub fn new_keyutils_from_attributes( + attributes: &HashMap, + ) -> Result { + let target = attributes.get("target").map(|target| target.as_str()); + let service = attributes + .get("service") + .ok_or(decode_error(Error::NoResult))?; + let user = attributes + .get("username") + .ok_or(decode_error(Error::NoResult))?; + + let keyutils = KeyutilsCredential::new_with_target(target, service.as_str(), user.as_str()) + .map_err(|err| crate::Error::NoStorageAccess(Box::new(err)))?; + + Ok(keyutils) + } + /// Create a credential from an underlying item. /// /// The created credential will have all the attributes and label /// of the underlying item, so you can examine them. pub fn new_from_item(item: &Item) -> Result { let attributes = item.get_attributes().map_err(decode_error)?; + + #[cfg(all(target_os = "linux", feature = "linux-native"))] + let keyutils = Self::new_keyutils_from_attributes(&attributes).ok(); + let target = attributes.get("target").cloned(); + Ok(Self { attributes, label: item.get_label().map_err(decode_error)?, target, + #[cfg(all(target_os = "linux", feature = "linux-native"))] + keyutils, }) } @@ -844,7 +924,7 @@ mod tests { } fn delete_collection(name: &str) { - #[cfg(not(feature = "async-secret-service"))] + #[cfg(feature = "sync-secret-service")] use dbus_secret_service::{EncryptionType, SecretService}; #[cfg(feature = "async-secret-service")] use secret_service::{blocking::SecretService, EncryptionType}; @@ -856,7 +936,7 @@ mod tests { } fn create_v1_entry(name: &str, password: &str) { - #[cfg(not(feature = "async-secret-service"))] + #[cfg(feature = "sync-secret-service")] use dbus_secret_service::{EncryptionType, SecretService}; #[cfg(feature = "async-secret-service")] use secret_service::{blocking::SecretService, EncryptionType}; From 3261e24c69ee4bec556d48c756dc904999bb0082 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20DOUIN?= Date: Sun, 13 Oct 2024 10:46:20 +0200 Subject: [PATCH 02/18] move ss + keyutils to a dedicated keystore --- .github/workflows/ci.yaml | 13 +- Cargo.toml | 11 +- src/lib.rs | 53 ++++---- src/secret_service.rs | 83 +------------ src/secret_service_with_keyutils.rs | 180 ++++++++++++++++++++++++++++ 5 files changed, 228 insertions(+), 112 deletions(-) create mode 100644 src/secret_service_with_keyutils.rs diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 7419f3d..56aba48 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -35,19 +35,18 @@ jobs: run: cargo clippy -- -D warnings - name: Build and Test - run: cargo test --features=apple-native,windows-native --verbose + run: cargo test --features=default-apple,default-windows --verbose - name: Build the CLI release - run: cargo build --release --features=apple-native,windows-native --example keyring-cli + run: cargo build --release --features=default-apple,default-windows --example keyring-cli ci_nix: runs-on: ubuntu-latest strategy: matrix: features: - - "linux-native" - - "sync-secret-service" - - "linux-native,sync-secret-service" + - "default-linux,sync-secret-service" + - "default-linux-headless,sync-secret-service" - "sync-secret-service,crypto-rust" - "sync-secret-service,crypto-openssl" - "async-secret-service,tokio,crypto-rust" @@ -102,7 +101,7 @@ jobs: target: aarch64-apple-ios - name: Build iOS library - run: cargo build --target aarch64-apple-ios --features=apple-native --example=iostest + run: cargo build --target aarch64-apple-ios --features=default-apple --example=iostest msrv_native: runs-on: ubuntu-latest @@ -118,4 +117,4 @@ jobs: components: clippy - name: Clippy check - run: cargo clippy --features=linux-native -- -D warnings + run: cargo clippy --features=default-linux -- -D warnings diff --git a/Cargo.toml b/Cargo.toml index 1dc7919..c52bf3f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -13,12 +13,19 @@ exclude = [".github/"] readme = "README.md" [features] +default-linux = ["linux-native", "secret-service"] +default-linux-headless = ["linux-native"] +default-bsd = ["secret-service"] +default-apple = ["apple-native"] +default-windows = ["windows-native"] + linux-native = ["dep:linux-keyutils"] apple-native = ["dep:security-framework"] windows-native = ["dep:windows-sys", "dep:byteorder"] -sync-secret-service = ["dep:dbus-secret-service"] -async-secret-service = ["dep:secret-service", "dep:zbus"] +secret-service = [] +sync-secret-service = ["secret-service", "dep:dbus-secret-service"] +async-secret-service = ["secret-service", "dep:secret-service", "dep:zbus"] crypto-rust = ["dbus-secret-service?/crypto-rust", "secret-service?/crypto-rust"] crypto-openssl = ["dbus-secret-service?/crypto-openssl", "secret-service?/crypto-openssl"] tokio = ["zbus?/tokio"] diff --git a/src/lib.rs b/src/lib.rs index 3677623..955caf5 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -185,46 +185,53 @@ pub mod mock; #[cfg(all(feature = "sync-secret-service", feature = "async-secret-service"))] compile_error!("This crate cannot use the secret-service both synchronously and asynchronously"); +// +// can't use both sync and async secret service +// +#[cfg(all( + feature = "secret-service", + not(any(feature = "sync-secret-service", feature = "async-secret-service")) +))] +compile_error!("This crate cannot use the secret-service without an explicit flavor: sync-secret-service or async-secret-service"); + // // pick the *nix keystore // #[cfg(all(target_os = "linux", feature = "linux-native"))] pub mod keyutils; -// use keyutils as default if secret-service is not available -#[cfg(all( - target_os = "linux", - feature = "linux-native", - not(any(feature = "sync-secret-service", feature = "async-secret-service")) -))] +#[cfg(all(target_os = "linux", feature = "default-linux-headless"))] pub use keyutils as default; #[cfg(all( any(target_os = "linux", target_os = "freebsd", target_os = "openbsd"), - any(feature = "sync-secret-service", feature = "async-secret-service") + feature = "secret-service", ))] pub mod secret_service; -// use secret-service as default if it's available #[cfg(all( - any(target_os = "linux", target_os = "freebsd", target_os = "openbsd"), - any(feature = "sync-secret-service", feature = "async-secret-service"), + any(target_os = "freebsd", target_os = "openbsd"), + feature = "default-bsd", ))] pub use secret_service as default; +#[cfg(all( + target_os = "linux", + all(feature = "linux-native", feature = "secret-service"), +))] +pub mod secret_service_with_keyutils; +#[cfg(all(target_os = "linux", feature = "default-linux"))] +pub use secret_service_with_keyutils as default; + // fallback to mock if neither keyutils nor secret service is available #[cfg(any( all( target_os = "linux", - not(any( - feature = "linux-native", - feature = "sync-secret-service", - feature = "async-secret-service" - )) + not(any(feature = "default-linux", feature = "default-linux-headless")) ), all( any(target_os = "freebsd", target_os = "openbsd"), - not(any(feature = "sync-secret-service", feature = "async-secret-service")) - ) + not(feature = "default-bsd") + ), ))] pub use mock as default; @@ -233,16 +240,16 @@ pub use mock as default; // #[cfg(all(target_os = "macos", feature = "apple-native"))] pub mod macos; -#[cfg(all(target_os = "macos", feature = "apple-native"))] +#[cfg(all(target_os = "macos", feature = "default-apple"))] pub use macos as default; -#[cfg(all(target_os = "macos", not(feature = "apple-native")))] +#[cfg(all(target_os = "macos", not(feature = "default-apple")))] pub use mock as default; #[cfg(all(target_os = "ios", feature = "apple-native"))] pub mod ios; -#[cfg(all(target_os = "ios", feature = "apple-native"))] +#[cfg(all(target_os = "ios", feature = "default-apple"))] pub use ios as default; -#[cfg(all(target_os = "ios", not(feature = "apple-native")))] +#[cfg(all(target_os = "ios", not(feature = "default-apple")))] pub use mock as default; // @@ -251,9 +258,9 @@ pub use mock as default; #[cfg(all(target_os = "windows", feature = "windows-native"))] pub mod windows; -#[cfg(all(target_os = "windows", not(feature = "windows-native")))] +#[cfg(all(target_os = "windows", not(feature = "default-windows")))] pub use mock as default; -#[cfg(all(target_os = "windows", feature = "windows-native"))] +#[cfg(all(target_os = "windows", feature = "default-windows"))] pub use windows as default; #[cfg(not(any( diff --git a/src/secret_service.rs b/src/secret_service.rs index 07b4234..ff9fb53 100644 --- a/src/secret_service.rs +++ b/src/secret_service.rs @@ -97,9 +97,6 @@ use secret_service::{ EncryptionType, Error, }; -#[cfg(all(target_os = "linux", feature = "linux-native"))] -use crate::keyutils::KeyutilsCredential; - use super::credential::{Credential, CredentialApi, CredentialBuilder, CredentialBuilderApi}; use super::error::{decode_password, Error as ErrorCode, Result}; @@ -116,8 +113,6 @@ pub struct SsCredential { pub attributes: HashMap, pub label: String, target: Option, - #[cfg(all(target_os = "linux", feature = "linux-native"))] - keyutils: Option, } impl CredentialApi for SsCredential { @@ -149,14 +144,7 @@ impl CredentialApi for SsCredential { let ss = SecretService::connect(session_type).map_err(platform_failure)?; // first try to find a unique, existing, matching item and set its password match self.map_matching_items(|i| set_item_secret(i, secret), true) { - Ok(_) => { - #[cfg(all(target_os = "linux", feature = "linux-native"))] - if let Some(keyutils) = &self.keyutils { - let _ = keyutils.set_secret(secret); - } - - return Ok(()); - } + Ok(_) => return Ok(()), Err(ErrorCode::NoEntry) => {} Err(err) => return Err(err), } @@ -176,11 +164,6 @@ impl CredentialApi for SsCredential { ) .map_err(platform_failure)?; - #[cfg(all(target_os = "linux", feature = "linux-native"))] - if let Some(keyutils) = &self.keyutils { - let _ = keyutils.set_secret(secret); - } - Ok(()) } @@ -192,13 +175,6 @@ impl CredentialApi for SsCredential { /// returns an [Ambiguous](ErrorCode::Ambiguous) /// error with a credential for each matching item. fn get_password(&self) -> Result { - #[cfg(all(target_os = "linux", feature = "linux-native"))] - if let Some(keyutils) = &self.keyutils { - if let Ok(password) = keyutils.get_password() { - return Ok(password); - } - } - let passwords: Vec = self.map_matching_items(get_item_password, true)?; Ok(passwords[0].clone()) } @@ -211,15 +187,7 @@ impl CredentialApi for SsCredential { /// returns an [Ambiguous](ErrorCode::Ambiguous) /// error with a credential for each matching item. fn get_secret(&self) -> Result> { - #[cfg(all(target_os = "linux", feature = "linux-native"))] - if let Some(keyutils) = &self.keyutils { - if let Ok(secret) = keyutils.get_secret() { - return Ok(secret); - } - } - let secrets: Vec> = self.map_matching_items(get_item_secret, true)?; - Ok(secrets[0].clone()) } @@ -245,12 +213,6 @@ impl CredentialApi for SsCredential { /// error with a credential for each matching item. fn delete_credential(&self) -> Result<()> { self.map_matching_items(delete_item, true)?; - - #[cfg(all(target_os = "linux", feature = "linux-native"))] - if let Some(keyutils) = &self.keyutils { - let _ = keyutils.delete_credential(); - } - Ok(()) } @@ -280,9 +242,6 @@ impl SsCredential { return Err(empty_target()); } - #[cfg(all(target_os = "linux", feature = "linux-native"))] - let keyutils = KeyutilsCredential::new_with_target(target, service, user).ok(); - let target = target.unwrap_or("default"); let attributes = HashMap::from([ @@ -299,8 +258,6 @@ impl SsCredential { env!("CARGO_PKG_VERSION"), ), target: Some(target.to_string()), - #[cfg(all(target_os = "linux", feature = "linux-native"))] - keyutils, }) } @@ -309,9 +266,6 @@ impl SsCredential { /// This emulates what keyring v1 did, and can be very handy when you need to /// access an old v1 credential that's in your secret service default collection. pub fn new_with_no_target(service: &str, user: &str) -> Result { - #[cfg(all(target_os = "linux", feature = "linux-native"))] - let keyutils = KeyutilsCredential::new_with_target(None, service, user).ok(); - let attributes = HashMap::from([ ("service".to_string(), service.to_string()), ("username".to_string(), user.to_string()), @@ -324,51 +278,20 @@ impl SsCredential { env!("CARGO_PKG_VERSION"), ), target: None, - #[cfg(all(target_os = "linux", feature = "linux-native"))] - keyutils, }) } - /// Create a keyutils credential from an underlying item's attributes. - /// - /// The created credential will have all the attributes and label - /// of the underlying item, so you can examine them. - #[cfg(all(target_os = "linux", feature = "linux-native"))] - pub fn new_keyutils_from_attributes( - attributes: &HashMap, - ) -> Result { - let target = attributes.get("target").map(|target| target.as_str()); - let service = attributes - .get("service") - .ok_or(decode_error(Error::NoResult))?; - let user = attributes - .get("username") - .ok_or(decode_error(Error::NoResult))?; - - let keyutils = KeyutilsCredential::new_with_target(target, service.as_str(), user.as_str()) - .map_err(|err| crate::Error::NoStorageAccess(Box::new(err)))?; - - Ok(keyutils) - } - /// Create a credential from an underlying item. /// /// The created credential will have all the attributes and label /// of the underlying item, so you can examine them. pub fn new_from_item(item: &Item) -> Result { let attributes = item.get_attributes().map_err(decode_error)?; - - #[cfg(all(target_os = "linux", feature = "linux-native"))] - let keyutils = Self::new_keyutils_from_attributes(&attributes).ok(); - let target = attributes.get("target").cloned(); - Ok(Self { attributes, label: item.get_label().map_err(decode_error)?, target, - #[cfg(all(target_os = "linux", feature = "linux-native"))] - keyutils, }) } @@ -504,7 +427,7 @@ impl SsCredential { /// of the credential much easier. But since the secret service expects /// a map from &str to &str, we have this utility to transform the /// credential's map into one of the right form. - fn all_attributes(&self) -> HashMap<&str, &str> { + pub(crate) fn all_attributes(&self) -> HashMap<&str, &str> { self.attributes .iter() .map(|(k, v)| (k.as_str(), v.as_str())) @@ -513,7 +436,7 @@ impl SsCredential { /// Similar to [all_attributes](SsCredential::all_attributes), /// but this just selects the ones we search on - fn search_attributes(&self, omit_target: bool) -> HashMap<&str, &str> { + pub(crate) fn search_attributes(&self, omit_target: bool) -> HashMap<&str, &str> { let mut result: HashMap<&str, &str> = HashMap::new(); if self.target.is_some() && !omit_target { result.insert("target", self.attributes["target"].as_str()); diff --git a/src/secret_service_with_keyutils.rs b/src/secret_service_with_keyutils.rs new file mode 100644 index 0000000..f444102 --- /dev/null +++ b/src/secret_service_with_keyutils.rs @@ -0,0 +1,180 @@ +/*! + +# secret-service-with-keyutils credential store + +TODO + + */ +use std::collections::HashMap; + +#[cfg(feature = "sync-secret-service")] +use dbus_secret_service::{Error, Item}; +#[cfg(feature = "async-secret-service")] +use secret_service::{blocking::Item, Error}; + +#[cfg(all(target_os = "linux", feature = "linux-native"))] +use crate::keyutils::KeyutilsCredential; +use crate::secret_service::{decode_error, SsCredential}; + +use super::credential::{Credential, CredentialApi, CredentialBuilder, CredentialBuilderApi}; +use super::error::Result; + +#[derive(Debug, Clone)] +pub struct SsKeyutilsCredential { + keyutils: Option, + ss: SsCredential, +} + +impl CredentialApi for SsKeyutilsCredential { + fn set_password(&self, password: &str) -> Result<()> { + self.set_secret(password.as_bytes()) + } + + fn set_secret(&self, secret: &[u8]) -> Result<()> { + self.ss.set_secret(secret)?; + + if let Some(keyutils) = &self.keyutils { + let _ = keyutils.set_secret(secret); + } + + Ok(()) + } + + fn get_password(&self) -> Result { + if let Some(keyutils) = &self.keyutils { + if let Ok(password) = keyutils.get_password() { + return Ok(password); + } + } + + self.ss.get_password() + } + + fn get_secret(&self) -> Result> { + if let Some(keyutils) = &self.keyutils { + if let Ok(secret) = keyutils.get_secret() { + return Ok(secret); + } + } + + self.ss.get_secret() + } + + fn get_attributes(&self) -> Result> { + self.ss.get_attributes() + } + + fn update_attributes(&self, attributes: &HashMap<&str, &str>) -> Result<()> { + self.ss.update_attributes(attributes) + } + + fn delete_credential(&self) -> Result<()> { + self.ss.delete_credential()?; + + if let Some(keyutils) = &self.keyutils { + let _ = keyutils.delete_credential(); + } + + Ok(()) + } + + fn as_any(&self) -> &dyn std::any::Any { + self + } + + fn debug_fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + std::fmt::Debug::fmt(self, f) + } +} + +impl KeyutilsCredential { + /// Create a keyutils credential from an underlying item's attributes. + /// + /// The created credential will have all the attributes and label + /// of the underlying item, so you can examine them. + pub fn new_from_item(item: &Item) -> Result { + let attributes = item.get_attributes().map_err(decode_error)?; + + let target = attributes.get("target").map(|target| target.as_str()); + let service = attributes + .get("service") + .ok_or(decode_error(Error::NoResult))?; + let user = attributes + .get("username") + .ok_or(decode_error(Error::NoResult))?; + + let keyutils = KeyutilsCredential::new_with_target(target, service.as_str(), user.as_str()) + .map_err(|err| crate::Error::NoStorageAccess(Box::new(err)))?; + + Ok(keyutils) + } +} + +impl SsKeyutilsCredential { + pub fn new_with_target(target: Option<&str>, service: &str, user: &str) -> Result { + let ss = SsCredential::new_with_target(target, service, user)?; + let keyutils = KeyutilsCredential::new_with_target(target, service, user).ok(); + Ok(Self { keyutils, ss }) + } + + pub fn new_with_no_target(service: &str, user: &str) -> Result { + let keyutils = KeyutilsCredential::new_with_target(None, service, user).ok(); + let ss = SsCredential::new_with_no_target(service, user)?; + Ok(Self { keyutils, ss }) + } + + pub fn new_from_item(item: &Item) -> Result { + let ss = SsCredential::new_from_item(item)?; + let keyutils = KeyutilsCredential::new_from_item(item).ok(); + Ok(Self { keyutils, ss }) + } + + pub fn new_from_matching_item(&self) -> Result { + let ss = self.ss.new_from_matching_item()?; + let keyutils = None; + Ok(Self { keyutils, ss }) + } + + pub fn get_all_passwords(&self) -> Result> { + self.ss.get_all_passwords() + } + + pub fn delete_all_passwords(&self) -> Result<()> { + self.ss.delete_all_passwords() + } + + pub fn all_attributes(&self) -> HashMap<&str, &str> { + self.ss.all_attributes() + } + + pub fn search_attributes(&self, omit_target: bool) -> HashMap<&str, &str> { + self.ss.search_attributes(omit_target) + } +} + +/// The builder for secret-service-with-keyutils credentials +#[derive(Debug, Default)] +pub struct SsKeyutilsCredentialBuilder {} + +/// Returns an instance of the secret-service-with-keyutils credential builder. +/// +/// If secret-service-with-keyutils is the default credential store, +/// this is called once when an entry is first created. +pub fn default_credential_builder() -> Box { + Box::new(SsKeyutilsCredentialBuilder {}) +} + +impl CredentialBuilderApi for SsKeyutilsCredentialBuilder { + /// Build an [SsKeyutilsCredential] for the given target, service, and user. + fn build(&self, target: Option<&str>, service: &str, user: &str) -> Result> { + Ok(Box::new(SsCredential::new_with_target( + target, service, user, + )?)) + } + + /// Return the underlying builder object with an `Any` type so that it can + /// be downgraded to an [SsKeyutilsCredentialBuilder] for platform-specific processing. + fn as_any(&self) -> &dyn std::any::Any { + self + } +} From 4bcef9dcd01c7b217061968d542583e195b56e93 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20DOUIN?= Date: Sun, 13 Oct 2024 10:49:30 +0200 Subject: [PATCH 03/18] clean typos --- src/lib.rs | 10 +++++----- src/secret_service.rs | 3 --- src/secret_service_with_keyutils.rs | 3 ++- 3 files changed, 7 insertions(+), 9 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 955caf5..b5390aa 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -186,11 +186,11 @@ pub mod mock; compile_error!("This crate cannot use the secret-service both synchronously and asynchronously"); // -// can't use both sync and async secret service +// can't use secret service without explicit flavor // #[cfg(all( feature = "secret-service", - not(any(feature = "sync-secret-service", feature = "async-secret-service")) + not(any(feature = "sync-secret-service", feature = "async-secret-service")), ))] compile_error!("This crate cannot use the secret-service without an explicit flavor: sync-secret-service or async-secret-service"); @@ -226,11 +226,11 @@ pub use secret_service_with_keyutils as default; #[cfg(any( all( target_os = "linux", - not(any(feature = "default-linux", feature = "default-linux-headless")) + not(any(feature = "default-linux", feature = "default-linux-headless")), ), all( any(target_os = "freebsd", target_os = "openbsd"), - not(feature = "default-bsd") + not(feature = "default-bsd"), ), ))] pub use mock as default; @@ -562,7 +562,7 @@ mod tests { pub fn generate_random_string_of_len(len: usize) -> String { // from the Rust Cookbook: // https://rust-lang-nursery.github.io/rust-cookbook/algorithms/randomness.html - use rand::{distributions::Alphanumeric, thread_rng, Rng}; + use rand::{distributions::Alphanumeric, thread_rng}; thread_rng() .sample_iter(&Alphanumeric) .take(len) diff --git a/src/secret_service.rs b/src/secret_service.rs index ff9fb53..9764f53 100644 --- a/src/secret_service.rs +++ b/src/secret_service.rs @@ -163,7 +163,6 @@ impl CredentialApi for SsCredential { "text/plain", ) .map_err(platform_failure)?; - Ok(()) } @@ -241,7 +240,6 @@ impl SsCredential { if let Some("") = target { return Err(empty_target()); } - let target = target.unwrap_or("default"); let attributes = HashMap::from([ @@ -250,7 +248,6 @@ impl SsCredential { ("target".to_string(), target.to_string()), ("application".to_string(), "rust-keyring".to_string()), ]); - Ok(Self { attributes, label: format!( diff --git a/src/secret_service_with_keyutils.rs b/src/secret_service_with_keyutils.rs index f444102..88f68bc 100644 --- a/src/secret_service_with_keyutils.rs +++ b/src/secret_service_with_keyutils.rs @@ -88,7 +88,8 @@ impl CredentialApi for SsKeyutilsCredential { } impl KeyutilsCredential { - /// Create a keyutils credential from an underlying item's attributes. + /// Create a keyutils credential from an underlying secret service + /// item's attributes. /// /// The created credential will have all the attributes and label /// of the underlying item, so you can examine them. From e08aeaa9e2612f2852297f7ee36de64a66da13f2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20DOUIN?= Date: Sun, 13 Oct 2024 10:51:29 +0200 Subject: [PATCH 04/18] fix ci --- .github/workflows/ci.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 56aba48..09fb187 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -45,8 +45,8 @@ jobs: strategy: matrix: features: + - "default-linux-headless" - "default-linux,sync-secret-service" - - "default-linux-headless,sync-secret-service" - "sync-secret-service,crypto-rust" - "sync-secret-service,crypto-openssl" - "async-secret-service,tokio,crypto-rust" @@ -117,4 +117,4 @@ jobs: components: clippy - name: Clippy check - run: cargo clippy --features=default-linux -- -D warnings + run: cargo clippy --features=default-linux-headless -- -D warnings From 460a858a62a2d720e12be877ef6cd2285d7109ea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20DOUIN?= Date: Sun, 13 Oct 2024 11:44:18 +0200 Subject: [PATCH 05/18] add condition to prevent multiple default-linux* features --- src/lib.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/lib.rs b/src/lib.rs index b5390aa..d771a8d 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -185,6 +185,12 @@ pub mod mock; #[cfg(all(feature = "sync-secret-service", feature = "async-secret-service"))] compile_error!("This crate cannot use the secret-service both synchronously and asynchronously"); +// +// can't use both default-linux and defaut-linux-headless +// +#[cfg(all(feature = "default-linux", feature = "default-linux-headless"))] +compile_error!("This crate can use only one default-linux* feature"); + // // can't use secret service without explicit flavor // From 62cfba335c9d47a72157a1b6c7bfdb3b33f3b183 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20DOUIN?= Date: Mon, 14 Oct 2024 07:40:08 +0200 Subject: [PATCH 06/18] adjust code after first review - Removed `default-*` features as breaking semver compat is not planned - Added `keyutils` cargo feature - Adjusted compilation condition for default stores - Replaced `keyutils: Option` by `KeyutilsCredential` in `SsKeyutilsCredential` --- Cargo.toml | 10 ++---- src/lib.rs | 50 +++++++++++------------------ src/secret_service_with_keyutils.rs | 48 +++++++++++---------------- 3 files changed, 40 insertions(+), 68 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index c52bf3f..3c7afaa 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -13,17 +13,13 @@ exclude = [".github/"] readme = "README.md" [features] -default-linux = ["linux-native", "secret-service"] -default-linux-headless = ["linux-native"] -default-bsd = ["secret-service"] -default-apple = ["apple-native"] -default-windows = ["windows-native"] - -linux-native = ["dep:linux-keyutils"] +linux-native = ["keyutils"] apple-native = ["dep:security-framework"] windows-native = ["dep:windows-sys", "dep:byteorder"] +keyutils = ["dep:linux-keyutils"] secret-service = [] +secret-service-with-keyutils = ["secret-service", "keyutils"] sync-secret-service = ["secret-service", "dep:dbus-secret-service"] async-secret-service = ["secret-service", "dep:secret-service", "dep:zbus"] crypto-rust = ["dbus-secret-service?/crypto-rust", "secret-service?/crypto-rust"] diff --git a/src/lib.rs b/src/lib.rs index d771a8d..fa9d2dd 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -185,12 +185,6 @@ pub mod mock; #[cfg(all(feature = "sync-secret-service", feature = "async-secret-service"))] compile_error!("This crate cannot use the secret-service both synchronously and asynchronously"); -// -// can't use both default-linux and defaut-linux-headless -// -#[cfg(all(feature = "default-linux", feature = "default-linux-headless"))] -compile_error!("This crate can use only one default-linux* feature"); - // // can't use secret service without explicit flavor // @@ -203,10 +197,13 @@ compile_error!("This crate cannot use the secret-service without an explicit fla // // pick the *nix keystore // - -#[cfg(all(target_os = "linux", feature = "linux-native"))] +#[cfg(all(target_os = "linux", feature = "keyutils"))] pub mod keyutils; -#[cfg(all(target_os = "linux", feature = "default-linux-headless"))] +#[cfg(all( + target_os = "linux", + feature = "linux-native", + not(feature = "secret-service"), +))] pub use keyutils as default; #[cfg(all( @@ -216,28 +213,20 @@ pub use keyutils as default; pub mod secret_service; #[cfg(all( any(target_os = "freebsd", target_os = "openbsd"), - feature = "default-bsd", + feature = "secret-service", + not(feature = "keyutils"), ))] pub use secret_service as default; -#[cfg(all( - target_os = "linux", - all(feature = "linux-native", feature = "secret-service"), -))] +#[cfg(all(target_os = "linux", feature = "secret-service-with-keyutils"))] pub mod secret_service_with_keyutils; -#[cfg(all(target_os = "linux", feature = "default-linux"))] +#[cfg(all(target_os = "linux", feature = "secret-service-with-keyutils"))] pub use secret_service_with_keyutils as default; // fallback to mock if neither keyutils nor secret service is available -#[cfg(any( - all( - target_os = "linux", - not(any(feature = "default-linux", feature = "default-linux-headless")), - ), - all( - any(target_os = "freebsd", target_os = "openbsd"), - not(feature = "default-bsd"), - ), +#[cfg(all( + any(target_os = "linux", target_os = "freebsd", target_os = "openbsd"), + not(any(feature = "keyutils", feature = "secret-service")), ))] pub use mock as default; @@ -246,27 +235,26 @@ pub use mock as default; // #[cfg(all(target_os = "macos", feature = "apple-native"))] pub mod macos; -#[cfg(all(target_os = "macos", feature = "default-apple"))] +#[cfg(all(target_os = "macos", feature = "apple-native"))] pub use macos as default; -#[cfg(all(target_os = "macos", not(feature = "default-apple")))] +#[cfg(all(target_os = "macos", not(feature = "apple-native")))] pub use mock as default; #[cfg(all(target_os = "ios", feature = "apple-native"))] pub mod ios; -#[cfg(all(target_os = "ios", feature = "default-apple"))] +#[cfg(all(target_os = "ios", feature = "apple-native"))] pub use ios as default; -#[cfg(all(target_os = "ios", not(feature = "default-apple")))] +#[cfg(all(target_os = "ios", not(feature = "apple-native")))] pub use mock as default; // // pick the Windows keystore // - #[cfg(all(target_os = "windows", feature = "windows-native"))] pub mod windows; -#[cfg(all(target_os = "windows", not(feature = "default-windows")))] +#[cfg(all(target_os = "windows", not(feature = "windows-native")))] pub use mock as default; -#[cfg(all(target_os = "windows", feature = "default-windows"))] +#[cfg(all(target_os = "windows", feature = "windows-native"))] pub use windows as default; #[cfg(not(any( diff --git a/src/secret_service_with_keyutils.rs b/src/secret_service_with_keyutils.rs index 88f68bc..6081cff 100644 --- a/src/secret_service_with_keyutils.rs +++ b/src/secret_service_with_keyutils.rs @@ -21,7 +21,7 @@ use super::error::Result; #[derive(Debug, Clone)] pub struct SsKeyutilsCredential { - keyutils: Option, + keyutils: KeyutilsCredential, ss: SsCredential, } @@ -32,32 +32,30 @@ impl CredentialApi for SsKeyutilsCredential { fn set_secret(&self, secret: &[u8]) -> Result<()> { self.ss.set_secret(secret)?; - - if let Some(keyutils) = &self.keyutils { - let _ = keyutils.set_secret(secret); - } - + let _ = self.keyutils.set_secret(secret); Ok(()) } fn get_password(&self) -> Result { - if let Some(keyutils) = &self.keyutils { - if let Ok(password) = keyutils.get_password() { - return Ok(password); - } + if let Ok(password) = self.keyutils.get_password() { + return Ok(password); } - self.ss.get_password() + let password = self.ss.get_password()?; + let _ = self.keyutils.set_password(&password); + + Ok(password) } fn get_secret(&self) -> Result> { - if let Some(keyutils) = &self.keyutils { - if let Ok(secret) = keyutils.get_secret() { - return Ok(secret); - } + if let Ok(secret) = self.keyutils.get_secret() { + return Ok(secret); } - self.ss.get_secret() + let secret = self.ss.get_secret()?; + let _ = self.keyutils.set_secret(&secret); + + Ok(secret) } fn get_attributes(&self) -> Result> { @@ -70,11 +68,7 @@ impl CredentialApi for SsKeyutilsCredential { fn delete_credential(&self) -> Result<()> { self.ss.delete_credential()?; - - if let Some(keyutils) = &self.keyutils { - let _ = keyutils.delete_credential(); - } - + let _ = self.keyutils.delete_credential(); Ok(()) } @@ -114,25 +108,19 @@ impl KeyutilsCredential { impl SsKeyutilsCredential { pub fn new_with_target(target: Option<&str>, service: &str, user: &str) -> Result { let ss = SsCredential::new_with_target(target, service, user)?; - let keyutils = KeyutilsCredential::new_with_target(target, service, user).ok(); + let keyutils = KeyutilsCredential::new_with_target(target, service, user)?; Ok(Self { keyutils, ss }) } pub fn new_with_no_target(service: &str, user: &str) -> Result { - let keyutils = KeyutilsCredential::new_with_target(None, service, user).ok(); + let keyutils = KeyutilsCredential::new_with_target(None, service, user)?; let ss = SsCredential::new_with_no_target(service, user)?; Ok(Self { keyutils, ss }) } pub fn new_from_item(item: &Item) -> Result { let ss = SsCredential::new_from_item(item)?; - let keyutils = KeyutilsCredential::new_from_item(item).ok(); - Ok(Self { keyutils, ss }) - } - - pub fn new_from_matching_item(&self) -> Result { - let ss = self.ss.new_from_matching_item()?; - let keyutils = None; + let keyutils = KeyutilsCredential::new_from_item(item)?; Ok(Self { keyutils, ss }) } From cd3ef48c93f1bac68ad28154d2e8c1497ef830ce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20DOUIN?= Date: Mon, 14 Oct 2024 07:43:48 +0200 Subject: [PATCH 07/18] revert ci changes --- .github/workflows/ci.yaml | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 09fb187..7419f3d 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -35,18 +35,19 @@ jobs: run: cargo clippy -- -D warnings - name: Build and Test - run: cargo test --features=default-apple,default-windows --verbose + run: cargo test --features=apple-native,windows-native --verbose - name: Build the CLI release - run: cargo build --release --features=default-apple,default-windows --example keyring-cli + run: cargo build --release --features=apple-native,windows-native --example keyring-cli ci_nix: runs-on: ubuntu-latest strategy: matrix: features: - - "default-linux-headless" - - "default-linux,sync-secret-service" + - "linux-native" + - "sync-secret-service" + - "linux-native,sync-secret-service" - "sync-secret-service,crypto-rust" - "sync-secret-service,crypto-openssl" - "async-secret-service,tokio,crypto-rust" @@ -101,7 +102,7 @@ jobs: target: aarch64-apple-ios - name: Build iOS library - run: cargo build --target aarch64-apple-ios --features=default-apple --example=iostest + run: cargo build --target aarch64-apple-ios --features=apple-native --example=iostest msrv_native: runs-on: ubuntu-latest @@ -117,4 +118,4 @@ jobs: components: clippy - name: Clippy check - run: cargo clippy --features=default-linux-headless -- -D warnings + run: cargo clippy --features=linux-native -- -D warnings From 1a90bc299c3d74c2ccd56f0d085ba922d26e241c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20DOUIN?= Date: Mon, 14 Oct 2024 07:57:10 +0200 Subject: [PATCH 08/18] add missing linux target os is secret service condition --- src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib.rs b/src/lib.rs index fa9d2dd..068e40a 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -212,7 +212,7 @@ pub use keyutils as default; ))] pub mod secret_service; #[cfg(all( - any(target_os = "freebsd", target_os = "openbsd"), + any(target_os = "linux", target_os = "freebsd", target_os = "openbsd"), feature = "secret-service", not(feature = "keyutils"), ))] From 34f21bfd6d02bcc2ef444e8d22ef6fe6b0478575 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20DOUIN?= Date: Mon, 14 Oct 2024 08:04:31 +0200 Subject: [PATCH 09/18] fix default conditions --- Cargo.toml | 2 +- src/lib.rs | 11 +++++++++-- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 3c7afaa..d2d5cef 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -13,7 +13,7 @@ exclude = [".github/"] readme = "README.md" [features] -linux-native = ["keyutils"] +linux-native = ["dep:linux-keyutils"] apple-native = ["dep:security-framework"] windows-native = ["dep:windows-sys", "dep:byteorder"] diff --git a/src/lib.rs b/src/lib.rs index 068e40a..d6fb31f 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -197,7 +197,10 @@ compile_error!("This crate cannot use the secret-service without an explicit fla // // pick the *nix keystore // -#[cfg(all(target_os = "linux", feature = "keyutils"))] +#[cfg(all( + target_os = "linux", + any(feature = "linux-native", feature = "keyutils"), +))] pub mod keyutils; #[cfg(all( target_os = "linux", @@ -226,7 +229,11 @@ pub use secret_service_with_keyutils as default; // fallback to mock if neither keyutils nor secret service is available #[cfg(all( any(target_os = "linux", target_os = "freebsd", target_os = "openbsd"), - not(any(feature = "keyutils", feature = "secret-service")), + not(any( + feature = "linux-native", + feature = "keyutils", + feature = "secret-service", + )), ))] pub use mock as default; From f3f48358f97bf5db6f1696e3c72d16c8e8b544e0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20DOUIN?= Date: Tue, 15 Oct 2024 16:12:56 +0200 Subject: [PATCH 10/18] adjust code after second review --- Cargo.toml | 10 +- src/keyutils_persistent.rs | 140 +++++++++++++++++++++++ src/lib.rs | 55 +++++---- src/secret_service.rs | 10 +- src/secret_service_with_keyutils.rs | 169 ---------------------------- 5 files changed, 182 insertions(+), 202 deletions(-) create mode 100644 src/keyutils_persistent.rs delete mode 100644 src/secret_service_with_keyutils.rs diff --git a/Cargo.toml b/Cargo.toml index d2d5cef..deefd2d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -13,15 +13,15 @@ exclude = [".github/"] readme = "README.md" [features] -linux-native = ["dep:linux-keyutils"] +linux-native = ["keyutils"] apple-native = ["dep:security-framework"] windows-native = ["dep:windows-sys", "dep:byteorder"] keyutils = ["dep:linux-keyutils"] -secret-service = [] -secret-service-with-keyutils = ["secret-service", "keyutils"] -sync-secret-service = ["secret-service", "dep:dbus-secret-service"] -async-secret-service = ["secret-service", "dep:secret-service", "dep:zbus"] +sync-persistent-keyutils = ["keyutils", "sync-secret-service"] +async-persistent-keyutils = ["keyutils", "async-secret-service"] +sync-secret-service = ["dep:dbus-secret-service"] +async-secret-service = ["dep:secret-service", "dep:zbus"] crypto-rust = ["dbus-secret-service?/crypto-rust", "secret-service?/crypto-rust"] crypto-openssl = ["dbus-secret-service?/crypto-openssl", "secret-service?/crypto-openssl"] tokio = ["zbus?/tokio"] diff --git a/src/keyutils_persistent.rs b/src/keyutils_persistent.rs new file mode 100644 index 0000000..44569ca --- /dev/null +++ b/src/keyutils_persistent.rs @@ -0,0 +1,140 @@ +/*! + +# keyutils-persistent credential store + +TODO + + */ +use std::collections::HashMap; + +use crate::credential::{ + Credential, CredentialApi, CredentialBuilder, CredentialBuilderApi, CredentialPersistence, +}; +use crate::keyutils::KeyutilsCredential; +use crate::secret_service::SsCredential; +use crate::{Error, Result}; + +#[derive(Debug, Clone)] +pub struct KeyutilsPersistentCredential { + keyutils: KeyutilsCredential, + ss: SsCredential, +} + +impl CredentialApi for KeyutilsPersistentCredential { + fn set_password(&self, password: &str) -> Result<()> { + self.set_secret(password.as_bytes()) + } + + fn set_secret(&self, secret: &[u8]) -> Result<()> { + let prev_secret = self.keyutils.get_secret()?; + self.keyutils.set_secret(secret)?; + + if let Err(err) = self.ss.set_secret(secret) { + self.keyutils.set_secret(&prev_secret)?; + return Err(err); + } + + Ok(()) + } + + fn get_password(&self) -> Result { + if let Ok(password) = self.keyutils.get_password() { + return Ok(password); + } + + let password = self.ss.get_password().map_err(ambigous_to_no_entry)?; + self.keyutils.set_password(&password)?; + + Ok(password) + } + + fn get_secret(&self) -> Result> { + if let Ok(secret) = self.keyutils.get_secret() { + return Ok(secret); + } + + let secret = self.ss.get_secret().map_err(ambigous_to_no_entry)?; + self.keyutils.set_secret(&secret)?; + + Ok(secret) + } + + fn get_attributes(&self) -> Result> { + let Ok(attributes) = self.ss.get_attributes() else { + return self.keyutils.get_attributes(); + }; + + Ok(attributes) + } + + fn update_attributes(&self, attributes: &HashMap<&str, &str>) -> Result<()> { + let Ok(()) = self.ss.update_attributes(attributes) else { + return self.keyutils.update_attributes(attributes); + }; + + Ok(()) + } + + fn delete_credential(&self) -> Result<()> { + self.keyutils.delete_credential()?; + self.ss.delete_credential()?; + Ok(()) + } + + fn as_any(&self) -> &dyn std::any::Any { + self + } + + fn debug_fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + std::fmt::Debug::fmt(self, f) + } +} + +impl KeyutilsPersistentCredential { + pub fn new_with_target(target: Option<&str>, service: &str, user: &str) -> Result { + let ss = SsCredential::new_with_target(target, service, user)?; + let keyutils = KeyutilsCredential::new_with_target(target, service, user)?; + Ok(Self { keyutils, ss }) + } +} + +/// The builder for secret-service-with-keyutils credentials +#[derive(Debug, Default)] +pub struct KeyutilsPersistentCredentialBuilder {} + +/// Returns an instance of the secret-service-with-keyutils credential builder. +/// +/// If secret-service-with-keyutils is the default credential store, +/// this is called once when an entry is first created. +pub fn default_credential_builder() -> Box { + Box::new(KeyutilsPersistentCredentialBuilder {}) +} + +impl CredentialBuilderApi for KeyutilsPersistentCredentialBuilder { + /// Build an [KeyutilsPersistentCredential] for the given target, service, and user. + fn build(&self, target: Option<&str>, service: &str, user: &str) -> Result> { + Ok(Box::new(SsCredential::new_with_target( + target, service, user, + )?)) + } + + /// Return the underlying builder object with an `Any` type so that it can + /// be downgraded to an [KeyutilsPersistentCredentialBuilder] for platform-specific processing. + fn as_any(&self) -> &dyn std::any::Any { + self + } + + /// Since this keystore keeps credentials in kernel memory, + /// they vanish on reboot + fn persistence(&self) -> CredentialPersistence { + CredentialPersistence::UntilDelete + } +} + +fn ambigous_to_no_entry(err: Error) -> Error { + if let Error::Ambiguous(_) = err { + return Error::NoEntry; + }; + + err +} diff --git a/src/lib.rs b/src/lib.rs index d6fb31f..589c6af 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -182,57 +182,64 @@ pub mod mock; // // can't use both sync and async secret service // -#[cfg(all(feature = "sync-secret-service", feature = "async-secret-service"))] -compile_error!("This crate cannot use the secret-service both synchronously and asynchronously"); - -// -// can't use secret service without explicit flavor -// -#[cfg(all( - feature = "secret-service", - not(any(feature = "sync-secret-service", feature = "async-secret-service")), +#[cfg(any( + all(feature = "sync-secret-service", feature = "async-secret-service"), + all( + feature = "sync-persistent-keyutils", + feature = "async-persistent-keyutils", + ) ))] -compile_error!("This crate cannot use the secret-service without an explicit flavor: sync-secret-service or async-secret-service"); +compile_error!("This crate cannot use the secret-service both synchronously and asynchronously"); // // pick the *nix keystore // -#[cfg(all( - target_os = "linux", - any(feature = "linux-native", feature = "keyutils"), -))] +#[cfg(all(target_os = "linux", feature = "keyutils"))] pub mod keyutils; #[cfg(all( target_os = "linux", feature = "linux-native", - not(feature = "secret-service"), + not(feature = "sync-secret-service"), + not(feature = "async-secret-service"), ))] pub use keyutils as default; #[cfg(all( any(target_os = "linux", target_os = "freebsd", target_os = "openbsd"), - feature = "secret-service", + any(feature = "sync-secret-service", feature = "async-secret-service"), ))] pub mod secret_service; #[cfg(all( any(target_os = "linux", target_os = "freebsd", target_os = "openbsd"), - feature = "secret-service", + any(feature = "sync-secret-service", feature = "async-secret-service"), not(feature = "keyutils"), ))] pub use secret_service as default; -#[cfg(all(target_os = "linux", feature = "secret-service-with-keyutils"))] -pub mod secret_service_with_keyutils; -#[cfg(all(target_os = "linux", feature = "secret-service-with-keyutils"))] -pub use secret_service_with_keyutils as default; +#[cfg(all( + target_os = "linux", + any( + feature = "sync-persistent-keyutils", + feature = "async-persistent-keyutils", + ) +))] +pub mod keyutils_persistent; +#[cfg(all( + target_os = "linux", + any( + feature = "sync-persistent-keyutils", + feature = "async-persistent-keyutils", + ), +))] +pub use keyutils_persistent as default; // fallback to mock if neither keyutils nor secret service is available #[cfg(all( any(target_os = "linux", target_os = "freebsd", target_os = "openbsd"), not(any( - feature = "linux-native", feature = "keyutils", - feature = "secret-service", + feature = "sync-secret-service", + feature = "async-secret-service", )), ))] pub use mock as default; @@ -563,6 +570,8 @@ mod tests { pub fn generate_random_string_of_len(len: usize) -> String { // from the Rust Cookbook: // https://rust-lang-nursery.github.io/rust-cookbook/algorithms/randomness.html + #[allow(unused_imports)] + use rand::Rng; use rand::{distributions::Alphanumeric, thread_rng}; thread_rng() .sample_iter(&Alphanumeric) diff --git a/src/secret_service.rs b/src/secret_service.rs index 9764f53..a27870e 100644 --- a/src/secret_service.rs +++ b/src/secret_service.rs @@ -89,7 +89,7 @@ issue for more details and possible workarounds. */ use std::collections::HashMap; -#[cfg(feature = "sync-secret-service")] +#[cfg(not(feature = "async-secret-service"))] use dbus_secret_service::{Collection, EncryptionType, Error, Item, SecretService}; #[cfg(feature = "async-secret-service")] use secret_service::{ @@ -424,7 +424,7 @@ impl SsCredential { /// of the credential much easier. But since the secret service expects /// a map from &str to &str, we have this utility to transform the /// credential's map into one of the right form. - pub(crate) fn all_attributes(&self) -> HashMap<&str, &str> { + fn all_attributes(&self) -> HashMap<&str, &str> { self.attributes .iter() .map(|(k, v)| (k.as_str(), v.as_str())) @@ -433,7 +433,7 @@ impl SsCredential { /// Similar to [all_attributes](SsCredential::all_attributes), /// but this just selects the ones we search on - pub(crate) fn search_attributes(&self, omit_target: bool) -> HashMap<&str, &str> { + fn search_attributes(&self, omit_target: bool) -> HashMap<&str, &str> { let mut result: HashMap<&str, &str> = HashMap::new(); if self.target.is_some() && !omit_target { result.insert("target", self.attributes["target"].as_str()); @@ -844,7 +844,7 @@ mod tests { } fn delete_collection(name: &str) { - #[cfg(feature = "sync-secret-service")] + #[cfg(not(feature = "async-secret-service"))] use dbus_secret_service::{EncryptionType, SecretService}; #[cfg(feature = "async-secret-service")] use secret_service::{blocking::SecretService, EncryptionType}; @@ -856,7 +856,7 @@ mod tests { } fn create_v1_entry(name: &str, password: &str) { - #[cfg(feature = "sync-secret-service")] + #[cfg(not(feature = "async-secret-service"))] use dbus_secret_service::{EncryptionType, SecretService}; #[cfg(feature = "async-secret-service")] use secret_service::{blocking::SecretService, EncryptionType}; diff --git a/src/secret_service_with_keyutils.rs b/src/secret_service_with_keyutils.rs deleted file mode 100644 index 6081cff..0000000 --- a/src/secret_service_with_keyutils.rs +++ /dev/null @@ -1,169 +0,0 @@ -/*! - -# secret-service-with-keyutils credential store - -TODO - - */ -use std::collections::HashMap; - -#[cfg(feature = "sync-secret-service")] -use dbus_secret_service::{Error, Item}; -#[cfg(feature = "async-secret-service")] -use secret_service::{blocking::Item, Error}; - -#[cfg(all(target_os = "linux", feature = "linux-native"))] -use crate::keyutils::KeyutilsCredential; -use crate::secret_service::{decode_error, SsCredential}; - -use super::credential::{Credential, CredentialApi, CredentialBuilder, CredentialBuilderApi}; -use super::error::Result; - -#[derive(Debug, Clone)] -pub struct SsKeyutilsCredential { - keyutils: KeyutilsCredential, - ss: SsCredential, -} - -impl CredentialApi for SsKeyutilsCredential { - fn set_password(&self, password: &str) -> Result<()> { - self.set_secret(password.as_bytes()) - } - - fn set_secret(&self, secret: &[u8]) -> Result<()> { - self.ss.set_secret(secret)?; - let _ = self.keyutils.set_secret(secret); - Ok(()) - } - - fn get_password(&self) -> Result { - if let Ok(password) = self.keyutils.get_password() { - return Ok(password); - } - - let password = self.ss.get_password()?; - let _ = self.keyutils.set_password(&password); - - Ok(password) - } - - fn get_secret(&self) -> Result> { - if let Ok(secret) = self.keyutils.get_secret() { - return Ok(secret); - } - - let secret = self.ss.get_secret()?; - let _ = self.keyutils.set_secret(&secret); - - Ok(secret) - } - - fn get_attributes(&self) -> Result> { - self.ss.get_attributes() - } - - fn update_attributes(&self, attributes: &HashMap<&str, &str>) -> Result<()> { - self.ss.update_attributes(attributes) - } - - fn delete_credential(&self) -> Result<()> { - self.ss.delete_credential()?; - let _ = self.keyutils.delete_credential(); - Ok(()) - } - - fn as_any(&self) -> &dyn std::any::Any { - self - } - - fn debug_fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - std::fmt::Debug::fmt(self, f) - } -} - -impl KeyutilsCredential { - /// Create a keyutils credential from an underlying secret service - /// item's attributes. - /// - /// The created credential will have all the attributes and label - /// of the underlying item, so you can examine them. - pub fn new_from_item(item: &Item) -> Result { - let attributes = item.get_attributes().map_err(decode_error)?; - - let target = attributes.get("target").map(|target| target.as_str()); - let service = attributes - .get("service") - .ok_or(decode_error(Error::NoResult))?; - let user = attributes - .get("username") - .ok_or(decode_error(Error::NoResult))?; - - let keyutils = KeyutilsCredential::new_with_target(target, service.as_str(), user.as_str()) - .map_err(|err| crate::Error::NoStorageAccess(Box::new(err)))?; - - Ok(keyutils) - } -} - -impl SsKeyutilsCredential { - pub fn new_with_target(target: Option<&str>, service: &str, user: &str) -> Result { - let ss = SsCredential::new_with_target(target, service, user)?; - let keyutils = KeyutilsCredential::new_with_target(target, service, user)?; - Ok(Self { keyutils, ss }) - } - - pub fn new_with_no_target(service: &str, user: &str) -> Result { - let keyutils = KeyutilsCredential::new_with_target(None, service, user)?; - let ss = SsCredential::new_with_no_target(service, user)?; - Ok(Self { keyutils, ss }) - } - - pub fn new_from_item(item: &Item) -> Result { - let ss = SsCredential::new_from_item(item)?; - let keyutils = KeyutilsCredential::new_from_item(item)?; - Ok(Self { keyutils, ss }) - } - - pub fn get_all_passwords(&self) -> Result> { - self.ss.get_all_passwords() - } - - pub fn delete_all_passwords(&self) -> Result<()> { - self.ss.delete_all_passwords() - } - - pub fn all_attributes(&self) -> HashMap<&str, &str> { - self.ss.all_attributes() - } - - pub fn search_attributes(&self, omit_target: bool) -> HashMap<&str, &str> { - self.ss.search_attributes(omit_target) - } -} - -/// The builder for secret-service-with-keyutils credentials -#[derive(Debug, Default)] -pub struct SsKeyutilsCredentialBuilder {} - -/// Returns an instance of the secret-service-with-keyutils credential builder. -/// -/// If secret-service-with-keyutils is the default credential store, -/// this is called once when an entry is first created. -pub fn default_credential_builder() -> Box { - Box::new(SsKeyutilsCredentialBuilder {}) -} - -impl CredentialBuilderApi for SsKeyutilsCredentialBuilder { - /// Build an [SsKeyutilsCredential] for the given target, service, and user. - fn build(&self, target: Option<&str>, service: &str, user: &str) -> Result> { - Ok(Box::new(SsCredential::new_with_target( - target, service, user, - )?)) - } - - /// Return the underlying builder object with an `Any` type so that it can - /// be downgraded to an [SsKeyutilsCredentialBuilder] for platform-specific processing. - fn as_any(&self) -> &dyn std::any::Any { - self - } -} From 3b229661d42e50599c858b7df3f3e985e21a0505 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20DOUIN?= Date: Tue, 15 Oct 2024 16:17:22 +0200 Subject: [PATCH 11/18] fix ci --- .github/workflows/ci.yaml | 6 ++++++ src/lib.rs | 5 ++++- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 7419f3d..a72727f 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -54,6 +54,12 @@ jobs: - "async-secret-service,async-io,crypto-rust" - "async-secret-service,tokio,crypto-openssl" - "async-secret-service,async-io,crypto-openssl" + - "sync-persistent-keyutils,crypto-rust" + - "sync-persistent-keyutils,crypto-openssl" + - "async-persistent-keyutils,tokio,crypto-rust" + - "async-persistent-keyutils,async-io,crypto-rust" + - "async-persistent-keyutils,tokio,crypto-openssl" + - "async-persistent-keyutils,async-io,crypto-openssl" steps: - name: Install CI dependencies diff --git a/src/lib.rs b/src/lib.rs index 589c6af..e5bd9a5 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -212,7 +212,10 @@ pub mod secret_service; #[cfg(all( any(target_os = "linux", target_os = "freebsd", target_os = "openbsd"), any(feature = "sync-secret-service", feature = "async-secret-service"), - not(feature = "keyutils"), + not(any( + feature = "sync-persistent-keyutils", + feature = "async-persistent-keyutils", + )), ))] pub use secret_service as default; From 827fd6ea652ffa42f1407ad47379ca1fba77275d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20DOUIN?= Date: Tue, 15 Oct 2024 21:33:10 +0200 Subject: [PATCH 12/18] adjust code after third review --- Cargo.toml | 7 +++---- src/keyutils_persistent.rs | 29 ++++++----------------------- src/lib.rs | 19 ++++++++++++------- 3 files changed, 21 insertions(+), 34 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index deefd2d..d4f11e9 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -13,13 +13,12 @@ exclude = [".github/"] readme = "README.md" [features] -linux-native = ["keyutils"] +linux-native = ["dep:linux-keyutils"] apple-native = ["dep:security-framework"] windows-native = ["dep:windows-sys", "dep:byteorder"] -keyutils = ["dep:linux-keyutils"] -sync-persistent-keyutils = ["keyutils", "sync-secret-service"] -async-persistent-keyutils = ["keyutils", "async-secret-service"] +sync-persistent-keyutils = ["dep:linux-keyutils", "sync-secret-service"] +async-persistent-keyutils = ["dep:linux-keyutils", "async-secret-service"] sync-secret-service = ["dep:dbus-secret-service"] async-secret-service = ["dep:secret-service", "dep:zbus"] crypto-rust = ["dbus-secret-service?/crypto-rust", "secret-service?/crypto-rust"] diff --git a/src/keyutils_persistent.rs b/src/keyutils_persistent.rs index 44569ca..54433e1 100644 --- a/src/keyutils_persistent.rs +++ b/src/keyutils_persistent.rs @@ -5,14 +5,12 @@ TODO */ -use std::collections::HashMap; - -use crate::credential::{ +use super::credential::{ Credential, CredentialApi, CredentialBuilder, CredentialBuilderApi, CredentialPersistence, }; -use crate::keyutils::KeyutilsCredential; -use crate::secret_service::SsCredential; -use crate::{Error, Result}; +use super::error::{Error, Result}; +use super::keyutils::KeyutilsCredential; +use super::secret_service::SsCredential; #[derive(Debug, Clone)] pub struct KeyutilsPersistentCredential { @@ -59,24 +57,9 @@ impl CredentialApi for KeyutilsPersistentCredential { Ok(secret) } - fn get_attributes(&self) -> Result> { - let Ok(attributes) = self.ss.get_attributes() else { - return self.keyutils.get_attributes(); - }; - - Ok(attributes) - } - - fn update_attributes(&self, attributes: &HashMap<&str, &str>) -> Result<()> { - let Ok(()) = self.ss.update_attributes(attributes) else { - return self.keyutils.update_attributes(attributes); - }; - - Ok(()) - } - fn delete_credential(&self) -> Result<()> { - self.keyutils.delete_credential()?; + // TODO: log the error + let _ = self.keyutils.delete_credential(); self.ss.delete_credential()?; Ok(()) } diff --git a/src/lib.rs b/src/lib.rs index e5bd9a5..aaef8fe 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -189,12 +189,19 @@ pub mod mock; feature = "async-persistent-keyutils", ) ))] -compile_error!("This crate cannot use the secret-service both synchronously and asynchronously"); +compile_error!("This crate cannot use both the sync and async versions of any credential store"); // // pick the *nix keystore // -#[cfg(all(target_os = "linux", feature = "keyutils"))] +#[cfg(all( + target_os = "linux", + any( + feature = "linux-native", + feature = "sync-persistent-keyutils", + feature = "async-persistent-keyutils", + ) +))] pub mod keyutils; #[cfg(all( target_os = "linux", @@ -239,11 +246,9 @@ pub use keyutils_persistent as default; // fallback to mock if neither keyutils nor secret service is available #[cfg(all( any(target_os = "linux", target_os = "freebsd", target_os = "openbsd"), - not(any( - feature = "keyutils", - feature = "sync-secret-service", - feature = "async-secret-service", - )), + not(feature = "linux-native"), + not(feature = "sync-secret-service"), + not(feature = "async-secret-service"), ))] pub use mock as default; From f6a4b78c5c44f71695a0aa6775d9a284badbdda2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20DOUIN?= Date: Tue, 15 Oct 2024 23:01:29 +0200 Subject: [PATCH 13/18] log error when cannot delete keyutils credential --- src/keyutils_persistent.rs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/keyutils_persistent.rs b/src/keyutils_persistent.rs index 54433e1..ca01d4b 100644 --- a/src/keyutils_persistent.rs +++ b/src/keyutils_persistent.rs @@ -5,6 +5,8 @@ TODO */ +use log::debug; + use super::credential::{ Credential, CredentialApi, CredentialBuilder, CredentialBuilderApi, CredentialPersistence, }; @@ -58,10 +60,10 @@ impl CredentialApi for KeyutilsPersistentCredential { } fn delete_credential(&self) -> Result<()> { - // TODO: log the error - let _ = self.keyutils.delete_credential(); - self.ss.delete_credential()?; - Ok(()) + if let Err(err) = self.keyutils.delete_credential() { + debug!("cannot delete keyutils credential: {err}"); + } + self.ss.delete_credential() } fn as_any(&self) -> &dyn std::any::Any { From b41fafcf799cc365cddb75a38ea7c02d4493037f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20DOUIN?= Date: Tue, 15 Oct 2024 23:05:41 +0200 Subject: [PATCH 14/18] rename cargo features --- .github/workflows/ci.yaml | 12 ++++++------ Cargo.toml | 4 ++-- src/lib.rs | 25 +++++++++---------------- 3 files changed, 17 insertions(+), 24 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index daa1c71..d010a83 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -56,12 +56,12 @@ jobs: - "async-secret-service,async-io,crypto-rust" - "async-secret-service,tokio,crypto-openssl" - "async-secret-service,async-io,crypto-openssl" - - "sync-persistent-keyutils,crypto-rust" - - "sync-persistent-keyutils,crypto-openssl" - - "async-persistent-keyutils,tokio,crypto-rust" - - "async-persistent-keyutils,async-io,crypto-rust" - - "async-persistent-keyutils,tokio,crypto-openssl" - - "async-persistent-keyutils,async-io,crypto-openssl" + - "linux-native-sync-persistent,crypto-rust" + - "linux-native-sync-persistent,crypto-openssl" + - "linux-native-async-persistent,tokio,crypto-rust" + - "linux-native-async-persistent,async-io,crypto-rust" + - "linux-native-async-persistent,tokio,crypto-openssl" + - "linux-native-async-persistent,async-io,crypto-openssl" steps: - name: Install CI dependencies diff --git a/Cargo.toml b/Cargo.toml index 2cc70bd..1740d9e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -17,8 +17,8 @@ linux-native = ["dep:linux-keyutils"] apple-native = ["dep:security-framework"] windows-native = ["dep:windows-sys", "dep:byteorder"] -sync-persistent-keyutils = ["dep:linux-keyutils", "sync-secret-service"] -async-persistent-keyutils = ["dep:linux-keyutils", "async-secret-service"] +linux-native-sync-persistent = ["linux-native", "sync-secret-service"] +linux-native-async-persistent = ["linux-native", "async-secret-service"] sync-secret-service = ["dep:dbus-secret-service"] async-secret-service = ["dep:secret-service", "dep:zbus"] crypto-rust = ["dbus-secret-service?/crypto-rust", "secret-service?/crypto-rust"] diff --git a/src/lib.rs b/src/lib.rs index 318dcbb..a69d7ed 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -187,8 +187,8 @@ pub mod mock; #[cfg(any( all(feature = "sync-secret-service", feature = "async-secret-service"), all( - feature = "sync-persistent-keyutils", - feature = "async-persistent-keyutils", + feature = "linux-native-sync-persistent", + feature = "linux-native-async-persistent", ) ))] compile_error!("This crate cannot use both the sync and async versions of any credential store"); @@ -196,14 +196,7 @@ compile_error!("This crate cannot use both the sync and async versions of any cr // // pick the *nix keystore // -#[cfg(all( - target_os = "linux", - any( - feature = "linux-native", - feature = "sync-persistent-keyutils", - feature = "async-persistent-keyutils", - ) -))] +#[cfg(all(target_os = "linux", feature = "linux-native"))] pub mod keyutils; #[cfg(all( target_os = "linux", @@ -222,8 +215,8 @@ pub mod secret_service; any(target_os = "linux", target_os = "freebsd", target_os = "openbsd"), any(feature = "sync-secret-service", feature = "async-secret-service"), not(any( - feature = "sync-persistent-keyutils", - feature = "async-persistent-keyutils", + feature = "linux-native-sync-persistent", + feature = "linux-native-async-persistent", )), ))] pub use secret_service as default; @@ -231,16 +224,16 @@ pub use secret_service as default; #[cfg(all( target_os = "linux", any( - feature = "sync-persistent-keyutils", - feature = "async-persistent-keyutils", + feature = "linux-native-sync-persistent", + feature = "linux-native-async-persistent", ) ))] pub mod keyutils_persistent; #[cfg(all( target_os = "linux", any( - feature = "sync-persistent-keyutils", - feature = "async-persistent-keyutils", + feature = "linux-native-sync-persistent", + feature = "linux-native-async-persistent", ), ))] pub use keyutils_persistent as default; From 160ad9ff2a76974ccb7f481bd77700554af4abae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20DOUIN?= Date: Tue, 15 Oct 2024 23:35:23 +0200 Subject: [PATCH 15/18] adjust code after forth review --- src/keyutils_persistent.rs | 19 +++++++++++++++---- src/lib.rs | 17 ++++++++++++----- 2 files changed, 27 insertions(+), 9 deletions(-) diff --git a/src/keyutils_persistent.rs b/src/keyutils_persistent.rs index ca01d4b..169f8b9 100644 --- a/src/keyutils_persistent.rs +++ b/src/keyutils_persistent.rs @@ -38,8 +38,13 @@ impl CredentialApi for KeyutilsPersistentCredential { } fn get_password(&self) -> Result { - if let Ok(password) = self.keyutils.get_password() { - return Ok(password); + match self.keyutils.get_password() { + Ok(password) => { + return Ok(password); + } + Err(err) => { + debug!("cannot get password from keyutils: {err}, trying from secret service") + } } let password = self.ss.get_password().map_err(ambigous_to_no_entry)?; @@ -49,8 +54,13 @@ impl CredentialApi for KeyutilsPersistentCredential { } fn get_secret(&self) -> Result> { - if let Ok(secret) = self.keyutils.get_secret() { - return Ok(secret); + match self.keyutils.get_secret() { + Ok(secret) => { + return Ok(secret); + } + Err(err) => { + debug!("cannot get secret from keyutils: {err}, trying from secret service") + } } let secret = self.ss.get_secret().map_err(ambigous_to_no_entry)?; @@ -63,6 +73,7 @@ impl CredentialApi for KeyutilsPersistentCredential { if let Err(err) = self.keyutils.delete_credential() { debug!("cannot delete keyutils credential: {err}"); } + self.ss.delete_credential() } diff --git a/src/lib.rs b/src/lib.rs index a69d7ed..439e11b 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -239,11 +239,18 @@ pub mod keyutils_persistent; pub use keyutils_persistent as default; // fallback to mock if neither keyutils nor secret service is available -#[cfg(all( - any(target_os = "linux", target_os = "freebsd", target_os = "openbsd"), - not(feature = "linux-native"), - not(feature = "sync-secret-service"), - not(feature = "async-secret-service"), +#[cfg(any( + all( + target_os = "linux", + not(feature = "linux-native"), + not(feature = "sync-secret-service"), + not(feature = "async-secret-service"), + ), + all( + any(target_os = "freebsd", target_os = "openbsd"), + not(feature = "sync-secret-service"), + not(feature = "async-secret-service"), + ) ))] pub use mock as default; From 6df3d93950eed289b3771b1244100a12e77c2b04 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20DOUIN?= Date: Sat, 19 Oct 2024 11:07:10 +0200 Subject: [PATCH 16/18] init doc + unit tests --- src/keyutils_persistent.rs | 132 ++++++++++++++++++++++++++++++++++--- 1 file changed, 123 insertions(+), 9 deletions(-) diff --git a/src/keyutils_persistent.rs b/src/keyutils_persistent.rs index 169f8b9..c2fcd66 100644 --- a/src/keyutils_persistent.rs +++ b/src/keyutils_persistent.rs @@ -2,9 +2,12 @@ # keyutils-persistent credential store -TODO +This store is a combination of the [keyutils](crate::keyutils) store +backed up with a persistent [secret-service](crate::secret_service) +store. */ + use log::debug; use super::credential::{ @@ -14,6 +17,10 @@ use super::error::{Error, Result}; use super::keyutils::KeyutilsCredential; use super::secret_service::SsCredential; +/// Representation of a keyutils-persistent credential. +/// +/// The credential owns a [KeyutilsCredential] for in-memory usage and +/// a [SsCredential] for persistence. #[derive(Debug, Clone)] pub struct KeyutilsPersistentCredential { keyutils: KeyutilsCredential, @@ -21,22 +28,38 @@ pub struct KeyutilsPersistentCredential { } impl CredentialApi for KeyutilsPersistentCredential { + /// Set a password in the underlying store fn set_password(&self, password: &str) -> Result<()> { self.set_secret(password.as_bytes()) } + /// Set a secret in the underlying store + /// + /// It sets first the secret in keyutils, then in + /// secret-service. If the late one fails, keyutils secret change + /// is reverted. fn set_secret(&self, secret: &[u8]) -> Result<()> { - let prev_secret = self.keyutils.get_secret()?; + let prev_secret = self.keyutils.get_secret(); self.keyutils.set_secret(secret)?; if let Err(err) = self.ss.set_secret(secret) { - self.keyutils.set_secret(&prev_secret)?; + match prev_secret { + Ok(ref secret) => self.keyutils.set_secret(secret), + Err(Error::NoEntry) => self.keyutils.delete_credential(), + Err(err) => Err(err), + }?; + return Err(err); } Ok(()) } + /// Retrieve a password from the underlying store + /// + /// The password is retrieved from keyutils. In case of error, the + /// password is retrieved from secret-service instead (and + /// keyutils is updated). fn get_password(&self) -> Result { match self.keyutils.get_password() { Ok(password) => { @@ -53,6 +76,11 @@ impl CredentialApi for KeyutilsPersistentCredential { Ok(password) } + /// Retrieve a secret from the underlying store + /// + /// The secret is retrieved from keyutils. In case of error, the + /// secret is retrieved from secret-service instead (and keyutils + /// is updated). fn get_secret(&self) -> Result> { match self.keyutils.get_secret() { Ok(secret) => { @@ -69,6 +97,10 @@ impl CredentialApi for KeyutilsPersistentCredential { Ok(secret) } + /// Delete a password from the underlying store. + /// + /// The credential is deleted from both keyutils and + /// secret-service. fn delete_credential(&self) -> Result<()> { if let Err(err) = self.keyutils.delete_credential() { debug!("cannot delete keyutils credential: {err}"); @@ -87,6 +119,11 @@ impl CredentialApi for KeyutilsPersistentCredential { } impl KeyutilsPersistentCredential { + /// Create the platform credential for a Keyutils entry. + /// + /// An explicit target string is interpreted as the KeyRing to use for the entry. + /// If none is provided, then we concatenate the user and service in the string + /// `keyring-rs:user@service`. pub fn new_with_target(target: Option<&str>, service: &str, user: &str) -> Result { let ss = SsCredential::new_with_target(target, service, user)?; let keyutils = KeyutilsCredential::new_with_target(target, service, user)?; @@ -94,14 +131,14 @@ impl KeyutilsPersistentCredential { } } -/// The builder for secret-service-with-keyutils credentials +/// The builder for keyutils-persistent credentials #[derive(Debug, Default)] pub struct KeyutilsPersistentCredentialBuilder {} -/// Returns an instance of the secret-service-with-keyutils credential builder. +/// Returns an instance of the keyutils-persistent credential builder. /// -/// If secret-service-with-keyutils is the default credential store, -/// this is called once when an entry is first created. +/// If keyutils-persistent is the default credential store, this is +/// called once when an entry is first created. pub fn default_credential_builder() -> Box { Box::new(KeyutilsPersistentCredentialBuilder {}) } @@ -120,13 +157,13 @@ impl CredentialBuilderApi for KeyutilsPersistentCredentialBuilder { self } - /// Since this keystore keeps credentials in kernel memory, - /// they vanish on reboot + /// This keystore keeps credentials thanks to the inner secret-service store. fn persistence(&self) -> CredentialPersistence { CredentialPersistence::UntilDelete } } +/// Replace any Ambiguous error with a NoEntry one fn ambigous_to_no_entry(err: Error) -> Error { if let Error::Ambiguous(_) = err { return Error::NoEntry; @@ -134,3 +171,80 @@ fn ambigous_to_no_entry(err: Error) -> Error { err } + +#[cfg(test)] +mod tests { + use crate::credential::CredentialPersistence; + use crate::{Entry, Error}; + + use super::{default_credential_builder, KeyutilsPersistentCredential}; + + #[test] + fn test_persistence() { + assert!(matches!( + default_credential_builder().persistence(), + CredentialPersistence::UntilDelete + )) + } + + fn entry_new(service: &str, user: &str) -> Entry { + crate::tests::entry_from_constructor( + KeyutilsPersistentCredential::new_with_target, + service, + user, + ) + } + + #[test] + fn test_invalid_parameter() { + let credential = KeyutilsPersistentCredential::new_with_target(Some(""), "service", "user"); + assert!( + matches!(credential, Err(Error::Invalid(_, _))), + "Created entry with empty target" + ); + } + + #[test] + fn test_empty_service_and_user() { + crate::tests::test_empty_service_and_user(entry_new); + } + + #[test] + fn test_missing_entry() { + crate::tests::test_missing_entry(entry_new); + } + + #[test] + fn test_empty_password() { + let entry = entry_new("empty password service", "empty password user"); + assert!( + matches!(entry.set_password(""), Err(Error::Invalid(_, _))), + "Able to set empty password" + ); + } + + #[test] + fn test_round_trip_ascii_password() { + crate::tests::test_round_trip_ascii_password(entry_new); + } + + #[test] + fn test_round_trip_non_ascii_password() { + crate::tests::test_round_trip_non_ascii_password(entry_new); + } + + #[test] + fn test_round_trip_random_secret() { + crate::tests::test_round_trip_random_secret(entry_new); + } + + #[test] + fn test_update() { + crate::tests::test_update(entry_new); + } + + #[test] + fn test_noop_get_update_attributes() { + crate::tests::test_noop_get_update_attributes(entry_new); + } +} From 658174ef994ca4971fa0686c69085048a895dd65 Mon Sep 17 00:00:00 2001 From: Daniel Brotsky Date: Thu, 24 Oct 2024 16:45:43 -0700 Subject: [PATCH 17/18] Fix new clippy warning. --- src/secret_service.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/secret_service.rs b/src/secret_service.rs index a27870e..2d600c4 100644 --- a/src/secret_service.rs +++ b/src/secret_service.rs @@ -402,7 +402,7 @@ impl SsCredential { let attributes = self.search_attributes(true); let search = collection.search_items(attributes).map_err(decode_error)?; if require_unique { - if search.len() == 0 && require_unique { + if search.is_empty() && require_unique { return Err(ErrorCode::NoEntry); } else if search.len() > 1 { let mut creds: Vec> = vec![]; @@ -520,7 +520,7 @@ pub fn get_item_password(item: &Item) -> Result { decode_password(bytes) } -//// Given an existing item, retrieve its secret. +/// Given an existing item, retrieve its secret. pub fn get_item_secret(item: &Item) -> Result> { let secret = item.get_secret().map_err(decode_error)?; Ok(secret) From f59afd50e4b9be4e333e6c050ac8f97f51fe632d Mon Sep 17 00:00:00 2001 From: Daniel Brotsky Date: Thu, 24 Oct 2024 16:46:46 -0700 Subject: [PATCH 18/18] Updated docs for new keystore. Also updated feature descriptions in the library docs, and the readme. Updated the cross-platform doc builder script to know about the new keystore. --- README.md | 4 +- build-xplat-docs.sh | 14 +++++-- src/keyutils_persistent.rs | 75 +++++++++++++++++++++++--------------- src/lib.rs | 29 +++++++++------ 4 files changed, 75 insertions(+), 47 deletions(-) diff --git a/README.md b/README.md index ff0ec5b..d5ea62a 100644 --- a/README.md +++ b/README.md @@ -56,14 +56,14 @@ This crate allows clients to "bring their own credential store" by providing tra This crate provides built-in implementations of the following platform-specific credential stores: -* _Linux_: The DBus-based Secret Service and the kernel keyutils. +* _Linux_: The DBus-based Secret Service, the kernel keyutils, and a combo of the two. * _FreeBSD_, _OpenBSD_: The DBus-based Secret Service. * _macOS_, _iOS_: The local keychain. * _Windows_: The Windows Credential Manager. To enable the stores you want, you use features: there is one feature for each possibly-included credential store. If you specify a feature (e.g., `dbus-secret-service`) _and_ your target platform (e.g., `freebsd`) supports that credential store, it will be included as the default credential store in that build. That way you can have a build command that specifies a single credential store for each of your target platforms, and use that same build command for all targets. -If you don't enable any credential stores that are supported on a given platform, or you enable multiple credential stores for some platform, the _mock_ keystore will be the default on that platform. See the [developer docs](https://docs.rs/keyring/) for details of which features control the inclusion of which credential stores (and which platforms each credential store targets). +If you don't enable any credential stores that are supported on a given platform, the _mock_ keystore will be the default on that platform. See the [developer docs](https://docs.rs/keyring/) for details of which features control the inclusion of which credential stores. ### Platform-specific issues diff --git a/build-xplat-docs.sh b/build-xplat-docs.sh index 77d6c90..56b855b 100644 --- a/build-xplat-docs.sh +++ b/build-xplat-docs.sh @@ -1,5 +1,11 @@ #!/bin/bash -cargo doc --no-deps --features=linux-native --target aarch64-unknown-linux-musl $OPEN_DOCS -cargo doc --no-deps --features=windows-native --target aarch64-pc-windows-msvc $OPEN_DOCS -cargo doc --no-deps --features=apple-native --target aarch64-apple-darwin $OPEN_DOCS -cargo doc --no-deps --features=apple-native --target aarch64-apple-ios $OPEN_DOCS +if [[ "$OSTYPE" == "linux"* ]]; then + cargo doc --no-deps --features=linux-native-sync-persistent $OPEN_DOCS + cargo doc --no-deps --features=sync-secret-service $OPEN_DOCS + cargo doc --no-deps --features=linux-native $OPEN_DOCS +elif [[ "$OSTYPE" == "darwin"* ]]; then + cargo doc --no-deps --features=linux-native --target aarch64-unknown-linux-musl $OPEN_DOCS + cargo doc --no-deps --features=windows-native --target aarch64-pc-windows-msvc $OPEN_DOCS + cargo doc --no-deps --features=apple-native --target aarch64-apple-darwin $OPEN_DOCS + cargo doc --no-deps --features=apple-native --target aarch64-apple-ios $OPEN_DOCS +fi \ No newline at end of file diff --git a/src/keyutils_persistent.rs b/src/keyutils_persistent.rs index c2fcd66..d9ff4d9 100644 --- a/src/keyutils_persistent.rs +++ b/src/keyutils_persistent.rs @@ -1,11 +1,33 @@ /*! -# keyutils-persistent credential store - -This store is a combination of the [keyutils](crate::keyutils) store -backed up with a persistent [secret-service](crate::secret_service) -store. - +# Linux (keyutils) store with Secret Service backing + +This store, contributed by [@soywod](https://github.com/soywod), +uses the [keyutils module](crate::keyutils) as a cache +available to headless processes, while using the +[secret-service module](crate::secret_service) +to provide credential storage beyond reboot. +The expected usage pattern +for this module is as follows: + +- Processes that run on headless systems are built with `keyutils` support via the + `linux-native` feature of this crate. After each reboot, these processes + are either launched after the keyutils cache has been reloaded from the secret service, + or (if launched immediately) they wait until the keyutils cache has been reloaded. +- A headed "configuration" process is built with this module that allows its user + to configure the credentials needed by the headless processes. After each reboot, + this process unlocks the secret service (see both the keyutils and secret-service + module for information about how this can be done headlessly, if desired) and then + accesses each of the configured credentials (which loads them into keyutils). At + that point the headless clients can be started (or become active, if already started). + +This store works by creating a keyutils entry and a secret-service entry for +each of its entries. Because keyutils entries don't have attributes, entries +in this store don't expose attributes either. Because keyutils entries can't +store empty passwords/secrets, this store's entries can't either. + +See the documentation for the `keyutils` and `secret-service` modules if you +want details about how the underlying storage is handled. */ use log::debug; @@ -15,7 +37,7 @@ use super::credential::{ }; use super::error::{Error, Result}; use super::keyutils::KeyutilsCredential; -use super::secret_service::SsCredential; +use super::secret_service::{SsCredential, SsCredentialBuilder}; /// Representation of a keyutils-persistent credential. /// @@ -36,13 +58,14 @@ impl CredentialApi for KeyutilsPersistentCredential { /// Set a secret in the underlying store /// /// It sets first the secret in keyutils, then in - /// secret-service. If the late one fails, keyutils secret change + /// secret-service. If the latter set fails, the former /// is reverted. fn set_secret(&self, secret: &[u8]) -> Result<()> { let prev_secret = self.keyutils.get_secret(); self.keyutils.set_secret(secret)?; if let Err(err) = self.ss.set_secret(secret) { + debug!("Failed set of secret-service: {err}; reverting keyutils"); match prev_secret { Ok(ref secret) => self.keyutils.set_secret(secret), Err(Error::NoEntry) => self.keyutils.delete_credential(), @@ -66,11 +89,11 @@ impl CredentialApi for KeyutilsPersistentCredential { return Ok(password); } Err(err) => { - debug!("cannot get password from keyutils: {err}, trying from secret service") + debug!("Failed get from keyutils: {err}; trying secret service") } } - let password = self.ss.get_password().map_err(ambigous_to_no_entry)?; + let password = self.ss.get_password().map_err(ambiguous_to_no_entry)?; self.keyutils.set_password(&password)?; Ok(password) @@ -87,11 +110,11 @@ impl CredentialApi for KeyutilsPersistentCredential { return Ok(secret); } Err(err) => { - debug!("cannot get secret from keyutils: {err}, trying from secret service") + debug!("Failed get from keyutils: {err}; trying secret service") } } - let secret = self.ss.get_secret().map_err(ambigous_to_no_entry)?; + let secret = self.ss.get_secret().map_err(ambiguous_to_no_entry)?; self.keyutils.set_secret(&secret)?; Ok(secret) @@ -121,9 +144,8 @@ impl CredentialApi for KeyutilsPersistentCredential { impl KeyutilsPersistentCredential { /// Create the platform credential for a Keyutils entry. /// - /// An explicit target string is interpreted as the KeyRing to use for the entry. - /// If none is provided, then we concatenate the user and service in the string - /// `keyring-rs:user@service`. + /// This just passes the arguments to the underlying two stores + /// and wraps their results with an entry that holds both. pub fn new_with_target(target: Option<&str>, service: &str, user: &str) -> Result { let ss = SsCredential::new_with_target(target, service, user)?; let keyutils = KeyutilsCredential::new_with_target(target, service, user)?; @@ -144,7 +166,7 @@ pub fn default_credential_builder() -> Box { } impl CredentialBuilderApi for KeyutilsPersistentCredentialBuilder { - /// Build an [KeyutilsPersistentCredential] for the given target, service, and user. + /// Build a [KeyutilsPersistentCredential] for the given target, service, and user. fn build(&self, target: Option<&str>, service: &str, user: &str) -> Result> { Ok(Box::new(SsCredential::new_with_target( target, service, user, @@ -152,19 +174,21 @@ impl CredentialBuilderApi for KeyutilsPersistentCredentialBuilder { } /// Return the underlying builder object with an `Any` type so that it can - /// be downgraded to an [KeyutilsPersistentCredentialBuilder] for platform-specific processing. + /// be downgraded to a [KeyutilsPersistentCredentialBuilder] for platform-specific processing. fn as_any(&self) -> &dyn std::any::Any { self } - /// This keystore keeps credentials thanks to the inner secret-service store. + /// Return the persistence of this store. + /// + /// This store's persistence derives from that of the secret service. fn persistence(&self) -> CredentialPersistence { - CredentialPersistence::UntilDelete + SsCredentialBuilder {}.persistence() } } /// Replace any Ambiguous error with a NoEntry one -fn ambigous_to_no_entry(err: Error) -> Error { +fn ambiguous_to_no_entry(err: Error) -> Error { if let Error::Ambiguous(_) = err { return Error::NoEntry; }; @@ -174,18 +198,9 @@ fn ambigous_to_no_entry(err: Error) -> Error { #[cfg(test)] mod tests { - use crate::credential::CredentialPersistence; use crate::{Entry, Error}; - use super::{default_credential_builder, KeyutilsPersistentCredential}; - - #[test] - fn test_persistence() { - assert!(matches!( - default_credential_builder().persistence(), - CredentialPersistence::UntilDelete - )) - } + use super::KeyutilsPersistentCredential; fn entry_new(service: &str, user: &str) -> Entry { crate::tests::entry_from_constructor( diff --git a/src/lib.rs b/src/lib.rs index 439e11b..5ad2db0 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -64,7 +64,6 @@ example, the macOS Keychain credential store is only included if the `"apple-nat feature is specified (and the crate is built with a macOS target). If no specified credential store features apply to a given platform, -or multiple credential store features apply to a given platform, this crate will use the (platform-independent) _mock_ credential store (see below) on that platform. There are no default features in this crate: you must specify explicitly which platform-specific @@ -78,6 +77,18 @@ Here are the available credential store features: - `linux-native`: Provides access to the `keyutils` storage on Linux. +- `linux-native-sync-persistent`: Uses both `keyutils` and `sync-secret-service` + (see below) for storage. See the docs for the `keyutils_persistent` + module for a full explanation of why both are used. Because this + store uses the `sync-secret-service`, you can use additional features related + to that store (described below). + +- `linux-native-async-persistent`: Uses both `keyutils` and `async-secret-service` + (see below) for storage. See the docs for the `keyutils_persistent` + module for a full explanation of why both are used. + Because this store uses the `async-secret-service`, you + must specify the additional features required by that store (described below). + - `sync-secret-service`: Provides access to the DBus-based [Secret Service](https://specifications.freedesktop.org/secret-service/latest/) storage on Linux, FreeBSD, and OpenBSD. This is a _synchronous_ keystore that provides @@ -99,17 +110,13 @@ Here are the available credential store features: installed on the user's machine, specify the `vendored` feature to statically link them with the built crate. -You cannot specify both the `sync-secret-service` and `async-secret-service` features; -this will produce a compile error. You must pick one or the other if you want to use -the secret service for credential storage. - The Linux platform is the only one for which this crate supplies multiple keystores: -secret-service and keyutils. The secret-service is the more widely used store, because -it provides persistence of credentials beyond reboot (which keyutils does not). However, -because secret-service relies on system UI for unlocking credentials, it often isn't -available on headless Linux installations, so keyutils is provided for those situations. -If you enable both the secret-service store and the keyutils store, the secret-service -store will be used as the default. +native (keyutils), sync or async secret service, and sync or async "combo" (both +keyutils and secret service). You cannot specify use of both sync and async +keystores; this will lead to a compile error. If you enable a combo keystore on Linux, +that will be the default keystore. If you don't enable a +combo keystore on Linux, but you do enable both the native and secret service keystores, +the secret service will be the default. ## Client-provided Credential Stores