Commit Graph

28 Commits

Author SHA1 Message Date
Pau Espin ebdc0d8c17 csn1: Avoid failing if optional DownlinkDualCarrierCapability_r7 is missing
All additional release fields are considered optional, and the
CSN_DESCR for Content_t already marks almost all as such, except
DownlinkDualCarrierCapability_r7.

It has been found that some MS transmits a MS RA Capability with a Length=61 bits
where the last bit in the buffer is setting the Exist bit for
DownlinkDualCarrierCapability_r7 as 1. Hence, the CSN1 decoder failed to
decode the whole message because it expected to keep reading there
despite there's no more bytes to read.

While this is could actually be considered an MS bug, let's relax our
expectancies and simply consider the case { 1 <end> } as it was { 0 },
and mark skip decoding DownlinkDualCarrierCapability_r7. That waht
wireshark (packet-gsm_a_gsm.c) or pycrate do for instance.

This patch itself doesn't fix the problem where actually the Exist bit
is stored as 1 in the output decoded structure, but simply allows keep
ongoing with decoding until the end. This issue will be fixed in a
follow-up patch.

Related: SYS#5552
Related: OS#4955
Related: OS#5020
Change-Id: I9a2541bd3544802a646890f32725201836abb0da
2021-10-20 15:36:01 +02:00
Pau Espin 089d734cd1 csn1: Add unit test showing RadioAccess Capability decoding failure
This RA Cap creaes a decoding error on our CSN1 decoder, but seems to be
handled properly by wireshark's own decoder as well as pycrate.

The ending bit of last byte in "MS RA capability 1" has a "1" which
according to spec should flag the existance of
DownlinkDualCarrierCapability_r7, but nothing else comes after it. This
matches the expectancies as per Length field of the first RA Cap.

Related: SYS#5552
Related: OS#4955
Related: OS#5020
Change-Id: I51235e8575f4b992b44078713ec67bbccfd13293
2021-10-20 15:36:01 +02:00
Pau Espin 2282b50e5c tests: RLCMACTest: Add one more sample RA capabilities to suite
This one is larger than some of the other already available.
The decoder is wroking as expected here.

Change-Id: I5d986f68395326f894349446194090b1ddaecd69
2021-10-07 18:32:54 +02:00
Vadim Yanitskiy 809dc8b046 tests/rlcmac: add more test vectors for Packet Resource Request
All vectors should be valid, since they were generated by an MS.
As can be seen, osmo-pcu fails to decode one of the vectors.

Change-Id: I37a2ddd394eeffa1cae0f3e419eeee0200a57fcf
OS#4955

Change-Id: Ib5677048f5668185ffe752f97c97d5612eee4d72
2021-01-30 14:14:24 +00:00
Pau Espin 259a694ba7 csn1: Fix readIndex pointer change in CSN_VARIABLE_ARRAY
There's actually 3 errors:
* Its value should be updated, not the pointer itself
* Value should be increased, not decreased
* bitvec_read_field() API is already advancing it, no need to do it

Fixes: OS#4838
Change-Id: I009abc373794e148091e637ffee80c6461960945
2020-11-24 11:22:06 +01:00
Vadim Yanitskiy a2d972a38a RLC/MAC: implement decoding of EGPRS Packet Channel Request
According to 3GPP TS 44.004, section 7.4a, two alternative RACH block
formats are specified: 8 bit (1 octet) and 11 bit. This change adds
CSN.1 definitions for 11 bit EGPRS Packet Channel Request as per
3GPP TS 44.060, table 11.2.5a.2.

Change-Id: I96df3352856933c9140177b2801a2c71f4134183
Related: OS#1548
2020-05-23 19:38:35 +07:00
Pau Espin e50ce6e45c rlcmac: Introduce MS Radio Access Capabilities 2 to fix related spare bits
There's two variants for the Ms Radio Access Capabilities.
* The usual encoding with spare bits (usually to fill up to octet boundary)
as defined in TS 24.008 Table 10.5.146
And there's too:
* MS Radio Access Capabilities 2 IE from TS44.060 section 12.30, which is
the same but removing all spare bits, and which is used in messages like
Packet Resource Request and Additional MS RAC messages.

The later is used basically for messages having extra IEs after the MS
Radio Access capabilities IE, since they are encoded immediatelly
afterwards.

So this patch does:
* Adds the expected spare bits (M_PADDING) to MS_Radio_Access_capability_t
* Creates a new MS_Radio_Access_capability2_t without padding
* Updates code to use the new "2" version where needed.

