mouse07410 / asn1c

The ASN.1 Compiler
http://lionet.info/asn1c/
BSD 2-Clause "Simplified" License
101 stars 73 forks source link

Wrong encoding of SBc-AP Write-Replace Warning Request (APER enc) #94

Closed pespin closed 2 years ago

pespin commented 2 years ago

Hi,

I'm introducing SBc-AP protocol support [1] to osmo-cbc [2] project. For that, I'm using code generated with asn1c commit 2cf625cf89beedbb8bd795727057dad3cd3ea149 (mouse07410 branch "vlm_master" including last fix of mine merged yesterday/today).

I'm validating the generated output using several methods:

I'm adding the SBc-AP support in osmo-cbc.git branch "pespin/sbcap".

First commit adds the skeletons + generated asn1c code (from asn files found in same repo in src/sbcap/asn1/*.asn) with mouse07410/vlm_master plus all the boilerplate to build the content in a library (the Makefile also modified slightly some headers to avoid some circular dependency issues, but that's another topic): https://cgit.osmocom.org/osmo-cbc/commit/?h=pespin/sbcap&id=469ab793be1d1fe4924a971ad0b6fb84a2ce8d20 asn1c code can be regenerated by calling "make -C src/ regen".

Second commit adds a unit test to encode SBc-AP Write-Replace Warning Request (you can run it with "make check"): https://cgit.osmocom.org/osmo-cbc/commit/?h=pespin/sbcap&id=8ff4fed49b63d184521aebb3cb3730651c9e49af It can be seen that the encoded output is: 00 00 00 0e 00 02 00 05 00 02 ab 01 00 0b 00 02 ab cd When decoding with wireshark, libfftranscode or asn1.io, there seems to be issues parsing the length of the item list (the IE list). When comparing to the message in the existing implementation (available above), it also doesn't match, it starts with 00 00 00 5f 00 00 0b, where of course values 5f and 0b are different since those contain the length of the message and the 11 IEs it contains. However, you can see how in between those there's an extra 00 byte.

In third commit, I apply the proposed changes in my asn1c PR [5]: https://cgit.osmocom.org/osmo-cbc/commit/?h=pespin/sbcap&id=2c54d1d135018ad57de70a7be47bc701b0a4970f You can see how the output of the unit test changes to something correct:

-Encoded message: 00 00 00 0e 00 02 00 05 00 02 ab 01 00 0b 00 02 ab cd 
+Encoded message: 00 00 00 0f 00 00 02 00 05 00 02 ab 01 00 0b 00 02 ab cd 

Once this change is applied, wireshark, fftranscode and as1n.io are happy with the message.

[1] 3GPP TS 29.168, https://www.etsi.org/deliver/etsi_ts/129100_129199/129168/16.00.00_60/ts_129168v160000p.pdf [2] https://osmocom.org/projects/osmo-cbc/wiki [3] https://gitea.osmocom.org/ttcn3/osmo-ttcn3-hacks/src/branch/pespin/sbcap [4] https://osmocom.org/projects/cellular-infrastructure/wiki/Titan_TTCN3_Testsuites#Proprietary-APERBER-transcoding-library-for-Iu-tests [5] https://github.com/mouse07410/asn1c/pull/93

pespin commented 2 years ago

I attach output of "sbcap_test" unit test BEFORE applying the asn1c changes from [5], compiled with -DASN_EMIT_DEBUG=1.

sbcap_test_before_change.txt

mouse07410 commented 2 years ago

Ok, do people interested in #91 and #93 agree that A. The current code violates X.691 INTEGER length encoding? B. The fix proposed in #93 restores conformance?

pespin commented 2 years ago

@mouse07410 the problem is specifically seen when encoding length of sequence (list of fields), which as it was pointed out, specified by X.691 11.9 (length determinant):

NOTE 1 ... list of fields (with the length count in components of a sequence-of or set-of).

In general in section 11.9, my understanding is that it usually talks about an upper bound "ub" that is less than 64K, where 64K=65536.

Hence, it usually talks about (ub < 65536 && lb >= 0). So, the maximum ub is 65536-1=65535. However, the code in aper_put_length() uses range, which is range=ub-lb+1. So, the maximum range to check for is 65536.

Hence, (range <= 65536 && range >= 0) is the counterpart of (ub < 65536 && lb >= 0) (note "<=" vs "<"). I'm new to reading ASN1 specs though, so please feel free to provide your own thoughts.

laf0rge commented 2 years ago

Thanks @pespin, this explains a lot. Indeed, it is the case that skeletons/constr_SEQUENCE_OF_aper.c and skeletons/constr_SET_OF_aper.c are using calls like aper_put_length(po, ct->upper_bound - ct->lower_bound + 1, list->count - ct->lower_bound, 0) where it computes a range (ub-lb+1).

In my opinion, to avoid further misunderstnanding, I would suggest to

This makes IMHO most sense as the chapter 11.9 sections realted to APER indeed never talk about range. Only 11.9.4 for UPER talks about range.

laf0rge commented 2 years ago

Ok, do people interested in #91 and #93 agree that A. The current code violates X.691 INTEGER length encoding? B. The fix proposed in #93 restores conformance?

To clarify/summarize the latest understanding: INTEGER indeed seems fine, but constrained length fields (such as those used for SEQUENCE OF or SET OF where there is a limited number of elements permitted) is wrong. This is caused by the confusing use of different terms than the spec (range instead of n) together with the constants for the term used in the spec (n)

mouse07410 commented 2 years ago

Thank you! So, does #93 need more work to accommodate suggestions from https://github.com/mouse07410/asn1c/issues/94#issuecomment-1179224305?

While it's possible to cherry-pick individual commits, I'd prefer to deal with the entire PR (time factors).

mouse07410 commented 2 years ago

@pespin and @laf0rge do you confirm that #93 can be merged as-is?

pespin commented 2 years ago

@mouse07410 IMHO it can already be merged, since it clearly shows some cases are fixed. Later on code can be improved to adapt to terminology used in the specs, but that'd be a separate commit/topic.

Other people having problems when these new patches are merged should provide more detailed bug reports on what is exactly failing on their side.

mouse07410 commented 2 years ago

@grzegorzniemirowski if you care about this issue, and have (detailed) bug reports (and, maybe, an X.691-compliant recommendation how to fix it), please comment here.

grzegorzniemirowski commented 2 years ago

@mouse07410 Detailed bug report has already been given in #62 The issue is that length is encoded in one byte but asn1c expects two bytes. Additionaly I can attach PCAP and test code.

#include <sys/types.h>
#include <stdio.h>
#include "asn_application.h"
#include "asn_internal.h"
#include "F1AP-PDU.h"

uint8_t buf[] = {
  0x00, 0x00, 0x00, 0x1c, 0x00, 0x00, 0x03, 0x00,
  0x4e, 0x00, 0x02, 0x00, 0x00, 0x00, 0x00, 0x40,
  0x01, 0x00, 0x00, 0x30, 0x00, 0x0a, 0x40, 0x01,
  0x00, 0x50, 0x00, 0x04, 0x60, 0x20, 0x00, 0x01
};

int main() {
    F1AP_PDU_t* pdu = NULL;
    int len = sizeof(buf);
    asn_dec_rval_t ret = aper_decode_complete(NULL, &asn_DEF_F1AP_PDU, &pdu, buf, len);
    printf("%ld %d\n", ret.consumed, ret.code);
    return 0;
}

F1AP_Reset.zip

pespin commented 2 years ago

hi @grzegorzniemirowski , can you provide information on how was that binary stream encoded? which implementation generated that binary stream? asn1c itself? which version?

grzegorzniemirowski commented 2 years ago

@pespin it was sent by Accelleran gNB-CU

pespin commented 2 years ago

I tried decoding it with https://asn1.io/asn1playground/default.aspx?

I had to pass "buf" hexstring values above removing the comas.

Console  Output

OSS ASN-1Step Version 10.2.1
Copyright (C) 2022 OSS Nokalva, Inc.  All rights reserved.
This product is licensed for use by "OSS Nokalva, Inc."

C0043I: 0 error messages, 0 warning messages and 0 informatory messages issued.

ASN1STEP: Decoding PDU #1 :

rec1value F1AP-PDU ::= 
  --TYPE INFORMATION: CHOICE
  --OFFSET: 0,0
  --choice index: <.11> (index = 3)
  choice-extension :
  {
    --TYPE INFORMATION: SEQUENCE
    --OFFSET: 0,2
    id 7476,
      --TYPE INFORMATION: INTEGER (0..65535)
      --OFFSET: 0,2; LENGTH: 2,6
      --padding: <010011>
      --contents: .1D.34
    criticality <value not decoded>,
      --TYPE INFORMATION: ENUMERATED {reject,ignore,notify}
      --OFFSET: 3,0; LENGTH: 0,2
      --contents: <.11>
D0071S: Value not among the ENUMERATED: 3; check field 'criticality' of field 'choice-extension' of PDU #1.
value: D0071S: Value not among the ENUMERATED: 3; check field 'criticality' of field 'choice-extension' of PDU #1.
S0014E: Printing of PER details failed with the return code '17'.

(....)
The console diagnostics might be truncated. To avoid this, please Sign In and have a valid license.
Results
Operation failed. See Console Output. 

So at first glance it looks like the initial encoding is wrong?

grzegorzniemirowski commented 2 years ago

It looks like your hex string was wrong. Here is PER encoded binary file to upload to the playground: F1AP_Reset_PER.zip. Please note that Wireshark has no problem with decoding as well.

OSS ASN-1Step Version 10.2.1
Copyright (C) 2022 OSS Nokalva, Inc.  All rights reserved.
This product is licensed for use by "OSS Nokalva, Inc."

C0043I: 0 error messages, 0 warning messages and 0 informatory messages issued.

ASN1STEP: Decoding PDU #1 :

rec1value F1AP-PDU ::= 
  --TYPE INFORMATION: CHOICE
  --OFFSET: 0,0
  --choice index: <.00> (index = 0)
  initiatingMessage :
  {
    --TYPE INFORMATION: SEQUENCE
    --OFFSET: 0,2
    procedureCode 0,
      --TYPE INFORMATION: INTEGER (0..255)
      --OFFSET: 0,2; LENGTH: 1,6
      --padding: <000000>
      --contents: .00
    criticality reject,
      --TYPE INFORMATION: ENUMERATED {reject,ignore,notify}
      --OFFSET: 2,0; LENGTH: 0,2
      --contents: <.00>
    --padding: <000000>
    --open type length: .1C (decoded as 28)
    value Reset:
    {
      --TYPE INFORMATION: OpenType - identified as SEQUENCE
      --OFFSET: 4,0
      --extension flag: <.0>
      protocolIEs
      {
        --TYPE INFORMATION: SEQUENCE (SIZE(0..65535)) OF
        --OFFSET: 4,1
        --padding: <0000000>
        --length: .00.03 (decoded as 3)
        {
          --TYPE INFORMATION: SEQUENCE
          --
(....)
The console diagnostics might be truncated. To avoid this, please Sign In and have a valid license.

You can also try different decoders. For example https://www.marben-products.com/decoder-asn1-nr/ Decoding attached PER encoded file produces following output:

<F1AP-PDU>
  <initiatingMessage>
    <procedureCode>0</procedureCode>
    <criticality>
      <reject/>
    </criticality>
    <value>
      <Reset>
        <protocolIEs>
          <SEQUENCE>
            <id>78</id>
            <criticality>
              <reject/>
            </criticality>
            <value>
              <TransactionID>0</TransactionID>
            </value>
          </SEQUENCE>
          <SEQUENCE>
            <id>0</id>
            <criticality>
              <ignore/>
            </criticality>
            <value>
              <Cause>
                <radioNetwork>
                  <unspecified/>
                </radioNetwork>
              </Cause>
            </value>
          </SEQUENCE>
          <SEQUENCE>
            <id>48</id>
            <criticality>
              <reject/>
            </criticality>
            <value>
              <ResetType>
                <partOfF1-Interface>
                  <SEQUENCE>
                    <id>80</id>
                    <criticality>
                      <reject/>
                    </criticality>
                    <value>
                      <UE-associatedLogicalF1-ConnectionItem>
                        <gNB-CU-UE-F1AP-ID>32</gNB-CU-UE-F1AP-ID>
                        <gNB-DU-UE-F1AP-ID>1</gNB-DU-UE-F1AP-ID>
                      </UE-associatedLogicalF1-ConnectionItem>
                    </value>
                  </SEQUENCE>
                </partOfF1-Interface>
              </ResetType>
            </value>
          </SEQUENCE>
        </protocolIEs>
      </Reset>
    </value>
  </initiatingMessage>
</F1AP-PDU>

32 bytes supplied, 32 bytes decoded.
*** DECODING SUCCESSFUL ***

And another one: https://ridenext.co.in/asndecoder.html It needs hex input in following form: 0000001c000003004e0002000000004001000030000a40010050000460200001 And the output is:

{

    "id-Reset": [
        {
            "initiatingMessage": [
                {
                    "procedureCode": 0
                },
                {
                    "criticality": 0
                },
                {
                    "value": [
                        {
                            "protocolIEs": [
                                {
                                    "id-TransactionID": 0
                                },
                                {
                                    "id-Cause": [
                                        {
                                            "radioNetwork": 0
                                        }
                                    ]
                                },
                                {
                                    "id-ResetType": [
                                        {
                                            "partOfF1_Interface": [
                                                [
                                                    {
                                                        "id": 80
                                                    },
                                                    {
                                                        "criticality": 0
                                                    },
                                                    {
                                                        "value": [
                                                            {
                                                                "id-UE-associatedLogicalF1-ConnectionItem": [
                                                                    {
                                                                        "gNB-CU-UE-F1AP-ID": 32
                                                                    },
                                                                    {
                                                                        "gNB-DU-UE-F1AP-ID": 1
                                                                    }
                                                                ]
                                                            }
                                                        ]
                                                    }
                                                ]
                                            ]
                                        }
                                    ]
                                }
                            ]
                        }
                    ]
                }
            ]
        }
    ]

}

Summing up: Wireshark and three ASN1 decoders have no problem with this message.

mouse07410 commented 2 years ago

@grzegorzniemirowski do #96 and #97 help the problem?

I don't have ASN.1 for F1AP, so am unlikely to reproduce.

laf0rge commented 2 years ago

As there's no OCTET STRING in the example, I don't think #96 will have any impact. #97 might.

grzegorzniemirowski commented 2 years ago

Indeed. They are not related to SEQUENCE length and they don't help. F1AP ASN: F1AP-16.7.0.zip

mouse07410 commented 2 years ago

I would suggest to

  • define the aper_put_length() to use n (number of elements in the sequence/set) as argument, not range
  • adjust the callers to pass n and not range. There are only two callers passing a non-negative number as second argument today: the two functions I mentioned above.

Update

This sounds reasonable, and at least worth trying. Do I understand correctly that the way length is encoded in APER should be different for, e.g., INTEGER and SEQUENCE OF? Should the fix apply only to skeletons/constr_SEQUENCE_OF_aper.c (and SET_OF), or also involve skeletons/aper_support.c?

I don't have X.691 - what does it say exactly about the length encoding?

What's the semantics of "range" for constraints on SEQUENCE OF?

Anybody cares to submit a PR implementing the above? To avoid messing with what's already working, I'd probably add another function, like aper_put_seq_length() with the parameters as described above.

Comments? PRs?

grzegorzniemirowski commented 2 years ago

What was wrong with #91 actually? What was broken by this PR?

mouse07410 commented 2 years ago

@grzegorzniemirowski @laf0rge do you think the line https://github.com/mouse07410/asn1c/blob/30219de2d3da888b4f1eea0dd79f2a505000401a/skeletons/constr_SEQUENCE_OF_aper.c#L57 should be

} else if (aper_put_length(po, ct->upper_bound - ct->lower_bound + (ct->lower_bound>0)?1:0, list->count - ct->lower_bound, 0) < 0)

and in aper_support.c change<= 65536 to < 65536? That's how I'd interpret X.691 §10.9.

Update

Also, looking at check-APER-support.c, I'm not certain it's correct, matching range against 65536, because it seems to me that X.691 X.691 §10.9 states less than 65536. Comments?

mouse07410 commented 2 years ago

F1AP ASN: F1AP-16.7.0.zip

Unfortunately, this ASN.1 file does not produce directly-usable (aka, compile-able) code:

.  .  .
clang -O3 -std=gnu18 -march=native -isysroot /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk  -DASN_PDU_COLLECTION -I. -o F1AP-PDU.o -c F1AP-PDU.c
In file included from F1AP-PDU.c:8:
In file included from ./F1AP-PDU.h:58:
In file included from ./InitiatingMessage.h:19:
In file included from ./Reset.h:15:
In file included from ./ProtocolIE-Container.h:1089:
In file included from ./ProtocolIE-Field.h:22:
In file included from ./UE-associatedLogicalF1-ConnectionItem.h:48:
In file included from ./ProtocolExtensionContainer.h:3663:
In file included from ./ProtocolExtensionField.h:25:
In file included from ./NPNBroadcastInformation.h:57:
./ProtocolIE-SingleContainer.h:22:9: error: unknown type name 'F1AP_PDU_ExtIEs_t'
typedef F1AP_PDU_ExtIEs_t        ProtocolIE_SingleContainer_10757P0_t;
        ^
./ProtocolIE-SingleContainer.h:23:9: error: unknown type name 'ResetType_ExtIEs_t'
typedef ResetType_ExtIEs_t       ProtocolIE_SingleContainer_10757P1_t;
        ^
./ProtocolIE-SingleContainer.h:24:9: error: unknown type name 'UE_associatedLogicalF1_ConnectionItemRes_t'; did you mean 'UE_associatedLogicalF1_ConnectionItem_t'?
typedef UE_associatedLogicalF1_ConnectionItemRes_t       ProtocolIE_SingleContainer_10757P2_t;
        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
        UE_associatedLogicalF1_ConnectionItem_t
./UE-associatedLogicalF1-ConnectionItem.h:38:3: note: 'UE_associatedLogicalF1_ConnectionItem_t' declared here
} UE_associatedLogicalF1_ConnectionItem_t;
  ^
In file included from F1AP-PDU.c:8:
In file included from ./F1AP-PDU.h:58:
In file included from ./InitiatingMessage.h:19:
In file included from ./Reset.h:15:
In file included from ./ProtocolIE-Container.h:1089:
In file included from ./ProtocolIE-Field.h:22:
In file included from ./UE-associatedLogicalF1-ConnectionItem.h:48:
In file included from ./ProtocolExtensionContainer.h:3663:
In file included from ./ProtocolExtensionField.h:25:
In file included from ./NPNBroadcastInformation.h:57:
./ProtocolIE-SingleContainer.h:25:9: error: unknown type name 'UE_associatedLogicalF1_ConnectionItemResAck_t'; did you mean 'UE_associatedLogicalF1_ConnectionItem_t'?
typedef UE_associatedLogicalF1_ConnectionItemResAck_t    ProtocolIE_SingleContainer_10757P3_t;
        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
        UE_associatedLogicalF1_ConnectionItem_t
./UE-associatedLogicalF1-ConnectionItem.h:38:3: note: 'UE_associatedLogicalF1_ConnectionItem_t' declared here
} UE_associatedLogicalF1_ConnectionItem_t;
  ^
In file included from F1AP-PDU.c:8:
In file included from ./F1AP-PDU.h:58:
In file included from ./InitiatingMessage.h:19:
In file included from ./Reset.h:15:
In file included from ./ProtocolIE-Container.h:1089:
In file included from ./ProtocolIE-Field.h:22:
In file included from ./UE-associatedLogicalF1-ConnectionItem.h:48:
In file included from ./ProtocolExtensionContainer.h:3663:
In file included from ./ProtocolExtensionField.h:25:
In file included from ./NPNBroadcastInformation.h:57:
./ProtocolIE-SingleContainer.h:26:9: error: unknown type name 'GNB_DU_Served_Cells_ItemIEs_t'
typedef GNB_DU_Served_Cells_ItemIEs_t    ProtocolIE_SingleContainer_10757P4_t;
        ^
./ProtocolIE-SingleContainer.h:27:9: error: unknown type name 'Cells_to_be_Activated_List_ItemIEs_t'
typedef Cells_to_be_Activated_List_ItemIEs_t     ProtocolIE_SingleContainer_10757P5_t;
        ^
./ProtocolIE-SingleContainer.h:28:9: error: unknown type name 'Served_Cells_To_Add_ItemIEs_t'
typedef Served_Cells_To_Add_ItemIEs_t    ProtocolIE_SingleContainer_10757P6_t;
        ^
./ProtocolIE-SingleContainer.h:29:9: error: unknown type name 'Served_Cells_To_Modify_ItemIEs_t'
typedef Served_Cells_To_Modify_ItemIEs_t         ProtocolIE_SingleContainer_10757P7_t;
        ^
./ProtocolIE-SingleContainer.h:30:9: error: unknown type name 'Served_Cells_To_Delete_ItemIEs_t'
typedef Served_Cells_To_Delete_ItemIEs_t         ProtocolIE_SingleContainer_10757P8_t;
        ^
./ProtocolIE-SingleContainer.h:31:9: error: unknown type name 'Cells_Status_ItemIEs_t'
typedef Cells_Status_ItemIEs_t   ProtocolIE_SingleContainer_10757P9_t;
        ^
./ProtocolIE-SingleContainer.h:32:9: error: unknown type name 'Dedicated_SIDelivery_NeededUE_ItemIEs_t'
typedef Dedicated_SIDelivery_NeededUE_ItemIEs_t  ProtocolIE_SingleContainer_10757P10_t;
        ^
./ProtocolIE-SingleContainer.h:33:9: error: unknown type name 'GNB_DU_TNL_Association_To_Remove_ItemIEs_t'
typedef GNB_DU_TNL_Association_To_Remove_ItemIEs_t       ProtocolIE_SingleContainer_10757P11_t;
        ^
./ProtocolIE-SingleContainer.h:34:9: error: unknown type name 'Cells_to_be_Deactivated_List_ItemIEs_t'
typedef Cells_to_be_Deactivated_List_ItemIEs_t   ProtocolIE_SingleContainer_10757P12_t;
        ^
./ProtocolIE-SingleContainer.h:35:9: error: unknown type name 'GNB_CU_TNL_Association_To_Add_ItemIEs_t'
typedef GNB_CU_TNL_Association_To_Add_ItemIEs_t  ProtocolIE_SingleContainer_10757P13_t;
        ^
./ProtocolIE-SingleContainer.h:36:9: error: unknown type name 'GNB_CU_TNL_Association_To_Remove_ItemIEs_t'
typedef GNB_CU_TNL_Association_To_Remove_ItemIEs_t       ProtocolIE_SingleContainer_10757P14_t;
        ^
./ProtocolIE-SingleContainer.h:37:9: error: unknown type name 'GNB_CU_TNL_Association_To_Update_ItemIEs_t'
typedef GNB_CU_TNL_Association_To_Update_ItemIEs_t       ProtocolIE_SingleContainer_10757P15_t;
        ^
./ProtocolIE-SingleContainer.h:38:9: error: unknown type name 'Cells_to_be_Barred_ItemIEs_t'
typedef Cells_to_be_Barred_ItemIEs_t     ProtocolIE_SingleContainer_10757P16_t;
        ^
./ProtocolIE-SingleContainer.h:39:9: error: unknown type name 'Protected_EUTRA_Resources_ItemIEs_t'
typedef Protected_EUTRA_Resources_ItemIEs_t      ProtocolIE_SingleContainer_10757P17_t;
        ^
./ProtocolIE-SingleContainer.h:40:9: error: unknown type name 'Neighbour_Cell_Information_ItemIEs_t'
typedef Neighbour_Cell_Information_ItemIEs_t     ProtocolIE_SingleContainer_10757P18_t;
        ^
fatal error: too many errors emitted, stopping now [-ferror-limit=]
20 errors generated.
make: *** [Makefile:23: F1AP-PDU.o] Error 1

Unfortunately, I don't have time to "massage" include files order and whatever else needed to produce a working codec library. So, as I said, I cannot experiment with F1AP myself, and will have to constrain myself to attempts to ensure that asn1c code conforms to X.691.

grzegorzniemirowski commented 2 years ago

The code is produced by asn1c so it would be asn1c issue, not ASN file issue. It's official ASN for F1AP and I have been using it for months with asn1c. I guess you had some parameters wrong because I have no such problems:

$ asn1c -pdu=F1AP-PDU -fno-include-deps -findirect-choice -fcompound-names -flink-skeletons -gen-APER -no-gen-JER -gen-OER -gen-UPER F1AP-16.7.0.asn
$ clang -O3 -std=gnu18 -DASN_PDU_COLLECTION -I. -o F1AP-PDU.o -c F1AP-PDU.c
$ file F1AP-PDU.o
F1AP-PDU.o: ELF 64-bit LSB relocatable, x86-64, version 1 (SYSV), not stripped
mouse07410 commented 2 years ago

I guess you had some parameters wrong . . .

Obviously so.

$ clang -O3 -std=gnu18 -DASN_PDU_COLLECTION -I. -o F1AP-PDU.o -c F1AP-PDU.c
$ file F1AP-PDU.o
F1AP-PDU.o: ELF 64-bit LSB relocatable, x86-64, version 1 (SYSV), not stripped

Look, I don't deal with and don't care for Open5G, and am not going to become an expert in it. My interest is strictly limited to improving asn1c. So, whatever you can do to help me reproducing this problem, could potentially improve the chances of getting it fixed. On my own, I don't have time to fool around 5G encoding.

What do I do with F1AP-PDU.o? What I need is converter-example working, so I can point it at .xer file and generate .aper, and vs. versa. Normally, I generate converter-example by

$ make all -f converter-example.mk

Update

Trying to build the converter:

$ make all -f converter-example.mk
.  .  .
clang -O3 -std=gnu18 -march=native -isysroot /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk  -DPDU=F1AP_PDU -I. -o ProtocolExtensionField.o -c ProtocolExtensionField.c
ProtocolExtensionField.c:66989:5: error: use of undeclared identifier 'asn_OER_memb_OCTET_STRING_SIZE_3__constr_96'; did you mean 'memb_OCTET_STRING_SIZE_3__constraint_924'?
                        &asn_OER_memb_OCTET_STRING_SIZE_3__constr_96,
                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
                         memb_OCTET_STRING_SIZE_3__constraint_924
ProtocolExtensionField.c:17440:1: note: 'memb_OCTET_STRING_SIZE_3__constraint_924' declared here
memb_OCTET_STRING_SIZE_3__constraint_924(const asn_TYPE_descriptor_t *td, const void *sptr,
^
ProtocolExtensionField.c:66992:5: error: use of undeclared identifier 'asn_PER_memb_OCTET_STRING_SIZE_3__constr_96'; did you mean 'memb_OCTET_STRING_SIZE_3__constraint_924'?
                        &asn_PER_memb_OCTET_STRING_SIZE_3__constr_96,
                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
                         memb_OCTET_STRING_SIZE_3__constraint_924
.  .  .
2 warnings and 2 errors generated.
make: *** [ProtocolExtensionField.o] Error 1
mouse07410 commented 2 years ago

@pespin I'd like you to explain why check-APER-support.c validates round-trip against 65536, rather than, e.g., 65535.

Also, I'd like to hear an answer to @grzegorzniemirowski 's question "what breaks when aper_support.c conforms closer to X.691 and uses strict < 65536 instead of the current <= 65536?"

pespin commented 2 years ago

@mouse07410 I used check_round_trip(65536, ...) since that was the faulty case (small value with range=65536) that I discovered when using the code generated from SBc-AP asn files (3GPP TS 29.168). Basically the case is an ASN1 sequence type which had a length determinant of range=65536 (0..65535), uint16

I saw the range=65536 in the debug output (-DASN_EMIT_DEBUG=1) when encoding the structure, and it saw it was the part which was encoding the unexpected missing byte. So I basically ported that to a unit test to be able to reproduce it easily, fix it and avoid future regressions.

I guess the best to figure out what's wrong with the decoding from @grzegorzniemirowski would be to use the hexstring he provided, and add a unit test decoding it and enable ASN_DEBUG, so that we can see exactly where it fails and we can reproduce easily. Then once fixed we also make sure it doesn't break since we have a unit test.

pespin commented 2 years ago

Actually, the best would be that @grzegorzniemirowski provides the debug output (-DASN_EMIT_DEBUG=1) since he's the once having the support structures from the asn file to decode. From the debug output we can perhaps find out the exact point at which it fails, and try to write a generic unit test which triggers the issue, so that there's no need to have lots of files in asn1c.git

pespin commented 2 years ago

Also, looking at check-APER-support.c, I'm not certain it's correct, matching range against 65536, because it seems to me that X.691 X.691 §10.9 states less than 65536. Comments?

I'm looking at https://www.itu.int/rec/dologin_pub.asp?lang=e&id=T-REC-X.691-202102-I!!PDF-E&type=items in there it seems to be section 11.9, not 10.9. In any case, it was already explained here: https://github.com/mouse07410/asn1c/issues/94#issuecomment-1179056586 In summary, the spec talks about "an upper bound that is less than 65536". Which for range=ub-lb+1, lb=0 and maximum possible ub<65536 (0..65535, uint16) it means range<=65536, which is what my patch fixes.

pespin commented 2 years ago

@grzegorzniemirowski also if possible include a pcap file containing the packet being decoded with ASN debug enabled, so that the hexstring can be identified easily with the protocol fields.

grzegorzniemirowski commented 2 years ago

@mouse07410 I don't care about Open5G too. I care about 3GPP ASN which should be supported as any other ASN. Or at least it would be very nice if it was. I have spent many hours on this issue so I'm not a person who just ask others to do the work. I'm not even the submitter of #62 but a contributor of PR which in my opinion was fixing the problem.

I don't need F1AP-PDU.o itself, I was just refering to your example of failing compilation and provided correct parameters to help you with code generation.

The converter building error is just another asn1c bug and it was mentioned earlier in #77 (see raoufkh's comment at the end). You need to manually edit generated code using compiler's hint: replace asn_PER_memb_OCTET_STRING_SIZE_3__constr_96 with memb_OCTET_STRING_SIZE_3__constraint_924. Then converter-example would be compiled correctly.

grzegorzniemirowski commented 2 years ago

@pespin You can find the PCAP in one of my previous comments. The important part of debug output was in original issue #62. I'm pasting it again in full length. It's visible that in order to get length the nsnnwn is read instead of one byte according to 11.9.a X.691.

  [PER got  2<=256 bits => span 2 +4[2..256]:00 (254) => 0x0] (asn_bit_data.c:132)
CHOICE F1AP-PDU got index 0 in range 2 (constr_CHOICE_aper.c:48)
Discovered CHOICE F1AP-PDU encodes initiatingMessage (constr_CHOICE_aper.c:80)
Decoding InitiatingMessage as SEQUENCE (APER) (constr_SEQUENCE_aper.c:40)
Decoding member "procedureCode" in InitiatingMessage (constr_SEQUENCE_aper.c:130)
Decoding NativeInteger ProcedureCode (APER) (NativeInteger_aper.c:21)
Integer with range 8 bits (INTEGER_aper.c:54)
Aligning 6 bits (aper_support.c:13)
  [PER got  6<=254 bits => span 8 +4[8..256]:00 (248) => 0x0] (asn_bit_data.c:132)
  [PER got  8<=248 bits => span 16 +5[8..248]:00 (240) => 0x0] (asn_bit_data.c:132)
Got value 0 + low 0 (INTEGER_aper.c:114)
NativeInteger ProcedureCode got value 0 (NativeInteger_aper.c:37)
Freeing INTEGER as a primitive type (asn_codecs_prim.c:16)
Decoding member "criticality" in InitiatingMessage (constr_SEQUENCE_aper.c:130)
Decoding Criticality as NativeEnumerated (NativeEnumerated_aper.c:33)
  [PER got  2<=240 bits => span 18 +6[2..240]:00 (238) => 0x0] (asn_bit_data.c:132)
Decoded Criticality = 0 (NativeEnumerated_aper.c:79)
Decoding member "value" in InitiatingMessage (constr_SEQUENCE_aper.c:130)
Getting open type Reset... (aper_opentype.c:25)
Aligning 6 bits (aper_support.c:13)
  [PER got  6<=238 bits => span 24 +6[8..240]:00 (232) => 0x0] (asn_bit_data.c:132)
  [PER got  8<=232 bits => span 32 +7[8..232]:1c (224) => 0x1c] (asn_bit_data.c:132)
  [PER got 24<=224 bits => span 56 +8[24..224]:00 (200) => 0x3] (asn_bit_data.c:132)
  [PER got 24<=200 bits => span 80 +11[24..200]:00 (176) => 0x4e00] (asn_bit_data.c:132)
  [PER got 24<=176 bits => span 104 +14[24..176]:02 (152) => 0x20000] (asn_bit_data.c:132)
  [PER got 24<=152 bits => span 128 +1[24..152]:00 (128) => 0x40] (asn_bit_data.c:132)
  [PER got 24<=128 bits => span 152 +4[24..128]:01 (104) => 0x10000] (asn_bit_data.c:132)
  [PER got 24<=104 bits => span 176 +7[24..104]:30 (80) => 0x30000a] (asn_bit_data.c:132)
  [PER got 24<=80 bits => span 200 +10[24..80]:40 (56) => 0x400100] (asn_bit_data.c:132)
  [PER got 24<=56 bits => span 224 +13[24..56]:50 (32) => 0x500004] (asn_bit_data.c:132)
  [PER got 24<=32 bits => span 248 +0[24..32]:60 (8) => 0x602000] (asn_bit_data.c:132)
  [PER got  8<= 8 bits => span 256 +3[8..8]:01 (0) => 0x1] (asn_bit_data.c:132)
Getting open type Reset encoded in 28 bytes (aper_opentype.c:50)
Decoding Reset as SEQUENCE (APER) (constr_SEQUENCE_aper.c:40)
  [PER got  1<=224 bits => span 1 +0[1..224]:00 (223) => 0x0] (asn_bit_data.c:132)
Decoding member "protocolIEs" in Reset (constr_SEQUENCE_aper.c:130)
getting nsnnwn with range 65536 (aper_support.c:74)
Aligning 7 bits (aper_support.c:13)
  [PER got  7<=223 bits => span 8 +0[8..224]:00 (216) => 0x0] (asn_bit_data.c:132)
  [PER got 16<=216 bits => span 24 +1[16..216]:00 (200) => 0x3] (asn_bit_data.c:132)
Preparing to fetch 3+0 elements from (null) (constr_SET_OF_aper.c:133)
SET OF ResetIEs decoding (constr_SET_OF_aper.c:153)
Decoding ResetIEs as SEQUENCE (APER) (constr_SEQUENCE_aper.c:40)
Decoding member "id" in ResetIEs (constr_SEQUENCE_aper.c:130)
Decoding NativeInteger ProtocolIE-ID (APER) (NativeInteger_aper.c:21)
Integer with range 16 bits (INTEGER_aper.c:54)
  [PER got 16<=200 bits => span 40 +3[16..200]:00 (184) => 0x4e] (asn_bit_data.c:132)
Got value 78 + low 0 (INTEGER_aper.c:114)
NativeInteger ProtocolIE-ID got value 78 (NativeInteger_aper.c:37)
Freeing INTEGER as a primitive type (asn_codecs_prim.c:16)
Decoding member "criticality" in ResetIEs (constr_SEQUENCE_aper.c:130)
Decoding Criticality as NativeEnumerated (NativeEnumerated_aper.c:33)
  [PER got  2<=184 bits => span 42 +5[2..184]:00 (182) => 0x0] (asn_bit_data.c:132)
Decoded Criticality = 0 (NativeEnumerated_aper.c:79)
Decoding member "value" in ResetIEs (constr_SEQUENCE_aper.c:130)
    Getting open type TransactionID... (aper_opentype.c:25)
Aligning 6 bits (aper_support.c:13)
  [PER got  6<=182 bits => span 48 +5[8..184]:00 (176) => 0x0] (asn_bit_data.c:132)
  [PER got  8<=176 bits => span 56 +6[8..176]:02 (168) => 0x2] (asn_bit_data.c:132)
  [PER got 16<=168 bits => span 72 +7[16..168]:00 (152) => 0x0] (asn_bit_data.c:132)
    Getting open type TransactionID encoded in 2 bytes (aper_opentype.c:50)
Decoding NativeInteger TransactionID (APER) (NativeInteger_aper.c:21)
  [PER got  1<=16 bits => span 1 +0[1..16]:00 (15) => 0x0] (asn_bit_data.c:132)
Integer with range 8 bits (INTEGER_aper.c:54)
Aligning 7 bits (aper_support.c:13)
  [PER got  7<=15 bits => span 8 +0[8..16]:00 (8) => 0x0] (asn_bit_data.c:132)
  [PER got  8<= 8 bits => span 16 +1[8..8]:00 (0) => 0x0] (asn_bit_data.c:132)
Got value 0 + low 0 (INTEGER_aper.c:114)
NativeInteger TransactionID got value 0 (NativeInteger_aper.c:37)
Freeing INTEGER as a primitive type (asn_codecs_prim.c:16)
    No padding (aper_opentype.c:77)
ProtocolIE-Container SET OF ResetIEs decoded 0, 0x1398250 (constr_SET_OF_aper.c:156)
SET OF ResetIEs decoding (constr_SET_OF_aper.c:153)
Decoding ResetIEs as SEQUENCE (APER) (constr_SEQUENCE_aper.c:40)
Decoding member "id" in ResetIEs (constr_SEQUENCE_aper.c:130)
Decoding NativeInteger ProtocolIE-ID (APER) (NativeInteger_aper.c:21)
Integer with range 16 bits (INTEGER_aper.c:54)
  [PER got 16<=152 bits => span 88 +9[16..152]:00 (136) => 0x0] (asn_bit_data.c:132)
Got value 0 + low 0 (INTEGER_aper.c:114)
NativeInteger ProtocolIE-ID got value 0 (NativeInteger_aper.c:37)
Freeing INTEGER as a primitive type (asn_codecs_prim.c:16)
Decoding member "criticality" in ResetIEs (constr_SEQUENCE_aper.c:130)
Decoding Criticality as NativeEnumerated (NativeEnumerated_aper.c:33)
  [PER got  2<=136 bits => span 90 +11[2..136]:40 (134) => 0x1] (asn_bit_data.c:132)
Decoded Criticality = 1 (NativeEnumerated_aper.c:79)
Decoding member "value" in ResetIEs (constr_SEQUENCE_aper.c:130)
    Getting open type Cause... (aper_opentype.c:25)
Aligning 6 bits (aper_support.c:13)
  [PER got  6<=134 bits => span 96 +11[8..136]:40 (128) => 0x0] (asn_bit_data.c:132)
  [PER got  8<=128 bits => span 104 +12[8..128]:01 (120) => 0x1] (asn_bit_data.c:132)
  [PER got  8<=120 bits => span 112 +13[8..120]:00 (112) => 0x0] (asn_bit_data.c:132)
    Getting open type Cause encoded in 1 bytes (aper_opentype.c:50)
  [PER got  3<= 8 bits => span 3 +8[3..8]:00 (5) => 0x0] (asn_bit_data.c:132)
CHOICE Cause got index 0 in range 3 (constr_CHOICE_aper.c:48)
Discovered CHOICE Cause encodes radioNetwork (constr_CHOICE_aper.c:80)
Decoding CauseRadioNetwork as NativeEnumerated (NativeEnumerated_aper.c:33)
  [PER got  1<= 5 bits => span 4 +8[4..8]:00 (4) => 0x0] (asn_bit_data.c:132)
  [PER got  4<= 4 bits => span 8 +8[8..8]:00 (0) => 0x0] (asn_bit_data.c:132)
Decoded CauseRadioNetwork = 0 (NativeEnumerated_aper.c:79)
    No padding (aper_opentype.c:77)
ProtocolIE-Container SET OF ResetIEs decoded 0, 0x13982c0 (constr_SET_OF_aper.c:156)
SET OF ResetIEs decoding (constr_SET_OF_aper.c:153)
Decoding ResetIEs as SEQUENCE (APER) (constr_SEQUENCE_aper.c:40)
Decoding member "id" in ResetIEs (constr_SEQUENCE_aper.c:130)
Decoding NativeInteger ProtocolIE-ID (APER) (NativeInteger_aper.c:21)
Integer with range 16 bits (INTEGER_aper.c:54)
  [PER got 16<=112 bits => span 128 +14[16..112]:00 (96) => 0x30] (asn_bit_data.c:132)
Got value 48 + low 0 (INTEGER_aper.c:114)
NativeInteger ProtocolIE-ID got value 48 (NativeInteger_aper.c:37)
Freeing INTEGER as a primitive type (asn_codecs_prim.c:16)
Decoding member "criticality" in ResetIEs (constr_SEQUENCE_aper.c:130)
Decoding Criticality as NativeEnumerated (NativeEnumerated_aper.c:33)
  [PER got  2<=96 bits => span 130 +0[2..96]:00 (94) => 0x0] (asn_bit_data.c:132)
Decoded Criticality = 0 (NativeEnumerated_aper.c:79)
Decoding member "value" in ResetIEs (constr_SEQUENCE_aper.c:130)
    Getting open type ResetType... (aper_opentype.c:25)
Aligning 6 bits (aper_support.c:13)
  [PER got  6<=94 bits => span 136 +0[8..96]:00 (88) => 0x0] (asn_bit_data.c:132)
  [PER got  8<=88 bits => span 144 +1[8..88]:0a (80) => 0xa] (asn_bit_data.c:132)
  [PER got 24<=80 bits => span 168 +2[24..80]:40 (56) => 0x400100] (asn_bit_data.c:132)
  [PER got 24<=56 bits => span 192 +5[24..56]:50 (32) => 0x500004] (asn_bit_data.c:132)
  [PER got 24<=32 bits => span 216 +8[24..32]:60 (8) => 0x602000] (asn_bit_data.c:132)
  [PER got  8<= 8 bits => span 224 +11[8..8]:01 (0) => 0x1] (asn_bit_data.c:132)
    Getting open type ResetType encoded in 10 bytes (aper_opentype.c:50)
  [PER got  2<=80 bits => span 2 +0[2..80]:40 (78) => 0x1] (asn_bit_data.c:132)
CHOICE ResetType got index 1 in range 2 (constr_CHOICE_aper.c:48)
Discovered CHOICE ResetType encodes partOfF1-Interface (constr_CHOICE_aper.c:80)
getting nsnnwn with range 65536 (aper_support.c:74)
Aligning 6 bits (aper_support.c:13)
  [PER got  6<=78 bits => span 8 +0[8..80]:40 (72) => 0x0] (asn_bit_data.c:132)
  [PER got 16<=72 bits => span 24 +1[16..72]:01 (56) => 0x100] (asn_bit_data.c:132)
Got to decode 256 elements (eff -1) (constr_SET_OF_aper.c:146)
SET OF ProtocolIE-SingleContainer decoding (constr_SET_OF_aper.c:153)
Decoding ProtocolIE-SingleContainer as SEQUENCE (APER) (constr_SEQUENCE_aper.c:40)
Decoding member "id" in ProtocolIE-SingleContainer (constr_SEQUENCE_aper.c:130)
Decoding NativeInteger ProtocolIE-ID (APER) (NativeInteger_aper.c:21)
Integer with range 16 bits (INTEGER_aper.c:54)
  [PER got 16<=56 bits => span 40 +3[16..56]:50 (40) => 0x5000] (asn_bit_data.c:132)
Got value 20480 + low 0 (INTEGER_aper.c:114)
NativeInteger ProtocolIE-ID got value 20480 (NativeInteger_aper.c:37)
Freeing INTEGER as a primitive type (asn_codecs_prim.c:16)
Decoding member "criticality" in ProtocolIE-SingleContainer (constr_SEQUENCE_aper.c:130)
Decoding Criticality as NativeEnumerated (NativeEnumerated_aper.c:33)
  [PER got  2<=40 bits => span 42 +5[2..40]:04 (38) => 0x0] (asn_bit_data.c:132)
Decoded Criticality = 0 (NativeEnumerated_aper.c:79)
Decoding member "value" in ProtocolIE-SingleContainer (constr_SEQUENCE_aper.c:130)
Aligning 6 bits (aper_support.c:13)
  [PER got  6<=38 bits => span 48 +5[8..40]:04 (32) => 0x4] (asn_bit_data.c:132)
  [PER got  8<=32 bits => span 56 +6[8..32]:60 (24) => 0x60] (asn_bit_data.c:132)
  [PER got 24<=24 bits => span 80 +7[24..24]:20 (0) => 0x200001] (asn_bit_data.c:132)
UE-associatedLogicalF1-ConnectionListRes SET OF ProtocolIE-SingleContainer decoded 0, 0x1398378 (constr_SET_OF_aper.c:156)
SET OF ProtocolIE-SingleContainer decoding (constr_SET_OF_aper.c:153)
Decoding ProtocolIE-SingleContainer as SEQUENCE (APER) (constr_SEQUENCE_aper.c:40)
Decoding member "id" in ProtocolIE-SingleContainer (constr_SEQUENCE_aper.c:130)
Decoding NativeInteger ProtocolIE-ID (APER) (NativeInteger_aper.c:21)
Integer with range 16 bits (INTEGER_aper.c:54)
Freeing INTEGER as a primitive type (asn_codecs_prim.c:16)
Failed decode id in ProtocolIE-SingleContainer (constr_SEQUENCE_aper.c:145)
UE-associatedLogicalF1-ConnectionListRes SET OF ProtocolIE-SingleContainer decoded 1, 0x13983e0 (constr_SET_OF_aper.c:156)
Failed decoding ProtocolIE-SingleContainer of UE-associatedLogicalF1-ConnectionListRes (SET OF) (constr_SET_OF_aper.c:166)
Freeing ProtocolIE-SingleContainer as SEQUENCE (constr_SEQUENCE.c:76)
Freeing ProtocolIE-ID as INTEGER (1, 0x13983e0, Native) (NativeInteger.c:104)
Freeing Criticality as INTEGER (1, 0x13983e4, Native) (NativeInteger.c:104)
Freeing value as CHOICE (constr_CHOICE.c:164)
Failed to decode partOfF1-Interface in ResetType (CHOICE) 1 (constr_CHOICE_aper.c:91)
Freeing ResetType as CHOICE (constr_CHOICE.c:164)
Freeing ProtocolIE-SingleContainer as SEQUENCE (constr_SEQUENCE.c:76)
Freeing ProtocolIE-ID as INTEGER (1, 0x1398378, Native) (NativeInteger.c:104)
Freeing Criticality as INTEGER (1, 0x139837c, Native) (NativeInteger.c:104)
Freeing value as CHOICE (constr_CHOICE.c:164)
Failed decode value in ResetIEs (constr_SEQUENCE_aper.c:145)
ProtocolIE-Container SET OF ResetIEs decoded 2, 0x1398308 (constr_SET_OF_aper.c:156)
Failed decoding ResetIEs of ProtocolIE-Container (SET OF) (constr_SET_OF_aper.c:166)
Freeing ResetIEs as SEQUENCE (constr_SEQUENCE.c:76)
Freeing ProtocolIE-ID as INTEGER (1, 0x1398308, Native) (NativeInteger.c:104)
Freeing Criticality as INTEGER (1, 0x139830c, Native) (NativeInteger.c:104)
Freeing value as CHOICE (constr_CHOICE.c:164)
Failed decode protocolIEs in Reset (constr_SEQUENCE_aper.c:145)
Freeing Reset as SEQUENCE (constr_SEQUENCE.c:76)
Freeing ResetIEs as SEQUENCE (constr_SEQUENCE.c:76)
Freeing ProtocolIE-ID as INTEGER (1, 0x1398250, Native) (NativeInteger.c:104)
Freeing Criticality as INTEGER (1, 0x1398254, Native) (NativeInteger.c:104)
Freeing value as CHOICE (constr_CHOICE.c:164)
Freeing TransactionID as INTEGER (1, 0x139825c, Native) (NativeInteger.c:104)
Freeing ResetIEs as SEQUENCE (constr_SEQUENCE.c:76)
Freeing ProtocolIE-ID as INTEGER (1, 0x13982c0, Native) (NativeInteger.c:104)
Freeing Criticality as INTEGER (1, 0x13982c4, Native) (NativeInteger.c:104)
Freeing value as CHOICE (constr_CHOICE.c:164)
Freeing Cause as CHOICE (constr_CHOICE.c:164)
Freeing CauseRadioNetwork as INTEGER (1, 0x13982d0, Native) (NativeInteger.c:104)
Failed decode value in InitiatingMessage (constr_SEQUENCE_aper.c:145)
Failed to decode initiatingMessage in F1AP-PDU (CHOICE) 2 (constr_CHOICE_aper.c:91)
pespin commented 2 years ago

@grzegorzniemirowski that debug output is from 2.5 years ago. I think I'd be great to have an output with current vlm_master to make sure there haven't been side effects with other changes. In any case, I think my encoding failure was encoding a "SEQUENCE_OF", while your decoding issue seems to be in "SEQUENCE". I don't know the exact differences between those, but that may be a good hint.

grzegorzniemirowski commented 2 years ago

@pespin you are right and yes, the above output is from current vlm_master, generated when writing the comment. I suppose length encoding is the same for SEQUENCE_OF and SEQUENCE. I would rather look at the fact that in case of problematic field the lower bound is 1 and upper bound is 65536. So range is 65536 and equal to ub. As upper bound is not less than 64K, nsnnwn shouldn't be used. But the problem is that aper_get_length() checks for range, not upper bound. I guess ct->upper_bound should be passed to this function, not range.

mouse07410 commented 2 years ago

I care about 3GPP ASN which should be supported as any other ASN

I stopped caring for 3GPP almost 20 years ago. I still do care that asn1c processes ASN.1 files correctly, which is why I'm spending my time here.

I don't need F1AP-PDU.o itself, I was just referring to your example of failing compilation and provided correct parameters to help you with code generation.

My point is - unless I'm given a way to locally build an executable that can encode and decode F1AP PDUs to/from APER, I cannot help with your specific example. The fact that I can manually compile one .c file does not help.

asn1c generates 1094 .c files from your F1AP ASN file. make all -f converter-example.mk successfully compiles 954 of them. So, showing me that somebody can compile one file doesn't help. Perhaps you can show me how to compile ProtocolExtensionField.c, which fails on my machine?

I think my encoding failure was encoding a "SEQUENCE_OF", while your decoding issue seems to be in "SEQUENCE". I don't know the exact differences between those, but that may be a good hint.

Well, I'm sure you know that, but SEQUENCE is like a C structure, with its length being the total size of all the components, plus possible tagging, etc. SEQUENCE OF is an array, with its length being the length of one element multiplied by the number of elements, plus some "metadata"...

I would rather look at the fact that in case of problematic field the lower bound is 1 and upper bound is 65536. So range is 65536 and equal to ub. As upper bound is not less than 64K, nsnnwn shouldn't be used. But the problem is that aper_get_length() checks for range, not upper bound. I guess ct->upper_bound should be passed to this function, not range.

So, you think it's the decoder's problem? Does it encode this message OK?

pespin commented 2 years ago

@grzegorzniemirowski are you sure upper bound is 65536 and not 65535 in the field you are mentioning? Which field exactly shows this (1..65536) range? That would indeed explain it perhaps. I think we all agree that it makes sense passing ub and lb to aper_get_length. That may well fix the issue.

mouse07410 commented 2 years ago

You need to manually edit generated code using compiler's hint: replace asn_PER_memb_OCTET_STRING_SIZE_3__constr_96 with memb_OCTET_STRING_SIZE_3__constraint_924. Then converter-example would be compiled correctly.

You are correct - then the example would be compiled and linked. But I rather doubt such a fix results in a correctly functioning converter.

The resulting converter indeed runs, but fails to convert XER file you posted to APER:

<F1AP-PDU>
  <initiatingMessage>
    <procedureCode>0</procedureCode>
    <criticality>
      <reject/>
    </criticality>
    <value>
      <Reset>
        <protocolIEs>
          <SEQUENCE>
            <id>78</id>
            <criticality>
              <reject/>
            </criticality>
            <value>
              <TransactionID>0</TransactionID>
            </value>
          </SEQUENCE>
          <SEQUENCE>
            <id>0</id>
            <criticality>
              <ignore/>
            </criticality>
            <value>
              <Cause>
                <radioNetwork>
                  <unspecified/>
                </radioNetwork>
              </Cause>
            </value>
          </SEQUENCE>
          <SEQUENCE>
            <id>48</id>
            <criticality>
              <reject/>
            </criticality>
            <value>
              <ResetType>
                <partOfF1-Interface>
                  <SEQUENCE>
                    <id>80</id>
                    <criticality>
                      <reject/>
                    </criticality>
                    <value>
                      <UE-associatedLogicalF1-ConnectionItem>
                        <gNB-CU-UE-F1AP-ID>32</gNB-CU-UE-F1AP-ID>
                        <gNB-DU-UE-F1AP-ID>1</gNB-DU-UE-F1AP-ID>
                      </UE-associatedLogicalF1-ConnectionItem>
                    </value>
                  </SEQUENCE>
                </partOfF1-Interface>
              </ResetType>
            </value>
          </SEQUENCE>
        </protocolIEs>
      </Reset>
    </value>
  </initiatingMessage>
</F1AP-PDU>
$ ./converter-example -dd -s 80000000 -b 1000000 -ixer -oaper f1ap_reset_1.xer > f1ap_reset_1.aper
AD: Processing f1ap_reset_1.xer
AD: Decoding 1632 bytes
AD: decode(0) consumed 181+0b (1632), code 2
AD: Clean up partially decoded F1AP-PDU
AD: ofp 1, no=181, oo=0, dbl=0
f1ap_reset_1.xer: Decode failed past byte 181: Input processing error
$ ./converter-example -dd -s 80000000 -b 1000000 -iaper f1ap_reset_3.aper 
AD: Processing f1ap_reset_3.aper
AD: Decoding 32 bytes
AD: decode(0) consumed 0+0b (32), code 2
AD: Clean up partially decoded F1AP-PDU
AD: ofp 1, no=0, oo=0, dbl=0
f1ap_reset_3.aper: Decode failed past byte 0: Input processing error
$ od -t x1 ~/src/asn-tst/F1AP/f1ap_reset_3.aper
0000000 00 00 00 1c 00 00 03 00 4e 00 02 00 00 00 00 40
0000020 01 00 00 30 00 0a 40 01 00 50 00 04 60 20 00 01
0000040
$ 

Update

I confirm that this patch (your #91 )

diff --git a/skeletons/aper_support.c b/skeletons/aper_support.c
index d6fa6b09..0ce9229b 100644
--- a/skeletons/aper_support.c
+++ b/skeletons/aper_support.c
@@ -22,7 +22,7 @@ aper_get_length(asn_per_data_t *pd, int range, int ebits, int *repeat) {

        *repeat = 0;

-       if (range <= 65536 && range >= 0)
+       if (range < 65536 && range >= 0)
                return aper_get_nsnnwn(pd, range);

        if (aper_get_align(pd) < 0)
@@ -163,7 +163,7 @@ aper_put_length(asn_per_outp_t *po, int range, size_t length, int *need_eom) {
        ASN_DEBUG("APER put length %zu with range %d", length, range);

        /* 10.9 X.691 Note 2 */
-       if (range <= 65536 && range >= 0)
+       if (range < 65536 && range >= 0)
                return aper_put_nsnnwn(po, range, length) ? -1 : (ssize_t)length;

        if (aper_put_align(po) < 0)

does fix the F1AP case. Changing only aper_get_length() or aper_put_length() was not sufficient - both had to be fixed.

pespin commented 2 years ago

@mouse07410 @grzegorzniemirowski @laf0rge please have a look at PR https://github.com/mouse07410/asn1c/pull/100 The first bunch of commits contains clean ups and small fixes unrelated to the issue at hand, which I did while reading the code. The last 2 commits rework aper_put_length and aper_get_length so that "lb" and "ub" are passed to it instead of "range". This allows applying nsnnwn for ub<65536.

I validated the existing unit tests and my SBc-AP generated code still work fine. Please @grzegorzniemirowski give it a try and see if it improves the situation.

mouse07410 commented 2 years ago

@pespin thank you - my test shows that #100 fixes the F1AP problem. Looks like I can merge it.

Update

Merged, thanks.

acetcom commented 2 years ago

@mouse07410, @grzegorzniemirowski, @laf0rge and @pespin

Thank you so much for your efforts.

I confirmed that commit 24247e2813a7510ebabe6a9b6b6b29fffa0eb27b works well in Open5GS.

Please let me know if you have any further questions in the Open5GS.

Thanks a lot! Sukchan

ruffyontheweb commented 2 years ago

Hi all. I'm working on testing open air interface and would just like to say thank you all for this these sets of timely commits.

mouse07410 commented 2 years ago

Well, one problem seems solved. Thanks to everybody who helped, especially @pespin , @grzegorzniemirowski , and @laf0rge . I'm closing this issue - please feel free to re-open if needed.

In the meanwhile, the next two hard problems:

Any takers? ;-)

laf0rge commented 2 years ago

On Tue, Jul 12, 2022 at 02:27:10AM -0700, grzegorzniemirowski wrote:

@mouse07410 Detailed bug report has already been given in #62 The issue is that length is encoded in one byte but asn1c expects two bytes. Additionaly I can attach PCAP and test code.

It would be useful if you could copy-paste / quote the relevant parts of the F1AP ASN.1 syntax here for easy review so others interested in helping don't have to find the PDF/DOC file, extract it, etc.

My rough guess is that now that we fixed the lower layer "support" bits, the bug must be at a higher level, i.e. the code generated calling the "support" functions? Or some different situation with constraints etc. that make your case different?

--