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 Verification: base Python support for extension policies #12432

Merged
merged 7 commits into from
Feb 11, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions src/cryptography/hazmat/bindings/_rust/x509.pyi
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,9 @@ class PolicyBuilder:
def time(self, new_time: datetime.datetime) -> PolicyBuilder: ...
def store(self, new_store: Store) -> PolicyBuilder: ...
def max_chain_depth(self, new_max_chain_depth: int) -> PolicyBuilder: ...
def extension_policies(
self, new_ca_policy: ExtensionPolicy, new_ee_policy: ExtensionPolicy
Copy link

@bluetech bluetech Feb 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might I suggest making new_ca_policy and new_ee_policy keyword-only arguments?

  • It will force calling code to be clearer to readers, which I think is good.
  • It will avoid possible confusion between the two parameters, which have the same type.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Opened #12476 to address this.

) -> PolicyBuilder: ...
def build_client_verifier(self) -> ClientVerifier: ...
def build_server_verifier(
self, subject: x509.verification.Subject
Expand All @@ -218,6 +221,14 @@ class Policy:
@property
def minimum_rsa_modulus(self) -> int: ...

class ExtensionPolicy:
@staticmethod
def permit_all() -> ExtensionPolicy: ...
@staticmethod
def webpki_defaults_ca() -> ExtensionPolicy: ...
@staticmethod
def webpki_defaults_ee() -> ExtensionPolicy: ...

class VerifiedClient:
@property
def subjects(self) -> list[x509.GeneralName] | None: ...
Expand Down
3 changes: 3 additions & 0 deletions src/cryptography/x509/verification.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@

__all__ = [
"ClientVerifier",
"ExtensionPolicy",
"Policy",
"PolicyBuilder",
"ServerVerifier",
"Store",
Expand All @@ -26,4 +28,5 @@
ServerVerifier = rust_x509.ServerVerifier
PolicyBuilder = rust_x509.PolicyBuilder
Policy = rust_x509.Policy
ExtensionPolicy = rust_x509.ExtensionPolicy
VerificationError = rust_x509.VerificationError
2 changes: 1 addition & 1 deletion src/rust/cryptography-x509-verification/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ pub struct ValidationError<'chain, B: CryptoOps> {
}

impl<'chain, B: CryptoOps> ValidationError<'chain, B> {
pub(crate) fn new(kind: ValidationErrorKind<'chain, B>) -> Self {
pub fn new(kind: ValidationErrorKind<'chain, B>) -> Self {
ValidationError { kind, cert: None }
}

Expand Down
24 changes: 24 additions & 0 deletions src/rust/cryptography-x509-verification/src/policy/extension.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ use crate::{
ops::CryptoOps, policy::Policy, ValidationError, ValidationErrorKind, ValidationResult,
};

#[derive(Clone)]
pub struct ExtensionPolicy<'cb, B: CryptoOps> {
pub authority_information_access: ExtensionValidator<'cb, B>,
pub authority_key_identifier: ExtensionValidator<'cb, B>,
Expand All @@ -28,6 +29,27 @@ pub struct ExtensionPolicy<'cb, B: CryptoOps> {
}

impl<'cb, B: CryptoOps + 'cb> ExtensionPolicy<'cb, B> {
pub fn new_permit_all() -> Self {
const fn make_permissive_validator<'cb, B: CryptoOps + 'cb>() -> ExtensionValidator<'cb, B>
{
ExtensionValidator::MaybePresent {
criticality: Criticality::Agnostic,
validator: None,
}
}

ExtensionPolicy {
authority_information_access: make_permissive_validator(),
authority_key_identifier: make_permissive_validator(),
subject_key_identifier: make_permissive_validator(),
key_usage: make_permissive_validator(),
subject_alternative_name: make_permissive_validator(),
basic_constraints: make_permissive_validator(),
name_constraints: make_permissive_validator(),
extended_key_usage: make_permissive_validator(),
}
}

pub fn new_default_webpki_ca() -> Self {
ExtensionPolicy {
// 5280 4.2.2.1: Authority Information Access
Expand Down Expand Up @@ -214,6 +236,7 @@ impl<'cb, B: CryptoOps + 'cb> ExtensionPolicy<'cb, B> {
}

/// Represents different criticality states for an extension.
#[derive(Clone)]
pub enum Criticality {
/// The extension MUST be marked as critical.
Critical,
Expand Down Expand Up @@ -258,6 +281,7 @@ pub type MaybeExtensionValidatorCallback<'cb, B> = Arc<
>;

/// Represents different validation states for an extension.
#[derive(Clone)]
pub enum ExtensionValidator<'cb, B: CryptoOps> {
/// The extension MUST NOT be present.
NotPresent,
Expand Down
4 changes: 2 additions & 2 deletions src/rust/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -135,8 +135,8 @@ mod _rust {
use crate::x509::sct::Sct;
#[pymodule_export]
use crate::x509::verify::{
PolicyBuilder, PyClientVerifier, PyPolicy, PyServerVerifier, PyStore, PyVerifiedClient,
VerificationError,
PolicyBuilder, PyClientVerifier, PyExtensionPolicy, PyPolicy, PyServerVerifier,
PyStore, PyVerifiedClient, VerificationError,
};
}

Expand Down
39 changes: 39 additions & 0 deletions src/rust/src/x509/verify/extension_policy.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
use super::PyCryptoOps;
use cryptography_x509_verification::policy::ExtensionPolicy;

#[pyo3::pyclass(
frozen,
module = "cryptography.x509.verification",
name = "ExtensionPolicy"
)]
pub(crate) struct PyExtensionPolicy {
rust_policy: ExtensionPolicy<'static, PyCryptoOps>,
}

impl PyExtensionPolicy {
pub(super) fn clone_rust_policy(&self) -> ExtensionPolicy<'static, PyCryptoOps> {
self.rust_policy.clone()
}

fn new(rust_policy: ExtensionPolicy<'static, PyCryptoOps>) -> Self {
PyExtensionPolicy { rust_policy }
}
}

#[pyo3::pymethods]
impl PyExtensionPolicy {
#[staticmethod]
pub(crate) fn permit_all() -> Self {
PyExtensionPolicy::new(ExtensionPolicy::new_permit_all())
}

#[staticmethod]
pub(crate) fn webpki_defaults_ca() -> Self {
PyExtensionPolicy::new(ExtensionPolicy::new_default_webpki_ca())
}

#[staticmethod]
pub(crate) fn webpki_defaults_ee() -> Self {
PyExtensionPolicy::new(ExtensionPolicy::new_default_webpki_ee())
}
}
79 changes: 65 additions & 14 deletions src/rust/src/x509/verify/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ use cryptography_x509_verification::trust_store::Store;
use cryptography_x509_verification::types::{DNSName, IPAddress};
use pyo3::types::{PyAnyMethods, PyListMethods};

mod extension_policy;
mod policy;
use super::parse_general_names;
use crate::backend::keys;
Expand All @@ -20,8 +21,10 @@ use crate::utils::cstr_from_literal;
use crate::x509::certificate::Certificate as PyCertificate;
use crate::x509::common::{datetime_now, py_to_datetime};
use crate::x509::sign;
pub(crate) use extension_policy::PyExtensionPolicy;
pub(crate) use policy::PyPolicy;

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

impl CryptoOps for PyCryptoOps {
Expand Down Expand Up @@ -82,6 +85,8 @@ pub(crate) struct PolicyBuilder {
time: Option<asn1::DateTime>,
store: Option<pyo3::Py<PyStore>>,
max_chain_depth: Option<u8>,
ca_ext_policy: Option<pyo3::Py<PyExtensionPolicy>>,
ee_ext_policy: Option<pyo3::Py<PyExtensionPolicy>>,
}

impl PolicyBuilder {
Expand All @@ -90,6 +95,8 @@ impl 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.as_ref().map(|p| p.clone_ref(py)),
ee_ext_policy: self.ee_ext_policy.as_ref().map(|p| p.clone_ref(py)),
}
}
}
Expand All @@ -102,6 +109,8 @@ impl PolicyBuilder {
time: None,
store: None,
max_chain_depth: None,
ca_ext_policy: None,
ee_ext_policy: None,
}
}

