ystero-dev / hampi

Rust ASN.1 Toolkit
Other
43 stars 18 forks source link

Alignment error parsing a NGAP (APER) NG Setup Request #117

Closed yutotakano closed 2 months ago

yutotakano commented 2 months ago

Hi! Thank you for the Hampi crate, it's very useful to have a Rust-based ASN.1 compiler as an alternative to asn1c and FFI :)

I wanted to ask if I have just found a bug in the parsing of a NGAP NGSetupRequest message (which is APER-encoded):

    let buf: [u8; 57] = [
        0x00, 0x15, 0x00, 0x35, 0x00, 0x00, 0x04, 0x00, 0x1b, 0x00, 0x08, 0x00, 0x02, 0xf8, 0x39,
        0x03, 0x80, 0x00, 0x04, 0x00, 0x52, 0x40, 0x09, 0x03, 0x00, 0x4e, 0x65, 0x72, 0x76, 0x69,
        0x6f, 0x6e, 0x00, 0x66, 0x00, 0x10, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x02, 0xf8, 0x39,
        0x00, 0x00, 0x10, 0x08, 0x00, 0x00, 0x01, 0x00, 0x15, 0x40, 0x01, 0x40,
    ];

    let mut codec_data = PerCodecData::from_slice_aper(&buf);
    let ngap_pdu = ngap::NGAP_PDU::aper_decode(&mut codec_data).expect("Error decoding NGAP PDU");
    println!("Decoded NGAP PaDU: {:?}", ngap_pdu);

This results in the following error, complaining that the first byte of the gNB ID field (byte 15, first byte on second line) should not be 0x03.

Error { cause: InvalidAlignment, msg: "3 Padding bits at Offset 125 not all '0'.", context: [] }

In fact, changing that byte to 0x00 will make it parse successfully (although I have not verified the contents), but is not what we want since the byte sequence is supposed to be valid to begin with. The byte sequence is taken from a valid 5G NGAP trace that e.g. Wireshark and Open5GS will parse fine, attached.

The gNB ID field is defined in ASN.1 as BIT STRING (SIZE(22..32)). I believe the 0x03 is the length of the gNB ID that follows it (0x80 0x00 0x04, which is what Wireshark displays).

Still not very sure if this is a bug, but if it is, could it be fixed? Otherwise, how should I work around it?

pcap attachment

gabhijit commented 2 months ago

Hi, Thanks for reporting this. Is it possible to enable log level trace or debug and submit the output here? That should help in identifying the issue.

gabhijit commented 2 months ago

2024-05-06T05:35:38Z TRACE trybuild000::ngap] decode: GNB_ID
[2024-05-06T05:35:38Z TRACE asn1_codecs::per::aper::decode] decode_choice_idx: lb: 0, ub: 1, extensible: false
[2024-05-06T05:35:38Z TRACE asn1_codecs::per::common::decode::decode_internal] decode_constrained_whole_number: lb: 0, ub: 1
[2024-05-06T05:35:38Z TRACE asn1_codecs::per] Decoding Bits as Integer. offset: 120, bits: 1
[2024-05-06T05:35:38Z TRACE asn1_codecs::per] Decoded Value: 0
[2024-05-06T05:35:38Z TRACE asn1_codecs::per] PerCodecData: offset: 121
[2024-05-06T05:35:38Z TRACE asn1_codecs::per] PerCodecData: offset: 121
[2024-05-06T05:35:38Z TRACE trybuild000::ngap] decode: GNB_ID_gNB_ID
[2024-05-06T05:35:38Z TRACE asn1_codecs::per::aper::decode] decode_bitstring: lb: Some(22), ub: Some(32), is_extensible: false
[2024-05-06T05:35:38Z TRACE asn1_codecs::per::common::decode::decode_internal] decode_constrained_length_determinent, lb: 22, ub: 32
[2024-05-06T05:35:38Z TRACE asn1_codecs::per::common::decode::decode_internal] decode_constrained_whole_number: lb: 22, ub: 32
[2024-05-06T05:35:38Z TRACE asn1_codecs::per] Decoding Bits as Integer. offset: 121, bits: 4
[2024-05-06T05:35:38Z TRACE asn1_codecs::per] Decoded Value: 0
[2024-05-06T05:35:38Z TRACE asn1_codecs::per::common::decode::decode_internal] decoded length : 22
[2024-05-06T05:35:38Z TRACE asn1_codecs::per] PerCodecData: offset: 125
[2024-05-06T05:35:38Z TRACE asn1_codecs::per] Aligning Codec Buffer with 3 bits

Here is the log output. Indeed as you say there is an error here. Likely a bug in bitstring_decode will take a look.

gabhijit commented 2 months ago

I believe I have found out what's the cause - here is the Decoded output - Can you please confirm - whether this is right?

