From f3710bf49bfb3ed93971e93e80b51eb490fe8caf Mon Sep 17 00:00:00 2001 From: Simon Dieterle Date: Mon, 19 Dec 2022 23:11:11 +0100 Subject: [PATCH] add lax mode for indefinte length and constructed OctetStrings --- der/Cargo.toml | 1 + der/src/asn1/octet_string.rs | 11 +++++++++++ der/src/error.rs | 4 ++++ der/src/length.rs | 6 ++++++ der/src/reader.rs | 36 ++++++++++++++++++++++++++++++++++-- der/src/reader/nested.rs | 28 +++++++++++++++++++++++++++- der/src/reader/slice.rs | 15 ++++++++++++--- der/src/tag.rs | 2 ++ 8 files changed, 97 insertions(+), 6 deletions(-) diff --git a/der/Cargo.toml b/der/Cargo.toml index c0da5cf68..10ebe3095 100644 --- a/der/Cargo.toml +++ b/der/Cargo.toml @@ -30,6 +30,7 @@ proptest = "1" [features] alloc = [] derive = ["der_derive"] +lax = [] oid = ["const-oid"] pem = ["alloc", "pem-rfc7468/alloc", "zeroize"] real = [] diff --git a/der/src/asn1/octet_string.rs b/der/src/asn1/octet_string.rs index 6f5b91a3d..0ce03f45e 100644 --- a/der/src/asn1/octet_string.rs +++ b/der/src/asn1/octet_string.rs @@ -8,6 +8,9 @@ use crate::{ #[cfg(feature = "alloc")] use alloc::vec::Vec; +#[cfg(feature = "lax")] +use crate::decode::Decode; + /// ASN.1 `OCTET STRING` type: borrowed form. /// /// Octet strings represent contiguous sequences of octets, a.k.a. bytes. @@ -51,6 +54,14 @@ impl AsRef<[u8]> for OctetStringRef<'_> { impl<'a> DecodeValue<'a> for OctetStringRef<'a> { fn decode_value>(reader: &mut R, header: Header) -> Result { + #[cfg(feature = "lax")] + /// only reads a single value of a constructed OctetString + if header.length == Length::ZERO { + let header = Header::decode(reader)?; + let inner = ByteSlice::decode_value(reader, header)?; + return Ok(Self { inner }); + } + let inner = ByteSlice::decode_value(reader, header)?; Ok(Self { inner }) } diff --git a/der/src/error.rs b/der/src/error.rs index 5e492a48d..b03589ad1 100644 --- a/der/src/error.rs +++ b/der/src/error.rs @@ -165,6 +165,9 @@ pub enum ErrorKind { /// Date-and-time related errors. DateTime, + /// Emd-of-content related errors. + EndOfContent, + /// This error indicates a previous DER parsing operation resulted in /// an error and tainted the state of a `Decoder` or `Encoder`. /// @@ -308,6 +311,7 @@ impl fmt::Display for ErrorKind { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { ErrorKind::DateTime => write!(f, "date/time error"), + ErrorKind::EndOfContent => write!(f, "end-of-content invalid"), ErrorKind::Failed => write!(f, "operation failed"), #[cfg(feature = "std")] ErrorKind::FileNotFound => write!(f, "file not found"), diff --git a/der/src/length.rs b/der/src/length.rs index 76ee0e997..471cc3ce1 100644 --- a/der/src/length.rs +++ b/der/src/length.rs @@ -205,6 +205,10 @@ impl<'a> Decode<'a> for Length { // Note: per X.690 Section 8.1.3.6.1 the byte 0x80 encodes indefinite // lengths, which are not allowed in DER, so disallow that byte. len if len < 0x80 => Ok(len.into()), + // Note: we don't know the length so we set it to zero. Consumers like + // Header should make sure to use the correct length. + #[cfg(feature = "lax")] + _len @ 0x80 => Ok(Self::ZERO), // 1-4 byte variable-sized length prefix tag @ 0x81..=0x84 => { let nbytes = tag.checked_sub(0x80).ok_or(ErrorKind::Overlength)? as usize; @@ -355,6 +359,8 @@ mod tests { } #[test] + #[cfg(not(feature = "lax"))] + // Note: lax mode allows for reading BER-style indefinte length fn reject_indefinite_lengths() { assert!(Length::from_der(&[0x80]).is_err()); } diff --git a/der/src/reader.rs b/der/src/reader.rs index b917323c2..de90cf384 100644 --- a/der/src/reader.rs +++ b/der/src/reader.rs @@ -15,6 +15,9 @@ use crate::{ #[cfg(feature = "alloc")] use alloc::vec::Vec; +#[cfg(feature = "lax")] +const EOC_LENGTH: Length = Length::new(2); + /// Reader trait which reads DER-encoded input. pub trait Reader<'r>: Sized { /// Get the length of the input. @@ -56,7 +59,12 @@ pub trait Reader<'r>: Sized { /// Decode a value which impls the [`Decode`] trait. fn decode>(&mut self) -> Result { - T::decode(self).map_err(|e| e.nested(self.position())) + let value = T::decode(self).map_err(|e| e.nested(self.position())); + + #[cfg(feature = "lax")] + self.read_eoc()?; + + value } /// Return an error with the given [`ErrorKind`], annotating it with @@ -67,8 +75,18 @@ pub trait Reader<'r>: Sized { /// Finish decoding, returning the given value if there is no /// remaining data, or an error otherwise - fn finish(self, value: T) -> Result { + fn finish(&mut self, value: T) -> Result { + #[cfg(feature = "lax")] + self.read_eoc()?; + if !self.is_finished() { + // Note: lax mode never finishes + #[cfg(feature = "lax")] + { + return Ok(value); + } + + Err(ErrorKind::TrailingData { decoded: self.position(), remaining: self.remaining_len(), @@ -111,6 +129,20 @@ pub trait Reader<'r>: Sized { Ok(buf[0]) } + #[cfg(feature = "lax")] + /// Read a end-of-content value and verify it. + fn read_eoc(&mut self) -> Result<()> { + let next = self.peek_byte(); + if next == Some(0) { + let eoc = self.read_slice(EOC_LENGTH)?; + if eoc.ne(&[0u8, 0]) { + return Err(ErrorKind::EndOfContent.at(self.position())) + } + } + + Ok(()) + } + /// Attempt to read input data, writing it into the provided buffer, and /// returning a slice on success. /// diff --git a/der/src/reader/nested.rs b/der/src/reader/nested.rs index 40ede69ac..feb416db0 100644 --- a/der/src/reader/nested.rs +++ b/der/src/reader/nested.rs @@ -12,16 +12,32 @@ pub struct NestedReader<'i, R> { /// Position within the nested input. position: Length, + + /// Keeps track if the reader has indefinite length + indefinite: bool, } impl<'i, 'r, R: Reader<'r>> NestedReader<'i, R> { /// Create a new nested reader which can read the given [`Length`]. pub(crate) fn new(inner: &'i mut R, len: Length) -> Result { + + #[cfg(feature = "lax")] + if len.is_zero() { + + return Ok(Self { + inner: inner, + input_len: len, + position: Length::ZERO, + indefinite: true, + }) + } + if len <= inner.remaining_len() { Ok(Self { inner, input_len: len, position: Length::ZERO, + indefinite: false, }) } else { Err(ErrorKind::Incomplete { @@ -37,7 +53,7 @@ impl<'i, 'r, R: Reader<'r>> NestedReader<'i, R> { fn advance_position(&mut self, len: Length) -> Result<()> { let new_position = (self.position + len)?; - if new_position <= self.input_len { + if self.indefinite | (new_position <= self.input_len) { self.position = new_position; Ok(()) } else { @@ -55,6 +71,16 @@ impl<'i, 'r, R: Reader<'r>> Reader<'r> for NestedReader<'i, R> { self.input_len } + /// Get the number of bytes still remaining in the buffer or inner buffer remaining_len when indefinire + fn remaining_len(&self) -> Length { + if self.indefinite { + return self.inner.remaining_len() + } + + debug_assert!(self.position() <= self.input_len()); + self.input_len().saturating_sub(self.position()) + } + fn peek_byte(&self) -> Option { if self.is_finished() { None diff --git a/der/src/reader/slice.rs b/der/src/reader/slice.rs index fe7560807..acff38dc6 100644 --- a/der/src/reader/slice.rs +++ b/der/src/reader/slice.rs @@ -97,10 +97,16 @@ impl<'a> Reader<'a> for SliceReader<'a> { return Err(self.error(ErrorKind::Failed)); } - T::decode(self).map_err(|e| { + let value = T::decode(self).map_err(|e| { self.failed = true; e.nested(self.position) - }) + }); + + // Note: peek to see if BER-style indefinite length sections ends now + #[cfg(feature = "lax")] + self.read_eoc()?; + + value } fn error(&mut self, kind: ErrorKind) -> Error { @@ -108,7 +114,10 @@ impl<'a> Reader<'a> for SliceReader<'a> { kind.at(self.position) } - fn finish(self, value: T) -> Result { + fn finish(&mut self, value: T) -> Result { + #[cfg(feature = "lax")] + self.read_eoc()?; + if self.is_failed() { Err(ErrorKind::Failed.at(self.position)) } else if !self.is_finished() { diff --git a/der/src/tag.rs b/der/src/tag.rs index 0e7bf2013..34911a176 100644 --- a/der/src/tag.rs +++ b/der/src/tag.rs @@ -281,6 +281,8 @@ impl TryFrom for Tag { 0x18 => Ok(Tag::GeneralizedTime), 0x1A => Ok(Tag::VisibleString), 0x1d => Ok(Tag::BmpString), + #[cfg(feature = "lax")] + 0x24 => Ok(Tag::OctetString), // constructed 0x30 => Ok(Tag::Sequence), // constructed 0x31 => Ok(Tag::Set), // constructed 0x40..=0x7E => Ok(Tag::Application {