diff --git a/apps/desktop/desktop_native/Cargo.lock b/apps/desktop/desktop_native/Cargo.lock index 04723698d73..5787e3af4ce 100644 --- a/apps/desktop/desktop_native/Cargo.lock +++ b/apps/desktop/desktop_native/Cargo.lock @@ -904,6 +904,7 @@ dependencies = [ "keytar", "libc", "log", + "memsec", "oo7", "pin-project", "pkcs8", @@ -911,7 +912,6 @@ dependencies = [ "rsa", "russh-cryptovec", "scopeguard", - "secmem-proc", "security-framework", "security-framework-sys", "serde", @@ -1790,6 +1790,17 @@ dependencies = [ "autocfg", ] +[[package]] +name = "memsec" +version = "0.7.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c797b9d6bb23aab2fc369c65f871be49214f5c759af65bde26ffaaa2b646b492" +dependencies = [ + "getrandom 0.2.16", + "libc", + "windows-sys 0.45.0", +] + [[package]] name = "mime" version = "0.3.17" @@ -2711,16 +2722,6 @@ dependencies = [ "windows-sys 0.59.0", ] -[[package]] -name = "rustix-linux-procfs" -version = "0.1.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2fc84bf7e9aa16c4f2c758f27412dc9841341e16aa682d9c7ac308fe3ee12056" -dependencies = [ - "once_cell", - "rustix 1.0.7", -] - [[package]] name = "rustversion" version = "1.0.20" @@ -2799,21 +2800,6 @@ dependencies = [ "zeroize", ] -[[package]] -name = "secmem-proc" -version = "0.3.7" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "473559b1d28f530c3a9b5f91a2866053e2b1c528a0e43dae83048139c99490c2" -dependencies = [ - "anyhow", - "cfg-if", - "libc", - "rustix 1.0.7", - "rustix-linux-procfs", - "thiserror 2.0.12", - "windows 0.61.1", -] - [[package]] name = "security-framework" version = "3.1.0" @@ -3841,6 +3827,15 @@ dependencies = [ "windows-link", ] +[[package]] +name = "windows-sys" +version = "0.45.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "75283be5efb2831d37ea142365f009c02ec203cd29a3ebecbc093d52315b66d0" +dependencies = [ + "windows-targets 0.42.2", +] + [[package]] name = "windows-sys" version = "0.52.0" @@ -3859,6 +3854,21 @@ dependencies = [ "windows-targets 0.52.6", ] +[[package]] +name = "windows-targets" +version = "0.42.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8e5180c00cd44c9b1c88adb3693291f1cd93605ded80c250a75d472756b4d071" +dependencies = [ + "windows_aarch64_gnullvm 0.42.2", + "windows_aarch64_msvc 0.42.2", + "windows_i686_gnu 0.42.2", + "windows_i686_msvc 0.42.2", + "windows_x86_64_gnu 0.42.2", + "windows_x86_64_gnullvm 0.42.2", + "windows_x86_64_msvc 0.42.2", +] + [[package]] name = "windows-targets" version = "0.48.5" @@ -3890,6 +3900,12 @@ dependencies = [ "windows_x86_64_msvc 0.52.6", ] +[[package]] +name = "windows_aarch64_gnullvm" +version = "0.42.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "597a5118570b68bc08d8d59125332c54f1ba9d9adeedeef5b99b02ba2b0698f8" + [[package]] name = "windows_aarch64_gnullvm" version = "0.48.5" @@ -3902,6 +3918,12 @@ version = "0.52.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "32a4622180e7a0ec044bb555404c800bc9fd9ec262ec147edd5989ccd0c02cd3" +[[package]] +name = "windows_aarch64_msvc" +version = "0.42.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e08e8864a60f06ef0d0ff4ba04124db8b0fb3be5776a5cd47641e942e58c4d43" + [[package]] name = "windows_aarch64_msvc" version = "0.48.5" @@ -3914,6 +3936,12 @@ version = "0.52.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "09ec2a7bb152e2252b53fa7803150007879548bc709c039df7627cabbd05d469" +[[package]] +name = "windows_i686_gnu" +version = "0.42.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c61d927d8da41da96a81f029489353e68739737d3beca43145c8afec9a31a84f" + [[package]] name = "windows_i686_gnu" version = "0.48.5" @@ -3932,6 +3960,12 @@ version = "0.52.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "0eee52d38c090b3caa76c563b86c3a4bd71ef1a819287c19d586d7334ae8ed66" +[[package]] +name = "windows_i686_msvc" +version = "0.42.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "44d840b6ec649f480a41c8d80f9c65108b92d89345dd94027bfe06ac444d1060" + [[package]] name = "windows_i686_msvc" version = "0.48.5" @@ -3953,6 +3987,12 @@ dependencies = [ "windows-core 0.61.0", ] +[[package]] +name = "windows_x86_64_gnu" +version = "0.42.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8de912b8b8feb55c064867cf047dda097f92d51efad5b491dfb98f6bbb70cb36" + [[package]] name = "windows_x86_64_gnu" version = "0.48.5" @@ -3965,6 +4005,12 @@ version = "0.52.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "147a5c80aabfbf0c7d901cb5895d1de30ef2907eb21fbbab29ca94c5b08b1a78" +[[package]] +name = "windows_x86_64_gnullvm" +version = "0.42.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "26d41b46a36d453748aedef1486d5c7a85db22e56aff34643984ea85514e94a3" + [[package]] name = "windows_x86_64_gnullvm" version = "0.48.5" @@ -3977,6 +4023,12 @@ version = "0.52.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "24d5b23dc417412679681396f2b49f3de8c1473deb516bd34410872eff51ed0d" +[[package]] +name = "windows_x86_64_msvc" +version = "0.42.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9aec5da331524158c6d1a4ac0ab1541149c0b9505fde06423b02f5ef0106b9f0" + [[package]] name = "windows_x86_64_msvc" version = "0.48.5" diff --git a/apps/desktop/desktop_native/Cargo.toml b/apps/desktop/desktop_native/Cargo.toml index 1e2c29da70d..122d4dbc3be 100644 --- a/apps/desktop/desktop_native/Cargo.toml +++ b/apps/desktop/desktop_native/Cargo.toml @@ -40,7 +40,6 @@ rand = "=0.9.1" rsa = "=0.9.6" russh-cryptovec = "=0.7.3" scopeguard = "=1.2.0" -secmem-proc = "=0.3.7" security-framework = "=3.1.0" security-framework-sys = "=2.13.0" serde = "=1.0.209" diff --git a/apps/desktop/desktop_native/core/Cargo.toml b/apps/desktop/desktop_native/core/Cargo.toml index 6a58c30b015..b12df226ff7 100644 --- a/apps/desktop/desktop_native/core/Cargo.toml +++ b/apps/desktop/desktop_native/core/Cargo.toml @@ -35,7 +35,6 @@ log = { workspace = true } rand = { workspace = true } russh-cryptovec = { workspace = true } scopeguard = { workspace = true } -secmem-proc = { workspace = true } sha2 = { workspace = true } ssh-encoding = { workspace = true } ssh-key = { workspace = true, features = [ @@ -60,7 +59,7 @@ chacha20 = "0.9.1" chacha20poly1305 = "0.10.1" serde = { workspace = true, features = ["derive"] } serde_json.workspace = true -windows = { workspace = true, features = ["UI_Accessibility"] } +memsec = { version = "0.7.0", features = ["alloc_ext"] } [target.'cfg(windows)'.dependencies] widestring = { workspace = true, optional = true } diff --git a/apps/desktop/desktop_native/core/src/biometric_v2/linux.rs b/apps/desktop/desktop_native/core/src/biometric_v2/linux.rs new file mode 100644 index 00000000000..23189172d36 --- /dev/null +++ b/apps/desktop/desktop_native/core/src/biometric_v2/linux.rs @@ -0,0 +1,100 @@ +//! This file implements Polkit based system unlock. +//! +//! # Security +//! This section describes the assumed security model and security guarantees achieved. In the required security +//! guarantee is that a locked vault - a running app - cannot be unlocked when the device (user-space) +//! is compromised in this state. +//! +//! When first unlocking the app, the app sends the user-key to this module, which holds it in secure memory, +//! protected by memfd_secret. This makes it inaccessible to other processes, even if they compromise root, a kernel compromise +//! has circumventable best-effort protections. While the app is running this key is held in memory, even if locked. +//! When unlocking, the app will prompt the user via `polkit` to get a yes/no decision on whether to release the key to the app. + +use anyhow::{anyhow, Result}; +use tokio::sync::Mutex; +use zbus::Connection; +use zbus_polkit::policykit1::{AuthorityProxy, CheckAuthorizationFlags, Subject}; +use std::sync::Arc; + +use crate::{ + secure_memory::* +}; + +pub struct BiometricLockSystem { + // The userkeys that are held in memory MUST be protected from memory dumping attacks, to ensure + // locked vaults cannot be unlocked + secure_memory: Arc> +} + +impl BiometricLockSystem { + pub fn new() -> Self { + Self { + secure_memory: Arc::new(Mutex::new(crate::secure_memory::memfd_secret::MemfdSecretKVStore::new())) + } + } +} + +impl super::BiometricV2Trait 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) + } + } + } + + 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) + } + + async fn enroll_persistent(&self, _user_id: &str, _key: &[u8]) -> Result<()> { + 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); + } + + async fn unlock(&self, user_id: &str, _hwnd: Vec) -> Result> { + if self.authenticate(Vec::new(), "".to_string()).await? == false { + return Err(anyhow!("Authentication failed")); + } + + let secure_memory = self.secure_memory.lock().await; + secure_memory.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; + return Ok(secure_memory.has(user_id)); + } + + async fn has_persistent(&self, _user_id: &str) -> Result { + return Ok(false); + } +} diff --git a/apps/desktop/desktop_native/core/src/biometric_v2/mod.rs b/apps/desktop/desktop_native/core/src/biometric_v2/mod.rs index 28c7a2b3c2a..d128524fef6 100644 --- a/apps/desktop/desktop_native/core/src/biometric_v2/mod.rs +++ b/apps/desktop/desktop_native/core/src/biometric_v2/mod.rs @@ -1,12 +1,12 @@ use anyhow::{Result}; #[allow(clippy::module_inception)] -#[cfg_attr(target_os = "linux", path = "unimplemented.rs")] +#[cfg_attr(target_os = "linux", path = "linux.rs")] #[cfg_attr(target_os = "macos", path = "unimplemented.rs")] #[cfg_attr(target_os = "windows", path = "windows.rs")] mod biometric_v2; -#[cfg_attr(target_os = "windows", path = "windows_focus.rs")] +#[cfg(target_os = "windows")] mod windows_focus; pub use biometric_v2::BiometricLockSystem; diff --git a/apps/desktop/desktop_native/core/src/biometric_v2/windows.rs b/apps/desktop/desktop_native/core/src/biometric_v2/windows.rs index b42c22f4bdb..058d7550bc3 100644 --- a/apps/desktop/desktop_native/core/src/biometric_v2/windows.rs +++ b/apps/desktop/desktop_native/core/src/biometric_v2/windows.rs @@ -20,6 +20,7 @@ //! Since the keychain can be accessed by all user-space processes, the challenge is known to all userspace processes. //! Therefore, to circumvent the security measure, the attacker would need to create a fake Windows-Hello prompt, and //! get the user to confirm it. + use std::{ffi::c_void, sync::{atomic::AtomicBool, Arc}}; use aes::cipher::KeyInit; 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 new file mode 100644 index 00000000000..c25306a8b7d --- /dev/null +++ b/apps/desktop/desktop_native/core/src/secure_memory/memfd_secret.rs @@ -0,0 +1,106 @@ +use std::{collections::HashMap, ptr::NonNull, sync::LazyLock}; + +use crate::secure_memory::SecureMemoryStore; + +/// https://man.archlinux.org/man/memfd_secret.2.en +/// The memfd_secret store protects the data using the `memfd_secret` syscall. The +/// 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 +pub(crate) struct MemfdSecretKVStore { + map: HashMap>, +} + +impl MemfdSecretKVStore { + pub(crate) fn new() -> Self { + MemfdSecretKVStore { + map: HashMap::new(), + } + } +} + +impl SecureMemoryStore for MemfdSecretKVStore { + fn put(&mut self, key: String, value: &[u8]) { + let mut ptr: std::ptr::NonNull<[u8]> = unsafe { + memsec::memfd_secret_sized(value.len()).expect("memfd_secret_sized should work") + }; + unsafe { + std::ptr::copy_nonoverlapping(value.as_ptr(), ptr.as_mut().as_mut_ptr(), value.len()); + } + self.map.insert(key, ptr); + } + + fn get(&self, key: &str) -> Option> { + let ptr = self.map.get(key)?; + let value = unsafe { ptr.as_ref() }; + Some(value.to_vec()) + } + + fn has(&self, key: &str) -> bool { + self.map.contains_key(key) + } + + fn remove(&mut self, key: &str) { + if let Some(value) = self.map.remove(key) { + unsafe { + memsec::free_memfd_secret(value); + } + } + } + + fn clear(&mut self) { + for (_, value) in self.map.drain() { + unsafe { + memsec::free_memfd_secret(value); + } + } + } +} + +impl Drop for MemfdSecretKVStore { + fn drop(&mut self) { + self.clear(); + } +} + +pub(super) fn is_supported() -> bool { + // To test if memfd_secret is supported, we try to allocate a 1 byte and see if that + // succeeds. + static IS_SUPPORTED: LazyLock = LazyLock::new(|| { + let Some(ptr): Option> = (unsafe { memsec::memfd_secret_sized(1) }) else { + return false; + }; + + // Check that the pointer is readable and writable + let result = unsafe { + let ptr = ptr.as_ptr() as *mut u8; + *ptr = 30; + *ptr += 107; + *ptr == 137 + }; + + unsafe { memsec::free_memfd_secret(ptr) }; + result + }); + *IS_SUPPORTED +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_memfd_secret_kv_store() { + let mut store = MemfdSecretKVStore::new(); + let key = "test_key".to_string(); + let value = vec![1, 2, 3, 4, 5]; + + store.put(key.clone(), &value); + assert!(store.has(&key)); + assert_eq!(store.get(&key), Some(value.clone())); + + store.remove(&key); + assert!(!store.has(&key)); + assert_eq!(store.get(&key), None); + } +} \ No newline at end of file diff --git a/apps/desktop/desktop_native/core/src/secure_memory/mod.rs b/apps/desktop/desktop_native/core/src/secure_memory/mod.rs index 798a19d49ea..5e941319d6f 100644 --- a/apps/desktop/desktop_native/core/src/secure_memory/mod.rs +++ b/apps/desktop/desktop_native/core/src/secure_memory/mod.rs @@ -1,7 +1,7 @@ #[cfg(target_os = "windows")] pub(crate) mod dpapi; #[cfg(target_os = "linux")] -mod unimplemented; +pub(crate) mod memfd_secret; #[cfg(target_os = "macos")] mod unimplemented;