From 1e25841893c56c6046da86691e780ead8bf1a6d7 Mon Sep 17 00:00:00 2001 From: Peter Goodspeed-Niklaus Date: Tue, 28 Apr 2026 15:42:52 +0200 Subject: [PATCH 1/4] chore: `cargo fix -p certval` --- certval/src/environment/pki_environment.rs | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/certval/src/environment/pki_environment.rs b/certval/src/environment/pki_environment.rs index 12a9e19a..3cc0a051 100644 --- a/certval/src/environment/pki_environment.rs +++ b/certval/src/environment/pki_environment.rs @@ -74,19 +74,19 @@ pub struct PkiEnvironment { //Storage and retrieval interfaces //-------------------------------------------------------------------------- /// List of trait objects that provide access to trust anchors - trust_anchor_sources: Vec>, + trust_anchor_sources: Vec>, /// List of trait objects that provide access to certificates - certificate_sources: Vec>, + certificate_sources: Vec>, /// List of trait objects that provide access to CRLs - crl_sources: Vec>, + crl_sources: Vec>, /// List of trait objects that provide access to cached revocation status determinations - revocation_cache: Vec>, + revocation_cache: Vec>, /// List of trait objects that provide access to blocklist and last modified info - check_remote: Vec>, + check_remote: Vec>, //-------------------------------------------------------------------------- //Miscellaneous interfaces @@ -268,7 +268,7 @@ impl PkiEnvironment { } /// add_trust_anchor_source adds a [`TrustAnchorSource`] object to the list used by get_trust_anchor. - pub fn add_trust_anchor_source(&mut self, c: Box<(dyn TrustAnchorSource + Send + Sync)>) { + pub fn add_trust_anchor_source(&mut self, c: Box) { self.trust_anchor_sources.push(c); } @@ -366,7 +366,7 @@ impl PkiEnvironment { } /// add_certificate_source adds a [`CertificateSource`] object to the list. - pub fn add_certificate_source(&mut self, c: Box<(dyn CertificateSource + Send + Sync)>) { + pub fn add_certificate_source(&mut self, c: Box) { self.certificate_sources.push(c); } @@ -398,7 +398,7 @@ impl PkiEnvironment { } /// add_crl_source adds a [`CrlSource`] object to the list. - pub fn add_crl_source(&mut self, c: Box<(dyn CrlSource + Send + Sync)>) { + pub fn add_crl_source(&mut self, c: Box) { self.crl_sources.push(c); } @@ -451,7 +451,7 @@ impl PkiEnvironment { } /// add_revocation_cache adds a [`RevocationStatusCache`] object to the list. - pub fn add_revocation_cache(&mut self, c: Box<(dyn RevocationStatusCache + Send + Sync)>) { + pub fn add_revocation_cache(&mut self, c: Box) { self.revocation_cache.push(c); } @@ -528,7 +528,7 @@ impl PkiEnvironment { } /// add_check_remote adds a [`CheckRemoteResource`] object to the list. - pub fn add_check_remote(&mut self, c: Box<(dyn CheckRemoteResource + Send + Sync)>) { + pub fn add_check_remote(&mut self, c: Box) { self.check_remote.push(c); } From 049558a35a9583cb938fac2a96aa1b28fb881361 Mon Sep 17 00:00:00 2001 From: Peter Goodspeed-Niklaus Date: Tue, 28 Apr 2026 15:44:31 +0200 Subject: [PATCH 2/4] chore: `cargo clippy --fix -p certval` --- certval/src/builder/uri_utils.rs | 2 +- certval/src/revocation/crl.rs | 2 +- certval/src/validator/path_validator.rs | 22 +++++----------------- certval/src/validator/pdv_trust_anchor.rs | 2 +- certval/src/validator/policy_graph.rs | 6 +++--- certval/src/validator/policy_tree.rs | 6 +++--- 6 files changed, 14 insertions(+), 26 deletions(-) diff --git a/certval/src/builder/uri_utils.rs b/certval/src/builder/uri_utils.rs index e342bb52..1973e037 100644 --- a/certval/src/builder/uri_utils.rs +++ b/certval/src/builder/uri_utils.rs @@ -228,7 +228,7 @@ pub async fn fetch_to_buffer( let fname_from_response = response .url() .path_segments() - .and_then(|segments| segments.last()) + .and_then(|mut segments| segments.next_back()) .and_then(|name| if name.is_empty() { None } else { Some(name) }) .unwrap_or("tmp.bin"); diff --git a/certval/src/revocation/crl.rs b/certval/src/revocation/crl.rs index e69df87a..e98424d0 100644 --- a/certval/src/revocation/crl.rs +++ b/certval/src/revocation/crl.rs @@ -546,7 +546,7 @@ pub(crate) fn get_crl_info(crl: &CertificateList) -> Result { } if idp_name.is_none() { // not supporting non-DN/URI DPs - return Err(Error::Unrecognized.into()); + return Err(Error::Unrecognized); } } Some(DistributionPointName::NameRelativeToCRLIssuer(_unsupported)) => { diff --git a/certval/src/validator/path_validator.rs b/certval/src/validator/path_validator.rs index 830782bf..a2dbfed8 100644 --- a/certval/src/validator/path_validator.rs +++ b/certval/src/validator/path_validator.rs @@ -273,14 +273,8 @@ pub fn check_names( // Read input variables from path settings let mut pbufs = BTreeMap::new(); let mut ebufs = BTreeMap::new(); - let initial_perm = match get_initial_permitted_subtrees_as_set(cps, &mut pbufs) { - Ok(ip) => ip, - Err(e) => return Err(e), - }; - let initial_excl = match get_initial_excluded_subtrees_as_set(cps, &mut ebufs) { - Ok(ie) => ie, - Err(e) => return Err(e), - }; + let initial_perm = get_initial_permitted_subtrees_as_set(cps, &mut pbufs)?; + let initial_excl = get_initial_excluded_subtrees_as_set(cps, &mut ebufs)?; // for convenience, combine target into array with the intermediate CA certs let mut v = cp.intermediates.clone(); @@ -650,12 +644,9 @@ pub fn enforce_trust_anchor_constraints( if let PDVExtension::NameConstraints(nc) = nc { if let Some(permitted) = &nc.permitted_subtrees { let mut initial_perm = - match get_initial_permitted_subtrees_with_default_as_set( + get_initial_permitted_subtrees_with_default_as_set( cps, &mut pbufs, - ) { - Ok(ip) => ip, - Err(e) => return Err(e), - }; + )?; initial_perm.calculate_union(permitted); set_initial_permitted_subtrees_from_set(&mut mod_cps, &initial_perm); } @@ -667,10 +658,7 @@ pub fn enforce_trust_anchor_constraints( if let Some(PDVExtension::NameConstraints(nc)) = name_constraints { if let Some(excluded) = &nc.excluded_subtrees { let mut initial_excl = - match get_initial_excluded_subtrees_with_default_as_set(cps, &mut ebufs) { - Ok(ie) => ie, - Err(e) => return Err(e), - }; + get_initial_excluded_subtrees_with_default_as_set(cps, &mut ebufs)?; initial_excl.calculate_union(excluded); set_initial_excluded_subtrees_from_set(&mut mod_cps, &initial_excl); } diff --git a/certval/src/validator/pdv_trust_anchor.rs b/certval/src/validator/pdv_trust_anchor.rs index df327bfe..d32ddfbe 100644 --- a/certval/src/validator/pdv_trust_anchor.rs +++ b/certval/src/validator/pdv_trust_anchor.rs @@ -129,7 +129,7 @@ impl TryFrom<&TrustAnchor<'_>> for PDVTrustAnchorChoice { let key_id = match spki.subject_public_key.as_bytes() { Some(b) => Sha1::digest(b), None => { - error!("Failed to calculate key identifier for {}", n.to_string()); + error!("Failed to calculate key identifier for {}", n); return Err(Error::Unrecognized); } }; diff --git a/certval/src/validator/policy_graph.rs b/certval/src/validator/policy_graph.rs index bd2d9c59..2d5d2ace 100644 --- a/certval/src/validator/policy_graph.rs +++ b/certval/src/validator/policy_graph.rs @@ -61,17 +61,17 @@ pub fn check_certificate_policies_graph( // Initialize state variables (RFC 6.1.2 a, d, e, and f) let mut valid_policy_graph = Vec::::new(); - let mut explicit_policy: u32 = if let true = initial_explicit_policy_indicator { + let mut explicit_policy: u32 = if initial_explicit_policy_indicator { 0 } else { certs_in_cert_path + 1 }; - let mut inhibit_any_policy: u32 = if let true = initial_inhibit_any_policy_indicator { + let mut inhibit_any_policy: u32 = if initial_inhibit_any_policy_indicator { 0 } else { certs_in_cert_path + 1 }; - let mut policy_mapping: u32 = if let true = initial_policy_mapping_inhibit_indicator { + let mut policy_mapping: u32 = if initial_policy_mapping_inhibit_indicator { 0 } else { certs_in_cert_path + 1 diff --git a/certval/src/validator/policy_tree.rs b/certval/src/validator/policy_tree.rs index 65098f78..326280ca 100644 --- a/certval/src/validator/policy_tree.rs +++ b/certval/src/validator/policy_tree.rs @@ -61,17 +61,17 @@ pub fn check_certificate_policies( // Initialize state variables (RFC 6.1.2 a, d, e, and f) let mut valid_policy_tree = Vec::::new(); - let mut explicit_policy: u32 = if let true = initial_explicit_policy_indicator { + let mut explicit_policy: u32 = if initial_explicit_policy_indicator { 0 } else { certs_in_cert_path + 1 }; - let mut inhibit_any_policy: u32 = if let true = initial_inhibit_any_policy_indicator { + let mut inhibit_any_policy: u32 = if initial_inhibit_any_policy_indicator { 0 } else { certs_in_cert_path + 1 }; - let mut policy_mapping: u32 = if let true = initial_policy_mapping_inhibit_indicator { + let mut policy_mapping: u32 = if initial_policy_mapping_inhibit_indicator { 0 } else { certs_in_cert_path + 1 From 42338c734b52ebd4fdf9a1746d529c190f5a1779 Mon Sep 17 00:00:00 2001 From: Peter Goodspeed-Niklaus Date: Tue, 28 Apr 2026 15:52:30 +0200 Subject: [PATCH 3/4] refactor: simplify implementation of `find_all_partial_paths_internal` The existing implementation was mainly written 4 years ago, and used idioms even older than that. Rust has come a long way since then! This is a purely mechanical refactoring accomplished by repeatedly scanning for old patterns and replacing them with something more modern. But all substitutions preserved 100% semantic equivalence. No serious attempt has been made to actually understand the code. On the other hand, the code should be much easier to understand now. --- certval/src/source/cert_source.rs | 217 ++++++++++++++---------------- 1 file changed, 102 insertions(+), 115 deletions(-) diff --git a/certval/src/source/cert_source.rs b/certval/src/source/cert_source.rs index 5bf37c93..d4c0685d 100644 --- a/certval/src/source/cert_source.rs +++ b/certval/src/source/cert_source.rs @@ -1241,133 +1241,120 @@ impl CertSource { pass: u8, partial_paths: &mut Vec>>>, ) { + macro_rules! or_continue { + ($pattern:pat = $e:expr) => { + let $pattern = $e else { + continue; + }; + }; + } + // Instantiate a map that will aggregate paths built relative to the 0th or pass-1 row in // self.buffers_and_paths.partial_paths, if any. let mut new_additions: BTreeMap>> = BTreeMap::new(); // iterate over all certs in the self.certs vector. - for (cur_cert_index, cur_cert) in self.certs.iter().enumerate() { + for (cur_cert_index, cur_cert) in self + .certs + .iter() + .enumerate() // skip over elements that don't have a cert (these correspond to buffers in // self.buffers_and_paths.buffers that could not be parsed when self.certs was prepared. - if let Some(cur_cert) = cur_cert { - let cur_cert_hex_skid = hex_skid_from_cert(cur_cert); - if 0 == pass { - let ta = pe.get_trust_anchor_for_target(cur_cert); - if let Ok(ta) = ta { - // RFC 5914 TAs do not necessary have to have a name, if this is one of those, ignore it - let ta_name = get_trust_anchor_name(&ta.decoded_ta); - if let Ok(ta_name) = ta_name { - if compare_names(&cur_cert.decoded_cert.tbs_certificate.issuer, ta_name) - { - let defer_cert = - DeferDecodeSigned::from_der(cur_cert.encoded_cert.as_slice()); - if let Ok(defer_cert) = defer_cert { - let spki = get_subject_public_key_info_from_trust_anchor( - &ta.decoded_ta, - ); - let r = pe.verify_signature_message( - pe, - &defer_cert.tbs_field, - cur_cert.decoded_cert.signature.raw_bytes(), - &cur_cert.decoded_cert.tbs_certificate.signature, - spki, - ); - if let Ok(_r) = r { - let new_path = vec![cur_cert_index]; - if new_additions.contains_key(&cur_cert_hex_skid) { - if !new_additions[&cur_cert_hex_skid] - .contains(&new_path) - { - let mut v = - new_additions[&cur_cert_hex_skid].clone(); - v.push(new_path); - new_additions.insert(cur_cert_hex_skid.clone(), v); - } - } else { - new_additions - .insert(cur_cert_hex_skid.clone(), vec![new_path]); - } - } - } - } - } + .filter_map(|(idx, cur_cert)| cur_cert.as_ref().map(|cur_cert| (idx, cur_cert))) + { + let cur_cert_hex_skid = hex_skid_from_cert(cur_cert); + if pass == 0 { + or_continue!(Ok(ta) = pe.get_trust_anchor_for_target(cur_cert)); + // RFC 5914 TAs do not necessary have to have a name, if this is one of those, ignore it + or_continue!(Ok(ta_name) = get_trust_anchor_name(&ta.decoded_ta)); + + if !compare_names(&cur_cert.decoded_cert.tbs_certificate.issuer, ta_name) { + continue; + } + + or_continue!( + Ok(defer_cert) = DeferDecodeSigned::from_der(cur_cert.encoded_cert.as_slice()) + ); + + let spki = get_subject_public_key_info_from_trust_anchor(&ta.decoded_ta); + or_continue!( + Ok(_) = pe.verify_signature_message( + pe, + &defer_cert.tbs_field, + cur_cert.decoded_cert.signature.raw_bytes(), + &cur_cert.decoded_cert.tbs_certificate.signature, + spki, + ) + ); + + let new_path = vec![cur_cert_index]; + let paths = new_additions.entry(cur_cert_hex_skid.clone()).or_default(); + if !paths.contains(&new_path) { + paths.push(new_path); + } + } else { + or_continue!( + Ok(defer_cert) = DeferDecodeSigned::from_der(cur_cert.encoded_cert.as_slice()) + ); + + // look for matches in map from previous row of partial_paths + let last_row = &partial_paths[(pass - 1) as usize]; + + // get list of SKIDs for possible issuers (based on AKID and name lookups) + let prospective_issuers = self.find_prospective_issuers(&cur_cert); + for prospective_path in prospective_issuers + .into_iter() + .filter_map(|prospective_issuer| last_row.get(&prospective_issuer)) + .flatten() + { + //log_message(&PeLogLevels::PeDebug, format!("LAST ROW FOR PASS #{} {:?}", pass, last_row.keys()).as_str()); + + or_continue!( + Some(prospective_ca_cert) = prospective_path + .last() + .and_then(|&cert_idx| self.certs.get(cert_idx).map(Option::as_ref)) + .flatten() + ); + + // should path settings be used more generally than time of interest? + // Not doing that at present because policy and name constraints + // are more variable than use of current time as time of interest + if self.get_operative_path_len_constraint(prospective_path) == 0 + || !(compare_names( + &cur_cert.decoded_cert.tbs_certificate.issuer, + &prospective_ca_cert.decoded_cert.tbs_certificate.subject, + ) && self.check_names_in_partial_path(prospective_path) + && self.check_validity_in_partial_path(prospective_path, cps)) + { + continue; } - } else { - let defer_cert = DeferDecodeSigned::from_der(cur_cert.encoded_cert.as_slice()); - if let Ok(defer_cert) = defer_cert { - // look for matches in map from previous row of partial_paths - let last_row = &partial_paths[(pass - 1) as usize]; - - // get list of SKIDs for possible issuers (based on AKID and name lookups) - let prospective_issuers = self.find_prospective_issuers(cur_cert); - for k in prospective_issuers { - //log_message(&PeLogLevels::PeDebug, format!("LAST ROW FOR PASS #{} {:?}", pass, last_row.keys()).as_str()); - if !last_row.contains_key(&k) { - continue; - } - let prospective_paths = &last_row[&k]; - for prospective_path in prospective_paths { - let prospective_ca_cert = - &self.certs[prospective_path[prospective_path.len() - 1]]; - if let Some(prospective_ca_cert) = prospective_ca_cert { - if 0 == self.get_operative_path_len_constraint(prospective_path) - { - continue; - } - // should path settings be used more generally than time of interest? - // Not doing that at present because policy and name constraints - // are more variable than use of current time as time of interest - if compare_names( - &cur_cert.decoded_cert.tbs_certificate.issuer, - &prospective_ca_cert.decoded_cert.tbs_certificate.subject, - ) && self.check_names_in_partial_path(prospective_path) - && self - .check_validity_in_partial_path(prospective_path, cps) - { - let r = pe.verify_signature_message( - pe, - &defer_cert.tbs_field, - cur_cert.decoded_cert.signature.raw_bytes(), - &cur_cert.decoded_cert.tbs_certificate.signature, - &prospective_ca_cert - .decoded_cert - .tbs_certificate - .subject_public_key_info, - ); - if let Ok(_r) = r { - if !self.pub_key_in_path(cur_cert, prospective_path) { - let mut new_path = prospective_path.clone(); - new_path.push(cur_cert_index); - if new_additions - .contains_key(&cur_cert_hex_skid.clone()) - { - if !new_additions[&cur_cert_hex_skid] - .contains(&new_path) - { - let mut v = new_additions - [&cur_cert_hex_skid] - .clone(); - v.push(new_path); - new_additions - .insert(cur_cert_hex_skid.clone(), v); - } - } else { - new_additions.insert( - cur_cert_hex_skid.clone(), - vec![new_path], - ); - } - } - } - } // end compare_names - } - } + or_continue!( + Ok(_) = pe.verify_signature_message( + pe, + &defer_cert.tbs_field, + cur_cert.decoded_cert.signature.raw_bytes(), + &cur_cert.decoded_cert.tbs_certificate.signature, + &prospective_ca_cert + .decoded_cert + .tbs_certificate + .subject_public_key_info, + ) + ); + + if !self.pub_key_in_path(&cur_cert, prospective_path) { + let mut new_path = prospective_path.clone(); + new_path.push(cur_cert_index); + + let paths = new_additions.entry(cur_cert_hex_skid.clone()).or_default(); + if !paths.contains(&new_path) { + paths.push(new_path); } } - } // end if 0 == pass { - } // end if let Some(cur_cert) = cur_cert { + } + } // end if 0 == pass { } + if !new_additions.is_empty() { // error!("NEW ADDITIONS FOR PASS #{}: {:?}", pass, new_additions); partial_paths.push(new_additions); From f5eab2febcb5ac486d43eb5204aebe1210c096b1 Mon Sep 17 00:00:00 2001 From: Peter Goodspeed-Niklaus Date: Tue, 28 Apr 2026 15:52:30 +0200 Subject: [PATCH 4/4] refactor: simplify implementation of `get_paths_for_target` The existing implementation was mainly written 4 years ago, and used idioms even older than that. Rust has come a long way since then! This is a purely mechanical refactoring accomplished by repeatedly scanning for old patterns and replacing them with something more modern. But all substitutions preserved 100% semantic equivalence. No serious attempt has been made to actually understand the code. On the other hand, the code should be easier to understand now. --- certval/src/source/cert_source.rs | 250 ++++++++++++++---------------- 1 file changed, 120 insertions(+), 130 deletions(-) diff --git a/certval/src/source/cert_source.rs b/certval/src/source/cert_source.rs index d4c0685d..5117f837 100644 --- a/certval/src/source/cert_source.rs +++ b/certval/src/source/cert_source.rs @@ -71,6 +71,14 @@ use ciborium::from_reader; use std::sync::{Arc, RwLock}; +macro_rules! or_continue { + ($pattern:pat = $e:expr) => { + let $pattern = $e else { + continue; + }; + }; +} + /// The CertFile struct associates a string, notionally containing a filename or URI, with a vector /// of bytes. The vector of bytes is assumed to contain a binary DER encoded certificate. #[derive(Clone, Serialize, Deserialize)] @@ -1241,14 +1249,6 @@ impl CertSource { pass: u8, partial_paths: &mut Vec>>>, ) { - macro_rules! or_continue { - ($pattern:pat = $e:expr) => { - let $pattern = $e else { - continue; - }; - }; - } - // Instantiate a map that will aggregate paths built relative to the 0th or pass-1 row in // self.buffers_and_paths.partial_paths, if any. let mut new_additions: BTreeMap>> = BTreeMap::new(); @@ -1377,22 +1377,21 @@ impl CertificateSource for CertSource { threshold: usize, time_of_interest: u64, ) -> Result<()> { - if let Err(e) = valid_at_time(&target.decoded_cert.tbs_certificate, time_of_interest, true) - { - error!( - "No paths found because target is not valid at indicated time of interest ({})", - time_of_interest - ); - return Err(e); - } + valid_at_time(&target.decoded_cert.tbs_certificate, time_of_interest, true).inspect_err( + |_| { + error!( + "No paths found because target is not valid at indicated time of interest ({})", + time_of_interest + ) + }, + )?; - let ta = pe.get_trust_anchor_for_target(target); - if let Ok(ta) = ta { + if let Ok(ta) = pe.get_trust_anchor_for_target(target) { let path = CertificationPath::new(ta.clone(), vec![], target.clone()); paths.push(path); } - let mut akid_hex = "".to_string(); + let mut akid_hex = String::new(); let mut name_vec = vec![&target.decoded_cert.tbs_certificate.issuer]; let akid_ext = target.get_extension(&ID_CE_AUTHORITY_KEY_IDENTIFIER); if let Ok(Some(PDVExtension::AuthorityKeyIdentifier(akid))) = akid_ext { @@ -1412,132 +1411,123 @@ impl CertificateSource for CertSource { let mut ii = 0; while ii < 2 { ii += 1; - if !akid_hex.is_empty() { - let partial_paths = if let Ok(g) = self.buffers_and_paths.partial_paths.read() { - g - } else { - return Err(Error::Unrecognized); - }; + if akid_hex.is_empty() { + let fname = get_filename_from_cert_metadata(target); + error!( + "Missing AKID in target and failed to find by name - {}", + fname + ); + } else { + let partial_paths = self + .buffers_and_paths + .partial_paths + .read() + .map_err(|_| Error::Unrecognized)?; + + for indices in partial_paths + .iter() + .filter_map(|partial| partial.get(&akid_hex)) + .flatten() + { + if !above_threshold(indices, threshold) { + continue; + } - for p in partial_paths.iter() { - if p.contains_key(&akid_hex) { - let indices_vec = &p[&akid_hex]; - for indices in indices_vec { - if !above_threshold(indices, threshold) { - continue; - } + // This block accounts for CAs that use different names for same SKID. Could add name constraints check here too, maybe. + or_continue!(Some(last_index) = indices.last().copied()); + if let Some(ca) = &self.certs[last_index] { + if !compare_names( + &ca.decoded_cert.tbs_certificate.subject, + &target.decoded_cert.tbs_certificate.issuer, + ) { + error!("Encountered CA that is likely using same SKID with different names. Skipping partial path due to name mismatch."); + continue; + } + } - // This block accounts for CAs that use different names for same SKID. Could add name constraints check here too, maybe. - let last_index = if let Some(li) = indices.last() { - li - } else { - continue; - }; - let issuer = &self.certs[*last_index]; - if let Some(ca) = issuer { - if !compare_names( - &ca.decoded_cert.tbs_certificate.subject, - &target.decoded_cert.tbs_certificate.issuer, - ) { - error!("Encountered CA that is likely using same SKID with different names. Skipping partial path due to name mismatch."); - continue; - } - } + let mut ta = None; + let mut found_blank = false; - let mut ta = None; - let mut intermediates = vec![]; - let mut found_blank = false; - for (i, index) in indices.iter().enumerate() { - if let Some(cert) = &self.certs[*index] { - intermediates.push(cert.clone()); - if 0 == i { - let mut ta_akid_hex = "".to_string(); - let mut ta_name_vec = - vec![&target.decoded_cert.tbs_certificate.issuer]; - let ca_akid_ext = - cert.get_extension(&ID_CE_AUTHORITY_KEY_IDENTIFIER); - if let Ok(Some(PDVExtension::AuthorityKeyIdentifier( - ca_akid, - ))) = ca_akid_ext - { - if let Some(ca_kid) = &ca_akid.key_identifier { - ta_akid_hex = buffer_to_hex(ca_kid.as_bytes()); - } else if let Some(names) = - &ca_akid.authority_cert_issuer - { - for n in names { - if let GeneralName::DirectoryName(dn) = n { - ta_name_vec.push(dn); - } - } - } - } - - if !ta_akid_hex.is_empty() { - if let Ok(new_ta) = - pe.get_trust_anchor_by_hex_skid(&ta_akid_hex) - { - ta = Some(new_ta); - } - } else { - let fname = get_filename_from_cert_metadata(cert); - error!("Missing AKID for trust anchor - {}", fname); - if let Ok(new_ta) = pe.get_trust_anchor_for_target(cert) - { - error!("Found trust anchor by name"); - ta = Some(new_ta); - } - } - } - } else { - // some cert slots are empty (due to parse or validity error). skip those. - found_blank = true; - break; - } + let intermediates = indices + .iter() + .copied() + .map_while(|index| { + let maybe_cert = &self.certs[index]; + if maybe_cert.is_none() { + found_blank = true; } - if !found_blank { - if let Some(ta) = ta { - let path = CertificationPath::new( - ta.clone(), - intermediates, - target.clone(), - ); - if !pub_key_repeats(&path) { - ii = 2; - paths.push(path); + maybe_cert.clone() + }) + .collect::>(); + + if let Some(cert) = intermediates.first() { + let mut ta_akid_hex = "".to_string(); + let mut ta_name_vec = vec![&target.decoded_cert.tbs_certificate.issuer]; + + if let Ok(Some(PDVExtension::AuthorityKeyIdentifier(ca_akid))) = + cert.get_extension(&ID_CE_AUTHORITY_KEY_IDENTIFIER) + { + if let Some(ca_kid) = &ca_akid.key_identifier { + ta_akid_hex = buffer_to_hex(ca_kid.as_bytes()); + } else if let Some(names) = &ca_akid.authority_cert_issuer { + for n in names { + if let GeneralName::DirectoryName(dn) = n { + ta_name_vec.push(dn); } } } } + + if !ta_akid_hex.is_empty() { + ta = pe.get_trust_anchor_by_hex_skid(&ta_akid_hex).ok(); + } else { + let fname = get_filename_from_cert_metadata(cert); + error!("Missing AKID for trust anchor - {}", fname); + ta = pe + .get_trust_anchor_for_target(cert) + .ok() + .inspect(|_| error!("Found trust anchor by name")); + } + } + + if !found_blank { + if let Some(ta) = ta { + let path = + CertificationPath::new(ta.clone(), intermediates, target.clone()); + if !pub_key_repeats(&path) { + ii = 2; + paths.push(path); + } + } } } - } else { - let fname = get_filename_from_cert_metadata(target); - error!( - "Missing AKID in target and failed to find by name - {}", - fname - ); } if akid_hex.is_empty() || paths_count == paths.len() { // try to use name map to find AKID let mut changed = false; - for n in &name_vec { - let name_str = name_to_string(n); - if self.name_map.contains_key(&name_str) { - for i in &self.name_map[&name_str] { - if let Some(cert) = &self.certs[*i] { - let skid = hex_skid_from_cert(cert); - if !skid.is_empty() { - debug!( - "Using calculated key identifier in lieu of AKID for {}", - name_str - ); - akid_hex = skid; - changed = true; - break; - } - } + for name_str in name_vec.iter().map(|name| name_to_string(name)) { + for cert in self + .name_map + .get(&name_str) + .into_iter() + .flatten() + .copied() + .filter_map(|cert_idx| { + self.certs.get(cert_idx).map(Option::as_ref).flatten() + }) + { + let skid = hex_skid_from_cert(cert); + if !skid.is_empty() { + debug!( + "Using calculated key identifier in lieu of AKID for {}", + name_str + ); + akid_hex = skid; + changed = true; + // this preserves existing behavior but we might actually prefer to + // break the outer loop instead of the inner + break; } } }