Skip to content

WIP: Parsing with lax mode #810

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions der/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ proptest = "1"
[features]
alloc = []
derive = ["der_derive"]
lax = []
oid = ["const-oid"]
pem = ["alloc", "pem-rfc7468/alloc", "zeroize"]
real = []
Expand Down
11 changes: 11 additions & 0 deletions der/src/asn1/octet_string.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -51,6 +54,14 @@ impl AsRef<[u8]> for OctetStringRef<'_> {

impl<'a> DecodeValue<'a> for OctetStringRef<'a> {
fn decode_value<R: Reader<'a>>(reader: &mut R, header: Header) -> Result<Self> {
#[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 })
}
Expand Down
4 changes: 4 additions & 0 deletions der/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
///
Expand Down Expand Up @@ -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"),
Expand Down
6 changes: 6 additions & 0 deletions der/src/length.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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());
}
Expand Down
36 changes: 34 additions & 2 deletions der/src/reader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -56,7 +59,12 @@ pub trait Reader<'r>: Sized {

/// Decode a value which impls the [`Decode`] trait.
fn decode<T: Decode<'r>>(&mut self) -> Result<T> {
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
Expand All @@ -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<T>(self, value: T) -> Result<T> {
fn finish<T>(&mut self, value: T) -> Result<T> {
#[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(),
Expand Down Expand Up @@ -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.
///
Expand Down
28 changes: 27 additions & 1 deletion der/src/reader/nested.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Self> {

#[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 {
Expand All @@ -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 {
Expand All @@ -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<u8> {
if self.is_finished() {
None
Expand Down
15 changes: 12 additions & 3 deletions der/src/reader/slice.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,18 +97,27 @@ 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 {
self.failed = true;
kind.at(self.position)
}

fn finish<T>(self, value: T) -> Result<T> {
fn finish<T>(&mut self, value: T) -> Result<T> {
#[cfg(feature = "lax")]
self.read_eoc()?;

if self.is_failed() {
Err(ErrorKind::Failed.at(self.position))
} else if !self.is_finished() {
Expand Down
2 changes: 2 additions & 0 deletions der/src/tag.rs
Original file line number Diff line number Diff line change
Expand Up @@ -281,6 +281,8 @@ impl TryFrom<u8> 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 {
Expand Down