der: refactor Reader::read_nested

Gets rid of a provided method along with the `NestedReader` type, in order to fix #1228. Instead, each reader must now implement its own strategy for nested reading.
RustCrypto · Aug 18, 2024 · e27c484 · e27c484
1 parent 8279b9d
commit e27c484
Show file tree

Hide file tree

Showing 7 changed files with 77 additions and 134 deletions.
diff --git a/der/src/bytes_ref.rs b/der/src/bytes_ref.rs
@@ -49,6 +49,16 @@ impl<'a> BytesRef<'a> {
     pub fn is_empty(self) -> bool {
         self.len() == Length::ZERO
     }
+
+    /// Get a prefix of a [`BytesRef`] of the given length.
+    pub fn prefix(self, length: Length) -> Result<Self> {
+        let inner = self
+            .as_slice()
+            .get(..usize::try_from(length)?)
+            .ok_or_else(|| Error::incomplete(self.length))?;
+
+        Ok(Self { length, inner })
+    }
 }
 
 impl AsRef<[u8]> for BytesRef<'_> {

diff --git a/der/src/lib.rs b/der/src/lib.rs
@@ -367,7 +367,7 @@ pub use crate::{
     header::Header,
     length::{IndefiniteLength, Length},
     ord::{DerOrd, ValueOrd},
-    reader::{nested::NestedReader, slice::SliceReader, Reader},
+    reader::{slice::SliceReader, Reader},
     tag::{Class, FixedTag, Tag, TagMode, TagNumber, Tagged},
     writer::{slice::SliceWriter, Writer},
 };

diff --git a/der/src/reader.rs b/der/src/reader.rs
@@ -1,12 +1,9 @@
 //! Reader trait.
 
-pub(crate) mod nested;
 #[cfg(feature = "pem")]
 pub(crate) mod pem;
 pub(crate) mod slice;
 
-pub(crate) use nested::NestedReader;
-
 use crate::{
     asn1::ContextSpecific, Decode, DecodeValue, Encode, EncodingRules, Error, ErrorKind, FixedTag,
     Header, Length, Tag, TagMode, TagNumber,
@@ -35,6 +32,12 @@ pub trait Reader<'r>: Sized {
     /// Get the position within the buffer.
     fn position(&self) -> Length;
 
+    /// Read nested data of the given length.
+    fn read_nested<T, F, E>(&mut self, len: Length, f: F) -> Result<T, E>
+    where
+        E: From<Error>,
+        F: FnOnce(&mut Self) -> Result<T, E>;
+
     /// Attempt to read data borrowed directly from the input as a slice,
     /// updating the internal cursor position.
     ///
@@ -130,17 +133,6 @@ pub trait Reader<'r>: Sized {
         Ok(buf)
     }
 
