Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Failed to parse S1AP and RANAP asn.1 #185

Open
brchiu opened this issue Aug 18, 2017 · 63 comments
Open

Failed to parse S1AP and RANAP asn.1 #185

brchiu opened this issue Aug 18, 2017 · 63 comments

Comments

@brchiu
Copy link
Contributor

brchiu commented Aug 18, 2017

I'd like to report another problem when processing S1AP, RANAP and NBAP asn1 files with newest repository acdca41.

They are OK to be parsed and C code generated in 94f0b64

FATAL: Information Object Set S1AP-ELEMENTARY-PROCEDURES contains no objects at line 247
FATAL: Cannot compile "InitiatingMessage" (20:1) at line 224
FATAL: Cannot compile "InitiatingMessage" (20:1) at line 224
Makefile:1140: recipe for target 'regenerate-from-asn1-source' failed

FATAL: Information Object Set RANAP-ELEMENTARY-PROCEDURES contains no objects at line 233
FATAL: Cannot compile "InitiatingMessage" (20:1) at line 204
FATAL: Cannot compile "InitiatingMessage" (20:1) at line 204
Makefile:1312: recipe for target 'regenerate-from-asn1-source' failed
make: *** [regenerate-from-asn1-source] Error 70

If you need these asn.1 files, please let me know.

Thanks for your great effort !

@brchiu
Copy link
Contributor Author

brchiu commented Aug 19, 2017

By the way, though S1AP and RANAP can be parsed and generate corresponding C code for S1AP and RANAP with commit 94f0b64, due to APER not support in this repository, S1AP and RANAP message can not be encoded or decoded yet.

We still need to merge APER implementation, for example #125 or #115, in the future.

@brchiu brchiu mentioned this issue Aug 24, 2017
@brchiu
Copy link
Contributor Author

brchiu commented Sep 17, 2017

Part of problems during parsing S1AP's ASN.1 was fixed in fix_s1ap branch of my fork.

But there still be unsolved ones. It looks like current asn1c can not handle following ASN.1.
A minimum excerpt is in this gist.

PROTOCOL-IES ::= CLASS {
	&id		ProtocolIE-ID	UNIQUE,
	&criticality	Criticality,
	&Value,
	&presence	Presence
}
WITH SYNTAX {
	ID				&id
	CRITICALITY		&criticality
	TYPE			&Value
	PRESENCE		&presence
}

ProtocolIE-Container {PROTOCOL-IES : IEsSetParam} ::= 
	SEQUENCE (SIZE (0..number1)) OF
	ProtocolIE-Field {{IEsSetParam}}

ProtocolIE-SingleContainer {PROTOCOL-IES : IEsSetParam} ::= 
	ProtocolIE-Field {{IEsSetParam}}

ProtocolIE-Field {PROTOCOL-IES : IEsSetParam} ::= SEQUENCE {
	id		PROTOCOL-IES.&id		({IEsSetParam}),
	criticality	PROTOCOL-IES.&criticality	({IEsSetParam}{@id}),
	value		PROTOCOL-IES.&Value		({IEsSetParam}{@id})
}

@laf0rge
Copy link

laf0rge commented Oct 9, 2017

@brchiu You state "though S1AP and RANAP can be parsed and generate corresponding C code for S1AP and RANAP with commit 94f0b64". I can confirm this, both for RANAP and S1AP. I then did a git bisect and found that ea6635b is the offending commit that breaks parsing of the S1AP/RANAP (and likely many other 3GPP) ASN1 syntax.

Unfortunately that's a very large commit and the commit log doesn't provide a lot of explanation :/

@brchiu
Copy link
Contributor Author

brchiu commented Oct 9, 2017

hi, @laf0rge,

I am quite interested in several open source projects related to telecom, e.g. your Osmocom and openairinterface... etc.

@vlm add many good and useful features in subsequent commits. For example, the ability to parse into the content of PDU instead of leaving it as ANY type there in 94f0b64. I do appreciate his great effort.
I am trying to fix it in my fork on top of current master. I have some progress, very slow, but not complete.

Moreover, APER is required to decode S1AP, RANAP .... etc message. I did some merge from @mouse07410's repository. It also in the midway of work.
If anyone who is familiar with APER encoding rule, perhaps he/she can take it over or review then enhance it.

@laf0rge
Copy link

laf0rge commented Oct 9, 2017

@brchiu APER is not the difficult part, I think that's rather easy to solve. It doesn't require too much knowledge about asn1c internals. APER encoding rules are very easy, and not all that different from UPER, so the existing code can be used as template. Also, there's the hacks by Eurecom/OAI and the code by @mouse07410 so lots of point to start. The proper approach to clean this up and complete is is by means of a test suite againts which the code can be validated. There's an industry-proven UPER implementation in Erlang, and I have access to proprietary asn1 compilers, so generating trusted test data from known implementations is also an option that way. I'm happy to contribute on that side in the months to come.

However, the fact that asn1c fails to process the S1AP/RANAP syntax and has FATAL aborts is sad, particularly as it appears to be a regression. I strongly suggest you get your "minimal" sample for reproducing the bug submitted as a test case into upstream asn1c to avoid any such regressions from re-appearing. Fixing that looks like it requires quite a lot of knowledge about asn1c internals, and that's where I have no clue, sorry.

