From c6e3ca76baf975ddb07b476def9e0d9c3ed8fd67 Mon Sep 17 00:00:00 2001 From: Daniel Brotsky Date: Mon, 8 Nov 2021 13:09:02 -0800 Subject: [PATCH 1/5] Start work on normalizing keyring names (#60). Add get_password_for_target on Windows. --- src/windows.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/windows.rs b/src/windows.rs index 42981a5..c5633e6 100644 --- a/src/windows.rs +++ b/src/windows.rs @@ -91,13 +91,17 @@ impl<'a> Keyring<'a> { } pub fn get_password(&self) -> Result { + let target_name: String = [self.username, self.service].join("."); + self.get_password_for_target(&target_name) + } + + pub fn get_password_for_target(&self, target_name: &str) -> Result { // passing uninitialized pcredential. // Should be ok; it's freed by a windows api // call CredFree. let mut pcredential = MaybeUninit::uninit(); - let target_name: String = [self.username, self.service].join("."); - let target_name = to_wstr(&target_name); + let target_name = to_wstr(target_name); let cred_type = CRED_TYPE_GENERIC; From 4a6821191241439666ab2785b69e1896c0c51052 Mon Sep 17 00:00:00 2001 From: Daniel Brotsky Date: Wed, 10 Nov 2021 10:24:06 -0800 Subject: [PATCH 2/5] Finish refactoring; fix #60 on Mac Keyrings now just keep the platform-specific attributes needed to store and retrieve the corresponding platform keyring item. This simplifies the platform-specific code and allows the actual keyring implementation to be platform independent (and thus documentable properly). In addition, you can now create a Keyring by passing in a mapper function which specifies exactly what platform attributes should be used given the service and username. This allows compatibility with 3rd-party keyring clients. --- src/attrs.rs | 92 ++++++++++++++++++++++++++++++++++++ src/error.rs | 10 +++- src/lib.rs | 104 ++++++++++++++++++++++++++++++++++------- src/macos.rs | 130 +++++++++++++++------------------------------------ 4 files changed, 224 insertions(+), 112 deletions(-) create mode 100644 src/attrs.rs diff --git a/src/attrs.rs b/src/attrs.rs new file mode 100644 index 0000000..4d4b146 --- /dev/null +++ b/src/attrs.rs @@ -0,0 +1,92 @@ +/* +Every platform's secure storage system keeps a set of attributes with +each stored item. Which attributes are allowed can vary, as can which +of the attributes are required and which are used to identify the item. + +The attribute model supported by this crate is that each item has only +two attributes: username and service, and they are used together to +uniquely identify the item. + +The mismatch between this crate's attribute model and the underlying +platform's attribute model can lead to incompatibility with 3rd-party +applications whose attribute model, while consistent with the underlying +platform model, may be more or less fine-grained than this crate's model. + +For example: + +* On Windows, generic credential are identified by an arbitrary string, +and that string may not be constructed by a third party application +the same way this crate constructs it from username and service. +* On Linux, additional attributes can be used by 3rd parties to produce +credentials identified my more than just the two attributes this crate +uses by default. + +Thus, to provide interoperability with 3rd party credential clients, +we provide a way for clients of this crate to override this crate's +default algorithm for how the username and service are combined so as to +produce the platform-specific attributes that identify each item. + */ + +use std::collections::HashMap; + +#[derive(Debug)] +pub enum Platform { + Linux, + Windows, + MacOs, +} + +#[derive(Debug)] +pub struct LinuxIdentity { + pub attributes: HashMap<&'static str, String>, +} + +#[derive(Debug)] +pub struct WinIdentity { + pub target_name: String, +} + +#[derive(Debug)] +pub struct MacIdentity { + pub service: String, + pub account: String, +} + +#[derive(Debug)] +pub enum PlatformIdentity { + Linux(LinuxIdentity), + Win(WinIdentity), + Mac(MacIdentity), +} + +impl PlatformIdentity { + pub fn matches_platform(&self, os: &Platform) -> bool { + match self { + PlatformIdentity::Linux(_) => matches!(os, Platform::Linux), + PlatformIdentity::Mac(_) => matches!(os, Platform::MacOs), + PlatformIdentity::Win(_) => matches!(os, Platform::Windows), + } + } +} + +// TODO: Make this a Fn trait so we can accept closures +pub type IdentityMapper = fn(&Platform, &str, &str) -> PlatformIdentity; + +pub fn default_identity_mapper(os: Platform, service: &str, username: &str) -> PlatformIdentity { + match os { + Platform::Linux => PlatformIdentity::Linux(LinuxIdentity { + attributes: HashMap::from([ + ("service", service.to_string()), + ("username", username.to_string()), + ("application", String::from("rust-keyring")), + ]), + }), + Platform::Windows => PlatformIdentity::Win(WinIdentity { + target_name: format!("{}.{}", username, service), + }), + Platform::MacOs => PlatformIdentity::Mac(MacIdentity { + service: service.to_string(), + account: username.to_string(), + }), + } +} diff --git a/src/error.rs b/src/error.rs index 4a159c9..b216dfe 100644 --- a/src/error.rs +++ b/src/error.rs @@ -19,6 +19,7 @@ pub enum KeyringError { NoBackendFound, NoPasswordFound, Parse(FromUtf8Error), + BadPlatformMapValue, } impl fmt::Display for KeyringError { @@ -34,7 +35,11 @@ impl fmt::Display for KeyringError { KeyringError::WindowsVaultError => write!(f, "Windows Vault Error"), KeyringError::NoBackendFound => write!(f, "Keyring error: No Backend Found"), KeyringError::NoPasswordFound => write!(f, "Keyring Error: No Password Found"), - KeyringError::Parse(ref err) => write!(f, "Keyring Parse Error: {}", err), + KeyringError::Parse(ref err) => write!(f, "Password Parse Error: {}", err), + KeyringError::BadPlatformMapValue => write!( + f, + "Keyring Error: Custom IdentityMapper value doesn't match this platform" + ), } } } @@ -55,6 +60,9 @@ impl error::Error for KeyringError { KeyringError::NoBackendFound => "No Backend Found", KeyringError::NoPasswordFound => "No Password Found", KeyringError::Parse(ref err) => err.description(), + KeyringError::BadPlatformMapValue => { + "Custom IdentityMapper value doesn't match this platform" + } } } diff --git a/src/lib.rs b/src/lib.rs index 75b2f7f..e40ba4d 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -2,27 +2,68 @@ //! //! Allows for setting and getting passwords on Linux, OSX, and Windows -// Configure for Linux -#[cfg(target_os = "linux")] -mod linux; -#[cfg(target_os = "linux")] -pub use linux::Keyring; - -// Configure for Windows -#[cfg(target_os = "windows")] -mod windows; -#[cfg(target_os = "windows")] -pub use windows::Keyring; - -// Configure for OSX -#[cfg(target_os = "macos")] -mod macos; -#[cfg(target_os = "macos")] -pub use macos::Keyring; - +mod attrs; mod error; + +use crate::error::KeyringError::BadPlatformMapValue; +pub use attrs::{IdentityMapper, Platform, PlatformIdentity}; pub use error::{KeyringError, Result}; +// compile-time Platform known at runtime +fn platform() -> Platform { + #[cfg(target_os = "linux")] + return Platform::Linux; + #[cfg(target_os = "windows")] + return Platform::Windows; + #[cfg(target_os = "macos")] + return Platform::MacOs; +} + +// Platform-specific implementations +#[cfg_attr(target_os = "linux", path = "linux.rs")] +#[cfg_attr(target_os = "windows", path = "windows.rs")] +#[cfg_attr(target_os = "macos", path = "macos.rs")] +mod platform; + +#[derive(Debug)] +pub struct Keyring { + map: PlatformIdentity, +} + +impl Keyring { + pub fn new(service: &str, username: &str) -> Keyring { + Keyring { + map: attrs::default_identity_mapper(platform(), service, username), + } + } + + pub fn new_with_mapper( + service: &str, + username: &str, + mapper: IdentityMapper, + ) -> Result { + let os = platform(); + let map = mapper(&os, service, username); + if map.matches_platform(&os) { + Ok(Keyring { map }) + } else { + Err(BadPlatformMapValue) + } + } + + pub fn set_password(&self, password: &str) -> Result<()> { + platform::set_password(&self.map, password) + } + + pub fn get_password(&self) -> Result { + platform::get_password(&self.map) + } + + pub fn delete_password(&self) -> Result<()> { + platform::delete_password(&self.map) + } +} + #[cfg(test)] mod tests { use super::*; @@ -33,6 +74,18 @@ mod tests { static TEST_ASCII_PASSWORD: &'static str = "my_password"; static TEST_NON_ASCII_PASSWORD: &'static str = "大根"; + #[test] + #[serial] + fn test_empty_keyring() { + let service = generate_random_string(); + let username = generate_random_string(); + let keyring = Keyring::new(&service, &username); + assert!( + keyring.get_password().is_err(), + "Read a password from a non-existent platform item" + ) + } + #[test] #[serial] fn test_empty_password_input() { @@ -75,4 +128,19 @@ mod tests { "Able to read a deleted password" ) } + + // TODO: write tests for custom mappers. + // This might be better done in a separate test file. + + // utility + fn generate_random_string() -> String { + // from the Rust Cookbook: + // https://rust-lang-nursery.github.io/rust-cookbook/algorithms/randomness.html + use rand::{distributions::Alphanumeric, thread_rng, Rng}; + thread_rng() + .sample_iter(&Alphanumeric) + .take(30) + .map(char::from) + .collect() + } } diff --git a/src/macos.rs b/src/macos.rs index 029d3d8..36fc259 100644 --- a/src/macos.rs +++ b/src/macos.rs @@ -1,70 +1,46 @@ -use crate::error::Result; use security_framework::os::macos::keychain::SecKeychain; use security_framework::os::macos::passwords::find_generic_password; -use std::path::Path; -pub struct Keyring<'a> { - service: &'a str, - username: &'a str, - path: Option<&'a Path>, -} - -// Eventually try to get collection into the Keyring struct? -impl<'a> Keyring<'a> { - pub fn new(service: &'a str, username: &'a str) -> Keyring<'a> { - Keyring { - service, - username, - path: None, - } - } - - #[cfg(feature = "macos-specify-keychain")] - pub fn use_keychain(service: &'a str, username: &'a str, path: &'a Path) -> Keyring<'a> { - Keyring { - service, - username, - path: Some(path), - } - } - - fn get_keychain(&self) -> security_framework::base::Result { - match self.path { - Some(path) => SecKeychain::open(path), - _ => SecKeychain::default(), - } - } +use crate::{KeyringError, PlatformIdentity, Result}; - pub fn set_password(&self, password: &str) -> Result<()> { - self.get_keychain()?.set_generic_password( - self.service, - self.username, - password.as_bytes(), - )?; +fn get_keychain() -> Result { + SecKeychain::default().map_err(KeyringError::MacOsKeychainError) +} +pub fn set_password(map: &PlatformIdentity, password: &str) -> Result<()> { + if let PlatformIdentity::Mac(map) = map { + get_keychain()? + .set_generic_password(&map.service, &map.account, password.as_bytes()) + .map_err(KeyringError::MacOsKeychainError)?; Ok(()) + } else { + Err(KeyringError::BadPlatformMapValue) } +} - pub fn get_password(&self) -> Result { +pub fn get_password(map: &PlatformIdentity) -> Result { + if let PlatformIdentity::Mac(map) = map { let (password_bytes, _) = - find_generic_password(Some(&[self.get_keychain()?]), self.service, self.username)?; - + find_generic_password(Some(&[get_keychain()?]), &map.service, &map.account) + .map_err(KeyringError::MacOsKeychainError)?; // Mac keychain allows non-UTF8 values, but this library only supports adding UTF8 items // to the keychain, so this should only fail if we are trying to retrieve a non-UTF8 // password that was added to the keychain by another library - - let password = String::from_utf8(password_bytes.to_vec())?; - + let password = String::from_utf8(password_bytes.to_vec()).map_err(KeyringError::Parse)?; Ok(password) + } else { + Err(KeyringError::BadPlatformMapValue) } +} - pub fn delete_password(&self) -> Result<()> { - let (_, item) = - find_generic_password(Some(&[self.get_keychain()?]), self.service, self.username)?; - +pub fn delete_password(map: &PlatformIdentity) -> Result<()> { + if let PlatformIdentity::Mac(map) = map { + let (_, item) = find_generic_password(Some(&[get_keychain()?]), &map.service, &map.account) + .map_err(KeyringError::MacOsKeychainError)?; item.delete(); - Ok(()) + } else { + Err(KeyringError::BadPlatformMapValue) } } @@ -72,6 +48,8 @@ impl<'a> Keyring<'a> { #[cfg(target_os = "macos")] mod test { use super::*; + use crate::attrs::default_identity_mapper; + use crate::Platform; use serial_test::serial; #[test] @@ -79,60 +57,26 @@ mod test { fn test_basic() { let password_1 = "大根"; let password_2 = "0xE5A4A7E6A0B9"; // Above in hex string + let map = default_identity_mapper(Platform::MacOs, "test-service", "test-user"); - let keyring = Keyring::new("testservice", "testuser"); - - keyring.set_password(password_1).unwrap(); - let res_1 = keyring.get_password().unwrap(); + set_password(&map, password_1).unwrap(); + let response_1 = get_password(&map).unwrap(); assert_eq!( - res_1, password_1, + response_1, password_1, "Stored and retrieved passwords don't match" ); - keyring.set_password(password_2).unwrap(); - let res_2 = keyring.get_password().unwrap(); + set_password(&map, password_2).unwrap(); + let response_2 = get_password(&map).unwrap(); assert_eq!( - res_2, password_2, + response_2, password_2, "Stored and retrieved passwords don't match" ); - keyring.delete_password().unwrap(); + delete_password(&map).unwrap(); assert!( - keyring.get_password().is_err(), + get_password(&map).is_err(), "Able to read a deleted password" ) } - - #[test] - #[ignore] - #[cfg(feature = "macos-specify-keychain")] - #[serial] - fn test_basic_with_features() { - use security_framework::os::macos::keychain; - use tempfile::tempdir; - - let password_1 = "大根"; - let password_2 = "0xE5A4A7E6A0B9"; // Above in hex string - - let dir = tempdir().unwrap(); - let temp_keychain_path = dir.path().join("Temporary.keychain"); - dbg!(&temp_keychain_path); - let temp_keychain = keychain::CreateOptions::new(); - temp_keychain - .create(&temp_keychain_path) - .expect("Could not create temp keychain"); - let keyring = Keyring::use_keychain("testservice", "testuser", &temp_keychain_path); - - keyring.set_password(password_1).unwrap(); - let res_1 = keyring.get_password().unwrap(); - println!("{}:{}", res_1, password_1); - assert_eq!(res_1, password_1); - - keyring.set_password(password_2).unwrap(); - let res_2 = keyring.get_password().unwrap(); - println!("{}:{}", res_2, password_2); - assert_eq!(res_2, password_2); - - keyring.delete_password().unwrap(); - } } From cc397b526cd852f38d843721970ad9d9acf2857f Mon Sep 17 00:00:00 2001 From: Daniel Brotsky Date: Wed, 10 Nov 2021 12:46:12 -0800 Subject: [PATCH 3/5] Get Linux to compile. 1. Use Strings rather than strs to simplify lifetime management of the LinuxIdentity attributes. 2. Remove the mac keychain feature, which cannot be implemented due to deprecation of separate keychains on mac. --- Cargo.toml | 4 -- src/attrs.rs | 31 ++++++++++++--- src/linux.rs | 109 +++++++++++++++++++++++++++++++-------------------- 3 files changed, 91 insertions(+), 53 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index f5dab08..f287cef 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -9,10 +9,6 @@ repository = "https://github.com/hwchen/keyring-rs.git" version = "0.10.1" edition = "2018" -[features] -default = [] -macos-specify-keychain = [] - [target.'cfg(target_os = "macos")'.dependencies] security-framework = "2.4.2" diff --git a/src/attrs.rs b/src/attrs.rs index 4d4b146..5fcdb3d 100644 --- a/src/attrs.rs +++ b/src/attrs.rs @@ -38,7 +38,21 @@ pub enum Platform { #[derive(Debug)] pub struct LinuxIdentity { - pub attributes: HashMap<&'static str, String>, + pub attributes: HashMap, + pub label: String, +} + +impl LinuxIdentity { + pub fn attributes(&self) -> HashMap<&str, &str> { + self.attributes + .iter() + .map(|(k, v)| (k.as_str(), v.as_str())) + .collect() + } + + pub fn label(&self) -> &str { + self.label.as_str() + } } #[derive(Debug)] @@ -72,14 +86,19 @@ impl PlatformIdentity { // TODO: Make this a Fn trait so we can accept closures pub type IdentityMapper = fn(&Platform, &str, &str) -> PlatformIdentity; -pub fn default_identity_mapper(os: Platform, service: &str, username: &str) -> PlatformIdentity { - match os { +pub fn default_identity_mapper( + platform: Platform, + service: &str, + username: &str, +) -> PlatformIdentity { + match platform { Platform::Linux => PlatformIdentity::Linux(LinuxIdentity { attributes: HashMap::from([ - ("service", service.to_string()), - ("username", username.to_string()), - ("application", String::from("rust-keyring")), + ("service".to_string(), service.to_string()), + ("username".to_string(), username.to_string()), + ("application".to_string(), "rust-keyring".to_string()), ]), + label: format!("Password for service '{}', user '{}'", service, username), }), Platform::Windows => PlatformIdentity::Win(WinIdentity { target_name: format!("{}.{}", username, service), diff --git a/src/linux.rs b/src/linux.rs index d853e8c..68dfd91 100644 --- a/src/linux.rs +++ b/src/linux.rs @@ -1,64 +1,87 @@ -use crate::error::{KeyringError, Result}; use secret_service::{EncryptionType, SecretService}; -use std::collections::HashMap; -pub struct Keyring<'a> { - attributes: HashMap<&'a str, &'a str>, - service: &'a str, - username: &'a str, -} - -// Eventually try to get collection into the Keyring struct? -impl<'a> Keyring<'a> { - pub fn new(service: &'a str, username: &'a str) -> Keyring<'a> { - let attributes = HashMap::from([("service", service), ("username", username)]); - Keyring { - attributes, - service, - username, - } - } +use crate::{KeyringError, PlatformIdentity, Result}; - pub fn set_password(&self, password: &str) -> Result<()> { - let ss = SecretService::new(EncryptionType::Dh)?; - let collection = ss.get_default_collection()?; - if collection.is_locked()? { - collection.unlock()?; +pub fn set_password(map: &PlatformIdentity, password: &str) -> Result<()> { + if let PlatformIdentity::Linux(map) = map { + let ss = + SecretService::new(EncryptionType::Dh).map_err(KeyringError::SecretServiceError)?; + let collection = ss + .get_default_collection() + .map_err(KeyringError::SecretServiceError)?; + if collection + .is_locked() + .map_err(KeyringError::SecretServiceError)? + { + collection + .unlock() + .map_err(KeyringError::SecretServiceError)?; } - let mut attrs = self.attributes.clone(); - attrs.insert("application", "rust-keyring"); - let label = &format!("Password for {} on {}", self.username, self.service)[..]; collection.create_item( - label, - attrs, + map.label(), + map.attributes(), password.as_bytes(), true, // replace "text/plain", )?; Ok(()) + } else { + Err(KeyringError::BadPlatformMapValue) } +} - pub fn get_password(&self) -> Result { - let ss = SecretService::new(EncryptionType::Dh)?; - let collection = ss.get_default_collection()?; - if collection.is_locked()? { - collection.unlock()?; +pub fn get_password(map: &PlatformIdentity) -> Result { + if let PlatformIdentity::Linux(map) = map { + let ss = + SecretService::new(EncryptionType::Dh).map_err(KeyringError::SecretServiceError)?; + let collection = ss + .get_default_collection() + .map_err(KeyringError::SecretServiceError)?; + if collection + .is_locked() + .map_err(KeyringError::SecretServiceError)? + { + collection + .unlock() + .map_err(KeyringError::SecretServiceError)?; } - let search = collection.search_items(self.attributes.clone())?; + let search = collection + .search_items(map.attributes()) + .map_err(KeyringError::SecretServiceError)?; let item = search.get(0).ok_or(KeyringError::NoPasswordFound)?; let secret_bytes = item.get_secret()?; - let secret = String::from_utf8(secret_bytes)?; - Ok(secret) + // Linux keyring allows non-UTF8 values, but this library only supports adding UTF8 items + // to the keyring, so this should only fail if we are trying to retrieve a non-UTF8 + // password that was added to the keyring by another library + let password = String::from_utf8(secret_bytes).map_err(KeyringError::Parse)?; + Ok(password) + } else { + Err(KeyringError::BadPlatformMapValue) } +} - pub fn delete_password(&self) -> Result<()> { - let ss = SecretService::new(EncryptionType::Dh)?; - let collection = ss.get_default_collection()?; - if collection.is_locked()? { - collection.unlock()?; +pub fn delete_password(map: &PlatformIdentity) -> Result<()> { + if let PlatformIdentity::Linux(map) = map { + let ss = + SecretService::new(EncryptionType::Dh).map_err(KeyringError::SecretServiceError)?; + let collection = ss + .get_default_collection() + .map_err(KeyringError::SecretServiceError)?; + if collection + .is_locked() + .map_err(KeyringError::SecretServiceError)? + { + collection + .unlock() + .map_err(KeyringError::SecretServiceError)?; } - let search = collection.search_items(self.attributes.clone())?; + let search = collection + .search_items(map.attributes()) + .map_err(KeyringError::SecretServiceError)?; let item = search.get(0).ok_or(KeyringError::NoPasswordFound)?; - Ok(item.delete()?) + item.delete().map_err(KeyringError::SecretServiceError)?; + Ok(()) + } else { + Err(KeyringError::BadPlatformMapValue) } } From 9587247ade65550f2972b51df4d1660871344410 Mon Sep 17 00:00:00 2001 From: Daniel Brotsky Date: Wed, 10 Nov 2021 21:32:38 -0800 Subject: [PATCH 4/5] Get Windows to a compilable state. 1. add username into the map 2. remove platform conditionals in platform-specific files --- src/attrs.rs | 5 +++ src/macos.rs | 1 - src/windows.rs | 94 +++++++++++++++++++------------------------------- 3 files changed, 40 insertions(+), 60 deletions(-) diff --git a/src/attrs.rs b/src/attrs.rs index 5fcdb3d..36fe5d4 100644 --- a/src/attrs.rs +++ b/src/attrs.rs @@ -58,6 +58,7 @@ impl LinuxIdentity { #[derive(Debug)] pub struct WinIdentity { pub target_name: String, + pub username: String, } #[derive(Debug)] @@ -101,7 +102,11 @@ pub fn default_identity_mapper( label: format!("Password for service '{}', user '{}'", service, username), }), Platform::Windows => PlatformIdentity::Win(WinIdentity { + // Note: default concatenation of user and service name is + // needed because windows identity is on target_name only + // See issue here: https://github.com/jaraco/keyring/issues/47 target_name: format!("{}.{}", username, service), + username: username.to_string(), }), Platform::MacOs => PlatformIdentity::Mac(MacIdentity { service: service.to_string(), diff --git a/src/macos.rs b/src/macos.rs index 36fc259..2848d80 100644 --- a/src/macos.rs +++ b/src/macos.rs @@ -45,7 +45,6 @@ pub fn delete_password(map: &PlatformIdentity) -> Result<()> { } #[cfg(test)] -#[cfg(target_os = "macos")] mod test { use super::*; use crate::attrs::default_identity_mapper; diff --git a/src/windows.rs b/src/windows.rs index c5633e6..a19a20e 100644 --- a/src/windows.rs +++ b/src/windows.rs @@ -1,4 +1,3 @@ -use crate::error::{KeyringError, Result}; use byteorder::{ByteOrder, LittleEndian}; use std::ffi::OsStr; use std::iter::once; @@ -14,32 +13,17 @@ use winapi::um::wincred::{ CRED_TYPE_GENERIC, PCREDENTIALW, PCREDENTIAL_ATTRIBUTEW, }; +use crate::{KeyringError, PlatformIdentity, Result}; + // DWORD is u32 // LPCWSTR is *const u16 // BOOL is i32 (false = 0, true = 1) // PCREDENTIALW = *mut CREDENTIALW - -// Note: decision to concatenate user and service name -// to create target is because Windows assumes one user -// per service. See issue here: https://github.com/jaraco/keyring/issues/47 - -pub struct Keyring<'a> { - service: &'a str, - username: &'a str, -} - -impl<'a> Keyring<'a> { - pub fn new(service: &'a str, username: &'a str) -> Keyring<'a> { - Keyring { service, username } - } - - pub fn set_password(&self, password: &str) -> Result<()> { - // Setting values of credential - +pub fn set_password(map: &PlatformIdentity, password: &str) -> Result<()> { + if let PlatformIdentity::Win(map) = map { let flags = 0; let cred_type = CRED_TYPE_GENERIC; - let target_name: String = [self.username, self.service].join("."); - let mut target_name = to_wstr(&target_name); + let mut target_name = to_wstr(&map.target_name); // empty string for comments, and target alias, // I don't use here @@ -64,7 +48,7 @@ impl<'a> Keyring<'a> { let persist = CRED_PERSIST_ENTERPRISE; let attribute_count = 0; let attributes: PCREDENTIAL_ATTRIBUTEW = std::ptr::null_mut(); - let mut username = to_wstr(self.username); + let mut username = to_wstr(&map.username); let mut credential = CREDENTIALW { Flags: flags, @@ -88,23 +72,19 @@ impl<'a> Keyring<'a> { 0 => Err(KeyringError::WindowsVaultError), _ => Ok(()), } + } else { + Err(KeyringError::BadPlatformMapValue) } +} - pub fn get_password(&self) -> Result { - let target_name: String = [self.username, self.service].join("."); - self.get_password_for_target(&target_name) - } - - pub fn get_password_for_target(&self, target_name: &str) -> Result { +pub fn get_password(map: &PlatformIdentity) -> Result { + if let PlatformIdentity::Win(map) = map { // passing uninitialized pcredential. // Should be ok; it's freed by a windows api // call CredFree. let mut pcredential = MaybeUninit::uninit(); - - let target_name = to_wstr(target_name); - + let target_name = to_wstr(&map.target_name); let cred_type = CRED_TYPE_GENERIC; - // Windows api call match unsafe { CredReadW(target_name.as_ptr(), cred_type, 0, pcredential.as_mut_ptr()) } { 0 => unsafe { @@ -118,12 +98,10 @@ impl<'a> Keyring<'a> { let pcredential = unsafe { pcredential.assume_init() }; // Dereferencing pointer to credential let credential: CREDENTIALW = unsafe { *pcredential }; - // get blob by creating an array from the pointer // and the length reported back from the credential let blob_pointer: *const u8 = credential.CredentialBlob; let blob_len: usize = credential.CredentialBlobSize as usize; - // blob needs to be transformed from bytes to an // array of u16, which will then be transformed into // a utf8 string. As noted above, this is to allow @@ -132,27 +110,25 @@ impl<'a> Keyring<'a> { let blob: &[u8] = unsafe { slice::from_raw_parts(blob_pointer, blob_len) }; let mut blob_u16 = vec![0; blob_len / 2]; LittleEndian::read_u16_into(blob, &mut blob_u16); - // Now can get utf8 string from the array let password = String::from_utf16(&blob_u16).map_err(|_| KeyringError::WindowsVaultError); - // Free the credential unsafe { CredFree(pcredential as *mut _); } - password } } + } else { + Err(KeyringError::BadPlatformMapValue) } +} - pub fn delete_password(&self) -> Result<()> { - let target_name: String = [self.username, self.service].join("."); - +pub fn delete_password(map: &PlatformIdentity) -> Result<()> { + if let PlatformIdentity::Win(map) = map { let cred_type = CRED_TYPE_GENERIC; - let target_name = to_wstr(&target_name); - + let target_name = to_wstr(&map.target_name); match unsafe { CredDeleteW(target_name.as_ptr(), cred_type, 0) } { 0 => unsafe { match GetLastError() { @@ -163,63 +139,63 @@ impl<'a> Keyring<'a> { }, _ => Ok(()), } + } else { + Err(KeyringError::BadPlatformMapValue) } } -// helper function for turning utf8 strings to windows -// utf16 +// helper function for turning utf8 strings to windows utf16 fn to_wstr(s: &str) -> Vec { OsStr::new(s).encode_wide().chain(once(0)).collect() } - fn to_wstr_no_null(s: &str) -> Vec { OsStr::new(s).encode_wide().collect() } #[cfg(test)] -#[cfg(target_os = "windows")] mod test { use super::*; + use crate::attrs::default_identity_mapper; + use crate::Platform; #[test] fn test_basic() { let password_1 = "大根"; let password_2 = "0xE5A4A7E6A0B9"; // Above in hex string + let map = default_identity_mapper(Platform::MacOs, "test-service", "test-user"); - let keyring = Keyring::new("testservice", "testuser"); - - keyring.set_password(password_1).unwrap(); - let res_1 = keyring.get_password().unwrap(); + set_password(&map, password_1).unwrap(); + let response_1 = get_password(&map).unwrap(); assert_eq!( - res_1, password_1, + response_1, password_1, "Stored and retrieved passwords don't match" ); - keyring.set_password(password_2).unwrap(); - let res_2 = keyring.get_password().unwrap(); + set_password(&map, password_2).unwrap(); + let response_2 = get_password(&map).unwrap(); assert_eq!( - res_2, password_2, + response_2, password_2, "Stored and retrieved passwords don't match" ); - keyring.delete_password().unwrap(); + delete_password(&map).unwrap(); assert!( - keyring.get_password().is_err(), + get_password(&map).is_err(), "Able to read a deleted password" ) } #[test] fn test_no_password() { - let keyring = Keyring::new("testservice", "test-no-password"); - let result = keyring.get_password(); + let map = default_identity_mapper(Platform::Windows, "testservice", "test-no-password"); + let result = get_password(&map); match result { Ok(_) => panic!("expected KeyringError::NoPassword, got Ok"), Err(KeyringError::NoPasswordFound) => (), Err(e) => panic!("expected KeyringError::NoPassword, got {:}", e), } - let result = keyring.delete_password(); + let result = delete_password(&map); match result { Ok(_) => panic!("expected Err(KeyringError::NoPassword), got Ok()"), Err(KeyringError::NoPasswordFound) => (), From a393a3ced8d978c178acd930a398de40199da5a3 Mon Sep 17 00:00:00 2001 From: Daniel Brotsky Date: Wed, 10 Nov 2021 21:51:17 -0800 Subject: [PATCH 5/5] Fix typo in Windows tests. --- src/windows.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/windows.rs b/src/windows.rs index a19a20e..734fc3e 100644 --- a/src/windows.rs +++ b/src/windows.rs @@ -162,7 +162,7 @@ mod test { fn test_basic() { let password_1 = "大根"; let password_2 = "0xE5A4A7E6A0B9"; // Above in hex string - let map = default_identity_mapper(Platform::MacOs, "test-service", "test-user"); + let map = default_identity_mapper(Platform::Windows, "test-service", "test-user"); set_password(&map, password_1).unwrap(); let response_1 = get_password(&map).unwrap();