Expand Down Expand Up @@ -144,6 +153,22 @@ impl PolicyBuilder {
})
}

fn extension_policies(
&self,
py: pyo3::Python<'_>,
new_ca_policy: pyo3::Py<PyExtensionPolicy>,
new_ee_policy: pyo3::Py<PyExtensionPolicy>,
) -> CryptographyResult<PolicyBuilder> {
// Enough to check one of the two, since they can only be set together.
policy_builder_set_once_check!(self, ca_ext_policy, "extension policies");

Ok(PolicyBuilder {
ca_ext_policy: Some(new_ca_policy),
ee_ext_policy: Some(new_ee_policy),
..self.py_clone(py)
})
}

fn build_client_verifier(&self, py: pyo3::Python<'_>) -> CryptographyResult<PyClientVerifier> {
let store = match self.store.as_ref() {
Some(s) => s.clone_ref(py),
Expand All @@ -162,8 +187,17 @@ impl PolicyBuilder {
};

let policy_definition = OwnedPolicyDefinition::new(None, |_subject| {
// TODO: Pass extension policies here once implemented in cryptography-x509-verification.
PolicyDefinition::client(PyCryptoOps {}, time, self.max_chain_depth, None, None)
PolicyDefinition::client(
PyCryptoOps {},
time,
self.max_chain_depth,
self.ca_ext_policy
.as_ref()
.map(|p| p.get().clone_rust_policy()),
self.ee_ext_policy
.as_ref()
.map(|p| p.get().clone_rust_policy()),
)
});

let py_policy = PyPolicy {
Expand Down Expand Up @@ -208,14 +242,17 @@ impl PolicyBuilder {
.expect("subject_owner for ServerVerifier can not be None"),
)?;

// TODO: Pass extension policies here once implemented in cryptography-x509-verification.
Ok::<PyCryptoPolicyDefinition<'_>, pyo3::PyErr>(PolicyDefinition::server(
PyCryptoOps {},
subject,
time,
self.max_chain_depth,
None,
None,
self.ca_ext_policy
.as_ref()
.map(|p| p.get().clone_rust_policy()),
self.ee_ext_policy
.as_ref()
.map(|p| p.get().clone_rust_policy()),
))
})?;

