diff --git a/packages/secrets/src/keyring/src/os/mac/error.rs b/packages/secrets/src/keyring/src/os/mac/error.rs index 36910c51af..980527b003 100644 --- a/packages/secrets/src/keyring/src/os/mac/error.rs +++ b/packages/secrets/src/keyring/src/os/mac/error.rs @@ -22,6 +22,7 @@ impl Error { self.0.get() as _ } + /// Gets the message matching an OSStatus error code, if one exists. pub fn message(&self) -> Option { unsafe { let s = SecCopyErrorMessageString(self.code(), std::ptr::null_mut()); @@ -54,12 +55,12 @@ impl Display for Error { } } +/// Handles the OSStatus code from macOS FFI calls (error handling helper fn) #[inline(always)] pub fn handle_os_status(err: OSStatus) -> Result<(), Error> { match err { // errSecSuccess 0 => Ok(()), - // TODO: better error handling err => Err(Error::from_code(err)), } } diff --git a/packages/secrets/src/keyring/src/os/mac/ffi.rs b/packages/secrets/src/keyring/src/os/mac/ffi.rs index b5b221c68b..e76e92c040 100644 --- a/packages/secrets/src/keyring/src/os/mac/ffi.rs +++ b/packages/secrets/src/keyring/src/os/mac/ffi.rs @@ -6,7 +6,7 @@ use std::ffi::{c_char, c_void}; /// /// Keychain item reference types. /// -/// Source (lib/SecBase.h): +/// See lib/SecBase.h here: /// https://opensource.apple.com/source/libsecurity_keychain/libsecurity_keychain-55050.2/ /// pub enum OpaqueSecKeychainItemRef {} @@ -15,37 +15,30 @@ pub type SecKeychainItemRef = *mut OpaqueSecKeychainItemRef; pub type SecKeychainRef = *mut OpaqueSecKeychainRef; /// -/// Certificate item reference types. -/// -/// Source: +/// Certificate item reference types. /// https://developer.apple.com/documentation/security/opaqueseccertificateref /// pub enum OpaqueSecCertificateRef {} pub type SecCertificateRef = *mut OpaqueSecCertificateRef; /// -/// Identity reference types. -/// -/// Source: +/// Identity reference types. /// https://developer.apple.com/documentation/security/opaquesecidentityref /// pub enum OpaqueSecIdentityRef {} pub type SecIdentityRef = *mut OpaqueSecIdentityRef; /// -/// Key reference types. -/// -/// Source: +/// Key reference types. /// https://developer.apple.com/documentation/security/seckeyref /// pub enum OpaqueSecKeyRef {} pub type SecKeyRef = *mut OpaqueSecKeyRef; /// -/// Keychain attribute structure for searching items. -/// -/// Source: -/// https://developer.apple.com/documentation/security/seckeychainattribute +/// Keychain attribute structure for searching items. +/// https://developer.apple.com/documentation/security/seckeychainattribute, +/// https://developer.apple.com/documentation/security/seckeychainattributelist /// #[repr(C)] #[derive(Copy, Clone)] @@ -72,7 +65,7 @@ pub struct SecKeychainAttributeList { #[link(name = "Security", kind = "framework")] extern "C" { // keychain.rs: - pub fn SecKeychainCopyDefault(keychain: *mut SecKeychainRef) -> OSStatus; + pub fn SecCopyErrorMessageString(status: OSStatus, reserved: *mut c_void) -> CFStringRef; pub fn SecKeychainAddGenericPassword( keychain: SecKeychainRef, service_name_length: u32, @@ -83,6 +76,7 @@ extern "C" { password_data: *const c_void, item_ref: *mut SecKeychainItemRef, ) -> OSStatus; + pub fn SecKeychainCopyDefault(keychain: *mut SecKeychainRef) -> OSStatus; pub fn SecKeychainFindGenericPassword( keychain_or_array: CFTypeRef, service_name_len: u32, @@ -94,9 +88,9 @@ extern "C" { item_ref: *mut SecKeychainItemRef, ) -> OSStatus; pub fn SecKeychainGetTypeID() -> CFTypeID; - pub fn SecCopyErrorMessageString(status: OSStatus, reserved: *mut c_void) -> CFStringRef; // keychain_item.rs: + pub fn SecKeychainItemDelete(item_ref: SecKeychainItemRef) -> OSStatus; pub fn SecKeychainItemGetTypeID() -> CFTypeID; pub fn SecKeychainItemModifyAttributesAndData( item_ref: SecKeychainItemRef, @@ -104,18 +98,17 @@ extern "C" { length: u32, data: *const c_void, ) -> OSStatus; - pub fn SecKeychainItemDelete(item_ref: SecKeychainItemRef) -> OSStatus; // keychain_search.rs: pub fn SecItemCopyMatching(query: CFDictionaryRef, result: *mut CFTypeRef) -> OSStatus; - pub static kSecClass: CFStringRef; - pub static kSecClassGenericPassword: CFStringRef; pub static kSecAttrAccount: CFStringRef; pub static kSecAttrLabel: CFStringRef; pub static kSecAttrService: CFStringRef; + pub static kSecClass: CFStringRef; + pub static kSecClassGenericPassword: CFStringRef; pub static kSecMatchLimit: CFStringRef; - pub static kSecReturnData: CFStringRef; pub static kSecReturnAttributes: CFStringRef; + pub static kSecReturnData: CFStringRef; pub static kSecReturnRef: CFStringRef; // misc.rs: diff --git a/packages/secrets/src/keyring/src/os/mac/keychain.rs b/packages/secrets/src/keyring/src/os/mac/keychain.rs index 78112772ba..250b7c2583 100644 --- a/packages/secrets/src/keyring/src/os/mac/keychain.rs +++ b/packages/secrets/src/keyring/src/os/mac/keychain.rs @@ -46,6 +46,14 @@ impl SecKeychain { unsafe { Ok(Self::wrap_under_create_rule(keychain)) } } + /// + /// set_password + /// Attempts to set the password within the keychain for a given service and account. + /// + /// Returns: + /// - Nothing if the password was set successfully, or + /// - An `Error` object if an error was encountered + /// pub fn set_password(&self, service: &str, account: &str, password: &[u8]) -> Result<(), Error> { match self.find_password(service, account) { Ok((_, mut item)) => item.set_password(password), @@ -63,6 +71,15 @@ impl SecKeychain { }, } } + + /// + /// find_password + /// Attempts to find a password within the keychain matching a given service and account. + /// + /// Returns: + /// - A pair containing the KeychainItem object with its password data if the password was found, or + /// - An `Error` object if an error was encountered + /// pub fn find_password( &self, service: &str, diff --git a/packages/secrets/src/keyring/src/os/mac/keychain_item.rs b/packages/secrets/src/keyring/src/os/mac/keychain_item.rs index a93e406142..8008e2781f 100644 --- a/packages/secrets/src/keyring/src/os/mac/keychain_item.rs +++ b/packages/secrets/src/keyring/src/os/mac/keychain_item.rs @@ -5,7 +5,6 @@ use crate::os::mac::ffi::{ }; use core_foundation::base::TCFType; use core_foundation::{declare_TCFType, impl_TCFType}; -use core_foundation_sys::base::OSStatus; /* * SecKeychainItem: https://developer.apple.com/documentation/security/seckeychainitem @@ -21,11 +20,27 @@ impl_TCFType! { } impl SecKeychainItem { + /// + /// delete + /// Attempts to delete this keychain item from the keychain. + /// + /// Returns: + /// - Nothing if the deletion request was successful, or + /// - An `Error` object if an error was encountered + /// #[inline] - pub fn delete(self) -> OSStatus { - unsafe { SecKeychainItemDelete(self.as_CFTypeRef() as *mut _) } + pub fn delete(self) -> Result<(), Error> { + unsafe { handle_os_status(SecKeychainItemDelete(self.as_CFTypeRef() as *mut _)) } } + /// + /// set_password + /// Attempts to set the password for this keychain item. + /// + /// Returns: + /// - Nothing if the password was set successfully, or + /// - An `Error` object if an error was encountered + /// pub fn set_password(&mut self, password: &[u8]) -> Result<(), Error> { unsafe { handle_os_status(SecKeychainItemModifyAttributesAndData( diff --git a/packages/secrets/src/keyring/src/os/mac/keychain_search.rs b/packages/secrets/src/keyring/src/os/mac/keychain_search.rs index aee72b7b47..8de998dfbe 100644 --- a/packages/secrets/src/keyring/src/os/mac/keychain_search.rs +++ b/packages/secrets/src/keyring/src/os/mac/keychain_search.rs @@ -43,10 +43,15 @@ pub enum SearchResult { } impl SearchResult { - /// Returns `Some(hash_map)` containing the attribute keys/values - /// if the result is of the `Dict` variant, or `None` otherwise. + /// + /// parse_dict + /// Tries to parse a CFDictionary object into a hashmap of string pairs. + /// + /// Returns: + /// - `Some(hash_map)` containing the attribute keys/values if parsed successfully + /// - `None` otherwise #[must_use] - pub fn simplify_dict(&self) -> Option> { + pub fn parse_dict(&self) -> Option> { match *self { Self::Dict(ref d) => unsafe { // build map of attributes to return for this search result @@ -86,7 +91,8 @@ impl SearchResult { /// get_item /// /// item: The item reference to convert to a SearchResult -/// Returns: a SearchResult enum variant based on the item reference provided. +/// Returns: +/// - a SearchResult enum variant based on the item reference provided. /// unsafe fn get_item(item: CFTypeRef) -> SearchResult { let type_id = CFGetTypeID(item); @@ -156,8 +162,9 @@ impl KeychainSearch { /// Executes a search within the keychain, factoring in the set search options. /// - /// Returns: If successful, a `Vec` containing a list of search results; - /// an `Error` otherwise. + /// Returns: + /// - If successful, a `Vec` containing a list of search results + /// - an `Error` object otherwise pub fn execute(&self) -> Result, Error> { let mut params = vec![]; diff --git a/packages/secrets/src/keyring/src/os/mac/mod.rs b/packages/secrets/src/keyring/src/os/mac/mod.rs index 8d3e23b40d..3ff763774c 100644 --- a/packages/secrets/src/keyring/src/os/mac/mod.rs +++ b/packages/secrets/src/keyring/src/os/mac/mod.rs @@ -10,13 +10,13 @@ mod misc; use error::Error; use crate::os::mac::error::ERR_SEC_ITEM_NOT_FOUND; -use crate::os::mac::keychain_search::KeychainSearch; +use crate::os::mac::keychain_search::{KeychainSearch, SearchResult}; use keychain::SecKeychain; impl From for KeyringError { fn from(error: Error) -> Self { KeyringError::Library { - name: "security_framework".to_owned(), + name: "macOS Security.framework".to_owned(), details: format!("{:?}", error.message()), } } @@ -106,7 +106,7 @@ pub fn delete_password(service: &String, account: &String) -> Result { - item.delete(); + item.delete()?; return Ok(true); } Err(err) if err.code() == ERR_SEC_ITEM_NOT_FOUND => Ok(false), @@ -136,15 +136,23 @@ pub fn find_credentials( .execute() { Ok(search_results) => { - for result in search_results { - if let Some(result_map) = result.simplify_dict() { - credentials.push(( - result_map.get("acct").unwrap().to_owned(), - result_map.get("v_Data").unwrap().to_owned(), - )) - } - } - return Ok(!credentials.is_empty()); + *credentials = search_results + .iter() + .filter_map(|result| match result { + SearchResult::Dict(_) => { + return match result.parse_dict() { + Some(attrs) => Some(( + attrs.get("acct").unwrap().to_owned(), + attrs.get("v_Data").unwrap().to_owned(), + )), + None => None, + }; + } + _ => None, + }) + .collect(); + + Ok(!credentials.is_empty()) } Err(err) if err.code() == ERR_SEC_ITEM_NOT_FOUND => Ok(false), Err(err) => Err(KeyringError::from(err)),