formats icon indicating copy to clipboard operation
formats copied to clipboard

der: breaking changes in `ObjectIdentifier` and `AnyRef` with `tag_mode = IMPLICIT`

Open kamulos opened this issue 6 months ago • 8 comments

So I was just trying to change my application from my self-made support for private tags to the upstream support in the release candidates. (Thanks @tarcieri and @turbocool3r for following through on that).

In my code two things broke, that might not be intentional. So this example is a minimized version of my issues:

use der::{AnyRef, Decode, FixedTag, Sequence, oid::ObjectIdentifier};

#[derive(Debug, Sequence)]
pub struct SomeType<'a> {
    pub oid: ObjectIdentifier,
    #[asn1(context_specific = "0", tag_mode = "IMPLICIT")]
    pub data: AnyRef<'a>,
}

fn main() {
    let _ = ObjectIdentifier::TAG;

    let bytes = b"\x30\x12\x06\x07\x2a\x86\x48\xce\x3d\x02\x01\x80\x07\x63\x6f\x6e\x74\x65\x6e\x74";
    let decoded = SomeType::from_der(bytes).unwrap();
    println!("{:02x?}", decoded.data.value());
}
  1. ObjectIdentifier::TAG does not work anymore without specifying MAX_SIZE. This broke in d4395cdb10f3b7ecfb2ccba4d3dd29e74f576576
  2. AnyRef does not work anymore with tag_mode = IMPLICIT. That broke in be1caa3cc42b06bf01a37d1858826a4905b74904

kamulos avatar Aug 01 '25 20:08 kamulos

Well,

  1. ObjectIdentifier::TAG is equal to Tag::ObjectIdentifier. Sorry for the breaking change. https://github.com/RustCrypto/formats/pull/1851#discussion_r2249318528 You can create a type alias like:
type ObjectIdentifier20 = der::ObjectIdentifier<20>
  1. AnyRef does not know if it is constructed, statically, and IMPLICIT en/decoder needs to know that. Also, tag inside of AnyRef looks unused and reduntant, so I recommend to use OctetStringRef instead of AnyRef.
#[asn1(context_specific = "0", tag_mode = "IMPLICIT")]
pub data: OctetStringRef<'a>,

or even better (edited):

#[asn1(context_specific = "0", tag_mode = "IMPLICIT", type = "OCTET STRING")]
pub data: &'a [u8],

dishmaker avatar Aug 02 '25 16:08 dishmaker

  1. Tag::ObjectIdentifier is actually way better in my opinion. So I am happy about that :) I am however wondering why this broke: shouldn't Rust take the default if it cannot infer the value?
  2. data: &'a [u8] looks perfect, but this does not work either for me. I get: the trait bound &[u8]: IsConstructed is not satisfied Consider adding impl of FixedTag to &[u8]
  3. data: OctetStringRef<'a> compiles and works with the example I gave. But I now experience a different issue. In the actual data I have the constructed bit is set.
use der::{AnyRef, Decode, Sequence, asn1::OctetStringRef, oid::ObjectIdentifier};

#[derive(Debug, Sequence)]
pub struct SomeType<'a> {
    pub oid: ObjectIdentifier,
    #[asn1(context_specific = "0", tag_mode = "IMPLICIT")]
    pub data: OctetStringRef<'a>,
}

fn main() {
    let bytes = b"\x30\x12\x06\x07\x2a\x86\x48\xce\x3d\x02\x01\xa0\x07\x63\x6f\x6e\x74\x65\x6e\x74";
    let decoded = SomeType::from_der(bytes).unwrap();
    println!("{:02x?}", decoded.data.as_bytes());
}

This panics with the following error:

called `Result::unwrap()` on an `Err` value: Error { kind: Noncanonical { tag: Tag(0x00: CONTEXT-SPECIFIC [0] (constructed)) }, position: Some(Length(13)) }

Adding constructed = "true" also does not help. Switching back to AnyRef in an older version of der works. I think it was pretty nice to have an AnyRef, that just does not care about the content. This was a good way to iteratively build the Rust structs, and using AnyRef as a placeholder or as raw data access.

Is there anything preventing AnyRef to have the same functionality it had?

kamulos avatar Aug 02 '25 18:08 kamulos

I am however wondering why this broke: shouldn't Rust take the default if it cannot infer the value?

I don't think it "falls back" on the default if inference fails, for either default generic types or const generic defaults. If you're in a context where a generic parameter can be inferred, it will only attempt inference.

The reason it worked before was the FixedTag impl was only for one concrete size. Now that the impl is generic, it can't infer the size like it could before.

tarcieri avatar Aug 02 '25 18:08 tarcieri

data: &'a [u8] looks perfect, but this does not work either for me

Oh, sorry, i think you should add type = "OCTET STRING"

#[asn1(context_specific = "0", tag_mode = "IMPLICIT", type = "OCTET STRING")]
pub data: &'a [u8],

In the actual data I have the constructed bit is set.

Because the inner type is constructed for some reason, you'll need to create a newtype with IsConstructed true. But I wonder why do you need IMPLICIT encoding here. Shouldn't that be EXPLICIT AnyRef?

dishmaker avatar Aug 02 '25 20:08 dishmaker

Is there anything preventing AnyRef to have the same functionality it had?

Sorry for opinionated changes in der. The same functionality should work, but only with EXPLICIT encoding. Per IMPLICIT and AnyRef case: personally i'm against such combination, because AnyRef holds unused redundant Tag inside.

dishmaker avatar Aug 02 '25 20:08 dishmaker

I think you can use SequenceRef with IMPLICIT for the tag to be constructed.

dishmaker avatar Aug 02 '25 21:08 dishmaker

But I wonder why do you need IMPLICIT encoding here. Shouldn't that be EXPLICIT AnyRef?

Well an explicit AnyRef without any #[asn1(...)] Attribute will work, but then I don't get the checks during parsing, that it really is "context specific 0".

I thInk you can use SequenceRef with IMPLICIT for the tag to be constructed.

I tried that and on first glance it seems to work. I only need the additional changes of #1977 . Will do some extensive tests next week.

I am very aware, that the formats I am trying to parse are cursed, but the discoverability of OctetStringRef and SequenceRef for this purpose is pretty low. So the next person doing something similar will also have a hard time.

For the moment I am happy, @tarcieri feel free to close this if all breaking changes here are acceptable.

kamulos avatar Aug 02 '25 22:08 kamulos

It's good to hear people's pain points. I can keep it open for now and see if it trips anyone else up.

tarcieri avatar Aug 02 '25 23:08 tarcieri