Commit Graph

16 Commits

Author SHA1 Message Date
Neels Hofmeyr a77eade744 mgcp_conn_get(): match conn Id ('I:') despite leading zeros
The Connection Identifier is defined as a hex string, so clients may send the
ID back with or without leading zeros. Ignore all leading zeros when comparing.

A specific SCCPlite MSC is observed to DLCX with Connection Identifier with
leading zeros removed, which would mismatch pefore this patch.

Extend test_conn_id_matching() in mgcp_test.c to include leading zero tests.

Now, mgcp_conn_get() would match a valid id with *any* amount of leading zeros,
even if that far surpasses the permitted conn id length. Valid lengths of
incoming conn ids should be and is checked elsewhere.

Related: OS#3509
Change-Id: If55a64a2da47b6eff035711c08e4114d70dbec91
2018-09-07 04:14:51 +02:00
Neels Hofmeyr 6531726a02 mgcp_conn_get(): compare conn Id ('I:') case insensitively
The Connection Identifier is defined as a hex string, so clients may send the
ID back in lower case. Convert to upper case before comparing.

A specific SCCPlite MSC is observed to DLCX with Connection Identifier in lower
case, which would mismatch pefore this patch.

Add test_conn_id_matching() in mgcp_test.c to verify case insensitivity.

Cosmetic: use strcmp(), not strncmp(). In the presence of a terminating nul as
we can assume here, this makes no functional difference, but it clarifies the
code.

Related: OS#3508
Depends: Ib0ee1206b9f31d7ba25c31f8008119ac55440797 (libosmocore)
Change-Id: I8e52278c3abe9e9c8c848c2b1538bce443f68a43
2018-09-07 04:14:40 +02:00
Neels Hofmeyr 5336f57f05 fix mgcp_verify_ci(): off-by-one in max len check
MGCP_CONN_ID_MAXLEN actually includes a terminating nul, so we need to compare
strlen() against MGCP_CONN_ID_MAXLEN-1.

Log the length if it is too long.

Add MDCX_TOO_LONG_CI test to mgcp_test.c, testing a conn id of 33 characters.
Before this patch, the test returns error code 515 meaning "not found", while
now it returns 510 meaning "invalid", showing the off-by-one. Same is
illustrated by the error log ("not found" before, "too long" now), but the
error log is not verified by mgcp_test.c.

Change-Id: I8d6cc96be252bb486e94f343a8c7cae641ff9429
2018-09-03 23:08:08 +02:00
Neels Hofmeyr 08e07046f9 mgcp_test: fix log of conn_id presence
Flip logic to accurately log whether an 'I:' is included, instead of logging
the opposite.

Note that it isn't possible to log the actual conn ID, because they are random
and differ in every test run, which would collide with the fixed expected
output file mgcp_test.ok.

Change-Id: Idcd731b9daf618b97d8f7e6a776266071cd29e08
2018-08-30 14:23:12 +00:00
Neels Hofmeyr f2388eab71 X-Osmo-IGN: rather parse items by token, not char
Adjust the X-Osmo-IGN parsing to use string tokens instead of parsing single
characters.

Reconsidering the first implementation as a poor choice, rather specify the
format of X-Osmo-IGN as any-length string tokens separated by spaces, which is
more flexible and more future proof.

See also osmo-gsm-manuals If15a88c3b5b40fd1d24ad0f94f3231f678669ab1 which
defines the X-Osmo-IGN format as string tokens, matching this patch.

In mgcp_test, add an unknown X-Osmo-IGN item. Though the output is not checked
by the testsuite.at, running manually shows the error log about the unkown
X-Osmo-IGN item.

Change-Id: Ia6fe5ead4b601931c1bf41b29fc1b237aac37d2c
2018-08-28 19:38:22 +00:00
Neels Hofmeyr e6d8e91b3a add X-Osmo-IGN MGCP header to ignore CallID
The format is

  CRCX ...
  C: ...
  M: ...
  X-Osmo-IGN: C

So far the only ignorable element is C, i.e. the CallID. Any other items may be
added in the future.

(I initially intended to also add '@' to ignore the endpoint name's domain
part, but in the osmo-mgw code base the domain part is verified long before any
additional headers are even parsed, so sparing that refactoring for now.)

The intention is that osmo-bsc will issue "X-Osmo-IGN: C" for all SCCPlite
calls, because we are unable to retrieve the CallID that the MSC sends to
osmo-mgw for the network side of the endpoint.

Testing with a specific SCCPlite MSC, I actually observe that all CallIDs are
1, even for concurrent calls. So, an alternative hacky solution would have been
to always pass CallID == 1 for SCCPlite connections from osmo-bsc.

