Skip to content

Commit

Permalink
Implement review
Browse files Browse the repository at this point in the history 
Signed-off-by: Klim Tsoutsman <[email protected]>
  • Loading branch information
@tsoutsman
tsoutsman committed Aug 31, 2024
1 parent 49acfaf commit 2c96328
Showing 1 changed file with 73 additions and 54 deletions.
127 changes: 73 additions & 54 deletions security-framework/src/item.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,35 +32,35 @@ pub struct ItemClass(CFStringRef);

impl ItemClass {
/// Look for `SecKeychainItem`s corresponding to generic passwords.
#[inline(always)]
#[inline]
#[must_use]
pub fn generic_password() -> Self {
unsafe { Self(kSecClassGenericPassword) }
}

/// Look for `SecKeychainItem`s corresponding to internet passwords.
#[inline(always)]
#[inline]
#[must_use]
pub fn internet_password() -> Self {
unsafe { Self(kSecClassInternetPassword) }
}

/// Look for `SecCertificate`s.
#[inline(always)]
#[inline]
#[must_use]
pub fn certificate() -> Self {
unsafe { Self(kSecClassCertificate) }
}

/// Look for `SecKey`s.
#[inline(always)]
#[inline]
#[must_use]
pub fn key() -> Self {
unsafe { Self(kSecClassKey) }
}

/// Look for `SecIdentity`s.
#[inline(always)]
#[inline]
#[must_use]
pub fn identity() -> Self {
unsafe { Self(kSecClassIdentity) }
Expand All @@ -73,17 +73,17 @@ pub struct KeyClass(CFStringRef);

impl KeyClass {
/// `kSecAttrKeyClassPublic`
#[inline(always)]
#[inline]
#[must_use] pub fn public() -> Self {
unsafe { Self(kSecAttrKeyClassPublic) }
}
/// `kSecAttrKeyClassPrivate`
#[inline(always)]
#[inline]
#[must_use] pub fn private() -> Self {
unsafe { Self(kSecAttrKeyClassPrivate) }
}
/// `kSecAttrKeyClassSymmetric`
#[inline(always)]
#[inline]
#[must_use] pub fn symmetric() -> Self {
unsafe { Self(kSecAttrKeyClassSymmetric) }
}
Expand Down Expand Up @@ -151,29 +151,29 @@ impl crate::ItemSearchOptionsInternals for ItemSearchOptions {

impl ItemSearchOptions {
/// Creates a new builder with default options.
#[inline(always)]
#[inline]
#[must_use]
pub fn new() -> Self {
Self::default()
}

/// Search only for items of the specified class.
#[inline(always)]
#[inline]
pub fn class(&mut self, class: ItemClass) -> &mut Self {
self.class = Some(class);
self
}

/// Whether search for an item should be case insensitive or not.
#[inline(always)]
#[inline]
pub fn case_insensitive(&mut self, case_insensitive: Option<bool>) -> &mut Self {
self.case_insensitive = case_insensitive;
self
}

/// Search only for keys of the specified class. Also sets self.class to
/// `ItemClass::key()`.
#[inline(always)]
#[inline]
pub fn key_class(&mut self, key_class: KeyClass) -> &mut Self {
self.class(ItemClass::key());
self.key_class = Some(key_class);
Expand All @@ -182,23 +182,23 @@ impl ItemSearchOptions {

/// Load Security Framework objects (`SecCertificate`, `SecKey`, etc) for
/// the results.
#[inline(always)]
#[inline]
pub fn load_refs(&mut self, load_refs: bool) -> &mut Self {
self.load_refs = load_refs;
self
}

/// Load Security Framework object attributes for
/// the results.
#[inline(always)]
#[inline]
pub fn load_attributes(&mut self, load_attributes: bool) -> &mut Self {
self.load_attributes = load_attributes;
self
}

/// Load Security Framework objects data for
/// the results.
#[inline(always)]
#[inline]
pub fn load_data(&mut self, load_data: bool) -> &mut Self {
self.load_data = load_data;
self
Expand All @@ -207,55 +207,56 @@ impl ItemSearchOptions {
/// Limit the number of search results.
///
/// If this is not called, the default limit is 1.
#[inline(always)]
#[inline]
pub fn limit<T: Into<Limit>>(&mut self, limit: T) -> &mut Self {
self.limit = Some(limit.into());
self
}

/// Search for an item with the given label.
#[inline(always)]
#[inline]
pub fn label(&mut self, label: &str) -> &mut Self {
self.label = Some(CFString::new(label));
self
}

/// Whether untrusted certificates should be returned.
#[inline(always)]
#[inline]
pub fn trusted_only(&mut self, trusted_only: Option<bool>) -> &mut Self {
self.trusted_only = trusted_only;
self
}

/// Search for an item with the given service.
#[inline(always)]
#[inline]
pub fn service(&mut self, service: &str) -> &mut Self {
self.service = Some(CFString::new(service));
self
}

/// Search for an item with exactly the given subject.
#[inline(always)]
#[inline]
pub fn subject(&mut self, subject: &str) -> &mut Self {
self.subject = Some(CFString::new(subject));
self
}

/// Search for an item with the given account.
#[inline(always)]
#[inline]
pub fn account(&mut self, account: &str) -> &mut Self {
self.account = Some(CFString::new(account));
self
}

/// Search for an item with a specific access group.
#[inline]
pub fn access_group(&mut self, access_group: &str) -> &mut Self {
self.access_group = Some(CFString::new(access_group));
self
}

/// Sets `kSecAttrAccessGroup` to `kSecAttrAccessGroupToken`
#[inline(always)]
#[inline]
pub fn access_group_token(&mut self) -> &mut Self {
self.access_group = unsafe { Some(CFString::wrap_under_get_rule(kSecAttrAccessGroupToken)) };
self
Expand All @@ -266,7 +267,7 @@ impl ItemSearchOptions {
/// This is only compatible with [`ItemClass::certificate`], to search for
/// a key by public key hash use [`ItemSearchOptions::application_label`]
/// instead.
#[inline(always)]
#[inline]
pub fn pub_key_hash(&mut self, pub_key_hash: &[u8]) -> &mut Self {
self.pub_key_hash = Some(CFData::from_buffer(pub_key_hash));
self
Expand All @@ -277,15 +278,15 @@ impl ItemSearchOptions {
/// This is only compatible with [`ItemClass::key`], to search for a
/// certificate by the public key hash use [`ItemSearchOptions::pub_key_hash`]
/// instead.
#[inline(always)]
#[inline]
pub fn application_label(&mut self, app_label: &[u8]) -> &mut Self {
self.app_label = Some(CFData::from_buffer(app_label));
self
}

/// Populates a `CFDictionary` to be passed to `update_item` or `delete_item`.
#[inline(always)]
pub fn to_dictionary(&self) -> CFDictionary {
#[inline]
fn to_dictionary(&self) -> CFDictionary {
unsafe {
let mut params = CFMutableDictionary::from_CFType_pairs(&[]);

Expand Down Expand Up @@ -441,6 +442,13 @@ impl ItemSearchOptions {
Ok(items)
}
}

/// Deletes objects matching the search options.
///
/// Translates to `SecItemDelete`.
pub fn delete(&self) -> Result<()> {
cvt(unsafe { SecItemDelete(self.to_dictionary().as_concrete_TypeRef()) })
}
}

unsafe fn get_item(item: CFTypeRef) -> SearchResult {
Expand Down Expand Up @@ -578,8 +586,7 @@ impl SearchResult {
/// Builder-pattern struct for specifying options for `add_item` (`SecAddItem`
/// wrapper).
///
/// When finished populating options, call `to_dictionary()` and pass the
/// resulting `CFDictionary` to `add_item`.
/// When finished populating options call [`Self::add`].
pub struct ItemAddOptions {
/// The value (by ref or data) of the item to add, required.
pub value: ItemAddValue,
Expand Down Expand Up @@ -642,6 +649,7 @@ impl ItemAddOptions {
self
}
/// Populates a `CFDictionary` to be passed to `add_item`.
#[deprecated(since = "3.0.0", note = "use `ItemAddOptions::add` instead")]
pub fn to_dictionary(&self) -> CFDictionary {
let mut dict = CFMutableDictionary::from_CFType_pairs(&[]);

Expand Down Expand Up @@ -697,6 +705,13 @@ impl ItemAddOptions {

dict.to_immutable()
}
/// Adds the item to the keychain.
///
/// Translates to `SecItemAdd`.
pub fn add(&self) -> Result<()> {
#[allow(deprecated)]
cvt(unsafe { SecItemAdd(self.to_dictionary().as_concrete_TypeRef(), std::ptr::null_mut()) })
}
}

/// Value of an item to add to the keychain.
Expand Down Expand Up @@ -744,12 +759,11 @@ impl AddRef {
/// Builder-pattern struct for specifying options for `update_item` (`SecUpdateItem`
/// wrapper).
///
/// When finished populating options, call `to_dictionary()` and pass the
/// resulting `CFDictionary` to `update_item`.
/// When finished populating options call [`update_item`].
#[derive(Default)]
pub struct ItemUpdateOptions {
/// Optional value (by ref or data) of the item to update.
pub value: Option<ItemAddValue>,
pub value: Option<ItemUpdateValue>,
/// Optional kSecAttrAccount attribute.
pub account_name: Option<CFString>,
/// Optional kSecAttrAccessGroup attribute.
Expand All @@ -765,11 +779,9 @@ pub struct ItemUpdateOptions {
/// Optional keychain location.
pub location: Option<Location>,
/// Optional kSecClass.
///
/// This overrides the class provided by `value`.
///
/// If set to `None`, `value`'s class is ignored, and not added to the `CFDictionary`.
pub class: Option<Option<ItemClass>>,
/// Overwrites `value`'s class if set.
pub class: Option<ItemClass>,
}

impl ItemUpdateOptions {
Expand All @@ -780,12 +792,12 @@ impl ItemUpdateOptions {
}

/// Specifies the value of the item.
pub fn set_value(&mut self, value: ItemAddValue) -> &mut Self {
pub fn set_value(&mut self, value: ItemUpdateValue) -> &mut Self {
self.value = Some(value);
self
}
/// Specifies the `kSecClass` attribute.
pub fn set_class(&mut self, class: Option<ItemClass>) -> &mut Self {
pub fn set_class(&mut self, class: ItemClass) -> &mut Self {
self.class = Some(class);
self
}
Expand Down Expand Up @@ -825,27 +837,27 @@ impl ItemUpdateOptions {
self
}
/// Populates a `CFDictionary` to be passed to `update_item`.
pub fn to_dictionary(&self) -> CFDictionary {
fn to_dictionary(&self) -> CFDictionary {
let mut dict = CFMutableDictionary::from_CFType_pairs(&[]);

if let Some(ref value) = self.value {
let class_opt = match value {
ItemAddValue::Ref(ref_) => ref_.class(),
ItemAddValue::Data { class, .. } => Some(*class),
ItemUpdateValue::Ref(ref_) => ref_.class(),
ItemUpdateValue::Data(_) => None,
};
// We only write value's class if `set_class` wasn't called.
// `self.class` overwrites `value`'s class if set.
if self.class.is_none() {
if let Some(class) = class_opt {
dict.add(&unsafe { kSecClass }.to_void(), &class.0.to_void());
}
}
let value_pair = match value {
ItemAddValue::Ref(ref_) => (unsafe { kSecValueRef }.to_void(), ref_.ref_()),
ItemAddValue::Data { data, .. } => (unsafe { kSecValueData }.to_void(), data.to_void()),
ItemUpdateValue::Ref(ref_) => (unsafe { kSecValueRef }.to_void(), ref_.ref_()),
ItemUpdateValue::Data(data) => (unsafe { kSecValueData }.to_void(), data.to_void()),
};
dict.add(&value_pair.0, &value_pair.1);
}
if let Some(Some(class)) = self.class {
if let Some(class) = self.class {
dict.add(&unsafe { kSecClass }.to_void(), &class.0.to_void());
}
if let Some(location) = &self.location {
Expand Down Expand Up @@ -888,6 +900,17 @@ impl ItemUpdateOptions {
}
}

/// Value of an item to update in the keychain.
pub enum ItemUpdateValue {
/// Pass item by Ref (kSecValueRef)
Ref(AddRef),
/// Pass item by Data (kSecValueData)
///
/// Note that if the [`ItemClass`] of the updated data is different to the original data
/// stored in the keychain, it should be specified using [`ItemUpdateOptions::set_class`].
Data(CFData),
}

/// Which keychain to add an item to.
///
/// <https://developer.apple.com/documentation/technotes/tn3137-on-mac-keychains>
Expand All @@ -913,24 +936,20 @@ pub enum Location {

/// Translates to `SecItemAdd`. Use `ItemAddOptions` to build an `add_params`
/// `CFDictionary`.
#[deprecated(since = "3.0.0", note = "use `ItemAddOptions::add` instead")]
#[allow(deprecated)]
pub fn add_item(add_params: CFDictionary) -> Result<()> {
cvt(unsafe { SecItemAdd(add_params.as_concrete_TypeRef(), std::ptr::null_mut()) })
}

/// Translates to `SecItemUpdate`. Use `ItemSearchOptions` to build a `search_params` `CFDictionary`
/// and `ItemUpdateOptions` to build an `update_params` `CFDictionary`.
pub fn update_item(search_params: CFDictionary, update_params: CFDictionary) -> Result<()> {
/// Translates to `SecItemUpdate`.
pub fn update_item(search_params: &ItemSearchOptions, update_params: &ItemUpdateOptions) -> Result<()> {
cvt(unsafe { SecItemUpdate(
search_params.as_concrete_TypeRef(), update_params.as_concrete_TypeRef()
search_params.to_dictionary().as_concrete_TypeRef(),
update_params.to_dictionary().as_concrete_TypeRef()
)})
}

/// Translates to `SecItemDelete`. Use `ItemSearchOptions` to build a `search_params`
/// `CFDictionary`.
pub fn delete_item(search_params: CFDictionary) -> Result<()> {
cvt(unsafe { SecItemDelete(search_params.as_concrete_TypeRef()) })
}

#[cfg(test)]
mod test {
use super::*;
Expand Down

0 comments on commit 2c96328

Please sign in to comment.