Skip to content

Commit

Permalink
X509 custom verification groundwork (#11559)
Browse files Browse the repository at this point in the history
* 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.
  • Loading branch information
deivse authored Oct 9, 2024
1 parent cb0a83f commit 1767ad0
Show file tree
Hide file tree
Showing 5 changed files with 37 additions and 18 deletions.
1 change: 1 addition & 0 deletions docs/spelling_wordlist.txt
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,7 @@ unencrypted
unicode
unpadded
unpadding
validator
Ventura
verifier
Verifier
Expand Down
7 changes: 5 additions & 2 deletions docs/x509/verification.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
2 changes: 1 addition & 1 deletion src/cryptography/hazmat/bindings/_rust/x509.pyi
Original file line number Diff line number Diff line change
Expand Up @@ -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]: ...

Expand Down
44 changes: 29 additions & 15 deletions src/rust/src/x509/verify.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,16 @@ pub(crate) struct PolicyBuilder {
max_chain_depth: Option<u8>,
}

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]
Expand All @@ -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<PyStore>) -> CryptographyResult<PolicyBuilder> {
fn store(
&self,
py: pyo3::Python<'_>,
new_store: pyo3::Py<PyStore>,
) -> CryptographyResult<PolicyBuilder> {
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)
})
}

Expand All @@ -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)
})
}

Expand All @@ -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 })
}
Expand Down Expand Up @@ -170,12 +182,14 @@ impl PolicyBuilder {

let policy = OwnedPolicy::try_new(subject_owner, |subject_owner| {
let subject = build_subject(py, subject_owner)?;
Ok::<PyCryptoPolicy<'_>, pyo3::PyErr>(PyCryptoPolicy(Policy::server(

// TODO: Pass extension policies here once implemented in cryptography-x509-verification.
Ok::<PyCryptoPolicy<'_>, pyo3::PyErr>(Policy::server(
PyCryptoOps {},
subject,
time,
self.max_chain_depth,
)))
))
})?;

Ok(PyServerVerifier {
Expand All @@ -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 {
Expand Down Expand Up @@ -215,7 +229,7 @@ self_cell::self_cell!(
)]
pub(crate) struct PyVerifiedClient {
#[pyo3(get)]
subjects: pyo3::Py<pyo3::PyAny>,
subjects: Option<pyo3::Py<pyo3::PyAny>>,
#[pyo3(get)]
chain: pyo3::Py<pyo3::types::PyList>,
}
Expand All @@ -233,7 +247,7 @@ pub(crate) struct PyClientVerifier {

impl PyClientVerifier {
fn as_policy(&self) -> &Policy<'_, PyCryptoOps> {
&self.policy.0
&self.policy
}
}

Expand Down Expand Up @@ -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(),
})
}
Expand All @@ -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()
}
}

Expand Down
1 change: 1 addition & 0 deletions tests/x509/verification/test_verification.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 1767ad0

Please sign in to comment.