From f491a1672d43b0f407224d37399c487643f91d9f Mon Sep 17 00:00:00 2001 From: Trae Yelovich Date: Mon, 18 Sep 2023 15:31:10 -0400 Subject: [PATCH 01/17] wip(secrets/mac): use core-foundation instead of security-framework Signed-off-by: Trae Yelovich --- packages/secrets/src/keyring/Cargo.toml | 3 +- .../src/keyring/src/os/mac/keychain.rs | 42 +++++++++++++++++++ 2 files changed, 44 insertions(+), 1 deletion(-) create mode 100644 packages/secrets/src/keyring/src/os/mac/keychain.rs diff --git a/packages/secrets/src/keyring/Cargo.toml b/packages/secrets/src/keyring/Cargo.toml index ba973e38ef..8e712cb52b 100644 --- a/packages/secrets/src/keyring/Cargo.toml +++ b/packages/secrets/src/keyring/Cargo.toml @@ -27,7 +27,8 @@ features = [ version = "0.48.0" [target.'cfg(target_os = "macos")'.dependencies] -security-framework = "2.9.1" +core-foundation = "0.9.3" +cvt = "0.1.2" [target.'cfg(any(target_os = "freebsd", target_os = "linux"))'.dependencies] glib = "0.17.10" diff --git a/packages/secrets/src/keyring/src/os/mac/keychain.rs b/packages/secrets/src/keyring/src/os/mac/keychain.rs new file mode 100644 index 0000000000..6943282b00 --- /dev/null +++ b/packages/secrets/src/keyring/src/os/mac/keychain.rs @@ -0,0 +1,42 @@ +use core_foundation::base::TCFType; + +declare_TCFType! { + Keychain +} + +impl Keychain { + pub unsafe fn default() -> Result { + let mut keychain = std::ptr::null_mut(); + cvt(SecKeychainCopyDefault(&mut keychain))?; + Ok(Self::wrap_under_create_rule(keychain)) + } + + pub unsafe fn set_password(&self, service: &str, account: &str, password: &[u8]) -> Result<()> { + cvt(SecKeychainAddGenericPassword( + self.as_CFTypeRef() as *mut _, + service.len() as u32, + service.as_ptr().cast(), + account.len() as u32, + account.as_ptr().cast(), + password.len() as u32, + password.as_ptr().cast(), + std::ptr::null_mut() + )) + } +} + +pub enum OpaqueSecKeychainRef {} +pub type SecKeychainRef = *mut OpaqueSecKeychainRef; +extern "C" { + pub fn SecKeychainCopyDefault(keychain: *mut SecKeychainRef) -> OSStatus; + pub fn SecKeychainAddGenericPassword( + keychain: SecKeychainRef, + serviceNameLength: u32, + serviceName: *const c_char, + accountNameLength: u32, + accountName: *const c_char, + passwordLength: u32, + passwordData: *const c_void, + itemRef: *mut SecKeychainItemRef, + ) -> OSStatus; +} \ No newline at end of file From 13b92ae9ccdd743698d2d67232238e288b2f1727 Mon Sep 17 00:00:00 2001 From: Trae Yelovich Date: Tue, 19 Sep 2023 11:29:09 -0400 Subject: [PATCH 02/17] feat(secrets): core-foundation progress (set, get, delete, findPassword) Signed-off-by: Trae Yelovich --- packages/secrets/src/keyring/Cargo.lock | 36 ++-- packages/secrets/src/keyring/Cargo.toml | 1 + .../src/keyring/src/os/mac/keychain.rs | 184 +++++++++++++++--- .../src/keyring/src/os/mac/keychain_item.rs | 66 +++++++ .../src/keyring/src/os/{mac.rs => mac/mod.rs} | 36 ++-- 5 files changed, 254 insertions(+), 69 deletions(-) create mode 100644 packages/secrets/src/keyring/src/os/mac/keychain_item.rs rename packages/secrets/src/keyring/src/os/{mac.rs => mac/mod.rs} (83%) diff --git a/packages/secrets/src/keyring/Cargo.lock b/packages/secrets/src/keyring/Cargo.lock index 9894558af8..603444d7af 100644 --- a/packages/secrets/src/keyring/Cargo.lock +++ b/packages/secrets/src/keyring/Cargo.lock @@ -86,6 +86,15 @@ dependencies = [ "syn 2.0.28", ] +[[package]] +name = "cvt" +version = "0.1.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d2ae9bf77fbf2d39ef573205d554d87e86c12f1994e9ea335b0651b9b278bcf1" +dependencies = [ + "cfg-if", +] + [[package]] name = "equivalent" version = "1.0.1" @@ -274,6 +283,9 @@ name = "keyring" version = "1.0.0" dependencies = [ "cfg-if", + "core-foundation", + "core-foundation-sys", + "cvt", "gio", "glib", "libsecret", @@ -281,7 +293,6 @@ dependencies = [ "napi", "napi-build", "napi-derive", - "security-framework", "thiserror", "windows-sys", ] @@ -498,29 +509,6 @@ version = "0.7.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "e5ea92a5b6195c6ef2a0295ea818b312502c6fc94dde986c5553242e18fd4ce2" -[[package]] -name = "security-framework" -version = "2.9.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "05b64fb303737d99b81884b2c63433e9ae28abebe5eb5045dcdd175dc2ecf4de" -dependencies = [ - "bitflags 1.3.2", - "core-foundation", - "core-foundation-sys", - "libc", - "security-framework-sys", -] - -[[package]] -name = "security-framework-sys" -version = "2.9.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e932934257d3b408ed8f30db49d85ea163bfe74961f017f405b025af298f0c7a" -dependencies = [ - "core-foundation-sys", - "libc", -] - [[package]] name = "semver" version = "1.0.18" diff --git a/packages/secrets/src/keyring/Cargo.toml b/packages/secrets/src/keyring/Cargo.toml index 8e712cb52b..0789180aba 100644 --- a/packages/secrets/src/keyring/Cargo.toml +++ b/packages/secrets/src/keyring/Cargo.toml @@ -28,6 +28,7 @@ version = "0.48.0" [target.'cfg(target_os = "macos")'.dependencies] core-foundation = "0.9.3" +core-foundation-sys = "0.8.4" cvt = "0.1.2" [target.'cfg(any(target_os = "freebsd", target_os = "linux"))'.dependencies] diff --git a/packages/secrets/src/keyring/src/os/mac/keychain.rs b/packages/secrets/src/keyring/src/os/mac/keychain.rs index 6943282b00..f772b10d49 100644 --- a/packages/secrets/src/keyring/src/os/mac/keychain.rs +++ b/packages/secrets/src/keyring/src/os/mac/keychain.rs @@ -1,42 +1,172 @@ -use core_foundation::base::TCFType; +use std::ffi::{c_char, c_void}; +use std::fmt::{Debug, Display, Formatter}; +use std::num::NonZeroI32; +use std::ops::Deref; +use core_foundation_sys::base::{CFTypeID, CFTypeRef, OSStatus}; +use core_foundation::{impl_TCFType, base::TCFType, declare_TCFType}; +use core_foundation::string::CFString; +use core_foundation_sys::string::CFStringRef; +use crate::os::mac::keychain_item::{KeychainItem, KeychainItemRef}; + +#[derive(Copy, Clone)] +pub struct Error(NonZeroI32); + +impl Error { + #[inline] + #[must_use] + pub fn from_code(code: OSStatus) -> Self { + Self(NonZeroI32::new(code).unwrap_or_else(|| NonZeroI32::new(1).unwrap())) + } + + pub fn code(self) -> i32 { + self.0.get() as _ + } + + pub fn message(&self) -> Option { + unsafe { + let s = SecCopyErrorMessageString(self.code(), std::ptr::null_mut()); + if s.is_null() { + None + } else { + Some(CFString::wrap_under_create_rule(s).to_string()) + } + } + } +} + +impl Debug for Error { + fn fmt(&self, fmt: &mut Formatter<'_>) -> std::fmt::Result { + let mut builder = fmt.debug_struct("Error"); + builder.field("code", &self.0); + if let Some(message) = self.message() { + builder.field("message", &message); + } + builder.finish() + } +} + +impl Display for Error { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + match self.message() { + Some(msg) => write!(f, "{}", msg), + None => write!(f, "code: {}", self.code()) + } + } +} + +pub enum OpaqueSecKeychainRef {} + +pub type KeychainRef = *mut OpaqueSecKeychainRef; declare_TCFType! { - Keychain + Keychain, KeychainRef +} + +impl_TCFType!(Keychain, KeychainRef, SecKeychainGetTypeID); + +#[inline(always)] +pub fn cvt(err: OSStatus) -> Result<(), Error> { + match err { + // errSecSuccess + 0 => Ok(()), + // TODO: better error handling + err => Err(Error::from_code(err)) + } +} + +pub struct KeychainItemPassword { + pub data: *const u8, + pub data_len: usize, +} + +impl AsRef<[u8]> for KeychainItemPassword { + #[inline] + fn as_ref(&self) -> &[u8] { + unsafe { std::slice::from_raw_parts(self.data, self.data_len) } + } +} + +impl Deref for KeychainItemPassword { + type Target = [u8]; + #[inline] + fn deref(&self) -> &Self::Target { + self.as_ref() + } } impl Keychain { - pub unsafe fn default() -> Result { + pub fn default() -> Result { let mut keychain = std::ptr::null_mut(); - cvt(SecKeychainCopyDefault(&mut keychain))?; - Ok(Self::wrap_under_create_rule(keychain)) + unsafe { cvt(SecKeychainCopyDefault(&mut keychain))?; } + unsafe { Ok(Self::wrap_under_create_rule(keychain)) } + } + + 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), + _ => unsafe { cvt(SecKeychainAddGenericPassword( + self.as_CFTypeRef() as *mut _, + service.len() as u32, + service.as_ptr().cast(), + account.len() as u32, + account.as_ptr().cast(), + password.len() as u32, + password.as_ptr().cast(), + std::ptr::null_mut(), + )) } + } } + pub fn find_password(&self, service: &str, account: &str) -> Result<(KeychainItemPassword, KeychainItem), Error> { + let keychain_ref = self.as_CFTypeRef(); + + let mut len = 0; + let mut data = std::ptr::null_mut(); + let mut item = std::ptr::null_mut(); - pub unsafe fn set_password(&self, service: &str, account: &str, password: &[u8]) -> Result<()> { - cvt(SecKeychainAddGenericPassword( - self.as_CFTypeRef() as *mut _, - service.len() as u32, - service.as_ptr().cast(), - account.len() as u32, - account.as_ptr().cast(), - password.len() as u32, - password.as_ptr().cast(), - std::ptr::null_mut() - )) + unsafe { + cvt(SecKeychainFindGenericPassword( + keychain_ref, + service.len() as u32, + service.as_ptr().cast(), + account.len() as u32, + account.as_ptr().cast(), + &mut len, + &mut data, + &mut item, + ))?; + Ok(( + KeychainItemPassword { + data: data as *const _, + data_len: len as usize, + }, + KeychainItem::wrap_under_create_rule(item) + )) + } } } -pub enum OpaqueSecKeychainRef {} -pub type SecKeychainRef = *mut OpaqueSecKeychainRef; extern "C" { - pub fn SecKeychainCopyDefault(keychain: *mut SecKeychainRef) -> OSStatus; + pub fn SecKeychainCopyDefault(keychain: *mut KeychainRef) -> OSStatus; pub fn SecKeychainAddGenericPassword( - keychain: SecKeychainRef, - serviceNameLength: u32, - serviceName: *const c_char, - accountNameLength: u32, - accountName: *const c_char, - passwordLength: u32, - passwordData: *const c_void, - itemRef: *mut SecKeychainItemRef, + keychain: KeychainRef, + service_name_length: u32, + service_name: *const c_char, + account_name_length: u32, + account_name: *const c_char, + password_length: u32, + password_data: *const c_void, + item_ref: *mut KeychainItemRef, + ) -> OSStatus; + pub fn SecKeychainFindGenericPassword( + keychain_or_array: CFTypeRef, + service_name_len: u32, + service_name: *const c_char, + account_name_len: u32, + account_name: *const c_char, + password_len: *mut u32, + password: *mut *mut c_void, + item_ref: *mut KeychainItemRef, ) -> OSStatus; + pub fn SecKeychainGetTypeID() -> CFTypeID; + pub fn SecCopyErrorMessageString(status: OSStatus, reserved: *mut c_void) -> CFStringRef; } \ No newline at end of file diff --git a/packages/secrets/src/keyring/src/os/mac/keychain_item.rs b/packages/secrets/src/keyring/src/os/mac/keychain_item.rs new file mode 100644 index 0000000000..b0fd0312db --- /dev/null +++ b/packages/secrets/src/keyring/src/os/mac/keychain_item.rs @@ -0,0 +1,66 @@ +use std::ffi::c_void; +use std::fmt::Error; +use core_foundation::{declare_TCFType, impl_TCFType}; +use core_foundation::base::TCFType; +use core_foundation_sys::base::{CFTypeID, OSStatus}; +use crate::os::mac::keychain; +use crate::os::mac::keychain::cvt; + +pub enum OpaqueSecKeychainItemRef {} +pub type KeychainItemRef = *mut OpaqueSecKeychainItemRef; + +declare_TCFType! { + KeychainItem, KeychainItemRef +} + +impl_TCFType! { + KeychainItem, + KeychainItemRef, + SecKeychainItemGetTypeID +} + +impl KeychainItem { + #[inline] + pub fn delete(self) -> OSStatus { + unsafe { SecKeychainItemDelete(self.as_CFTypeRef() as *mut _) } + } + + pub fn set_password(&mut self, password: &[u8]) -> Result<(), keychain::Error> { + unsafe { + cvt(SecKeychainItemModifyAttributesAndData( + self.as_CFTypeRef() as *mut _, + std::ptr::null(), + password.len() as u32, + password.as_ptr().cast() + ))?; + } + + Ok(()) + } +} + +#[repr(C)] +#[derive(Copy, Clone)] +pub struct KeychainAttribute { + pub tag: u32, + pub length: u32, + pub data: *mut c_void +} + +#[repr(C)] +#[derive(Copy, Clone)] +pub struct KeychainAttributeList { + pub count: u32, + pub attr: *mut KeychainAttribute +} + +extern "C" { + pub fn SecKeychainItemGetTypeID() -> CFTypeID; + pub fn SecKeychainItemModifyAttributesAndData( + item_ref: KeychainItemRef, + attr_list: *const KeychainAttributeList, + length: u32, + data: *const c_void + ) -> OSStatus; + pub fn SecKeychainItemDelete(item_ref: KeychainItemRef) -> OSStatus; +} \ No newline at end of file diff --git a/packages/secrets/src/keyring/src/os/mac.rs b/packages/secrets/src/keyring/src/os/mac/mod.rs similarity index 83% rename from packages/secrets/src/keyring/src/os/mac.rs rename to packages/secrets/src/keyring/src/os/mac/mod.rs index 8497ecd696..c913c39dbd 100644 --- a/packages/secrets/src/keyring/src/os/mac.rs +++ b/packages/secrets/src/keyring/src/os/mac/mod.rs @@ -1,18 +1,16 @@ -extern crate security_framework; use super::error::KeyringError; -use security_framework::{ - item::{ItemClass, ItemSearchOptions}, - os::macos::keychain::SecKeychain, -}; +mod keychain; +mod keychain_item; +use keychain::Keychain; const ERR_SEC_ITEM_NOT_FOUND: i32 = -25300; -impl From for KeyringError { - fn from(error: security_framework::base::Error) -> Self { +impl From for KeyringError { + fn from(error: keychain::Error) -> Self { KeyringError::Library { name: "security_framework".to_owned(), - details: format!("{:?}", error), + details: format!("{:?}", error.message()), } } } @@ -32,8 +30,8 @@ pub fn set_password( account: &String, password: &mut String, ) -> Result { - let keychain = SecKeychain::default().unwrap(); - match keychain.set_generic_password(service.as_str(), account.as_str(), password.as_bytes()) { + let keychain = Keychain::default().unwrap(); + match keychain.set_password(service.as_str(), account.as_str(), password.as_bytes()) { Ok(()) => Ok(true), Err(err) => Err(KeyringError::from(err)), } @@ -50,8 +48,8 @@ pub fn set_password( /// - A `KeyringError` if there were any issues interacting with the credential vault /// pub fn get_password(service: &String, account: &String) -> Result, KeyringError> { - let keychain = SecKeychain::default().unwrap(); - match keychain.find_generic_password(service.as_str(), account.as_str()) { + let keychain = Keychain::default().unwrap(); + match keychain.find_password(service.as_str(), account.as_str()) { Ok((pw, _)) => Ok(Some(String::from_utf8(pw.to_owned())?)), Err(err) if err.code() == ERR_SEC_ITEM_NOT_FOUND => Ok(None), Err(err) => Err(KeyringError::from(err)), @@ -77,8 +75,8 @@ pub fn find_password(service: &String) -> Result, KeyringError> { }); } - let keychain = SecKeychain::default().unwrap(); - match keychain.find_generic_password(cred_attrs[0], cred_attrs[1]) { + let keychain = Keychain::default().unwrap(); + match keychain.find_password(cred_attrs[0], cred_attrs[1]) { Ok((pw, _)) => { let pw_str = String::from_utf8(pw.to_owned())?; return Ok(Some(pw_str)); @@ -98,8 +96,8 @@ pub fn find_password(service: &String) -> Result, KeyringError> { /// - A `KeyringError` if there were any issues interacting with the credential vault /// pub fn delete_password(service: &String, account: &String) -> Result { - let keychain = SecKeychain::default().unwrap(); - match keychain.find_generic_password(service.as_str(), account.as_str()) { + let keychain = Keychain::default().unwrap(); + match keychain.find_password(service.as_str(), account.as_str()) { Ok((_, item)) => { item.delete(); return Ok(true); @@ -123,7 +121,9 @@ pub fn find_credentials( service: &String, credentials: &mut Vec<(String, String)>, ) -> Result { - match ItemSearchOptions::new() + // TODO: implement ItemSearchOptions + Ok(false) + /*match ItemSearchOptions::new() .class(ItemClass::generic_password()) .label(service.as_str()) .limit(i32::MAX as i64) @@ -145,5 +145,5 @@ pub fn find_credentials( } Err(err) if err.code() == ERR_SEC_ITEM_NOT_FOUND => Ok(false), Err(err) => Err(KeyringError::from(err)), - } + }*/ } From ecdc0be1732ed67999bbe64d8a75593fc78af60e Mon Sep 17 00:00:00 2001 From: Trae Yelovich Date: Tue, 19 Sep 2023 15:22:31 -0400 Subject: [PATCH 03/17] chore(secrets): separate ffi logic in macOS code Signed-off-by: Trae Yelovich --- .../secrets/src/keyring/src/os/mac/error.rs | 62 ++++++++++ .../secrets/src/keyring/src/os/mac/ffi.rs | 53 +++++++++ .../src/keyring/src/os/mac/keychain.rs | 106 ++---------------- .../src/keyring/src/os/mac/keychain_item.rs | 25 +---- .../secrets/src/keyring/src/os/mac/mod.rs | 47 ++++---- 5 files changed, 156 insertions(+), 137 deletions(-) create mode 100644 packages/secrets/src/keyring/src/os/mac/error.rs create mode 100644 packages/secrets/src/keyring/src/os/mac/ffi.rs diff --git a/packages/secrets/src/keyring/src/os/mac/error.rs b/packages/secrets/src/keyring/src/os/mac/error.rs new file mode 100644 index 0000000000..06d544ebf7 --- /dev/null +++ b/packages/secrets/src/keyring/src/os/mac/error.rs @@ -0,0 +1,62 @@ +use std::fmt::{Debug, Display, Formatter}; +use std::num::NonZeroI32; +use core_foundation::base::TCFType; +use core_foundation::string::CFString; +use core_foundation_sys::base::OSStatus; +use crate::os::mac::ffi::SecCopyErrorMessageString; + +#[derive(Copy, Clone)] +pub struct Error(NonZeroI32); + +impl Error { + #[inline] + #[must_use] + pub fn from_code(code: OSStatus) -> Self { + Self(NonZeroI32::new(code).unwrap_or_else(|| NonZeroI32::new(1).unwrap())) + } + + pub fn code(self) -> i32 { + self.0.get() as _ + } + + pub fn message(&self) -> Option { + unsafe { + let s = SecCopyErrorMessageString(self.code(), std::ptr::null_mut()); + if s.is_null() { + None + } else { + Some(CFString::wrap_under_create_rule(s).to_string()) + } + } + } +} + +impl Debug for Error { + fn fmt(&self, fmt: &mut Formatter<'_>) -> std::fmt::Result { + let mut builder = fmt.debug_struct("Error"); + builder.field("code", &self.0); + if let Some(message) = self.message() { + builder.field("message", &message); + } + builder.finish() + } +} + +impl Display for Error { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + match self.message() { + Some(msg) => write!(f, "{}", msg), + None => write!(f, "code: {}", self.code()) + } + } +} + +#[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 new file mode 100644 index 0000000000..903dcb9a4a --- /dev/null +++ b/packages/secrets/src/keyring/src/os/mac/ffi.rs @@ -0,0 +1,53 @@ +use std::ffi::{c_char, c_void}; +use core_foundation_sys::base::{CFTypeID, CFTypeRef, OSStatus}; +use core_foundation_sys::string::CFStringRef; + +pub enum OpaqueSecKeychainItemRef {} + +pub enum OpaqueSecKeychainRef {} + +pub type KeychainItemRef = *mut OpaqueSecKeychainItemRef; +pub type KeychainRef = *mut OpaqueSecKeychainRef; + +/// +/// Defined below are the C functions that the Rust logic uses +/// to interact with macOS's Security.framework. +/// +/// Since we can call C functions directly from Rust, we just need to define the +/// fn prototypes ahead of time - Rust will link to the proper C symbols during compile time. +/// +extern "C" { + // used in keychain.rs: + pub fn SecKeychainCopyDefault(keychain: *mut KeychainRef) -> OSStatus; + pub fn SecKeychainAddGenericPassword( + keychain: KeychainRef, + service_name_length: u32, + service_name: *const c_char, + account_name_length: u32, + account_name: *const c_char, + password_length: u32, + password_data: *const c_void, + item_ref: *mut KeychainItemRef, + ) -> OSStatus; + pub fn SecKeychainFindGenericPassword( + keychain_or_array: CFTypeRef, + service_name_len: u32, + service_name: *const c_char, + account_name_len: u32, + account_name: *const c_char, + password_len: *mut u32, + password: *mut *mut c_void, + item_ref: *mut KeychainItemRef, + ) -> OSStatus; + pub fn SecKeychainGetTypeID() -> CFTypeID; + pub fn SecCopyErrorMessageString(status: OSStatus, reserved: *mut c_void) -> CFStringRef; + // used in keychain_item.rs: + pub fn SecKeychainItemGetTypeID() -> CFTypeID; + pub fn SecKeychainItemModifyAttributesAndData( + item_ref: KeychainItemRef, + attr_list: *const KeychainAttributeList, + length: u32, + data: *const c_void, + ) -> OSStatus; + pub fn SecKeychainItemDelete(item_ref: KeychainItemRef) -> OSStatus; +} \ No newline at end of file diff --git a/packages/secrets/src/keyring/src/os/mac/keychain.rs b/packages/secrets/src/keyring/src/os/mac/keychain.rs index f772b10d49..a1b82e359d 100644 --- a/packages/secrets/src/keyring/src/os/mac/keychain.rs +++ b/packages/secrets/src/keyring/src/os/mac/keychain.rs @@ -1,62 +1,10 @@ -use std::ffi::{c_char, c_void}; -use std::fmt::{Debug, Display, Formatter}; -use std::num::NonZeroI32; +use std::fmt::{Debug, Display}; use std::ops::Deref; -use core_foundation_sys::base::{CFTypeID, CFTypeRef, OSStatus}; use core_foundation::{impl_TCFType, base::TCFType, declare_TCFType}; -use core_foundation::string::CFString; -use core_foundation_sys::string::CFStringRef; -use crate::os::mac::keychain_item::{KeychainItem, KeychainItemRef}; +use crate::os::mac::error::{Error, handle_os_status}; +use crate::os::mac::keychain_item::{KeychainItem}; +use crate::os::mac::ffi::{KeychainRef, SecKeychainCopyDefault, SecKeychainAddGenericPassword, SecKeychainFindGenericPassword}; -#[derive(Copy, Clone)] -pub struct Error(NonZeroI32); - -impl Error { - #[inline] - #[must_use] - pub fn from_code(code: OSStatus) -> Self { - Self(NonZeroI32::new(code).unwrap_or_else(|| NonZeroI32::new(1).unwrap())) - } - - pub fn code(self) -> i32 { - self.0.get() as _ - } - - pub fn message(&self) -> Option { - unsafe { - let s = SecCopyErrorMessageString(self.code(), std::ptr::null_mut()); - if s.is_null() { - None - } else { - Some(CFString::wrap_under_create_rule(s).to_string()) - } - } - } -} - -impl Debug for Error { - fn fmt(&self, fmt: &mut Formatter<'_>) -> std::fmt::Result { - let mut builder = fmt.debug_struct("Error"); - builder.field("code", &self.0); - if let Some(message) = self.message() { - builder.field("message", &message); - } - builder.finish() - } -} - -impl Display for Error { - fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { - match self.message() { - Some(msg) => write!(f, "{}", msg), - None => write!(f, "code: {}", self.code()) - } - } -} - -pub enum OpaqueSecKeychainRef {} - -pub type KeychainRef = *mut OpaqueSecKeychainRef; declare_TCFType! { Keychain, KeychainRef @@ -64,16 +12,6 @@ declare_TCFType! { impl_TCFType!(Keychain, KeychainRef, SecKeychainGetTypeID); -#[inline(always)] -pub fn cvt(err: OSStatus) -> Result<(), Error> { - match err { - // errSecSuccess - 0 => Ok(()), - // TODO: better error handling - err => Err(Error::from_code(err)) - } -} - pub struct KeychainItemPassword { pub data: *const u8, pub data_len: usize, @@ -97,14 +35,15 @@ impl Deref for KeychainItemPassword { impl Keychain { pub fn default() -> Result { let mut keychain = std::ptr::null_mut(); - unsafe { cvt(SecKeychainCopyDefault(&mut keychain))?; } + unsafe { handle_os_status(SecKeychainCopyDefault(&mut keychain))?; } unsafe { Ok(Self::wrap_under_create_rule(keychain)) } } 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), - _ => unsafe { cvt(SecKeychainAddGenericPassword( + _ => unsafe { + handle_os_status(SecKeychainAddGenericPassword( self.as_CFTypeRef() as *mut _, service.len() as u32, service.as_ptr().cast(), @@ -113,7 +52,8 @@ impl Keychain { password.len() as u32, password.as_ptr().cast(), std::ptr::null_mut(), - )) } + )) + } } } pub fn find_password(&self, service: &str, account: &str) -> Result<(KeychainItemPassword, KeychainItem), Error> { @@ -124,7 +64,7 @@ impl Keychain { let mut item = std::ptr::null_mut(); unsafe { - cvt(SecKeychainFindGenericPassword( + handle_os_status(SecKeychainFindGenericPassword( keychain_ref, service.len() as u32, service.as_ptr().cast(), @@ -143,30 +83,4 @@ impl Keychain { )) } } -} - -extern "C" { - pub fn SecKeychainCopyDefault(keychain: *mut KeychainRef) -> OSStatus; - pub fn SecKeychainAddGenericPassword( - keychain: KeychainRef, - service_name_length: u32, - service_name: *const c_char, - account_name_length: u32, - account_name: *const c_char, - password_length: u32, - password_data: *const c_void, - item_ref: *mut KeychainItemRef, - ) -> OSStatus; - pub fn SecKeychainFindGenericPassword( - keychain_or_array: CFTypeRef, - service_name_len: u32, - service_name: *const c_char, - account_name_len: u32, - account_name: *const c_char, - password_len: *mut u32, - password: *mut *mut c_void, - item_ref: *mut KeychainItemRef, - ) -> OSStatus; - pub fn SecKeychainGetTypeID() -> CFTypeID; - pub fn SecCopyErrorMessageString(status: OSStatus, reserved: *mut c_void) -> CFStringRef; } \ No newline at end of file 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 b0fd0312db..0726830bdd 100644 --- a/packages/secrets/src/keyring/src/os/mac/keychain_item.rs +++ b/packages/secrets/src/keyring/src/os/mac/keychain_item.rs @@ -1,13 +1,9 @@ use std::ffi::c_void; -use std::fmt::Error; use core_foundation::{declare_TCFType, impl_TCFType}; use core_foundation::base::TCFType; -use core_foundation_sys::base::{CFTypeID, OSStatus}; -use crate::os::mac::keychain; -use crate::os::mac::keychain::cvt; - -pub enum OpaqueSecKeychainItemRef {} -pub type KeychainItemRef = *mut OpaqueSecKeychainItemRef; +use core_foundation_sys::base::OSStatus; +use crate::os::mac::ffi::{SecKeychainItemDelete, SecKeychainItemModifyAttributesAndData}; +use crate::os::mac::error::{Error, handle_os_status}; declare_TCFType! { KeychainItem, KeychainItemRef @@ -25,9 +21,9 @@ impl KeychainItem { unsafe { SecKeychainItemDelete(self.as_CFTypeRef() as *mut _) } } - pub fn set_password(&mut self, password: &[u8]) -> Result<(), keychain::Error> { + pub fn set_password(&mut self, password: &[u8]) -> Result<(), Error> { unsafe { - cvt(SecKeychainItemModifyAttributesAndData( + handle_os_status(SecKeychainItemModifyAttributesAndData( self.as_CFTypeRef() as *mut _, std::ptr::null(), password.len() as u32, @@ -52,15 +48,4 @@ pub struct KeychainAttribute { pub struct KeychainAttributeList { pub count: u32, pub attr: *mut KeychainAttribute -} - -extern "C" { - pub fn SecKeychainItemGetTypeID() -> CFTypeID; - pub fn SecKeychainItemModifyAttributesAndData( - item_ref: KeychainItemRef, - attr_list: *const KeychainAttributeList, - length: u32, - data: *const c_void - ) -> OSStatus; - pub fn SecKeychainItemDelete(item_ref: KeychainItemRef) -> OSStatus; } \ No newline at end of file diff --git a/packages/secrets/src/keyring/src/os/mac/mod.rs b/packages/secrets/src/keyring/src/os/mac/mod.rs index c913c39dbd..dd846322f8 100644 --- a/packages/secrets/src/keyring/src/os/mac/mod.rs +++ b/packages/secrets/src/keyring/src/os/mac/mod.rs @@ -2,12 +2,17 @@ use super::error::KeyringError; mod keychain; mod keychain_item; +mod ffi; +mod error; + +use error::Error; use keychain::Keychain; + const ERR_SEC_ITEM_NOT_FOUND: i32 = -25300; -impl From for KeyringError { - fn from(error: keychain::Error) -> Self { +impl From for KeyringError { + fn from(error: Error) -> Self { KeyringError::Library { name: "security_framework".to_owned(), details: format!("{:?}", error.message()), @@ -15,12 +20,12 @@ impl From for KeyringError { } } -/// +/// /// Attempts to set a password for a given service and account. -/// +/// /// - `service`: The service name for the new credential /// - `account`: The account name for the new credential -/// +/// /// Returns: /// - `true` if the credential was stored successfully /// - A `KeyringError` if there were any issues interacting with the credential vault @@ -37,16 +42,16 @@ pub fn set_password( } } -/// +/// /// Returns a password contained in the given service and account, if found. -/// +/// /// - `service`: The service name that matches the credential of interest /// - `account`: The account name that matches the credential of interest -/// +/// /// Returns: /// - `Some(password)` if a matching credential was found; `None` otherwise /// - A `KeyringError` if there were any issues interacting with the credential vault -/// +/// pub fn get_password(service: &String, account: &String) -> Result, KeyringError> { let keychain = Keychain::default().unwrap(); match keychain.find_password(service.as_str(), account.as_str()) { @@ -56,15 +61,15 @@ pub fn get_password(service: &String, account: &String) -> Result } } -/// +/// /// Returns the first password (if any) that matches the given service pattern. -/// +/// /// - `service`: The service pattern that matches the credential of interest -/// +/// /// Returns: /// - `Some(password)` if a matching credential was found; `None` otherwise /// - A `KeyringError` if there were any issues interacting with the credential vault -/// +/// pub fn find_password(service: &String) -> Result, KeyringError> { let cred_attrs: Vec<&str> = service.split("/").collect(); if cred_attrs.len() < 2 { @@ -85,16 +90,16 @@ pub fn find_password(service: &String) -> Result, KeyringError> { } } -/// +/// /// Attempts to delete the password associated with a given service and account. -/// +/// /// - `service`: The service name of the credential to delete /// - `account`: The account name of the credential to delete -/// +/// /// Returns: /// - `true` if a matching credential was deleted; `false` otherwise /// - A `KeyringError` if there were any issues interacting with the credential vault -/// +/// pub fn delete_password(service: &String, account: &String) -> Result { let keychain = Keychain::default().unwrap(); match keychain.find_password(service.as_str(), account.as_str()) { @@ -107,16 +112,16 @@ pub fn delete_password(service: &String, account: &String) -> Result, From 7801b01571c63bd5dfa0cce773ad38d9d4c3625c Mon Sep 17 00:00:00 2001 From: Trae Yelovich Date: Tue, 19 Sep 2023 16:34:07 -0400 Subject: [PATCH 04/17] fix(secrets): macOS errors during build Signed-off-by: Trae Yelovich --- .../secrets/src/keyring/src/os/mac/ffi.rs | 15 +++++++++++++++ .../src/keyring/src/os/mac/keychain.rs | 2 +- .../src/keyring/src/os/mac/keychain_item.rs | 19 ++----------------- 3 files changed, 18 insertions(+), 18 deletions(-) diff --git a/packages/secrets/src/keyring/src/os/mac/ffi.rs b/packages/secrets/src/keyring/src/os/mac/ffi.rs index 903dcb9a4a..15da58213b 100644 --- a/packages/secrets/src/keyring/src/os/mac/ffi.rs +++ b/packages/secrets/src/keyring/src/os/mac/ffi.rs @@ -9,6 +9,21 @@ pub enum OpaqueSecKeychainRef {} pub type KeychainItemRef = *mut OpaqueSecKeychainItemRef; pub type KeychainRef = *mut OpaqueSecKeychainRef; +#[repr(C)] +#[derive(Copy, Clone)] +pub struct KeychainAttribute { + pub tag: u32, + pub length: u32, + pub data: *mut c_void +} + +#[repr(C)] +#[derive(Copy, Clone)] +pub struct KeychainAttributeList { + pub count: u32, + pub attr: *mut KeychainAttribute +} + /// /// Defined below are the C functions that the Rust logic uses /// to interact with macOS's Security.framework. diff --git a/packages/secrets/src/keyring/src/os/mac/keychain.rs b/packages/secrets/src/keyring/src/os/mac/keychain.rs index a1b82e359d..ad9625ae85 100644 --- a/packages/secrets/src/keyring/src/os/mac/keychain.rs +++ b/packages/secrets/src/keyring/src/os/mac/keychain.rs @@ -3,7 +3,7 @@ use std::ops::Deref; use core_foundation::{impl_TCFType, base::TCFType, declare_TCFType}; use crate::os::mac::error::{Error, handle_os_status}; use crate::os::mac::keychain_item::{KeychainItem}; -use crate::os::mac::ffi::{KeychainRef, SecKeychainCopyDefault, SecKeychainAddGenericPassword, SecKeychainFindGenericPassword}; +use crate::os::mac::ffi::{KeychainRef, SecKeychainCopyDefault, SecKeychainAddGenericPassword, SecKeychainFindGenericPassword, SecKeychainGetTypeID}; declare_TCFType! { 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 0726830bdd..62257ff621 100644 --- a/packages/secrets/src/keyring/src/os/mac/keychain_item.rs +++ b/packages/secrets/src/keyring/src/os/mac/keychain_item.rs @@ -2,7 +2,7 @@ use std::ffi::c_void; use core_foundation::{declare_TCFType, impl_TCFType}; use core_foundation::base::TCFType; use core_foundation_sys::base::OSStatus; -use crate::os::mac::ffi::{SecKeychainItemDelete, SecKeychainItemModifyAttributesAndData}; +use crate::os::mac::ffi::{KeychainItemRef, SecKeychainItemDelete, SecKeychainItemGetTypeID, SecKeychainItemModifyAttributesAndData}; use crate::os::mac::error::{Error, handle_os_status}; declare_TCFType! { @@ -27,25 +27,10 @@ impl KeychainItem { self.as_CFTypeRef() as *mut _, std::ptr::null(), password.len() as u32, - password.as_ptr().cast() + password.as_ptr().cast(), ))?; } Ok(()) } -} - -#[repr(C)] -#[derive(Copy, Clone)] -pub struct KeychainAttribute { - pub tag: u32, - pub length: u32, - pub data: *mut c_void -} - -#[repr(C)] -#[derive(Copy, Clone)] -pub struct KeychainAttributeList { - pub count: u32, - pub attr: *mut KeychainAttribute } \ No newline at end of file From b50095dda8e787c368efe26332ade17a6ddc1741 Mon Sep 17 00:00:00 2001 From: Trae Yelovich Date: Wed, 20 Sep 2023 15:28:19 -0400 Subject: [PATCH 05/17] feat(secrets): implement KeychainSearch for findCredentials Signed-off-by: Trae Yelovich --- .../secrets/src/keyring/src/os/mac/ffi.rs | 19 ++ .../src/keyring/src/os/mac/keychain_search.rs | 190 ++++++++++++++++++ .../secrets/src/keyring/src/os/mac/misc.rs | 43 ++++ .../secrets/src/keyring/src/os/mac/mod.rs | 20 +- 4 files changed, 261 insertions(+), 11 deletions(-) create mode 100644 packages/secrets/src/keyring/src/os/mac/keychain_search.rs create mode 100644 packages/secrets/src/keyring/src/os/mac/misc.rs diff --git a/packages/secrets/src/keyring/src/os/mac/ffi.rs b/packages/secrets/src/keyring/src/os/mac/ffi.rs index 15da58213b..98e5f2fec7 100644 --- a/packages/secrets/src/keyring/src/os/mac/ffi.rs +++ b/packages/secrets/src/keyring/src/os/mac/ffi.rs @@ -1,5 +1,6 @@ use std::ffi::{c_char, c_void}; use core_foundation_sys::base::{CFTypeID, CFTypeRef, OSStatus}; +use core_foundation_sys::dictionary::CFDictionaryRef; use core_foundation_sys::string::CFStringRef; pub enum OpaqueSecKeychainItemRef {} @@ -56,6 +57,7 @@ extern "C" { ) -> OSStatus; pub fn SecKeychainGetTypeID() -> CFTypeID; pub fn SecCopyErrorMessageString(status: OSStatus, reserved: *mut c_void) -> CFStringRef; + // used in keychain_item.rs: pub fn SecKeychainItemGetTypeID() -> CFTypeID; pub fn SecKeychainItemModifyAttributesAndData( @@ -65,4 +67,21 @@ extern "C" { data: *const c_void, ) -> OSStatus; pub fn SecKeychainItemDelete(item_ref: KeychainItemRef) -> OSStatus; + + // used in 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 kSecMatchLimit: CFStringRef; + pub static kSecReturnData: CFStringRef; + pub static kSecReturnAttributes: CFStringRef; + pub static kSecReturnRef: CFStringRef; + + // used in misc.rs: + pub fn SecCertificateGetTypeID() -> CFTypeID; + pub fn SecIdentityGetTypeID() -> CFTypeID; + pub fn SecKeyGetTypeID() -> CFTypeID; } \ No newline at end of file diff --git a/packages/secrets/src/keyring/src/os/mac/keychain_search.rs b/packages/secrets/src/keyring/src/os/mac/keychain_search.rs new file mode 100644 index 0000000000..f742ff672b --- /dev/null +++ b/packages/secrets/src/keyring/src/os/mac/keychain_search.rs @@ -0,0 +1,190 @@ +use std::collections::HashMap; +use core_foundation::array::CFArray; +use core_foundation::base::{CFType, TCFType}; +use core_foundation::boolean::CFBoolean; +use core_foundation::data::CFData; +use core_foundation::date::CFDate; +use core_foundation::dictionary::CFDictionary; +use core_foundation::number::CFNumber; +use core_foundation::string::CFString; +use core_foundation_sys::base::{CFCopyDescription, CFGetTypeID, CFRelease, CFTypeRef}; +use crate::os::mac::error::{Error, handle_os_status}; +use crate::os::mac::ffi::{kSecAttrAccount, kSecAttrLabel, kSecAttrService, kSecClass, kSecClassGenericPassword, kSecMatchLimit, kSecReturnAttributes, kSecReturnData, kSecReturnRef, SecItemCopyMatching}; +use crate::os::mac::misc::{Certificate, Identity, Key}; +use crate::os::mac::keychain_item::KeychainItem; + +#[derive(Default)] +pub struct KeychainSearch { + label: Option, + service: Option, + account: Option, + load_attrs: bool, + load_data: bool, + load_refs: bool, +} + +pub enum Reference { + Identity(Identity), + Certificate(Certificate), + Key(Key), + KeychainItem(KeychainItem), +} + +pub enum SearchResult { + Ref(Reference), + Dict(CFDictionary), + Data(Vec), + Other, +} + +impl SearchResult { + #[must_use] + pub fn simplify_dict(&self) -> Option> { + match *self { + Self::Dict(ref d) => unsafe { + let mut retmap = HashMap::new(); + let (keys, values) = d.get_keys_and_values(); + for (k, v) in keys.iter().zip(values.iter()) { + let key_cfstr = CFString::wrap_under_get_rule((*k).cast()); + let val: String = match CFGetTypeID(*v) { + cfstring if cfstring == CFString::type_id() => { + format!("{}", CFString::wrap_under_get_rule((*v).cast())) + } + cfdata if cfdata == CFData::type_id() => { + let buf = CFData::wrap_under_get_rule((*v).cast()); + let mut vec = Vec::new(); + vec.extend_from_slice(buf.bytes()); + format!("{}", String::from_utf8_lossy(&vec)) + } + cfdate if cfdate == CFDate::type_id() => format!("{}", CFString::wrap_under_create_rule(CFCopyDescription(*v))), + _ => String::from("unknown") + }; + retmap.insert(format!("{}", key_cfstr), val); + } + Some(retmap) + } + _ => None + } + } +} + +unsafe fn get_item(item: CFTypeRef) -> SearchResult { + let type_id = CFGetTypeID(item); + if type_id == CFData::type_id() { + let data = CFData::wrap_under_get_rule(item as *mut _); + let mut buf = Vec::new(); + buf.extend_from_slice(data.bytes()); + return SearchResult::Data(buf); + } + + if type_id == CFDictionary::<*const u8, *const u8>::type_id() { + return SearchResult::Dict(CFDictionary::wrap_under_get_rule(item as *mut _)); + } + + if type_id == KeychainItem::type_id() { + return SearchResult::Ref(Reference::KeychainItem( + KeychainItem::wrap_under_get_rule(item as *mut _) + )); + } + + let reference = match type_id { + r if r == Certificate::type_id() => Reference::Certificate(Certificate::wrap_under_get_rule(item as *mut _)), + r if r == Key::type_id() => Reference::Key(Key::wrap_under_get_rule(item as *mut _)), + r if r == Identity::type_id() => Reference::Identity(Identity::wrap_under_get_rule(item as *mut _)), + _ => panic!("Bad type received from SecItemCopyMatching: {}", type_id) + }; + + SearchResult::Ref(reference) +} + +impl KeychainSearch { + #[inline(always)] + #[must_use] + pub fn new() -> Self { + Self::default() + } + + pub fn label(&mut self, label: &str) -> &mut Self { + self.label = Some(CFString::new(label)); + self + } + + pub fn with_attrs(&mut self) -> &mut Self { + self.load_attrs = true; + self + } + + pub fn with_data(&mut self) -> &mut Self { + self.load_data = true; + self + } + + pub fn with_refs(&mut self) -> &mut Self { + self.load_refs = true; + self + } + + pub fn execute(&self) -> Result, Error> { + let mut params = vec![]; + + unsafe { + params.push((CFString::wrap_under_get_rule(kSecClass), CFType::wrap_under_get_rule(kSecClassGenericPassword.cast()))); + + if let Some(ref label) = self.label { + params.push((CFString::wrap_under_get_rule(kSecAttrLabel), label.as_CFType())); + } + + if let Some(ref service) = self.service { + params.push((CFString::wrap_under_get_rule(kSecAttrService), service.as_CFType())); + } + + if let Some(ref acc) = self.account { + params.push((CFString::wrap_under_get_rule(kSecAttrAccount), acc.as_CFType())); + } + + if self.load_data { + params.push(( + CFString::wrap_under_get_rule(kSecReturnData), + CFBoolean::true_value().into_CFType() + )); + } + if self.load_attrs { + params.push(( + CFString::wrap_under_get_rule(kSecReturnAttributes), + CFBoolean::true_value().into_CFType() + )); + } + if self.load_refs { + params.push(( + CFString::wrap_under_get_rule(kSecReturnRef), + CFBoolean::true_value().into_CFType() + )); + } + + params.push((CFString::wrap_under_get_rule(kSecMatchLimit), CFNumber::from(i32::MAX).into_CFType())); + + let params = CFDictionary::from_CFType_pairs(¶ms); + let mut ret = std::ptr::null(); + + handle_os_status(SecItemCopyMatching(params.as_concrete_TypeRef(), &mut ret))?; + if ret.is_null() { + return Ok(vec![]); + } + let type_id = CFGetTypeID(ret); + + let mut items = vec![]; + if type_id == CFArray::::type_id() { + let array: CFArray = CFArray::wrap_under_create_rule(ret as *mut _); + for item in array.iter() { + items.push(get_item(item.as_CFTypeRef())); + } + } else { + items.push(get_item(ret)); + + CFRelease(ret); + } + + Ok(items) + } + } +} diff --git a/packages/secrets/src/keyring/src/os/mac/misc.rs b/packages/secrets/src/keyring/src/os/mac/misc.rs new file mode 100644 index 0000000000..5dd3b2d6a1 --- /dev/null +++ b/packages/secrets/src/keyring/src/os/mac/misc.rs @@ -0,0 +1,43 @@ +use core_foundation::{declare_TCFType, impl_TCFType}; +use crate::os::mac::ffi::{SecCertificateGetTypeID, SecIdentityGetTypeID, SecKeyGetTypeID}; +use core_foundation::base::TCFType; + +pub enum OpaqueSecIdentityRef {} + +pub type SecIdentityRef = *mut OpaqueSecIdentityRef; + +declare_TCFType!( + Identity, + SecIdentityRef +); +impl_TCFType!( + Identity, + SecIdentityRef, + SecIdentityGetTypeID +); + +pub enum OpaqueSecCertificateRef {} +pub type SecCertificateRef = *mut OpaqueSecCertificateRef; + +declare_TCFType!( + Certificate, + SecCertificateRef +); +impl_TCFType!( + Certificate, + SecCertificateRef, + SecCertificateGetTypeID +); + +pub enum OpaqueSecKeyRef {} +pub type SecKeyRef = *mut OpaqueSecKeyRef; + +declare_TCFType!( + Key, + SecKeyRef +); +impl_TCFType!( + Key, + SecKeyRef, + SecKeyGetTypeID +); \ No newline at end of file diff --git a/packages/secrets/src/keyring/src/os/mac/mod.rs b/packages/secrets/src/keyring/src/os/mac/mod.rs index dd846322f8..bfa80b3a14 100644 --- a/packages/secrets/src/keyring/src/os/mac/mod.rs +++ b/packages/secrets/src/keyring/src/os/mac/mod.rs @@ -4,10 +4,13 @@ mod keychain; mod keychain_item; mod ffi; mod error; +mod keychain_search; +mod misc; use error::Error; use keychain::Keychain; +use crate::os::mac::keychain_search::KeychainSearch; const ERR_SEC_ITEM_NOT_FOUND: i32 = -25300; @@ -126,17 +129,12 @@ pub fn find_credentials( service: &String, credentials: &mut Vec<(String, String)>, ) -> Result { - // TODO: implement ItemSearchOptions - Ok(false) - /*match ItemSearchOptions::new() - .class(ItemClass::generic_password()) + match KeychainSearch::new() .label(service.as_str()) - .limit(i32::MAX as i64) - .load_attributes(true) - .load_data(true) - .load_refs(true) - .search() - { + .with_attrs() + .with_data() + .with_refs() + .execute() { Ok(search_results) => { for result in search_results { if let Some(result_map) = result.simplify_dict() { @@ -150,5 +148,5 @@ pub fn find_credentials( } Err(err) if err.code() == ERR_SEC_ITEM_NOT_FOUND => Ok(false), Err(err) => Err(KeyringError::from(err)), - }*/ + } } From 1db47b098c6e7fcfde85b7aa86df05000c1fdb78 Mon Sep 17 00:00:00 2001 From: Trae Yelovich Date: Thu, 21 Sep 2023 11:01:54 -0400 Subject: [PATCH 06/17] test: adjust SecCertificateGetTypeID fn name to resolve ci errors Signed-off-by: Trae Yelovich --- packages/secrets/src/keyring/Cargo.lock | 10 ---------- packages/secrets/src/keyring/Cargo.toml | 1 - packages/secrets/src/keyring/src/os/mac/ffi.rs | 1 + packages/secrets/src/keyring/src/os/mac/misc.rs | 4 ++-- 4 files changed, 3 insertions(+), 13 deletions(-) diff --git a/packages/secrets/src/keyring/Cargo.lock b/packages/secrets/src/keyring/Cargo.lock index 603444d7af..3ec0ec60e7 100644 --- a/packages/secrets/src/keyring/Cargo.lock +++ b/packages/secrets/src/keyring/Cargo.lock @@ -86,15 +86,6 @@ dependencies = [ "syn 2.0.28", ] -[[package]] -name = "cvt" -version = "0.1.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d2ae9bf77fbf2d39ef573205d554d87e86c12f1994e9ea335b0651b9b278bcf1" -dependencies = [ - "cfg-if", -] - [[package]] name = "equivalent" version = "1.0.1" @@ -285,7 +276,6 @@ dependencies = [ "cfg-if", "core-foundation", "core-foundation-sys", - "cvt", "gio", "glib", "libsecret", diff --git a/packages/secrets/src/keyring/Cargo.toml b/packages/secrets/src/keyring/Cargo.toml index 0789180aba..0d6dd44ab5 100644 --- a/packages/secrets/src/keyring/Cargo.toml +++ b/packages/secrets/src/keyring/Cargo.toml @@ -29,7 +29,6 @@ version = "0.48.0" [target.'cfg(target_os = "macos")'.dependencies] core-foundation = "0.9.3" core-foundation-sys = "0.8.4" -cvt = "0.1.2" [target.'cfg(any(target_os = "freebsd", target_os = "linux"))'.dependencies] glib = "0.17.10" diff --git a/packages/secrets/src/keyring/src/os/mac/ffi.rs b/packages/secrets/src/keyring/src/os/mac/ffi.rs index 98e5f2fec7..3d76ecdac0 100644 --- a/packages/secrets/src/keyring/src/os/mac/ffi.rs +++ b/packages/secrets/src/keyring/src/os/mac/ffi.rs @@ -82,6 +82,7 @@ extern "C" { // used in misc.rs: pub fn SecCertificateGetTypeID() -> CFTypeID; + pub fn _SecCertificateGetTypeID() -> CFTypeID; pub fn SecIdentityGetTypeID() -> CFTypeID; pub fn SecKeyGetTypeID() -> CFTypeID; } \ No newline at end of file diff --git a/packages/secrets/src/keyring/src/os/mac/misc.rs b/packages/secrets/src/keyring/src/os/mac/misc.rs index 5dd3b2d6a1..f116614da5 100644 --- a/packages/secrets/src/keyring/src/os/mac/misc.rs +++ b/packages/secrets/src/keyring/src/os/mac/misc.rs @@ -1,5 +1,5 @@ use core_foundation::{declare_TCFType, impl_TCFType}; -use crate::os::mac::ffi::{SecCertificateGetTypeID, SecIdentityGetTypeID, SecKeyGetTypeID}; +use crate::os::mac::ffi::{_SecCertificateGetTypeID, SecIdentityGetTypeID, SecKeyGetTypeID}; use core_foundation::base::TCFType; pub enum OpaqueSecIdentityRef {} @@ -26,7 +26,7 @@ declare_TCFType!( impl_TCFType!( Certificate, SecCertificateRef, - SecCertificateGetTypeID + _SecCertificateGetTypeID ); pub enum OpaqueSecKeyRef {} From c7c4107fc3d94931554c6f5c60d100df54741206 Mon Sep 17 00:00:00 2001 From: Trae Yelovich Date: Thu, 21 Sep 2023 11:19:08 -0400 Subject: [PATCH 07/17] fix(secrets): link macOS FFI functions to Security.framework Signed-off-by: Trae Yelovich --- packages/secrets/src/keyring/src/os/mac/ffi.rs | 3 ++- packages/secrets/src/keyring/src/os/mac/misc.rs | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/secrets/src/keyring/src/os/mac/ffi.rs b/packages/secrets/src/keyring/src/os/mac/ffi.rs index 3d76ecdac0..bfb1f34055 100644 --- a/packages/secrets/src/keyring/src/os/mac/ffi.rs +++ b/packages/secrets/src/keyring/src/os/mac/ffi.rs @@ -32,6 +32,8 @@ pub struct KeychainAttributeList { /// Since we can call C functions directly from Rust, we just need to define the /// fn prototypes ahead of time - Rust will link to the proper C symbols during compile time. /// +/// +#[link(name = "Security", kind = "framework")] extern "C" { // used in keychain.rs: pub fn SecKeychainCopyDefault(keychain: *mut KeychainRef) -> OSStatus; @@ -82,7 +84,6 @@ extern "C" { // used in misc.rs: pub fn SecCertificateGetTypeID() -> CFTypeID; - pub fn _SecCertificateGetTypeID() -> CFTypeID; pub fn SecIdentityGetTypeID() -> CFTypeID; pub fn SecKeyGetTypeID() -> CFTypeID; } \ No newline at end of file diff --git a/packages/secrets/src/keyring/src/os/mac/misc.rs b/packages/secrets/src/keyring/src/os/mac/misc.rs index f116614da5..5dd3b2d6a1 100644 --- a/packages/secrets/src/keyring/src/os/mac/misc.rs +++ b/packages/secrets/src/keyring/src/os/mac/misc.rs @@ -1,5 +1,5 @@ use core_foundation::{declare_TCFType, impl_TCFType}; -use crate::os::mac::ffi::{_SecCertificateGetTypeID, SecIdentityGetTypeID, SecKeyGetTypeID}; +use crate::os::mac::ffi::{SecCertificateGetTypeID, SecIdentityGetTypeID, SecKeyGetTypeID}; use core_foundation::base::TCFType; pub enum OpaqueSecIdentityRef {} @@ -26,7 +26,7 @@ declare_TCFType!( impl_TCFType!( Certificate, SecCertificateRef, - _SecCertificateGetTypeID + SecCertificateGetTypeID ); pub enum OpaqueSecKeyRef {} From 588321e955c76841ea1541e9939cab7a0a8658f7 Mon Sep 17 00:00:00 2001 From: Trae Yelovich Date: Mon, 25 Sep 2023 10:08:02 -0400 Subject: [PATCH 08/17] chore(secrets/mac): Add doc comments, clean up Rust logic Signed-off-by: Trae Yelovich --- packages/secrets/src/keyring/src/os/error.rs | 1 + .../secrets/src/keyring/src/os/mac/error.rs | 13 +- .../secrets/src/keyring/src/os/mac/ffi.rs | 94 ++++++++++----- .../src/keyring/src/os/mac/keychain.rs | 41 ++++--- .../src/keyring/src/os/mac/keychain_item.rs | 25 ++-- .../src/keyring/src/os/mac/keychain_search.rs | 112 +++++++++++++----- .../secrets/src/keyring/src/os/mac/misc.rs | 56 +++------ .../secrets/src/keyring/src/os/mac/mod.rs | 22 ++-- 8 files changed, 225 insertions(+), 139 deletions(-) diff --git a/packages/secrets/src/keyring/src/os/error.rs b/packages/secrets/src/keyring/src/os/error.rs index 572ca2489d..7d7c251d32 100644 --- a/packages/secrets/src/keyring/src/os/error.rs +++ b/packages/secrets/src/keyring/src/os/error.rs @@ -11,6 +11,7 @@ pub enum KeyringError { #[error("[keyring] {name:?} library returned an error:\n\n{details:?}")] Library { name: String, details: String }, + #[cfg(not(target_os = "macos"))] #[error("[keyring] An OS error has occurred:\n\n{0}")] Os(String), diff --git a/packages/secrets/src/keyring/src/os/mac/error.rs b/packages/secrets/src/keyring/src/os/mac/error.rs index 06d544ebf7..36910c51af 100644 --- a/packages/secrets/src/keyring/src/os/mac/error.rs +++ b/packages/secrets/src/keyring/src/os/mac/error.rs @@ -1,13 +1,16 @@ -use std::fmt::{Debug, Display, Formatter}; -use std::num::NonZeroI32; +use crate::os::mac::ffi::SecCopyErrorMessageString; use core_foundation::base::TCFType; use core_foundation::string::CFString; use core_foundation_sys::base::OSStatus; -use crate::os::mac::ffi::SecCopyErrorMessageString; +use std::fmt::{Debug, Display, Formatter}; +use std::num::NonZeroI32; #[derive(Copy, Clone)] pub struct Error(NonZeroI32); +/// errSecItemNotFound +pub const ERR_SEC_ITEM_NOT_FOUND: i32 = -25300; + impl Error { #[inline] #[must_use] @@ -46,7 +49,7 @@ impl Display for Error { fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { match self.message() { Some(msg) => write!(f, "{}", msg), - None => write!(f, "code: {}", self.code()) + None => write!(f, "code: {}", self.code()), } } } @@ -57,6 +60,6 @@ pub fn handle_os_status(err: OSStatus) -> Result<(), Error> { // errSecSuccess 0 => Ok(()), // TODO: better error handling - err => Err(Error::from_code(err)) + 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 bfb1f34055..b5b221c68b 100644 --- a/packages/secrets/src/keyring/src/os/mac/ffi.rs +++ b/packages/secrets/src/keyring/src/os/mac/ffi.rs @@ -1,51 +1,87 @@ -use std::ffi::{c_char, c_void}; use core_foundation_sys::base::{CFTypeID, CFTypeRef, OSStatus}; use core_foundation_sys::dictionary::CFDictionaryRef; use core_foundation_sys::string::CFStringRef; +use std::ffi::{c_char, c_void}; +/// +/// Keychain item reference types. +/// +/// Source (lib/SecBase.h): +/// https://opensource.apple.com/source/libsecurity_keychain/libsecurity_keychain-55050.2/ +/// pub enum OpaqueSecKeychainItemRef {} - pub enum OpaqueSecKeychainRef {} +pub type SecKeychainItemRef = *mut OpaqueSecKeychainItemRef; +pub type SecKeychainRef = *mut OpaqueSecKeychainRef; + +/// +/// Certificate item reference types. +/// +/// Source: +/// https://developer.apple.com/documentation/security/opaqueseccertificateref +/// +pub enum OpaqueSecCertificateRef {} +pub type SecCertificateRef = *mut OpaqueSecCertificateRef; + +/// +/// Identity reference types. +/// +/// Source: +/// https://developer.apple.com/documentation/security/opaquesecidentityref +/// +pub enum OpaqueSecIdentityRef {} +pub type SecIdentityRef = *mut OpaqueSecIdentityRef; -pub type KeychainItemRef = *mut OpaqueSecKeychainItemRef; -pub type KeychainRef = *mut OpaqueSecKeychainRef; +/// +/// Key reference types. +/// +/// Source: +/// 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 +/// #[repr(C)] #[derive(Copy, Clone)] -pub struct KeychainAttribute { +pub struct SecKeychainAttribute { pub tag: u32, pub length: u32, - pub data: *mut c_void + pub data: *mut c_void, } - #[repr(C)] #[derive(Copy, Clone)] -pub struct KeychainAttributeList { +pub struct SecKeychainAttributeList { pub count: u32, - pub attr: *mut KeychainAttribute + pub attr: *mut SecKeychainAttribute, } -/// -/// Defined below are the C functions that the Rust logic uses -/// to interact with macOS's Security.framework. -/// -/// Since we can call C functions directly from Rust, we just need to define the -/// fn prototypes ahead of time - Rust will link to the proper C symbols during compile time. -/// -/// +/* + * Defined below are the C functions that the Rust logic + * uses to interact with macOS's Security.framework. + * + * Since we can call C functions directly using Rust FFI, we just need to define + * the function prototypes ahead of time, and link them to the Security.framework library. + * Rust will evaluate these symbols during compile time. + */ #[link(name = "Security", kind = "framework")] extern "C" { - // used in keychain.rs: - pub fn SecKeychainCopyDefault(keychain: *mut KeychainRef) -> OSStatus; + // keychain.rs: + pub fn SecKeychainCopyDefault(keychain: *mut SecKeychainRef) -> OSStatus; pub fn SecKeychainAddGenericPassword( - keychain: KeychainRef, + keychain: SecKeychainRef, service_name_length: u32, service_name: *const c_char, account_name_length: u32, account_name: *const c_char, password_length: u32, password_data: *const c_void, - item_ref: *mut KeychainItemRef, + item_ref: *mut SecKeychainItemRef, ) -> OSStatus; pub fn SecKeychainFindGenericPassword( keychain_or_array: CFTypeRef, @@ -55,22 +91,22 @@ extern "C" { account_name: *const c_char, password_len: *mut u32, password: *mut *mut c_void, - item_ref: *mut KeychainItemRef, + item_ref: *mut SecKeychainItemRef, ) -> OSStatus; pub fn SecKeychainGetTypeID() -> CFTypeID; pub fn SecCopyErrorMessageString(status: OSStatus, reserved: *mut c_void) -> CFStringRef; - // used in keychain_item.rs: + // keychain_item.rs: pub fn SecKeychainItemGetTypeID() -> CFTypeID; pub fn SecKeychainItemModifyAttributesAndData( - item_ref: KeychainItemRef, - attr_list: *const KeychainAttributeList, + item_ref: SecKeychainItemRef, + attr_list: *const SecKeychainAttributeList, length: u32, data: *const c_void, ) -> OSStatus; - pub fn SecKeychainItemDelete(item_ref: KeychainItemRef) -> OSStatus; + pub fn SecKeychainItemDelete(item_ref: SecKeychainItemRef) -> OSStatus; - // used in keychain_search.rs: + // keychain_search.rs: pub fn SecItemCopyMatching(query: CFDictionaryRef, result: *mut CFTypeRef) -> OSStatus; pub static kSecClass: CFStringRef; pub static kSecClassGenericPassword: CFStringRef; @@ -82,8 +118,8 @@ extern "C" { pub static kSecReturnAttributes: CFStringRef; pub static kSecReturnRef: CFStringRef; - // used in misc.rs: + // misc.rs: pub fn SecCertificateGetTypeID() -> CFTypeID; pub fn SecIdentityGetTypeID() -> CFTypeID; pub fn SecKeyGetTypeID() -> CFTypeID; -} \ No newline at end of file +} diff --git a/packages/secrets/src/keyring/src/os/mac/keychain.rs b/packages/secrets/src/keyring/src/os/mac/keychain.rs index ad9625ae85..78112772ba 100644 --- a/packages/secrets/src/keyring/src/os/mac/keychain.rs +++ b/packages/secrets/src/keyring/src/os/mac/keychain.rs @@ -1,17 +1,22 @@ -use std::fmt::{Debug, Display}; +use crate::os::mac::error::{handle_os_status, Error}; +use crate::os::mac::ffi::{ + SecKeychainAddGenericPassword, SecKeychainCopyDefault, SecKeychainFindGenericPassword, + SecKeychainGetTypeID, SecKeychainRef, +}; +use crate::os::mac::keychain_item::SecKeychainItem; +use core_foundation::{base::TCFType, declare_TCFType, impl_TCFType}; use std::ops::Deref; -use core_foundation::{impl_TCFType, base::TCFType, declare_TCFType}; -use crate::os::mac::error::{Error, handle_os_status}; -use crate::os::mac::keychain_item::{KeychainItem}; -use crate::os::mac::ffi::{KeychainRef, SecKeychainCopyDefault, SecKeychainAddGenericPassword, SecKeychainFindGenericPassword, SecKeychainGetTypeID}; - +/* + * SecKeychain: https://developer.apple.com/documentation/security/seckeychain + * SecKeychainRef: https://developer.apple.com/documentation/security/seckeychainref + */ declare_TCFType! { - Keychain, KeychainRef + SecKeychain, SecKeychainRef } +impl_TCFType!(SecKeychain, SecKeychainRef, SecKeychainGetTypeID); -impl_TCFType!(Keychain, KeychainRef, SecKeychainGetTypeID); - +/* Wrapper struct for handling passwords within SecKeychainItem objects. */ pub struct KeychainItemPassword { pub data: *const u8, pub data_len: usize, @@ -32,10 +37,12 @@ impl Deref for KeychainItemPassword { } } -impl Keychain { +impl SecKeychain { pub fn default() -> Result { let mut keychain = std::ptr::null_mut(); - unsafe { handle_os_status(SecKeychainCopyDefault(&mut keychain))?; } + unsafe { + handle_os_status(SecKeychainCopyDefault(&mut keychain))?; + } unsafe { Ok(Self::wrap_under_create_rule(keychain)) } } @@ -53,10 +60,14 @@ impl Keychain { password.as_ptr().cast(), std::ptr::null_mut(), )) - } + }, } } - pub fn find_password(&self, service: &str, account: &str) -> Result<(KeychainItemPassword, KeychainItem), Error> { + pub fn find_password( + &self, + service: &str, + account: &str, + ) -> Result<(KeychainItemPassword, SecKeychainItem), Error> { let keychain_ref = self.as_CFTypeRef(); let mut len = 0; @@ -79,8 +90,8 @@ impl Keychain { data: data as *const _, data_len: len as usize, }, - KeychainItem::wrap_under_create_rule(item) + SecKeychainItem::wrap_under_create_rule(item), )) } } -} \ No newline at end of file +} 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 62257ff621..a93e406142 100644 --- a/packages/secrets/src/keyring/src/os/mac/keychain_item.rs +++ b/packages/secrets/src/keyring/src/os/mac/keychain_item.rs @@ -1,21 +1,26 @@ -use std::ffi::c_void; -use core_foundation::{declare_TCFType, impl_TCFType}; +use crate::os::mac::error::{handle_os_status, Error}; +use crate::os::mac::ffi::{ + SecKeychainItemDelete, SecKeychainItemGetTypeID, SecKeychainItemModifyAttributesAndData, + SecKeychainItemRef, +}; use core_foundation::base::TCFType; +use core_foundation::{declare_TCFType, impl_TCFType}; use core_foundation_sys::base::OSStatus; -use crate::os::mac::ffi::{KeychainItemRef, SecKeychainItemDelete, SecKeychainItemGetTypeID, SecKeychainItemModifyAttributesAndData}; -use crate::os::mac::error::{Error, handle_os_status}; +/* + * SecKeychainItem: https://developer.apple.com/documentation/security/seckeychainitem + * SecKeychainItemRef: https://developer.apple.com/documentation/security/seckeychainitemref + */ declare_TCFType! { - KeychainItem, KeychainItemRef + SecKeychainItem, SecKeychainItemRef } - impl_TCFType! { - KeychainItem, - KeychainItemRef, + SecKeychainItem, + SecKeychainItemRef, SecKeychainItemGetTypeID } -impl KeychainItem { +impl SecKeychainItem { #[inline] pub fn delete(self) -> OSStatus { unsafe { SecKeychainItemDelete(self.as_CFTypeRef() as *mut _) } @@ -33,4 +38,4 @@ impl KeychainItem { Ok(()) } -} \ No newline at end of file +} 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 f742ff672b..aee72b7b47 100644 --- a/packages/secrets/src/keyring/src/os/mac/keychain_search.rs +++ b/packages/secrets/src/keyring/src/os/mac/keychain_search.rs @@ -1,4 +1,10 @@ -use std::collections::HashMap; +use crate::os::mac::error::{handle_os_status, Error}; +use crate::os::mac::ffi::{ + kSecAttrAccount, kSecAttrLabel, kSecAttrService, kSecClass, kSecClassGenericPassword, + kSecMatchLimit, kSecReturnAttributes, kSecReturnData, kSecReturnRef, SecItemCopyMatching, +}; +use crate::os::mac::keychain_item::SecKeychainItem; +use crate::os::mac::misc::{SecCertificate, SecIdentity, SecKey}; use core_foundation::array::CFArray; use core_foundation::base::{CFType, TCFType}; use core_foundation::boolean::CFBoolean; @@ -8,11 +14,9 @@ use core_foundation::dictionary::CFDictionary; use core_foundation::number::CFNumber; use core_foundation::string::CFString; use core_foundation_sys::base::{CFCopyDescription, CFGetTypeID, CFRelease, CFTypeRef}; -use crate::os::mac::error::{Error, handle_os_status}; -use crate::os::mac::ffi::{kSecAttrAccount, kSecAttrLabel, kSecAttrService, kSecClass, kSecClassGenericPassword, kSecMatchLimit, kSecReturnAttributes, kSecReturnData, kSecReturnRef, SecItemCopyMatching}; -use crate::os::mac::misc::{Certificate, Identity, Key}; -use crate::os::mac::keychain_item::KeychainItem; +use std::collections::HashMap; +/// Keychain Search structure to reference when making searches within the keychain. #[derive(Default)] pub struct KeychainSearch { label: Option, @@ -23,29 +27,36 @@ pub struct KeychainSearch { load_refs: bool, } +/// Reference enum for categorizing search results based on item type. pub enum Reference { - Identity(Identity), - Certificate(Certificate), - Key(Key), - KeychainItem(KeychainItem), + Identity(SecIdentity), + Certificate(SecCertificate), + Key(SecKey), + KeychainItem(SecKeychainItem), } +/// Enum for organizing types of items found during the keychain search operation. pub enum SearchResult { Ref(Reference), Dict(CFDictionary), Data(Vec), - Other, } impl SearchResult { + /// Returns `Some(hash_map)` containing the attribute keys/values + /// if the result is of the `Dict` variant, or `None` otherwise. #[must_use] pub fn simplify_dict(&self) -> Option> { match *self { Self::Dict(ref d) => unsafe { + // build map of attributes to return for this search result let mut retmap = HashMap::new(); let (keys, values) = d.get_keys_and_values(); for (k, v) in keys.iter().zip(values.iter()) { + // get key as CFString from pointer let key_cfstr = CFString::wrap_under_get_rule((*k).cast()); + + // get value based on CFType let val: String = match CFGetTypeID(*v) { cfstring if cfstring == CFString::type_id() => { format!("{}", CFString::wrap_under_get_rule((*v).cast())) @@ -56,20 +67,31 @@ impl SearchResult { vec.extend_from_slice(buf.bytes()); format!("{}", String::from_utf8_lossy(&vec)) } - cfdate if cfdate == CFDate::type_id() => format!("{}", CFString::wrap_under_create_rule(CFCopyDescription(*v))), - _ => String::from("unknown") + cfdate if cfdate == CFDate::type_id() => format!( + "{}", + CFString::wrap_under_create_rule(CFCopyDescription(*v)) + ), + _ => String::from("unknown"), }; retmap.insert(format!("{}", key_cfstr), val); } Some(retmap) - } - _ => None + }, + _ => None, } } } +/// +/// get_item +/// +/// item: The item reference to convert to a SearchResult +/// Returns: a SearchResult enum variant based on the item reference provided. +/// unsafe fn get_item(item: CFTypeRef) -> SearchResult { let type_id = CFGetTypeID(item); + + // if type is a raw buffer, return Vec of bytes based on item size if type_id == CFData::type_id() { let data = CFData::wrap_under_get_rule(item as *mut _); let mut buf = Vec::new(); @@ -77,21 +99,29 @@ unsafe fn get_item(item: CFTypeRef) -> SearchResult { return SearchResult::Data(buf); } + // if type is dictionary of items, cast as CFDictionary object if type_id == CFDictionary::<*const u8, *const u8>::type_id() { return SearchResult::Dict(CFDictionary::wrap_under_get_rule(item as *mut _)); } - if type_id == KeychainItem::type_id() { + // if type is a single Keychain item, return it as a reference + if type_id == SecKeychainItem::type_id() { return SearchResult::Ref(Reference::KeychainItem( - KeychainItem::wrap_under_get_rule(item as *mut _) + SecKeychainItem::wrap_under_get_rule(item as *mut _), )); } + // handle certificate, cryptographic key, and identity types as + // they can also appear in search results for the keychain let reference = match type_id { - r if r == Certificate::type_id() => Reference::Certificate(Certificate::wrap_under_get_rule(item as *mut _)), - r if r == Key::type_id() => Reference::Key(Key::wrap_under_get_rule(item as *mut _)), - r if r == Identity::type_id() => Reference::Identity(Identity::wrap_under_get_rule(item as *mut _)), - _ => panic!("Bad type received from SecItemCopyMatching: {}", type_id) + r if r == SecCertificate::type_id() => { + Reference::Certificate(SecCertificate::wrap_under_get_rule(item as *mut _)) + } + r if r == SecKey::type_id() => Reference::Key(SecKey::wrap_under_get_rule(item as *mut _)), + r if r == SecIdentity::type_id() => { + Reference::Identity(SecIdentity::wrap_under_get_rule(item as *mut _)) + } + _ => panic!("Bad type received from SecItemCopyMatching: {}", type_id), }; SearchResult::Ref(reference) @@ -124,54 +154,76 @@ impl KeychainSearch { self } + /// 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. pub fn execute(&self) -> Result, Error> { let mut params = vec![]; unsafe { - params.push((CFString::wrap_under_get_rule(kSecClass), CFType::wrap_under_get_rule(kSecClassGenericPassword.cast()))); + params.push(( + CFString::wrap_under_get_rule(kSecClass), + CFType::wrap_under_get_rule(kSecClassGenericPassword.cast()), + )); + // Handle any parameters that were configured before execution (label, service, account) if let Some(ref label) = self.label { - params.push((CFString::wrap_under_get_rule(kSecAttrLabel), label.as_CFType())); + params.push(( + CFString::wrap_under_get_rule(kSecAttrLabel), + label.as_CFType(), + )); } - if let Some(ref service) = self.service { - params.push((CFString::wrap_under_get_rule(kSecAttrService), service.as_CFType())); + params.push(( + CFString::wrap_under_get_rule(kSecAttrService), + service.as_CFType(), + )); } - if let Some(ref acc) = self.account { - params.push((CFString::wrap_under_get_rule(kSecAttrAccount), acc.as_CFType())); + params.push(( + CFString::wrap_under_get_rule(kSecAttrAccount), + acc.as_CFType(), + )); } + // Add params to fetch data, attributes, and/or refs if requested from search options if self.load_data { params.push(( CFString::wrap_under_get_rule(kSecReturnData), - CFBoolean::true_value().into_CFType() + CFBoolean::true_value().into_CFType(), )); } if self.load_attrs { params.push(( CFString::wrap_under_get_rule(kSecReturnAttributes), - CFBoolean::true_value().into_CFType() + CFBoolean::true_value().into_CFType(), )); } if self.load_refs { params.push(( CFString::wrap_under_get_rule(kSecReturnRef), - CFBoolean::true_value().into_CFType() + CFBoolean::true_value().into_CFType(), )); } - params.push((CFString::wrap_under_get_rule(kSecMatchLimit), CFNumber::from(i32::MAX).into_CFType())); + // Remove the default limit of 0 by requesting all items that match the search + params.push(( + CFString::wrap_under_get_rule(kSecMatchLimit), + CFNumber::from(i32::MAX).into_CFType(), + )); let params = CFDictionary::from_CFType_pairs(¶ms); let mut ret = std::ptr::null(); + // handle copy operation status and get type ID based on return value handle_os_status(SecItemCopyMatching(params.as_concrete_TypeRef(), &mut ret))?; if ret.is_null() { return Ok(vec![]); } let type_id = CFGetTypeID(ret); + // Build vector of items based on return reference type let mut items = vec![]; if type_id == CFArray::::type_id() { let array: CFArray = CFArray::wrap_under_create_rule(ret as *mut _); diff --git a/packages/secrets/src/keyring/src/os/mac/misc.rs b/packages/secrets/src/keyring/src/os/mac/misc.rs index 5dd3b2d6a1..1af69da4df 100644 --- a/packages/secrets/src/keyring/src/os/mac/misc.rs +++ b/packages/secrets/src/keyring/src/os/mac/misc.rs @@ -1,43 +1,21 @@ -use core_foundation::{declare_TCFType, impl_TCFType}; -use crate::os::mac::ffi::{SecCertificateGetTypeID, SecIdentityGetTypeID, SecKeyGetTypeID}; +use crate::os::mac::ffi::{ + SecCertificateGetTypeID, SecCertificateRef, SecIdentityGetTypeID, SecIdentityRef, + SecKeyGetTypeID, SecKeyRef, +}; use core_foundation::base::TCFType; +use core_foundation::{declare_TCFType, impl_TCFType}; -pub enum OpaqueSecIdentityRef {} - -pub type SecIdentityRef = *mut OpaqueSecIdentityRef; - -declare_TCFType!( - Identity, - SecIdentityRef -); -impl_TCFType!( - Identity, - SecIdentityRef, - SecIdentityGetTypeID -); - -pub enum OpaqueSecCertificateRef {} -pub type SecCertificateRef = *mut OpaqueSecCertificateRef; - -declare_TCFType!( - Certificate, - SecCertificateRef -); -impl_TCFType!( - Certificate, - SecCertificateRef, - SecCertificateGetTypeID -); +// Structure that represents identities within the keychain +// https://developer.apple.com/documentation/security/secidentity +declare_TCFType!(SecIdentity, SecIdentityRef); +impl_TCFType!(SecIdentity, SecIdentityRef, SecIdentityGetTypeID); -pub enum OpaqueSecKeyRef {} -pub type SecKeyRef = *mut OpaqueSecKeyRef; +// Structure that represents certificates within the keychain +// https://developer.apple.com/documentation/security/seccertificate +declare_TCFType!(SecCertificate, SecCertificateRef); +impl_TCFType!(SecCertificate, SecCertificateRef, SecCertificateGetTypeID); -declare_TCFType!( - Key, - SecKeyRef -); -impl_TCFType!( - Key, - SecKeyRef, - SecKeyGetTypeID -); \ No newline at end of file +// Structure that represents cryptographic keys within the keychain +// https://developer.apple.com/documentation/security/seckey +declare_TCFType!(SecKey, SecKeyRef); +impl_TCFType!(SecKey, SecKeyRef, SecKeyGetTypeID); diff --git a/packages/secrets/src/keyring/src/os/mac/mod.rs b/packages/secrets/src/keyring/src/os/mac/mod.rs index bfa80b3a14..8d3e23b40d 100644 --- a/packages/secrets/src/keyring/src/os/mac/mod.rs +++ b/packages/secrets/src/keyring/src/os/mac/mod.rs @@ -1,18 +1,17 @@ use super::error::KeyringError; +mod error; +mod ffi; mod keychain; mod keychain_item; -mod ffi; -mod error; mod keychain_search; mod misc; use error::Error; -use keychain::Keychain; +use crate::os::mac::error::ERR_SEC_ITEM_NOT_FOUND; use crate::os::mac::keychain_search::KeychainSearch; - -const ERR_SEC_ITEM_NOT_FOUND: i32 = -25300; +use keychain::SecKeychain; impl From for KeyringError { fn from(error: Error) -> Self { @@ -38,7 +37,7 @@ pub fn set_password( account: &String, password: &mut String, ) -> Result { - let keychain = Keychain::default().unwrap(); + let keychain = SecKeychain::default().unwrap(); match keychain.set_password(service.as_str(), account.as_str(), password.as_bytes()) { Ok(()) => Ok(true), Err(err) => Err(KeyringError::from(err)), @@ -56,7 +55,7 @@ pub fn set_password( /// - A `KeyringError` if there were any issues interacting with the credential vault /// pub fn get_password(service: &String, account: &String) -> Result, KeyringError> { - let keychain = Keychain::default().unwrap(); + let keychain = SecKeychain::default().unwrap(); match keychain.find_password(service.as_str(), account.as_str()) { Ok((pw, _)) => Ok(Some(String::from_utf8(pw.to_owned())?)), Err(err) if err.code() == ERR_SEC_ITEM_NOT_FOUND => Ok(None), @@ -83,7 +82,7 @@ pub fn find_password(service: &String) -> Result, KeyringError> { }); } - let keychain = Keychain::default().unwrap(); + let keychain = SecKeychain::default().unwrap(); match keychain.find_password(cred_attrs[0], cred_attrs[1]) { Ok((pw, _)) => { let pw_str = String::from_utf8(pw.to_owned())?; @@ -104,7 +103,7 @@ pub fn find_password(service: &String) -> Result, KeyringError> { /// - A `KeyringError` if there were any issues interacting with the credential vault /// pub fn delete_password(service: &String, account: &String) -> Result { - let keychain = Keychain::default().unwrap(); + let keychain = SecKeychain::default().unwrap(); match keychain.find_password(service.as_str(), account.as_str()) { Ok((_, item)) => { item.delete(); @@ -119,7 +118,7 @@ pub fn delete_password(service: &String, account: &String) -> Result { for result in search_results { if let Some(result_map) = result.simplify_dict() { From 803b04e515626021ef94376568b8054d9c4ae3cd Mon Sep 17 00:00:00 2001 From: Trae Yelovich Date: Mon, 25 Sep 2023 11:55:29 -0400 Subject: [PATCH 09/17] chore(secrets): Bump Secrets SDK to 7.18.6 Signed-off-by: Trae Yelovich --- npm-shrinkwrap.json | 6 +++--- packages/cli/package.json | 2 +- packages/secrets/package.json | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/npm-shrinkwrap.json b/npm-shrinkwrap.json index 37ba37e4e1..babfe4712a 100644 --- a/npm-shrinkwrap.json +++ b/npm-shrinkwrap.json @@ -23099,7 +23099,7 @@ "node": ">=14.0.0" }, "optionalDependencies": { - "@zowe/secrets-for-zowe-sdk": "7.18.5" + "@zowe/secrets-for-zowe-sdk": "7.18.6" } }, "packages/cli/node_modules/brace-expansion": { @@ -23170,7 +23170,7 @@ }, "packages/secrets": { "name": "@zowe/secrets-for-zowe-sdk", - "version": "7.18.5", + "version": "7.18.6", "hasInstallScript": true, "license": "EPL-2.0", "devDependencies": { @@ -29664,7 +29664,7 @@ "@zowe/imperative": "5.18.1", "@zowe/perf-timing": "1.0.7", "@zowe/provisioning-for-zowe-sdk": "7.18.2", - "@zowe/secrets-for-zowe-sdk": "7.18.5", + "@zowe/secrets-for-zowe-sdk": "7.18.6", "@zowe/zos-console-for-zowe-sdk": "7.18.2", "@zowe/zos-files-for-zowe-sdk": "7.18.2", "@zowe/zos-jobs-for-zowe-sdk": "7.18.2", diff --git a/packages/cli/package.json b/packages/cli/package.json index 96ab4282d9..7bc36ee6fd 100644 --- a/packages/cli/package.json +++ b/packages/cli/package.json @@ -93,7 +93,7 @@ "which": "^2.0.2" }, "optionalDependencies": { - "@zowe/secrets-for-zowe-sdk": "7.18.5" + "@zowe/secrets-for-zowe-sdk": "7.18.6" }, "engines": { "node": ">=14.0.0" diff --git a/packages/secrets/package.json b/packages/secrets/package.json index 85a3fb9b77..d265ccde63 100644 --- a/packages/secrets/package.json +++ b/packages/secrets/package.json @@ -3,7 +3,7 @@ "description": "Credential management facilities for Imperative, Zowe CLI, and extenders.", "repository": "https://github.com/zowe/zowe-cli.git", "author": "Zowe", - "version": "7.18.5", + "version": "7.18.6", "homepage": "https://github.com/zowe/zowe-cli/tree/master/packages/secrets#readme", "bugs": { "url": "https://github.com/zowe/zowe-cli/issues" From b36ad06b24ccee66742ab0a825e7781cd22fb3c5 Mon Sep 17 00:00:00 2001 From: Trae Yelovich Date: Mon, 25 Sep 2023 12:07:03 -0400 Subject: [PATCH 10/17] chore: Update CLI and Secrets SDK changelogs Signed-off-by: Trae Yelovich --- packages/cli/CHANGELOG.md | 4 ++++ packages/secrets/CHANGELOG.md | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/packages/cli/CHANGELOG.md b/packages/cli/CHANGELOG.md index 6a563fb015..718e9fa635 100644 --- a/packages/cli/CHANGELOG.md +++ b/packages/cli/CHANGELOG.md @@ -2,6 +2,10 @@ All notable changes to the Zowe CLI package will be documented in this file. +## Recent Changes + +- BugFix: Bump Secrets SDK to `7.18.6` to use `core-foundation-rs` instead of the now-archived `security-framework` crate. + ## `7.18.5` - BugFix: Bump Secrets SDK to `7.18.5` to resolve build failures for FreeBSD users. diff --git a/packages/secrets/CHANGELOG.md b/packages/secrets/CHANGELOG.md index 020b0a8324..873592ff3c 100644 --- a/packages/secrets/CHANGELOG.md +++ b/packages/secrets/CHANGELOG.md @@ -2,6 +2,10 @@ All notable changes to the Zowe Secrets SDK package will be documented in this file. +## `7.18.6` + +- BugFix: Use `core-foundation-rs` instead of `security-framework` for macOS logic, as `security-framework` is now archived. [#1802](https://github.com/zowe/zowe-cli/issues/1802) + ## `7.18.5` - BugFix: Enable `KeyringError::Library` enum variant to fix building on FreeBSD targets. From 2d1bb598fdad2b275682317bb24dd6ddf2712040 Mon Sep 17 00:00:00 2001 From: Trae Yelovich Date: Mon, 25 Sep 2023 14:15:03 -0400 Subject: [PATCH 11/17] fix(secrets/mac): Better error handling & doc comments Signed-off-by: Trae Yelovich --- .../secrets/src/keyring/src/os/mac/error.rs | 3 +- .../secrets/src/keyring/src/os/mac/ffi.rs | 33 ++++++++----------- .../src/keyring/src/os/mac/keychain.rs | 17 ++++++++++ .../src/keyring/src/os/mac/keychain_item.rs | 21 ++++++++++-- .../src/keyring/src/os/mac/keychain_search.rs | 19 +++++++---- .../secrets/src/keyring/src/os/mac/mod.rs | 32 +++++++++++------- 6 files changed, 83 insertions(+), 42 deletions(-) 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)), From 50baa47ef9cd1bcb7c2e3a2927bfa8ab7208ff36 Mon Sep 17 00:00:00 2001 From: Trae Yelovich Date: Mon, 25 Sep 2023 16:15:06 -0400 Subject: [PATCH 12/17] test(secrets): add test case for findCredentials w/ 1 match Signed-off-by: Trae Yelovich --- packages/secrets/src/keyring/__test__/index.spec.mjs | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/packages/secrets/src/keyring/__test__/index.spec.mjs b/packages/secrets/src/keyring/__test__/index.spec.mjs index 87abb1e383..e8d3fce976 100644 --- a/packages/secrets/src/keyring/__test__/index.spec.mjs +++ b/packages/secrets/src/keyring/__test__/index.spec.mjs @@ -183,6 +183,17 @@ test.serial("findPassword for CJK symbols", async (t) => { t.is(pw, "「こんにちは世界」"); }); +test.serial("findCredentials works when only one credential is found", async (t) => { + await setPassword("TestKeyring2", "TestOneCred", "pass"); + + const creds = await findCredentials("TestKeyring2"); + t.deepEqual(creds, [{ + account: "TestOneCred", + password: "pass" + }]); + await deletePassword("TestKeyring2", "TestOneCred"); +}); + test("deletePassword deletes all test credentials", async (t) => { console.log( "\nThe deletePassword test is running. There is an intended delay of 5 seconds to wait for the keyring to update." From 0e9237718e86e7631f7a07e819c15ff0f648d7b0 Mon Sep 17 00:00:00 2001 From: Trae Yelovich Date: Mon, 25 Sep 2023 16:36:18 -0400 Subject: [PATCH 13/17] fix(secrets/linux): call secret_value_unref after building password string Signed-off-by: Trae Yelovich --- packages/secrets/src/keyring/src/os/unix.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/secrets/src/keyring/src/os/unix.rs b/packages/secrets/src/keyring/src/os/unix.rs index 3229ba46b9..46baf5f2e0 100644 --- a/packages/secrets/src/keyring/src/os/unix.rs +++ b/packages/secrets/src/keyring/src/os/unix.rs @@ -194,13 +194,12 @@ pub fn find_credentials( match item.secret() { Some(secret) => { let bytes = secret.get(); + let acc = attrs.get("account").unwrap().clone(); + let pw = String::from_utf8(bytes).unwrap_or("".to_string()); unsafe { libsecret_sys::secret_value_unref(secret.as_ptr() as *mut _); } - let acc = attrs.get("account").unwrap().clone(); - let pw = String::from_utf8(bytes).unwrap_or("".to_string()); - Some((acc, pw)) } None => None, From 3993053d4f80c228e91792256bd70815e5679a95 Mon Sep 17 00:00:00 2001 From: Trae Yelovich Date: Tue, 26 Sep 2023 09:22:32 -0400 Subject: [PATCH 14/17] wip(fix): bus error for findCredentials linux impl Signed-off-by: Trae Yelovich --- packages/secrets/src/keyring/Cargo.lock | 1 + packages/secrets/src/keyring/Cargo.toml | 1 + packages/secrets/src/keyring/src/os/unix.rs | 11 +++++++++-- 3 files changed, 11 insertions(+), 2 deletions(-) diff --git a/packages/secrets/src/keyring/Cargo.lock b/packages/secrets/src/keyring/Cargo.lock index 3ec0ec60e7..33b3050595 100644 --- a/packages/secrets/src/keyring/Cargo.lock +++ b/packages/secrets/src/keyring/Cargo.lock @@ -278,6 +278,7 @@ dependencies = [ "core-foundation-sys", "gio", "glib", + "glib-sys", "libsecret", "libsecret-sys", "napi", diff --git a/packages/secrets/src/keyring/Cargo.toml b/packages/secrets/src/keyring/Cargo.toml index 0d6dd44ab5..a6bf582f48 100644 --- a/packages/secrets/src/keyring/Cargo.toml +++ b/packages/secrets/src/keyring/Cargo.toml @@ -32,6 +32,7 @@ core-foundation-sys = "0.8.4" [target.'cfg(any(target_os = "freebsd", target_os = "linux"))'.dependencies] glib = "0.17.10" +glib-sys = "0.17.10" gio = "0.17.10" libsecret = "0.3.0" libsecret-sys = "0.3.0" diff --git a/packages/secrets/src/keyring/src/os/unix.rs b/packages/secrets/src/keyring/src/os/unix.rs index 46baf5f2e0..c84473531e 100644 --- a/packages/secrets/src/keyring/src/os/unix.rs +++ b/packages/secrets/src/keyring/src/os/unix.rs @@ -1,5 +1,7 @@ extern crate libsecret; +extern crate glib_sys; use glib::translate::{FromGlibPtrContainer, ToGlibPtr}; +use glib_sys::{g_hash_table_unref}; use libsecret::{ prelude::CollectionExtManual, traits::ItemExt, SearchFlags, Service, ServiceFlags, }; @@ -197,16 +199,21 @@ pub fn find_credentials( let acc = attrs.get("account").unwrap().clone(); let pw = String::from_utf8(bytes).unwrap_or("".to_string()); unsafe { - libsecret_sys::secret_value_unref(secret.as_ptr() as *mut _); + secret.unref(); + g_hash_table_unref(attrs.to_glib_full()); } Some((acc, pw)) } - None => None, + None => { + g_hash_table_unref(attrs.to_glib_full()); + None + }, } }) .collect(); *credentials = valid_creds; + Ok(true) } From 182cf46ffe388694960cf0e34512e3cf738d715c Mon Sep 17 00:00:00 2001 From: Trae Yelovich Date: Tue, 26 Sep 2023 14:57:14 +0000 Subject: [PATCH 15/17] fix(secrets/linux): remove redundant free for secret item values Signed-off-by: Trae Yelovich --- packages/secrets/src/keyring/Cargo.lock | 54 +++++++------------ packages/secrets/src/keyring/Cargo.toml | 12 ++--- .../src/keyring/__test__/index.spec.mjs | 22 ++++---- packages/secrets/src/keyring/src/os/unix.rs | 18 +++---- 4 files changed, 44 insertions(+), 62 deletions(-) diff --git a/packages/secrets/src/keyring/Cargo.lock b/packages/secrets/src/keyring/Cargo.lock index 33b3050595..ca9f2a105d 100644 --- a/packages/secrets/src/keyring/Cargo.lock +++ b/packages/secrets/src/keyring/Cargo.lock @@ -11,24 +11,12 @@ dependencies = [ "memchr", ] -[[package]] -name = "anyhow" -version = "1.0.72" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3b13c32d80ecc7ab747b80c3784bce54ee8a7a0cc4fbda9bf4cda2cf6fe90854" - [[package]] name = "autocfg" version = "1.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d468802bab17cbc0cc575e9b053f41e72aa36bfa6b7f55e3529ffa43161b97fa" -[[package]] -name = "bitflags" -version = "1.3.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "bef38d45163c2f1dde094a7dfd33ccf595c92905c8f8f4fdc18d06fb1037718a" - [[package]] name = "bitflags" version = "2.3.3" @@ -157,11 +145,10 @@ dependencies = [ [[package]] name = "gio" -version = "0.17.10" +version = "0.18.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a6973e92937cf98689b6a054a9e56c657ed4ff76de925e36fc331a15f0c5d30a" +checksum = "57052f84e8e5999b258e8adf8f5f2af0ac69033864936b8b6838321db2f759b1" dependencies = [ - "bitflags 1.3.2", "futures-channel", "futures-core", "futures-io", @@ -177,9 +164,9 @@ dependencies = [ [[package]] name = "gio-sys" -version = "0.17.10" +version = "0.18.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0ccf87c30a12c469b6d958950f6a9c09f2be20b7773f7e70d20b867fdf2628c3" +checksum = "37566df850baf5e4cb0dfb78af2e4b9898d817ed9263d1090a2df958c64737d2" dependencies = [ "glib-sys", "gobject-sys", @@ -190,11 +177,11 @@ dependencies = [ [[package]] name = "glib" -version = "0.17.10" +version = "0.18.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d3fad45ba8d4d2cea612b432717e834f48031cd8853c8aaf43b2c79fec8d144b" +checksum = "1c316afb01ce8067c5eaab1fc4f2cd47dc21ce7b6296358605e2ffab23ccbd19" dependencies = [ - "bitflags 1.3.2", + "bitflags", "futures-channel", "futures-core", "futures-executor", @@ -213,24 +200,23 @@ dependencies = [ [[package]] name = "glib-macros" -version = "0.17.10" +version = "0.18.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "eca5c79337338391f1ab8058d6698125034ce8ef31b72a442437fa6c8580de26" +checksum = "f8da903822b136d42360518653fcf154455defc437d3e7a81475bf9a95ff1e47" dependencies = [ - "anyhow", "heck", "proc-macro-crate", "proc-macro-error", "proc-macro2", "quote", - "syn 1.0.109", + "syn 2.0.28", ] [[package]] name = "glib-sys" -version = "0.17.10" +version = "0.18.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d80aa6ea7bba0baac79222204aa786a6293078c210abe69ef1336911d4bdc4f0" +checksum = "063ce2eb6a8d0ea93d2bf8ba1957e78dbab6be1c2220dd3daca57d5a9d869898" dependencies = [ "libc", "system-deps", @@ -238,9 +224,9 @@ dependencies = [ [[package]] name = "gobject-sys" -version = "0.17.10" +version = "0.18.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "cd34c3317740a6358ec04572c1bcfd3ac0b5b6529275fae255b237b314bb8062" +checksum = "0850127b514d1c4a4654ead6dedadb18198999985908e6ffe4436f53c785ce44" dependencies = [ "glib-sys", "libc", @@ -306,23 +292,21 @@ dependencies = [ [[package]] name = "libsecret" -version = "0.3.0" +version = "0.4.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "accb700635d0b1b296d83c93fa5d112400168d04481db0ccca946293af9f0206" +checksum = "ac6fae6ebe590e06ef9d01b125e46b7d4c05ccbd5961f12b4aefe2ecd010220f" dependencies = [ - "bitflags 1.3.2", "gio", "glib", "libc", "libsecret-sys", - "once_cell", ] [[package]] name = "libsecret-sys" -version = "0.3.0" +version = "0.4.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2b06012ca123d27ccceffa112d0f930c23eb549a2447dc710e99f5dc2d1040b2" +checksum = "9b716fc5e1c82eb0d28665882628382ab0e0a156a6d73580e33f0ac6ac8d2540" dependencies = [ "gio-sys", "glib-sys", @@ -344,7 +328,7 @@ version = "2.13.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "0ede2d12cd6fce44da537a4be1f5510c73be2506c2e32dfaaafd1f36968f3a0e" dependencies = [ - "bitflags 2.3.3", + "bitflags", "ctor", "napi-derive", "napi-sys", diff --git a/packages/secrets/src/keyring/Cargo.toml b/packages/secrets/src/keyring/Cargo.toml index a6bf582f48..24829da005 100644 --- a/packages/secrets/src/keyring/Cargo.toml +++ b/packages/secrets/src/keyring/Cargo.toml @@ -31,11 +31,11 @@ core-foundation = "0.9.3" core-foundation-sys = "0.8.4" [target.'cfg(any(target_os = "freebsd", target_os = "linux"))'.dependencies] -glib = "0.17.10" -glib-sys = "0.17.10" -gio = "0.17.10" -libsecret = "0.3.0" -libsecret-sys = "0.3.0" +glib = "0.18.2" +glib-sys = "0.18.1" +gio = "0.18.2" +libsecret = "0.4.0" +libsecret-sys = "0.4.0" [build-dependencies] napi-build = "2" @@ -43,4 +43,4 @@ napi-build = "2" [profile.release] lto = true opt-level = "z" # Optimize for size. -strip = "symbols" \ No newline at end of file +strip = "symbols" diff --git a/packages/secrets/src/keyring/__test__/index.spec.mjs b/packages/secrets/src/keyring/__test__/index.spec.mjs index e8d3fce976..6ca2cd4c60 100644 --- a/packages/secrets/src/keyring/__test__/index.spec.mjs +++ b/packages/secrets/src/keyring/__test__/index.spec.mjs @@ -163,6 +163,17 @@ test.serial( } ); +test.serial("findCredentials works when only one credential is found", async (t) => { + await setPassword("TestKeyring2", "TestOneCred", "pass"); + + const creds = await findCredentials("TestKeyring2"); + t.deepEqual(creds, [{ + account: "TestOneCred", + password: "pass" + }]); + await deletePassword("TestKeyring2", "TestOneCred"); +}); + test.serial("findPassword for ASCII string", async (t) => { const pw = await findPassword("TestKeyring/TestASCII"); t.is(pw, "ASCII string"); @@ -183,17 +194,6 @@ test.serial("findPassword for CJK symbols", async (t) => { t.is(pw, "「こんにちは世界」"); }); -test.serial("findCredentials works when only one credential is found", async (t) => { - await setPassword("TestKeyring2", "TestOneCred", "pass"); - - const creds = await findCredentials("TestKeyring2"); - t.deepEqual(creds, [{ - account: "TestOneCred", - password: "pass" - }]); - await deletePassword("TestKeyring2", "TestOneCred"); -}); - test("deletePassword deletes all test credentials", async (t) => { console.log( "\nThe deletePassword test is running. There is an intended delay of 5 seconds to wait for the keyring to update." diff --git a/packages/secrets/src/keyring/src/os/unix.rs b/packages/secrets/src/keyring/src/os/unix.rs index c84473531e..db121368c0 100644 --- a/packages/secrets/src/keyring/src/os/unix.rs +++ b/packages/secrets/src/keyring/src/os/unix.rs @@ -4,6 +4,7 @@ use glib::translate::{FromGlibPtrContainer, ToGlibPtr}; use glib_sys::{g_hash_table_unref}; use libsecret::{ prelude::CollectionExtManual, traits::ItemExt, SearchFlags, Service, ServiceFlags, + password_free }; use std::collections::HashMap; @@ -181,32 +182,29 @@ pub fn find_credentials( match collection.search_sync( Some(&get_schema()), HashMap::from([("service", service.as_str())]), - SearchFlags::ALL | SearchFlags::LOAD_SECRETS, + SearchFlags::ALL | SearchFlags::UNLOCK | SearchFlags::LOAD_SECRETS, gio::Cancellable::NONE, ) { Ok(vec) => { let valid_creds: Vec<(String, String)> = vec .iter() .filter_map(|item| { - let attrs: HashMap = unsafe { - let attrs = - libsecret_sys::secret_item_get_attributes(item.to_glib_none().0); - FromGlibPtrContainer::from_glib_full(attrs) - }; match item.secret() { Some(secret) => { + let attrs: HashMap = unsafe { + let attrs = libsecret_sys::secret_item_get_attributes(item.to_glib_none().0); + FromGlibPtrContainer::from_glib_full(attrs) + }; let bytes = secret.get(); - let acc = attrs.get("account").unwrap().clone(); let pw = String::from_utf8(bytes).unwrap_or("".to_string()); + + let acc = attrs.get("account").unwrap().clone(); unsafe { - secret.unref(); g_hash_table_unref(attrs.to_glib_full()); } - Some((acc, pw)) } None => { - g_hash_table_unref(attrs.to_glib_full()); None }, } From 29eb79cfeff9d9af926d42c993a34eacba502a69 Mon Sep 17 00:00:00 2001 From: Trae Yelovich Date: Tue, 26 Sep 2023 11:55:19 -0400 Subject: [PATCH 16/17] chore: Update CLI and Secrets SDK changelogs Signed-off-by: Trae Yelovich --- packages/cli/CHANGELOG.md | 2 +- packages/secrets/CHANGELOG.md | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/cli/CHANGELOG.md b/packages/cli/CHANGELOG.md index 718e9fa635..775ce1c001 100644 --- a/packages/cli/CHANGELOG.md +++ b/packages/cli/CHANGELOG.md @@ -4,7 +4,7 @@ All notable changes to the Zowe CLI package will be documented in this file. ## Recent Changes -- BugFix: Bump Secrets SDK to `7.18.6` to use `core-foundation-rs` instead of the now-archived `security-framework` crate. +- BugFix: Bump Secrets SDK to `7.18.6` to use `core-foundation-rs` instead of the now-archived `security-framework` crate, and to include the edge-case bug fix for Linux. ## `7.18.5` diff --git a/packages/secrets/CHANGELOG.md b/packages/secrets/CHANGELOG.md index 873592ff3c..054df8cec3 100644 --- a/packages/secrets/CHANGELOG.md +++ b/packages/secrets/CHANGELOG.md @@ -5,6 +5,7 @@ All notable changes to the Zowe Secrets SDK package will be documented in this f ## `7.18.6` - BugFix: Use `core-foundation-rs` instead of `security-framework` for macOS logic, as `security-framework` is now archived. [#1802](https://github.com/zowe/zowe-cli/issues/1802) +- BugFix: Resolve bug where `findCredentials` scenarios with one match causes a segmentation fault on Linux. ## `7.18.5` From 7a3b9648130ec095be73271537152ed44d7d75d8 Mon Sep 17 00:00:00 2001 From: Trae Yelovich Date: Tue, 26 Sep 2023 15:01:40 -0400 Subject: [PATCH 17/17] chore(secrets): remove unused import in unix.rs; format w/ rust-analyzer Signed-off-by: Trae Yelovich --- packages/secrets/src/keyring/src/os/unix.rs | 83 ++++++++++----------- 1 file changed, 39 insertions(+), 44 deletions(-) diff --git a/packages/secrets/src/keyring/src/os/unix.rs b/packages/secrets/src/keyring/src/os/unix.rs index db121368c0..723817a7d3 100644 --- a/packages/secrets/src/keyring/src/os/unix.rs +++ b/packages/secrets/src/keyring/src/os/unix.rs @@ -1,10 +1,9 @@ -extern crate libsecret; extern crate glib_sys; +extern crate libsecret; use glib::translate::{FromGlibPtrContainer, ToGlibPtr}; -use glib_sys::{g_hash_table_unref}; +use glib_sys::g_hash_table_unref; use libsecret::{ prelude::CollectionExtManual, traits::ItemExt, SearchFlags, Service, ServiceFlags, - password_free }; use std::collections::HashMap; @@ -21,7 +20,7 @@ impl From for KeyringError { /// /// Returns the libsecret schema that corresponds to service and account attributes. -/// +/// fn get_schema() -> libsecret::Schema { libsecret::Schema::new( "org.freedesktop.Secret.Generic", @@ -35,20 +34,20 @@ fn get_schema() -> libsecret::Schema { /// /// Builds an attribute map with the given service and account names. -/// +/// /// Returns: /// - A `HashMap` built with the `service` and `account` values provided. Used for attribute functions. -/// +/// fn get_attribute_map<'a>(service: &'a str, account: &'a str) -> HashMap<&'a str, &'a str> { HashMap::from([("service", service), ("account", account)]) } -/// +/// /// Attempts to set a password for a given service and account. -/// +/// /// - `service`: The service name for the new credential /// - `account`: The account name for the new credential -/// +/// /// Returns: /// - `true` if the credential was stored successfully /// - A `KeyringError` if there were any issues interacting with the credential vault @@ -74,16 +73,16 @@ pub fn set_password( } } -/// +/// /// Returns a password contained in the given service and account, if found. -/// +/// /// - `service`: The service name that matches the credential of interest /// - `account`: The account name that matches the credential of interest -/// +/// /// Returns: /// - `Some(password)` if a matching credential was found; `None` otherwise /// - A `KeyringError` if there were any issues interacting with the credential vault -/// +/// pub fn get_password(service: &String, account: &String) -> Result, KeyringError> { let attributes = get_attribute_map(service.as_str(), account.as_str()); @@ -96,15 +95,15 @@ pub fn get_password(service: &String, account: &String) -> Result } } -/// +/// /// Returns the first password (if any) that matches the given service pattern. -/// +/// /// - `service`: The service pattern that matches the credential of interest -/// +/// /// Returns: /// - `Some(password)` if a matching credential was found; `None` otherwise /// - A `KeyringError` if there were any issues interacting with the credential vault -/// +/// pub fn find_password(service: &String) -> Result, KeyringError> { let attributes = if service.contains("/") && service.len() > 1 { // In format "service/account" @@ -123,16 +122,16 @@ pub fn find_password(service: &String) -> Result, KeyringError> { } } -/// +/// /// Attempts to delete the password associated with a given service and account. -/// +/// /// - `service`: The service name of the credential to delete /// - `account`: The account name of the credential to delete -/// +/// /// Returns: /// - `true` if a matching credential was deleted; `false` otherwise /// - A `KeyringError` if there were any issues interacting with the credential vault -/// +/// pub fn delete_password(service: &String, account: &String) -> Result { match libsecret::password_clear_sync( Some(&get_schema()), @@ -147,16 +146,16 @@ pub fn delete_password(service: &String, account: &String) -> Result, @@ -188,30 +187,26 @@ pub fn find_credentials( Ok(vec) => { let valid_creds: Vec<(String, String)> = vec .iter() - .filter_map(|item| { - match item.secret() { - Some(secret) => { - let attrs: HashMap = unsafe { - let attrs = libsecret_sys::secret_item_get_attributes(item.to_glib_none().0); - FromGlibPtrContainer::from_glib_full(attrs) - }; - let bytes = secret.get(); - let pw = String::from_utf8(bytes).unwrap_or("".to_string()); + .filter_map(|item| match item.secret() { + Some(secret) => { + let attrs: HashMap = unsafe { + let attrs = + libsecret_sys::secret_item_get_attributes(item.to_glib_none().0); + FromGlibPtrContainer::from_glib_full(attrs) + }; + let bytes = secret.get(); + let pw = String::from_utf8(bytes).unwrap_or("".to_string()); - let acc = attrs.get("account").unwrap().clone(); - unsafe { - g_hash_table_unref(attrs.to_glib_full()); - } - Some((acc, pw)) + let acc = attrs.get("account").unwrap().clone(); + unsafe { + g_hash_table_unref(attrs.to_glib_full()); } - None => { - None - }, + Some((acc, pw)) } + None => None, }) .collect(); *credentials = valid_creds; - Ok(true) }