Simplify ownership of VerificationCertificates

This removes a lifetime, at the cost of acquiring the GIL to do some increfs.

src/rust/cryptography-x509-verification/src/certificate.rs

@@ -68,6 +68,20 @@ Xw4nMqk=
         ) -> Result<(), Self::Err> {
             Ok(())
         }
+
+        fn clone_public_key(key: &Self::Key) -> Self::Key {
+            key.clone()
+        }
+
+        fn clone_extra(extra: &Self::CertificateExtra) -> Self::CertificateExtra {
+            extra.clone()
+        }
+    }
+
+    #[test]
+    fn test_clone() {
+        assert_eq!(PublicKeyErrorOps::clone_public_key(&()), ());
+        assert_eq!(PublicKeyErrorOps::clone_extra(&()), ());
     }
 
     #[test]

src/rust/cryptography-x509-verification/src/lib.rs

@@ -239,22 +239,22 @@ impl<'a, 'chain> NameChain<'a, 'chain> {
     }
 }
 
-pub type Chain<'a, 'c, B> = Vec<&'a VerificationCertificate<'c, B>>;
-
-pub fn verify<'a, 'chain: 'a, B: CryptoOps>(
-    leaf: &'a VerificationCertificate<'chain, B>,
-    intermediates: &'a [&'a VerificationCertificate<'chain, B>],
-    policy: &'a Policy<'_, B>,
-    store: &'a Store<'chain, B>,
-) -> ValidationResult<Chain<'a, 'chain, B>> {
+pub type Chain<'c, B> = Vec<VerificationCertificate<'c, B>>;
+
+pub fn verify<'chain, B: CryptoOps>(
+    leaf: &VerificationCertificate<'chain, B>,
+    intermediates: &[VerificationCertificate<'chain, B>],
+    policy: &Policy<'_, B>,
+    store: &Store<'chain, B>,
+) -> ValidationResult<Chain<'chain, B>> {
     let builder = ChainBuilder::new(intermediates, policy, store);
 
     let mut budget = Budget::new();
     builder.build_chain(leaf, &mut budget)
 }
 
 struct ChainBuilder<'a, 'chain, B: CryptoOps> {
-    intermediates: &'a [&'a VerificationCertificate<'chain, B>],
+    intermediates: &'a [VerificationCertificate<'chain, B>],
     policy: &'a Policy<'a, B>,
     store: &'a Store<'chain, B>,
 }
@@ -278,9 +278,9 @@ impl ApplyNameConstraintStatus {
     }
 }
 
