Skip to content

Commit

Permalink
der: add IndefiniteLength type (#830)
Browse files Browse the repository at this point in the history
Per #823 and #827, making PKCS#7 work interoperably will involve
supporting at limited number of BER productions, one of which is
indefinite lengths.

The current built-in `Length` type rejects them, as a proper DER parser
is expected to.

This commit adds a separate `IndefiniteLength` type as a newtype of
`Option<Length>` with support for parsing indefinite lengths.
  • Loading branch information
tarcieri authored Jan 2, 2023
1 parent b79a4c1 commit 1f1dd1a
Show file tree
Hide file tree
Showing 3 changed files with 134 additions and 4 deletions.
4 changes: 4 additions & 0 deletions der/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,9 @@ pub enum ErrorKind {
#[cfg_attr(docsrs, doc(cfg(feature = "std")))]
Io(std::io::ErrorKind),

/// Indefinite length disallowed.
IndefiniteLength,

/// Incorrect length for a given field.
Length {
/// Tag of the value being decoded.
Expand Down Expand Up @@ -321,6 +324,7 @@ impl fmt::Display for ErrorKind {
),
#[cfg(feature = "std")]
ErrorKind::Io(err) => write!(f, "I/O error: {:?}", err),
ErrorKind::IndefiniteLength => write!(f, "indefinite length disallowed"),
ErrorKind::Length { tag } => write!(f, "incorrect length for {}", tag),
ErrorKind::Noncanonical { tag } => {
write!(f, "ASN.1 {} not canonically encoded as DER", tag)
Expand Down
132 changes: 129 additions & 3 deletions der/src/length.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,13 @@ const MAX_DER_OCTETS: usize = 5;
/// Maximum length as a `u32` (256 MiB).
const MAX_U32: u32 = 0xfff_ffff;

/// Octet identifying an indefinite length as described in X.690 Section
/// 8.1.3.6.1:
///
/// > The single octet shall have bit 8 set to one, and bits 7 to
/// > 1 set to zero.
const INDEFINITE_LENGTH_OCTET: u8 = 0b10000000; // 0x80

/// ASN.1-encoded length.
///
/// Maximum length is defined by the [`Length::MAX`] constant (256 MiB).
Expand Down Expand Up @@ -204,7 +211,8 @@ impl<'a> Decode<'a> for Length {
match reader.read_byte()? {
// 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()),
len if len < INDEFINITE_LENGTH_OCTET => Ok(len.into()),
INDEFINITE_LENGTH_OCTET => Err(ErrorKind::IndefiniteLength.into()),
// 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 @@ -299,9 +307,113 @@ impl<'a> arbitrary::Arbitrary<'a> for Length {
}
}

/// Length type with support for indefinite lengths as used by ASN.1 BER,
/// as described in X.690 Section 8.1.3.6:
///
/// > 8.1.3.6 For the indefinite form, the length octets indicate that the
/// > contents octets are terminated by end-of-contents
/// > octets (see 8.1.5), and shall consist of a single octet.
/// >
/// > 8.1.3.6.1 The single octet shall have bit 8 set to one, and bits 7 to
/// > 1 set to zero.
/// >
/// > 8.1.3.6.2 If this form of length is used, then end-of-contents octets
/// > (see 8.1.5) shall be present in the encoding following the contents
/// > octets.
///
/// Indefinite lengths are non-canonical and therefore invalid DER, however
/// there are interoperability corner cases where we have little choice but to
/// tolerate some BER productions where this is helpful.
#[derive(Copy, Clone, Debug, Eq, PartialEq, PartialOrd, Ord)]
pub struct IndefiniteLength(Option<Length>);

impl IndefiniteLength {
/// Length of `0`.
pub const ZERO: Self = Self(Some(Length::ZERO));

/// Length of `1`.
pub const ONE: Self = Self(Some(Length::ONE));

/// Indefinite length.
pub const INDEFINITE: Self = Self(None);
}

impl IndefiniteLength {
/// Create a definite length from a type which can be converted into a
/// `Length`.
pub fn new(length: impl Into<Length>) -> Self {
Self(Some(length.into()))
}

/// Is this length definite?
pub fn is_definite(self) -> bool {
self.0.is_some()
}
/// Is this length indefinite?
pub fn is_indefinite(self) -> bool {
self.0.is_none()
}
}

impl<'a> Decode<'a> for IndefiniteLength {
fn decode<R: Reader<'a>>(reader: &mut R) -> Result<IndefiniteLength> {
if reader.peek_byte() == Some(INDEFINITE_LENGTH_OCTET) {
// Consume the byte we already peeked at.
let byte = reader.read_byte()?;
debug_assert_eq!(byte, INDEFINITE_LENGTH_OCTET);

Ok(Self::INDEFINITE)
} else {
Length::decode(reader).map(Into::into)
}
}
}

impl Encode for IndefiniteLength {
fn encoded_len(&self) -> Result<Length> {
match self.0 {
Some(length) => length.encoded_len(),
None => Ok(Length::ONE),
}
}

fn encode(&self, writer: &mut impl Writer) -> Result<()> {
match self.0 {
Some(length) => length.encode(writer),
None => writer.write_byte(INDEFINITE_LENGTH_OCTET),
}
}
}

impl From<Length> for IndefiniteLength {
fn from(length: Length) -> IndefiniteLength {
Self(Some(length))
}
}

impl From<Option<Length>> for IndefiniteLength {
fn from(length: Option<Length>) -> IndefiniteLength {
IndefiniteLength(length)
}
}

impl From<IndefiniteLength> for Option<Length> {
fn from(length: IndefiniteLength) -> Option<Length> {
length.0
}
}

impl TryFrom<IndefiniteLength> for Length {
type Error = Error;

fn try_from(length: IndefiniteLength) -> Result<Length> {
length.0.ok_or_else(|| ErrorKind::IndefiniteLength.into())
}
}

#[cfg(test)]
mod tests {
use super::Length;
use super::{IndefiniteLength, Length};
use crate::{Decode, DerOrd, Encode, ErrorKind};
use core::cmp::Ordering;

Expand Down Expand Up @@ -368,8 +480,22 @@ mod tests {
}

#[test]
fn reject_indefinite_lengths() {
fn indefinite_lengths() {
// DER disallows indefinite lengths
assert!(Length::from_der(&[0x80]).is_err());

// The `IndefiniteLength` type supports them
let indefinite_length = IndefiniteLength::from_der(&[0x80]).unwrap();
assert!(indefinite_length.is_indefinite());
assert_eq!(indefinite_length, IndefiniteLength::INDEFINITE);

// It also supports definite lengths.
let length = IndefiniteLength::from_der(&[0x83, 0x01, 0x00, 0x00]).unwrap();
assert!(length.is_definite());
assert_eq!(
Length::try_from(0x10000u32).unwrap(),
length.try_into().unwrap()
);
}

#[test]
Expand Down
2 changes: 1 addition & 1 deletion der/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -360,7 +360,7 @@ pub use crate::{
encode_ref::{EncodeRef, EncodeValueRef},
error::{Error, ErrorKind, Result},
header::Header,
length::Length,
length::{IndefiniteLength, Length},
ord::{DerOrd, ValueOrd},
reader::{slice::SliceReader, Reader},
tag::{Class, FixedTag, Tag, TagMode, TagNumber, Tagged},
Expand Down

0 comments on commit 1f1dd1a

Please sign in to comment.