From 9ea4ab286841d2b729974b15c695ce9a59cf3366 Mon Sep 17 00:00:00 2001 From: Tony Arcieri Date: Wed, 2 Mar 2022 14:09:26 -0700 Subject: [PATCH] const-oid: use `Arcs` to validate OID well-formedness 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 | 52 ++++++++++++++++++++++++++----------------- const-oid/src/lib.rs | 45 ++++++++----------------------------- 2 files changed, 41 insertions(+), 56 deletions(-) diff --git a/const-oid/src/arcs.rs b/const-oid/src/arcs.rs index 75dae7a90..b3f9f2443 100644 --- a/const-oid/src/arcs.rs +++ b/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::(); +const ARC_MAX_BYTES: usize = mem::size_of::(); /// 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 { + /// 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> { 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 { + // 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 diff --git a/const-oid/src/lib.rs b/const-oid/src/lib.rs index 9a11e4625..200f5833b 100644 --- a/const-oid/src/lib.rs +++ b/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.