From 9f51f85b106fd724e0b621801c14f3ebc356fcaa Mon Sep 17 00:00:00 2001 From: Bernd Schoolmann Date: Fri, 29 Aug 2025 14:52:17 +0200 Subject: [PATCH] Clean up linux implementation --- .../core/src/biometric/linux.rs | 99 +++++++++++-------- .../core/src/secure_memory/memfd_secret.rs | 6 ++ 2 files changed, 66 insertions(+), 39 deletions(-) diff --git a/apps/desktop/desktop_native/core/src/biometric/linux.rs b/apps/desktop/desktop_native/core/src/biometric/linux.rs index 2621186b2e9..0e159e268da 100644 --- a/apps/desktop/desktop_native/core/src/biometric/linux.rs +++ b/apps/desktop/desktop_native/core/src/biometric/linux.rs @@ -42,62 +42,32 @@ impl Default for BiometricLockSystem { impl super::BiometricTrait for BiometricLockSystem { async fn authenticate(&self, _hwnd: Vec, _message: String) -> Result { - let connection = Connection::system().await?; - let proxy = AuthorityProxy::new(&connection).await?; - let subject = Subject::new_for_owner(std::process::id(), None, None)?; - let details = std::collections::HashMap::new(); - let result = proxy - .check_authorization( - &subject, - "com.bitwarden.Bitwarden.unlock", - &details, - CheckAuthorizationFlags::AllowUserInteraction.into(), - "", - ) - .await; - - match result { - Ok(result) => Ok(result.is_authorized), - Err(e) => { - println!("polkit biometric error: {:?}", e); - Ok(false) - } - } + polkit_authenticate_bitwarden_policy().await } async fn authenticate_available(&self) -> Result { - let connection = Connection::system().await?; - let proxy = AuthorityProxy::new(&connection).await?; - let actions = proxy.enumerate_actions("en").await?; - for action in actions { - if action.action_id == "com.bitwarden.Bitwarden.unlock" { - return Ok(true); - } - } - Ok(false) + polkit_is_bitwarden_policy_available().await } async fn enroll_persistent(&self, _user_id: &str, _key: &[u8]) -> Result<()> { + // Not implemented Ok(()) } async fn provide_key(&self, user_id: &str, key: &[u8]) { - let mut secure_memory = self.secure_memory.lock().await; - secure_memory.put(user_id.to_string(), key); + self.secure_memory.lock().await.put(user_id.to_string(), key); } async fn unlock(&self, user_id: &str, _hwnd: Vec) -> Result> { - if !(self.authenticate(Vec::new(), "".to_string()).await?) { + if !polkit_authenticate_bitwarden_policy().await? { return Err(anyhow!("Authentication failed")); } - let secure_memory = self.secure_memory.lock().await; - secure_memory.get(user_id).ok_or(anyhow!("No key found")) + self.secure_memory.lock().await.get(user_id).ok_or(anyhow!("No key found")) } async fn unlock_available(&self, user_id: &str) -> Result { - let secure_memory = self.secure_memory.lock().await; - Ok(secure_memory.has(user_id)) + Ok(self.secure_memory.lock().await.has(user_id)) } async fn has_persistent(&self, _user_id: &str) -> Result { @@ -105,8 +75,59 @@ impl super::BiometricTrait for BiometricLockSystem { } async fn unenroll(&self, user_id: &str) -> Result<(), anyhow::Error> { - let mut secure_memory = self.secure_memory.lock().await; - secure_memory.remove(user_id); + self.secure_memory.lock().await.remove(user_id); Ok(()) } } + +/// Perform a polkit authorization against the bitwarden unlock policy. Note: This relies on no custom +/// rules in the system skipping the authorization check, in which case this counts as UV / authentication. +async fn polkit_authenticate_bitwarden_policy() -> Result { + println!("[Polkit] Authenticating / performing UV"); + + let connection = Connection::system().await?; + let proxy = AuthorityProxy::new(&connection).await?; + let subject = Subject::new_for_owner(std::process::id(), None, None)?; + let details = std::collections::HashMap::new(); + let authorization_result = proxy + .check_authorization( + &subject, + "com.bitwarden.Bitwarden.unlock", + &details, + CheckAuthorizationFlags::AllowUserInteraction.into(), + "", + ) + .await; + + match authorization_result { + Ok(result) => Ok(result.is_authorized), + Err(e) => { + eprintln!("[Polkit] Error performing authentication: {:?}", e); + Ok(false) + } + } +} + +async fn polkit_is_bitwarden_policy_available() -> Result { + let connection = Connection::system().await?; + let proxy = AuthorityProxy::new(&connection).await?; + let actions = proxy.enumerate_actions("en").await?; + for action in actions { + if action.action_id == "com.bitwarden.Bitwarden.unlock" { + return Ok(true); + } + } + Ok(false) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[tokio::test] + #[ignore] + async fn test_polkit_authenticate() { + let result = polkit_authenticate_bitwarden_policy().await; + assert!(result.is_ok()); + } +} \ No newline at end of file diff --git a/apps/desktop/desktop_native/core/src/secure_memory/memfd_secret.rs b/apps/desktop/desktop_native/core/src/secure_memory/memfd_secret.rs index 8ce30a2b0ee..c7057fcd332 100644 --- a/apps/desktop/desktop_native/core/src/secure_memory/memfd_secret.rs +++ b/apps/desktop/desktop_native/core/src/secure_memory/memfd_secret.rs @@ -9,6 +9,10 @@ use crate::secure_memory::SecureMemoryStore; /// data is inaccessible to other user-mode processes, and even to root in most cases. /// If arbitrary data can be executed in the kernel, the data can still be retrieved: /// https://github.com/JonathonReinhart/nosecmem +/// +/// Warning: There is a maximum amount of concurrent memfd_secret protected items. Only +/// use this sparingly, or extend the implementation to use one secret + in-memory encryption, +/// or to reserve a large protected area in which we allocate our items. pub(crate) struct MemfdSecretKVStore { map: HashMap>, } @@ -103,6 +107,8 @@ mod tests { store.put(key.clone(), &value); assert!(store.has(&key), "Store should have key for size {}", size); assert_eq!(store.get(&key), Some(value), "Value mismatch for size {}", size); + // The test will not pass when we don't remove the keys, because there is a limit of concurrent memfd_secret memory spaces. + store.remove(&key); } }