`Option<T>` with `#[derive(DerSequence)]` fails in certain scenarios

[Alpha-3]

impl<'a, T> FromDer<'a> for Option<T> has a bug where deserialization of Option<T> in a struct annotated with #[derive(DerSequence)] will fail in with DerConstraintFailed(..) in certain cases.

Example of test failure where test() here will fail with Error(DerConstraintFailed(InvalidBoolean)) from TestBool::from_der(expected).unwrap() as bool/Boolean has a strict DER constraint check on the first byte of the following field:

#[derive(DerSequence, Debug, PartialEq)]
struct TestBool {
    a: u16,
    b: Option<bool>,
    c: u32,
}

#[test]
fn test() {
    let x = TestBool {
        a: 0x1234,
        b: None,
        c: 0x5678,
    };

    let mut buffer = vec![];
    let _ = x.write_der(&mut buffer).unwrap();

    let expected = &[48, 8, 2, 2, 18, 52, 2, 2, 86, 120];
    assert_eq!(buffer, expected);

    let (_, val) = TestBool::from_der(expected).unwrap();
    assert_eq!(val, x);
}

#[test]
fn test2() {
    let x = TestBool {
        a: 0x1234,
        b: Some(true),
        c: 0x5678,
    };

    let mut buffer = vec![];
    let _ = x.write_der(&mut buffer).unwrap();

    let expected = &[48, 11, 2, 2, 18, 52, 1, 1, 255, 2, 2, 86, 120];
    assert_eq!(buffer, expected);

    let (_, val) = TestBool::from_der(expected).unwrap();
    assert_eq!(val, x);
}

A similar example where the contained type has more relaxed constraints (u32 instead of bool) works:

#[derive(DerSequence, Debug, PartialEq)]
struct TestInt {
    a: u16,
    b: Option<u32>,
    c: bool,
}

#[test]
fn test3() {
    let x = TestInt {
        a: 0x1234,
        b: None,
        c: true,
    };

    let mut buffer = vec![];
    let _ = x.write_der(&mut buffer).unwrap();

    let expected = &[48, 7, 2, 2, 18, 52, 1, 1, 255];
    assert_eq!(buffer, expected);

    let (_, val) = TestInt::from_der(expected).unwrap();
    assert_eq!(val, x);
}

#[test]
fn test4() {
    let x = TestInt {
        a: 0x1234,
        b: Some(0x5678),
        c: true,
    };

    let mut buffer = vec![];
    let _ = x.write_der(&mut buffer).unwrap();

    let expected = &[48, 11, 2, 2, 18, 52, 2, 2, 86, 120, 1, 1, 255];
    assert_eq!(buffer, expected);

    let (_, val) = TestInt::from_der(expected).unwrap();
    assert_eq!(val, x);
}

This could be fixed by accounting for Error::DerConstraintFailed in impl<'a, T> FromDer<'a> for Option<T>, but perhaps there are other edge cases to consider as well?

impl<'a, T> FromDer<'a> for Option<T>
where
    T: FromDer<'a>,
{
    fn from_der(bytes: &'a [u8]) -> ParseResult<Self> {
        if bytes.is_empty() {
            return Ok((bytes, None));
        }
        match T::from_der(bytes) {
            Ok((rem, t)) => Ok((rem, Some(t))),
            Err(nom::Err::Error(Error::UnexpectedTag { .. })) => Ok((bytes, None)),
            Err(nom::Err::Error(Error::DerConstraintFailed(..))) => Ok((bytes, None)),
            Err(e) => Err(e),
        }
    }
}

[chifflier]

Hi,

Thank you for the bug report, it took me some time to get back in the code and then figure out entirely what is happening.

After investigating, the chain of events leading to the problem is:

• Option::from_der calls T::from_der here
• FromDer::from_der, first calls Any::from_der here ) which suceeds (parsing an integer), then calls <T>::check_constraints with T = bool (which will fail).

It is not directly possible to add Error::DerConstraintFailed to Option::from_der, this would be too laxist and would allow any encoding to pass.

I'm still investigating, but I think the solution is to change Option::from_der to

1. parse the header and check the tag first
2. call T::from_der