Skip to content

Commit

Permalink
Extract PathNode from Cert
Browse files Browse the repository at this point in the history
  • Loading branch information
djc committed Sep 8, 2023
1 parent 7ff3664 commit 547ba9c
Show file tree
Hide file tree
Showing 7 changed files with 150 additions and 130 deletions.
70 changes: 25 additions & 45 deletions src/cert.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,20 +17,14 @@ use crate::error::{DerTypeId, Error};
use crate::signed_data::SignedData;
use crate::x509::{remember_extension, set_extension_once, DistributionPointName, Extension};

/// An enumeration indicating whether a [`Cert`] is a leaf end-entity cert, or a linked
/// list node from the CA `Cert` to a child `Cert` it issued.
pub enum EndEntityOrCa<'a> {
/// The [`Cert`] is a leaf end-entity certificate.
EndEntity,

/// The [`Cert`] is an issuer certificate, and issued the referenced child `Cert`.
Ca(&'a Cert<'a>),
/// A node in a [`Cert`] path, represented as a linked list.
pub struct PathNode<'a> {
pub(crate) cert: &'a Cert<'a>,
pub(crate) issued: Option<&'a PathNode<'a>>,
}

/// A parsed X509 certificate.
pub struct Cert<'a> {
pub(crate) ee_or_ca: EndEntityOrCa<'a>,

pub(crate) serial: untrusted::Input<'a>,
pub(crate) signed_data: SignedData<'a>,
pub(crate) issuer: untrusted::Input<'a>,
Expand All @@ -51,10 +45,7 @@ pub struct Cert<'a> {
}

