From 1767ad0a462f47a0112221ca7e7cf1684a9b1869 Mon Sep 17 00:00:00 2001 From: Ivan Desiatov <76527282+deivse@users.noreply.github.com> Date: Wed, 9 Oct 2024 04:27:15 +0200 Subject: [PATCH] X509 custom verification groundwork (#11559) * Add CustomPolicyBuilder foundation. * Add EKU getters to ClientVerifier and ServerVerifier. * Document the implemented part of custom verification. * Remove `subject` field from VerifiedClient, rename `sans` back to `subjects`. * Remove EKU-related setters, getters and documentation from this PR. * Use double backticks in reStructuredText. * Remove CustomPolicyBuilder in favor of extending PolicyBuilder. * Code style improvements. * Resolve coverage issues. --- docs/spelling_wordlist.txt | 1 + docs/x509/verification.rst | 7 ++- .../hazmat/bindings/_rust/x509.pyi | 2 +- src/rust/src/x509/verify.rs | 44 ++++++++++++------- tests/x509/verification/test_verification.py | 1 + 5 files changed, 37 insertions(+), 18 deletions(-) diff --git a/docs/spelling_wordlist.txt b/docs/spelling_wordlist.txt index 6a0282266821..f8e6d4232ae0 100644 --- a/docs/spelling_wordlist.txt +++ b/docs/spelling_wordlist.txt @@ -140,6 +140,7 @@ unencrypted unicode unpadded unpadding +validator Ventura verifier Verifier diff --git a/docs/x509/verification.rst b/docs/x509/verification.rst index b0e1daee2994..70aafd48f94c 100644 --- a/docs/x509/verification.rst +++ b/docs/x509/verification.rst @@ -111,12 +111,15 @@ the root of trust: .. versionadded:: 43.0.0 + .. versionchanged:: 44.0.0 + Made ``subjects`` optional with the addition of custom extension policies. + .. attribute:: subjects - :type: list of :class:`~cryptography.x509.GeneralName` + :type: list of :class:`~cryptography.x509.GeneralName` or None The subjects presented in the verified client's Subject Alternative Name - extension. + extension or ``None`` if the extension is not present. .. attribute:: chain diff --git a/src/cryptography/hazmat/bindings/_rust/x509.pyi b/src/cryptography/hazmat/bindings/_rust/x509.pyi index aa85657fcfd8..983200df5e45 100644 --- a/src/cryptography/hazmat/bindings/_rust/x509.pyi +++ b/src/cryptography/hazmat/bindings/_rust/x509.pyi @@ -69,7 +69,7 @@ class PolicyBuilder: class VerifiedClient: @property - def subjects(self) -> list[x509.GeneralName]: ... + 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 index dbe95a494267..face9acf674f 100644 --- a/src/rust/src/x509/verify.rs +++ b/src/rust/src/x509/verify.rs @@ -75,6 +75,16 @@ pub(crate) struct PolicyBuilder { max_chain_depth: Option, } +impl PolicyBuilder { + fn py_clone(&self, py: pyo3::Python<'_>) -> PolicyBuilder { + PolicyBuilder { + time: self.time.clone(), + store: self.store.as_ref().map(|s| s.clone_ref(py)), + max_chain_depth: self.max_chain_depth, + } + } +} + #[pyo3::pymethods] impl PolicyBuilder { #[new] @@ -95,18 +105,20 @@ impl PolicyBuilder { Ok(PolicyBuilder { time: Some(py_to_datetime(py, new_time)?), - store: self.store.as_ref().map(|s| s.clone_ref(py)), - max_chain_depth: self.max_chain_depth, + ..self.py_clone(py) }) } - fn store(&self, new_store: pyo3::Py) -> CryptographyResult { + fn store( + &self, + py: pyo3::Python<'_>, + new_store: pyo3::Py, + ) -> CryptographyResult { policy_builder_set_once_check!(self, store, "trust store"); Ok(PolicyBuilder { - time: self.time.clone(), store: Some(new_store), - max_chain_depth: self.max_chain_depth, + ..self.py_clone(py) }) } @@ -118,9 +130,8 @@ impl PolicyBuilder { policy_builder_set_once_check!(self, max_chain_depth, "maximum chain depth"); Ok(PolicyBuilder { - time: self.time.clone(), - store: self.store.as_ref().map(|s| s.clone_ref(py)), max_chain_depth: Some(new_max_chain_depth), + ..self.py_clone(py) }) } @@ -141,7 +152,8 @@ impl PolicyBuilder { None => datetime_now(py)?, }; - let policy = PyCryptoPolicy(Policy::client(PyCryptoOps {}, time, self.max_chain_depth)); + // TODO: Pass extension policies here once implemented in cryptography-x509-verification. + let policy = Policy::client(PyCryptoOps {}, time, self.max_chain_depth); Ok(PyClientVerifier { policy, store }) } @@ -170,12 +182,14 @@ impl PolicyBuilder { let policy = OwnedPolicy::try_new(subject_owner, |subject_owner| { let subject = build_subject(py, subject_owner)?; - Ok::, pyo3::PyErr>(PyCryptoPolicy(Policy::server( + + // TODO: Pass extension policies here once implemented in cryptography-x509-verification. + Ok::, pyo3::PyErr>(Policy::server( PyCryptoOps {}, subject, time, self.max_chain_depth, - ))) + )) })?; Ok(PyServerVerifier { @@ -186,7 +200,7 @@ impl PolicyBuilder { } } -struct PyCryptoPolicy<'a>(Policy<'a, PyCryptoOps>); +type PyCryptoPolicy<'a> = Policy<'a, PyCryptoOps>; /// This enum exists solely to provide heterogeneously typed ownership for `OwnedPolicy`. enum SubjectOwner { @@ -215,7 +229,7 @@ self_cell::self_cell!( )] pub(crate) struct PyVerifiedClient { #[pyo3(get)] - subjects: pyo3::Py, + subjects: Option>, #[pyo3(get)] chain: pyo3::Py, } @@ -233,7 +247,7 @@ pub(crate) struct PyClientVerifier { impl PyClientVerifier { fn as_policy(&self) -> &Policy<'_, PyCryptoOps> { - &self.policy.0 + &self.policy } } @@ -305,7 +319,7 @@ impl PyClientVerifier { let py_gns = parse_general_names(py, &leaf_gns)?; Ok(PyVerifiedClient { - subjects: py_gns, + subjects: Some(py_gns), chain: py_chain.unbind(), }) } @@ -326,7 +340,7 @@ pub(crate) struct PyServerVerifier { impl PyServerVerifier { fn as_policy(&self) -> &Policy<'_, PyCryptoOps> { - &self.policy.borrow_dependent().0 + self.policy.borrow_dependent() } } diff --git a/tests/x509/verification/test_verification.py b/tests/x509/verification/test_verification.py index f5e70bab3538..1d2f9261c57d 100644 --- a/tests/x509/verification/test_verification.py +++ b/tests/x509/verification/test_verification.py @@ -139,6 +139,7 @@ def test_verify(self): verified_client = verifier.verify(leaf, []) assert verified_client.chain == [leaf] + 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