From c21d3b074ccbd754f197333375cef3cf90c46a4a Mon Sep 17 00:00:00 2001
From: Brian Smith <brian@briansmith.org>
Date: Thu, 18 Feb 2021 10:03:03 -0800
Subject: [PATCH] Implement iteration through certificate chain as an iterator.

Factor out the boilerplate of navigating through the linked list.
---
 src/cert.rs        | 25 +++++++++++++++++++++++
 src/name/verify.rs | 36 ++++++++++++++++------------------
 src/verify_cert.rs | 49 +++++++++++++---------------------------------
 3 files changed, 56 insertions(+), 54 deletions(-)

diff --git a/src/cert.rs b/src/cert.rs
index c9516aba..0ea46a6c 100644
--- a/src/cert.rs
+++ b/src/cert.rs
@@ -34,6 +34,31 @@ pub struct Cert<'a> {
     pub subject_alt_name: Option<untrusted::Input<'a>>,
 }
 
+impl<'a> Cert<'a> {
+    /// Returns an iterator of all the certs in the chain so far.
+    ///
+    /// The first item will be `self` and the last item will be the end-entity
+    /// certificate.
+    pub fn iter_from_self_through_end_entity(&'a self) -> impl Iterator<Item = &'a Cert<'a>> + 'a {
+        struct Iter<'c> {
+            next: Option<&'c Cert<'c>>,
+        }
+        impl<'c> Iterator for Iter<'c> {
+            type Item = &'c Cert<'c>;
+
+            fn next(&mut self) -> Option<Self::Item> {
+                let next_next = match self.next?.ee_or_ca {
+                    EndEntityOrCA::EndEntity => None,
+                    EndEntityOrCA::CA(c) => Some(c),
+                };
+                core::mem::replace(&mut self.next, next_next)
+            }
+        }
+
+        Iter { next: Some(self) }
+    }
+}
+
 pub fn parse_cert<'a>(
     cert_der: untrusted::Input<'a>,
     ee_or_ca: EndEntityOrCA<'a>,
diff --git a/src/name/verify.rs b/src/name/verify.rs
index 78dee196..4f25328c 100644
--- a/src/name/verify.rs
+++ b/src/name/verify.rs
@@ -16,10 +16,7 @@ use super::{
     dns_name::{self, DnsNameRef},
     ip_address,
 };
-use crate::{
-    cert::{Cert, EndEntityOrCA},
-    der, Error,
-};
+use crate::{cert::Cert, der, Error};
 
 pub fn verify_cert_dns_name(
     cert: &crate::EndEntityCert,
@@ -79,21 +76,22 @@ pub fn check_name_constraints(
     let permitted_subtrees = parse_subtrees(input, der::Tag::ContextSpecificConstructed0)?;
     let excluded_subtrees = parse_subtrees(input, der::Tag::ContextSpecificConstructed1)?;
 
-    let mut child = subordinate_certs;
-    loop {
-        iterate_names(child.subject, child.subject_alt_name, Ok(()), &|name| {
-            check_presented_id_conforms_to_constraints(name, permitted_subtrees, excluded_subtrees)
-        })?;
-
-        child = match child.ee_or_ca {
-            EndEntityOrCA::CA(child_cert) => child_cert,
-            EndEntityOrCA::EndEntity => {
-                break;
-            }
-        };
-    }
-
-    Ok(())
+    subordinate_certs
+        .iter_from_self_through_end_entity()
+        .try_for_each(|subordinate| {
+            iterate_names(
+                subordinate.subject,
+                subordinate.subject_alt_name,
+                Ok(()),
+                &|name| {
+                    check_presented_id_conforms_to_constraints(
+                        name,
+                        permitted_subtrees,
+                        excluded_subtrees,
+                    )
+                },
+            )
+        })
 }
 
 fn check_presented_id_conforms_to_constraints(
diff --git a/src/verify_cert.rs b/src/verify_cert.rs
index 70cdaa60..2651c8d1 100644
--- a/src/verify_cert.rs
+++ b/src/verify_cert.rs
@@ -89,22 +89,13 @@ pub fn build_chain(
             return Err(Error::UnknownIssuer);
         }
 
-        // Prevent loops; see RFC 4158 section 5.2.
-        let mut prev = cert;
-        loop {
-            if potential_issuer.spki.value() == prev.spki.value()
-                && potential_issuer.subject == prev.subject
-            {
-                return Err(Error::UnknownIssuer);
-            }
-            match &prev.ee_or_ca {
-                EndEntityOrCA::EndEntity => {
-                    break;
-                }
-                EndEntityOrCA::CA(child_cert) => {
-                    prev = child_cert;
-                }
-            }
+        // See RFC 4158 section 5.2.
+        let loop_detected = cert.iter_from_self_through_end_entity().any(|cert| {
+            potential_issuer.spki.value() == cert.spki.value()
+                && potential_issuer.subject == cert.subject
+        });
+        if loop_detected {
+            return Err(Error::UnknownIssuer);
         }
 
         untrusted::read_all_optional(potential_issuer.name_constraints, Error::BadDER, |value| {
@@ -133,25 +124,13 @@ fn check_signatures(
     cert_chain: &Cert,
     trust_anchor_key: untrusted::Input,
 ) -> Result<(), Error> {
-    let mut spki_value = trust_anchor_key;
-    let mut cert = cert_chain;
-    loop {
-        signed_data::verify_signed_data(supported_sig_algs, spki_value, &cert.signed_data)?;
-
-        // TODO: check revocation
-
-        match &cert.ee_or_ca {
-            EndEntityOrCA::CA(child_cert) => {
-                spki_value = cert.spki.value();
-                cert = child_cert;
-            }
-            EndEntityOrCA::EndEntity => {
-                break;
-            }
-        }
-    }
-
-    Ok(())
+    cert_chain
+        .iter_from_self_through_end_entity()
+        .try_fold(trust_anchor_key, |spki_value, cert| {
+            signed_data::verify_signed_data(supported_sig_algs, spki_value, &cert.signed_data)?;
+            Ok(cert.spki.value())
+        })
+        .map(|_| ())
 }
 
 fn check_issuer_independent_properties(