Skip to content

Commit

Permalink
Remove CustomPolicyBuilder in favor of extending PolicyBuilder.
Browse files Browse the repository at this point in the history
  • Loading branch information
deivse committed Sep 29, 2024
1 parent a5c9e74 commit 1ebb1e7
Show file tree
Hide file tree
Showing 7 changed files with 97 additions and 297 deletions.
61 changes: 0 additions & 61 deletions docs/x509/verification.rst
Original file line number Diff line number Diff line change
Expand Up @@ -297,64 +297,3 @@ the root of trust:
for server verification.

:returns: An instance of :class:`ClientVerifier`

.. class:: CustomPolicyBuilder

.. versionadded:: 44.0.0

A CustomPolicyBuilder provides a builder-style interface for constructing a
Verifier, but provides additional control over the verification policy compared to :class:`PolicyBuilder`.

.. method:: time(new_time)

Sets the verifier's verification time.

If not called explicitly, this is set to :meth:`datetime.datetime.now`
when :meth:`build_server_verifier` or :meth:`build_client_verifier`
is called.

:param new_time: The :class:`datetime.datetime` to use in the verifier

:returns: A new instance of :class:`PolicyBuilder`

.. method:: store(new_store)

Sets the verifier's trust store.

:param new_store: The :class:`Store` to use in the verifier

:returns: A new instance of :class:`PolicyBuilder`

.. method:: max_chain_depth(new_max_chain_depth)

Sets the verifier's maximum chain building depth.

This depth behaves tracks the length of the intermediate CA
chain: a maximum depth of zero means that the leaf must be directly
issued by a member of the store, a depth of one means no more than
one intermediate CA, and so forth. Note that self-issued intermediates
don't count against the chain depth, per RFC 5280.

:param new_max_chain_depth: The maximum depth to allow in the verifier

:returns: A new instance of :class:`PolicyBuilder`

.. method:: build_server_verifier(subject)

Builds a verifier for verifying server certificates.

:param subject: A :class:`Subject` to use in the verifier

:returns: An instance of :class:`ServerVerifier`

.. method:: build_client_verifier()

Builds a verifier for verifying client certificates.

.. warning::

This API is not suitable for website (i.e. server) certificate
verification. You **must** use :meth:`build_server_verifier`
for server verification.

:returns: An instance of :class:`ClientVerifier`
11 changes: 0 additions & 11 deletions src/cryptography/hazmat/bindings/_rust/x509.pyi
Original file line number Diff line number Diff line change
Expand Up @@ -67,17 +67,6 @@ class PolicyBuilder:
self, subject: x509.verification.Subject
) -> ServerVerifier: ...

class CustomPolicyBuilder:
def time(self, new_time: datetime.datetime) -> CustomPolicyBuilder: ...
def store(self, new_store: Store) -> CustomPolicyBuilder: ...
def max_chain_depth(
self, new_max_chain_depth: int
) -> CustomPolicyBuilder: ...
def build_client_verifier(self) -> ClientVerifier: ...
def build_server_verifier(
self, subject: x509.verification.Subject
) -> ServerVerifier: ...

class VerifiedClient:
@property
def subjects(self) -> list[x509.GeneralName] | None: ...
Expand Down
2 changes: 0 additions & 2 deletions src/cryptography/x509/verification.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
__all__ = [
"ClientVerifier",
"PolicyBuilder",
"CustomPolicyBuilder",
"ServerVerifier",
"Store",
"Subject",
Expand All @@ -26,5 +25,4 @@
ClientVerifier = rust_x509.ClientVerifier
ServerVerifier = rust_x509.ServerVerifier
PolicyBuilder = rust_x509.PolicyBuilder
CustomPolicyBuilder = rust_x509.CustomPolicyBuilder
VerificationError = rust_x509.VerificationError
4 changes: 2 additions & 2 deletions src/rust/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -132,8 +132,8 @@ mod _rust {
use crate::x509::sct::Sct;
#[pymodule_export]
use crate::x509::verify::{
CustomPolicyBuilder, PolicyBuilder, PyClientVerifier, PyServerVerifier, PyStore,
PyVerifiedClient, VerificationError,
PolicyBuilder, PyClientVerifier, PyServerVerifier, PyStore, PyVerifiedClient,
VerificationError,
};
}

Expand Down
223 changes: 65 additions & 158 deletions src/rust/src/x509/verify.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,88 +74,15 @@ pub(crate) struct PolicyBuilder {
time: Option<asn1::DateTime>,
store: Option<pyo3::Py<PyStore>>,
max_chain_depth: Option<u8>,
}

#[pyo3::pymethods]
impl PolicyBuilder {
#[new]
fn new() -> PolicyBuilder {
PolicyBuilder {
time: None,
store: None,
max_chain_depth: None,
}
}

fn time(
&self,
py: pyo3::Python<'_>,
new_time: pyo3::Bound<'_, pyo3::PyAny>,
) -> CryptographyResult<PolicyBuilder> {
policy_builder_set_once_check!(self, time, "validation time");

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,
})
}

