From 9bbd05f75d6feec80d901b262ea1e4296cee88e1 Mon Sep 17 00:00:00 2001 From: Derek Miller Date: Fri, 22 Apr 2022 16:01:30 -0500 Subject: [PATCH 1/2] Added an `unrecognized_extensions` hash to the Cert struct --- src/cert.rs | 53 ++++++++++++++++++++++++++++++++++++++--------------- src/der.rs | 1 + 2 files changed, 39 insertions(+), 15 deletions(-) diff --git a/src/cert.rs b/src/cert.rs index 75e022a1..2b62735d 100644 --- a/src/cert.rs +++ b/src/cert.rs @@ -13,6 +13,8 @@ // OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. use crate::{der, signed_data, Error}; +#[cfg(feature = "std")] +use std::collections::HashMap; pub enum EndEntityOrCa<'a> { EndEntity, @@ -32,6 +34,8 @@ pub struct Cert<'a> { pub eku: Option>, pub name_constraints: Option>, pub subject_alt_name: Option>, + #[cfg(feature = "std")] + pub unrecognized_extensions: HashMap<&'a[u8], untrusted::Input<'a>>, } pub fn parse_cert<'a>( @@ -93,6 +97,8 @@ pub(crate) fn parse_cert_internal<'a>( eku: None, name_constraints: None, subject_alt_name: None, + #[cfg(feature = "std")] + unrecognized_extensions: HashMap::new(), }; // mozilla::pkix allows the extensions to be omitted. However, since @@ -166,42 +172,59 @@ enum Understood { fn remember_extension<'a>( cert: &mut Cert<'a>, - extn_id: untrusted::Input, + extn_id: untrusted::Input<'a>, value: untrusted::Input<'a>, ) -> Result { // We don't do anything with certificate policies so we can safely ignore // all policy-related stuff. We assume that the policy-related extensions // are not marked critical. - // id-ce 2.5.29 - static ID_CE: [u8; 2] = oid![2, 5, 29]; - - if extn_id.len() != ID_CE.len() + 1 || !extn_id.as_slice_less_safe().starts_with(&ID_CE) { - return Ok(Understood::No); - } - - let out = match *extn_id.as_slice_less_safe().last().unwrap() { + let extension_id = &*extn_id.as_slice_less_safe(); + // note: the "strange" arrays below could be generated by the `oid!` macro. However, + // I can't find a way to make Rust let me put the output of that macro in a match expression. + // It looks like it's in progress, however (https://github.com/rust-lang/rust/issues/74446). + // Until then, I'm afraid we're left with this jankiness (suggestions appreciated) + let out = match extension_id { // id-ce-keyUsage 2.5.29.15. We ignore the KeyUsage extension. For CA // certificates, BasicConstraints.cA makes KeyUsage redundant. Firefox // and other common browsers do not check KeyUsage for end-entities, // though it would be kind of nice to ensure that a KeyUsage without // the keyEncipherment bit could not be used for RSA key exchange. - 15 => { + [85, 29, 15] => { return Ok(Understood::Yes); } // id-ce-subjectAltName 2.5.29.17 - 17 => &mut cert.subject_alt_name, + [85, 29, 17] => &mut cert.subject_alt_name, // id-ce-basicConstraints 2.5.29.19 - 19 => &mut cert.basic_constraints, + [85, 29, 19] => &mut cert.basic_constraints, // id-ce-nameConstraints 2.5.29.30 - 30 => &mut cert.name_constraints, + [85, 29, 30] => &mut cert.name_constraints, // id-ce-extKeyUsage 2.5.29.37 - 37 => &mut cert.eku, - + [85, 29, 37] => &mut cert.eku, + + //This is not a recognized extension, add it to the unrecognized + // extension hash and return `Understood::No` + #[cfg(feature = "std")] + _ => { + let value = value.read_all(Error::BadDER, |value| { + Ok(value.read_bytes_to_end()) + })?; + match cert.unrecognized_extensions.insert(extension_id, value) { + Some(_) => { + // There appears to be two unrecognized extensions with the same ID. + return Err(Error::ExtensionValueInvalid); + } + None => { + // Insertion was successful, and the extension is unique to the certificate + return Ok(Understood::No) + }, + } + } + #[cfg(not(feature = "std"))] _ => { return Ok(Understood::No); } diff --git a/src/der.rs b/src/der.rs index c4cad81b..97e1cfd0 100644 --- a/src/der.rs +++ b/src/der.rs @@ -165,6 +165,7 @@ pub fn time_choice(input: &mut untrusted::Reader) -> Result { }) } +#[allow(unused_macros)] macro_rules! oid { ( $first:expr, $second:expr, $( $tail:expr ),* ) => ( From 5ddfa7a18c4f88e1b033608c946d7bf46aaa7432 Mon Sep 17 00:00:00 2001 From: Derek Miller Date: Fri, 29 Apr 2022 10:50:10 -0500 Subject: [PATCH 2/2] Found a way to use the oid macro for the extension match expressions --- src/cert.rs | 15 ++++++++++----- src/der.rs | 1 - 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/src/cert.rs b/src/cert.rs index 2b62735d..fce500b5 100644 --- a/src/cert.rs +++ b/src/cert.rs @@ -178,6 +178,11 @@ fn remember_extension<'a>( // We don't do anything with certificate policies so we can safely ignore // all policy-related stuff. We assume that the policy-related extensions // are not marked critical. + const ID_CE_KEY_USAGE: &[u8] = &oid![2, 5, 29, 15]; + const ID_CE_SUBJECT_ALT_NAME: &[u8] = &oid![2, 5, 29, 17]; + const ID_CE_BASIC_CONSTRAINTS: &[u8] = &oid![2, 5, 29, 19]; + const ID_CE_NAME_CONSTRAINTS: &[u8] = &oid![2, 5, 29, 30]; + const ID_CE_EXT_KEY_USAGE: &[u8] = &oid![2, 5, 29, 37]; let extension_id = &*extn_id.as_slice_less_safe(); // note: the "strange" arrays below could be generated by the `oid!` macro. However, @@ -190,21 +195,21 @@ fn remember_extension<'a>( // and other common browsers do not check KeyUsage for end-entities, // though it would be kind of nice to ensure that a KeyUsage without // the keyEncipherment bit could not be used for RSA key exchange. - [85, 29, 15] => { + ID_CE_KEY_USAGE => { return Ok(Understood::Yes); } // id-ce-subjectAltName 2.5.29.17 - [85, 29, 17] => &mut cert.subject_alt_name, + ID_CE_SUBJECT_ALT_NAME => &mut cert.subject_alt_name, // id-ce-basicConstraints 2.5.29.19 - [85, 29, 19] => &mut cert.basic_constraints, + ID_CE_BASIC_CONSTRAINTS => &mut cert.basic_constraints, // id-ce-nameConstraints 2.5.29.30 - [85, 29, 30] => &mut cert.name_constraints, + ID_CE_NAME_CONSTRAINTS => &mut cert.name_constraints, // id-ce-extKeyUsage 2.5.29.37 - [85, 29, 37] => &mut cert.eku, + ID_CE_EXT_KEY_USAGE => &mut cert.eku, //This is not a recognized extension, add it to the unrecognized // extension hash and return `Understood::No` diff --git a/src/der.rs b/src/der.rs index 97e1cfd0..c4cad81b 100644 --- a/src/der.rs +++ b/src/der.rs @@ -165,7 +165,6 @@ pub fn time_choice(input: &mut untrusted::Reader) -> Result { }) } -#[allow(unused_macros)] macro_rules! oid { ( $first:expr, $second:expr, $( $tail:expr ),* ) => (