impl<'a> Cert<'a> {
pub(crate) fn from_der(
cert_der: untrusted::Input<'a>,
ee_or_ca: EndEntityOrCa<'a>,
) -> Result<Self, Error> {
pub(crate) fn from_der(cert_der: untrusted::Input<'a>) -> Result<Self, Error> {
let (tbs, signed_data) =
cert_der.read_all(Error::TrailingData(DerTypeId::Certificate), |cert_der| {
der::nested(
Expand Down Expand Up @@ -94,8 +85,6 @@ impl<'a> Cert<'a> {
// contain them.

let mut cert = Cert {
ee_or_ca,

signed_data,
serial,
issuer,
Expand Down Expand Up @@ -153,12 +142,6 @@ impl<'a> Cert<'a> {
self.subject.as_slice_less_safe()
}

/// Returns an indication of whether the certificate is an end-entity (leaf) certificate,
/// or a certificate authority.
pub fn end_entity_or_ca(&self) -> &EndEntityOrCa {
&self.ee_or_ca
}

/// Returns an iterator over the certificate's cRLDistributionPoints extension values, if any.
pub(crate) fn crl_distribution_points(
&self,
Expand Down Expand Up @@ -321,7 +304,7 @@ impl<'a> FromDer<'a> for CrlDistributionPoint<'a> {

#[cfg(test)]
mod tests {
use crate::cert::{Cert, EndEntityOrCa};
use crate::cert::Cert;
#[cfg(feature = "alloc")]
use crate::{
cert::{CrlDistributionPoint, DistributionPointName},
Expand All @@ -335,13 +318,11 @@ mod tests {
// is read correctly here instead of in tests/integration.rs.
fn test_serial_read() {
let ee = include_bytes!("../tests/misc/serial_neg_ee.der");
let cert = Cert::from_der(untrusted::Input::from(ee), EndEntityOrCa::EndEntity)
.expect("failed to parse certificate");
let cert = Cert::from_der(untrusted::Input::from(ee)).expect("failed to parse certificate");
assert_eq!(cert.serial.as_slice_less_safe(), &[255, 33, 82, 65, 17]);

let ee = include_bytes!("../tests/misc/serial_large_positive.der");
let cert = Cert::from_der(untrusted::Input::from(ee), EndEntityOrCa::EndEntity)
.expect("failed to parse certificate");
let cert = Cert::from_der(untrusted::Input::from(ee)).expect("failed to parse certificate");
assert_eq!(
cert.serial.as_slice_less_safe(),
&[
Expand All @@ -356,10 +337,9 @@ mod tests {
fn test_crl_distribution_point_netflix() {
let ee = include_bytes!("../tests/netflix/ee.der");
let inter = include_bytes!("../tests/netflix/inter.der");
let ee_cert = Cert::from_der(untrusted::Input::from(ee), EndEntityOrCa::EndEntity)
.expect("failed to parse EE cert");
let cert = Cert::from_der(untrusted::Input::from(inter), EndEntityOrCa::Ca(&ee_cert))
.expect("failed to parse certificate");
let ee_cert = Cert::from_der(untrusted::Input::from(ee)).expect("failed to parse EE cert");
let cert =
Cert::from_der(untrusted::Input::from(inter)).expect("failed to parse certificate");

// The end entity certificate shouldn't have a distribution point.
assert!(ee_cert.crl_distribution_points.is_none());
Expand Down Expand Up @@ -423,8 +403,8 @@ mod tests {
#[cfg(feature = "alloc")]
fn test_crl_distribution_point_with_reasons() {
let der = include_bytes!("../tests/crl_distrib_point/with_reasons.der");
let cert = Cert::from_der(untrusted::Input::from(der), EndEntityOrCa::EndEntity)
.expect("failed to parse certificate");
let cert =
Cert::from_der(untrusted::Input::from(der)).expect("failed to parse certificate");

// We expect to be able to parse the intermediate certificate's CRL distribution points.
let crl_distribution_points = cert
Expand Down Expand Up @@ -462,8 +442,8 @@ mod tests {
#[cfg(feature = "alloc")]
fn test_crl_distribution_point_with_crl_issuer() {
let der = include_bytes!("../tests/crl_distrib_point/with_crl_issuer.der");
let cert = Cert::from_der(untrusted::Input::from(der), EndEntityOrCa::EndEntity)
.expect("failed to parse certificate");
let cert =
Cert::from_der(untrusted::Input::from(der)).expect("failed to parse certificate");

// We expect to be able to parse the intermediate certificate's CRL distribution points.
let crl_distribution_points = cert
Expand All @@ -490,8 +470,8 @@ mod tests {
// Created w/
// ascii2der -i tests/crl_distrib_point/unknown_tag.der.txt -o tests/crl_distrib_point/unknown_tag.der
let der = include_bytes!("../tests/crl_distrib_point/unknown_tag.der");
let cert = Cert::from_der(untrusted::Input::from(der), EndEntityOrCa::EndEntity)
.expect("failed to parse certificate");
let cert =
Cert::from_der(untrusted::Input::from(der)).expect("failed to parse certificate");

// We expect there to be a distribution point extension, but parsing it should fail
// due to the unknown tag in the SEQUENCE.
Expand All @@ -508,8 +488,8 @@ mod tests {
// Created w/
// ascii2der -i tests/crl_distrib_point/only_reasons.der.txt -o tests/crl_distrib_point/only_reasons.der
let der = include_bytes!("../tests/crl_distrib_point/only_reasons.der");
let cert = Cert::from_der(untrusted::Input::from(der), EndEntityOrCa::EndEntity)
.expect("failed to parse certificate");
let cert =
Cert::from_der(untrusted::Input::from(der)).expect("failed to parse certificate");

// We expect there to be a distribution point extension, but parsing it should fail
// because no distribution points or cRLIssuer are set in the SEQUENCE, just reason codes.
Expand All @@ -524,8 +504,8 @@ mod tests {
#[cfg(feature = "alloc")]
fn test_crl_distribution_point_name_relative_to_issuer() {
let der = include_bytes!("../tests/crl_distrib_point/dp_name_relative_to_issuer.der");
let cert = Cert::from_der(untrusted::Input::from(der), EndEntityOrCa::EndEntity)
.expect("failed to parse certificate");
let cert =
Cert::from_der(untrusted::Input::from(der)).expect("failed to parse certificate");

// We expect to be able to parse the intermediate certificate's CRL distribution points.
let crl_distribution_points = cert
Expand Down Expand Up @@ -564,8 +544,8 @@ mod tests {
// Created w/
// ascii2der -i tests/crl_distrib_point/unknown_dp_name_tag.der.txt > tests/crl_distrib_point/unknown_dp_name_tag.der
let der = include_bytes!("../tests/crl_distrib_point/unknown_dp_name_tag.der");
let cert = Cert::from_der(untrusted::Input::from(der), EndEntityOrCa::EndEntity)
.expect("failed to parse certificate");
let cert =
Cert::from_der(untrusted::Input::from(der)).expect("failed to parse certificate");

// We expect to be able to parse the intermediate certificate's CRL distribution points.
let crl_distribution_points = cert
Expand All @@ -589,8 +569,8 @@ mod tests {
#[cfg(feature = "alloc")]
fn test_crl_distribution_point_multiple() {
let der = include_bytes!("../tests/crl_distrib_point/multiple_distribution_points.der");
let cert = Cert::from_der(untrusted::Input::from(der), EndEntityOrCa::EndEntity)
.expect("failed to parse certificate");
let cert =
Cert::from_der(untrusted::Input::from(der)).expect("failed to parse certificate");

// We expect to be able to parse the intermediate certificate's CRL distribution points.
let crl_distribution_points = cert
Expand Down
33 changes: 21 additions & 12 deletions src/crl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@

use pki_types::SignatureVerificationAlgorithm;

use crate::cert::{lenient_certificate_serial_number, Cert, EndEntityOrCa};
use crate::cert::{lenient_certificate_serial_number, PathNode};
use crate::der::{self, DerIterator, FromDer, Tag, CONSTRUCTED, CONTEXT_SPECIFIC};
use crate::error::{DerTypeId, Error};
use crate::signed_data::{self, SignedData};
Expand Down Expand Up @@ -771,17 +771,17 @@ impl<'a> IssuingDistributionPoint<'a> {
/// * Distribution point names relative to an issuer.
/// * General names of a type other than URI.
/// * Malformed names or invalid IDP or CRL DP extensions.
pub(crate) fn authoritative_for(&self, cert: &Cert<'a>) -> bool {
pub(crate) fn authoritative_for(&self, node: &PathNode<'a>) -> bool {
assert!(!self.only_contains_attribute_certs); // We check this at time of parse.

// Check that the scope of the CRL issuing distribution point could include the cert.
if self.only_contains_ca_certs && matches!(cert.ee_or_ca, EndEntityOrCa::EndEntity)
|| self.only_contains_user_certs && matches!(cert.ee_or_ca, EndEntityOrCa::Ca(_))
if self.only_contains_ca_certs && node.issued.is_none()
|| self.only_contains_user_certs && node.issued.is_some()
{
return false;
}

let cert_dps = match cert.crl_distribution_points() {
let cert_dps = match node.cert.crl_distribution_points() {
// If the certificate has no distribution points, then the CRL can be authoritative
// based on the issuer matching and the scope including the cert.
None => return true,
Expand Down Expand Up @@ -856,8 +856,8 @@ mod tests {
use alloc::vec::Vec;

use crate::{
crl::IssuingDistributionPoint, subject_name::GeneralName, x509::DistributionPointName,
BorrowedCertRevocationList, Cert, CertRevocationList, EndEntityOrCa, Error,
cert::PathNode, crl::IssuingDistributionPoint, subject_name::GeneralName,
x509::DistributionPointName, BorrowedCertRevocationList, Cert, CertRevocationList, Error,
RevocationReason,
};

Expand Down Expand Up @@ -999,10 +999,16 @@ mod tests {

// The IDP shouldn't be considered authoritative for a CA Cert.
let ee = include_bytes!("../tests/client_auth_revocation/no_crl_ku_chain.ee.der");
let ee = Cert::from_der(untrusted::Input::from(&ee[..]), EndEntityOrCa::EndEntity).unwrap();
let ee = Cert::from_der(untrusted::Input::from(&ee[..])).unwrap();
let ca = include_bytes!("../tests/client_auth_revocation/no_crl_ku_chain.int.a.ca.der");
let ca = Cert::from_der(untrusted::Input::from(&ca[..]), EndEntityOrCa::Ca(&ee)).unwrap();
assert!(!crl_issuing_dp.authoritative_for(&ca))
let ca = Cert::from_der(untrusted::Input::from(&ca[..])).unwrap();
assert!(!crl_issuing_dp.authoritative_for(&PathNode {
cert: &ca,
issued: Some(&PathNode {
cert: &ee,
issued: None
}),
}))
}

#[test]
Expand All @@ -1023,8 +1029,11 @@ mod tests {

// The IDP shouldn't be considered authoritative for an EE Cert.
let ee = include_bytes!("../tests/client_auth_revocation/no_crl_ku_chain.ee.der");
let ee = Cert::from_der(untrusted::Input::from(&ee[..]), EndEntityOrCa::EndEntity).unwrap();
assert!(!crl_issuing_dp.authoritative_for(&ee))
let ee = Cert::from_der(untrusted::Input::from(&ee[..])).unwrap();
assert!(!crl_issuing_dp.authoritative_for(&PathNode {
cert: &ee,
issued: None
}))
}

#[test]
Expand Down
12 changes: 6 additions & 6 deletions src/end_entity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use pki_types::{CertificateDer, SignatureVerificationAlgorithm, TrustAnchor};
#[cfg(feature = "alloc")]
use crate::subject_name::GeneralDnsNameRef;
use crate::{
cert, signed_data, subject_name, verify_cert, Error, KeyUsage, RevocationOptions,
cert, signed_data, subject_name, verify_cert, Error, KeyUsage, PathNode, RevocationOptions,
SubjectNameRef, Time,
};

Expand Down Expand Up @@ -63,10 +63,7 @@ impl<'a> TryFrom<&'a CertificateDer<'a>> for EndEntityCert<'a> {
/// `cert_der`.
fn try_from(cert: &'a CertificateDer<'a>) -> Result<Self, Self::Error> {
Ok(Self {
inner: cert::Cert::from_der(
untrusted::Input::from(cert.as_ref()),
cert::EndEntityOrCa::EndEntity,
)?,
inner: cert::Cert::from_der(untrusted::Input::from(cert.as_ref()))?,
})
}
}
Expand Down Expand Up @@ -108,7 +105,10 @@ impl<'a> EndEntityCert<'a> {
intermediate_certs,
revocation,
},
&self.inner,
&PathNode {
cert: &self.inner,
issued: None,
},
time,
)
}
Expand Down
2 changes: 1 addition & 1 deletion src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ mod x509;
pub(crate) mod test_utils;

pub use {
cert::{Cert, EndEntityOrCa},
cert::{Cert, PathNode},
crl::{BorrowedCertRevocationList, BorrowedRevokedCert, CertRevocationList, RevocationReason},
end_entity::EndEntityCert,
error::{DerTypeId, Error},
Expand Down
14 changes: 7 additions & 7 deletions src/subject_name/verify.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ use super::dns_name::{self, DnsNameRef};
use super::dns_name::{GeneralDnsNameRef, WildcardDnsNameRef};
use super::ip_address::{self, IpAddrRef};
use super::name::SubjectNameRef;
use crate::cert::{Cert, EndEntityOrCa};
use crate::cert::PathNode;
use crate::der::{self, FromDer};
use crate::error::{DerTypeId, Error};
use crate::verify_cert::Budget;
Expand Down Expand Up @@ -94,7 +94,7 @@ pub(crate) fn verify_cert_subject_name(
// https://tools.ietf.org/html/rfc5280#section-4.2.1.10
pub(crate) fn check_name_constraints(
constraints: Option<&mut untrusted::Reader>,
subordinate_certs: &Cert,
subordinate_certs: &PathNode<'_>,
budget: &mut Budget,
) -> Result<(), Error> {
let constraints = match constraints {
Expand All @@ -117,8 +117,8 @@ pub(crate) fn check_name_constraints(

let mut child = subordinate_certs;
loop {
let result =
NameIterator::new(Some(child.subject), child.subject_alt_name).find_map(|result| {
let result = NameIterator::new(Some(child.cert.subject), child.cert.subject_alt_name)
.find_map(|result| {
let name = match result {
Ok(name) => name,
Err(err) => return Some(Err(err)),
Expand All @@ -136,9 +136,9 @@ pub(crate) fn check_name_constraints(
return Err(err);
}

child = match child.ee_or_ca {
EndEntityOrCa::Ca(child_cert) => child_cert,
EndEntityOrCa::EndEntity => {
child = match child.issued {
Some(child_cert) => child_cert,
None => {
break;
}
};
Expand Down
4 changes: 2 additions & 2 deletions src/trust_anchor.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use pki_types::{CertificateDer, TrustAnchor};

use crate::cert::{lenient_certificate_serial_number, Cert, EndEntityOrCa};
use crate::cert::{lenient_certificate_serial_number, Cert};
use crate::der;
use crate::error::{DerTypeId, Error};

Expand All @@ -20,7 +20,7 @@ pub fn extract_trust_anchor<'a>(cert: &'a CertificateDer<'a>) -> Result<TrustAnc
// certificate using a special parser for v1 certificates. Notably, that
// parser doesn't allow extensions, so there's no need to worry about
// embedded name constraints in a v1 certificate.
match Cert::from_der(cert_der, EndEntityOrCa::EndEntity) {
match Cert::from_der(cert_der) {
Ok(cert) => Ok(TrustAnchor::from(cert)),
Err(Error::UnsupportedCertVersion) => {
extract_trust_anchor_from_v1_cert_der(cert_der).or(Err(Error::BadDer))
Expand Down
Loading

0 comments on commit 547ba9c

Please sign in to comment.