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
diff --git a/src/libosmo-mgcp-client/mgcp_client.c b/src/libosmo-mgcp-client/mgcp_client.c
index a89da99..7e83f95 100644
--- a/src/libosmo-mgcp-client/mgcp_client.c
+++ b/src/libosmo-mgcp-client/mgcp_client.c
@@ -257,6 +257,58 @@
return 0;
}
+/* Parse a line like "I: 0cedfd5a19542d197af9afe5231f1d61" */
+static int mgcp_parse_conn_id(struct mgcp_response *r, const char *line)
+{
+ if (strlen(line) < 4)
+ goto response_parse_failure;
+
+ if (memcmp("I: ", line, 3) != 0)
+ goto response_parse_failure;
+
+ osmo_strlcpy(r->head.conn_id, line + 3, sizeof(r->head.conn_id));
+ return 0;
+
+response_parse_failure:
+ LOGP(DLMGCP, LOGL_ERROR,
+ "Failed to parse MGCP response (connectionIdentifier)\n");
+ return -EINVAL;
+}
+
+/* Parse MGCP parameters of the response */
+static int parse_head_params(struct mgcp_response *r)
+{
+ char *line;
+ int rc = 0;
+ OSMO_ASSERT(r->body);
+ char *data = r->body;
+ char *data_end = strstr(r->body, "\n\n");
+
+ /* Protect SDP body, for_each_non_empty_line() will
+ * only parse until it hits \0 mark. */
+ if (data_end)
+ *data_end = '\0';
+
+ for_each_non_empty_line(line, data) {
+ switch (line[0]) {
+ case 'I':
+ rc = mgcp_parse_conn_id(r, line);
+ if (rc)
+ goto exit;
+ break;
+ default:
+ /* skip unhandled parameters */
+ break;
+ }
+ }
+exit:
+ /* Restore original state */
+ if (data_end)
+ *data_end = '\n';
+
+ return rc;
+}
+
static struct mgcp_response_pending *mgcp_client_response_pending_get(
struct mgcp_client *mgcp,
struct mgcp_response *r)
@@ -287,7 +339,13 @@
rc = mgcp_response_parse_head(&r, msg);
if (rc) {
- LOGP(DLMGCP, LOGL_ERROR, "Cannot parse MGCP response\n");
+ LOGP(DLMGCP, LOGL_ERROR, "Cannot parse MGCP response (head)\n");
+ return -1;
+ }
+
+ rc = parse_head_params(&r);
+ if (rc) {
+ LOGP(DLMGCP, LOGL_ERROR, "Cannot parse MGCP response (head parameters)\n");
return -1;
}
@@ -648,7 +706,6 @@
#define MGCP_CRCX_MANDATORY (MGCP_MSG_PRESENCE_ENDPOINT | \
MGCP_MSG_PRESENCE_CALL_ID | \
- MGCP_MSG_PRESENCE_CONN_ID | \
MGCP_MSG_PRESENCE_CONN_MODE)
#define MGCP_MDCX_MANDATORY (MGCP_MSG_PRESENCE_ENDPOINT | \
MGCP_MSG_PRESENCE_CONN_ID)
@@ -719,8 +776,7 @@
rc += msgb_printf(msg, "I: %s\r\n", mgcp_msg->conn_id);
/* Add local connection options */
- if (mgcp_msg->presence & MGCP_MSG_PRESENCE_CONN_ID
- && mgcp_msg->verb == MGCP_VERB_CRCX)
+ if (mgcp_msg->verb == MGCP_VERB_CRCX)
rc += msgb_printf(msg, "L: p:20, a:AMR, nt:IN\r\n");
/* Add mode */
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_conn.c b/src/libosmo-mgcp/mgcp_conn.c
index 31713cb..e33596d 100644
--- a/src/libosmo-mgcp/mgcp_conn.c
+++ b/src/libosmo-mgcp/mgcp_conn.c
@@ -25,6 +25,46 @@
#include <osmocom/mgcp/mgcp_internal.h>
#include <osmocom/mgcp/mgcp_common.h>
#include <osmocom/mgcp/mgcp_ep.h>
+#include <osmocom/gsm/gsm_utils.h>
+#include <ctype.h>
+
+/* Allocate a new connection identifier. According to RFC3435, they must
+ * be unique only within the scope of the endpoint. */
+static int mgcp_alloc_id(struct mgcp_endpoint *endp, char *id)
+{
+ int i;
+ int k;
+ int rc;
+ uint8_t id_bin[16];
+ char *id_hex;
+
+ /* Generate a connection id that is unique for the current endpoint.
+ * Technically a counter would be sufficient, but in order to
+ * be able to find a specific connection in large logfiles and to
+ * prevent unintentional connections we assign the connection
+ * identifiers randomly from a reasonable large number space */
+ for (i = 0; i < 32; i++) {
+ rc = osmo_get_rand_id(id_bin, sizeof(id_bin));
+ if (rc < 0)
+ return rc;
+
+ id_hex = osmo_hexdump_nospc(id_bin, sizeof(id_bin));
+ for (k = 0; k < strlen(id_hex); k++)
+ id_hex[k] = toupper(id_hex[k]);
+
+ /* ensure that the generated conn_id is unique
+ * for this endpoint */
+ if (!mgcp_conn_get_rtp(endp, id_hex)) {
+ osmo_strlcpy(id, id_hex, MGCP_CONN_ID_LENGTH);
+ return 0;
+ }
+ }
+
+ LOGP(DLMGCP, LOGL_ERROR, "endpoint:%x, unable to generate a unique connectionIdentifier\n",
+ ENDPOINT_NUMBER(endp));
+
+ return -1;
+}
/* Reset codec state and free memory */
static void mgcp_rtp_codec_reset(struct mgcp_rtp_codec *codec)
@@ -78,30 +118,19 @@
* \param[in] type connection type (e.g. MGCP_CONN_TYPE_RTP)
* \returns pointer to allocated connection, NULL on error */
struct mgcp_conn *mgcp_conn_alloc(void *ctx, struct mgcp_endpoint *endp,
- const char *id, enum mgcp_conn_type type,
- char *name)
+ enum mgcp_conn_type type, char *name)
{
struct mgcp_conn *conn;
+ int rc;
+
OSMO_ASSERT(endp);
OSMO_ASSERT(endp->conns.next != NULL && endp->conns.prev != NULL);
OSMO_ASSERT(strlen(name) < sizeof(conn->name));
- /* Id is a mandatory parameter */
- if (!id)
- return NULL;
-
- /* Prevent over long id strings */
- if (strlen(id) >= MGCP_CONN_ID_LENGTH)
- return NULL;
-
/* Do not allow more then two connections */
if (llist_count(&endp->conns) >= endp->type->max_conns)
return NULL;
- /* Prevent duplicate connection IDs */
- if (mgcp_conn_get(endp, id))
- return NULL;
-
/* Create new connection and add it to the list */
conn = talloc_zero(ctx, struct mgcp_conn);
if (!conn)
@@ -112,7 +141,11 @@
conn->mode_orig = MGCP_CONN_NONE;
conn->u.rtp.conn = conn;
strcpy(conn->name, name);
- osmo_strlcpy(conn->id, id, sizeof(conn->id));
+ rc = mgcp_alloc_id(endp, conn->id);
+ if (rc < 0) {
+ talloc_free(conn);
+ return NULL;
+ }
switch (type) {
case MGCP_CONN_TYPE_RTP:
diff --git a/src/libosmo-mgcp/mgcp_protocol.c b/src/libosmo-mgcp/mgcp_protocol.c
index 672a8d4..feca8da 100644
--- a/src/libosmo-mgcp/mgcp_protocol.c
+++ b/src/libosmo-mgcp/mgcp_protocol.c
@@ -447,7 +447,7 @@
char *line;
int have_sdp = 0, osmux_cid = -1;
struct mgcp_conn_rtp *conn = NULL;
- const char *conn_id = NULL;
+ struct mgcp_conn *_conn = NULL;
char conn_name[512];
LOGP(DLMGCP, LOGL_NOTICE, "CRCX: creating new connection ...\n");
@@ -468,7 +468,10 @@
callid = (const char *)line + 3;
break;
case 'I':
- conn_id = (const char *)line + 3;
+ /* It is illegal to send a connection identifier
+ * together with a CRCX, the MGW will assign the
+ * connection identifier by itself on CRCX */
+ return create_err_response(NULL, 523, "CRCX", p->trans);
break;
case 'M':
mode = (const char *)line + 3;
@@ -510,13 +513,6 @@
return create_err_response(endp, 400, "CRCX", p->trans);
}
- if (!conn_id) {
- 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);
- }
-
/* Check if we are able to accept the creation of another connection */
if (llist_count(&endp->conns) >= endp->type->max_conns) {
LOGP(DLMGCP, LOGL_ERROR,
@@ -560,32 +556,17 @@
set_local_cx_options(endp->tcfg->endpoints, &endp->local_options,
local_options);
- /* 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-%s", callid, conn_id);
- mgcp_conn_alloc(NULL, endp, conn_id, MGCP_CONN_TYPE_RTP,
- conn_name);
- conn = mgcp_conn_get_rtp(endp, conn_id);
- if (!conn) {
+ snprintf(conn_name, sizeof(conn_name), "%s", callid);
+ _conn = mgcp_conn_alloc(NULL, endp, MGCP_CONN_TYPE_RTP, conn_name);
+ if (!_conn) {
LOGP(DLMGCP, LOGL_ERROR,
"CRCX: endpoint:%x unable to allocate RTP connection\n",
ENDPOINT_NUMBER(endp));
goto error2;
}
+ conn = mgcp_conn_get_rtp(endp, _conn->id);
+ OSMO_ASSERT(conn);
if (mgcp_parse_conn_mode(mode, endp, conn->conn) != 0) {
error_code = 517;