Convert CSR creation to Rust (#6495)

* Convert CSR creation to Rust

* put this back

* unused

* coverage

10 files changed, 125 insertions, 168 deletions

diff --git a/docs/hazmat/backends/interfaces.rst b/docs/hazmat/backends/interfaces.rst
index b22c70e33..627fe7448 100644
--- a/docs/hazmat/backends/interfaces.rst
+++ b/docs/hazmat/backends/interfaces.rst
@@ -500,28 +500,6 @@ A specific ``backend`` may provide one or more of these interfaces.
 
     A backend with methods for working with X.509 objects.
 
-    .. method:: create_x509_csr(builder, private_key, algorithm)
-
-        .. versionadded:: 1.0
-
-        :param builder: An instance of
-            :class:`~cryptography.x509.CertificateSigningRequestBuilder`.
-
-        :param private_key: The
-            :class:`~cryptography.hazmat.primitives.asymmetric.rsa.RSAPrivateKey`,
-            :class:`~cryptography.hazmat.primitives.asymmetric.dsa.DSAPrivateKey` or
-            :class:`~cryptography.hazmat.primitives.asymmetric.ec.EllipticCurvePrivateKey`
-            that will be used to sign the request.  When the request is
-            signed by a certificate authority, the private key's associated
-            public key will be stored in the resulting certificate.
-
-        :param algorithm: The
-            :class:`~cryptography.hazmat.primitives.hashes.HashAlgorithm`
-            that will be used to generate the request signature.
-
-        :returns: A new instance of
-            :class:`~cryptography.x509.CertificateSigningRequest`.
-
     .. method:: create_x509_crl(builder, private_key, algorithm)
 
         .. versionadded:: 1.2
diff --git a/src/cryptography/hazmat/backends/interfaces.py b/src/cryptography/hazmat/backends/interfaces.py
index dce35ba6e..eaaf9a569 100644
--- a/src/cryptography/hazmat/backends/interfaces.py
+++ b/src/cryptography/hazmat/backends/interfaces.py
@@ -15,8 +15,6 @@ if typing.TYPE_CHECKING:
     from cryptography.x509.base import (
         CertificateRevocationList,
         CertificateRevocationListBuilder,
-        CertificateSigningRequest,
-        CertificateSigningRequestBuilder,
         RevokedCertificate,
         RevokedCertificateBuilder,
     )
@@ -278,17 +276,6 @@ class DERSerializationBackend(metaclass=abc.ABCMeta):
 
 class X509Backend(metaclass=abc.ABCMeta):
     @abc.abstractmethod
-    def create_x509_csr(
-        self,
-        builder: "CertificateSigningRequestBuilder",
-        private_key: "PRIVATE_KEY_TYPES",
-        algorithm: typing.Optional["hashes.HashAlgorithm"],
-    ) -> "CertificateSigningRequest":
-        """
-        Create and sign an X.509 CSR from a CSR builder object.
-        """
-
-    @abc.abstractmethod
     def create_x509_crl(
         self,
         builder: "CertificateRevocationListBuilder",
diff --git a/src/cryptography/hazmat/backends/openssl/backend.py b/src/cryptography/hazmat/backends/openssl/backend.py
index b781d12fb..055802d50 100644
--- a/src/cryptography/hazmat/backends/openssl/backend.py
+++ b/src/cryptography/hazmat/backends/openssl/backend.py
@@ -854,87 +854,6 @@ class Backend(BackendInterface):
                 "MD5 hash algorithm is only supported with RSA keys"
             )
 
-    def create_x509_csr(
-        self,
-        builder: x509.CertificateSigningRequestBuilder,
-        private_key: PRIVATE_KEY_TYPES,
-        algorithm: typing.Optional[hashes.HashAlgorithm],
-    ) -> x509.CertificateSigningRequest:
-        if not isinstance(builder, x509.CertificateSigningRequestBuilder):
-            raise TypeError("Builder type mismatch.")
-        self._x509_check_signature_params(private_key, algorithm)
-
-        # Resolve the signature algorithm.
-        evp_md = self._evp_md_x509_null_if_eddsa(private_key, algorithm)
-
-        # Create an empty request.
-        x509_req = self._lib.X509_REQ_new()
-        self.openssl_assert(x509_req != self._ffi.NULL)
-        x509_req = self._ffi.gc(x509_req, self._lib.X509_REQ_free)
-
-        # Set x509 version.
-        res = self._lib.X509_REQ_set_version(x509_req, x509.Version.v1.value)
-        self.openssl_assert(res == 1)
-
-        # Set subject name.
-        res = self._lib.X509_REQ_set_subject_name(
-            x509_req, _encode_name_gc(self, builder._subject_name)
-        )
-        self.openssl_assert(res == 1)
-
-        # Set subject public key.
-        public_key = private_key.public_key()
-        res = self._lib.X509_REQ_set_pubkey(
-            x509_req, public_key._evp_pkey  # type: ignore[union-attr]
-        )
-        self.openssl_assert(res == 1)
-
-        # Add extensions.
-        sk_extension = self._lib.sk_X509_EXTENSION_new_null()
-        self.openssl_assert(sk_extension != self._ffi.NULL)
-        sk_extension = self._ffi.gc(
-            sk_extension,
-            lambda x: self._lib.sk_X509_EXTENSION_pop_free(
-                x,
-                self._ffi.addressof(
-                    self._lib._original_lib, "X509_EXTENSION_free"
-                ),
-            ),
-        )
-        # Don't GC individual extensions because the memory is owned by
-        # sk_extensions and will be freed along with it.
-        self._create_x509_extensions(
-            extensions=builder._extensions,
-            rust_handler=rust_x509.encode_csr_extension,
-            x509_obj=sk_extension,
-            add_func=self._lib.sk_X509_EXTENSION_insert,
-            gc=False,
-        )
-        res = self._lib.X509_REQ_add_extensions(x509_req, sk_extension)
-        self.openssl_assert(res == 1)
-
-        # Add attributes (all bytes encoded as ASN1 UTF8_STRING)
-        for attr_oid, attr_val in builder._attributes:
-            obj = _txt2obj_gc(self, attr_oid.dotted_string)
-            res = self._lib.X509_REQ_add1_attr_by_OBJ(
-                x509_req,
-                obj,
-                x509.name._ASN1Type.UTF8String.value,
-                attr_val,
-                len(attr_val),
-            )
-            self.openssl_assert(res == 1)
-
-        # Sign the request using the requester's private key.
-        res = self._lib.X509_REQ_sign(
-            x509_req, private_key._evp_pkey, evp_md  # type: ignore[union-attr]
-        )
-        if res == 0:
-            errors = self._consume_errors_with_text()
-            raise ValueError("Signing failed", errors)
-
-        return self._ossl2csr(x509_req)
-
     def _evp_md_x509_null_if_eddsa(self, private_key, algorithm):
         if isinstance(
             private_key, (ed25519.Ed25519PrivateKey, ed448.Ed448PrivateKey)
@@ -999,7 +918,6 @@ class Backend(BackendInterface):
             rust_handler=rust_x509.encode_crl_extension,
             x509_obj=x509_crl,
             add_func=self._lib.X509_CRL_add_ext,
-            gc=True,
         )
 
         # add revoked certificates
@@ -1024,7 +942,6 @@ class Backend(BackendInterface):
                 rust_handler=rust_x509.encode_crl_entry_extension,
                 x509_obj=x509_revoked,
                 add_func=self._lib.X509_REVOKED_add_ext,
-                gc=True,
             )
 
             res = self._lib.X509_CRL_add0_revoked(x509_crl, x509_revoked)
