Commit Graph

198 Commits

Author SHA1 Message Date
Pau Espin 843d9038ce mgw: Allocate mgcp_conn instance under tcfg->endpoints
The connection becomes to the endpoint, so let's not use the NULL
context there.

Related: OS#3950
Change-Id: I6f6441c3ef21aac577af08eb018bacbca4c45fb7
2019-09-19 17:44:16 +02:00
Neels Hofmeyr 401b740ccd explicitly free codecs in mgcp_rtp_conn_cleanup()
There are allocated bits in conn->end.codecs[], free them.

This is not fixing a memleak, since mgcp_rtp_conn_cleanup() is currently only
called from mgcp_conn_free(), which soon after frees the conn; the conn serves
as talloc parent for the codec strings freed in this patch.

The rationale: it is better style to explicitly free them, to also guard
against future callers of mgcp_rtp_conn_cleanup() which might expect complete
cleanup.

Change-Id: Ic471107ce6e94d9ce582d887429c744ff93e3053
2019-08-28 04:57:19 +02:00
Neels Hofmeyr a468b0f57f mgcp_codec_add: fix audio_name size check
Needs to account for terminating '\0'.

Change-Id: I27896beef6ffcc1cb6207daaba6c8b2b03eb513d
2019-08-28 04:56:52 +02:00
Neels Hofmeyr b0cfa7272e mgcp_codec: codec_set(): log about all possible errors
In codec_set(), for each 'goto error', log the specific error cause.

Also add a TODO and a FIXME comment about inventing dynamic payload type
numbers.

Change-Id: I0b44b574c814882b6f8ae7cd738a6f481cd721fd
2019-08-28 00:17:40 +02:00
Neels Hofmeyr 683e05f60b ptmap: implicitly match '/8000' and '/8000/1'
In codecs_same(), do not compare the complete audio_name. The parts of it are
already checked individually:
- subtype_name ("AMR"),
- rate ("8000"; defaults to 8000 if omitted) and
- channels ("1"; defaults to 1 if omitted)
So by also checking the complete audio_name, we brushed over the match of
implicit "/8000" and "/8000/1", which otherwise works out fine.

As a result, translating payload type numbers in RTP headers now also works if
one conn of an endpoint set an rtpmap with "AMR/8000" and the other conn set
"AMR/8000/1".

It seems to me that most PBX out there generate ptmaps omitting the "/1", so
fixing this should make us more interoperable with third party SDP.

See IETF RFC4566 section 6. SDP Attributes:
  For audio streams, <encoding parameters> indicates the number
  of audio channels.  This parameter is OPTIONAL and may be
  omitted if the number of channels is one, provided that no
  additional parameters are needed.

Also allowing to omit the "/8000" is a mere side effect of this patch.
Omitting the rate does not seem to be specified in an RFC, but is logical for
audio codecs defined to require exactly 8000 set as rate (most GSM codecs).

Add tests in mgcp_test.c.

Change-Id: Iab00bf9a55b1847f85999077114b37e70fb677c2
2019-08-28 00:17:40 +02:00
Neels Hofmeyr 16b637bf1b differentiate AMR octet-aligned=0 vs =1
Add corresponding tests in mgcp_test.c

Change-Id: Ib8be73a7ca1b95ce794d130e8eb206dcee700124
2019-08-28 00:17:40 +02:00
Neels Hofmeyr ce64f18587 fix memleak: actually free strings in mgcp_codec_reset_all()
The audio_name and subtype_name are allocated from talloc, so they need to be
freed before resetting the codec array. Use mgcp_codec_free() to ensure this.

Change-Id: I07f207dcb7ce66bbf3445a30af41e696677b384f
2019-08-27 21:53:17 +00:00
Neels Hofmeyr 667fa59b0c mgcp_codec: split codec_free() off of codec_init()
Both are used only in the same .c file, so make them static.

