librasn / rasn

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

Fix/issue 165 #172

Closed 6d7a closed 11 months ago

6d7a commented 11 months ago

Closes #165

I cross-checked the encoding output in the issue using asn1.io, assuming the following ASN1 to represent the struct mentioned here:

    Issue-165 DEFINITIONS IMPLICIT TAGS ::=
    BEGIN

    Signatures ::= SEQUENCE OF MultisigPolicySignature

    MultisigPolicySignature ::= CHOICE {
        key [0] SEQUENCE {
            key [0] UTF8String
        }
    }

    END

When encoding nested SEQUENCEs, the rasn encoder did not take the custom tag into account, which in case of the nestes key-SEQUENCE and was producing a wrong tag, i.e. UNIVERSAL 0. I changed the call in the macro for encoding inner items from a simple encode to encode_with_tag. The tests run fine, and I tried to trace the different cases we could encounter through the macro process and I couldn't find any side-effects on other cases. You'll likely have a much more complete view of how the different macro calls come together, @XAMPPRocky.

Let me know if I need to do more digging.

XAMPPRocky commented 11 months ago

Thank you for your PR!

orthecreedence commented 11 months ago

Thanks @6d7a! I wasn't sure if it was an encode/decode issue...I was leaning decode, but looks like my intuition was wrong. Looks like I need to do a bit more reading of ASN1. I'll keep that asn1.io site in my back pocket from now on.

6d7a commented 11 months ago

No worries, thanks for the extensive documentation of the issue, that helped a lot during debugging. asn1.io is a pretty solid resource, at least as far as 5 compilations per hour get you.

XAMPPRocky commented 11 months ago

at least as far as 5 compilations per hour get you.

Yeah let's just a say strong motivation for getting more of the codecs working and the compiler, is to hopefully one day create a version of asn1.io that can run on GitHub pages so you can compile and test ASN.1 without the ridiculous restrictions and licences OSS put on you.