@@ -1040,7 +957,7 @@ class Backend(BackendInterface):
         return self._ossl2crl(x509_crl)
 
     def _create_x509_extensions(
-        self, extensions, rust_handler, x509_obj, add_func, gc
+        self, extensions, rust_handler, x509_obj, add_func
     ):
         for i, extension in enumerate(extensions):
             x509_extension = self._create_x509_extension(
@@ -1048,10 +965,9 @@ class Backend(BackendInterface):
             )
             self.openssl_assert(x509_extension != self._ffi.NULL)
 
-            if gc:
-                x509_extension = self._ffi.gc(
-                    x509_extension, self._lib.X509_EXTENSION_free
-                )
+            x509_extension = self._ffi.gc(
+                x509_extension, self._lib.X509_EXTENSION_free
+            )
             res = add_func(x509_obj, x509_extension, i)
             self.openssl_assert(res >= 1)
 
diff --git a/src/cryptography/hazmat/bindings/_rust/x509.pyi b/src/cryptography/hazmat/bindings/_rust/x509.pyi
index d7e07dd4d..86f4f6df2 100644
--- a/src/cryptography/hazmat/bindings/_rust/x509.pyi
+++ b/src/cryptography/hazmat/bindings/_rust/x509.pyi
@@ -11,7 +11,6 @@ def load_pem_x509_crl(data: bytes) -> x509.CertificateRevocationList: ...
 def load_der_x509_crl(data: bytes) -> x509.CertificateRevocationList: ...
 def load_pem_x509_csr(data: bytes) -> x509.CertificateSigningRequest: ...
 def load_der_x509_csr(data: bytes) -> x509.CertificateSigningRequest: ...