Move codec_set() guts into codec_add(): codec_set is only called by codec_add.
If codec_set were left separate, it'd look like the codec_init() is a bug and
lacks a codec_free() first. When looking at the entire context in codec_add(),
it becomes obvious that codec_init() should be called, not codec_free(),
because it is populating a previously unused entry.

Preparation to fix a memleak in a conn's codec list.

Change-Id: I120cab0a352a1e7b31c8f9c720c47b2c291311d7
2019-08-27 21:53:17 +00:00
Neels Hofmeyr 5a6220f43b mgcp_send(): stop looping on conversion error
If mgcp_send() runs a transcoder loop, break the loop if rfc5993_hr_convert()
or amr_oa_bwe_convert() return with error. Possibly fixes an infinite loop
situation for erratic packets? (Didn't check for that in detail.)

Change-Id: Iba115a0b1d74e7cefba5dcdd777e98ddea9eba8c
2019-08-21 23:06:02 +02:00
Neels Hofmeyr 7c6dd3c2c3 fix crashes: don't assert on incoming RTP packet size
Remove various OSMO_ASSERT() on size of incoming packets. Doing an assert on
incoming data is a DoS attack vector, absolute no-go. Instead, return -EINVAL
and keep running.

Change some return values to be able to distinguish successful operation from
invalid RTP sizes. In rtp_data_net(), make sure to return negative if the RTP
packet was invalid.

Some of the error return codes implemented here will only be used in upcoming
patch Iba115a0b1d74e7cefba5dcdd777e98ddea9eba8c.

Change-Id: I6bc6ee950ce07bcc2c585c30fad02b81153bdde2
2019-08-21 23:05:35 +02:00
Neels Hofmeyr 740af6ed44 mgcp_codec: constify 'param' arg
Change-Id: I3ec6b57298f78604d5cd453f1db6d90ddfd6a2ba
2019-08-09 02:28:37 +02:00
Neels Hofmeyr 782d607962 rename codecs_cmp() to codecs_same()
The name 'cmp' implies a return value of -1, 0, 1 to indicate smaller, match or
larger. Since this function returns bool, it should not be named with 'cmp'.

Change-Id: I2d41b1a32300e295551e85d3f9ab82dd2b0e86b8
2019-08-09 02:28:37 +02:00
Pau Espin a2b1c5e6f6 Fix return variable of strtoul()
Return variable specified by strtoul() is "unsigned long int". If
"unsigned int" is used, according to Coverity the return value can never
be ULONG_MAX:

CID 202173:  Integer handling issues  (CONSTANT_EXPRESSION_RESULT)
"pt == 18446744073709551615UL /* 9223372036854775807L * 2UL + 1UL */" is always false regardless of the values of its operands. This occurs as the logical second operand of "&&".

Furthermore, PT is 7 bit in RTP header [1], so let's avoid accepting
incorrect values.

[1] https://tools.ietf.org/html/rfc3550#section-5

Fixes: c5c1430a1c ("Catch unsigned integer MGCP parsing errors with strtoul")
Fixes: Coverity CID#202172
FIxes: Coverity CID#202173
Change-Id: Ice9eee6a252fab73dbab5ebf3cfc83c1b354fd08
2019-07-29 18:33:38 +02:00
Pau Espin c5c1430a1c Catch unsigned integer MGCP parsing errors with strtoul
Checks to find if strotul failed are taken both from:
man strtoul
man strtol

Change-Id: Ifba1c1e3151d6f92f9da3d4ca2569a5908455ca8
2019-07-25 12:14:48 +00:00
Harald Welte 06a49fc04b mgcp_sdp: Don't check if an unsigned int is below 0
Change-Id: I129f5c6175a8e961bc08b9768bdf22a2232e2fcb
Closes: CID#188849
2019-07-21 09:28:28 +02:00
Pau Espin 9b508f6b53 mgw: Allow receiving uppercase noanswer keyword
MGCP RFC3435 (https://tools.ietf.org/html/rfc3435) states almost all
text has to be handled in a case-insensitive way, except SDP parts.

Related: OS#4001
Change-Id: I637cb20f0af4de33ebf6589b1aff260d57d03e7b
2019-07-03 12:29:43 +02:00
Pau Espin 6e26c70f46 mgw: Allow receiving lowercase X-Osmo-Ign Callid field
MGCP RFC3435 (https://tools.ietf.org/html/rfc3435) states almost all
text has to be handled in a case-insensitive way, except SDP parts.

Related: OS#4001

Change-Id: Ifc1b3bfe6ff6922df478cea89bbbb291b5fa5706
2019-07-03 12:29:43 +02:00
Pau Espin 9a34592c09 mgw: Allow receiving lowercase MGCP header keyword
MGCP RFC3435 (https://tools.ietf.org/html/rfc3435) states almost all
text has to be handled in a case-insensitive way, except SDP parts.

Related: OS#4001
Change-Id: I7d1e55faddafa3c3093d38513d4a434ecf5ea5bd
2019-07-03 12:29:43 +02:00
Pau Espin 7eb6f2cb56 mgw: Make check of duplicated LCO fields case insensitive
Otherwise it would not catch a duplicate if first the param is
introduced in upper case and later in lower case, or the other way
around.

MGCP RFC3435 (https://tools.ietf.org/html/rfc3435) states almost all
text has to be handled in a case-insensitive way, except SDP parts.

Related: OS#4001
Change-Id: I254bfa3a2d2562441ca3a576cc8e1e7967d9c495
2019-07-03 12:29:42 +02:00
Pau Espin 83fd8a5692 mgw: Support receiving lowercase LCO codec
MGCP RFC3435 (https://tools.ietf.org/html/rfc3435) states almost all
text has to be handled in a case-insensitive way, except SDP parts.

Related: OS#4001
Change-Id: I51dc1cdcbe2a5587769335fbecb5039ef22cae5d
2019-07-03 12:29:42 +02:00
Pau Espin 17058484d1 mgw: Support receiving uppercase connection mode
MGCP RFC3435 (https://tools.ietf.org/html/rfc3435) states almost all
text has to be handled in a case-insensitive way, except SDP parts.

Related: OS#4001
Change-Id: I4da93dfc69b5585a197a7e201a1afb72c2f97030
2019-07-03 12:29:42 +02:00
Pau Espin 0c6c3c1da6 mgw: Support lowercase header parameters
MGCP RFC3435 (https://tools.ietf.org/html/rfc3435) states almost all
text has to be handled in a case-insensitive way, except SDP parts.

Related: OS#4001
Change-Id: I48252415f9d0cd985ad097f334aa4c1665f52511
2019-07-03 12:28:45 +02:00
Pau Espin fe9a1fe03b mgw: Support uppercase LCO options
MGCP RFC3435 (https://tools.ietf.org/html/rfc3435) states almost all
text has to be handled in a case-insensitive way, except SDP parts.

Related: OS#4001
Change-Id: Ic28a5eacc4c441d68e8a20d2743956ab2e01125d
2019-07-03 12:27:53 +02:00
Oliver Smith 189f29e939 vty: update desc of conn-timeout
It can be used together with LCLS, just make sure to also enable
keep-alive packets.

In OS#3429 it was pointed out, that during LCLS the media path remains
active but is not used. Without any traffic flowing, this looks like a
timed out connection and so it will be killed if conn-timeout is set.

However, OsmoBSC and OsmoMSC have an option to enable RTP keep-alive
packets (through libosmo-mgcp, originally intended to keep connections
behind NAT open). If that option is enabled, the keep-alive packets
should also prevent the conn-timeout.

Related: OS#3783
Change-Id: Ib4d19104d558a26a444a80fb36f4b7b33bc5cc59
2019-06-26 16:28:14 +02:00
Oliver Smith d2ce444008 vty: allow 0 as conn-timeout to disable it
VTY command to disable conn-timeout again, after it has been enabled.
conn-timeout was introduced in [1].

[1] Change-Id I18886052e090466f73829133c24f011806cc1fe0.
Change-Id: I7dee7dafaaf4bb93fd692ea06b52b9e012beac6d
2019-06-26 16:27:54 +02:00
Pau Espin df7d97e4b4 osmux: Fix hardcoded rtp payload_type 98 in osmux conn
Depends on: libosmo-netif.git I5cbeb494a8932953d9fd2dc24dacf8cd97fd84e4
Change-Id: I24698a9613bc0de9460c6ad2d1067c152ebcf0b2
2019-05-19 19:28:49 +02:00
Pau Espin 85978dadab osmux: Fix CID release for non-enabled connections
Change-Id: If65c70b421476776e20233733722d72aa26d69a8
2019-05-19 07:31:51 +00:00
Pau Espin 9aaaab6b3b osmux: Fix loopback for Osmux connections
Move code in RTP specific path to generic dispatch_rtp_cb. This way
loopback logic is applied both for Osmux and RTP connections.

Change-Id: Ia30f5a14f150e4d151eac4d1046ea834f1685a5f
2019-05-15 23:32:09 +02:00
Pau Espin c1ad553d86 osmux: Use DUMMY ft msg as per Osmux spec
That MGCP_DUMMY_LOAD is an old hack prior to Osmux spec update, and it's
not nice since it cannot be 100% distinguished from a usual AMR ft
frame.

Let's use the correct DUMMY ft type and build it according spec. Allow
handling differently the old format for a while until we are sure no old
implementations (like bsc-nat) exist sending that kind of message.

Change-Id: Ib17d20b87b28aade49ba60519b56a96e694819af
2019-05-15 23:00:55 +02:00
Pau Espin c1bf4694e7 mgw, mgcp-li: Handle X-Osmux param name as case insensitive
RFC3435 states most text (except SDP) must be handled as case
insensitive.

Related: OS#4001
Change-Id: Iac073f1db46569b46eddeaecc9934a2986bd50f1
2019-05-14 20:35:13 +02:00
Pau Espin 1442c5a99e osmux: Redo read/write osmux glue code to have data routed correctly
Remove old BTS/NET no longer in use and meaningless. Use new osmo-mgw
APIs to inject payload RTP<->Osmux on the correct socket and conn.

Change-Id: I60b6ba3ffdc74efff945ba13a0b736798bdf5d8c
2019-05-14 11:36:33 +02:00
Pau Espin dac2ca7e21 osmux: Delay osmux enable of conn until remote addr is configured by MDCX
Change-Id: I243e53681ebeb3d9cd8ed38bb132172b41745795
2019-05-14 11:36:33 +02:00
Pau Espin 375252279c osmux: Improve logging around osmux enabling events
Change-Id: Iab687b97010fd484cb353b240b120c9c382066fa
2019-05-14 11:36:33 +02:00
Pau Espin 295570c631 osmux: Use remote port to send osmux frames
Previously the local one was used but nobody cared because probably
everybody was using default 1984 on different IP addresses.

Change-Id: I01e590465fa247185d74103578681e9041249099
2019-05-14 11:36:33 +02:00
Pau Espin a93c6e9263 osmux: Provide correct local port during mgcp resp
Also document some possible future improvements for local addr.

Change-Id: I12c8fcdc8b772b9f92a70774406d4662f44bd9a9
2019-05-14 11:36:33 +02:00
Pau Espin 14f8a08f44 osmux: Drop unneeded OSMUX_STATE_NEGOTIATING
Change-Id: I94e7df3287d037975adc16c5ada05adf94269ead
2019-05-14 11:36:33 +02:00
Pau Espin c63f15a9a7 mgcp-cli: Parse X-Osmux on MDCX response
During MDCX state is already changed to ACTIVATING but we still want to
send the local CID back to announce that we still use same local CID.

Change-Id: If182a48743ebe03f97caf9034e49b9947014bdf9
2019-05-14 11:36:29 +02:00
Pau Espin 6be2c49538 osmux: Handle Osmux MGCP extension in MDCX messages
Change-Id: I65e53bd5dd08b58c253e03d2f358f3be523a2688
2019-05-14 11:32:33 +02:00
Pau Espin fa810e8ccd osmux: Mark conn_rtp->type as osmux during CRCX
We also update code to allow setting up RTP related fields to succeed
during CRCX. We also update code to allow setting up RTP related fields to
succeed during CRCX.

Change-Id: Ia6e723d9a28ba38fc3382a4fb35ea6e5bab30c09
2019-05-13 18:56:56 +02:00
Pau Espin 2b89617aad osmux: Allocate CID during CRCX
Change-Id: Ie0e1835ff7e99421de9a5741a5eb57a11c004f7e
2019-05-13 18:56:56 +02:00
Pau Espin b542b0457b vty: Allow enabling Osmux
Change-Id: Ica2f82473bf1934502444be2325ee2049d938781
2019-05-13 18:56:56 +02:00
Pau Espin 120568651a cosmetic: osmux: Document network byte order in port variable
Change-Id: Ia367ef08625265bc9cbdfcc693720a9b88852f4a
2019-05-13 18:56:56 +02:00
Pau Espin f1d301a9a6 cosmetic: mgcp_udp_send: Document port param is in network byte order
Change-Id: I7c4a388eba850ac066e60db089d46da0247773ec
2019-05-13 18:56:56 +02:00
Pau Espin c9a6280c94 osmux: Use LOGPCONN in several log calls
Change-Id: Ieb2c4b53db2df44e0dfbedb7de76d8cf6c83da91
2019-05-13 13:20:13 +02:00
Pau Espin b5583cde41 osmux: Fix reception of legacy dummy payloads
Size check had a bug. Take the opportunity to print wrong frames on
error.

Change-Id: I9f0d4e28a2019c7ad94344f2c34d17c365bebea9
2019-05-13 12:59:51 +02:00
Harald Welte 3ac604e3ad handle NULL return of rate_ctr_group_alloc()
Change-Id: Ieadded9c088ef8f86164400a60ce542e3c868e9d
Related: OS#3701
2019-05-08 22:15:27 +00:00
Pau Espin 9fb8ddf00e osmux: Document func and return different rc upon osmux init failure
Change-Id: Id8593bc374b598e63a70c60ac256273b9d99ba6e
2019-05-08 16:39:13 +00:00
Pau Espin f027f17dcb osmux: Log osmux socket during osmux_init
Change-Id: I43a658b19765b1c3b3cc42f78602b793ee36c67d
2019-05-06 17:41:37 +00:00
Pau Espin ac772d8b0c mgcp_osmux.c: osmux_enable_endpoint: Fix incorrect return check
osmux_xfrm_input_open_circuit returns 0 on success and -1 on error.
Confusion comes from that function being implemented by calling
osmux_batch_add_circuit which returns NULL on error.

cherry-picked from: openbsc.git ac1b03c8e59408336d07527e2597171cb99ed654.

Change-Id: Iba018aa57901642ea4c486526a973fe6023e10cf
2019-05-06 18:02:02 +02:00
Pau Espin 8de58e79b8 osmux: Cleanup of CID alloc pool APIs
* Cleanup naming to make it far more clear
* Drop 2 variables holding CID values (allocated_cid, cid), in favour of
1 value holdinf the value and one bool stating whether the value is
used.
* Change conn_osmux_allocate_cid to allow allocating either next
available CID or a specific one, in preparation for forthcoming patches.

This commit can be merged straight away because anyway osmux cannot be
enabled in current status (blocked by vty config) and
(conn_)osmux_allocate_cid was/is not called anywhere. However, it helps
improving code base for future re-introduction of Osmux as it is
envisioned.

Change-Id: I737a248ac6c74add8e917fe2e2f36779d0f1d685
2019-04-25 21:38:38 +00:00