Related: I257ad574d8060fef19afce9798bd8a5a7f8c99fe (osmo-bsc)
Change-Id: Id7ae275ffde8ea9389270cfe3db087ee8db00b51
2018-08-25 16:47:44 +02:00
Philipp Maier bc0346e080 mgw: clean up codec negotiation (sdp)
The codec negotiation via SDP is currently in a neglected state. Also
osmo-mgw does some kind of codec decision wile the SDP is parsed, the
result is information for one codec, even when there are multiple codecs
negotiated. This is problematic because we loose all information about
alternate codecs while we parse. This should be untangled and the
information should be presevered. Also we are not really capable
picking a default. Wehen we do not supply any codec information (not
even LCO), then we should pick a sane default codec.

- separate the codec decision from the sdp parser and concentrate
  codec related code in a separate c file
- add support for multiple codecs in one SDP negotiation
- do not initalize "magic" codec defaults during conn allocation
- do not allow invalid payload types, especially not 255. When
  someone tries to select an invalid payload type, do not fail
  hard, just pick a sane default.
- handle the codec decision in protocol.c, pick a sane default
  codec when no (valid) codec has been negotiated (no LCO, no SDP)

Change-Id: If730d022ba6bdb217ad4e20b3fbbd1114dbb4b8f
Closes: OS#2658
Related: OS#3114
Related: OS#2728
2018-06-23 11:39:44 +00:00
Philipp Maier 3d7b58d77a protocol: reject illegal lco options
At the moment osmo-mgw will accept multiple lco options. (e.g.
p:10, a:PCMU, p:10) If LCO appear multiple times, than the first
appearance of will be parsed and used, all following appearances
will be ignored. However, having multiple appearances of LCO is
illegal and affected requests should be rejected. Also osmo-mgw
should reject illegal formatted LCO strings

- make sure that multiple appearances of LCOs will be rejected
- make sure that illegal formated LCOs are rejected
- add testcases with garbeled LCO and valid LCO examples

Change-Id: Iae2fddfa5f2bcfc952f8ab217b3056694e5f7812
Closes: OS#3119
2018-06-06 16:41:04 +02:00
Pau Espin 2da99a2946 mgcp_stat: Don't print osmux stats if it is off
Otherwise we get Osmux stats during a session using RTP, which is
confusing.

Change-Id: I6fcd680a073fbf8769488ffa2b2b32098c87edf4
2018-02-20 13:12:02 +01:00
Philipp Maier 12943ea0c1 mgcp: make domain name configurable
At the moment the MGW has a fixed domain name string that is not even
checked properly.

- Make domain name configurable, use the current "mgw" string as
  defualt to maintain compatibility

- Check the domain name with each request. If the endpoint contains
  an unexpected domain name, the request must be rejected.

Change-Id: Ia91ac428ba83ac1f9b52a0ec8dbf00ef7876da9e
2018-01-19 10:15:47 +01:00
Philipp Maier 7df419b434 mgcp_test: add returncode check
The returncode of get_conn_id_from_response() is unchecked, which
is intentional since some of the test messages will intentionally cause
this function to fail (the response does not inclde a connection
identifier). This means it makes no sense to assert this function
to a fixed value.

In order to spot regressions better print a log message depending
on the return code.

Change-Id: I9bce9ca39b7751b557374b7ab57c6c9005bcdb7d
Fixes: Coverity CID#180534
2017-12-04 17:34:11 +01:00
Philipp Maier ffd75e420c libosmo-mgcp: Connection Identifiers are allocated by MGW, not CA
The MGCP connection identifier is allocated by the MGW while processing
the CRCX, see RFC3435 2.1.3.2:. Including/Accepting a connection
identifier in CRCX is "forbidden" as per RFC3435 Section 3.2.2.

So the MGW side must *reject* a CRCX message with 'I' parameter, and
allocate a connection identifier which is subsequently returned in the
response.

Closes: OS#2648
Change-Id: Iab6a6038e7610c62f34e642cd49c93d11151252c
2017-12-01 11:58:24 +00:00
Philipp Maier 87bd9be0b0 Initially implement the new osmo-mgw and libosmo-mgcp
Leave the old osmo-bsc_mgcp and libosmo-legacy-mgcp as it is; on a copy thereof
(added by a previous commit), apply changes to initially implement the new
osmo-mgw.

Adjust build system and debian packaging to accomodate the new libosmo-mgcp and
osmo-mgw.

The main differences:

*) use a list to manage rtp connections.

Aggregate all rtp related information inside a single struct.

Use a linked list to manage the both connections (net and bts).
The idea behind using a list is that we might support conference
calls at some later point.

Store the linked list in struct mgcp_endpoint, have a private linked
list for each endpoint. The list contains connection items which are
implemented in struct mgcp_conn. A connection is allocated and freed
using the functions in mgcp_conn.c. A connection is allocated on the
reception of a CRCX command and freed with the reception of a DLCX
command.

*) remove external transcoder feature

Fortunatelly the external transcoder feature is not needed
anymore. This patch removes the related code.