-def encode_csr_extension(ext: x509.Extension) -> bytes: ...
 def encode_crl_extension(ext: x509.Extension) -> bytes: ...
 def encode_crl_entry_extension(ext: x509.Extension) -> bytes: ...
 def encode_name_bytes(name: x509.Name) -> bytes: ...
@@ -20,6 +19,11 @@ def create_x509_certificate(
     private_key: PRIVATE_KEY_TYPES,
     hash_algorithm: typing.Optional[hashes.HashAlgorithm],
 ) -> x509.Certificate: ...
+def create_x509_csr(
+    builder: x509.CertificateSigningRequestBuilder,
+    private_key: PRIVATE_KEY_TYPES,
+    hash_algorithm: typing.Optional[hashes.HashAlgorithm],
+) -> x509.CertificateSigningRequest: ...
 
 class Sct: ...
 class Certificate: ...
diff --git a/src/cryptography/x509/base.py b/src/cryptography/x509/base.py
index 4f7efc414..361f014f4 100644
--- a/src/cryptography/x509/base.py
+++ b/src/cryptography/x509/base.py
@@ -544,10 +544,9 @@ class CertificateSigningRequestBuilder(object):
         """
         Signs the request using the requestor's private key.
         """
-        backend = _get_backend(backend)
         if self._subject_name is None:
             raise ValueError("A CertificateSigningRequest must have a subject")
-        return backend.create_x509_csr(self, private_key, algorithm)
+        return rust_x509.create_x509_csr(self, private_key, algorithm)
 
 
 class CertificateBuilder(object):
diff --git a/src/rust/src/x509/certificate.rs b/src/rust/src/x509/certificate.rs
index 0466815f7..7557d951a 100644
--- a/src/rust/src/x509/certificate.rs
+++ b/src/rust/src/x509/certificate.rs
@@ -1059,7 +1059,7 @@ fn set_bit(vals: &mut [u8], n: usize, set: bool) {
     }
 }
 
