fixes #6804 -- improve error message quality with invalid characters in name attributes (#6843)

docs/development/test-vectors.rst

@@ -282,6 +282,8 @@ Custom X.509 Vectors
 * ``utf8_common_name.pem`` - An RSA 2048 bit self-signed CA certificate
   generated using OpenSSL that contains a UTF8String common name with the value
   "We heart UTF8!™".
+* ``invalid_utf8_common_name.pem`` - A certificate that contains a
+  ``UTF8String`` common name with an invalid UTF-8 byte sequence.
 * ``two_basic_constraints.pem`` - An RSA 2048 bit self-signed certificate
   containing two basic constraints extensions.
 * ``basic_constraints_not_critical.pem`` - An RSA 2048 bit self-signed

src/rust/src/asn1.rs

@@ -45,6 +45,15 @@ impl From<PyAsn1Error> for pyo3::PyErr {
     }
 }
 
+impl PyAsn1Error {
+    pub(crate) fn add_location(self, loc: asn1::ParseLocation) -> Self {
+        match self {
+            PyAsn1Error::Py(e) => PyAsn1Error::Py(e),
+            PyAsn1Error::Asn1(e) => PyAsn1Error::Asn1(e.add_location(loc)),
+        }
+    }
+}
+
 // The primary purpose of this alias is for brevity to keep function signatures
 // to a single-line as a work around for coverage issues. See
 // https://github.com/pyca/cryptography/pull/6173
@@ -218,3 +227,14 @@ pub(crate) fn create_submodule(py: pyo3::Python<'_>) -> pyo3::PyResult<&pyo3::pr
 
     Ok(submod)
 }
+
+#[cfg(test)]
+mod tests {
+    use super::PyAsn1Error;
+
+    #[test]
+    fn test_pyasn1error_add_location() {
+        let py_err = pyo3::PyErr::new::<pyo3::exceptions::PyValueError, _>("Error!");
+        PyAsn1Error::Py(py_err).add_location(asn1::ParseLocation::Field("meh"));
+    }
+}

src/rust/src/x509/certificate.rs

@@ -193,12 +193,18 @@ impl Certificate {
 
     #[getter]
     fn issuer<'p>(&self, py: pyo3::Python<'p>) -> pyo3::PyResult<&'p pyo3::PyAny> {
-        x509::parse_name(py, &self.raw.borrow_value().tbs_cert.issuer)
+        Ok(
+            x509::parse_name(py, &self.raw.borrow_value().tbs_cert.issuer)
+                .map_err(|e| e.add_location(asn1::ParseLocation::Field("issuer")))?,
+        )
     }
 
     #[getter]
     fn subject<'p>(&self, py: pyo3::Python<'p>) -> pyo3::PyResult<&'p pyo3::PyAny> {
-        x509::parse_name(py, &self.raw.borrow_value().tbs_cert.subject)
+        Ok(
+            x509::parse_name(py, &self.raw.borrow_value().tbs_cert.subject)
+                .map_err(|e| e.add_location(asn1::ParseLocation::Field("subject")))?,
+        )
     }
 
     #[getter]

src/rust/src/x509/common.rs

@@ -328,14 +328,14 @@ pub(crate) struct Extension<'a> {
 pub(crate) fn parse_name<'p>(
     py: pyo3::Python<'p>,
     name: &Name<'_>,
-) -> pyo3::PyResult<&'p pyo3::PyAny> {
+) -> Result<&'p pyo3::PyAny, PyAsn1Error> {
     let x509_module = py.import("cryptography.x509")?;
     let py_rdns = pyo3::types::PyList::empty(py);
     for rdn in name.unwrap_read().clone() {
         let py_rdn = parse_rdn(py, &rdn)?;
         py_rdns.append(py_rdn)?;
     }
-    x509_module.call_method1("Name", (py_rdns,))
+    Ok(x509_module.call_method1("Name", (py_rdns,))?)
 }
 
 fn parse_name_attribute(
@@ -714,6 +714,13 @@ impl<'a, T: asn1::SimpleAsn1Writable<'a>, U: asn1::SimpleAsn1Writable<'a>>
     }
 }
 
+pub(crate) fn add_to_module(module: &pyo3::prelude::PyModule) -> pyo3::PyResult<()> {
+    module.add_wrapped(pyo3::wrap_pyfunction!(encode_extension_value))?;
+    module.add_wrapped(pyo3::wrap_pyfunction!(encode_name_bytes))?;
+
+    Ok(())
+}
+
 #[cfg(test)]
 mod tests {
     use super::{Asn1ReadableOrWritable, RawTlv};
@@ -736,10 +743,3 @@ mod tests {
         assert!(RawTlv::can_parse(123));
     }
 }
-
-pub(crate) fn add_to_module(module: &pyo3::prelude::PyModule) -> pyo3::PyResult<()> {
-    module.add_wrapped(pyo3::wrap_pyfunction!(encode_extension_value))?;
-    module.add_wrapped(pyo3::wrap_pyfunction!(encode_name_bytes))?;
-
-    Ok(())
-}