Note RLCMACTest long de/encoding line logs change only because the name
of the struct changes (the "2" is added).

Change-Id: Ibd756f80a03452a651e2771dbc628d701e55ac4b
2020-03-23 19:02:01 +01:00
Pau Espin 7faa5da209 rlcmac: Fix bug receiving RA cap
It seems the assumptions regarding maximum number of RA capabilitites
in one message were wrong. Doing some rough calculations, each RA
capabilitiy value (without extensions) can take around 20ish bits, which
means for a message containing up to 52 bytes that quite a lot of
different values could be theoretically fed in. Let's be safe and
increase the array size to be able to handle all different access
technologies listed in See TS 24.008 table 10.5.146 following
restrictions:
* "The MS Radio Access capability is a type 4 information element, with a maximum length of 52 octets."
* "Among the three Access Type Technologies GSM 900-P, GSM 900-E and GSM 900-R only one shall be present."
* "the mobile station should provide the relevant radio access
  capability for either GSM 1800 band OR GSM 1900 band, not both".

Wireshark requires similar fix (it's not important though because it
currently uses another ad-hoc decoder for RAcap).

Related: OS#4463
Change-Id: I5334eaacfbc238fae8bea50c9e9667c2117f81ff
2020-03-23 19:01:58 +01:00
Pau Espin efad80bfbf csn1: Validate recursive array max size during decoding
This way if CSN1 encoded bitstream contains more elements than what the
defintion expects it will fail instead of overflowing the decoded
buffer.

RA cap struct placed in unit test is taken from a real android phone
sending the value when attaching to the network. Then SGSN sends it back
and osmo-pcu would crash similar to unit test:
*** stack smashing detected ***: terminated
 Process terminating with default action of signal 6 (SIGABRT): dumping core
at 0x4C62CE5: raise (in /usr/lib/libc-2.31.so)
by 0x4C4C856: abort (in /usr/lib/libc-2.31.so)
by 0x4CA62AF: __libc_message (in /usr/lib/libc-2.31.so)
by 0x4D36069: __fortify_fail (in /usr/lib/libc-2.31.so)
by 0x4D36033: __stack_chk_fail (in /usr/lib/libc-2.31.so)
by 0x124706: testRAcap2(void*) (RLCMACTest.cpp:468)

Related: OS#4463
Change-Id: I9fe0e55e0a6a41ae2cc885fba490c1d4a186231e
2020-03-23 15:34:11 +01:00
Pau Espin 866becefff tests/RLCMACTest: Several fixes and improvements to RAcap tests
It was recently discovered that the Racap value used for testRAcap was
actually malformed (it was taken from a TTCN3 test). It had the presence
bit for "EGPRS multislot class" set but no struct was put after it.

So let's move that malformed value to a new test named
testMalformedRAcap(). Since it doesn't make sense trying to re-encode or
do similar things with an initially malformed value, let's drop all
those parts in this new test.

For the old testRAcap() test, use a new bitstream this time with the
"EGPRS multoslot class" struct set inside (class 3).

Change-Id: I1e7f8d8866695732ee24a79d8b54d660fd4f22d5
2020-03-21 01:24:28 +01:00
Vadim Yanitskiy 29aeb901e4 csn1: fix: do not return 0 if no bits left in the buffer
Both csnStreamDecoder() and csnStreamEncoder() shall not return 0
prematurely if no more bits left in the input / output bit-vector.

Returning CSN_ERROR_NEED_MORE_BITS_TO_UNPACK might make more sense,
however we don't know in advance (i.e. without entering the loop)
whether it's an error or not. Some CSN.1 definitions have names
like 'M_*_OR_NULL', what basically means that they're optional
and can be ignored or omitted.

Most of the case statements do check whether the number of remaining
bits is enough to unpack / pack a value, so let's leave it up to
the current CSN_* handler (pointed by pDescr) if no bits left.

Return CSN_ERROR_NEED_MORE_BITS_TO_UNPACK only if the number of
remaining bits is negative as this is an error in any case.

Change-Id: Ie3a15e210624599e39b1e70c8d34efc10c552f6c
2020-03-11 19:55:55 +00:00
Vadim Yanitskiy 773cb74ec8 rlcmac: fix encode_gsm_*(): do not suppress encoding errors
Change-Id: Ieec8e6e0823c6f6985f9d343af6d503b8fe9e6ab
2020-03-11 19:55:55 +00:00
Vincent Helfre 1145fd263c gsm_rlcmac: improve dissection of MS RA Capability IE
Port from wireshark.git de028e81c53f9c45ccc5adb3bffd2f16ae2017bf

