Bug 1690278 - osclientcerts: cache key handles to potentially avoid multiple pin prompts r=mbirghan

When osclientcerts obtains or uses an OS handle on a private key, the
underlying implementation may display some sort of authentication or pin
prompt. In some cases, caching this handle rather than obtaining it multiple
times can prevent multiple prompts. So, this is what this patch does.

Differential Revision: https://phabricator.services.mozilla.com/D106222

security/manager/ssl/osclientcerts/src/backend_macos.rs

@@ -624,6 +624,7 @@ pub struct Key {
     modulus: Option<Vec<u8>>,
     ec_params: Option<Vec<u8>>,
     key_type_enum: KeyType,
+    key_handle: Option<SecKey>,
 }
 
 impl Key {
@@ -680,6 +681,7 @@ impl Key {
             modulus,
             ec_params,
             key_type_enum,
+            key_handle: None,
         })
     }
 
@@ -762,7 +764,7 @@ impl Key {
     }
 
     pub fn get_signature_length(
-        &self,
+        &mut self,
         data: &[u8],
         params: &Option<CK_RSA_PKCS_PSS_PARAMS>,
     ) -> Result<usize, ()> {
@@ -774,11 +776,26 @@ impl Key {
 
     // The input data is a hash. What algorithm we use depends on the size of the hash.
     pub fn sign(
-        &self,
+        &mut self,
         data: &[u8],
         params: &Option<CK_RSA_PKCS_PSS_PARAMS>,
     ) -> Result<Vec<u8>, ()> {
-        let key = sec_identity_copy_private_key(&self.identity)?;
+        // If this key hasn't been used for signing yet, there won't be a cached key handle. Obtain
+        // and cache it if this is the case. Doing so can cause the underlying implementation to
+        // show an authentication or pin prompt to the user. Caching the handle can avoid causing
+        // multiple prompts to be displayed in some cases.
+        if self.key_handle.is_none() {
+            let _ = self
+                .key_handle
+                .replace(sec_identity_copy_private_key(&self.identity)?);
+        }
+        let key = match &self.key_handle {
+            Some(key) => key,
+            None => {
+                error!("key_handle not set when it should have just been set?");
+                return Err(());
+            }
+        };
         let sign_params = SignParams::new(self.key_type_enum, data.len(), params)?;
         let signing_algorithm = sign_params.get_algorithm();
         let data = CFData::from_buffer(data);

security/manager/ssl/osclientcerts/src/backend_windows.rs

@@ -572,6 +572,8 @@ pub struct Key {
     key_type_enum: KeyType,
     /// Which slot this key should be exposed on.
     slot_type: SlotType,
+    /// A handle on the OS mechanism that represents this key.
+    key_handle: Option<KeyHandle>,
 }
 
 impl Key {
@@ -620,6 +622,7 @@ impl Key {
             ec_params,
             key_type_enum,
             slot_type: SlotType::Modern,
+            key_handle: None,
         })
     }
 
@@ -705,7 +708,7 @@ impl Key {
     }
 
     pub fn get_signature_length(
-        &self,
+        &mut self,
         data: &[u8],
         params: &Option<CK_RSA_PKCS_PSS_PARAMS>,
     ) -> Result<usize, ()> {
@@ -716,7 +719,7 @@ impl Key {
     }
 
     pub fn sign(
-        &self,
+        &mut self,
         data: &[u8],
         params: &Option<CK_RSA_PKCS_PSS_PARAMS>,
     ) -> Result<Vec<u8>, ()> {
@@ -727,14 +730,25 @@ impl Key {
     /// do_signature: if true, actually perform the signature. Otherwise, return a `Vec<u8>` of the
     /// length the signature would be, if performed.
     fn sign_internal(
-        &self,
+        &mut self,
         data: &[u8],
         params: &Option<CK_RSA_PKCS_PSS_PARAMS>,
         do_signature: bool,
     ) -> Result<Vec<u8>, ()> {
-        // Acquiring a handle on the key can cause the OS to show some UI to the user, so we do this
-        // as late as possible (i.e. here).
-        let key = KeyHandle::from_cert(&self.cert)?;
+        // If this key hasn't been used for signing yet, there won't be a cached key handle. Obtain
+        // and cache it if this is the case. Doing so can cause the underlying implementation to
+        // show an authentication or pin prompt to the user. Caching the handle can avoid causing
+        // multiple prompts to be displayed in some cases.
+        if self.key_handle.is_none() {
+            let _ = self.key_handle.replace(KeyHandle::from_cert(&self.cert)?);
+        }
+        let key = match &self.key_handle {
+            Some(key) => key,
+            None => {
+                error!("key_handle not set when it should have just been set?");
+                return Err(());
+            }
+        };
         key.sign(data, params, do_signature, self.key_type_enum)
     }
 }

security/manager/ssl/osclientcerts/src/manager.rs

@@ -583,7 +583,7 @@ impl Manager {
     }
 
     pub fn get_signature_length(
-        &self,
+        &mut self,
         session: CK_SESSION_HANDLE,
         data: &[u8],
     ) -> Result<usize, ()> {
@@ -591,7 +591,7 @@ impl Manager {
             Some((key_handle, params)) => (key_handle, params),
             None => return Err(()),
         };
-        let key = match self.objects.get(&key_handle) {
+        let key = match self.objects.get_mut(&key_handle) {
             Some(Object::Key(key)) => key,
             _ => return Err(()),
         };
@@ -605,7 +605,7 @@ impl Manager {
             Some((key_handle, params)) => (key_handle, params),
             None => return Err(()),
         };
-        let key = match self.objects.get(&key_handle) {
+        let key = match self.objects.get_mut(&key_handle) {
             Some(Object::Key(key)) => key,
             _ => return Err(()),
         };