Skip to content

Commit

Permalink
Don't repeat bad APIs
Browse files Browse the repository at this point in the history
  • Loading branch information
kornelski committed Dec 18, 2024
1 parent 878034e commit a240237
Show file tree
Hide file tree
Showing 12 changed files with 33 additions and 3 deletions.
1 change: 1 addition & 0 deletions security-framework/src/authorization.rs
Original file line number Diff line number Diff line change
Expand Up @@ -359,6 +359,7 @@ impl Authorization {
///
/// If `name` isn't convertable to a `CString` it will return
/// Err(errSecConversionError).
// TODO: deprecate and remove. CFDictionary should not be exposed in public Rust APIs.
pub fn get_right<T: Into<Vec<u8>>>(name: T) -> Result<CFDictionary<CFString, CFTypeRef>> {
let name = cstring_or_err!(name)?;
let mut dict = MaybeUninit::<CFDictionaryRef>::uninit();
Expand Down
3 changes: 3 additions & 0 deletions security-framework/src/item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -294,6 +294,7 @@ impl ItemSearchOptions {
}

/// Populates a `CFDictionary` to be passed to `update_item` or `delete_item`.
// CFDictionary should not be exposed in public Rust APIs.
#[inline]
fn to_dictionary(&self) -> CFDictionary {
unsafe {
Expand Down Expand Up @@ -676,6 +677,7 @@ impl ItemAddOptions {
}
/// Populates a `CFDictionary` to be passed to `add_item`.
#[deprecated(since = "3.0.0", note = "use `ItemAddOptions::add` instead")]
// CFDictionary should not be exposed in public Rust APIs.
pub fn to_dictionary(&self) -> CFDictionary {
let mut dict = CFMutableDictionary::from_CFType_pairs(&[]);

Expand Down Expand Up @@ -873,6 +875,7 @@ impl ItemUpdateOptions {
self
}
/// Populates a `CFDictionary` to be passed to `update_item`.
// CFDictionary should not be exposed in public Rust APIs.
#[inline]
fn to_dictionary(&self) -> CFDictionary {
let mut dict = CFMutableDictionary::from_CFType_pairs(&[]);
Expand Down
3 changes: 3 additions & 0 deletions security-framework/src/key.rs
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,7 @@ impl SecKey {

#[cfg(any(feature = "OSX_10_12", target_os = "ios", target_os = "tvos", target_os = "watchos", target_os = "visionos"))]
/// Translates to `SecKeyCopyAttributes`
// TODO: deprecate and remove. CFDictionary should not be exposed in public Rust APIs.
#[must_use]
pub fn attributes(&self) -> CFDictionary {
let pka = unsafe { SecKeyCopyAttributes(self.to_void() as _) };
Expand All @@ -184,6 +185,7 @@ impl SecKey {

#[cfg(any(feature = "OSX_10_12", target_os = "ios", target_os = "tvos", target_os = "watchos", target_os = "visionos"))]
/// Translates to `SecKeyCopyExternalRepresentation`
// TODO: deprecate and remove. CFData should not be exposed in public Rust APIs.
#[must_use]
pub fn external_representation(&self) -> Option<CFData> {
let mut error: CFErrorRef = ::std::ptr::null_mut();
Expand Down Expand Up @@ -406,6 +408,7 @@ impl GenerateKeyOptions {
}

/// Collect options into a `CFDictioanry`
// CFDictionary should not be exposed in public Rust APIs.
#[deprecated(note = "Pass the options to SecKey::new")]
pub fn to_dictionary(&self) -> CFDictionary {
#[cfg(target_os = "macos")]
Expand Down
2 changes: 2 additions & 0 deletions security-framework/src/os/macos/certificate_oids.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,15 @@ impl CertificateOid {
/// Returns the underlying raw pointer corresponding to this OID.
#[inline(always)]
#[must_use]
// FIXME: Don't expose CFStringRef in Rust APIs
pub fn as_ptr(&self) -> CFStringRef {
self.0
}

/// Returns the string representation of the OID.
#[inline]
#[must_use]
// FIXME: Don't expose CFString in Rust APIs
pub fn to_str(&self) -> CFString {
unsafe { CFString::wrap_under_get_rule(self.0) }
}
Expand Down
2 changes: 2 additions & 0 deletions security-framework/src/os/macos/code_signing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,7 @@ impl SecCode {

/// Retrieves the location on disk of signed code, given a code or static
/// code object.
// FIXME: Don't expose CFURL in Rust APIs.
pub fn path(&self, flags: Flags) -> Result<CFURL> {
let mut url = MaybeUninit::uninit();

Expand Down Expand Up @@ -290,6 +291,7 @@ impl SecStaticCode {

/// Retrieves the location on disk of signed code, given a code or static
/// code object.
// FIXME: Don't expose CFURL in Rust APIs.
pub fn path(&self, flags: Flags) -> Result<CFURL> {
let mut url = MaybeUninit::uninit();

Expand Down
1 change: 1 addition & 0 deletions security-framework/src/os/macos/digest_transform.rs
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,7 @@ impl Builder {
}

/// Computes the digest of the data.
// FIXME: deprecate and remove: don't expose CFData in Rust APIs.
pub fn execute(&self, data: &CFData) -> Result<CFData, CFError> {
unsafe {
let digest_type = match self.digest_type {
Expand Down
2 changes: 2 additions & 0 deletions security-framework/src/os/macos/encrypt_transform.rs
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,7 @@ impl Builder {
}

/// Encrypts data with a provided key.
// FIXME: deprecate and remove: don't expose CFData in Rust APIs.
pub fn encrypt(&self, key: &SecKey, data: &CFData) -> Result<CFData, CFError> {
unsafe {
let mut error = ptr::null_mut();
Expand All @@ -160,6 +161,7 @@ impl Builder {
}

/// Decrypts data with a provided key.
// FIXME: deprecate and remove: don't expose CFData in Rust APIs.
pub fn decrypt(&self, key: &SecKey, data: &CFData) -> Result<CFData, CFError> {
unsafe {
let mut error = ptr::null_mut();
Expand Down
8 changes: 5 additions & 3 deletions security-framework/src/os/macos/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ pub mod secure_transport;
pub mod transform;

#[cfg(test)]
pub mod test {
pub(crate) mod test {
use crate::identity::SecIdentity;
use crate::item::{ItemClass, ItemSearchOptions, Reference, SearchResult};
use crate::os::macos::item::ItemSearchOptionsExt;
Expand All @@ -26,7 +26,8 @@ pub mod test {
use std::io::prelude::*;
use std::path::Path;

#[must_use] pub fn identity(dir: &Path) -> SecIdentity {
#[must_use]
pub(crate) fn identity(dir: &Path) -> SecIdentity {
// FIXME https://github.com/rust-lang/rust/issues/30018
let keychain = keychain(dir);
let mut items = p!(ItemSearchOptions::new()
Expand All @@ -39,7 +40,8 @@ pub mod test {
}
}

#[must_use] pub fn keychain(dir: &Path) -> SecKeychain {
#[must_use]
pub(crate) fn keychain(dir: &Path) -> SecKeychain {
let path = dir.join("server.keychain");
let mut file = p!(File::create(&path));
p!(file.write_all(include_bytes!("../../../test/server.keychain")));
Expand Down
1 change: 1 addition & 0 deletions security-framework/src/os/macos/transform.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ impl SecTransform {
/// Executes the transform.
///
/// The return type depends on the type of transform.
// FIXME: deprecate and remove: don't expose CFType in Rust APIs.
pub fn execute(&mut self) -> Result<CFType, CFError> {
unsafe {
let mut error = ptr::null_mut();
Expand Down
7 changes: 7 additions & 0 deletions security-framework/src/passwords.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,12 @@ pub fn set_generic_password_options(
/// keychain entry exists, fails with error code `errSecItemNotFound`.
pub fn get_generic_password(service: &str, account: &str) -> Result<Vec<u8>> {
let mut options = PasswordOptions::new_generic_password(service, account);
#[allow(deprecated)]
options.query.push((
unsafe { CFString::wrap_under_get_rule(kSecReturnData) },
CFBoolean::from(true).into_CFType(),
));
#[allow(deprecated)]
let params = CFDictionary::from_CFType_pairs(&options.query);
let mut ret: CFTypeRef = std::ptr::null();
cvt(unsafe { SecItemCopyMatching(params.as_concrete_TypeRef(), &mut ret) })?;
Expand All @@ -53,6 +55,7 @@ pub fn get_generic_password(service: &str, account: &str) -> Result<Vec<u8>> {
/// If none exists, fails with error code `errSecItemNotFound`.
pub fn delete_generic_password(service: &str, account: &str) -> Result<()> {
let options = PasswordOptions::new_generic_password(service, account);
#[allow(deprecated)]
let params = CFDictionary::from_CFType_pairs(&options.query);
cvt(unsafe { SecItemDelete(params.as_concrete_TypeRef()) })
}
Expand Down Expand Up @@ -102,10 +105,12 @@ pub fn get_internet_password(
protocol,
authentication_type,
);
#[allow(deprecated)]
options.query.push((
unsafe { CFString::wrap_under_get_rule(kSecReturnData) },
CFBoolean::from(true).into_CFType(),
));
#[allow(deprecated)]
let params = CFDictionary::from_CFType_pairs(&options.query);
let mut ret: CFTypeRef = std::ptr::null();
cvt(unsafe { SecItemCopyMatching(params.as_concrete_TypeRef(), &mut ret) })?;
Expand All @@ -132,12 +137,14 @@ pub fn delete_internet_password(
protocol,
authentication_type,
);
#[allow(deprecated)]
let params = CFDictionary::from_CFType_pairs(&options.query);
cvt(unsafe { SecItemDelete(params.as_concrete_TypeRef()) })
}

// This starts by trying to create the password with the given query params.
// If the creation attempt reveals that one exists, its password is updated.
#[allow(deprecated)]
fn set_password_internal(options: &mut PasswordOptions, password: &[u8]) -> Result<()> {
let query_len = options.query.len();
options.query.push((
Expand Down
5 changes: 5 additions & 0 deletions security-framework/src/passwords_options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ use crate::access_control::SecAccessControl;
/// `PasswordOptions` constructor
pub struct PasswordOptions {
/// query built for the keychain request
#[deprecated(note = "This field should have been private. Please use setters that don't expose CFType")]
pub query: Vec<(CFString, CFType)>,
}

Expand Down Expand Up @@ -62,6 +63,7 @@ impl PasswordOptions {
CFString::from(account).into_CFType(),
),
];
#[allow(deprecated)]
Self { query }
}

Expand Down Expand Up @@ -115,11 +117,13 @@ impl PasswordOptions {
CFNumber::from(i32::from(port)).into_CFType(),
));
}
#[allow(deprecated)]
Self { query }
}

/// Add access control to the password
pub fn set_access_control_options(&mut self, options: AccessControlOptions) {
#[allow(deprecated)]
self.query.push((
unsafe { CFString::wrap_under_get_rule(kSecAttrAccessControl) },
SecAccessControl::create_with_flags(options.bits())
Expand All @@ -130,6 +134,7 @@ impl PasswordOptions {

/// Add access group to the password
pub fn set_access_group(&mut self, group: &str) {
#[allow(deprecated)]
self.query.push((
unsafe { CFString::wrap_under_get_rule(kSecAttrAccessGroup) },
CFString::from(group).into_CFType(),
Expand Down
1 change: 1 addition & 0 deletions security-framework/src/trust.rs
Original file line number Diff line number Diff line change
Expand Up @@ -265,6 +265,7 @@ impl SecTrust {
/// Note: evaluate must first be called on the `SecTrust`.
#[inline(always)]
#[must_use]
// FIXME: this should have been usize. Don't expose CFIndex in Rust APIs.
pub fn certificate_count(&self) -> CFIndex {
unsafe { SecTrustGetCertificateCount(self.0) }
}
Expand Down

0 comments on commit a240237

Please sign in to comment.