Commit Graph

14 Commits

Author SHA1 Message Date
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
Vadim Yanitskiy bbafcc1602 tests/rlcmac: also enable logging for DRLCMACDATA category
Change-Id: Idf0808461f7e7a1bce58d11a54238c215126451a
2020-03-07 16:13:26 +07: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
Pau Espin 0daf913e0c gsm_rlcmac: fix Packet_Resource_Request_t: s/Slot/I_LEVEL_TN/
This is how this field is named in Wireshark.

Change-Id: I140443c48af8e4bb1b6279e6de986879b7d9c276
2020-03-02 13:33:23 +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 e60d9c7e9d gsm_rlcmac: fix misleading LOGP statement in decode_gsm_ra_cap()
Change-Id: I48fd701566e1364ce7fccaa3e3a1a0296b932988
2020-02-18 05:30:57 +07:00
Vadim Yanitskiy 1553049226 csn1: use proper format specifier for unsigned integers
Change-Id: I33f86b79e72394bdb7d99762f8ec21d80e06dc30
2020-02-17 19:40:15 +07:00
Vadim Yanitskiy 8a87f913bd tests/rlcmac: additionally match debug output of the CSN.1 codec
This would allow us to catch more bugs. Note that I had to remove
printing of pointer address to make the output deterministic.

Change-Id: I1a77441eb957353c919bc73f8e3a2e38f4a383a9
2020-02-17 02:30:58 +07:00
Pau Espin ea39fade07 tests/rlcmac: Don't check stderr output
Current stderr output is empty anyway, and not checking it allows
enavling different log levels to easily debug issues.

Change-Id: I5b12e919e08a6eeaad31a459e5a15fdee4d76a61
2020-01-24 13:01:23 +01: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