const-oid: use Arcs to validate OID well-formedness (#459)

Consolidates the logic in `ObjectIdentifier::from_bytes` and the `Arcs`
type, allowing `Arcs` to be used to make a first pass to ensure that an
OID is well-formed.

const-oid/src/arcs.rs

@@ -22,10 +22,10 @@ pub(crate) const ARC_MAX_FIRST: Arc = 2;
 pub(crate) const ARC_MAX_SECOND: Arc = 39;
 
 /// Maximum number of bytes supported in an arc.
-pub(crate) const ARC_MAX_BYTES: usize = mem::size_of::<Arc>();
+const ARC_MAX_BYTES: usize = mem::size_of::<Arc>();
 
 /// Maximum value of the last byte in an arc.
-pub(crate) const ARC_MAX_LAST_OCTET: u8 = 0b11110000; // Max bytes of leading 1-bits
+const ARC_MAX_LAST_OCTET: u8 = 0b11110000; // Max bytes of leading 1-bits
 
 /// [`Iterator`] over [`Arc`] values (a.k.a. nodes) in an [`ObjectIdentifier`].
 ///
@@ -43,47 +43,50 @@ impl<'a> Arcs<'a> {
     pub(crate) fn new(oid: &'a ObjectIdentifier) -> Self {
         Self { oid, cursor: None }
     }
-}
-
-impl<'a> Iterator for Arcs<'a> {
-    type Item = Arc;
 
-    fn next(&mut self) -> Option<Arc> {
+    /// Try to parse the next arc in this OID.
+    ///
+    /// This method is fallible so it can be used as a first pass to determine
+    /// that the arcs in the OID are well-formed.
+    pub(crate) fn try_next(&mut self) -> Result<Option<Arc>> {
         match self.cursor {
             // Indicates we're on the root OID
             None => {
-                let root = RootArcs(self.oid.as_bytes()[0]);
+                let root = RootArcs::try_from(self.oid.as_bytes()[0])?;
                 self.cursor = Some(0);
-                Some(root.first_arc())
+                Ok(Some(root.first_arc()))
             }
             Some(0) => {
-                let root = RootArcs(self.oid.as_bytes()[0]);
+                let root = RootArcs::try_from(self.oid.as_bytes()[0])?;
                 self.cursor = Some(1);
-                Some(root.second_arc())
+                Ok(Some(root.second_arc()))
             }
             Some(offset) => {
                 let mut result = 0;
                 let mut arc_bytes = 0;
 
-                // TODO(tarcieri): consolidate this with `ObjectIdentifier::from_bytes`?
                 loop {
                     match self.oid.as_bytes().get(offset + arc_bytes).cloned() {
                         Some(byte) => {
                             arc_bytes += 1;
-                            debug_assert!(
-                                arc_bytes <= ARC_MAX_BYTES || byte & ARC_MAX_LAST_OCTET == 0,
-                                "OID arc overflowed"
-                            );
+
+                            if (arc_bytes > ARC_MAX_BYTES) && (byte & ARC_MAX_LAST_OCTET != 0) {
+                                return Err(Error::ArcTooBig);
+                            }
+
                             result = result << 7 | (byte & 0b1111111) as Arc;
 
                             if byte & 0b10000000 == 0 {
                                 self.cursor = Some(offset + arc_bytes);
-                                return Some(result);
+                                return Ok(Some(result));
                             }
                         }
                         None => {
-                            debug_assert_eq!(arc_bytes, 0, "truncated OID");
-                            return None;
+                            if arc_bytes == 0 {
+                                return Ok(None);
+                            } else {
+                                return Err(Error::Base128);
+                            }
                         }
                     }
                 }
@@ -92,12 +95,21 @@ impl<'a> Iterator for Arcs<'a> {
     }
 }
 
+impl<'a> Iterator for Arcs<'a> {
+    type Item = Arc;
+
+    fn next(&mut self) -> Option<Arc> {
+        // ObjectIdentifier constructors should ensure the OID is well-formed
+        self.try_next().expect("OID malformed")
+    }
+}
+
 /// Byte containing the first and second arcs of an OID.
 ///
 /// This is represented this way in order to reduce the overall size of the
 /// [`ObjectIdentifier`] struct.
 #[derive(Copy, Clone, Debug, Eq, PartialEq)]
-pub(crate) struct RootArcs(u8);
+struct RootArcs(u8);
 
 impl RootArcs {
     /// Create [`RootArcs`] from the first and second arc values represented

const-oid/src/lib.rs

@@ -26,10 +26,7 @@ pub use crate::{
     error::{Error, Result},
 };
 
-use crate::{
-    arcs::{RootArcs, ARC_MAX_BYTES, ARC_MAX_LAST_OCTET},
-    encoder::Encoder,
-};
+use crate::encoder::Encoder;
 use core::{fmt, str::FromStr};
 
 /// A named OID.
@@ -132,43 +129,19 @@ impl ObjectIdentifier {
             return Err(Error::Length);
         }
 
-        // Validate root arcs are in range
-        ber_bytes
-            .get(0)
-            .cloned()
-            .ok_or(Error::Length)
-            .and_then(RootArcs::try_from)?;
-
-        // Validate lower arcs are well-formed
-        let mut arc_offset = 1;
-        let mut arc_bytes = 0;
-
-        // TODO(tarcieri): consolidate this with `Arcs::next`?
-        while arc_offset < len {
-            match ber_bytes.get(arc_offset + arc_bytes).cloned() {
-                Some(byte) => {
-                    if (arc_bytes == ARC_MAX_BYTES) && (byte & ARC_MAX_LAST_OCTET != 0) {
-                        return Err(Error::ArcTooBig);
-                    }
-
-                    arc_bytes += 1;
-
-                    if byte & 0b10000000 == 0 {
-                        arc_offset += arc_bytes;
-                        arc_bytes = 0;
-                    }
-                }
-                None => return Err(Error::Base128),
-            }
-        }
-
         let mut bytes = [0u8; Self::MAX_SIZE];
         bytes[..len].copy_from_slice(ber_bytes);
 
-        Ok(Self {
+        let oid = Self {
             bytes,
             length: len as u8,
-        })
+        };
+
+        // Ensure arcs are well-formed
+        let mut arcs = oid.arcs();
+        while arcs.try_next()?.is_some() {}
+
+        Ok(oid)
     }
 
     /// Get the BER/DER serialization of this OID as bytes.