der: (re-)add Reader::read_value with EOC support

One of the blockers for addressing indefinite length handling (#779) has
been where to consume the EOC tag.

This commit (re)introduces `Reader::read_value` (added in #1877, removed
in #1887) and handles decoding the EOC there. This gives us a single
place where EOC can be handled for all constructed messages.

With these changes, the decoder is able to parse `cms_ber.bin` from
`cms/tests`, from which the `cms_der.bin` file has been translated. This
file provides a real-world example of nested indefinite lengths from
CMS.

Note, however, that the example contains a constructed `Any` which isn't
yet being correctly handled. A `TODO` for ensuring the BER and DER
decode identically has been added.

Author: tarcieri

cms/tests/tests_from_pkcs7_crate.rs

@@ -141,3 +141,14 @@ fn cms_decode_signed_der() {
     // should match the original
     assert_eq!(reencoded_der_signed_data_in_ci, der_signed_data_in_ci)
 }
+
+#[test]
+fn cms_decode_signed_ber() {
+    let cms_ber = include_bytes!("../tests/examples/cms_ber.bin");
+    let _ci_ber = ContentInfo::from_ber(cms_ber).unwrap();
+
+    // TODO(tarcieri): ensure BER and DER decode identically
+    // let cms_der = include_bytes!("../tests/examples/cms_der.bin");
+    // let ci_der = ContentInfo::from_der(cms_der).unwrap();
+    // assert_eq!(ci_ber, ci_der);
+}

der/src/length.rs

@@ -40,6 +40,9 @@ impl Length {
     /// Maximum length (`u32::MAX`).
     pub const MAX: Self = Self::new(u32::MAX);
 
+    /// Length of end-of-content octets (i.e. `00 00`).
+    pub(crate) const EOC_LEN: Self = Self::new(2);
+
     /// Maximum number of octets in a DER encoding of a [`Length`] using the
     /// rules implemented by this crate.
     pub(crate) const MAX_SIZE: usize = 5;
@@ -92,6 +95,25 @@ impl Length {
         Self::new(self.inner.saturating_sub(rhs.inner))
     }
 
+    /// If the length is indefinite, compute a length with the EOC marker removed
+    /// (i.e. the final two bytes `00 00`).
+    ///
+    /// Otherwise (as should always be the case with DER), the length is unchanged.
+    ///
+    /// This method notably preserves the `indefinite` flag when performing arithmetic.
+    pub(crate) fn sans_eoc(self) -> Self {
+        if self.indefinite {
+            // Indefinite lengths should always have EOC markers
+            debug_assert!(self.inner >= 2);
+            Self {
+                inner: self.inner.saturating_sub(2),
+                indefinite: true,
+            }
+        } else {
+            self
+        }
+    }
+
     /// Get initial octet of the encoded length (if one is required).
     ///
     /// From X.690 Section 8.1.3.5:
@@ -379,22 +401,12 @@ fn decode_indefinite_length<'a, R: Reader<'a>>(reader: &mut R) -> Result<Length>
     let start_pos = reader.position();
 
     loop {
-        let current_pos = reader.position();
-
         // Look for the end-of-contents marker
         if reader.peek_byte() == Some(EOC_TAG) {
-            // Drain the end-of-contents tag
-            reader.drain(Length::ONE)?;
-
-            // Read the length byte and ensure it's zero (i.e. the full EOC is `00 00`)
-            let length_byte = reader.read_byte()?;
-
-            if length_byte != 0 {
-                return Err(reader.error(ErrorKind::IndefiniteLength));
-            }
+            read_eoc(reader)?;
 
             // Compute how much we read and flag the decoded length as indefinite
-            let mut ret = (current_pos - start_pos)?;
+            let mut ret = (reader.position() - start_pos)?;
             ret.indefinite = true;
             return Ok(ret);
         }
@@ -404,6 +416,21 @@ fn decode_indefinite_length<'a, R: Reader<'a>>(reader: &mut R) -> Result<Length>
     }
 }
 
+/// Read an expected end-of-contents (EOC) marker: `00 00`.
+///
+/// # Errors
+///
+/// - Returns `ErrorKind::IndefiniteLength` if the EOC marker isn't present as expected.
+pub(crate) fn read_eoc<'a>(reader: &mut impl Reader<'a>) -> Result<()> {
+    for _ in 0..=1 {
+        if reader.read_byte()? != 0 {
+            return Err(reader.error(ErrorKind::IndefiniteLength));
+        }
+    }
+
+    Ok(())
+}
+
 #[cfg(test)]
 #[allow(clippy::unwrap_used)]
 mod tests {
@@ -507,9 +534,6 @@ mod tests {
         /// Length of example in octets.
         const EXAMPLE_LEN: usize = 68;
 
-        /// Length of end-of-content octets (i.e. `00 00`).
-        const EOC_LEN: usize = 2;
-
         /// Test vector from: <https://github.com/RustCrypto/formats/issues/779#issuecomment-2902948789>
         ///
         /// Notably this example contains nested indefinite lengths to ensure the decoder handles
@@ -534,18 +558,15 @@ mod tests {
 
         // Decode indefinite length
         let length = Length::decode(&mut reader).unwrap();
-        assert!(length.indefinite);
+        assert!(length.is_indefinite());
 
         // Decoding the length should leave the position at the end of the indefinite length octet
         let pos = usize::try_from(reader.position()).unwrap();
         assert_eq!(pos, 2);
 
         // The first two bytes are the header and the rest is the length of the message.
         // The last four are two end-of-content markers (2 * 2 bytes).
-        assert_eq!(
-            usize::try_from(length).unwrap(),
-            EXAMPLE_LEN - pos - (EOC_LEN * 2)
-        );
+        assert_eq!(usize::try_from(length).unwrap(), EXAMPLE_LEN - pos);
 
         // Read OID
         reader.tlv_bytes().unwrap();
@@ -564,7 +585,7 @@ mod tests {
 
         // Parse the inner indefinite length
         let length = Length::decode(&mut reader).unwrap();
-        assert!(length.indefinite);
-        assert_eq!(usize::try_from(length).unwrap(), 18);
+        assert!(length.is_indefinite());
+        assert_eq!(usize::try_from(length).unwrap(), 20);
     }
 }

der/src/reader.rs

@@ -9,7 +9,7 @@ mod position;
 
 use crate::{
     Decode, DecodeValue, Encode, EncodingRules, Error, ErrorKind, FixedTag, Header, Length, Tag,
-    TagMode, TagNumber, asn1::ContextSpecific,
+    TagMode, TagNumber, asn1::ContextSpecific, length::read_eoc,
 };
 
 #[cfg(feature = "alloc")]
@@ -41,7 +41,14 @@ pub trait Reader<'r>: Clone {
         E: From<Error>,
         F: FnOnce(&mut Self) -> Result<T, E>,
     {
-        self.read_nested(header.length, f)
+        let ret = self.read_nested(header.length.sans_eoc(), f)?;
+
+        // Consume EOC marker if the length is indefinite.
+        if header.length.is_indefinite() {
+            read_eoc(self)?;
+        }
+
+        Ok(ret)
     }
 
     /// Attempt to read data borrowed directly from the input as a slice,