This commit breaks transcoding of the test vectors containing
the MS RA Capability IE due to the reasons explained in [1].

The more fields we add, the longer gets the output of the CSN.1
encoder. This is not critical, since we never need to encode
messages containing the MS RA Capability IE on practice.

[1] Ibb4cbd3f5865415fd547e95fc24ff31df1aed4c0

Ported-by: Pau Espin Pedrol <pespin@sysmocom.de>
Change-Id: Ibb4cbd3f5865415fd547e95fc24ff31df1aed4c0
2020-03-07 05:04:40 +07:00
Vadim Yanitskiy 2679ec0a9f csn1: fix csnStreamDecoder(): skip bits unhandled by serialize()
This change fixes a bug that was reported by Keith Whyte and
confirmed in [1]. The problem is that a user-defined handler
in case of CSN_SERIALIZE may parse only a part of the given
bit-stream, leaving some bits unhandled. This is expected
because the sender (i.e. the MS) may use more recent RLC/MAC
message definitions containing new fields at the end.

Those bits that were left unhandled by serialize() shall not be
interpreted as continuation of the message, they shall be skipped.

Note that the encoded vector in the RLCMAC unit test still does
not match the original one. That's a known bug explained in [2].

[1] If5873355d52d7ddb06c2716154a88d34100f6ab5
[2] Ic46d6e56768f516203d27d8e7a5adb77afdf32b7

Change-Id: Id4cc042fed68fc54aca0355dcb986cab3f6b49ea
Related: OS#4338
2020-03-06 21:49:04 +00:00
Vadim Yanitskiy f22163b1df tests/rlcmac: add a new test vector for Packet Resource Request
This test vector (from HTC Desire 628) demonstrates a bug in the
CSN.1 decoder. For some reason, OsmoPCU fails to decode it:

  DCSN1 ERROR csnStreamDecoder: error NEED_MORE BITS TO UNPACK (-5)
              at EGPRS_TimeslotLinkQualityMeasurements (idx 164)

while Wireshark dissects it without any errors.

Change-Id: If5873355d52d7ddb06c2716154a88d34100f6ab5
Reported: https://osmocom.org/issues/4338#note-15
Related: OS#4338
2020-03-06 21:49:04 +00:00
Vadim Yanitskiy b47e53b5fa tests/rlcmac: also verify encoding of MS RA Capability
The main idea of this change is to demonstrate a weakness of the
CSN.1 codec that most likely causes a unit test breakage in [1].

The problem seems to be that the transitional structures, where
the CSN.1 decoder stores the results, do not contain any details
about presence of the optional fields (such as M_UINT_OR_NULL).

In other words, it's impossible to know whether some optional
field is omitted in the encoded message (NULL), or is it just
set to 0. This means that the encoder will always include all
optional fields, even if they're not present in the original
message.

[1] Ibb4cbd3f5865415fd547e95fc24ff31df1aed4c0

Change-Id: Ic46d6e56768f516203d27d8e7a5adb77afdf32b7
2020-03-02 13:33:23 +00:00
Vadim Yanitskiy 55f06c3d77 tests/rlcmac: fix malformed MS RA capability in testRAcap()
Long story short: as it turns out the test vector '12a5146200'O
has been generated by TITAN, and it's malformed. The length
indicator it contains must be at least 29 bits, not 21. This
field is calculated by TITAN automatically, so I guess there
is a bug somewhere in its RAW encoder implementation.

It's funny that Wireshark decodes the old malformed vector without
any problems if it's encapsulated into the BSSGP DL-UNITDATA. The
reason for that is because BSSGP dissector does not actually use
the CSN.1 codec and relies on its own hand-written parser [1],
which does not respect the length constraints.

Furthermore, table 10.5.146/3GPP TS 24.008, describing the format
of MS Radio Access Capability IE, has the following comment:

  < Multislot capability struct > ::=
    { 0 | 1 < HSCSD multislot class : bit (5) > }
    ...
  -- error: struct too short, assume features do not exist

so ideally our CSN.1 decoder should be more tolerant to the old
malformed vector, but unfortunately error handling is not implemented.

[1] See de_gmm_ms_radio_acc_cap() in epan/dissectors/packet-gsm_a_gm.c.

