librasn / rasn

A Safe #[no_std] ASN.1 Codec Framework
Other
183 stars 43 forks source link

Unable to round-trip with APER #201

Closed FrancisRussell closed 7 months ago

FrancisRussell commented 7 months ago

I've been trying to use retrofit rasn into a codebase which requires aligned PER messages. Though I'm somewhat limited with my understanding of ASN.1/PER, I managed to produce the following test case which to my understanding should just be an identity operation but currently fails, though potentially I am just using rasn incorrectly:

use rasn::prelude::*;

const T124_IDENTIFIER_KEY: &Oid = Oid::const_new(&[0, 0, 20, 124, 0, 1]);

#[derive(Debug, AsnType, rasn::Encode, rasn::Decode)]
#[rasn(choice)]
enum Key {
    Object(ObjectIdentifier),
    H221NonStandard(OctetString),
}

#[derive(Debug, AsnType, rasn::Encode, rasn::Decode)]
#[rasn(automatic_tags)]
struct ConnectData {
    t124_identifier_key: Key,
    connect_pdu: OctetString,
}

fn main() {
    let connect_pdu: OctetString = vec![0u8, 1u8, 2u8, 3u8].into();
    let connect_data = ConnectData {
        t124_identifier_key: Key::Object(T124_IDENTIFIER_KEY.into()),
        connect_pdu,
    };
    let encoded = rasn::aper::encode(&connect_data).expect("failed to encode");
    let decoded: ConnectData = rasn::aper::decode(&encoded).expect("failed to decode");
    println!("Reconstructed: {:?}", decoded);
}

When executed, I get the following failure (backtrace truncated):

