Skip to content
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

DER: Over-eager consumption in AnyRef #815

Closed
woodruffw opened this issue Dec 21, 2022 · 4 comments
Closed

DER: Over-eager consumption in AnyRef #815

woodruffw opened this issue Dec 21, 2022 · 4 comments

Comments

@woodruffw
Copy link
Contributor

This is a discrete issue for the behavior I'm seeing while testing #813, specifically in #813 (comment).

There, we have a struct that's annotated like so:

#[derive(Clone, Copy, Debug, Eq, PartialEq, Sequence)]
pub struct EncapsulatedContentInfo<'a> {
    /// indicates the type of content.
    pub e_content_type: ObjectIdentifier,

    /// encapsulated content
    #[asn1(context_specific = "0", optional = "true", tag_mode = "EXPLICIT")]
    pub e_content: Option<AnyRef<'a>>,
}

My expectation is that this definition would consume a DER structure like this:

SEQUENCE
  OBJECT IDENTIFIER 1.2.3.4
  [0]
    SEQUENCE
      ANYTHING HERE POSSIBLY JUST AN OCTET STRING

...leaving e_content_type as 1.2.3.4 and e_content as:

SEQUENCE
  ANYTHING HERE POSSIBLY JUST AN OCTET STRING

Experimentally, however, the borrowed slice in e_content's AnyRef contains:

ANYTHING HERE POSSIBLY JUST AN OCTET STRING

...meaning that the surrounding SEQUENCE was consumed. When I remove the #[asn1(...)] annotation from e_content, the behavior changes to what I expect:

SEQUENCE
  ANYTHING HERE POSSIBLY JUST AN OCTET STRING

This suggests a bug to me, somewhere in the interaction between the #[asn1(...)] annotation and AnyRef.

I'll work on a minimal reproduction now, but this behavior is currently visible with the PKCS7 blob I shared on the other PR (https://github.com/RustCrypto/formats/files/10270816/authroot.stl.zip) and the changes in that PR.

@woodruffw
Copy link
Contributor Author

Looking at the Decode impl for AnyRef, this is maybe a design decision that we're just holding incorrectly on #813:

impl<'a> Decode<'a> for AnyRef<'a> {
    fn decode<R: Reader<'a>>(reader: &mut R) -> Result<AnyRef<'a>> {
        let header = Header::decode(reader)?;

        Ok(Self {
            tag: header.tag,
            value: ByteSlice::decode_value(reader, header)?,
        })
    }
}

AnyRef looks for and consumes a header, so its interaction with #[asn1(...)] is that the annotation consumes the context-specific tag, and then AnyRef consumes the SEQUENCE tag that immediately follows it. As a result, we've actually asked the parser two consume two headers, even though it looks (on first glance) like we've only asked it to consume one.

Given that, I don't think this is a bug in the der crate, just a point of API confusion 🙂. I can update AnyRef's documentation to clarify that it's a "consuming" type rather than merely peeks at the TLV structure, if that sounds good to you @tarcieri.

@woodruffw
Copy link
Contributor Author

(This also makes sense given the decode_into API -- that's a consuming API that reuses the AnyRef's interior consumed tag, so it makes sense that AnyRef consumes the DER stream rather than just peeking it.)

@tarcieri
Copy link
Member

@woodruffw yeah, that’s expected. If you want to store the original DER, use der::Document

@woodruffw
Copy link
Contributor Author

Closing in that case, since this was entirely a misunderstanding of the APIs on my part!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants