diff --git a/keystore/src/connection/platform/generic/mod.rs b/keystore/src/connection/platform/generic/mod.rs index 0e7fbe3bcf..d11de8954c 100644 --- a/keystore/src/connection/platform/generic/mod.rs +++ b/keystore/src/connection/platform/generic/mod.rs @@ -208,34 +208,53 @@ impl SqlCipherConnection { pub static kSecAttrAccessible: CFStringRef; } + macro_rules! wrap_under_get_rule { + ($external_ref:expr) => { + // SAFETY: The method `CFString::wrap_under_get_rule` is required by rustc to be marked as unsafe + // because accessing any static item via FFI is intrinsically unsafe. For this reason, we can't + // just create a safe wrapper function here; rustc immediately flags it with an [E0133] because it + // needs that FFI static item to be its parameter. + // + // There isn't safety documentation anywhere in the CFString or TCFType documentation, and + // the implementation does only [basic checks], so we can assume that the main property we + // need to uphold is "the reference is not null". + // + // [E0133]: https://doc.rust-lang.org/stable/error_codes/E0133.html + // [basic checks]: https://github.com/servo/core-foundation-rs/blob/core-foundation-v0.10.0/core-foundation/src/lib.rs#L91-L95 + unsafe { CFString::wrap_under_get_rule($external_ref) } + }; + } + // Create a query that matches a: let query_params = CFDictionary::from_CFType_pairs(&[ // Class GenericPassword - (unsafe { CFString::wrap_under_get_rule(kSecClass) }, unsafe { - CFString::wrap_under_get_rule(kSecClassGenericPassword).as_CFType() - }), + ( + wrap_under_get_rule!(kSecClass), + wrap_under_get_rule!(kSecClassGenericPassword).as_CFType(), + ), // with Service = "wire.com" ( - unsafe { CFString::wrap_under_get_rule(kSecAttrService) }, + wrap_under_get_rule!(kSecAttrService), CFString::from(WIRE_SERVICE_NAME).as_CFType(), ), // Holding account name = `key` (in the following form: `keystore_salt_[sha256(file_path)]`) - ( - unsafe { CFString::wrap_under_get_rule(kSecAttrAccount) }, - CFString::from(key).as_CFType(), - ), + (wrap_under_get_rule!(kSecAttrAccount), CFString::from(key).as_CFType()), ]); // And now we ask to update the following properties: let payload_params = CFDictionary::from_CFType_pairs(&[( // Keychain Accessibility setting // See: https://developer.apple.com/documentation/security/ksecattraccessible - unsafe { CFString::wrap_under_get_rule(kSecAttrAccessible) }, + wrap_under_get_rule!(kSecAttrAccessible), // Set to AccessibleAfterFirstUnlock (i.e. is accessible after the first post-boot unlock) - unsafe { CFString::wrap_under_get_rule(kSecAttrAccessibleAfterFirstUnlock).as_CFType() }, + wrap_under_get_rule!(kSecAttrAccessibleAfterFirstUnlock).as_CFType(), )]); // Update the item in the keychain + // + // SAFETY: As before, the main source of unsafety here appears to simply be that this is an FFI function + // and nobody has constructed a safe wrapper aroud it. And sure, we can't trust the safety properties that + // external code has. But on the other hand, we can't not call this function. So the unsafe block should be fine. match unsafe { security_framework_sys::keychain_item::SecItemUpdate( query_params.as_concrete_TypeRef(),