mouse07410 / asn1c

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

APER encoding failed on a 32-bit machine #175

Closed acetcom closed 6 months ago

acetcom commented 6 months ago

APER encoding fails when using the asn_uint642INTEGER function on a 32-bit machine as shown below.

   asn_uint642INTEGER(AMF_UE_NGAP_ID, 0xffffffff);
   ...
   aper_encode_to_buffer(...)

This was first reported by the open5gs community in issues 2934 and I've been experimenting with it and it's failing the same.

I've added some test code to the asn1c branch of open5gs that you can easily experiment with. You can reproduce it by doing the following.

$ git clone https://github.com/open5gs/open5gs
$ cd open5gs
$ git checkout asn1c
$ meson build --prefix=`pwd`/install
$ ninja -C build
$ ./build/
$ ./build/tests/unit/unit
proto-message-test  : SUCCESS
s1ap-message-test   : SUCCESS
nas-message-test    : SUCCESS
gtp-message-test    : SUCCESS
ngap-message-test   : -02/04 14:11:09.489: [core] ERROR: Failed to encode ASN-PDU [-1] (../lib/asn1c/util/message.c:42)
02/04 14:11:09.489: [ngap] ERROR: ogs_asn_encode() failed (../lib/ngap/message.c:35)
|Line 290: expected non-NULL, but saw NULL
FAILED 1 of 5
sbi-message-test    : SUCCESS
security-test       : SUCCESS
crash-test          : SUCCESS
Failed Tests        Total   Fail    Failed %
===================================================
ngap-message-test           5      1     20.00%

This run is failing at line 290 of the tests/unit/ngap-message-test.c code.

266 
267     asn_uint642INTEGER(AMF_UE_NGAP_ID, 0xffffffff);
268     *RAN_UE_NGAP_ID = 1;
269 
270     NAS_PDU->size = gmmbuf->len;
...
288 
289     ngapbuf = ogs_ngap_encode(&pdu);
290     ABTS_PTR_NOTNULL(tc, ngapbuf);
291 
292     ogs_pkbuf_free(ngapbuf);

Of course, it works fine on 64-bit computers.

And the asn1c branch of open5gs used the commit below from mouse07410/asn1c.

commit a6bdcd22658d6b7c3ed218d8c1375907ac532b64 (HEAD -> vlm_master, origin/vlm_master, origin/HEAD)
Merge: 648f0bc3 a7d32490
Author: Mouse <mouse07410@users.noreply.github.com>
Date:   Thu Feb 1 21:41:56 2024 -0500

    Merge pull request #172 from v0-e/class-default-syntax-support

    Class default syntax support

Please let me know if you have any other questions.

Thanks a lot! Sukchan

v0-e commented 6 months ago

INTEGER APER encode/decode functions seem to be operating internally with long variables instead of intmax_t. That is probably the reason of the failure.

mouse07410 commented 6 months ago

Yes. If you search through the issues, you'll find a discussion about changing internal representation of various integer types to INTEGER or something similarly large, to avoid this implicit restriction. So far, nobody (including myself) stepped forward with an actual PR. I personally don't have either time or skills to do that, plus there's a question of what library to "outsource" processing of multi-precision integers to...

v0-e commented 6 months ago

Proposed solution with PR #176. Tested on a 32-bit machine, armv7l. The decoder also suffered from a similar behaviour.

mouse07410 commented 6 months ago

@v0-e thank you for the fix!

acetcom commented 6 months ago

@v0-e

I don't think this issue has been properly resolved yet. The test program I created earlier passes, but there are more unresolved cases.

If you git pull my updated test code from the asn1c branch of open5gs, you will get the following error, even with your modifications.

