SecCertificate::public_key_info_der #75
Conversation
| #[cfg(any(feature = "OSX_10_12", target_os = "ios"))] | ||
| /// Returns DER encoded subjectPublicKeyInfo of certificate if available. This can be used | ||
| /// for certificate pinning. | ||
| pub fn public_key_info_der(&self) -> Result<Option<Vec<u8>>> { |
There was a problem hiding this comment.
My concern is that this is a very specific single-purpose high-level function. Could it be broken down into components that help others write their public_key_info_der and alike themselves?
| // Imported from TrustKit | ||
| // https://github.com/datatheorem/TrustKit/blob/master/TrustKit/Pinning/TSKSPKIHashCache.m | ||
| unsafe { | ||
| let public_key = self.copy_public_key_from_certificate()?; |
There was a problem hiding this comment.
Is there a reason why this function is not public?
| unsafe { | ||
| let public_key = self.copy_public_key_from_certificate()?; | ||
| let mut error: CFErrorRef = ptr::null_mut(); | ||
| let public_key_attributes = SecKeyCopyAttributes(public_key); |
There was a problem hiding this comment.
Maybe there could be a high-level wrapper for SecKeyCopyAttributes, so that it doesn't require so much unsafe boilerplate for each property?
| return &EC_DSA_SECP_384_R1_ASN1_HEADER; | ||
| } | ||
| } | ||
| &[] |
There was a problem hiding this comment.
Better error handling would be to return None
|
|
||
| extern "C" { | ||
| #[cfg(any(feature = "OSX_10_12", target_os = "ios"))] | ||
| pub fn CFStringCompare( |
There was a problem hiding this comment.
You don't need this. CFString implements Eq, so you can compare them with just ==
There was a problem hiding this comment.
I'm using CFStringRef's due to comparing with constants kSecAttrKeyTypeRSA and kSecAttrKeyTypeECSECPrimeRandom. Can I turn those into CFString without allocating?
There was a problem hiding this comment.
I think you can. The CFString is a refcounted type, so a conversion from Ref to non-Ref should only need to change reference count:
There was a problem hiding this comment.
Also the eq is implemented as self.as_CFType().eq(&other.as_CFType())
There was a problem hiding this comment.
Ok switched to CFString now.
…pose SecCert::public_key
|
I've expanded SecKey and cut down on the big unsafe block into mostly safe code. |
|
Another small refactor to get rid of verbose error handling. Do you have any other suggestions for improving it? |
|
This is much nicer now. Thank you! |
|
Can this be released? |
No description provided.