thread 'main' panicked at src/main.rs:28:61:
failed to decode: DecodeError { kind: FieldError { name: "ConnectData.t124_identifier_key", nested: DecodeError { kind: Incomplete { needed: Size(6072) }, codec: Aper, backtrace: Backtrace(   0: <snafu::backtrace_shim::Backtrace as snafu::GenerateImplicitData>::generate
             at /home/fpr/local/cargo/registry/src/index.crates.io-6f17d22bba15001f/snafu-0.7.5/src/backtrace_shim.rs:15:19
      rasn::error::decode::DecodeError::from_kind
             at /home/fpr/local/cargo/registry/src/index.crates.io-6f17d22bba15001f/rasn-0.12.0/src/error/decode.rs:287:24
   1: <rasn::error::decode::DecodeError as rasn::de::Error>::incomplete
             at /home/fpr/local/cargo/registry/src/index.crates.io-6f17d22bba15001f/rasn-0.12.0/src/error/decode.rs:596:9
   2: rasn::error::decode::DecodeError::map_nom_err
             at /home/fpr/local/cargo/registry/src/index.crates.io-6f17d22bba15001f/rasn-0.12.0/src/error/decode.rs:277:52
   3: rasn::per::de::Decoder::decode_octets::{{closure}}::{{closure}}
             at /home/fpr/local/cargo/registry/src/index.crates.io-6f17d22bba15001f/rasn-0.12.0/src/per/de.rs:175:30
   4: core::result::Result<T,E>::map_err
             at /rustc/aa1a71e9e90f6eb3aed8cf79fc80bea304c17ecb/library/core/src/result.rs:829:27
   5: rasn::per::de::Decoder::decode_octets::{{closure}}
             at /home/fpr/local/cargo/registry/src/index.crates.io-6f17d22bba15001f/rasn-0.12.0/src/per/de.rs:174:33
   6: rasn::per::de::Decoder::decode_unknown_length
             at /home/fpr/local/cargo/registry/src/index.crates.io-6f17d22bba15001f/rasn-0.12.0/src/per/de.rs:206:17
   7: rasn::per::de::Decoder::decode_length
             at /home/fpr/local/cargo/registry/src/index.crates.io-6f17d22bba15001f/rasn-0.12.0/src/per/de.rs:301:20
   8: rasn::per::de::Decoder::decode_octets
             at /home/fpr/local/cargo/registry/src/index.crates.io-6f17d22bba15001f/rasn-0.12.0/src/per/de.rs:173:21
   9: <rasn::per::de::Decoder as rasn::de::Decoder>::decode_object_identifier
             at /home/fpr/local/cargo/registry/src/index.crates.io-6f17d22bba15001f/rasn-0.12.0/src/per/de.rs:680:22
  10: <rasn::types::oid::ObjectIdentifier as rasn::de::Decode>::decode_with_tag_and_constraints
             at /home/fpr/local/cargo/registry/src/index.crates.io-6f17d22bba15001f/rasn-0.12.0/src/de.rs:468:9
FrancisRussell commented 7 months ago

Ah, by adding automatic_tags to Key (I guess the omission might be part of the problem), I get instead:

thread 'main' panicked at src/main.rs:27:61:
failed to decode: DecodeError { kind: FieldError { name: "ConnectData.t124_identifier_key", nested: DecodeError { kind: Parser { msg: "Invalid length fragment" }, codec: Aper, backtrace: Backtrace(   0: rasn::error::decode::DecodeError::from_kind
   1: rasn::per::de::Decoder::decode_length
   2: rasn::per::de::Decoder::decode_octets
   3: <rasn::per::de::Decoder as rasn::de::Decoder>::decode_object_identifier
   4: <testcase::Key as rasn::types::DecodeChoice>::from_tag
   5: <rasn::per::de::Decoder as rasn::de::Decoder>::decode_choice
   6: <rasn::per::de::Decoder as rasn::de::Decoder>::decode_sequence
   7: rasn::per::decode
   8: testcase::main
   9: std::sys_common::backtrace::__rust_begin_short_backtrace
  10: std::rt::lang_start::{{closure}}
XAMPPRocky commented 7 months ago

Thank you for your issue! Can you check if you're using the latest version there was a fix for APER in the last version.

FrancisRussell commented 7 months ago

This was produced with rasn 0.12.0 and rustc 1.75.0-nightly (aa1a71e9e 2023-10-26).

Nicceboy commented 7 months ago

Seems like it happens in older versions too, and new changes did not introduce it.

Uper roundtrip fails too.

Nicceboy commented 7 months ago

There are two issues, at first the tag value as parameter is incorrect here:

https://github.com/XAMPPRocky/rasn/blob/cdc4e24458ec727c7a2b029cc94ff961eca1075d/src/per/enc.rs#L838

Should be Tag::OBJECT_IDENTIFIER. This fixes the issue for UPER.

Secondly, alignment is incorrectly parsed on APER (maybe when parsing unknown length), but I could not figure out the correct way yet. This would require reading standard quite a bit.

FrancisRussell commented 7 months ago

Since I feel context/a use-case is helpful, I came across this because I wanted to replicate the output of this function using rasn, though since I was unclear if I should be able to expect bit identical output, I wanted to get rasn to parse the existing output as well for sanity checking.

Nicceboy commented 7 months ago

Actually figured it out, it is similar problem that we just fixed.

When encoding choice, also the size of the choice index encoding should be noted when adjusting the alignment when encoding the inner value.

With current implementation this might be difficult to solve, since the inner value is encoded at first, so that we get the tag to identify the index for the correct variant.

But we should know it before we encode the inner value, so that we know the correct size to note...

Somehow we should know the selected variant before encoding the inner parts, is there any easy way?

XAMPPRocky commented 7 months ago

Somehow we should know the selected variant before encoding the inner parts, is there any easy way?

I don't think there's a way to know before right now but it probably wouldn't be fitfully to add that to the trait implementation.

@FrancisRussell The fix was just released with 0.12.1, so you should be able to keep going now :) If you have any questions or problems feel free to create an issue or discussion on the GitHub.

FrancisRussell commented 7 months ago

Thanks for the quick response! One thing I did note is that bumping rasn didn't cause rasn-derive to be bumped, which caused a compilation error until I did a cargo update.