@brchiu
Copy link
Contributor Author

brchiu commented Oct 9, 2017

@laf0rge,

I must say, it is a limitation but NOT an regression.

@vlm's current implementation focus on handling J2735 and 1609 specs which does not heavily use parameterized type as 3GPP specs do. So newly added features can not work on multiple instances of parameterized type .... etc. That is the current situation.

I don't have time to deal with merging of APER code simultaneously. Whether it is difficult part or not is irrelevant to me. If anyone expect S1AP .... etc to work in future asn1c, he/she had better to put effort on it.

Wish you can contribute to it.

@laf0rge
Copy link

laf0rge commented Oct 9, 2017 via email

@brchiu
Copy link
Contributor Author

brchiu commented Oct 24, 2017

Status Update :

With commit 3bbbd04, ASN.1 of S1AP, RANAP, X2AP, M3AP, LPPa, PCAP, XwAP can be parsed by asn1c and C files generated can be compiled.

However, there still be problem associated with ASN.1 of NBAP due to the following syntax it uses.
Specifically, asn1c is not able to handle { procedureCode id-cellSetup, ddMode fdd }.

cellSetupFDD NBAP-ELEMENTARY-PROCEDURE ::= {
	INITIATING MESSAGE      CellSetupRequestFDD
	SUCCESSFUL OUTCOME      CellSetupResponse
	UNSUCCESSFUL OUTCOME    CellSetupFailure
	MESSAGE DISCRIMINATOR	common
	PROCEDURE ID            { procedureCode id-cellSetup, ddMode fdd } 
	CRITICALITY             reject
}

Moreover, because of lacking APER codec functions for several types, e.g. OPENTYPE, in #226, I am not able to test the built s1ap-dump using S1AP messages at hand. And APER codec functions are not verified yet, it is just merged back from @mouse07410's repository.

Wish people who need this feature can jointly help to improve it.

@velichkov
Copy link
Contributor

Hi @brchiu,

I added OpenType APER support (copy-pasted 😊 uper functions) in my repo 2be7f4b I could open a PR to your repo or you could just cherry-pick this commit in your branch to include it in #226

With it I'm able the APER encode and then decode the following S1AP message (in XER format)

<S1AP-PDU>
    <initiatingMessage>
        <procedureCode>11</procedureCode>
        <criticality><ignore/></criticality>
        <value>
            <DownlinkNASTransport>
                <protocolIEs>
                </protocolIEs>
            </DownlinkNASTransport>
        </value>
    </initiatingMessage>
</S1AP-PDU>

@brchiu
Copy link
Contributor Author

brchiu commented Oct 25, 2017

@velichkov , Great !

However, a previous example decoded by @mouse07410 repository shows s1ap-dump -iaper -oxer sample-S1AP-InitialUEMessage.aper result in the following output.

<S1AP-PDU>
    <initiatingMessage>
        <procedureCode>12</procedureCode>
        <criticality><ignore/></criticality>
        <value>
            00 00 05 00 08 00 02 00 33 00 1A 00 2C 2B 07 41 
            72 0B F6 32 F5 49 80 00 01 00 00 00 01 04 E0 60 
            C0 C0 00 05 02 01 D0 11 D1 52 32 F5 49 00 3D 5C 
            20 00 90 11 03 57 18 86 F1 00 43 00 06 00 32 F5 
            49 00 3D 00 64 40 08 00 32 F5 49 00 00 10 00 00 
            86 40 01 30
        </value>
    </initiatingMessage>
</S1AP-PDU>

Looks like asn1c does not traverse down to each protocolIEs field.
There still be some work to do.

@velichkov
Copy link
Contributor

velichkov commented Oct 25, 2017

However, a previous example decoded by @mouse07410 repository shows s1ap-dump -iaper -oxer sample-S1AP-InitialUEMessage.aper result in the following output.

I'm testing with the generated converter-example and with this sample-S1AP-InitialUEMessage.aper it fails with

Decoding member "value" in ProtocolIE-Field (constr_SEQUENCE.c:1597)
Type selector is not defined for Open Type ProtocolIE-Field->value->value (OPEN_TYPE.c:419)
Failed to decode element ProtocolIE-Field (OPEN_TYPE.c:420)

so it does traverse down but the IOC table in ProtocolIE-Field.c is empty and I can't figure out why.

Maybe here we need to call the process callback similar to _asn1f_foreach_unparsed_union

@brchiu
Copy link
Contributor Author

brchiu commented Oct 25, 2017

It seems only IOC tables of InitiatingMessage, SuccessfulOutcome and UnsuccessfulOutcome are generated. And others are empty.

Taking aforementioned InitialUEMessage as an example, no corresponding data structure generated for the following relation.

InitialUEMessage ::= SEQUENCE {
    protocolIEs     ProtocolIE-Container   {{InitialUEMessage-IEs}},
    ...
}

InitialUEMessage-IEs S1AP-PROTOCOL-IES ::= {
    { ID id-eNB-UE-S1AP-ID  CRITICALITY reject TYPE ENB-UE-S1AP-ID  PRESENCE mandatory}|
    { ID id-NAS-PDU         CRITICALITY reject TYPE NAS-PDU         PRESENCE mandatory}|
.....
    { ID id-Coverage-Level  CRITICALITY ignore TYPE Coverage-Level  PRESENCE optional},
    ...
}

