Fix lifetime errors in asn1.rs with gil-refs disabled (pyca#10778)

* Fix lifetime errors in `asn1.rs` with `gil-refs` disabled * Fix docstring and remove unneeded KeepAlive * Address review comments * Update src/rust/src/x509/ocsp_req.rs Co-authored-by: Alex Gaynor <[email protected]> --------- Co-authored-by: Alex Gaynor <[email protected]>
trail-of-forks · Apr 13, 2024 · bdb2d8b · bdb2d8b
1 parent 6fd5d73
commit bdb2d8b
Show file tree

Hide file tree

Showing 8 changed files with 31 additions and 12 deletions.
diff --git a/src/rust/Cargo.lock b/src/rust/Cargo.lock
diff --git a/src/rust/cryptography-keepalive/Cargo.toml b/src/rust/cryptography-keepalive/Cargo.toml
@@ -8,3 +8,4 @@ publish = false
 rust-version = "1.65.0"
 
 [dependencies]
+pyo3 = { version = "0.21.1", features = ["abi3"] }
diff --git a/src/rust/cryptography-keepalive/src/lib.rs b/src/rust/cryptography-keepalive/src/lib.rs
@@ -4,6 +4,7 @@
 
 #![deny(rust_2018_idioms, clippy::undocumented_unsafe_blocks)]
 
+use pyo3::pybacked::PyBackedBytes;
 use std::cell::UnsafeCell;
 use std::ops::Deref;
 
@@ -19,6 +20,10 @@ pub unsafe trait StableDeref: Deref {}
 // slice returned by `deref` remains valid.
 unsafe impl<T> StableDeref for Vec<T> {}
 
+// SAFETY: `PyBackedBytes`'s data is on the heap and `bytes` objects in
+// Python are immutable.
+unsafe impl StableDeref for PyBackedBytes {}
+
 #[allow(clippy::new_without_default)]
 impl<T: StableDeref> KeepAlive<T> {
     pub fn new() -> Self {

diff --git a/src/rust/src/asn1.rs b/src/rust/src/asn1.rs
@@ -8,6 +8,7 @@ use cryptography_x509::common::{DssSignature, SubjectPublicKeyInfo, Time};
 use cryptography_x509::name::Name;
 use pyo3::prelude::PyAnyMethods;
 use pyo3::prelude::PyModuleMethods;
+use pyo3::pybacked::PyBackedBytes;
 use pyo3::types::IntoPyDict;
 use pyo3::ToPyObject;
 
@@ -73,7 +74,7 @@ fn decode_dss_signature(
 pub(crate) fn py_uint_to_big_endian_bytes<'p>(
     py: pyo3::Python<'p>,
     v: pyo3::Bound<'p, pyo3::types::PyLong>,
-) -> pyo3::PyResult<&'p [u8]> {
+) -> pyo3::PyResult<PyBackedBytes> {
     let zero = (0).to_object(py);
     if v.lt(zero)? {
         return Err(pyo3::exceptions::PyValueError::new_err(
@@ -124,9 +125,11 @@ fn encode_dss_signature<'p>(
     r: pyo3::Bound<'_, pyo3::types::PyLong>,
     s: pyo3::Bound<'_, pyo3::types::PyLong>,
 ) -> CryptographyResult<pyo3::Bound<'p, pyo3::types::PyBytes>> {
+    let r_bytes = py_uint_to_big_endian_bytes(py, r)?;
+    let s_bytes = py_uint_to_big_endian_bytes(py, s)?;
     let sig = DssSignature {
-        r: asn1::BigUint::new(py_uint_to_big_endian_bytes(py, r)?).unwrap(),
-        s: asn1::BigUint::new(py_uint_to_big_endian_bytes(py, s)?).unwrap(),
+        r: asn1::BigUint::new(&r_bytes).unwrap(),
+        s: asn1::BigUint::new(&s_bytes).unwrap(),
     };
     let result = asn1::write_single(&sig)?;
     Ok(pyo3::types::PyBytes::new_bound(py, &result))

diff --git a/src/rust/src/x509/certificate.rs b/src/rust/src/x509/certificate.rs
@@ -918,12 +918,13 @@ fn create_x509_certificate(
     let py_not_before = builder.getattr(pyo3::intern!(py, "_not_valid_before"))?;
     let py_not_after = builder.getattr(pyo3::intern!(py, "_not_valid_after"))?;
 
+    let serial_bytes = py_uint_to_big_endian_bytes(py, py_serial)?;
     let tbs_cert = cryptography_x509::certificate::TbsCertificate {
         version: builder
             .getattr(pyo3::intern!(py, "_version"))?
             .getattr(pyo3::intern!(py, "value"))?
             .extract()?,
-        serial: asn1::BigInt::new(py_uint_to_big_endian_bytes(py, py_serial)?).unwrap(),
+        serial: asn1::BigInt::new(&serial_bytes).unwrap(),
         signature_alg: sigalg.clone(),
         issuer: x509::common::encode_name(py, &py_issuer_name)?,
         validity: cryptography_x509::certificate::Validity {

diff --git a/src/rust/src/x509/crl.rs b/src/rust/src/x509/crl.rs
@@ -655,6 +655,7 @@ fn create_x509_crl(
         rsa_padding.to_owned(),
     )?;
     let mut revoked_certs = vec![];
+    let ka = cryptography_keepalive::KeepAlive::new();
     for py_revoked_cert in builder
         .getattr(pyo3::intern!(py, "_revoked_certificates"))?
         .iter()?
@@ -665,9 +666,9 @@ fn create_x509_crl(
             .extract()?;
         let py_revocation_date =
             py_revoked_cert.getattr(pyo3::intern!(py, "revocation_date_utc"))?;
+        let serial_bytes = ka.add(py_uint_to_big_endian_bytes(py, serial_number)?);
         revoked_certs.push(crl::RevokedCertificate {
-            user_certificate: asn1::BigUint::new(py_uint_to_big_endian_bytes(py, serial_number)?)
-                .unwrap(),
+            user_certificate: asn1::BigUint::new(serial_bytes).unwrap(),
             revocation_date: x509::certificate::time_from_py(py, &py_revocation_date)?,
             raw_crl_entry_extensions: x509::common::encode_extensions(
                 py,

diff --git a/src/rust/src/x509/extensions.rs b/src/rust/src/x509/extensions.rs
@@ -51,10 +51,11 @@ pub(crate) fn encode_authority_key_identifier<'a>(
     } else {
         None
     };
+    let serial_bytes;
     let authority_cert_serial_number =
         if let Some(authority_cert_serial_number) = aki.authority_cert_serial_number {
-            let serial_bytes = py_uint_to_big_endian_bytes(py, authority_cert_serial_number)?;
-            Some(asn1::BigUint::new(serial_bytes).unwrap())
+            serial_bytes = py_uint_to_big_endian_bytes(py, authority_cert_serial_number)?;
+            Some(asn1::BigUint::new(&serial_bytes).unwrap())
         } else {
             None
         };
@@ -215,6 +216,7 @@ fn encode_certificate_policies(
     ext: &pyo3::Bound<'_, pyo3::PyAny>,
 ) -> CryptographyResult<Vec<u8>> {
     let mut policy_informations = vec![];
+    let ka = cryptography_keepalive::KeepAlive::new();
     for py_policy_info in ext.iter()? {
         let py_policy_info = py_policy_info?;
         let py_policy_qualifiers =
@@ -245,7 +247,8 @@ fn encode_certificate_policies(
                             .getattr(pyo3::intern!(py, "notice_numbers"))?
                             .iter()?
                         {
-                            let bytes = py_uint_to_big_endian_bytes(ext.py(), py_num?.extract()?)?;
+                            let bytes =
+                                ka.add(py_uint_to_big_endian_bytes(ext.py(), py_num?.extract()?)?);
                             notice_numbers.push(asn1::BigUint::new(bytes).unwrap());
                         }
 
@@ -454,7 +457,7 @@ pub(crate) fn encode_extension(
                 .clone();
             let bytes = py_uint_to_big_endian_bytes(ext.py(), intval)?;
             Ok(Some(asn1::write_single(
-                &asn1::BigUint::new(bytes).unwrap(),
+                &asn1::BigUint::new(&bytes).unwrap(),
             )?))
         }
         &oid::ISSUER_ALTERNATIVE_NAME_OID | &oid::SUBJECT_ALTERNATIVE_NAME_OID => {
@@ -503,7 +506,7 @@ pub(crate) fn encode_extension(
                 .clone();
             let bytes = py_uint_to_big_endian_bytes(ext.py(), intval)?;
             Ok(Some(asn1::write_single(
-                &asn1::BigUint::new(bytes).unwrap(),
+                &asn1::BigUint::new(&bytes).unwrap(),
             )?))
         }
         &oid::ISSUING_DISTRIBUTION_POINT_OID => {

diff --git a/src/rust/src/x509/ocsp_req.rs b/src/rust/src/x509/ocsp_req.rs
@@ -171,6 +171,7 @@ fn create_ocsp_request(
     builder: &pyo3::Bound<'_, pyo3::PyAny>,
 ) -> CryptographyResult<OCSPRequest> {
     let builder_request = builder.getattr(pyo3::intern!(py, "_request"))?;
+    let serial_number_bytes;
 
     // Declare outside the if-block so the lifetimes are right.
     let (py_cert, py_issuer, py_hash, issuer_name_hash, issuer_key_hash): (
@@ -188,7 +189,8 @@ fn create_ocsp_request(
         (issuer_name_hash, issuer_key_hash, py_serial, py_hash) = builder
             .getattr(pyo3::intern!(py, "_request_hash"))?
             .extract()?;
-        let serial_number = asn1::BigInt::new(py_uint_to_big_endian_bytes(py, py_serial)?).unwrap();
+        serial_number_bytes = py_uint_to_big_endian_bytes(py, py_serial)?;
+        let serial_number = asn1::BigInt::new(&serial_number_bytes).unwrap();
         ocsp::certid_new_from_hash(
             py,
             &issuer_name_hash,