Expand Down Expand Up @@ -345,22 +382,24 @@ 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]
// NOTE: The `unwrap()` cannot fail, since the underlying policy
// enforces the well-formedness of the extension set.
let subjects = match &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)?;
{
Some(leaf_san) => {
let leaf_gns = leaf_san.value::<SubjectAlternativeName<'_>>()?;
Some(parse_general_names(py, &leaf_gns)?.unbind())
}
None => None,
};

Ok(PyVerifiedClient {
subjects: Some(py_gns.into()),
subjects,
chain: py_chain.unbind(),
})
}
Expand Down Expand Up @@ -534,3 +573,15 @@ impl PyStore {
})
}
}

#[cfg(test)]
mod tests {
use super::PyCryptoOps;

#[test]
fn test_crypto_ops_clone() {
// Just for coverage.
// The trait is needed to be able to clone ExtensionPolicy<'_, PyCryptoOps>.
let _ = PyCryptoOps {}.clone();
}
}
56 changes: 56 additions & 0 deletions tests/x509/verification/test_verification.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
from cryptography.hazmat._oid import ExtendedKeyUsageOID
from cryptography.x509.general_name import DNSName, IPAddress
from cryptography.x509.verification import (
ExtensionPolicy,
PolicyBuilder,
Store,
VerificationError,
Expand Down Expand Up @@ -209,6 +210,33 @@ def test_verify_fails_renders_oid(self):
):
verifier.verify(leaf, [])

def test_custom_ext_policy_no_san(self):
leaf = _load_cert(
os.path.join("x509", "custom", "no_sans.pem"),
x509.load_pem_x509_certificate,
)

store = Store([leaf])
validation_time = datetime.datetime.fromisoformat(
"2025-04-14T00:00:00+00:00"
)

builder = PolicyBuilder().store(store)
builder = builder.time(validation_time)

with pytest.raises(
VerificationError,
match="missing required extension",
):
builder.build_client_verifier().verify(leaf, [])

builder = builder.extension_policies(
ExtensionPolicy.webpki_defaults_ca(), ExtensionPolicy.permit_all()
)

verified_client = builder.build_client_verifier().verify(leaf, [])
assert verified_client.subjects is None


class TestServerVerifier:
@pytest.mark.parametrize(
Expand Down Expand Up @@ -261,3 +289,31 @@ def test_error_message(self):
match=r"<Certificate\(subject=.*?CN=www.cryptography.io.*?\)>",
):
verifier.verify(leaf, [])


class TestCustomExtensionPolicies:
leaf = _load_cert(
os.path.join("x509", "cryptography.io.pem"),
x509.load_pem_x509_certificate,
)
ca = _load_cert(
os.path.join("x509", "rapidssl_sha256_ca_g3.pem"),
x509.load_pem_x509_certificate,
)
store = Store([ca])
validation_time = datetime.datetime.fromisoformat(
"2018-11-16T00:00:00+00:00"
)

def test_static_methods(self):
builder = PolicyBuilder().store(self.store)
builder = builder.time(self.validation_time).max_chain_depth(16)
builder = builder.extension_policies(
ExtensionPolicy.webpki_defaults_ca(),
ExtensionPolicy.webpki_defaults_ee(),
)

builder.build_client_verifier().verify(self.leaf, [])
builder.build_server_verifier(DNSName("cryptography.io")).verify(
self.leaf, []
)