There still be some work to do.

@brchiu
Copy link
Contributor Author

brchiu commented Oct 26, 2017

Part of IOC generation problems is solved in my local branch.

However, due to complex relation between so many generated types, there is cyclic inclusion of header file occurs and some generated C files from S1AP ASN.1 fail to be compiled.
Specifically, generated ProtocolIE-Field.h includes a lot of header files of other types because many information object sets are instantiated by it.

ProtocolIE-Field {S1AP-PROTOCOL-IES : IEsSetParam} ::= SEQUENCE {
    id            S1AP-PROTOCOL-IES.&id            ({IEsSetParam}),
    criticality   S1AP-PROTOCOL-IES.&criticality   ({IEsSetParam}{@id}),
    value         S1AP-PROTOCOL-IES.&Value         ({IEsSetParam}{@id})
}

I think it might need a overhaul of file structure to solve this problem.
For example, break up ProtocolIE-Field.h to small ones, .... etc.

@velichkov
Copy link
Contributor

Hi @brchiu,

Part of IOC generation problems is solved in my local branch.

You have made a great progress!

However, due to complex relation between so many generated types, there is cyclic inclusion of header file occurs and some generated C files from S1AP ASN.1 fail to be compiled.
I think it might need a overhaul of file structure to solve this problem.
For example, break up ProtocolIE-Field.h to small ones, .... etc.

An alternative solution according to the documentation could be

   -findirect-choice
      When  generating code for a CHOICE type, compile the CHOICE mem-
      bers as indirect pointers instead of declaring them inline. Con-
      sider  using this option together with -fno-include-deps to pre-
      vent circular references.

I just tried with both options and the cyclic inclusion is removed at least for S1AP but the compilation fails with