-    /// Read nested data of the given length.
-    fn read_nested<'n, T, F, E>(&'n mut self, len: Length, f: F) -> Result<T, E>
-    where
-        F: FnOnce(&mut NestedReader<'n, Self>) -> Result<T, E>,
-        E: From<Error>,
-    {
-        let mut reader = NestedReader::new(self, len)?;
-        let ret = f(&mut reader)?;
-        Ok(reader.finish(ret)?)
-    }
-
     /// Read a byte vector of the given length.
     #[cfg(feature = "alloc")]
     fn read_vec(&mut self, len: Length) -> Result<Vec<u8>, Error> {
@@ -157,9 +149,9 @@ pub trait Reader<'r>: Sized {
 
     /// Read an ASN.1 `SEQUENCE`, creating a nested [`Reader`] for the body and
     /// calling the provided closure with it.
-    fn sequence<'n, F, T, E>(&'n mut self, f: F) -> Result<T, E>
+    fn sequence<F, T, E>(&mut self, f: F) -> Result<T, E>
     where
-        F: FnOnce(&mut NestedReader<'n, Self>) -> Result<T, E>,
+        F: FnOnce(&mut Self) -> Result<T, E>,
         E: From<Error>,
     {
         let header = Header::decode(self)?;

diff --git a/der/src/reader/nested.rs b/der/src/reader/nested.rs
diff --git a/der/src/reader/pem.rs b/der/src/reader/pem.rs
@@ -1,7 +1,7 @@
 //! Streaming PEM reader.
 
 use super::Reader;
-use crate::{Decode, EncodingRules, ErrorKind, Header, Length, Result};
+use crate::{Decode, EncodingRules, Error, ErrorKind, Header, Length};
 use pem_rfc7468::Decoder;
 
 /// `Reader` type which decodes PEM on-the-fly.
@@ -28,7 +28,7 @@ impl<'i> PemReader<'i> {
     /// Create a new PEM reader which decodes data on-the-fly.
     ///
     /// Uses the default 64-character line wrapping.
-    pub fn new(pem: &'i [u8]) -> Result<Self> {
+    pub fn new(pem: &'i [u8]) -> crate::Result<Self> {
         let decoder = Decoder::new(pem)?;
         let input_len = Length::try_from(decoder.remaining_len())?;
 
@@ -50,8 +50,8 @@ impl<'i> PemReader<'i> {
     /// output buffer.
     ///
     /// Attempts to fill the entire buffer, returning an error if there is not enough data.
-    pub fn peek_into(&self, buf: &mut [u8]) -> Result<()> {
-        self.decoder.clone().decode(buf)?;
+    pub fn peek_into(&self, buf: &mut [u8]) -> crate::Result<()> {
+        self.clone().read_into(buf)?;
         Ok(())
     }
 }
@@ -72,28 +72,48 @@ impl<'i> Reader<'i> for PemReader<'i> {
         self.peek_into(&mut byte).ok().map(|_| byte[0])
     }
 
-    fn peek_header(&self) -> Result<Header> {
+    fn peek_header(&self) -> crate::Result<Header> {
         Header::decode(&mut self.clone())
     }
 
     fn position(&self) -> Length {
         self.position
     }
 
-    fn read_slice(&mut self, _len: Length) -> Result<&'i [u8]> {
+    fn read_nested<T, F, E>(&mut self, len: Length, f: F) -> Result<T, E>
+    where
+        F: FnOnce(&mut Self) -> Result<T, E>,
+        E: From<Error>,
+    {
+        let nested_input_len = (self.position + len)?;
+        if nested_input_len > self.input_len {
+            return Err(Error::incomplete(self.input_len).into());
+        }
+
+        let orig_input_len = self.input_len;
+        self.input_len = nested_input_len;
+        let ret = f(self);
+        self.input_len = orig_input_len;
+        ret
+    }
+
+    fn read_slice(&mut self, _len: Length) -> crate::Result<&'i [u8]> {
         // Can't borrow from PEM because it requires decoding
         Err(ErrorKind::Reader.into())
     }
 
-    fn read_into<'o>(&mut self, buf: &'o mut [u8]) -> Result<&'o [u8]> {
-        let bytes = self.decoder.decode(buf)?;
-        self.position = (self.position + bytes.len())?;
-
-        debug_assert_eq!(
-            self.position,
-            (self.input_len - Length::try_from(self.decoder.remaining_len())?)?
-        );
-
-        Ok(bytes)
+    fn read_into<'o>(&mut self, buf: &'o mut [u8]) -> crate::Result<&'o [u8]> {
+        let new_position = (self.position + buf.len())?;
+        if new_position > self.input_len {
+            return Err(ErrorKind::Incomplete {
+                expected_len: new_position,
+                actual_len: self.input_len,
+            }
+            .at(self.position));
+        }
+
+        self.decoder.decode(buf)?;
+        self.position = new_position;
+        Ok(buf)
     }
 }
diff --git a/der/src/reader/slice.rs b/der/src/reader/slice.rs
@@ -91,6 +91,28 @@ impl<'a> Reader<'a> for SliceReader<'a> {
         self.position
     }
 
+    /// Read nested data of the given length.
+    fn read_nested<T, F, E>(&mut self, len: Length, f: F) -> Result<T, E>
+    where
+        F: FnOnce(&mut Self) -> Result<T, E>,
+        E: From<Error>,
+    {
+        let prefix_len = (self.position + len)?;
+        let mut nested_reader = self.clone();
+        nested_reader.bytes = self.bytes.prefix(prefix_len)?;
+
+        let ret = f(&mut nested_reader);
+        self.position = nested_reader.position;
+        self.failed = nested_reader.failed;
+
+        ret.and_then(|value| {
+            nested_reader.finish(value).map_err(|e| {
+                self.failed = true;
+                e.into()
+            })
+        })
+    }
+
     fn read_slice(&mut self, len: Length) -> Result<&'a [u8], Error> {
         if self.is_failed() {
             return Err(self.error(ErrorKind::Failed));

diff --git a/pkcs12/src/safe_bag.rs b/pkcs12/src/safe_bag.rs
@@ -43,7 +43,6 @@ impl<'a> ::der::DecodeValue<'a> for SafeBag {
         reader: &mut R,
         header: ::der::Header,
     ) -> ::der::Result<Self> {
-        use ::der::Reader as _;
         reader.read_nested(header.length, |reader| {
             let bag_id = reader.decode()?;
             let bag_value = match reader.tlv_bytes() {