From a32edd67278731dcaf6b08081a1c02f40ed52903 Mon Sep 17 00:00:00 2001 From: William Woodruff Date: Thu, 15 Jun 2023 12:14:05 -0400 Subject: [PATCH] more chain building sketching The basic idea is here. Signed-off-by: William Woodruff --- .../src/certificate.rs | 20 +++++++- .../cryptography-x509-validation/src/lib.rs | 34 ++++++++++++-- .../src/policy/mod.rs | 47 ++++++++++++++----- .../src/policy/profile/rfc5280.rs | 15 +++++- .../src/trust_store.rs | 24 +++++++++- .../cryptography-x509-validation/src/types.rs | 5 ++ 6 files changed, 122 insertions(+), 23 deletions(-) diff --git a/src/rust/cryptography-x509-validation/src/certificate.rs b/src/rust/cryptography-x509-validation/src/certificate.rs index da478330ad503..db242aecb71b8 100644 --- a/src/rust/cryptography-x509-validation/src/certificate.rs +++ b/src/rust/cryptography-x509-validation/src/certificate.rs @@ -11,7 +11,7 @@ use cryptography_x509::{ }, }; -use crate::{backend::Backend, extensions::ExtensionError}; +use crate::{backend::Backend, extensions::ExtensionError, types::Name}; pub enum CertificateError { DuplicateExtension(asn1::ObjectIdentifier), @@ -25,8 +25,12 @@ impl From for CertificateError { } pub(crate) trait CertificateExt<'a>: private::Sealed { + fn is_self_issued(&self) -> bool; fn is_self_signed(&self) -> bool; + fn issuer(&self) -> &Name<'a>; + fn subject(&self) -> &Name<'a>; + fn extensions(&'a self) -> Result, CertificateError>; fn authority_key_identifier(&'a self) -> Result>, CertificateError>; @@ -37,9 +41,21 @@ pub(crate) trait CertificateExt<'a>: private::Sealed { } impl<'a> CertificateExt<'a> for Certificate<'a> { + fn is_self_issued(&self) -> bool { + self.issuer() == self.subject() + } + fn is_self_signed(&self) -> bool { let pk = B::public_key(&self); - B::is_signed_by(&self, pk) + self.is_self_issued() && B::is_signed_by(&self, pk) + } + + fn issuer(&self) -> &Name<'a> { + &self.tbs_cert.issuer.unwrap_read() + } + + fn subject(&self) -> &Name<'a> { + &self.tbs_cert.subject.unwrap_read() } fn extensions(&'a self) -> Result, CertificateError> { diff --git a/src/rust/cryptography-x509-validation/src/lib.rs b/src/rust/cryptography-x509-validation/src/lib.rs index 46e5ed48b348c..5308fdf9d984e 100644 --- a/src/rust/cryptography-x509-validation/src/lib.rs +++ b/src/rust/cryptography-x509-validation/src/lib.rs @@ -46,21 +46,45 @@ impl<'a, B: Backend, P: policy::Profile> Validator<'a, B, P> { } fn build_chain( - &self, - working_cert: &Certificate, + &'a self, + working_cert: &'a Certificate, current_depth: u8, - ) -> Result, ValidationError> { + ) -> Result, ValidationError> { if current_depth > self.policy.max_chain_depth { return Err(PolicyError::Other("chain construction exceeds max depth").into()); } // Look in the store's root set to see if the working cert is listed. // If it is, we've reached the end. + // Observe that no issuer connection or signature verification happens + // here: inclusion in the root set implies a trust relationship, + // even if the working certificate is an EE or intermediate CA. + if self.store.trusted_certs.contains(working_cert) { + return Ok(vec![working_cert.clone()]); + } + + // Otherwise, we collect a list of potential issuers for this cert, + // and continue with the first that verifies. + for issuing_cert_candidate in self.store.potential_issuers(working_cert) { + // A candidate issuer is said to verify if it both + // signs for the working certificate and conforms to the + // policy. + if let Ok(next_depth) = + self.policy + .valid_issuer(issuing_cert_candidate, working_cert, current_depth) + { + let mut chain = vec![working_cert.clone()]; + chain.extend(self.build_chain(issuing_cert_candidate, next_depth)?); + return Ok(chain); + } + } - unimplemented!() + // We only reach this if we fail to hit our base case above, or if + // a chain building step fails to find a next valid certificate. + Err(PolicyError::Other("chain construction exhausted all candidates").into()) } - pub fn validate(&self, ee: &Certificate) -> Result, ValidationError> { + pub fn validate(&self, ee: &'a Certificate) -> Result, ValidationError> { // Before anything else, check whether the given EE cert // is well-formed according to our policy (and its underlying // certificate profile). diff --git a/src/rust/cryptography-x509-validation/src/policy/mod.rs b/src/rust/cryptography-x509-validation/src/policy/mod.rs index 2c77f970956e4..447532bc54ba5 100644 --- a/src/rust/cryptography-x509-validation/src/policy/mod.rs +++ b/src/rust/cryptography-x509-validation/src/policy/mod.rs @@ -96,18 +96,9 @@ impl Policy { } impl> Policy { - /// Returns a `Result` indicating whether the given `Certificate` is - /// considered well-formed under this policy's underlying profile. - /// - /// This does **not** indicate whether the `Certificate` is trustworthy - /// or otherwise valid. - pub(crate) fn profile_permits(&self, cert: &Certificate) -> Result<(), PolicyError> { - self.profile.permits_basic(cert)?; - - Ok(self - .profile - .permits_ca(cert) - .or_else(|_| self.profile.permits_ee(cert))?) + pub fn with_time(mut self, time: asn1::DateTime) -> Self { + self.validation_time = time; + self } fn permits_san(&self, cert: &Certificate) -> Result<(), PolicyError> { @@ -132,6 +123,15 @@ impl> Policy { } } + pub(crate) fn permits_ca(&self, cert: &Certificate) -> Result<(), PolicyError> { + self.profile.permits_basic(cert)?; + self.profile.permits_ca(cert)?; + + // TODO: Policy-level checks for EKUs, algorthms, etc. + + Ok(()) + } + pub(crate) fn permits_ee(&self, cert: &Certificate) -> Result<(), PolicyError> { // An end entity cert is considered "permitted" under a policy if: // 1. It satisfies the basic (EE and CA) requirements of the underlying profile; @@ -146,6 +146,29 @@ impl> Policy { Ok(()) } + + pub(crate) fn valid_issuer( + &self, + issuer: &Certificate, + child: &Certificate, + current_depth: u8, + ) -> Result { + // The issuer needs to be a valid CA. + self.permits_ca(issuer)?; + + // TODO: Check the pathlen constraint here. + + let pk = B::public_key(issuer); + if !B::is_signed_by(child, pk) { + return Err(PolicyError::Other("signature does not match")); + } + + // Self-issued issuers don't increase the working depth. + match issuer.is_self_issued() { + true => Ok(current_depth), + false => Ok(current_depth + 1), + } + } } #[cfg(test)] diff --git a/src/rust/cryptography-x509-validation/src/policy/profile/rfc5280.rs b/src/rust/cryptography-x509-validation/src/policy/profile/rfc5280.rs index 07a3442660fd4..ca83d2674d342 100644 --- a/src/rust/cryptography-x509-validation/src/policy/profile/rfc5280.rs +++ b/src/rust/cryptography-x509-validation/src/policy/profile/rfc5280.rs @@ -30,7 +30,7 @@ impl Default for RFC5280 { impl Profile for RFC5280 { fn permits_basic(&self, cert: &Certificate) -> Result<(), ProfileError> { - // 4.1.1.2: + // 4.1.1.2 / 4.1.2.3: // The top-level signatureAlgorithm and TBSCert signature algorithm // MUST match. if cert.signature_alg != cert.tbs_cert.spki.algorithm { @@ -39,7 +39,9 @@ impl Profile for RFC5280 { // 4.1.2.4: // The issuer MUST be a non-empty distinguished name. - // TODO + if cert.issuer().is_empty() { + return Err("certificate must have a non-empty Issuer".into()); + } // 4.1.2.5: // Validity dates before 2050 MUST be encoded as UTCTime; @@ -70,6 +72,12 @@ impl Profile for RFC5280 { } fn permits_ca(&self, cert: &Certificate) -> Result<(), ProfileError> { + // 4.1.2.6.: + // CA certificates MUST have a subject populated with a non-empty distinguished name. + if cert.subject().is_empty() { + return Err("CA certificate must have a non-empty Subject".into()); + } + // 4.2: // CA certificates must contain a few core extensions. This implies // that the CA certificate must be a v3 certificate, since earlier @@ -153,6 +161,9 @@ impl Profile for RFC5280 { } fn permits_ee(&self, _cert: &Certificate) -> Result<(), ProfileError> { + // 4.1.2.6: + // EE certificates MAY have their subject in either the subject or subjectAltName. + // If the subject is empty, then the subjectAltName MUST be marked critical. unimplemented!() } } diff --git a/src/rust/cryptography-x509-validation/src/trust_store.rs b/src/rust/cryptography-x509-validation/src/trust_store.rs index 44616261fdee5..3e236f0443a07 100644 --- a/src/rust/cryptography-x509-validation/src/trust_store.rs +++ b/src/rust/cryptography-x509-validation/src/trust_store.rs @@ -6,6 +6,8 @@ use std::collections::HashSet; use cryptography_x509::certificate::Certificate; +use crate::certificate::CertificateExt; + /// A `Store` represents the core state needed for X.509 path validation. pub struct Store<'a> { pub trusted_certs: HashSet>, @@ -14,12 +16,30 @@ pub struct Store<'a> { impl<'a> Store<'a> { pub fn new( - trusted: &impl IntoIterator>, - untrusted: &impl IntoIterator>, + trusted: impl IntoIterator>, + untrusted: impl IntoIterator>, ) -> Self { Store { trusted_certs: HashSet::from_iter(trusted), untrusted_certs: HashSet::from_iter(untrusted), } } + + pub(crate) fn potential_issuers( + &'a self, + cert: &'a Certificate, + ) -> impl Iterator { + // TODO: Optimizations: + // * Use a backing structure that allows us to search by name + // rather than doing a linear scan + // * Search by AKI and other identifiers? + self.untrusted_certs + .iter() + .filter(|candidate| candidate.subject() == cert.issuer()) + .chain( + self.trusted_certs + .iter() + .filter(|candidate| candidate.subject() == cert.issuer()), + ) + } } diff --git a/src/rust/cryptography-x509-validation/src/types.rs b/src/rust/cryptography-x509-validation/src/types.rs index 54e3bfa89b942..14e5d1927c753 100644 --- a/src/rust/cryptography-x509-validation/src/types.rs +++ b/src/rust/cryptography-x509-validation/src/types.rs @@ -4,6 +4,11 @@ //! Support types. +use cryptography_x509::common::AttributeTypeValue; + +/// Like `cryptography_x509::name::Name`, but with only the reading variant. +pub type Name<'a> = asn1::SequenceOf<'a, asn1::SetOf<'a, AttributeTypeValue<'a>>>; + /// Like `UnvalidatedIA5String`, but preserves the invariant that the /// underlying string is ASCII only. #[derive(Debug, PartialEq)]