Review at https://gerrit.osmocom.org/4905
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
---
M configure.ac
M src/libosmo-mgcp/Makefile.am
M src/libosmo-mgcp/mgcp_protocol.c
3 files changed, 33 insertions(+), 28 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-mgw refs/changes/05/4905/1
diff --git a/configure.ac b/configure.ac
index dea41b1..752d224 100644
--- a/configure.ac
+++ b/configure.ac
@@ -40,6 +40,7 @@
PKG_CHECK_MODULES(LIBOSMOCORE, libosmocore >= 0.10.0)
+PKG_CHECK_MODULES(LIBOSMOGSM, libosmogsm >= 0.10.0)
PKG_CHECK_MODULES(LIBOSMOVTY, libosmovty >= 0.10.0)
PKG_CHECK_MODULES(LIBOSMONETIF, libosmo-netif >= 0.1.0)
diff --git a/src/libosmo-mgcp/Makefile.am b/src/libosmo-mgcp/Makefile.am
index fce0e1b..a785d62 100644
--- a/src/libosmo-mgcp/Makefile.am
+++ b/src/libosmo-mgcp/Makefile.am
@@ -7,6 +7,7 @@
AM_CFLAGS = \
-Wall \
$(LIBOSMOCORE_CFLAGS) \
+ $(LIBOSMOGSM_CFLAGS) \
$(LIBOSMOVTY_CFLAGS) \
$(LIBOSMONETIF_CFLAGS) \
$(COVERAGE_CFLAGS) \
@@ -14,6 +15,7 @@
AM_LDFLAGS = \
$(LIBOSMOCORE_LIBS) \
+ $(LIBOSMOGSM_LIBS) \
$(LIBOSMOVTY_LIBS) \
$(LIBOSMONETIF_LIBS) \
$(COVERAGE_LDFLAGS) \
diff --git a/src/libosmo-mgcp/mgcp_protocol.c b/src/libosmo-mgcp/mgcp_protocol.c
index 9e04e50..2bf8847 100644
--- a/src/libosmo-mgcp/mgcp_protocol.c
+++ b/src/libosmo-mgcp/mgcp_protocol.c
@@ -32,6 +32,7 @@
#include <osmocom/core/msgb.h>
#include <osmocom/core/talloc.h>
#include <osmocom/core/select.h>
+#include <osmocom/gsm/gsm_utils.h>
#include <osmocom/mgcp/mgcp.h>
#include <osmocom/mgcp/mgcp_common.h>
@@ -434,6 +435,31 @@
return mgcp_parse_osmux_cid(line);
}
+/* Allocate a new connection identifier. According to RFC3435, they must
+ * be unique only within the scope of the endpoint. */
+static int mgcp_alloc_conn_id(struct mgcp_endpoint *endp)
+{
+ int i;
+
+ for (i = 0; i < 32; i++) {
+ uint32_t conn_id;
+ int rc;
+
+ rc = osmo_get_rand_id((uint8_t *) &conn_id, 4);
+ if (rc < 0)
+ return rc;
+
+ /* to distinguish it from negative responses */
+ conn_id &= 0x7fffffff;
+
+ /* Only accept connection ID != current */
+ if (!mgcp_conn_get_rtp(endp, conn_id))
+ return conn_id;
+ }
+
+ return -1;
+}
+
/* CRCX command handler, processes the received command */
static struct msgb *handle_create_con(struct mgcp_parse_data *p)
{
@@ -443,7 +469,6 @@
const char *local_options = NULL;
const char *callid = NULL;
- const char *ci = NULL;
const char *mode = NULL;
char *line;
int have_sdp = 0, osmux_cid = -1;
@@ -467,9 +492,6 @@
break;
case 'C':
callid = (const char *)line + 3;
- break;
- case 'I':
- ci = (const char *)line + 3;
break;
case 'M':
mode = (const char *)line + 3;
@@ -507,13 +529,6 @@
if (!mode) {
LOGP(DLMGCP, LOGL_ERROR,
"CRCX: endpoint:%x insufficient parameters, missing
mode\n",
- ENDPOINT_NUMBER(endp));
- return create_err_response(endp, 400, "CRCX", p->trans);
- }
-
- if (!ci) {
- LOGP(DLMGCP, LOGL_ERROR,
- "CRCX: endpoint:%x insufficient parameters, missing
connection id\n",
ENDPOINT_NUMBER(endp));
return create_err_response(endp, 400, "CRCX", p->trans);
}
@@ -561,26 +576,13 @@
set_local_cx_options(endp->tcfg->endpoints, &endp->local_options,
local_options);
- if (mgcp_parse_ci(&conn_id, ci)) {
+ /* Generate a new Connection Identifier */
+ conn_id = mgcp_alloc_conn_id(endp);
+ if (conn_id < 0) {
LOGP(DLMGCP, LOGL_ERROR,
- "CRCX: endpoint:%x insufficient parameters, missing ci
(connectionIdentifier)\n",
+ "CRCX: endpoint:%x cannot allocate connection id\n",
ENDPOINT_NUMBER(endp));
return create_err_response(endp, 400, "CRCX", p->trans);
- }
-
- /* Only accept another connection when the connection ID is different.
*/
- if (mgcp_conn_get_rtp(endp, conn_id)) {
- LOGP(DLMGCP, LOGL_ERROR,
- "CRCX: endpoint:%x there is already a connection with id
%u present!\n",
- conn_id, ENDPOINT_NUMBER(endp));
- if (tcfg->force_realloc) {
- /* Ignore the existing connection by just freeing it */
- mgcp_conn_free(endp, conn_id);
- } else {
- /* There is already a connection with that ID present,
- * leave everything as it is and return with an error.
*/
- return create_err_response(endp, 400, "CRCX", p->trans);
- }
}
snprintf(conn_name, sizeof(conn_name), "%s-%u", callid, conn_id);
--
To view, visit https://gerrit.osmocom.org/4905
To unsubscribe, visit https://gerrit.osmocom.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Iab6a6038e7610c62f34e642cd49c93d11151252c
Gerrit-PatchSet: 1
Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-Owner: Harald Welte <[email protected]>