-impl<'a, 'chain: 'a, B: CryptoOps> ChainBuilder<'a, 'chain, B> {
+impl<'a, 'chain, B: CryptoOps> ChainBuilder<'a, 'chain, B> {
     fn new(
-        intermediates: &'a [&'a VerificationCertificate<'chain, B>],
+        intermediates: &'a [VerificationCertificate<'chain, B>],
         policy: &'a Policy<'a, B>,
         store: &'a Store<'chain, B>,
     ) -> Self {
@@ -300,27 +300,27 @@ impl<'a, 'chain: 'a, B: CryptoOps> ChainBuilder<'a, 'chain, B> {
         self.store
             .get_by_subject(&cert.certificate().tbs_cert.issuer)
             .iter()
-            .chain(self.intermediates.iter().copied().filter(|&candidate| {
+            .chain(self.intermediates.iter().filter(|&candidate| {
                 candidate.certificate().subject() == cert.certificate().issuer()
             }))
     }
 
     fn build_chain_inner(
         &self,
-        working_cert: &'a VerificationCertificate<'chain, B>,
+        working_cert: &VerificationCertificate<'chain, B>,
         current_depth: u8,
         working_cert_extensions: &Extensions<'chain>,
         name_chain: NameChain<'_, 'chain>,
         budget: &mut Budget,
-    ) -> ValidationResult<Chain<'a, 'chain, B>> {
+    ) -> ValidationResult<Chain<'chain, B>> {
         if let Some(nc) = working_cert_extensions.get_extension(&NAME_CONSTRAINTS_OID) {
             name_chain.evaluate_constraints(&nc.value()?, budget)?;
         }
 
         // Look in the store's root set to see if the working cert is listed.
         // If it is, we've reached the end.
         if self.store.contains(working_cert) {
-            return Ok(vec![working_cert]);
+            return Ok(vec![working_cert.clone()]);
         }
 
         // Check that our current depth does not exceed our policy-configured
@@ -383,7 +383,7 @@ impl<'a, 'chain: 'a, B: CryptoOps> ChainBuilder<'a, 'chain, B> {
                         budget,
                     ) {
                         Ok(mut chain) => {
-                            chain.push(working_cert);
+                            chain.push(working_cert.clone());
                             return Ok(chain);
                         }
                         // Immediately return on fatal error.
@@ -413,9 +413,9 @@ impl<'a, 'chain: 'a, B: CryptoOps> ChainBuilder<'a, 'chain, B> {
 
     fn build_chain(
         &self,
-        leaf: &'a VerificationCertificate<'chain, B>,
+        leaf: &VerificationCertificate<'chain, B>,
         budget: &mut Budget,
-    ) -> ValidationResult<Chain<'a, 'chain, B>> {
+    ) -> ValidationResult<Chain<'chain, B>> {
         // Before anything else, check whether the given leaf cert
         // is well-formed according to our policy (and its underlying
         // certificate profile).

src/rust/cryptography-x509-verification/src/ops.rs

@@ -5,13 +5,13 @@
 use cryptography_x509::certificate::Certificate;
 
 pub struct VerificationCertificate<'a, B: CryptoOps> {
-    cert: Certificate<'a>,
+    cert: &'a Certificate<'a>,
     public_key: once_cell::sync::OnceCell<B::Key>,
     extra: B::CertificateExtra,
 }
 
 impl<'a, B: CryptoOps> VerificationCertificate<'a, B> {
-    pub fn new(cert: Certificate<'a>, extra: B::CertificateExtra) -> Self {
+    pub fn new(cert: &'a Certificate<'a>, extra: B::CertificateExtra) -> Self {
         VerificationCertificate {
             cert,
             extra,
@@ -20,7 +20,7 @@ impl<'a, B: CryptoOps> VerificationCertificate<'a, B> {
     }
 
     pub fn certificate(&self) -> &Certificate<'a> {
-        &self.cert
+        self.cert
     }
 
     pub fn public_key(&self, ops: &B) -> Result<&B::Key, B::Err> {
@@ -40,6 +40,22 @@ impl<B: CryptoOps> PartialEq for VerificationCertificate<'_, B> {
 }
 impl<B: CryptoOps> Eq for VerificationCertificate<'_, B> {}
 
+impl<B: CryptoOps> Clone for VerificationCertificate<'_, B> {
+    fn clone(&self) -> Self {
+        Self {
+            cert: self.cert,
+            extra: B::clone_extra(&self.extra),
+            public_key: {
+                let cell = once_cell::sync::OnceCell::new();
+                if let Some(k) = self.public_key.get() {
+                    cell.set(B::clone_public_key(k)).ok().unwrap();
+                }
+                cell
+            },
+        }
+    }
+}
+
 pub trait CryptoOps {
     /// A public key type for this cryptographic backend.
     type Key;
@@ -58,6 +74,12 @@ pub trait CryptoOps {
     /// Verifies the signature on `Certificate` using the given
     /// `Key`.
     fn verify_signed_by(&self, cert: &Certificate<'_>, key: &Self::Key) -> Result<(), Self::Err>;
+
+    // Makes a `clone` of `Key`
+    fn clone_public_key(extra: &Self::Key) -> Self::Key;
+
+    // Makes a `clone` of `CertificateExtra`
+    fn clone_extra(extra: &Self::CertificateExtra) -> Self::CertificateExtra;
 }
 
 #[cfg(test)]

src/rust/cryptography-x509-verification/src/trust_store.rs

@@ -51,8 +51,10 @@ mod tests {
     #[test]
     fn test_store() {
         let cert_pem = v1_cert_pem();
-        let cert1 = VerificationCertificate::new(cert(&cert_pem), ());
-        let cert2 = VerificationCertificate::new(cert(&cert_pem), ());
+        let c1 = cert(&cert_pem);
+        let c2 = cert(&cert_pem);
+        let cert1 = VerificationCertificate::new(&c1, ());
+        let cert2 = VerificationCertificate::new(&c2, ());
         let store = Store::<'_, PublicKeyErrorOps>::new([cert1]);
 
         assert!(store.contains(&cert2));

src/rust/src/x509/verify.rs

@@ -46,6 +46,14 @@ impl CryptoOps for PyCryptoOps {
             )
         })
     }
+
+    fn clone_public_key(key: &Self::Key) -> Self::Key {
+        pyo3::Python::with_gil(|py| key.clone_ref(py))
+    }
+
+    fn clone_extra(extra: &Self::CertificateExtra) -> Self::CertificateExtra {
+        pyo3::Python::with_gil(|py| extra.clone_ref(py))
+    }
 }
 
 pyo3::create_exception!(
@@ -277,23 +285,14 @@ impl PyClientVerifier {
 
         let intermediates = intermediates
             .iter()
-            .map(|i| {
-                VerificationCertificate::new(
-                    i.get().raw.borrow_dependent().clone(),
-                    i.clone_ref(py),
-                )
-            })
+            .map(|i| VerificationCertificate::new(i.get().raw.borrow_dependent(), i.clone_ref(py)))
             .collect::<Vec<_>>();
-        let intermediate_refs = intermediates.iter().collect::<Vec<_>>();
 
-        let v = VerificationCertificate::new(
-            leaf.get().raw.borrow_dependent().clone(),
-            leaf.clone_ref(py),
-        );
+        let v = VerificationCertificate::new(leaf.get().raw.borrow_dependent(), leaf.clone_ref(py));
 
         let chain = cryptography_x509_verification::verify(
             &v,
-            &intermediate_refs,
+            &intermediates,
             policy,
             store.raw.borrow_dependent(),
         )
@@ -370,23 +369,14 @@ impl PyServerVerifier {
 
         let intermediates = intermediates
             .iter()
-            .map(|i| {
-                VerificationCertificate::new(
-                    i.get().raw.borrow_dependent().clone(),
-                    i.clone_ref(py),
-                )
-            })
+            .map(|i| VerificationCertificate::new(i.get().raw.borrow_dependent(), i.clone_ref(py)))
             .collect::<Vec<_>>();
-        let intermediate_refs = intermediates.iter().collect::<Vec<_>>();
 
-        let v = VerificationCertificate::new(
-            leaf.get().raw.borrow_dependent().clone(),
-            leaf.clone_ref(py),
-        );
+        let v = VerificationCertificate::new(leaf.get().raw.borrow_dependent(), leaf.clone_ref(py));
 
         let chain = cryptography_x509_verification::verify(
             &v,
-            &intermediate_refs,
+            &intermediates,
             policy,
             store.raw.borrow_dependent(),
         )
@@ -479,10 +469,7 @@ impl PyStore {
         Ok(Self {
             raw: RawPyStore::new(certs, |v| {
                 Store::new(v.iter().map(|t| {
-                    VerificationCertificate::new(
-                        t.get().raw.borrow_dependent().clone(),
-                        t.clone_ref(py),
-                    )
+                    VerificationCertificate::new(t.get().raw.borrow_dependent(), t.clone_ref(py))
                 }))
             }),
         })