cc -DASN_DISABLE_OER_SUPPORT  -DPDU=S1AP_PDU -DASN_EMIT_DEBUG=1 -I. -g3 -O0 -o ProtocolIE-Field.o -c ProtocolIE-Field.c
ProtocolIE-Field.c:15995:1: error: redefinition of ‘memb_id_constraint_0’
 memb_id_constraint_0(const asn_TYPE_descriptor_t *td, const void *sptr,
 ^~~~~~~~~~~~~~~~~~~~
ProtocolIE-Field.c:15977:1: note: previous definition of ‘memb_id_constraint_0’ was here
 memb_id_constraint_0(const asn_TYPE_descriptor_t *td, const void *sptr,
 ^~~~~~~~~~~~~~~~~~~~
ProtocolIE-Field.c:16001:1: error: redefinition of ‘memb_criticality_constraint_0’
 memb_criticality_constraint_0(const asn_TYPE_descriptor_t *td, const void *sptr,
 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
ProtocolIE-Field.c:15983:1: note: previous definition of ‘memb_criticality_constraint_0’ was here
 memb_criticality_constraint_0(const asn_TYPE_descriptor_t *td, const void *sptr,

@brchiu
Copy link
Contributor Author

brchiu commented Nov 2, 2017

hi, @velichkov,

Many thanks for your suggestion. 👍
I use my newest fix_s1ap_code_generation_2, with the command line option -fcompound-names -fno-include-deps -gen-PER -findirect-choice, the generated C files from S1AP ASN.1 get compiled !!

Below is the first several lines of my Makefile for s1ap for your reference

ASN_PROGRAM = s1ap-dump
CFLAGS += -DASN_CONVERTER_TITLE="S1AP decoder" -DHAVE_CONFIG_H -DJUNKTEST -D_DEFAULT_SOURCE
begin: S1AP-PDU.c maybe-wip-pause all

-include Makefile.am.example

S1AP-PDU.c: ../sample.makefile.regen ../s1ap-14.4.0.asn1
	make regen-makefile
	@touch S1AP-PDU.c
	make

regen-makefile:
	TITLE="S1AP decoder" \
	ASN_CMDOPTS="-fcompound-names -fno-include-deps -gen-PER -findirect-choice" \
	ASN_MODULES="../s1ap-14.4.0.asn1" \
	ASN_PDU=S1AP-PDU \
	ASN_PROGRAM=s1ap-dump \
	../sample.makefile.regen

However, after merged with #226, s1ap-dump still fail to decode the sample-S1AP-InitialContextSetup.aper under mouse07410's examples/sample.source.S1AP directory, which I created from the content of certain wireshark capture file. Error message is here

Based on the following excerpt,

Decoding member "uEaggregateMaximumBitRateDL" in UEAggregateMaximumBitrate (constr_SEQUENCE.c:1597)
Integer with range 34 bits (INTEGER.c:849)
Can encode 34 (5 bytes) in 3 bits (INTEGER.c:863)
  [PER got  3<=72 bits => span 11 +1[3..72]:05 (69) => 0x0] (asn_bit_data.c:139)
Got length 1 (INTEGER.c:870)
Aligning 5 bits (per_support.c:301)
  [PER got  5<=69 bits => span 16 +1[8..72]:05 (64) => 0x5] (asn_bit_data.c:139)
Failed to decode element BitRate (INTEGER.c:872)
Failed decode uEaggregateMaximumBitRateDL in UEAggregateMaximumBitrate (constr_SEQUENCE.c:1607)

Then checking s1ap-14.4.0.asn, BitRate indeed needs 34 bits.

BitRate	::= INTEGER (0..10000000000) 

And in asn_bit_data.c, it is incapable of getting more than 31 bits.

	/*
	 * Extract specified number of bits.
	 */
	if(off <= 8)
		accum = nbits ? (buf[0]) >> (8 - off) : 0;
	else if(off <= 16)
		accum = ((buf[0] << 8) + buf[1]) >> (16 - off);
	else if(off <= 24)
		accum = ((buf[0] << 16) + (buf[1] << 8) + buf[2]) >> (24 - off);
	else if(off <= 31)
		accum = (((uint32_t)buf[0] << 24) + (buf[1] << 16)
			+ (buf[2] << 8) + (buf[3])) >> (32 - off);
	else if(nbits <= 31) {
		asn_bit_data_t tpd = *pd;
		/* Here are we with our 31-bits limit plus 1..7 bits offset. */
		asn_get_undo(&tpd, nbits);
		/* The number of available bits in the stream allow
		 * for the following operations to take place without
		 * invoking the ->refill() function */
		accum  = asn_get_few_bits(&tpd, nbits - 24) << 24;
		accum |= asn_get_few_bits(&tpd, 24);
	} else {
		asn_get_undo(pd, nbits);
		return -1;
	}

Can any one have a solution to this ?
Looks like we are close to the goal.

@velichkov
Copy link
Contributor

I use my newest branch head : commit 6ab95f5, with the command line option -fcompound-names -fno-include-deps -gen-PER -findirect-choice, the generated C files from S1AP ASN.1 get compiled !!

🎉 Yeah, that's really a great news. Many thanks for your great efforts !!!
I just extracted again the asn1 from the 36413-e40.doc and I'm able to compile it as well.

still fail to decode the sample-S1AP-InitialContextSetup.aper under mouse07410's examples/sample.source.S1AP directory, which I created from the content of certain wireshark capture file

Could you upload the pcap file if you still have it?

Can any one have a solution to this ?

I will try to fix it this weekend.

@brchiu
Copy link
Contributor Author

brchiu commented Nov 2, 2017

I try to decode other S1AP sample messages available, i.e. sample-S1AP-InitialUEMessage.aper, sample-S1AP-InitialUEMessage2.aper and sample-S1AP-InitiatingMessage.aper, they all can be decoded.

🎉 🎉 🎉

Below is an example :

$ ./s1ap-dump -per-nopad -iaper sample-S1AP-InitialUEMessage.aper
<S1AP-PDU>
    <initiatingMessage>
        <procedureCode>12</procedureCode>
        <criticality><ignore/></criticality>
        <value>
            <InitialUEMessage>
                <protocolIEs>
                    <ProtocolIE-Field>
                        <id>8</id>
                        <criticality><reject/></criticality>
                        <value>
                            <ENB-UE-S1AP-ID>51</ENB-UE-S1AP-ID>
                        </value>
                    </ProtocolIE-Field>
                    <ProtocolIE-Field>
                        <id>26</id>
                        <criticality><reject/></criticality>
                        <value>
                            <NAS-PDU>
                                07 41 72 0B F6 32 F5 49 80 00 01 00 00 00 01 04 
                                E0 60 C0 C0 00 05 02 01 D0 11 D1 52 32 F5 49 00 
                                3D 5C 20 00 90 11 03 57 18 86 F1
                            </NAS-PDU>
                        </value>
                    </ProtocolIE-Field>
                    <ProtocolIE-Field>
                        <id>67</id>
                        <criticality><reject/></criticality>
                        <value>
                            <TAI>
                                <pLMNidentity>32 F5 49</pLMNidentity>
                                <tAC>00 3D</tAC>
                            </TAI>
                        </value>
                    </ProtocolIE-Field>
                    <ProtocolIE-Field>
                        <id>100</id>
                        <criticality><ignore/></criticality>
                        <value>
                            <EUTRAN-CGI>
                                <pLMNidentity>32 F5 49</pLMNidentity>
                                <cell-ID>
                                    0000000000000000000100000000
                                </cell-ID>
                            </EUTRAN-CGI>
                        </value>
                    </ProtocolIE-Field>
                    <ProtocolIE-Field>
                        <id>134</id>
                        <criticality><ignore/></criticality>
                        <value>
                            <RRC-Establishment-Cause><mo-Signalling/></RRC-Establishment-Cause>
                        </value>
                    </ProtocolIE-Field>
                </protocolIEs>
            </InitialUEMessage>
        </value>
    </initiatingMessage>
</S1AP-PDU>

As for the wireshark captures, I don't remember where I downloaded them, perhaps from Wireshark's wiki but they do not exist any more. We should have them for validating the decoding result. 😢

As for these binary *.aper files, you can have it at @mouse07410's repository.

UPDATE: I have sent pull request #234 and the binary files are there.

@laf0rge, we have some progress for S1AP message processing.
Could you kindly provide some S1AP and other 3GPP protocol messages for basic testing ?
Or tell us where we can access them ?

@brchiu
Copy link
Contributor Author

brchiu commented Nov 2, 2017

One memory leak in s1ap-dump might need be solved.
It looks like a general problem in converter-examples.c.

==27543== Memcheck, a memory error detector
==27543== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==27543== Using Valgrind-3.13.0 and LibVEX; rerun with -h for copyright info
==27543== Command: ./s1ap-dump -per-nopad -iaper sample-S1AP-InitialUEMessage.aper
==27543== Parent PID: 2039
==27543== 
==27543== 
==27543== HEAP SUMMARY:
==27543==     in use at exit: 8,192 bytes in 1 blocks
==27543==   total heap usage: 40 allocs, 39 frees, 15,557 bytes allocated
==27543== 
==27543== 8,192 bytes in 1 blocks are still reachable in loss record 1 of 1
==27543==    at 0x4C2FA3F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==27543==    by 0x4C31D84: realloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==27543==    by 0x19E847: data_decode_from_file (in /media/brchiu/DATA/DocCollect/SoftwareTech/asn1c/asn1c_br3/examples/sample.source.S1AP/s1ap-dump)
==27543==    by 0x19D7B9: main (in /media/brchiu/DATA/DocCollect/SoftwareTech/asn1c/asn1c_br3/examples/sample.source.S1AP/s1ap-dump)
==27543== 
==27543== LEAK SUMMARY:
==27543==    definitely lost: 0 bytes in 0 blocks
==27543==    indirectly lost: 0 bytes in 0 blocks
==27543==      possibly lost: 0 bytes in 0 blocks
==27543==    still reachable: 8,192 bytes in 1 blocks
==27543==         suppressed: 0 bytes in 0 blocks
==27543== 
==27543== For counts of detected and suppressed errors, rerun with: -v
==27543== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)

@velichkov
Copy link
Contributor

Hi @brchiu,

About the failure to decode sample-S1AP-InitialContextSetup.aper if you comment out lines 1557 - 1572 in const_SEQUENCE.c, then it will be decoded successfully.

/* Get Padding */
padding = (8 - (pd->moved % 8)) % 8;
if(padding > 0)
ASN_DEBUG("For element %s,offset= %ld Padding bits = %d", td->name, pd->moved, padding);
#if 0 /* old way of removing padding */
per_get_few_bits(pd, padding);
#else /* Experimental fix proposed by @mhanna123 */
if(edx != (td->elements_count-1))
per_get_few_bits(pd, padding);
else {
if(specs->roms_count && (padding > 0))
ASN_DEBUG(">>>>> not skipping padding of %d bits for element:%ld out of %d", padding, edx, td->elements_count);
else
per_get_few_bits(pd, padding);
}
#endif /* dealing with padding */

This code was added in mouse07410#29 in commits 569eeaf and 12850ab

brchiu added a commit to brchiu/asn1c that referenced this issue Nov 5, 2017
@brchiu
Copy link
Contributor Author

brchiu commented Nov 5, 2017

Hi, @velichkov,

Many thanks for your advice. It works !

brchiu added a commit to brchiu/asn1c that referenced this issue Nov 5, 2017
@mouse07410
Copy link

I believe those lines were added to address a problem, mouse07410#30 to be specific.

Does it apply to the current master here? If so, should anything be done about it?

@velichkov
Copy link
Contributor

Hi @mouse07410,

I believe those lines were added to address a problem, mouse07410#30 to be specific.

Yes, I know.

Does it apply to the current master here?

Yes, it does.

If so, should anything be done about it?

Yes, someone needs to implement a proper fix. The master in your fork is currently broken as well and cannot decode this S1AP parameter and other SEQUENCEs as well (see mouse07410#31). It seems you shouldn't have accepted this fix as it is neither correct nor complete, there is no equivalent code in SEQUENCE_encode_aper so the encode/decode is not symmetrical.

My priority right now is to help @brchiu in his efforts to improve IOC support.

@mouse07410
Copy link

mouse07410 commented Nov 7, 2017

My priority right now is to help @brchiu in his efforts to improve IOC support.

OK, makes sense. Thanks!

velichkov added a commit to velichkov/asn1c that referenced this issue Mar 14, 2018
On i686 the long is 4 bytes and the right shift with more then 32 bits
is probably an undefined behaviour

See vlm#185 (comment)
velichkov added a commit to velichkov/asn1c that referenced this issue Mar 14, 2018
On i686 the long is 4 bytes and when converted to uint64_t some big
unsigned values get converted to very big negative values

See vlm#185 (comment)
@velichkov
Copy link
Contributor

velichkov commented Mar 14, 2018

Hi @acetcom,

I just pushed 3 commits in my fork - two APER fixes (fec0acc, f5dc5cc) and one XER (8d06d92)

$ file ./s1ap-dump 
./s1ap-dump: ELF 32-bit LSB executable, Intel 80386, version 1 (SYSV), dynamically linked, interpreter /lib/ld-linux.so.2, for GNU/Linux 2.6.32, BuildID[sha1]=58db588dc25d71cb014f8c63f3f4577123e710fb, with debug_info, not stripped
$ ./s1ap-dump -t
000b4080 8c000003 00000002 00010008 00020001 001a0079 78efefef efefefef 
efefefef efefefef efefefef efefefef efefefef efefefef efefefef efefefef 
efefefef efefefef efefefef efefefef efefefef efefefef efefefef efefefef 
efefefef efefefef efefefef efefefef efefefef efefefef efefefef efefefef 
efefefef efefefef efefefef efefefef ef

The problems were also reproducible with make check as I've already added the PDUs you provided in my fork.

@acetcom
Copy link

acetcom commented Mar 14, 2018

Hi, @velichkov

Great! NextEPC 32bit version is also successfully ported with this.

For your reference, I've only changed the following code based on your s1ap branch.

acetcom@sejin123:~/Documents/git/asn1c.velichkov/skeletons$ git diff
diff --git a/skeletons/per_support.h b/skeletons/per_support.h
index 23079c9..1378030 100644
--- a/skeletons/per_support.h
+++ b/skeletons/per_support.h
@@ -25,7 +25,11 @@ typedef struct asn_per_constraint_s {
        int  range_bits;                /* Full number of bits in the range */
        int  effective_bits;            /* Effective bits */
        long lower_bound;               /* "lb" value */
+#if 0 /* modified by acetcom */
        long upper_bound;               /* "ub" value */
+#else
+       int64_t upper_bound;            /* "ub" value */
+#endif
 } asn_per_constraint_t;
 typedef struct asn_per_constraints_s {
        asn_per_constraint_t value;

The attached pcap was created by new S1AP API.
s1ap-32bit.tar.gz

And also, you can find the example code in NextEPC repository.

git clone https://github.com/acetcom/nextepc
git checkout s1ap

The new NextEPC will be released after testing real eNodeB. 1~2 week might be needed.

Thank you very much!!!

Best Regards,
Sukchan

@mouse07410
Copy link

@velichkov what branch of your fork should I cherry-pick those three commits from (assuming it matters?)? Or maybe you can make a PR to my vlm_master branch (which will replace master fairly soon)?

@velichkov
Copy link
Contributor

Hi @mouse07410,

@velichkov what branch of your fork should I cherry-pick those three commits from

The branch name is s1ap and is based on the brchiu's s1ap branch.

(assuming it matters?)?

Depends if you need 32bit support or not.

Or maybe you can make a PR to my vlm_master branch (which will replace master fairly soon)?

I prefer if you could merge my branch directly or review and cherry-pick some of the commits that you find useful because the whole procedure of creating additional branches, rebasing, opening PRs is tedious and time consuming.

@mouse07410
Copy link

The branch name is s1ap and is based on the brchiu's s1ap branch.

Thanks!

(assuming it matters?)?

Depends if you need 32bit support or not.

Actually, what I meant to ask was - do I need to know the branch name in order to cherry-pick a commit from your repo. ;-)

It appears that I don't - cherry-picking worked OK without specifying the branch.

I prefer if you could merge my branch directly or review and cherry-pick some of the commits that you find useful...

Fair enough, at least for those more straightforward "mergers".

Could you tell me what s1ap branch is, and how it is related to your master and to vlm/master? Is this branch an enhancement that should be integrated into the main master (in your and/or my forks, if not in the upstream), or is it a modification that is necessary to deal with S1AP ASN.1, but supporting it is likely to break something else?

mouse07410 pushed a commit to mouse07410/asn1c that referenced this issue Mar 14, 2018
On i686 the long is 4 bytes and the right shift with more then 32 bits
is probably an undefined behaviour

See vlm#185 (comment)
mouse07410 pushed a commit to mouse07410/asn1c that referenced this issue Mar 14, 2018
On i686 the long is 4 bytes and when converted to uint64_t some big
unsigned values get converted to very big negative values

See vlm#185 (comment)
@velichkov
Copy link
Contributor

Hi @mouse07410,

Could you tell me what s1ap branch is, and how it is related to your master and to vlm/master

My master is pretty out of date because I don't use it. My s1ap branch is based on @brchiu's s1ap where he has merged the IOC and APER branches needed to compile the S1AP.

Is this branch an enhancement that should be integrated into the main master (in your and/or my forks, if not in the upstream), or is it a modification that is necessary to deal with S1AP ASN.1, but supporting it is likely to break something else?

My changes are not specific to the S1AP protocol and they need to be integrated in the mainline. I saw you've already cherry-picked the three commits mentioned in the previous posts and most probably you will want a23f13f and ca9847d as well. The first fixes a segmentation fault while freeing OPEN_TYPE structs and with the second the -per-nopad option is no longer needed when decoding APER PDUs.

@mouse07410
Copy link

My master is pretty out of date because I don't use it

Ah, OK. Good to know.

My changes are not specific to the S1AP protocol and they need to be integrated in the mainline

OK, good. I'm merging your s1ap branch (as it includes @brchiu's IOC and APER) into my vlm_master that will replace the current master (which by now became outdated, as it does not support OER). I assume that merge includes the two commits you recommended.

Thanks!

@mouse07410
Copy link

@velichkov OK, merged. My vlm_master now has your s1ap. I will let you and @brchiu know when I make it the "main" master of my fork.

@acetcom
Copy link

acetcom commented Mar 16, 2018

Hi, @velichkov and @mouse07410

I've tested mouse07410/vlm_master branch with NextEPC test framework. It is also correctly working. But, I'd just like to know the following difference between velichkov/s1ap and mouse07410/vlm_master. Currently, NextEPC is applied with velichkov/s1ap.

acetcom@sejin123:~/Documents/git/nextepc/lib/s1ap/asn1c$ git diff INTEGER.c
diff --git a/lib/s1ap/asn1c/INTEGER.c b/lib/s1ap/asn1c/INTEGER.c
index 39bae07..11ed138 100644
--- a/lib/s1ap/asn1c/INTEGER.c
+++ b/lib/s1ap/asn1c/INTEGER.c
@@ -507,7 +507,7 @@ INTEGER__xer_body_decode(const asn_TYPE_descriptor_t *td, void *sptr,
                /* The last symbol encountered was a digit. */
         switch(asn_strtoimax_lim(dec_value_start, &dec_value_end, &dec_value)) {
         case ASN_STRTOX_OK:
-            if(specs && specs->field_unsigned && dec_value <= ULONG_MAX) {
+            if(specs && specs->field_unsigned && (uintmax_t) dec_value <= ULONG_MAX) {
                 break;
             } else if(dec_value >= LONG_MIN && dec_value <= LONG_MAX) {
                 break;
@@ -777,12 +777,16 @@ INTEGER_encode_uper(const asn_TYPE_descriptor_t *td,
                /* #11.5.6 -> #11.3 */
                ASN_DEBUG("Encoding integer %ld (%lu) with range %d bits",
                        value, value - ct->lower_bound, ct->range_bits);
-        if(per_long_range_rebase(value, ct->lower_bound, ct->upper_bound, &v)) {
-            ASN__ENCODE_FAILED;
-        }
+       if(specs && specs->field_unsigned) {
+               v = (unsigned long)value - (unsigned long)ct->lower_bound;
+       } else {
+               if(per_long_range_rebase(value, ct->lower_bound, ct->upper_bound, &v)) {
+                       ASN__ENCODE_FAILED;
+               }
+       }
         if(uper_put_constrained_whole_number_u(po, v, ct->range_bits))
-            ASN__ENCODE_FAILED;
-               ASN__ENCODED_OK(er);
+               ASN__ENCODE_FAILED;
+       ASN__ENCODED_OK(er);
        }
 
        if(ct && ct->lower_bound) {

@mouse07410
Copy link

mouse07410 commented Mar 16, 2018

Ha, you noticed. ;-)

The difference you see contains:

And also it looks like indentation could be tightened up a little in that file. ;-)

@acetcom
Copy link

acetcom commented Mar 16, 2018

For reference, mouse07410/vlm_master is also correctly working in 32bit machine.

@velichkov
Copy link
Contributor

Hi @mouse07410,

a fix for #252 provided in #260 (BTW, @velichkov what do you think of this fix? It looks fine to me, but I don't have a 32-bit machine to test it).

The per_long_range_rebase function is performing some range checks that this fix does not, so this fix is not complete.

For example if value is less then ct->lower_bound the encoded value would not be correct while in the signed case the per_long_range_rebase will return -1 and encode is going to fail.

@mouse07410
Copy link

mouse07410 commented Mar 18, 2018

The per_long_range_rebase() function is performing some range checks that this fix does not, so this fix is not complete.

Would you recommend then copying all (or most) of the checks that per_long_range_rebase() function is doing, to here https://github.com/mouse07410/asn1c/blob/8a29c8e5b09b757e3420473c940f913b9bd90fcc/skeletons/INTEGER.c#L781?

Or would you recommend abandoning (reverting) this patch?

Update
Also, could you help me understand what the following code in skeletons/per_support.c is doing:

. . . . .
  if((ub < 0) == (lb < 0)) {
        bounds_range = ub - lb;
. . . . .

Update2
@acetcom could you confirm that the current vlm_master works on your 32-bit machine? (I've added some checks to it.)

@velichkov, I added a few checks, modeling them after per_long_range_rebase(). Does the code now look better? Would you add any more checks?

@mouse07410
Copy link

@velichkov I hate to bother you, as you're already helping a lot - but would you mind taking a look at the fix I applied to skeletons/INTEGER.c, and comment on whether the added checks are sufficient in your opinion?

@acetcom
Copy link

acetcom commented Mar 26, 2018

@mouse07410 and @velichkov

Sorry for late response!

Today(26/March), I've tested asn1c library with NextEPC test framework.

git checkout https://github.com/mouse07410/asn1c
git checkout vlm_master

git checkout https://github.com/velichkov/asn1c
git checkout s1ap

In 32bit machine, both are correctly worked for NextEPC.
No Problem!

Thanks a lot!

@velichkov
Copy link
Contributor

Hi @mouse07410,

but would you mind taking a look at the fix I applied to skeletons/INTEGER.c, and comment on whether the added checks are sufficient in your opinion?

I think these checks are sufficient.

@mouse07410
Copy link

Thank you!

@acetcom
Copy link

acetcom commented Apr 15, 2018

Hi @velichkov

Yesterday, I built an LTE network using srsENB and NextEPC. You can find the setup guide in the document.

During the experiment, I've found one problem for decoding E-RAB Setup Response of S1AP message.

I've attached srsenb.tar.gz file.

  • srsenb.aper : Binary data file for s1ap-dump
  • srsenb.pcapng : Wireshark packet capture. E-RAB Setup Response captured in number 36.
  • e_rab_set_response.png : screen shot

I think that id-CriticalityDiagnostics(58) seems to be related to the problem.
Basically, I have no idea whether srsENB encoding is correct or not.

The following is a result of execution.

Sukchanui-MacBook-Pro:sample.source.S1AP acetcom$ ./s1ap-dump -iaper -per-nopad ./srsenb.aper 
./srsenb.aper: Decode failed past byte 0: Input processing error

My apologies in advance if the packet is wrong encoded.

Thanks a lot!

@velichkov
Copy link
Contributor

Hi @acetcom,

srsenb.pcapng : Wireshark packet capture. E-RAB Setup Response captured in number 36.
e_rab_set_response.png : screen shot
Basically, I have no idea whether srsENB encoding is correct or not.

Most probably the srsENB encoding is not correct because wireshark does not decode it as well.

I think that id-CriticalityDiagnostics(58) seems to be related to the problem.

Yes the problem is related with this parameter. To get some more info run s1ap-dump with -dd option

Getting open type CriticalityDiagnostics encoded in 13 bytes (per_opentype.c:438)
Decoding CriticalityDiagnostics as SEQUENCE (APER) (constr_SEQUENCE.c:1509)
  [PER got  1<=104 bits => span 1 +0[1..104]:03 (103) => 0x0] (asn_bit_data.c:139)
  [PER got  5<=103 bits => span 6 +0[6..104]:03 (98) => 0x0] (asn_bit_data.c:139)
Read in presence bitmap for CriticalityDiagnostics of 5 bits (0..) (constr_SEQUENCE.c:1532)
  [PER got  1<= 5 bits => span 1 +0[1..5]:00 (4) => 0x0] (asn_bit_data.c:139)
Member CriticalityDiagnostics->procedureCode is optional, p=0 (1->5) (constr_SEQUENCE.c:1581)
  [PER got  1<= 4 bits => span 2 +0[2..5]:00 (3) => 0x0] (asn_bit_data.c:139)
Member CriticalityDiagnostics->triggeringMessage is optional, p=0 (2->5) (constr_SEQUENCE.c:1581)
  [PER got  1<= 3 bits => span 3 +0[3..5]:00 (2) => 0x0] (asn_bit_data.c:139)
Member CriticalityDiagnostics->procedureCriticality is optional, p=0 (3->5) (constr_SEQUENCE.c:1581)
  [PER got  1<= 2 bits => span 4 +0[4..5]:00 (1) => 0x0] (asn_bit_data.c:139)
Member CriticalityDiagnostics->iEsCriticalityDiagnostics is optional, p=0 (4->5) (constr_SEQUENCE.c:1581)
  [PER got  1<= 1 bits => span 5 +0[5..5]:00 (0) => 0x0] (asn_bit_data.c:139)
Member CriticalityDiagnostics->iE-Extensions is optional, p=0 (5->5) (constr_SEQUENCE.c:1581)
Too large padding 98 in open type (per_opentype.c:461)

@acetcom
Copy link

acetcom commented Apr 15, 2018

Thank you for your quick reply.

I released NextEPC v0.3.7(Supporting S1AP Release 14 with new S1AP library) yesterday, and I am really glad that I can keep it.

From now on I will also use the -dd option.
And let me try to fix this part with the creator of the srsENB.

Sorry for this noise.

@brchiu
Copy link
Contributor Author

brchiu commented Apr 15, 2018

hi, @acetcom, just for your information.
Online tool provided by http://asn1-playground.oss.com/ also failed to decode this srsenb.aper.

@abodin
Copy link

abodin commented Feb 18, 2019

Dear @brchiu ,
With regard to the problem in compiling "Information Object Set", the problem seems solved and S1AP, RANAP, X2AP, M3AP, LPPa, PCAP, XwAP are successfully compiled, as you mentioned, after commit 3bbbd04 by @vlm .
However, when compiling the GSM_MAP protocol, that issue still rises.

asn1c -fcompound-names -gen-PER GSMMAP.asn MAP-MS-DataTypes.asn MAP-SS-DataTypes.asn MAP-CommonDataTypes.asn MAP-ExtensionDataTypes.asn MAP-SS-Code.asn MAP-BS-Code.asn MAP-TS-Code.asn MAP-ER-DataTypes.asn MAP-OM-DataTypes.asn MAP-CH-DataTypes.asn MAP-DialogueInformation.asn MAP-SM-DataTypes.asn

Do you have any clue that how this can be resolved? Maybe with similar approach which addressed the problem for S1AP.

Thanks

@brchiu
Copy link
Contributor Author

brchiu commented Mar 15, 2019

@abodin , these PRs for information object set are of no help to the problem encountered with GSMMAP.asn, you need to dig into asn1p_y.y to have a solution. Good luck !

@bhati-github
Copy link

https://github.com/bhati-github/5GCore_NMP.git
Network Message Protocol
This can be used across all possible signaling interfaces and very easy to parse.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants