diff --git a/src/error.rs b/src/error.rs index a8ad9789..a40b562e 100644 --- a/src/error.rs +++ b/src/error.rs @@ -13,6 +13,7 @@ // OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. use core::fmt; +use core::ops::ControlFlow; /// An error that occurs during certificate validation or name validation. #[derive(Clone, Copy, Debug, PartialEq, Eq)] @@ -259,6 +260,17 @@ impl Error { } } +impl From for ControlFlow { + fn from(value: Error) -> Self { + match value { + // If an error is fatal, we've exhausted the potential for continued search. + err if err.is_fatal() => Self::Break(err), + // Otherwise we've rejected one candidate chain, but may continue to search for others. + err => Self::Continue(err), + } + } +} + impl fmt::Display for Error { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { write!(f, "{:?}", self) diff --git a/src/verify_cert.rs b/src/verify_cert.rs index d878ae72..9fcd280e 100644 --- a/src/verify_cert.rs +++ b/src/verify_cert.rs @@ -13,6 +13,7 @@ // OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. use core::default::Default; +use core::ops::ControlFlow; use crate::{ cert::{Cert, EndEntityOrCa}, @@ -29,7 +30,10 @@ pub(crate) struct ChainOptions<'a> { } pub(crate) fn build_chain(opts: &ChainOptions, cert: &Cert, time: time::Time) -> Result<(), Error> { - build_chain_inner(opts, cert, time, 0, &mut Budget::default()) + build_chain_inner(opts, cert, time, 0, &mut Budget::default()).map_err(|e| match e { + ControlFlow::Break(err) => err, + ControlFlow::Continue(err) => err, + }) } fn build_chain_inner( @@ -38,7 +42,7 @@ fn build_chain_inner( time: time::Time, sub_ca_count: usize, budget: &mut Budget, -) -> Result<(), Error> { +) -> Result<(), ControlFlow> { let used_as_ca = used_as_ca(&cert.ee_or_ca); check_issuer_independent_properties(cert, time, used_as_ca, sub_ca_count, opts.eku.inner)?; @@ -50,7 +54,7 @@ fn build_chain_inner( const MAX_SUB_CA_COUNT: usize = 6; if sub_ca_count >= MAX_SUB_CA_COUNT { - return Err(Error::MaximumPathDepthExceeded); + return Err(Error::MaximumPathDepthExceeded.into()); } } UsedAsCa::No => { @@ -64,7 +68,7 @@ fn build_chain_inner( |trust_anchor: &TrustAnchor| { let trust_anchor_subject = untrusted::Input::from(trust_anchor.subject); if cert.issuer != trust_anchor_subject { - return Err(Error::UnknownIssuer); + return Err(Error::UnknownIssuer.into()); } // TODO: check_distrust(trust_anchor_subject, trust_anchor_spki)?; @@ -86,11 +90,11 @@ fn build_chain_inner( let err = match result { Ok(()) => return Ok(()), // Fatal errors should halt further path building. - res @ Err(err) if err.is_fatal() => return res, + res @ Err(ControlFlow::Break(_)) => return res, // Non-fatal errors should be carried forward as the default_error for subsequent // loop_while_non_fatal_error processing and only returned once all other path-building // options have been exhausted. - Err(err) => err, + Err(ControlFlow::Continue(err)) => err, }; loop_while_non_fatal_error(err, opts.intermediate_certs, |cert_der| { @@ -98,7 +102,7 @@ fn build_chain_inner( Cert::from_der(untrusted::Input::from(cert_der), EndEntityOrCa::Ca(cert))?; if potential_issuer.subject != cert.issuer { - return Err(Error::UnknownIssuer); + return Err(Error::UnknownIssuer.into()); } // Prevent loops; see RFC 4158 section 5.2. @@ -107,7 +111,7 @@ fn build_chain_inner( if potential_issuer.spki.value() == prev.spki.value() && potential_issuer.subject == prev.subject { - return Err(Error::UnknownIssuer); + return Err(Error::UnknownIssuer.into()); } match &prev.ee_or_ca { EndEntityOrCa::EndEntity => { @@ -135,7 +139,7 @@ fn check_signed_chain( trust_anchor: &TrustAnchor, crls: &[&dyn CertRevocationList], budget: &mut Budget, -) -> Result<(), Error> { +) -> Result<(), ControlFlow> { let mut spki_value = untrusted::Input::from(trust_anchor.spki); let mut issuer_subject = untrusted::Input::from(trust_anchor.subject); let mut issuer_key_usage = None; // TODO(XXX): Consider whether to track TrustAnchor KU. @@ -175,7 +179,7 @@ fn check_signed_chain_name_constraints( cert_chain: &Cert, trust_anchor: &TrustAnchor, budget: &mut Budget, -) -> Result<(), Error> { +) -> Result<(), ControlFlow> { let mut cert = cert_chain; let mut name_constraints = trust_anchor .name_constraints @@ -567,8 +571,8 @@ impl KeyUsageMode { fn loop_while_non_fatal_error( default_error: Error, values: V, - mut f: impl FnMut(V::Item) -> Result<(), Error>, -) -> Result<(), Error> + mut f: impl FnMut(V::Item) -> Result<(), ControlFlow>, +) -> Result<(), ControlFlow> where V: IntoIterator, { @@ -577,13 +581,13 @@ where match f(v) { Ok(()) => return Ok(()), // Fatal errors should halt further looping. - res @ Err(err) if err.is_fatal() => return res, + res @ Err(ControlFlow::Break(_)) => return res, // Non-fatal errors should be ranked by specificity and only returned // once all other path-building options have been exhausted. - Err(new_error) => error = error.most_specific(new_error), + Err(ControlFlow::Continue(new_error)) => error = error.most_specific(new_error), } } - Err(error) + Err(error.into()) } #[cfg(test)] @@ -610,7 +614,7 @@ mod tests { intermediate_count: usize, trust_anchor_is_actual_issuer: TrustAnchorIsActualIssuer, budget: Option, - ) -> Error { + ) -> ControlFlow { let ca_cert = make_issuer("Bogus Subject", None); let ca_cert_der = ca_cert.serialize_der().unwrap(); @@ -639,16 +643,16 @@ mod tests { #[test] #[cfg(feature = "alloc")] fn test_too_many_signatures() { - assert_eq!( + assert!(matches!( build_degenerate_chain(5, TrustAnchorIsActualIssuer::Yes, None), - Error::MaximumSignatureChecksExceeded - ); + ControlFlow::Break(Error::MaximumSignatureChecksExceeded) + )); } #[test] #[cfg(feature = "alloc")] fn test_too_many_path_calls() { - assert_eq!( + assert!(matches!( build_degenerate_chain( 10, TrustAnchorIsActualIssuer::No, @@ -660,12 +664,12 @@ mod tests { ..Budget::default() }) ), - Error::MaximumPathBuildCallsExceeded - ); + ControlFlow::Break(Error::MaximumPathBuildCallsExceeded) + )); } #[cfg(feature = "alloc")] - fn build_linear_chain(chain_length: usize) -> Result<(), Error> { + fn build_linear_chain(chain_length: usize) -> Result<(), ControlFlow> { let ca_cert = make_issuer(format!("Bogus Subject {chain_length}"), None); let ca_cert_der = ca_cert.serialize_der().unwrap(); @@ -689,18 +693,21 @@ mod tests { #[test] #[cfg(feature = "alloc")] fn longest_allowed_path() { - assert_eq!(build_linear_chain(1), Ok(())); - assert_eq!(build_linear_chain(2), Ok(())); - assert_eq!(build_linear_chain(3), Ok(())); - assert_eq!(build_linear_chain(4), Ok(())); - assert_eq!(build_linear_chain(5), Ok(())); - assert_eq!(build_linear_chain(6), Ok(())); + assert!(build_linear_chain(1).is_ok()); + assert!(build_linear_chain(2).is_ok()); + assert!(build_linear_chain(3).is_ok()); + assert!(build_linear_chain(4).is_ok()); + assert!(build_linear_chain(5).is_ok()); + assert!(build_linear_chain(6).is_ok()); } #[test] #[cfg(feature = "alloc")] fn path_too_long() { - assert_eq!(build_linear_chain(7), Err(Error::MaximumPathDepthExceeded)); + assert!(matches!( + build_linear_chain(7), + Err(ControlFlow::Continue(Error::MaximumPathDepthExceeded)) + )); } #[test] @@ -750,13 +757,13 @@ mod tests { // Validation should succeed with the name constraint comparison budget allocated above. // This shows that we're not consuming budget on unused intermediates: we didn't budget // enough comparisons for that to pass the overall chain building. - verify_chain( + assert!(verify_chain( &ca_cert_der, &intermediates_der, &ee_cert, Some(passing_budget), ) - .unwrap(); + .is_ok()); let failing_budget = Budget { // See passing_budget: 2 comparisons is not sufficient. @@ -773,7 +780,12 @@ mod tests { Some(failing_budget), ); - assert_eq!(result, Err(Error::MaximumNameConstraintComparisonsExceeded)); + assert!(matches!( + result, + Err(ControlFlow::Break( + Error::MaximumNameConstraintComparisonsExceeded + )) + )); } #[cfg(feature = "alloc")] @@ -782,7 +794,7 @@ mod tests { intermediates_der: &[Vec], ee_cert_der: &[u8], budget: Option, - ) -> Result<(), Error> { + ) -> Result<(), ControlFlow> { use crate::ECDSA_P256_SHA256; use crate::{EndEntityCert, Time};