Change-Id: I5f810397b8d09c18e069168023429f6a4d899c86
2020-02-19 00:07:56 +07:00
Vadim Yanitskiy 5574a58cd6 csn1: fix csnStreamDecoder(): catch unknown CSN_CHOICE values
After the recent changes [1], it was noticed that one of the unit
tests fails. In particular, a decode-encode cycle of Packet
Polling Request produces a different vector:

  vector1 = 49 13 e0 08 50 88 40 13 a8 04 8b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b
  vector2 = 49 13 01 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b
  vector1 == vector2 : FALSE

As it turns out, the original (input) vector itself is malformed
because it contails no valid identity, and thus violates the
specs. The CSN.1 decoder from Pycrate [2] throws an exception
while trying to decode it. I believe we should do the same.

Let's stop decoding the bit stream and return an error in case
if neither of a given list of the choice items matched.

[1] Ia0f8cc224a4c38e80699f834fd83d4c0d99322ea
[2] https://github.com/P1sec/pycrate

Change-Id: I420144773ed5e80372534e0f18db5e74cdb2999d
Fixes: OS#4392
2020-02-11 05:58:44 +07:00
Pau Espin 5e300ce565 Check return code of rlcmac decode/encode functions
Change-Id: Iabcb768bd714680aa768b35c786dea2015d1e451
2020-02-05 17:26:02 +01:00
Pau Espin cdbc5dbd1c tests/rlcmac: Add test to showcase that decode_gsm_ra_cap() fails
Currently code using that function in osmo-pcu is disabled, allegadly
because SGSN was sending incorrect values, but it looks more like a CSN1
issue.

Related: OS#1525, OS#3499
Change-Id: I92c86397f988afaa791871d823a45fa85054f3bb
2020-01-28 13:29:42 +01:00
Pau Espin 87bfbe4fe2 tests/rlcmac: Use osmo_hexdump to print buffers
Old method takes lots of lines of codes and prints inn unconfortable way
because left-trailing zeros are dropped, making it difficult to split in
bytes.

Change-Id: I56c24f934824e4e52a91a7273aec384b2e15aa67
2020-01-24 12:42:03 +01:00
Pau Espin 5cb002f0ef tests/rlcmac: Fix missing commas with unexpected results
Change-Id: Ia0f8cc224a4c38e80699f834fd83d4c0d99322ea
2020-01-24 12:42:00 +01:00
Pau Espin 54681c3ffe tests/rlcmac: print test name at the start
Change-Id: Ib8f0fcbd6bb68d77727c021f0d90d5248e895772
2020-01-23 21:59:23 +01:00
Saurabh Sharan 2b09c39c9c Fix issue in encoding CSN_RECURSIVE_ARRAY
The remaining_bits_len is correctly decremented while encoding
CSN_RECURSIVE_ARRAY for fixing the bug.
Details of the bug is in https://projects.osmocom.org/issues/1641

During introduction of basic EGPRS feature new hex dump message
PUASS, from a different working network log was used in Unit test.
It exposed the issue of incorrect handling of recursive array
encoding in osmo-pcu.

Fixes: OS#1641
2016-03-16 15:01:53 +01:00
Saurabh Sharan bacb65b48b Add test vectors for EGPRS messages
This patch is the test suite modification for the fix encoding of
padding bits. New test vectors have been added both in downlink
and uplink.
2016-03-15 10:05:07 +01:00
Saurabh Sharan 656eed5975 Fix encoding of padding bits to start with 0 bit
This patch is for fixing encoding of padding bits according to the
3gpp spec 44.060 section 11, wherein it shall always start with 0
bit followed with spare padding bits.

During introduction of basic EGPRS feature new hex dump messages
from a different working network log were used in Unit test. These
exposed the issue of incorrect handling of padding bits encoding
in osmo-pcu.

Corrections in the existing test vector of rlcmac is also updated.
In testsuite tbf appropriate corrections for the Tbftest.err is
also done.
2016-03-15 10:04:34 +01:00
Holger Hans Peter Freyther dfe17d7f91 tests: Fix the expected result (re-add whitespace)
As a last minute change I probably ran git rebase --whitespace=fix
on the patch set and broke the test result. Update the expected
file and tests should pass.
2013-08-03 14:59:38 +02:00
Holger Hans Peter Freyther e13298d093 tests: Move the RLCMACTest into the test directory and setup autotest 2013-08-02 13:40:20 +04:00