src/rust/src/x509/crl.rs

@@ -231,7 +231,10 @@ impl CertificateRevocationList {
 
     #[getter]
     fn issuer<'p>(&self, py: pyo3::Python<'p>) -> pyo3::PyResult<&'p pyo3::PyAny> {
-        x509::parse_name(py, &self.raw.borrow_value().tbs_cert_list.issuer)
+        Ok(x509::parse_name(
+            py,
+            &self.raw.borrow_value().tbs_cert_list.issuer,
+        )?)
     }
 
     #[getter]

src/rust/src/x509/csr.rs

@@ -128,7 +128,10 @@ impl CertificateSigningRequest {
 
     #[getter]
     fn subject<'p>(&self, py: pyo3::Python<'p>) -> pyo3::PyResult<&'p pyo3::PyAny> {
-        x509::parse_name(py, &self.raw.borrow_value().csr_info.subject)
+        Ok(x509::parse_name(
+            py,
+            &self.raw.borrow_value().csr_info.subject,
+        )?)
     }
 
     #[getter]

src/rust/src/x509/ocsp_resp.rs

@@ -135,7 +135,7 @@ impl OCSPResponse {
     fn responder_name<'p>(&self, py: pyo3::Python<'p>) -> pyo3::PyResult<&'p pyo3::PyAny> {
         let resp = self.requires_successful_response()?;
         match resp.tbs_response_data.responder_id {
-            ResponderId::ByName(ref name) => x509::parse_name(py, name),
+            ResponderId::ByName(ref name) => Ok(x509::parse_name(py, name)?),
             ResponderId::ByKey(_) => Ok(py.None().into_ref(py)),
         }
     }

tests/x509/test_x509.py

@@ -981,6 +981,17 @@ class TestRSACertificate:
             x509.NameAttribute(NameOID.COMMON_NAME, "We heart UTF8!\u2122")
         ]
 
+    def test_invalid_unicode_name(self, backend):
+        cert = _load_cert(
+            os.path.join("x509", "custom", "invalid_utf8_common_name.pem"),
+            x509.load_pem_x509_certificate,
+            backend,
+        )
+        with pytest.raises(ValueError, match="subject"):
+            cert.subject
+        with pytest.raises(ValueError, match="issuer"):
+            cert.issuer
+
     def test_non_ascii_dns_name(self, backend):
         cert = _load_cert(
             os.path.join("x509", "utf8-dnsname.pem"),

vectors/cryptography_vectors/x509/custom/invalid_utf8_common_name.pem

@@ -0,0 +1,17 @@
+-----BEGIN CERTIFICATE-----
+MIICsjCCAZoCCQDfCSnalLBhujANBgkqhkiG9w0BAQUFADAbMRkwFwYDVQQDDBBX
+ZSBoZWFydCBVVEY4IeKEMB4XDTE1MDExODAzNTM1NFoXDTE1MDIxNzAzNTM1NFow
+GzEZMBcGA1UEAwwQV2UgaGVhcnQgVVRGOCHihDCCASIwDQYJKoZIhvcNAQEBBQAD
+ggEPADCCAQoCggEBAOBG4RmNZtn5lqI9PawhjCTHQ3aEBv3G4o/gKu+dqWzhweC7
+Dl2u7Wc9NUJzGLBjqhCMCt/HysbO5rJj0DRQA+uyspe+7SCORHg29NpRCH735nq7
+wdaExkZ/wsnp9l3BVggq9x7tbAmEi383sFTLPcjId3kN6/RTaihWSiOKUc/J2Rzu
+vpcpbqbkzmHAMR3wHccuZO3OsSibO59NUR1ogYiTEid51L4glV2PsEDWxB7Epbdv
+/Hsf1GY9yMQpBGGQ7YwZ30FKAzWg5sWrWgHdpM4rvnFlta1T4pEqIDmVm13H3TtN
+1hTwBJP7tNZcg304vSa55xOz3NbQUEjuiIUZhS0CAwEAATANBgkqhkiG9w0BAQUF
+AAOCAQEAEQ2bnpTYlnWzDGI7sTwTOBDzc8WlbPzF+2ZkbtRI3GfCeAdp2+UgBJgC
+pIzOMzn73hHhCZ8/4Ca9v2KumetCZJh38uIjzzzTre8yfSAZBHD8QpYKhsR4RYl6
+syWitP5j49W860C5SKGAio8tSuJpSzpwaF8xl/UoyAthBu7xTPtEmWsvUJLLW23C
+qSpXCA8iefePk2l/Y7iCIZrv4QB85pTKlL+LVcaTFOHRi8sHwFrA/O2hbdUp8YKt
+SdYV1ERjwfNYy7Ci3MamIaVlIxsuJSo8wY0aXcoqqAy3c33vdrvUP+HQiAQSUt56
+1KhGQMAo+zuCVuI18WZ0gsaAYWL+CA==
+-----END CERTIFICATE-----