fn store(&self, 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,
})
}

fn max_chain_depth(
&self,
py: pyo3::Python<'_>,
new_max_chain_depth: u8,
) -> CryptographyResult<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),
})
}

fn build_client_verifier(&self, py: pyo3::Python<'_>) -> CryptographyResult<PyClientVerifier> {
build_client_verifier_impl(py, &self.store, &self.time, |time| {
Policy::client(PyCryptoOps {}, time, self.max_chain_depth)
})
}

fn build_server_verifier(
&self,
py: pyo3::Python<'_>,
subject: pyo3::PyObject,
) -> CryptographyResult<PyServerVerifier> {
build_server_verifier_impl(py, &self.store, &self.time, subject, |subject, time| {
Policy::server(PyCryptoOps {}, subject, time, self.max_chain_depth)
})
}
}

#[pyo3::pyclass(frozen, module = "cryptography.x509.verification")]
pub(crate) struct CustomPolicyBuilder {
time: Option<asn1::DateTime>,
store: Option<pyo3::Py<PyStore>>,
max_chain_depth: Option<u8>,
ca_ext_policy: Option<ExtensionPolicy<PyCryptoOps>>,
ee_ext_policy: Option<ExtensionPolicy<PyCryptoOps>>,
}

impl CustomPolicyBuilder {
impl PolicyBuilder {
/// Clones the builder, requires the GIL token to increase
/// reference count for `self.store`.
fn py_clone(&self, py: pyo3::Python<'_>) -> CustomPolicyBuilder {
CustomPolicyBuilder {
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,
Expand All @@ -166,10 +93,10 @@ impl CustomPolicyBuilder {
}

#[pyo3::pymethods]
impl CustomPolicyBuilder {
impl PolicyBuilder {
#[new]
fn new() -> CustomPolicyBuilder {
CustomPolicyBuilder {
fn new() -> PolicyBuilder {
PolicyBuilder {
time: None,
store: None,
max_chain_depth: None,
Expand All @@ -182,10 +109,10 @@ impl CustomPolicyBuilder {
&self,
py: pyo3::Python<'_>,
new_time: pyo3::Bound<'_, pyo3::PyAny>,
) -> CryptographyResult<CustomPolicyBuilder> {
) -> CryptographyResult<PolicyBuilder> {
policy_builder_set_once_check!(self, time, "validation time");

Ok(CustomPolicyBuilder {
Ok(PolicyBuilder {
time: Some(py_to_datetime(py, new_time)?),
..self.py_clone(py)
})
Expand All @@ -195,10 +122,10 @@ impl CustomPolicyBuilder {
&self,
py: pyo3::Python<'_>,
new_store: pyo3::Py<PyStore>,
) -> CryptographyResult<CustomPolicyBuilder> {
) -> CryptographyResult<PolicyBuilder> {
policy_builder_set_once_check!(self, store, "trust store");

Ok(CustomPolicyBuilder {
Ok(PolicyBuilder {
store: Some(new_store),
..self.py_clone(py)
})
Expand All @@ -208,100 +135,80 @@ impl CustomPolicyBuilder {
&self,
py: pyo3::Python<'_>,
new_max_chain_depth: u8,
) -> CryptographyResult<CustomPolicyBuilder> {
) -> CryptographyResult<PolicyBuilder> {
policy_builder_set_once_check!(self, max_chain_depth, "maximum chain depth");

Ok(CustomPolicyBuilder {
Ok(PolicyBuilder {
max_chain_depth: Some(new_max_chain_depth),
..self.py_clone(py)
})
}

fn build_client_verifier(&self, py: pyo3::Python<'_>) -> CryptographyResult<PyClientVerifier> {
build_client_verifier_impl(py, &self.store, &self.time, |time| {
// TODO: Replace with a custom policy once it's implemented in cryptography-x509-verification
Policy::client(PyCryptoOps {}, time, self.max_chain_depth)
})
let store = match self.store.as_ref() {
Some(s) => s.clone_ref(py),
None => {
return Err(CryptographyError::from(
pyo3::exceptions::PyValueError::new_err(
"A client verifier must have a trust store.",
),
));
}
};

let time = match self.time.as_ref() {
Some(t) => t.clone(),
None => datetime_now(py)?,
};

// 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 })
}

fn build_server_verifier(
&self,
py: pyo3::Python<'_>,
subject: pyo3::PyObject,
) -> CryptographyResult<PyServerVerifier> {
build_server_verifier_impl(py, &self.store, &self.time, subject, |subject, time| {
// TODO: Replace with a custom policy once it's implemented in cryptography-x509-verification
Policy::server(PyCryptoOps {}, subject, time, self.max_chain_depth)
let store = match self.store.as_ref() {
Some(s) => s.clone_ref(py),
None => {
return Err(CryptographyError::from(
pyo3::exceptions::PyValueError::new_err(
"A server verifier must have a trust store.",
),
));
}
};

let time = match self.time.as_ref() {
Some(t) => t.clone(),
None => datetime_now(py)?,
};
let subject_owner = build_subject_owner(py, &subject)?;

let policy = OwnedPolicy::try_new(subject_owner, |subject_owner| {
let subject = build_subject(py, subject_owner)?;

// 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 {
py_subject: subject,
policy,
store,
})
}
}

/// This is a helper to avoid code duplication between PolicyBuilder and CustomPolicyBuilder.
fn build_server_verifier_impl(
py: pyo3::Python<'_>,
store: &Option<pyo3::Py<PyStore>>,
time: &Option<asn1::DateTime>,
subject: pyo3::PyObject,
make_policy: impl Fn(Subject<'_>, asn1::DateTime) -> PyCryptoPolicy<'_>,
) -> CryptographyResult<PyServerVerifier> {
let store = match store {
Some(s) => s.clone_ref(py),
None => {
return Err(CryptographyError::from(
pyo3::exceptions::PyValueError::new_err(
"A server verifier must have a trust store.",
),
));
}
};

let time = match time.as_ref() {
Some(t) => t.clone(),
None => datetime_now(py)?,
};
let subject_owner = build_subject_owner(py, &subject)?;

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

Ok(PyServerVerifier {
py_subject: subject,
policy,
store,
})
}

/// This is a helper to avoid code duplication between PolicyBuilder and CustomPolicyBuilder.
fn build_client_verifier_impl(
py: pyo3::Python<'_>,
store: &Option<pyo3::Py<PyStore>>,
time: &Option<asn1::DateTime>,
make_policy: impl Fn(asn1::DateTime) -> PyCryptoPolicy<'static>,
) -> CryptographyResult<PyClientVerifier> {
let store = match store.as_ref() {
Some(s) => s.clone_ref(py),
None => {
return Err(CryptographyError::from(
pyo3::exceptions::PyValueError::new_err(
"A client verifier must have a trust store.",
),
));
}
};

let time = match time.as_ref() {
Some(t) => t.clone(),
None => datetime_now(py)?,
};

Ok(PyClientVerifier {
policy: make_policy(time),
store,
})
}

type PyCryptoPolicy<'a> = Policy<'a, PyCryptoOps>;

/// This enum exists solely to provide heterogeneously typed ownership for `OwnedPolicy`.
Expand Down
Loading

0 comments on commit 1ebb1e7

Please sign in to comment.