From 324eb6f8ea85f8134a659983fe42f067aa18e0c3 Mon Sep 17 00:00:00 2001
From: William Woodruff <william@trailofbits.com>
Date: Thu, 14 Sep 2023 15:50:47 -0400
Subject: [PATCH] rust: add PyCryptoOps (#9606)

* rust: add PyCryptoOps

Reimplements `verify_directly_issued_by` in terms of `PyCryptoOps`,
for free coverage.

Signed-off-by: William Woodruff <william@trailofbits.com>

* rust: is_signed_by -> verify_signed_by

Signed-off-by: William Woodruff <william@trailofbits.com>

---------

Signed-off-by: William Woodruff <william@trailofbits.com>
---
 src/rust/Cargo.lock                           |  1 +
 src/rust/Cargo.toml                           |  1 +
 .../cryptography-x509-validation/src/ops.rs   |  2 +-
 src/rust/src/x509/certificate.rs              | 14 +++----
 src/rust/src/x509/verify.rs                   | 41 ++++++++++++++++++-
 5 files changed, 49 insertions(+), 10 deletions(-)

diff --git a/src/rust/Cargo.lock b/src/rust/Cargo.lock
index efb128ee8985..74419631a5cf 100644
--- a/src/rust/Cargo.lock
+++ b/src/rust/Cargo.lock
@@ -90,6 +90,7 @@ dependencies = [
  "cryptography-cffi",
  "cryptography-openssl",
  "cryptography-x509",
+ "cryptography-x509-validation",
  "foreign-types-shared",
  "once_cell",
  "openssl",
diff --git a/src/rust/Cargo.toml b/src/rust/Cargo.toml
index 9d41d805fc16..6a30b6afbf59 100644
--- a/src/rust/Cargo.toml
+++ b/src/rust/Cargo.toml
@@ -14,6 +14,7 @@ pyo3 = { version = "0.19", features = ["abi3-py37"] }
 asn1 = { version = "0.15.5", default-features = false }
 cryptography-cffi = { path = "cryptography-cffi" }
 cryptography-x509 = { path = "cryptography-x509" }
+cryptography-x509-validation = { path = "cryptography-x509-validation" }
 cryptography-openssl = { path = "cryptography-openssl" }
 pem = { version = "3", default-features = false }
 openssl = "0.10.57"
diff --git a/src/rust/cryptography-x509-validation/src/ops.rs b/src/rust/cryptography-x509-validation/src/ops.rs
index faacca5c47a8..47e3f2cd07ef 100644
--- a/src/rust/cryptography-x509-validation/src/ops.rs
+++ b/src/rust/cryptography-x509-validation/src/ops.rs
@@ -18,5 +18,5 @@ pub trait CryptoOps {
 
     /// Verifies the signature on `Certificate` using the given
     /// `Key`.
-    fn is_signed_by(&self, cert: &Certificate<'_>, key: Self::Key) -> Result<(), Self::Err>;
+    fn verify_signed_by(&self, cert: &Certificate<'_>, key: Self::Key) -> Result<(), Self::Err>;
 }
diff --git a/src/rust/src/x509/certificate.rs b/src/rust/src/x509/certificate.rs
index 5ebd7a24e002..3ed8f55cf848 100644
--- a/src/rust/src/x509/certificate.rs
+++ b/src/rust/src/x509/certificate.rs
@@ -7,6 +7,7 @@ use crate::asn1::{
 };
 use crate::backend::hashes;
 use crate::error::{CryptographyError, CryptographyResult};
+use crate::x509::verify::PyCryptoOps;
 use crate::x509::{extensions, sct, sign};
 use crate::{exceptions, types, x509};
 use cryptography_x509::certificate::Certificate as RawCertificate;
@@ -20,6 +21,7 @@ use cryptography_x509::extensions::{
 };
 use cryptography_x509::extensions::{Extension, SubjectAlternativeName};
 use cryptography_x509::{common, oid};
+use cryptography_x509_validation::ops::CryptoOps;
 use pyo3::{IntoPy, ToPyObject};
 use std::collections::hash_map::DefaultHasher;
 use std::hash::{Hash, Hasher};
@@ -267,7 +269,6 @@ impl Certificate {
 
     fn verify_directly_issued_by(
         &self,
-        py: pyo3::Python<'_>,
         issuer: pyo3::PyRef<'_, Certificate>,
     ) -> CryptographyResult<()> {
         if self.raw.borrow_dependent().tbs_cert.signature_alg
@@ -286,13 +287,10 @@ impl Certificate {
                 ),
             ));
         };
-        sign::verify_signature_with_signature_algorithm(
-            py,
-            issuer.public_key(py)?,
-            &self.raw.borrow_dependent().signature_alg,
-            self.raw.borrow_dependent().signature.as_bytes(),
-            &asn1::write_single(&self.raw.borrow_dependent().tbs_cert)?,
-        )
+
+        let ops = PyCryptoOps {};
+        let issuer_key = ops.public_key(issuer.raw.borrow_dependent())?;
+        ops.verify_signed_by(self.raw.borrow_dependent(), issuer_key)
     }
 }
 
diff --git a/src/rust/src/x509/verify.rs b/src/rust/src/x509/verify.rs
index aef4d6a1c3ce..a0e221660641 100644
--- a/src/rust/src/x509/verify.rs
+++ b/src/rust/src/x509/verify.rs
@@ -2,7 +2,46 @@
 // 2.0, and the BSD License. See the LICENSE file in the root of this repository
 // for complete details.
 
-use crate::x509::certificate::Certificate as PyCertificate;
+use cryptography_x509::certificate::Certificate;
+use cryptography_x509_validation::ops::CryptoOps;
+
+use crate::x509::sign;
+use crate::{
+    error::{CryptographyError, CryptographyResult},
+    types,
+    x509::certificate::Certificate as PyCertificate,
+};
+
+pub(crate) struct PyCryptoOps {}
+
+impl CryptoOps for PyCryptoOps {
+    type Key = pyo3::Py<pyo3::PyAny>;
+    type Err = CryptographyError;
+
+    fn public_key(&self, cert: &Certificate<'_>) -> Result<Self::Key, Self::Err> {
+        pyo3::Python::with_gil(|py| -> Result<Self::Key, Self::Err> {
+            // This makes an unnecessary copy. It'd be nice to get rid of it.
+            let spki_der = pyo3::types::PyBytes::new(py, &asn1::write_single(&cert.tbs_cert.spki)?);
+
+            Ok(types::LOAD_DER_PUBLIC_KEY
+                .get(py)?
+                .call1((spki_der,))?
+                .into())
+        })
+    }
+
+    fn verify_signed_by(&self, cert: &Certificate<'_>, key: Self::Key) -> Result<(), Self::Err> {
+        pyo3::Python::with_gil(|py| -> CryptographyResult<()> {
+            sign::verify_signature_with_signature_algorithm(
+                py,
+                key.as_ref(py),
+                &cert.signature_alg,
+                cert.signature.as_bytes(),
+                &asn1::write_single(&cert.tbs_cert)?,
+            )
+        })
+    }
+}
 
 #[pyo3::pyclass(
     frozen,