Skip to content

Commit

Permalink
verification: client verification APIs (pyca#10345)
Browse files Browse the repository at this point in the history
* verification: WIP client verification skeleton

Signed-off-by: William Woodruff <[email protected]>

* verify: fill in build_client_verifier

Signed-off-by: William Woodruff <[email protected]>

* implement ClientVerifier.verify

Signed-off-by: William Woodruff <[email protected]>

* verification: make Python 3.8 happy

Signed-off-by: William Woodruff <[email protected]>

* switch to a full VerifiedClient type

Signed-off-by: William Woodruff <[email protected]>

* remove the SubjectOwner::None hack

Signed-off-by: William Woodruff <[email protected]>

* docs: fix ClientVerifier

Signed-off-by: William Woodruff <[email protected]>

* verification: replace match with if

Signed-off-by: William Woodruff <[email protected]>

* return GNs directly, not whole extension

Signed-off-by: William Woodruff <[email protected]>

* docs/verification: document UnsupportedGeneralNameType raise

Signed-off-by: William Woodruff <[email protected]>

* lib: RFC822 checks on NCs

* test_limbo: enable client tests

* tests: flake

* test_verification: more Python API coverage

* verification: filter GNs by NC support

* verification: forbid unsupported NC GNs

This is what we should have been doing originally, per
RFC 5280 4.2.1.10:

> If a name constraints extension that is marked as critical
> imposes constraints on a particular name form, and an instance of
> that name form appears in the subject field or subjectAltName
> extension of a subsequent certificate, then the application MUST
> either process the constraint or reject the certificate.

* docs/verification: remove old sentence

Signed-off-by: William Woodruff <[email protected]>

* verification: ensure the right EKU for client/server paths

Signed-off-by: William Woodruff <[email protected]>

* test_limbo: fixup EKU assertion

* verification: feedback

---------

Signed-off-by: William Woodruff <[email protected]>
  • Loading branch information
woodruffw committed Mar 21, 2024
1 parent ee8e8c4 commit 4a3e7dc
Show file tree
Hide file tree
Showing 9 changed files with 361 additions and 32 deletions.
84 changes: 83 additions & 1 deletion docs/x509/verification.rst
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,73 @@ the root of trust:
:class:`cryptography.x509.general_name.DNSName`,
:class:`cryptography.x509.general_name.IPAddress`.

.. class:: VerifiedClient

.. versionadded:: 43.0.0

.. attribute:: subjects

:type: list of :class:`~cryptography.x509.GeneralName`

The subjects presented in the verified client's Subject Alternative Name
extension.

.. attribute:: chain

:type: A list of :class:`~cryptography.x509.Certificate`, in leaf-first order

The chain of certificates that forms the valid chain to the client
certificate.


.. class:: ClientVerifier

.. versionadded:: 43.0.0

A ClientVerifier verifies client certificates.

It contains and describes various pieces of configurable path
validation logic, such as how deep prospective validation chains may go,
which signature algorithms are allowed, and so forth.

ClientVerifier instances cannot be constructed directly;
:class:`PolicyBuilder` must be used.

.. attribute:: validation_time

:type: :class:`datetime.datetime`

The verifier's validation time.

.. attribute:: max_chain_depth

:type: :class:`int`

The verifier's maximum intermediate CA chain depth.

.. attribute:: store

:type: :class:`Store`

The verifier's trust store.

.. method:: verify(leaf, intermediates)

Performs path validation on ``leaf``, returning a valid path
if one exists. The path is returned in leaf-first order:
the first member is ``leaf``, followed by the intermediates used
(if any), followed by a member of the ``store``.

:param leaf: The leaf :class:`~cryptography.x509.Certificate` to validate
:param intermediates: A :class:`list` of intermediate :class:`~cryptography.x509.Certificate` to attempt to use

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

:raises VerificationError: If a valid chain cannot be constructed

:raises UnsupportedGeneralNameType: If a valid chain exists, but contains an unsupported general name type

.. class:: ServerVerifier

.. versionadded:: 42.0.0
Expand Down Expand Up @@ -174,7 +241,8 @@ the root of trust:
Sets the verifier's verification time.

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

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

Expand Down Expand Up @@ -209,3 +277,17 @@ the root of trust:
:param subject: A :class:`Subject` to use in the verifier

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

.. method:: build_client_verifier()

.. versionadded:: 43.0.0

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`
20 changes: 20 additions & 0 deletions src/cryptography/hazmat/bindings/_rust/x509.pyi
Original file line number Diff line number Diff line change
Expand Up @@ -62,10 +62,30 @@ 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 build_client_verifier(self) -> ClientVerifier: ...
def build_server_verifier(
self, subject: x509.verification.Subject
) -> ServerVerifier: ...

class VerifiedClient:
@property
def subjects(self) -> list[x509.GeneralName]: ...
@property
def chain(self) -> list[x509.Certificate]: ...

class ClientVerifier:
@property
def validation_time(self) -> datetime.datetime: ...
@property
def store(self) -> Store: ...
@property
def max_chain_depth(self) -> int: ...
def verify(
self,
leaf: x509.Certificate,
intermediates: list[x509.Certificate],
) -> VerifiedClient: ...

class ServerVerifier:
@property
def subject(self) -> x509.verification.Subject: ...
Expand Down
4 changes: 4 additions & 0 deletions src/cryptography/x509/verification.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,17 @@
__all__ = [
"Store",
"Subject",
"VerifiedClient",
"ClientVerifier",
"ServerVerifier",
"PolicyBuilder",
"VerificationError",
]

Store = rust_x509.Store
Subject = typing.Union[DNSName, IPAddress]
VerifiedClient = rust_x509.VerifiedClient
ClientVerifier = rust_x509.ClientVerifier
ServerVerifier = rust_x509.ServerVerifier
PolicyBuilder = rust_x509.PolicyBuilder
VerificationError = rust_x509.VerificationError
14 changes: 14 additions & 0 deletions src/rust/cryptography-x509-verification/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ use cryptography_x509::{
name::GeneralName,
oid::{NAME_CONSTRAINTS_OID, SUBJECT_ALTERNATIVE_NAME_OID},
};
use types::{RFC822Constraint, RFC822Name};

use crate::certificate::cert_is_self_issued;
use crate::ops::{CryptoOps, VerificationCertificate};
Expand Down Expand Up @@ -137,6 +138,19 @@ impl<'a, 'chain> NameChain<'a, 'chain> {
))),
}
}
(GeneralName::RFC822Name(pattern), GeneralName::RFC822Name(name)) => {
match (RFC822Constraint::new(pattern.0), RFC822Name::new(name.0)) {
(Some(pattern), Some(name)) => Ok(Applied(pattern.matches(&name))),
(_, None) => Err(ValidationError::Other(format!(
"unsatisfiable RFC822 name constraint: malformed SAN {:?}",
name.0,
))),
(None, _) => Err(ValidationError::Other(format!(
"malformed RFC822 name constraints: {:?}",
pattern.0
))),
}
}
// All other matching pairs of (constraint, name) are currently unsupported.
(GeneralName::OtherName(_), GeneralName::OtherName(_))
| (GeneralName::X400Address(_), GeneralName::X400Address(_))
Expand Down
20 changes: 11 additions & 9 deletions src/rust/cryptography-x509-verification/src/policy/extension.rs
Original file line number Diff line number Diff line change
Expand Up @@ -303,15 +303,17 @@ pub(crate) mod ee {
_ => (),
};

let san: SubjectAlternativeName<'_> = extn.value()?;
if !policy
.subject
.as_ref()
.map_or_else(|| false, |sub| sub.matches(&san))
{
return Err(ValidationError::Other(
"leaf certificate has no matching subjectAltName".into(),
));
// NOTE: We only verify the SAN against the policy's subject if the
// policy actually contains one. This enables both client and server
// profiles to use this validator, **with the expectation** that
// server profile construction requires a subject to be present.
if let Some(sub) = policy.subject.as_ref() {
let san: SubjectAlternativeName<'_> = extn.value()?;
if !sub.matches(&san) {
return Err(ValidationError::Other(
"leaf certificate has no matching subjectAltName".into(),
));
}
}

Ok(())
Expand Down
50 changes: 43 additions & 7 deletions src/rust/cryptography-x509-verification/src/policy/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@ use cryptography_x509::common::{
use cryptography_x509::extensions::{BasicConstraints, Extensions, SubjectAlternativeName};
use cryptography_x509::name::GeneralName;
use cryptography_x509::oid::{
BASIC_CONSTRAINTS_OID, EC_SECP256R1, EC_SECP384R1, EC_SECP521R1, EKU_SERVER_AUTH_OID,
BASIC_CONSTRAINTS_OID, EC_SECP256R1, EC_SECP384R1, EC_SECP521R1, EKU_CLIENT_AUTH_OID,
EKU_SERVER_AUTH_OID,
};
use once_cell::sync::Lazy;

Expand Down Expand Up @@ -234,20 +235,19 @@ pub struct Policy<'a, B: CryptoOps> {
}

impl<'a, B: CryptoOps> Policy<'a, B> {
/// Create a new policy with defaults for the server certificate profile
/// defined in the CA/B Forum's Basic Requirements.
pub fn server(
fn new(
ops: B,
subject: Subject<'a>,
subject: Option<Subject<'a>>,
time: asn1::DateTime,
max_chain_depth: Option<u8>,
extended_key_usage: ObjectIdentifier,
) -> Self {
Self {
ops,
max_chain_depth: max_chain_depth.unwrap_or(DEFAULT_MAX_CHAIN_DEPTH),
subject: Some(subject),
subject,
validation_time: time,
extended_key_usage: EKU_SERVER_AUTH_OID.clone(),
extended_key_usage,
minimum_rsa_modulus: WEBPKI_MINIMUM_RSA_MODULUS,
permitted_public_key_algorithms: Arc::clone(&*WEBPKI_PERMITTED_SPKI_ALGORITHMS),
permitted_signature_algorithms: Arc::clone(&*WEBPKI_PERMITTED_SIGNATURE_ALGORITHMS),
Expand Down Expand Up @@ -316,6 +316,9 @@ impl<'a, B: CryptoOps> Policy<'a, B> {
Some(ee::key_usage),
),
// CA/B 7.1.2.7.12 Subscriber Certificate Subject Alternative Name
// This validator handles both client and server cases by only matching against
// the SAN if the profile contains a subject, which it won't in the client
// validation case.
subject_alternative_name: ExtensionValidator::present(
Criticality::Agnostic,
Some(ee::subject_alternative_name),
Expand All @@ -337,6 +340,39 @@ impl<'a, B: CryptoOps> Policy<'a, B> {
}
}

/// Create a new policy with suitable defaults for client certification
/// validation.
///
/// **IMPORTANT**: This is **not** the appropriate API for verifying
/// website (i.e. server) certificates. For that, you **must** use
/// [`Policy::server`].
pub fn client(ops: B, time: asn1::DateTime, max_chain_depth: Option<u8>) -> Self {
Self::new(
ops,
None,
time,
max_chain_depth,
EKU_CLIENT_AUTH_OID.clone(),
)
}

/// Create a new policy with defaults for the server certificate profile
/// defined in the CA/B Forum's Basic Requirements.
pub fn server(
ops: B,
subject: Subject<'a>,
time: asn1::DateTime,
max_chain_depth: Option<u8>,
) -> Self {
Self::new(
ops,
Some(subject),
time,
max_chain_depth,
EKU_SERVER_AUTH_OID.clone(),
)
}

fn permits_basic(&self, cert: &Certificate<'_>) -> Result<(), ValidationError> {
// CA/B 7.1.1:
// Certificates MUST be of type X.509 v3.
Expand Down
Loading

0 comments on commit 4a3e7dc

Please sign in to comment.