bitwarden/browser
1
0
Fork 0
mirror of https://github.com/bitwarden/browser synced 2025-12-19 09:43:23 +00:00
Code Issues Releases Wiki Activity

[PM-14252] Switch to oo7 and drop libsecret (#11900)

Browse Source 
* Switch to oo7 and drop libsecret

* Fix tests

* Fix windows

* Fix windows

* Fix windows

* Fix windows

* Add migration

* Update apps/desktop/desktop_native/core/src/password/unix.rs

Co-authored-by: Daniel García <dani-garcia@users.noreply.github.com>

* Remove libsecret in ci

* Move allow async to trait level

* Fix comment

* Pin oo7 dependency

---------

Co-authored-by: Matt Bishop <mbishop@bitwarden.com>
Co-authored-by: Daniel García <dani-garcia@users.noreply.github.com>
This commit is contained in:
Bernd Schoolmann
2024-12-04 17:03:34 +01:00
committed by GitHub
parent 98702d9f50
commit 80a898bd8c
14 changed files with 278 additions and 394 deletions
Download Patch File Download Diff File Expand all files Collapse all files

20
apps/desktop/desktop_native/core/Cargo.toml
View File

@@ -10,15 +10,13 @@ default = ["sys"]
manual_test = []
sys = [
"dep:widestring",
"dep:windows",
"dep:core-foundation",
"dep:security-framework",
"dep:security-framework-sys",
"dep:gio",
"dep:libsecret",
"dep:zbus",
"dep:zbus_polkit",
"dep:widestring",
"dep:windows",
"dep:core-foundation",
"dep:security-framework",
"dep:security-framework-sys",
"dep:zbus",
"dep:zbus_polkit",
]
[dependencies]
@@ -85,7 +83,7 @@ security-framework = { version = "=3.0.0", optional = true }
security-framework-sys = { version = "=2.12.0", optional = true }
[target.'cfg(target_os = "linux")'.dependencies]
gio = { version = "=0.19.5", optional = true }
libsecret = { version = "=0.5.0", optional = true }
oo7 = "=0.3.3"
zbus = { version = "=4.4.0", optional = true }
zbus_polkit = { version = "=4.0.0", optional = true }

4
apps/desktop/desktop_native/core/src/biometric/macos.rs
View File

@@ -18,7 +18,7 @@ impl super::BiometricTrait for Biometric {
bail!("platform not supported");
}
fn get_biometric_secret(
async fn get_biometric_secret(
_service: &str,
_account: &str,
_key_material: Option<KeyMaterial>,
@@ -26,7 +26,7 @@ impl super::BiometricTrait for Biometric {
bail!("platform not supported");
}
fn set_biometric_secret(
async fn set_biometric_secret(
_service: &str,
_account: &str,
_secret: &str,

7
apps/desktop/desktop_native/core/src/biometric/mod.rs
View File

@@ -22,20 +22,19 @@ pub struct OsDerivedKey {
pub iv_b64: String,
}
#[allow(async_fn_in_trait)]
pub trait BiometricTrait {
#[allow(async_fn_in_trait)]
async fn prompt(hwnd: Vec<u8>, message: String) -> Result<bool>;
#[allow(async_fn_in_trait)]
async fn available() -> Result<bool>;
fn derive_key_material(secret: Option<&str>) -> Result<OsDerivedKey>;
fn set_biometric_secret(
async fn set_biometric_secret(
service: &str,
account: &str,
secret: &str,
key_material: Option<KeyMaterial>,
iv_b64: &str,
) -> Result<String>;
fn get_biometric_secret(
async fn get_biometric_secret(
service: &str,
account: &str,
key_material: Option<KeyMaterial>,

8
apps/desktop/desktop_native/core/src/biometric/unix.rs
View File

@@ -73,7 +73,7 @@ impl super::BiometricTrait for Biometric {
Ok(OsDerivedKey { key_b64, iv_b64 })
}
fn set_biometric_secret(
async fn set_biometric_secret(
service: &str,
account: &str,
secret: &str,
@@ -85,11 +85,11 @@ impl super::BiometricTrait for Biometric {
))?;
let encrypted_secret = encrypt(secret, &key_material, iv_b64)?;
crate::password::set_password(service, account, &encrypted_secret)?;
crate::password::set_password(service, account, &encrypted_secret).await?;
Ok(encrypted_secret)
}
fn get_biometric_secret(
async fn get_biometric_secret(
service: &str,
account: &str,
key_material: Option<KeyMaterial>,
@@ -98,7 +98,7 @@ impl super::BiometricTrait for Biometric {
"Key material is required for polkit protected keys"
))?;
let encrypted_secret = crate::password::get_password(service, account)?;
let encrypted_secret = crate::password::get_password(service, account).await?;
let secret = CipherString::from_str(&encrypted_secret)?;
return Ok(decrypt(&secret, &key_material)?);
}

42
apps/desktop/desktop_native/core/src/biometric/windows.rs
View File

@@ -121,7 +121,7 @@ impl super::BiometricTrait for Biometric {
Ok(OsDerivedKey { key_b64, iv_b64 })
}
fn set_biometric_secret(
async fn set_biometric_secret(
service: &str,
account: &str,
secret: &str,
@@ -133,11 +133,11 @@ impl super::BiometricTrait for Biometric {
))?;
let encrypted_secret = encrypt(secret, &key_material, iv_b64)?;
crate::password::set_password(service, account, &encrypted_secret)?;
crate::password::set_password(service, account, &encrypted_secret).await?;
Ok(encrypted_secret)
}
fn get_biometric_secret(
async fn get_biometric_secret(
service: &str,
account: &str,
key_material: Option<KeyMaterial>,
@@ -146,7 +146,7 @@ impl super::BiometricTrait for Biometric {
"Key material is required for Windows Hello protected keys"
))?;
let encrypted_secret = crate::password::get_password(service, account)?;
let encrypted_secret = crate::password::get_password(service, account).await?;
match CipherString::from_str(&encrypted_secret) {
Ok(secret) => {
// If the secret is a CipherString, it is encrypted and we need to decrypt it.
@@ -292,9 +292,9 @@ mod tests {
assert_eq!(decrypt(&secret, &key_material).unwrap(), "secret")
}
#[test]
fn get_biometric_secret_requires_key() {
let result = <Biometric as BiometricTrait>::get_biometric_secret("", "", None);
#[tokio::test]
async fn get_biometric_secret_requires_key() {
let result = <Biometric as BiometricTrait>::get_biometric_secret("", "", None).await;
assert!(result.is_err());
assert_eq!(
result.unwrap_err().to_string(),
@@ -302,29 +302,25 @@ mod tests {
);
}
#[test]
fn get_biometric_secret_handles_unencrypted_secret() {
scopeguard::defer! {
crate::password::delete_password("test", "test").unwrap();
}
#[tokio::test]
async fn get_biometric_secret_handles_unencrypted_secret() {
let test = "test";
let secret = "password";
let key_material = KeyMaterial {
os_key_part_b64: "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=".to_owned(),
client_key_part_b64: Some("AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=".to_owned()),
};
crate::password::set_password(test, test, secret).unwrap();
crate::password::set_password(test, test, secret).await.unwrap();
let result =
<Biometric as BiometricTrait>::get_biometric_secret(test, test, Some(key_material))
.await
.unwrap();
crate::password::delete_password("test", "test").await.unwrap();
assert_eq!(result, secret);
}
#[test]
fn get_biometric_secret_handles_encrypted_secret() {
scopeguard::defer! {
crate::password::delete_password("test", "test").unwrap();
}
#[tokio::test]
async fn get_biometric_secret_handles_encrypted_secret() {
let test = "test";
let secret =
CipherString::from_str("0.l9fhDUP/wDJcKwmEzcb/3w==|uP4LcqoCCj5FxBDP77NV6Q==").unwrap(); // output from test_encrypt
@@ -332,17 +328,19 @@ mod tests {
os_key_part_b64: "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=".to_owned(),
client_key_part_b64: Some("AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=".to_owned()),
};
crate::password::set_password(test, test, &secret.to_string()).unwrap();
crate::password::set_password(test, test, &secret.to_string()).await.unwrap();
let result =
<Biometric as BiometricTrait>::get_biometric_secret(test, test, Some(key_material))
.await
.unwrap();
crate::password::delete_password("test", "test").await.unwrap();
assert_eq!(result, "secret");
}
#[test]
fn set_biometric_secret_requires_key() {
let result = <Biometric as BiometricTrait>::set_biometric_secret("", "", "", None, "");
#[tokio::test]
async fn set_biometric_secret_requires_key() {
let result = <Biometric as BiometricTrait>::set_biometric_secret("", "", "", None, "").await;
assert!(result.is_err());
assert_eq!(
result.unwrap_err().to_string(),

31
apps/desktop/desktop_native/core/src/password/macos.rs
View File

@@ -3,26 +3,22 @@ use security_framework::passwords::{
delete_generic_password, get_generic_password, set_generic_password,
};
pub fn get_password(service: &str, account: &str) -> Result<String> {
pub async fn get_password(service: &str, account: &str) -> Result<String> {
let result = String::from_utf8(get_generic_password(&service, &account)?)?;
Ok(result)
}
pub fn get_password_keytar(service: &str, account: &str) -> Result<String> {
get_password(service, account)
}
pub fn set_password(service: &str, account: &str, password: &str) -> Result<()> {
pub async fn set_password(service: &str, account: &str, password: &str) -> Result<()> {
let result = set_generic_password(&service, &account, password.as_bytes())?;
Ok(result)
}
pub fn delete_password(service: &str, account: &str) -> Result<()> {
pub async fn delete_password(service: &str, account: &str) -> Result<()> {
let result = delete_generic_password(&service, &account)?;
Ok(result)
}
pub fn is_available() -> Result<bool> {
pub async fn is_available() -> Result<bool> {
Ok(true)
}
@@ -30,18 +26,17 @@ pub fn is_available() -> Result<bool> {
mod tests {
use super::*;
#[test]
fn test() {
scopeguard::defer!(delete_password("BitwardenTest", "BitwardenTest").unwrap_or({}););
set_password("BitwardenTest", "BitwardenTest", "Random").unwrap();
#[tokio::test]
async fn test() {
set_password("BitwardenTest", "BitwardenTest", "Random").await.unwrap();
assert_eq!(
"Random",
get_password("BitwardenTest", "BitwardenTest").unwrap()
get_password("BitwardenTest", "BitwardenTest").await.unwrap()
);
delete_password("BitwardenTest", "BitwardenTest").unwrap();
delete_password("BitwardenTest", "BitwardenTest").await.unwrap();
// Ensure password is deleted
match get_password("BitwardenTest", "BitwardenTest") {
match get_password("BitwardenTest", "BitwardenTest").await {
Ok(_) => panic!("Got a result"),
Err(e) => assert_eq!(
"The specified item could not be found in the keychain.",
@@ -50,9 +45,9 @@ mod tests {
}
}
#[test]
fn test_error_no_password() {
match get_password("Unknown", "Unknown") {
#[tokio::test]
async fn test_error_no_password() {
match get_password("Unknown", "Unknown").await {
Ok(_) => panic!("Got a result"),
Err(e) => assert_eq!(
"The specified item could not be found in the keychain.",

154
apps/desktop/desktop_native/core/src/password/unix.rs
View File

@@ -1,106 +1,106 @@
use anyhow::{anyhow, Result};
use libsecret::{password_clear_sync, password_lookup_sync, password_store_sync, Schema};
use oo7::dbus::{self};
use std::collections::HashMap;
pub fn get_password(service: &str, account: &str) -> Result<String> {
let res = password_lookup_sync(
Some(&get_schema()),
build_attributes(service, account),
gio::Cancellable::NONE,
)?;
match res {
Some(s) => Ok(String::from(s)),
None => Err(anyhow!("No password found")),
}
}
pub fn get_password_keytar(service: &str, account: &str) -> Result<String> {
get_password(service, account)
}
pub fn set_password(service: &str, account: &str, password: &str) -> Result<()> {
let result = password_store_sync(
Some(&get_schema()),
build_attributes(service, account),
Some(&libsecret::COLLECTION_DEFAULT),
&format!("{}/{}", service, account),
password,
gio::Cancellable::NONE,
)?;
Ok(result)
}
pub fn delete_password(service: &str, account: &str) -> Result<()> {
let result = password_clear_sync(
Some(&get_schema()),
build_attributes(service, account),
gio::Cancellable::NONE,
)?;
Ok(result)
}
pub fn is_available() -> Result<bool> {
let result = password_clear_sync(
Some(&get_schema()),
build_attributes("bitwardenSecretsAvailabilityTest", "test"),
gio::Cancellable::NONE,
);
match result {
Ok(_) => Ok(true),
pub async fn get_password(service: &str, account: &str) -> Result<String> {
match get_password_new(service, account).await {
Ok(res) => Ok(res),
Err(_) => {
println!("secret-service unavailable: {:?}", result);
Ok(false)
get_password_legacy(service, account).await
}
}
}
fn get_schema() -> Schema {
let mut attributes = std::collections::HashMap::new();
attributes.insert("service", libsecret::SchemaAttributeType::String);
attributes.insert("account", libsecret::SchemaAttributeType::String);
libsecret::Schema::new(
"org.freedesktop.Secret.Generic",
libsecret::SchemaFlags::NONE,
attributes,
)
async fn get_password_new(service: &str, account: &str) -> Result<String> {
let keyring = oo7::Keyring::new().await?;
let attributes = HashMap::from([("service", service), ("account", account)]);
let results = keyring.search_items(&attributes).await?;
let res = results.get(0);
match res {
Some(res) => {
let secret = res.secret().await?;
Ok(String::from_utf8(secret.to_vec())?)
},
None => Err(anyhow!("no result"))
}
}
fn build_attributes<'a>(service: &'a str, account: &'a str) -> HashMap<&'a str, &'a str> {
let mut attributes = HashMap::new();
attributes.insert("service", service);
attributes.insert("account", account);
// forces to read via secret service; remvove after 2025.03
async fn get_password_legacy(service: &str, account: &str) -> Result<String> {
println!("falling back to get legacy {} {}", service, account);
let svc = dbus::Service::new().await?;
let collection = svc.default_collection().await?;
let keyring = oo7::Keyring::DBus(collection);
let attributes = HashMap::from([("service", service), ("account", account)]);
let results = keyring.search_items(&attributes).await?;
let res = results.get(0);
match res {
Some(res) => {
let secret = res.secret().await?;
println!("deleting legacy secret service entry {} {}", service, account);
keyring.delete(&attributes).await?;
let secret_string = String::from_utf8(secret.to_vec())?;
set_password(service, account, &secret_string).await?;
Ok(secret_string)
},
None => Err(anyhow!("no result"))
}
}
attributes
pub async fn set_password(service: &str, account: &str, password: &str) -> Result<()> {
let keyring = oo7::Keyring::new().await?;
let attributes = HashMap::from([("service", service), ("account", account)]);
keyring.create_item("org.freedesktop.Secret.Generic", &attributes, password, true).await?;
Ok(())
}
pub async fn delete_password(service: &str, account: &str) -> Result<()> {
let keyring = oo7::Keyring::new().await?;
let attributes = HashMap::from([("service", service), ("account", account)]);
keyring.delete(&attributes).await?;
Ok(())
}
pub async fn is_available() -> Result<bool> {
match oo7::Keyring::new().await {
Ok(_) => Ok(true),
_ => Ok(false),
}
}
#[cfg(test)]
mod tests {
use super::*;
#[test]
fn test() {
scopeguard::defer!(delete_password("BitwardenTest", "BitwardenTest").unwrap_or({}););
set_password("BitwardenTest", "BitwardenTest", "Random").unwrap();
#[tokio::test]
async fn test() {
set_password("BitwardenTest", "BitwardenTest", "Random").await.unwrap();
assert_eq!(
"Random",
get_password("BitwardenTest", "BitwardenTest").unwrap()
get_password("BitwardenTest", "BitwardenTest").await.unwrap()
);
delete_password("BitwardenTest", "BitwardenTest").unwrap();
delete_password("BitwardenTest", "BitwardenTest").await.unwrap();
// Ensure password is deleted
match get_password("BitwardenTest", "BitwardenTest") {
Ok(_) => panic!("Got a result"),
Err(e) => assert_eq!("No password found", e.to_string()),
match get_password("BitwardenTest", "BitwardenTest").await {
Ok(_) => {
panic!("Got a result")
}
Err(e) => assert_eq!(
"no result",
e.to_string()
),
}
}
#[test]
fn test_error_no_password() {
match get_password("BitwardenTest", "BitwardenTest") {
#[tokio::test]
async fn test_error_no_password() {
match get_password("Unknown", "Unknown").await {
Ok(_) => panic!("Got a result"),
Err(e) => assert_eq!("No password found", e.to_string()),
Err(e) => assert_eq!(
"no result",
e.to_string()
),
}
}
}

69
apps/desktop/desktop_native/core/src/password/windows.rs
View File

@@ -13,7 +13,7 @@ use windows::{
const CRED_FLAGS_NONE: u32 = 0;
pub fn get_password<'a>(service: &str, account: &str) -> Result<String> {
pub async fn get_password<'a>(service: &str, account: &str) -> Result<String> {
let target_name = U16CString::from_str(target_name(service, account))?;
let mut credential: *mut CREDENTIALW = std::ptr::null_mut();
@@ -45,39 +45,7 @@ pub fn get_password<'a>(service: &str, account: &str) -> Result<String> {
Ok(String::from(password))
}
// Remove this after sufficient releases
pub fn get_password_keytar<'a>(service: &str, account: &str) -> Result<String> {
let target_name = U16CString::from_str(target_name(service, account))?;
let mut credential: *mut CREDENTIALW = std::ptr::null_mut();
let credential_ptr = &mut credential;
let result = unsafe {
CredReadW(
PCWSTR(target_name.as_ptr()),
CRED_TYPE_GENERIC,
CRED_FLAGS_NONE,
credential_ptr,
)
};
scopeguard::defer!({
unsafe { CredFree(credential as *mut _) };
});
result?;
let password = unsafe {
std::str::from_utf8_unchecked(std::slice::from_raw_parts(
(*credential).CredentialBlob,
(*credential).CredentialBlobSize as usize,
))
};
Ok(String::from(password))
}
pub fn set_password(service: &str, account: &str, password: &str) -> Result<()> {
pub async fn set_password(service: &str, account: &str, password: &str) -> Result<()> {
let mut target_name = U16CString::from_str(target_name(service, account))?;
let mut user_name = U16CString::from_str(account)?;
let last_written = FILETIME {
@@ -108,7 +76,7 @@ pub fn set_password(service: &str, account: &str, password: &str) -> Result<()>
Ok(())
}
pub fn delete_password(service: &str, account: &str) -> Result<()> {
pub async fn delete_password(service: &str, account: &str) -> Result<()> {
let target_name = U16CString::from_str(target_name(service, account))?;
unsafe {
@@ -122,7 +90,7 @@ pub fn delete_password(service: &str, account: &str) -> Result<()> {
Ok(())
}
pub fn is_available() -> Result<bool> {
pub async fn is_available() -> Result<bool> {
Ok(true)
}
@@ -142,36 +110,25 @@ fn convert_error(e: windows::core::Error) -> String {
mod tests {
use super::*;
#[test]
fn test() {
scopeguard::defer!(delete_password("BitwardenTest", "BitwardenTest").unwrap_or({}););
set_password("BitwardenTest", "BitwardenTest", "Random").unwrap();
#[tokio::test]
async fn test() {
set_password("BitwardenTest", "BitwardenTest", "Random").await.unwrap();
assert_eq!(
"Random",
get_password("BitwardenTest", "BitwardenTest").unwrap()
get_password("BitwardenTest", "BitwardenTest").await.unwrap()
);
delete_password("BitwardenTest", "BitwardenTest").unwrap();
delete_password("BitwardenTest", "BitwardenTest").await.unwrap();
// Ensure password is deleted
match get_password("BitwardenTest", "BitwardenTest") {
match get_password("BitwardenTest", "BitwardenTest").await {
Ok(_) => panic!("Got a result"),
Err(e) => assert_eq!("Password not found.", e.to_string()),
}
}
#[test]
fn test_get_password_keytar() {
scopeguard::defer!(delete_password("BitwardenTest", "BitwardenTest").unwrap_or({}););
keytar::set_password("BitwardenTest", "BitwardenTest", "HelloFromKeytar").unwrap();
assert_eq!(
"HelloFromKeytar",
get_password_keytar("BitwardenTest", "BitwardenTest").unwrap()
);
}
#[test]
fn test_error_no_password() {
match get_password("BitwardenTest", "BitwardenTest") {
#[tokio::test]
async fn test_error_no_password() {
match get_password("BitwardenTest", "BitwardenTest").await {
Ok(_) => panic!("Got a result"),
Err(e) => assert_eq!("Password not found.", e.to_string()),
}