*) vty: get rid of CONN_BTS and CONN_NET

Since the new connection model does not make a difference
between BTS and NET connections the VTY should not use
the fixed CONN_BTS and CONN_NET constants.

- Handle the conns list inside the endpoint directly
- introduce function to dump basic rtp connection info
- introduce human readable names for connections

Parts of the code adjusted to use generalized connections instead of explicit
BTS/NET ones:

- teach mgcp_send_dummy() to send dummy packets to any RTP connection
- network: generalize mgcp_bind_net/bts_rtp_port()
- network: generalize mgcp_send()
- tap: generalize call tapping feature
- stat: generalize statistics
- Replace rtp_data_net() and rtp_data_bts() with generalized rtp_data_rx()

*) mgcp_protocol.c fixes:

- check ci string before it is converted:
  In case of missing ci, a nullpointer is delivered to strtoul().
  Add a function that takes ci, checks it and converts it to an
  uint32_t. Use the return code to react on missing ci.
- output error message on missing CI.
- when parsing the mode, print log message when mode is missing.
- use mode_orig when mode is missing.
- fix ptime formatstring to use %u rather than %d.
- cosmetic: log when connection is deleted on DLCX.
- change loglevels of CRCX, MDCX, DLCX events from DEBUG to NOTICE.

*) mgcp_test

- apply rename of strline_r() to mgcp_strline().
- MGCP command macros:
  - Add 'I: 1' parameters.
  - Use proper port numbers:
    from m=audio 0 RTP/AVP 126
    to   m=audio 16002 RTP/AVP 128
  - Change ptime to 'a=ptime:40' because this is what the MGW currently
    returns.  CRCX generally feed a ptime:40 and this is expected to be
    returned.
- struct mgcp_test: Use only one ptype, there are no explicit BTS and NET
  endpoints anymore.
  Hence remove one column from tests[].
- test_messages():
  - Enable: remove '#if 0'
  - Remove concept of BTS and NET endpoints: test only one conn, as they are
    now interchangeable anyway.
  - remove endpoint init, now done internally.
  - add false asserts in error cases.
- test_retransmission():
  - remove endpoint init, now done internally.
  - add false asserts in error cases.
- test_packet_error_detection():
  - Remove concept of BTS and NET endpoints: test only one conn, as they are
    now interchangeable anyway. Use arbitrary conn ids (e.g. 4711).
  - remove endpoint init, now done internally.
  - add false assert in error case.
  - Assert that a conn really vanishes on DLCX, previously the conn would
    remain and just be unused, now it is actually discarded.
- test_no_cycle()
  - Remove concept of BTS and NET endpoints: test only one conn, as they are
    now interchangeable anyway. Use arbitrary conn ids (e.g. 4711).
- test_no_name()
  - Enable: remove '#if 0'.
  - remove endpoint init, now done internally.
  - add false assert in error case.
- mgcp_test.ok: adjust expected results to status quo:
  - We now see two dummy packets instead of one, now sent to both sides because
    we don't know of BTS or NET side. (maybe drop dummy packets later...)
  - packet duration, conn mode: now sane defaults show instead of unset.
- various whitespace and formatting changes from lindent.

Change-Id: Ie008599136c7ed8a0dfbb0cf803188975a499fc5
2017-10-05 01:40:43 +00:00
Neels Hofmeyr f83ec56212 create libosmo-mgcp and osmo-mgw by copying legacy code
This a cosmetic commit, copying libosmo-legacy-mgcp to libosmo-mgcp and
osmo-bsc_mgcp to osmo-mgw 1:1 at first, to provide a basis for next patches
that highlight the changes from legacy to new code.

Until osmo-msc and osmo-bsc are adjusted to operate with the new code, we will
keep the legacy code alongside the new code. The legacy code might be dropped
later.

Change-Id: Idf54481754a1765bdb2d0d7033bc0d7dc2018024
2017-10-05 01:40:43 +00:00
Neels Hofmeyr e9920f24b1 split off osmo-mgw: remove files, apply build, rename
Add vty and logging previously used from libcommon

Rename libmgcp to libosmo-legacy-mgcp and install.

Use DLMGCP, not DMGCP.

Slim down the public mgcpgw_client API, move all elements not actually used by
current callers to private headers / static c.

Depends: libosmocore I09c587e2d59472cbde852d467d457254746d9e67
Change-Id: I71a0a16ebaaef881c34235849601fc40aa12cfd7
2017-08-30 15:04:52 +02:00
Neels Hofmeyr 85929a7be7 move openbsc/* to repos root
This is the first step in creating this repository from the legacy openbsc.git.

Like all other Osmocom repositories, keep the autoconf and automake files in
the repository root. openbsc.git has been the sole exception, which ends now.

Change-Id: I9c6f2a448d9cb1cc088cf1cf6918b69d7e69b4e7
2017-08-27 17:44:55 +02:00