diff --git a/src/cert.rs b/src/cert.rs index 27c0cbe0..87dd33d9 100644 --- a/src/cert.rs +++ b/src/cert.rs @@ -57,7 +57,7 @@ impl<'a> Cert<'a> { ee_or_ca: EndEntityOrCa<'a>, ) -> Result { let (tbs, signed_data) = cert_der.read_all(Error::BadDer, |cert_der| { - der::nested(cert_der, der::Tag::Sequence, Error::BadDer, |der| { + der::nested(cert_der, der::Tag::Sequence, |der| { // limited to SEQUENCEs of size 2^16 or less. SignedData::from_der(der, der::TWO_BYTE_DER_SIZE) }) @@ -105,22 +105,20 @@ impl<'a> Cert<'a> { }; if !tbs.at_end() { - der::nested( - tbs, - der::Tag::ContextSpecificConstructed3, - Error::MalformedExtensions, - |tagged| { - der::nested_of_mut( - tagged, - der::Tag::Sequence, - der::Tag::Sequence, - Error::BadDer, - |extension| { - remember_cert_extension(&mut cert, &Extension::from_der(extension)?) - }, - ) - }, - )?; + der::nested(tbs, der::Tag::ContextSpecificConstructed3, |tagged| { + der::nested_of_mut( + tagged, + der::Tag::Sequence, + der::Tag::Sequence, + |extension| { + remember_cert_extension(&mut cert, &Extension::from_der(extension)?) + }, + ) + }) + .map_err(|err| match err { + Error::BadDer => Error::MalformedExtensions, + err => err, + })?; } Ok(cert) @@ -160,19 +158,18 @@ impl<'a> Cert<'a> { // mozilla::pkix supports v1, v2, v3, and v4, including both the implicit // (correct) and explicit (incorrect) encoding of v1. We allow only v3. fn version3(input: &mut untrusted::Reader) -> Result<(), Error> { - der::nested( - input, - der::Tag::ContextSpecificConstructed0, - Error::UnsupportedCertVersion, - |input| { - let version = u8::from_der(input)?; - if version != 2 { - // v3 - return Err(Error::UnsupportedCertVersion); - } - Ok(()) - }, - ) + der::nested(input, der::Tag::ContextSpecificConstructed0, |input| { + let version = u8::from_der(input)?; + if version != 2 { + // v3 + return Err(Error::UnsupportedCertVersion); + } + Ok(()) + }) + .map_err(|err| match err { + Error::BadDer => Error::UnsupportedCertVersion, + err => err, + }) } pub(crate) fn lenient_certificate_serial_number<'a>( @@ -273,7 +270,7 @@ impl<'a> FromDer<'a> for CrlDistributionPoint<'a> { crl_issuer: None, }; - der::nested(reader, Tag::Sequence, Error::BadDer, |der| { + der::nested(reader, Tag::Sequence, |der| { const DISTRIBUTION_POINT_TAG: u8 = CONTEXT_SPECIFIC | CONSTRUCTED; const REASONS_TAG: u8 = CONTEXT_SPECIFIC | 1; const CRL_ISSUER_TAG: u8 = CONTEXT_SPECIFIC | CONSTRUCTED | 2; diff --git a/src/crl.rs b/src/crl.rs index be6f0a66..42d55e38 100644 --- a/src/crl.rs +++ b/src/crl.rs @@ -260,7 +260,6 @@ impl<'a> FromDer<'a> for BorrowedCertRevocationList<'a> { let (tbs_cert_list, signed_data) = der::nested_limited( reader, Tag::Sequence, - Error::BadDer, |signed_der| SignedData::from_der(signed_der, der::MAX_DER_SIZE), der::MAX_DER_SIZE, )?; @@ -334,27 +333,20 @@ impl<'a> FromDer<'a> for BorrowedCertRevocationList<'a> { // identifier (Section 5.2.1) and the CRL number (Section 5.2.3) // extensions in all CRLs issued. // As a result of the above we parse this as a required section, not OPTIONAL. - der::nested( - tbs_cert_list, - Tag::ContextSpecificConstructed0, - Error::MalformedExtensions, - |tagged| { - der::nested_of_mut( - tagged, - Tag::Sequence, - Tag::Sequence, - Error::BadDer, - |extension| { - // RFC 5280 §5.2: - // If a CRL contains a critical extension - // that the application cannot process, then the application MUST NOT - // use that CRL to determine the status of certificates. However, - // applications may ignore unrecognized non-critical extensions. - crl.remember_extension(&Extension::from_der(extension)?) - }, - ) - }, - )?; + der::nested(tbs_cert_list, Tag::ContextSpecificConstructed0, |tagged| { + der::nested_of_mut(tagged, Tag::Sequence, Tag::Sequence, |extension| { + // RFC 5280 §5.2: + // If a CRL contains a critical extension + // that the application cannot process, then the application MUST NOT + // use that CRL to determine the status of certificates. However, + // applications may ignore unrecognized non-critical extensions. + crl.remember_extension(&Extension::from_der(extension)?) + }) + }) + .map_err(|err| match err { + Error::BadDer => Error::MalformedExtensions, + _ => err, + })?; Ok(crl) })?; @@ -484,7 +476,7 @@ impl<'a> BorrowedRevokedCert<'a> { impl<'a> FromDer<'a> for BorrowedRevokedCert<'a> { fn from_der(reader: &mut untrusted::Reader<'a>) -> Result { - der::nested(reader, Tag::Sequence, Error::BadDer, |der| { + der::nested(reader, Tag::Sequence, |der| { // RFC 5280 §4.1.2.2: // Certificate users MUST be able to handle serialNumber values up to 20 octets. // Conforming CAs MUST NOT use serialNumber values longer than 20 octets. @@ -526,7 +518,7 @@ impl<'a> FromDer<'a> for BorrowedRevokedCert<'a> { let mut reader = untrusted::Reader::new(ext_seq); loop { - der::nested(&mut reader, Tag::Sequence, Error::BadDer, |ext_der| { + der::nested(&mut reader, Tag::Sequence, |ext_der| { // RFC 5280 §5.3: // If a CRL contains a critical CRL entry extension that the application cannot // process, then the application MUST NOT use that CRL to determine the @@ -665,39 +657,34 @@ impl<'a> IssuingDistributionPoint<'a> { } // RFC 5280 section §4.2.1.13: - der::nested( - &mut untrusted::Reader::new(der), - Tag::Sequence, - Error::BadDer, - |der| { - while !der.at_end() { - let (tag, value) = der::read_tag_and_get_value(der)?; - match tag { - DISTRIBUTION_POINT_TAG => { - set_extension_once(&mut result.distribution_point, || Ok(value))? - } - ONLY_CONTAINS_USER_CERTS_TAG => { - result.only_contains_user_certs = decode_bool(value)? - } - ONLY_CONTAINS_CA_CERTS_TAG => { - result.only_contains_ca_certs = decode_bool(value)? - } - ONLY_CONTAINS_SOME_REASONS_TAG => { - set_extension_once(&mut result.only_some_reasons, || { - der::bit_string_flags(value) - })? - } - INDIRECT_CRL_TAG => result.indirect_crl = decode_bool(value)?, - ONLY_CONTAINS_ATTRIBUTE_CERTS_TAG => { - result.only_contains_attribute_certs = decode_bool(value)? - } - _ => return Err(Error::BadDer), + der::nested(&mut untrusted::Reader::new(der), Tag::Sequence, |der| { + while !der.at_end() { + let (tag, value) = der::read_tag_and_get_value(der)?; + match tag { + DISTRIBUTION_POINT_TAG => { + set_extension_once(&mut result.distribution_point, || Ok(value))? } + ONLY_CONTAINS_USER_CERTS_TAG => { + result.only_contains_user_certs = decode_bool(value)? + } + ONLY_CONTAINS_CA_CERTS_TAG => { + result.only_contains_ca_certs = decode_bool(value)? + } + ONLY_CONTAINS_SOME_REASONS_TAG => { + set_extension_once(&mut result.only_some_reasons, || { + der::bit_string_flags(value) + })? + } + INDIRECT_CRL_TAG => result.indirect_crl = decode_bool(value)?, + ONLY_CONTAINS_ATTRIBUTE_CERTS_TAG => { + result.only_contains_attribute_certs = decode_bool(value)? + } + _ => return Err(Error::BadDer), } + } - Ok(()) - }, - )?; + Ok(()) + })?; // RFC 5280 4.2.1.10: // Conforming CRLs issuers MUST set the onlyContainsAttributeCerts boolean to FALSE. diff --git a/src/der.rs b/src/der.rs index bb491993..ec98c086 100644 --- a/src/der.rs +++ b/src/der.rs @@ -105,11 +105,10 @@ pub(crate) fn expect_tag_and_get_value_limited<'a>( pub(crate) fn nested_limited<'a, R>( input: &mut untrusted::Reader<'a>, tag: Tag, - error: Error, decoder: impl FnOnce(&mut untrusted::Reader<'a>) -> Result, size_limit: usize, ) -> Result { - expect_tag_and_get_value_limited(input, tag, size_limit)?.read_all(error, decoder) + expect_tag_and_get_value_limited(input, tag, size_limit)?.read_all(Error::BadDer, decoder) } // TODO: investigate taking decoder as a reference to reduce generated code @@ -117,10 +116,9 @@ pub(crate) fn nested_limited<'a, R>( pub(crate) fn nested<'a, R>( input: &mut untrusted::Reader<'a>, tag: Tag, - error: Error, decoder: impl FnOnce(&mut untrusted::Reader<'a>) -> Result, ) -> Result { - nested_limited(input, tag, error, decoder, TWO_BYTE_DER_SIZE) + nested_limited(input, tag, decoder, TWO_BYTE_DER_SIZE) } pub(crate) fn expect_tag<'a>( @@ -264,12 +262,11 @@ pub(crate) fn nested_of_mut<'a>( input: &mut untrusted::Reader<'a>, outer_tag: Tag, inner_tag: Tag, - error: Error, mut decoder: impl FnMut(&mut untrusted::Reader<'a>) -> Result<(), Error>, ) -> Result<(), Error> { - nested(input, outer_tag, error, |outer| { + nested(input, outer_tag, |outer| { loop { - nested(outer, inner_tag, error, |inner| decoder(inner))?; + nested(outer, inner_tag, |inner| decoder(inner))?; if outer.at_end() { break; } @@ -281,7 +278,7 @@ pub(crate) fn nested_of_mut<'a>( pub(crate) fn bit_string_with_no_unused_bits<'a>( input: &mut untrusted::Reader<'a>, ) -> Result, Error> { - nested(input, Tag::BitString, Error::BadDer, |value| { + nested(input, Tag::BitString, |value| { let unused_bits_at_end = value.read_byte().map_err(|_| Error::BadDer)?; if unused_bits_at_end != 0 { return Err(Error::BadDer); @@ -385,7 +382,7 @@ impl<'a> FromDer<'a> for bool { return Ok(false); } - nested(reader, Tag::Boolean, Error::BadDer, |input| { + nested(reader, Tag::Boolean, |input| { match input.read_byte() { Ok(0xff) => Ok(true), Ok(0x00) => Ok(false), diff --git a/src/subject_name/verify.rs b/src/subject_name/verify.rs index 84d03406..d05592fc 100644 --- a/src/subject_name/verify.rs +++ b/src/subject_name/verify.rs @@ -449,8 +449,8 @@ static COMMON_NAME: untrusted::Input = untrusted::Input::from(&[85, 4, 3]); fn common_name(input: untrusted::Input) -> Result, Error> { let inner = &mut untrusted::Reader::new(input); - der::nested(inner, der::Tag::Set, Error::BadDer, |tagged| { - der::nested(tagged, der::Tag::Sequence, Error::BadDer, |tagged| { + der::nested(inner, der::Tag::Set, |tagged| { + der::nested(tagged, der::Tag::Sequence, |tagged| { while !tagged.at_end() { let name_oid = der::expect_tag(tagged, der::Tag::OID)?; if name_oid == COMMON_NAME { diff --git a/src/time.rs b/src/time.rs index e22a2ac9..20aad2d6 100644 --- a/src/time.rs +++ b/src/time.rs @@ -70,7 +70,7 @@ impl<'a> FromDer<'a> for Time { Ok(value) } - der::nested(input, expected_tag, Error::BadDer, |value| { + der::nested(input, expected_tag, |value| { let (year_hi, year_lo) = if is_utc_time { let lo = read_two_digits(value, 0, 99)?; let hi = if lo >= 50 { 19 } else { 20 }; diff --git a/src/trust_anchor.rs b/src/trust_anchor.rs index dde6102b..ff44fd0b 100644 --- a/src/trust_anchor.rs +++ b/src/trust_anchor.rs @@ -53,8 +53,8 @@ impl<'a> TrustAnchor<'a> { fn from_v1_der(cert_der: untrusted::Input<'a>) -> Result { // X.509 Certificate: https://tools.ietf.org/html/rfc5280#section-4.1. cert_der.read_all(Error::BadDer, |cert_der| { - der::nested(cert_der, der::Tag::Sequence, Error::BadDer, |cert_der| { - let anchor = der::nested(cert_der, der::Tag::Sequence, Error::BadDer, |tbs| { + der::nested(cert_der, der::Tag::Sequence, |cert_der| { + let anchor = der::nested(cert_der, der::Tag::Sequence, |tbs| { // The version number field does not appear in v1 certificates. lenient_certificate_serial_number(tbs)?;