Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

X509 custom verification groundwork #11559

Merged
merged 9 commits into from
Oct 9, 2024
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
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@ use cryptography_x509::{

use crate::{ops::CryptoOps, policy::Policy, ValidationError};

pub(crate) struct ExtensionPolicy<B: CryptoOps> {
#[derive(Clone)]
pub struct ExtensionPolicy<B: CryptoOps> {
pub(crate) authority_information_access: ExtensionValidator<B>,
pub(crate) authority_key_identifier: ExtensionValidator<B>,
pub(crate) subject_key_identifier: ExtensionValidator<B>,
Expand Down Expand Up @@ -123,6 +124,7 @@ impl<B: CryptoOps> ExtensionPolicy<B> {
}

/// Represents different criticality states for an extension.
#[derive(Clone)]
pub(crate) enum Criticality {
/// The extension MUST be marked as critical.
Critical,
Expand Down Expand Up @@ -151,6 +153,7 @@ type MaybeExtensionValidatorCallback<B> =
fn(&Policy<'_, B>, &Certificate<'_>, Option<&Extension<'_>>) -> Result<(), ValidationError>;

/// Represents different validation states for an extension.
#[derive(Clone)]
pub(crate) enum ExtensionValidator<B: CryptoOps> {
/// The extension MUST NOT be present.
NotPresent,
Expand Down
4 changes: 3 additions & 1 deletion src/rust/cryptography-x509-verification/src/policy/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,12 @@ use cryptography_x509::oid::{
use once_cell::sync::Lazy;

use crate::ops::CryptoOps;
use crate::policy::extension::{ca, common, ee, Criticality, ExtensionPolicy, ExtensionValidator};
use crate::policy::extension::{ca, common, ee, Criticality, ExtensionValidator};
use crate::types::{DNSName, DNSPattern, IPAddress};
use crate::{ValidationError, VerificationCertificate};

pub use crate::policy::extension::ExtensionPolicy;

// RSA key constraints, as defined in CA/B 6.1.5.
static WEBPKI_MINIMUM_RSA_MODULUS: usize = 2048;

Expand Down
89 changes: 60 additions & 29 deletions src/rust/src/x509/verify.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use cryptography_x509::{
};
use cryptography_x509_verification::{
ops::{CryptoOps, VerificationCertificate},
policy::{Policy, Subject},
policy::{ExtensionPolicy, Policy, Subject},
trust_store::Store,
types::{DNSName, IPAddress},
};
Expand All @@ -22,6 +22,7 @@ use crate::x509::sign;

use super::parse_general_names;

#[derive(Clone)]
pub(crate) struct PyCryptoOps {}

impl CryptoOps for PyCryptoOps {
Expand Down Expand Up @@ -73,6 +74,22 @@ pub(crate) struct PolicyBuilder {
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 PolicyBuilder {
/// Clones the builder, requires the GIL token to increase
/// reference count for `self.store`.
deivse marked this conversation as resolved.
Show resolved Hide resolved
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,
ca_ext_policy: self.ca_ext_policy.clone(),
ee_ext_policy: self.ee_ext_policy.clone(),
}
}
}

#[pyo3::pymethods]
Expand All @@ -83,6 +100,8 @@ impl PolicyBuilder {
time: None,
store: None,
max_chain_depth: None,
ca_ext_policy: None,
ee_ext_policy: None,
}
}

Expand All @@ -95,18 +114,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 +139,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 +161,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 +191,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 +209,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 +238,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 +256,7 @@ pub(crate) struct PyClientVerifier {

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

Expand Down Expand Up @@ -290,22 +313,30 @@ impl PyClientVerifier {
py_chain.append(c.extra())?;
}

// NOTE: These `unwrap()`s cannot fail, since the underlying policy
// enforces the presence of a SAN and the well-formedness of the
// extension set.
let leaf_san = &chain[0]
.certificate()
.extensions()
.ok()
.unwrap()
.get_extension(&SUBJECT_ALTERNATIVE_NAME_OID)
.unwrap();

let leaf_gns = leaf_san.value::<SubjectAlternativeName<'_>>()?;
let py_gns = parse_general_names(py, &leaf_gns)?;
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)?;
Some(py_gns)
}
}
};
deivse marked this conversation as resolved.
Show resolved Hide resolved

Ok(PyVerifiedClient {
subjects: py_gns,
subjects,
chain: py_chain.unbind(),
})
}
Expand All @@ -326,7 +357,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
Loading