Remove subject field from VerifiedClient, rename sans back to `su…

…bjects`.
pyca · Sep 19, 2024 · 8ec4c19 · 8ec4c19
1 parent 0852630
commit 8ec4c19
Show file tree

Hide file tree

Showing 5 changed files with 19 additions and 55 deletions.
diff --git a/docs/x509/verification.rst b/docs/x509/verification.rst
@@ -112,16 +112,9 @@ the root of trust:
     .. versionadded:: 43.0.0
 
     .. versionchanged:: 44.0.0
-        Renamed `subjects` to :attr:`sans`. 
-        Made `sans` optional, added :attr:`subject`.
+        Made `subjects` optional with the addition of custom extension policies.
 
-    .. attribute:: subject
-
-        :type: :class:`~cryptography.x509.Name`
-
-        The subject presented in the verified client's certificate.
-
-    .. attribute:: sans
+    .. attribute:: subjects
 
         :type: list of :class:`~cryptography.x509.GeneralName` or None
 

diff --git a/src/cryptography/hazmat/bindings/_rust/x509.pyi b/src/cryptography/hazmat/bindings/_rust/x509.pyi
@@ -81,9 +81,7 @@ class CustomPolicyBuilder:
 
 class VerifiedClient:
     @property
-    def subject(self) -> x509.Name: ...
-    @property
-    def sans(self) -> list[x509.GeneralName] | None: ...
+    def subjects(self) -> list[x509.GeneralName] | None: ...
     @property
     def chain(self) -> list[x509.Certificate]: ...
 

diff --git a/src/rust/src/x509/verify.rs b/src/rust/src/x509/verify.rs
@@ -13,10 +13,7 @@ use cryptography_x509_verification::{
     trust_store::Store,
     types::{DNSName, IPAddress},
 };
-use pyo3::{
-    types::{PyAnyMethods, PyListMethods},
-    ToPyObject,
-};
+use pyo3::types::{PyAnyMethods, PyListMethods};
 
 use crate::types;
 use crate::x509::certificate::Certificate as PyCertificate;
@@ -365,9 +362,7 @@ self_cell::self_cell!(
 )]
 pub(crate) struct PyVerifiedClient {
     #[pyo3(get)]
-    subject: pyo3::Py<pyo3::PyAny>,
-    #[pyo3(get)]
-    sans: Option<pyo3::Py<pyo3::PyAny>>,
+    subjects: Option<pyo3::Py<pyo3::PyAny>>,
     #[pyo3(get)]
     chain: pyo3::Py<pyo3::types::PyList>,
 }
@@ -448,32 +443,30 @@ impl PyClientVerifier {
             py_chain.append(c.extra())?;
         }
 
-        let cert = &chain[0].certificate();
-
-        let py_sans = || -> pyo3::PyResult<Option<pyo3::PyObject>> {
-            let leaf_san_ext = cert
+        let subjects = {
+            // NOTE: The `unwrap()` cannot fail, since the underlying policy
+            // enforces the well-formedness of the extension set.
+            let leaf_san_ext = &chain[0]
+                .certificate()
                 .extensions()
                 .ok()
                 .unwrap()
                 .get_extension(&SUBJECT_ALTERNATIVE_NAME_OID);
 
             match leaf_san_ext {
+                None => None,
                 Some(leaf_san) => {
                     let leaf_gns = leaf_san
                         .value::<SubjectAlternativeName<'_>>()
                         .map_err(|e| -> CryptographyError { e.into() })?;
                     let py_gns = parse_general_names(py, &leaf_gns)?;
-                    Ok(Some(py_gns))
+                    Some(py_gns)
                 }
-                None => Ok(None),
             }
-        }()?;
-
-        let py_subject = crate::x509::parse_name(py, cert.subject())?;
+        };
 
         Ok(PyVerifiedClient {
-            subject: py_subject.to_object(py),
-            sans: py_sans,
+            subjects,
             chain: py_chain.unbind(),
         })
     }

diff --git a/tests/x509/verification/test_limbo.py b/tests/x509/verification/test_limbo.py
@@ -169,7 +169,7 @@ def _limbo_testcase(
             expected_subjects = [
                 _get_limbo_peer(p) for p in testcase["expected_peer_names"]
             ]
-            assert expected_subjects == verified_client.sans
+            assert expected_subjects == verified_client.subjects
 
             built_chain = verified_client.chain
 

diff --git a/tests/x509/verification/test_verification.py b/tests/x509/verification/test_verification.py
@@ -185,30 +185,10 @@ def test_verify(self, builder_type: Type[AnyPolicyBuilder]):
         verified_client = verifier.verify(leaf, [])
         assert verified_client.chain == [leaf]
 
-        expected_subject = x509.Name(
-            [
-                x509.NameAttribute(
-                    x509.NameOID.ORGANIZATIONAL_UNIT_NAME, "GT48742965"
-                ),
-                x509.NameAttribute(
-                    x509.NameOID.ORGANIZATIONAL_UNIT_NAME,
-                    "See www.rapidssl.com/resources/cps (c)14",
-                ),
-                x509.NameAttribute(
-                    x509.NameOID.ORGANIZATIONAL_UNIT_NAME,
-                    "Domain Control Validated - RapidSSL(R)",
-                ),
-                x509.NameAttribute(
-                    x509.NameOID.COMMON_NAME, "www.cryptography.io"
-                ),
-            ]
-        )
-        assert verified_client.subject == expected_subject
-        assert verified_client.sans is not None
-        assert x509.DNSName("www.cryptography.io") in verified_client.sans
-        assert x509.DNSName("cryptography.io") in verified_client.sans
-
-        assert len(verified_client.sans) == 2
+        assert verified_client.subjects is not None
+        assert x509.DNSName("www.cryptography.io") in verified_client.subjects
+        assert x509.DNSName("cryptography.io") in verified_client.subjects
+        assert len(verified_client.subjects) == 2
 
     def test_verify_fails_renders_oid(
         self, builder_type: Type[AnyPolicyBuilder]