Decoded NGAP PDU: InitiatingMessage(
    InitiatingMessage {
        procedure_code: ProcedureCode(
            21,
        ),
        criticality: Criticality(
            0,
        ),
        value: Id_NGSetup(
            NGSetupRequest {
                protocol_i_es: NGSetupRequestProtocolIEs(
                    [
                        NGSetupRequestProtocolIEs_Entry {
                            id: ProtocolIE_ID(
                                27,
                            ),
                            criticality: Criticality(
                                0,
                            ),
                            value: Id_GlobalRANNodeID(
                                GlobalGNB_ID(
                                    GlobalGNB_ID {
                                        plmn_identity: PLMNIdentity(
                                            [
                                                2,
                                                248,
                                                57,
                                            ],
                                        ),
                                        gnb_id: GNB_ID(
                                            GNB_ID_gNB_ID(
                                                BitVec<u8, bitvec::order::Msb0> {
                                                    addr: 0x00006017286421a0,
                                                    head: 000,
                                                    bits: 22,
                                                    capacity: 64,
                                                } [
                                                    1,
                                                    0,
                                                    0,
                                                    0,
                                                    0,
                                                    0,
                                                    0,
                                                    0,
                                                    0,
                                                    0,
                                                    0,
                                                    0,
                                                    0,
                                                    0,
                                                    0,
                                                    0,
                                                    0,
                                                    0,
                                                    0,
                                                    0,
                                                    0,
                                                    1,
                                                ],
                                            ),
                                        ),
                                        ie_extensions: None,
                                    },
                                ),
                            ),
                        },
                        NGSetupRequestProtocolIEs_Entry {
                            id: ProtocolIE_ID(
                                82,
                            ),
                            criticality: Criticality(
                                1,
                            ),
                            value: Id_RANNodeName(
                                RANNodeName(
                                    "Nervion",
                                ),
                            ),
                        },
                        NGSetupRequestProtocolIEs_Entry {
                            id: ProtocolIE_ID(
                                102,
                            ),
                            criticality: Criticality(
                                0,
                            ),
                            value: Id_SupportedTAList(
                                SupportedTAList(
                                    [
                                        SupportedTAItem {
                                            tac: TAC(
                                                [
                                                    0,
                                                    0,
                                                    1,
                                                ],
                                            ),
                                            broadcast_plmn_list: BroadcastPLMNList(
                                                [
                                                    BroadcastPLMNItem {
                                                        plmn_identity: PLMNIdentity(
                                                            [
                                                                2,
                                                                248,
                                                                57,
                                                            ],
                                                        ),
                                                        tai_slice_support_list: SliceSupportList(
                                                            [
                                                                SliceSupportItem {
                                                                    s_nssai: S_NSSAI {
                                                                        sst: SST(
                                                                            [
                                                                                1,
                                                                            ],
                                                                        ),
                                                                        sd: Some(
                                                                            SD(
                                                                                [
                                                                                    0,
                                                                                    0,
                                                                                    1,
                                                                                ],
                                                                            ),
                                                                        ),
                                                                        ie_extensions: None,
                                                                    },
                                                                    ie_extensions: None,
                                                                },
                                                            ],
                                                        ),
                                                        ie_extensions: None,
                                                    },
                                                ],
                                            ),
                                            ie_extensions: None,
                                        },
                                    ],
                                ),
                            ),
                        },
                        NGSetupRequestProtocolIEs_Entry {
                            id: ProtocolIE_ID(
                                21,
                            ),
                            criticality: Criticality(
                                1,
                            ),
                            value: Id_DefaultPagingDRX(
                                PagingDRX(
                                    2,
                                ),
                            ),
                        },
                    ],
                ),
            },
        ),
    },
)

The problem is as follows - At the offset 125 as per the standards, the alignment bits are to be all zero. In this particular case the alignment bits are not zero. They are 011 I believe. Thus it is a hard error in the codec, but I believe a good idea may be to just have a warn! and let the decoding continue.

If this output makes sense, I will push a fix for this.

yutotakano commented 2 months ago

Yes, that's a correct parse!

After reading your analysis, I think our traffic generator was the buggy one for generating the wrong alignment bits then. And I guess software like Wireshark or asn1c (which Open5GS uses for their parser) just silently ignore and continue...

Well, this is interesting haha, it's certainly up to you whether the "fix" should go in since it's out-of-standard behaviour, I will also report this issue to the traffic generator side. But thanks for looking into it!

gabhijit commented 2 months ago

I think it's a good idea to be lenient while accepting incoming messages. It's not unlikely that some traffic generators (and/or actual code) send and accept such erroneous messages. I believe it's a good idea to log warning and accept it. Unless experience shows doing otherwise is a way better idea.

gabhijit commented 2 months ago

Fix for this was merged into master. You may want to test with latest master. Will release this sometime later.

yutotakano commented 2 months ago

Works well, thank you very much!