$ ./tests/unit/unit
proto-message-test  : SUCCESS
s1ap-message-test   : SUCCESS
nas-message-test    : SUCCESS
gtp-message-test    : SUCCESS
ngap-message-test   : |02/09 03:48:41.805: [core] ERROR: Failed to encode ASN-PDU [-1] (../lib/asn1c/util/message.c:42)
02/09 03:48:41.805: [ngap] ERROR: ogs_asn_encode() failed (../lib/ngap/message.c:35)
\Line 314: expected non-NULL, but saw NULL
02/09 03:48:41.805: [core] ERROR: Failed to encode ASN-PDU [-1] (../lib/asn1c/util/message.c:42)
02/09 03:48:41.805: [ngap] ERROR: ogs_asn_encode() failed (../lib/ngap/message.c:35)
FAILED 1 of 5
sbi-message-test    : SUCCESS
security-test       : SUCCESS
crash-test          : SUCCESS
Failed Tests        Total   Fail    Failed %
===================================================
ngap-message-test           5      1     20.00%

I upgraded my test program as below because it works fine when amf_ue_ngap_id is 0xffffffff, but not when 0xffffffffff1 and 0xf7ed938500000001.

290 static void ngap_message_test5_issues2934(abts_case *tc, void *data)
291 {
292     const char *payload =
293         "7e005c00 0d0199f9 07f0ff00 00000020"
294         "3190";
295     ogs_pkbuf_t *gmmbuf = NULL;
296     ogs_pkbuf_t *ngapbuf = NULL;
297     char hexbuf[OGS_HUGE_LEN];
298 
299     gmmbuf = ogs_pkbuf_alloc(NULL, OGS_MAX_SDU_LEN);
300     ogs_assert(gmmbuf);
301     ogs_pkbuf_put_data(gmmbuf,
302             ogs_hex_from_string(payload, hexbuf, sizeof(hexbuf)), 18);
303 
304     ngapbuf = build_uplink_nas_transport(1, 0xffffffff, gmmbuf);
305     ABTS_PTR_NOTNULL(tc, ngapbuf);
306     ogs_pkbuf_free(ngapbuf);
307 
308     gmmbuf = ogs_pkbuf_alloc(NULL, OGS_MAX_SDU_LEN);
309     ogs_assert(gmmbuf);
310     ogs_pkbuf_put_data(gmmbuf,
311             ogs_hex_from_string(payload, hexbuf, sizeof(hexbuf)), 18);
312 
313     ngapbuf = build_uplink_nas_transport(1, 0xffffffffff1, gmmbuf);
314     ABTS_PTR_NOTNULL(tc, ngapbuf);
315     ogs_pkbuf_free(ngapbuf);
316 
317     gmmbuf = ogs_pkbuf_alloc(NULL, OGS_MAX_SDU_LEN);
318     ogs_assert(gmmbuf);
319     ogs_pkbuf_put_data(gmmbuf,
320             ogs_hex_from_string(payload, hexbuf, sizeof(hexbuf)), 18);
321 
322     ngapbuf = build_uplink_nas_transport(1, 0xf7ed938500000001, gmmbuf);
323     ABTS_PTR_NOTNULL(tc, ngapbuf);
324     ogs_pkbuf_free(ngapbuf);
325 }

Please let me know if you have any other questions.

Thanks a lot! Sukchan

acetcom commented 6 months ago

@v0-e

Oh, I was wrong, the maximum value for amf_ue_ngap_id was 2^40-1.

I tested it again and it's working fine.

I apologize for the inconvinence. Sukchan

v0-e commented 6 months ago

Hey @acetcom, No worries, I'm glad I could provide support to your efforts. Keep up the great work with Open5GS.

The exact reason why the encoding was failing was because AMF_UE_NGAP_ID was being treated internally as a negative number (0xffffffff = -1), while it is positively constrained (up to the value the maximum value that you mentioned).

I suspect there will be more instances in asn1c in which the encoding/decoding methods misinterpret the signedness of (large) integer values.

acetcom commented 6 months ago

@v0-e

I think it's because the asn1c library is not used correctly for the type of large integers. If it happens again in Open5GS, I'll share it with you and we'll try to fix it together.

Thank you very much for the wonderful work you have done. Sukchan