-fn encode_certificate_extension(
+pub(crate) fn encode_certificate_extension(
     oid: &asn1::ObjectIdentifier<'_>,
     ext: &pyo3::PyAny,
 ) -> pyo3::PyResult<Option<Vec<u8>>> {
@@ -1249,30 +1249,11 @@ fn encode_certificate_extension(
     }
 }
 
-#[pyo3::prelude::pyfunction]
-fn encode_csr_extension(ext: &pyo3::PyAny) -> pyo3::PyResult<&pyo3::PyAny> {
-    let oid = asn1::ObjectIdentifier::from_string(
-        ext.getattr("oid")?
-            .getattr("dotted_string")?
-            .extract::<&str>()?,
-    )
-    .unwrap();
-    match encode_certificate_extension(&oid, ext.getattr("value")?)? {
-        Some(val) => Ok(pyo3::types::PyBytes::new(ext.py(), &val)),
-        None => Err(pyo3::exceptions::PyNotImplementedError::new_err(format!(
-            "Extension not supported: {}",
-            oid
-        ))),
-    }
-}
-
 pub(crate) fn add_to_module(module: &pyo3::prelude::PyModule) -> pyo3::PyResult<()> {
     module.add_wrapped(pyo3::wrap_pyfunction!(load_der_x509_certificate))?;
     module.add_wrapped(pyo3::wrap_pyfunction!(load_pem_x509_certificate))?;
     module.add_wrapped(pyo3::wrap_pyfunction!(create_x509_certificate))?;
 
-    module.add_wrapped(pyo3::wrap_pyfunction!(encode_csr_extension))?;
-
     module.add_class::<Certificate>()?;
 
     Ok(())
diff --git a/src/rust/src/x509/common.rs b/src/rust/src/x509/common.rs
index 8cb4cf345..ef1fd84a8 100644
--- a/src/rust/src/x509/common.rs
+++ b/src/rust/src/x509/common.rs
@@ -58,7 +58,7 @@ pub(crate) struct RawTlv<'a> {
 }
 
 impl<'a> RawTlv<'a> {
-    fn new(tag: u8, value: &'a [u8]) -> Self {
+    pub(crate) fn new(tag: u8, value: &'a [u8]) -> Self {
         RawTlv { tag, value }
     }
 
diff --git a/src/rust/src/x509/csr.rs b/src/rust/src/x509/csr.rs
index b978ecc25..769a80061 100644
--- a/src/rust/src/x509/csr.rs
+++ b/src/rust/src/x509/csr.rs
@@ -25,15 +25,23 @@ struct RawCsr<'a> {
 struct CertificationRequestInfo<'a> {
     version: u8,
     subject: x509::Name<'a>,
-    spki: asn1::Sequence<'a>,
+    spki: certificate::SubjectPublicKeyInfo<'a>,
     #[implicit(0, required)]
-    attributes: asn1::SetOf<'a, Attribute<'a>>,
+    attributes: x509::Asn1ReadableOrWritable<
+        'a,
+        asn1::SetOf<'a, Attribute<'a>>,
+        asn1::SetOfWriter<'a, Attribute<'a>, Vec<Attribute<'a>>>,
+    >,
 }
 
 #[derive(asn1::Asn1Read, asn1::Asn1Write)]
 struct Attribute<'a> {
     type_id: asn1::ObjectIdentifier<'a>,
-    values: asn1::SetOf<'a, asn1::Tlv<'a>>,
+    values: x509::Asn1ReadableOrWritable<
+        'a,
+        asn1::SetOf<'a, asn1::Tlv<'a>>,
+        asn1::SetOfWriter<'a, x509::common::RawTlv<'a>, [x509::common::RawTlv<'a>; 1]>,
+    >,
 }
 
 fn check_attribute_length<'a>(values: asn1::SetOf<'a, asn1::Tlv<'a>>) -> Result<(), PyAsn1Error> {
@@ -48,11 +56,11 @@ fn check_attribute_length<'a>(values: asn1::SetOf<'a, asn1::Tlv<'a>>) -> Result<
 
 impl CertificationRequestInfo<'_> {
     fn get_extension_attribute<'a>(&'a self) -> Result<Option<x509::Extensions<'a>>, PyAsn1Error> {
-        for attribute in self.attributes.clone() {
+        for attribute in self.attributes.unwrap_read().clone() {
             if attribute.type_id == *EXTENSION_REQUEST || attribute.type_id == *MS_EXTENSION_REQUEST
             {
-                check_attribute_length(attribute.values.clone())?;
-                let val = attribute.values.clone().next().unwrap();
+                check_attribute_length(attribute.values.unwrap_read().clone())?;
+                let val = attribute.values.unwrap_read().clone().next().unwrap();
                 let exts = asn1::parse_single::<x509::Extensions<'a>>(val.full_data())?;
                 return Ok(Some(exts));
             }
@@ -202,10 +210,17 @@ impl CertificateSigningRequest {
     ) -> pyo3::PyResult<&'p pyo3::PyAny> {
         let oid_str = oid.getattr("dotted_string")?.extract::<&str>()?;
         let rust_oid = asn1::ObjectIdentifier::from_string(oid_str).unwrap();
-        for attribute in self.raw.borrow_value().csr_info.attributes.clone() {
+        for attribute in self
+            .raw
+            .borrow_value()
+            .csr_info
+            .attributes
+            .unwrap_read()
+            .clone()
+        {
             if rust_oid == attribute.type_id {
-                check_attribute_length(attribute.values.clone())?;
-                let val = attribute.values.clone().next().unwrap();
+                check_attribute_length(attribute.values.unwrap_read().clone())?;
+                let val = attribute.values.unwrap_read().clone().next().unwrap();
                 // We allow utf8string, printablestring, and ia5string at this time
                 if val.tag() == asn1::Utf8String::TAG
                     || val.tag() == asn1::PrintableString::TAG
@@ -296,9 +311,84 @@ fn load_der_x509_csr(
     })
 }
 
+#[pyo3::prelude::pyfunction]
+fn create_x509_csr(
+    py: pyo3::Python<'_>,
+    builder: &pyo3::PyAny,
+    private_key: &pyo3::PyAny,
+    hash_algorithm: &pyo3::PyAny,
+) -> PyAsn1Result<CertificateSigningRequest> {
+    let sigalg = x509::sign::compute_signature_algorithm(py, private_key, hash_algorithm)?;
+    let serialization_mod = py.import("cryptography.hazmat.primitives.serialization")?;
+    let der_encoding = serialization_mod.getattr("Encoding")?.getattr("DER")?;
+    let spki_format = serialization_mod
+        .getattr("PublicFormat")?
+        .getattr("SubjectPublicKeyInfo")?;
+
+    let spki_bytes = private_key
+        .call_method0("public_key")?
+        .call_method1("public_bytes", (der_encoding, spki_format))?
+        .extract::<&[u8]>()?;
+
+    let mut attrs = vec![];
+    let ext_bytes;
+    if let Some(exts) = x509::common::encode_extensions(
+        py,
+        builder.getattr("_extensions")?,
+        x509::certificate::encode_certificate_extension,
+    )? {
+        ext_bytes = asn1::write_single(&exts);
+        attrs.push(Attribute {
+            type_id: (*EXTENSION_REQUEST).clone(),
+            values: x509::Asn1ReadableOrWritable::new_write(asn1::SetOfWriter::new([
+                asn1::parse_single(&ext_bytes)?,
+            ])),
+        })
+    }
+
+    for py_attr in builder.getattr("_attributes")?.iter()? {
+        let (py_oid, value): (&pyo3::PyAny, &[u8]) = py_attr?.extract()?;
+        let oid = asn1::ObjectIdentifier::from_string(
+            py_oid.getattr("dotted_string")?.extract::<&str>()?,
+        )
+        .unwrap();
+        let tag = if std::str::from_utf8(value).is_ok() {
+            asn1::Utf8String::TAG
+        } else {
+            return Err(PyAsn1Error::from(pyo3::exceptions::PyValueError::new_err(
+                "Attribute values must be valid utf-8.",
+            )));
+        };
+        attrs.push(Attribute {
+            type_id: oid,
+            values: x509::Asn1ReadableOrWritable::new_write(asn1::SetOfWriter::new([
+                x509::common::RawTlv::new(tag, value),
+            ])),
+        })
+    }
+
+    let csr_info = CertificationRequestInfo {
+        version: 0,
+        subject: x509::common::encode_name(py, builder.getattr("_subject_name")?)?,
+        spki: asn1::parse_single(spki_bytes)?,
+        attributes: x509::Asn1ReadableOrWritable::new_write(asn1::SetOfWriter::new(attrs)),
+    };
+
+    let tbs_bytes = asn1::write_single(&csr_info);
+    let signature = x509::sign::sign_data(py, private_key, hash_algorithm, &tbs_bytes)?;
+    let data = asn1::write_single(&RawCsr {
+        csr_info,
+        signature_alg: sigalg,
+        signature: asn1::BitString::new(signature, 0).unwrap(),
+    });
+    // TODO: extra copy as we round-trip through a slice
+    load_der_x509_csr(py, &data)
+}
+
 pub(crate) fn add_to_module(module: &pyo3::prelude::PyModule) -> pyo3::PyResult<()> {
     module.add_wrapped(pyo3::wrap_pyfunction!(load_der_x509_csr))?;
     module.add_wrapped(pyo3::wrap_pyfunction!(load_pem_x509_csr))?;
+    module.add_wrapped(pyo3::wrap_pyfunction!(create_x509_csr))?;
 
     module.add_class::<CertificateSigningRequest>()?;
 
diff --git a/tests/hazmat/backends/test_openssl.py b/tests/hazmat/backends/test_openssl.py
index f2233329d..234c3b052 100644
--- a/tests/hazmat/backends/test_openssl.py
+++ b/tests/hazmat/backends/test_openssl.py
@@ -472,18 +472,6 @@ class TestOpenSSLCMAC(object):
             backend.create_cmac_ctx(DummyCipherAlgorithm())
 
 
-class TestOpenSSLSignX509CSR(object):
-    def test_requires_csr_builder(self):
-        private_key = RSA_KEY_2048.private_key(backend)
-
-        with pytest.raises(TypeError):
-            backend.create_x509_csr(
-                object(),  # type: ignore[arg-type]
-                private_key,
-                DummyHashAlgorithm(),
-            )
-
-
 class TestOpenSSLSignX509CertificateRevocationList(object):
     def test_invalid_builder(self):
         private_key = RSA_KEY_2048.private_key(backend)
diff --git a/tests/x509/test_x509.py b/tests/x509/test_x509.py
index 5d0b4cb3e..5d6c7d921 100644
--- a/tests/x509/test_x509.py
+++ b/tests/x509/test_x509.py
@@ -4046,6 +4046,20 @@ class TestCertificateSigningRequestBuilder(object):
             == locality
         )
 
+    def test_add_attributes_non_utf8(self, backend):
+        _skip_curve_unsupported(backend, ec.SECP256R1())
+        private_key = ec.generate_private_key(ec.SECP256R1(), backend)
+        builder = (
+            x509.CertificateSigningRequestBuilder()
+            .subject_name(x509.Name([]))
+            .add_attribute(
+                x509.oid.AttributeOID.CHALLENGE_PASSWORD,
+                b"\xbb\xad\x16\x9a\xac\xc9\x03i\xec\xcc\xdd6\xcbh\xfc\xf3",
+            )
+        )
+        with pytest.raises(ValueError):
+            builder.sign(private_key, hashes.SHA256(), backend)
+
     def test_add_attribute_bad_types(self, backend):
         request = x509.CertificateSigningRequestBuilder()
         with pytest.raises(TypeError):