verify_cert: apply path building budget

This is intended to be complementary to the signature validation limit
fix and addresses briansmith/webpki#276 in the same manner as NSS
libmozpkix.
This commit is contained in:
Daniel McCarney 2023-09-08 14:03:37 -04:00 committed by Brian Smith
parent 3ee04be687
commit f566cf1b3b
1 changed files with 69 additions and 23 deletions

View File

@ -51,6 +51,8 @@ pub fn build_chain(
/// for testing).
enum InternalError {
MaximumSignatureChecksExceeded,
/// The maximum number of internal path building calls has been reached. Path complexity is too great.
MaximumPathBuildCallsExceeded,
}
enum ErrorOrInternalError {
@ -62,7 +64,8 @@ impl ErrorOrInternalError {
fn is_fatal(&self) -> bool {
match self {
ErrorOrInternalError::Error(_) => false,
ErrorOrInternalError::InternalError(InternalError::MaximumSignatureChecksExceeded) => {
ErrorOrInternalError::InternalError(InternalError::MaximumSignatureChecksExceeded)
| ErrorOrInternalError::InternalError(InternalError::MaximumPathBuildCallsExceeded) => {
true
}
}
@ -185,6 +188,7 @@ fn build_chain_inner(
UsedAsCa::Yes => sub_ca_count + 1,
};
budget.consume_build_chain_call()?;
build_chain_inner(
required_eku_if_present,
supported_sig_algs,
@ -228,6 +232,7 @@ fn check_signatures(
struct Budget {
signatures: usize,
build_chain_calls: usize,
}
impl Budget {
@ -239,6 +244,15 @@ impl Budget {
.ok_or(InternalError::MaximumSignatureChecksExceeded)?;
Ok(())
}
#[inline]
fn consume_build_chain_call(&mut self) -> Result<(), InternalError> {
self.build_chain_calls = self
.build_chain_calls
.checked_sub(1)
.ok_or(InternalError::MaximumPathBuildCallsExceeded)?;
Ok(())
}
}
impl Default for Budget {
@ -249,6 +263,10 @@ impl Default for Budget {
// being hit in real applications (see <https://github.com/spiffe/spire/issues/1004>).
// So this may actually be too aggressive.
signatures: 100,
// This limit is taken from NSS libmozpkix, see:
// <https://github.com/nss-dev/nss/blob/bb4a1d38dd9e92923525ac6b5ed0288479f3f3fc/lib/mozpkix/lib/pkixbuild.cpp#L381-L393>
build_chain_calls: 200_000,
}
}
}
@ -453,25 +471,35 @@ where
Err(Error::UnknownIssuer.into())
}
#[cfg(feature = "alloc")]
#[cfg(test)]
mod tests {
use alloc::string::ToString;
use alloc::vec::Vec;
use core::convert::TryFrom;
#[test]
#[cfg(feature = "alloc")]
fn test_too_many_signatures() {
use super::*;
use super::*;
use crate::verify_cert::{Budget, InternalError};
enum ChainTrustAnchor {
InChain,
NotInChain,
}
fn build_degenerate_chain(
intermediate_count: usize,
trust_anchor: ChainTrustAnchor,
) -> ErrorOrInternalError {
use crate::ECDSA_P256_SHA256;
use crate::{EndEntityCert, Time};
use alloc::{string::ToString, vec::Vec};
use core::convert::TryFrom;
let alg = &rcgen::PKCS_ECDSA_P256_SHA256;
let make_issuer = || {
let make_issuer = |org_name| {
let mut ca_params = rcgen::CertificateParams::new(Vec::new());
ca_params
.distinguished_name
.push(rcgen::DnType::OrganizationName, "Bogus Subject");
.push(rcgen::DnType::OrganizationName, org_name);
ca_params.is_ca = rcgen::IsCa::Ca(rcgen::BasicConstraints::Unconstrained);
ca_params.key_usages = vec![
rcgen::KeyUsagePurpose::KeyCertSign,
@ -482,13 +510,17 @@ mod tests {
rcgen::Certificate::from_params(ca_params).unwrap()
};
let ca_cert = make_issuer();
let ca_cert = make_issuer("Bogus Subject");
let ca_cert_der = ca_cert.serialize_der().unwrap();
let mut intermediates = Vec::with_capacity(101);
let mut intermediates = Vec::with_capacity(intermediate_count);
if let ChainTrustAnchor::InChain = trust_anchor {
intermediates.push(ca_cert_der.to_vec());
}
let mut issuer = ca_cert;
for _ in 0..101 {
let intermediate = make_issuer();
for _ in 0..intermediate_count {
let intermediate = make_issuer("Bogus Subject");
let intermediate_der = intermediate.serialize_der_with_signer(&issuer).unwrap();
intermediates.push(intermediate_der);
issuer = intermediate;
@ -500,29 +532,43 @@ mod tests {
let ee_cert = rcgen::Certificate::from_params(ee_params).unwrap();
let ee_cert_der = ee_cert.serialize_der_with_signer(&issuer).unwrap();
let anchors = &[TrustAnchor::try_from_cert_der(&ca_cert_der).unwrap()];
let trust_anchor = match trust_anchor {
ChainTrustAnchor::InChain => make_issuer("Bogus Trust Anchor").serialize_der().unwrap(),
ChainTrustAnchor::NotInChain => ca_cert_der.clone(),
};
let anchors = &[TrustAnchor::try_from_cert_der(&trust_anchor).unwrap()];
let time = Time::from_seconds_since_unix_epoch(0x1fed_f00d);
let cert = EndEntityCert::try_from(&ee_cert_der[..]).unwrap();
let intermediates_der: Vec<&[u8]> = intermediates.iter().map(|x| x.as_ref()).collect();
let intermediate_certs: &[&[u8]] = intermediates_der.as_ref();
let intermediate_certs = intermediates.iter().map(|x| x.as_ref()).collect::<Vec<_>>();
// TODO: Use `build_chain` when `Error` is made non-exhaustive.
let result = build_chain_inner(
build_chain_inner(
EKU_SERVER_AUTH,
&[&ECDSA_P256_SHA256],
anchors,
intermediate_certs,
&intermediate_certs,
cert.inner(),
time,
0,
&mut Budget::default(),
);
)
.unwrap_err()
}
#[test]
fn test_too_many_signatures() {
assert!(matches!(
build_degenerate_chain(5, ChainTrustAnchor::NotInChain),
ErrorOrInternalError::InternalError(InternalError::MaximumSignatureChecksExceeded),
));
}
#[test]
fn test_too_many_path_calls() {
let result = build_degenerate_chain(10, ChainTrustAnchor::InChain);
assert!(matches!(
result,
Err(ErrorOrInternalError::InternalError(
InternalError::MaximumSignatureChecksExceeded
))
ErrorOrInternalError::InternalError(InternalError::MaximumPathBuildCallsExceeded),
));
}
}