MGCP: Connection Identifiers are hex strings
The MGCP spec in RFC3435 is quite clear: Connection Identifiers are
hexadecimal strings of up to 32 characters. We should not print and
parse them as integers on either client or server.
Change the internal uint32_t representation of connection identifiers
to a string representation in the client and also in the server.
Closes: OS#2649
Change-Id: I0531a1b670d00cec50078423a2868207135b2436
diff --git a/src/libosmo-mgcp-client/mgcp_client.c b/src/libosmo-mgcp-client/mgcp_client.c
index a35ad6f..a89da99 100644
--- a/src/libosmo-mgcp-client/mgcp_client.c
+++ b/src/libosmo-mgcp-client/mgcp_client.c
@@ -716,7 +716,7 @@
/* Add connection id */
if (mgcp_msg->presence & MGCP_MSG_PRESENCE_CONN_ID)
- rc += msgb_printf(msg, "I: %u\r\n", mgcp_msg->conn_id);
+ 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
diff --git a/src/libosmo-mgcp/mgcp_conn.c b/src/libosmo-mgcp/mgcp_conn.c
index e07b766..31713cb 100644
--- a/src/libosmo-mgcp/mgcp_conn.c
+++ b/src/libosmo-mgcp/mgcp_conn.c
@@ -78,7 +78,7 @@
* \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,
- uint32_t id, enum mgcp_conn_type type,
+ const char *id, enum mgcp_conn_type type,
char *name)
{
struct mgcp_conn *conn;
@@ -86,6 +86,14 @@
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;
@@ -102,9 +110,9 @@
conn->type = type;
conn->mode = MGCP_CONN_NONE;
conn->mode_orig = MGCP_CONN_NONE;
- conn->id = id;
conn->u.rtp.conn = conn;
strcpy(conn->name, name);
+ osmo_strlcpy(conn->id, id, sizeof(conn->id));
switch (type) {
case MGCP_CONN_TYPE_RTP:
@@ -126,15 +134,17 @@
* \param[in] endp associated endpoint
* \param[in] id identification number of the connection
* \returns pointer to allocated connection, NULL if not found */
-struct mgcp_conn *mgcp_conn_get(struct mgcp_endpoint *endp, uint32_t id)
+struct mgcp_conn *mgcp_conn_get(struct mgcp_endpoint *endp, const char *id)
{
OSMO_ASSERT(endp);
+ OSMO_ASSERT(id);
+ OSMO_ASSERT(strlen(id) < MGCP_CONN_ID_LENGTH);
OSMO_ASSERT(endp->conns.next != NULL && endp->conns.prev != NULL);
struct mgcp_conn *conn;
llist_for_each_entry(conn, &endp->conns, entry) {
- if (conn->id == id)
+ if (strncmp(conn->id, id, sizeof(conn->id)) == 0)
return conn;
}
@@ -145,9 +155,12 @@
* \param[in] endp associated endpoint
* \param[in] id identification number of the connection
* \returns pointer to allocated connection, NULL if not found */
-struct mgcp_conn_rtp *mgcp_conn_get_rtp(struct mgcp_endpoint *endp, uint32_t id)
+struct mgcp_conn_rtp *mgcp_conn_get_rtp(struct mgcp_endpoint *endp,
+ const char *id)
{
OSMO_ASSERT(endp);
+ OSMO_ASSERT(id);
+ OSMO_ASSERT(strlen(id) < MGCP_CONN_ID_LENGTH);
OSMO_ASSERT(endp->conns.next != NULL && endp->conns.prev != NULL);
struct mgcp_conn *conn;
@@ -165,9 +178,11 @@
/*! free a connection by its ID.
* \param[in] endp associated endpoint
* \param[in] id identification number of the connection */
-void mgcp_conn_free(struct mgcp_endpoint *endp, uint32_t id)
+void mgcp_conn_free(struct mgcp_endpoint *endp, const char *id)
{
OSMO_ASSERT(endp);
+ OSMO_ASSERT(id);
+ OSMO_ASSERT(strlen(id) < MGCP_CONN_ID_LENGTH);
OSMO_ASSERT(endp->conns.next != NULL && endp->conns.prev != NULL);
struct mgcp_conn *conn;
@@ -235,7 +250,7 @@
* \returns human readble string */
char *mgcp_conn_dump(struct mgcp_conn *conn)
{
- static char str[sizeof(conn->name)+256];
+ static char str[sizeof(conn->name)+sizeof(conn->id)+256];
if (!conn) {
snprintf(str, sizeof(str), "(null connection)");
@@ -245,7 +260,7 @@
switch (conn->type) {
case MGCP_CONN_TYPE_RTP:
/* Dump RTP connection */
- snprintf(str, sizeof(str), "(%s/rtp, id:%u, ip:%s, "
+ snprintf(str, sizeof(str), "(%s/rtp, id:0x%s, ip:%s, "
"rtp:%u rtcp:%u)",
conn->name,
conn->id,
diff --git a/src/libosmo-mgcp/mgcp_msg.c b/src/libosmo-mgcp/mgcp_msg.c
index 763a5a1..9803921 100644
--- a/src/libosmo-mgcp/mgcp_msg.c
+++ b/src/libosmo-mgcp/mgcp_msg.c
@@ -330,21 +330,39 @@
* \param[in] endp pointer to endpoint
* \param{in] connection id to verify
* \returns 1 when connection id seems plausible, 0 on error */
-int mgcp_verify_ci(struct mgcp_endpoint *endp, const char *ci)
+int mgcp_verify_ci(struct mgcp_endpoint *endp, const char *conn_id)
{
- uint32_t id;
-
- if (!endp)
+ /* Check for null identifiers */
+ if (!conn_id) {
+ LOGP(DLMGCP, LOGL_ERROR,
+ "endpoint:%x invalid ConnectionIdentifier (missing)\n",
+ ENDPOINT_NUMBER(endp));
return -1;
+ }
- id = strtoul(ci, NULL, 10);
+ /* Check for empty connection identifiers */
+ if (strlen(conn_id) == 0) {
+ LOGP(DLMGCP, LOGL_ERROR,
+ "endpoint:%x invalid ConnectionIdentifier (empty)\n",
+ ENDPOINT_NUMBER(endp));
+ return -1;
+ }
- if (mgcp_conn_get(endp, id))
+ /* Check for over long connection identifiers */
+ if (strlen(conn_id) > MGCP_CONN_ID_LENGTH) {
+ LOGP(DLMGCP, LOGL_ERROR,
+ "endpoint:%x invalid ConnectionIdentifier (too long) 0x%s\n",
+ ENDPOINT_NUMBER(endp), conn_id);
+ return -1;
+ }
+
+ /* Check if connection exists */
+ if (mgcp_conn_get(endp, conn_id))
return 0;
LOGP(DLMGCP, LOGL_ERROR,
- "endpoint:%x No connection found under ConnectionIdentifier %u\n",
- ENDPOINT_NUMBER(endp), id);
+ "endpoint:%x no connection found under ConnectionIdentifier 0x%s\n",
+ ENDPOINT_NUMBER(endp), conn_id);
return -1;
}
@@ -386,20 +404,3 @@
return result;
}
-
-/*! Parse CI from a given string.
- * \param[out] caller provided memory to store the result
- * \param{in] string containing the connection id
- * \returns 0 on success, -1 on error */
-int mgcp_parse_ci(uint32_t *conn_id, const char *ci)
-{
-
- OSMO_ASSERT(conn_id);
-
- if (!ci)
- return -1;
-
- *conn_id = strtoul(ci, NULL, 10);
-
- return 0;
-}
diff --git a/src/libosmo-mgcp/mgcp_network.c b/src/libosmo-mgcp/mgcp_network.c
index d51b829..a02b0d1 100644
--- a/src/libosmo-mgcp/mgcp_network.c
+++ b/src/libosmo-mgcp/mgcp_network.c
@@ -73,11 +73,11 @@
rc = osmo_sock_local_ip(addr, inet_ntoa(conn->end.addr));
if (rc < 0)
LOGP(DRTP, LOGL_ERROR,
- "endpoint:%x CI:%i local interface auto detection failed, using configured addresses...\n",
+ "endpoint:%x CI:%s local interface auto detection failed, using configured addresses...\n",
ENDPOINT_NUMBER(endp), conn->conn->id);
else {
LOGP(DRTP, LOGL_DEBUG,
- "endpoint:%x CI:%i selected local rtp bind ip %s by probing using remote ip %s\n",
+ "endpoint:%x CI:%s selected local rtp bind ip %s by probing using remote ip %s\n",
ENDPOINT_NUMBER(endp), conn->conn->id, addr,
inet_ntoa(conn->end.addr));
return;
@@ -90,7 +90,7 @@
* if so, use that IP-Address */
strncpy(addr, endp->cfg->net_ports.bind_addr, INET_ADDRSTRLEN);
LOGP(DRTP, LOGL_DEBUG,
- "endpoint:%x CI:%i using configured rtp bind ip as local bind ip %s\n",
+ "endpoint:%x CI:%s using configured rtp bind ip as local bind ip %s\n",
ENDPOINT_NUMBER(endp), conn->conn->id, addr);
} else {
/* No specific bind IP is configured for the RTP traffic, so
@@ -98,7 +98,7 @@
* as bind IP */
strncpy(addr, endp->cfg->source_addr, INET_ADDRSTRLEN);
LOGP(DRTP, LOGL_DEBUG,
- "endpoint:%x CI:%i using mgcp bind ip as local rtp bind ip: %s\n",
+ "endpoint:%x CI:%s using mgcp bind ip as local rtp bind ip: %s\n",
ENDPOINT_NUMBER(endp), conn->conn->id, addr);
}
}
@@ -1217,7 +1217,7 @@
struct mgcp_rtp_end *end;
char local_ip_addr[INET_ADDRSTRLEN];
- snprintf(name, sizeof(name), "%s-%u", conn->conn->name, conn->conn->id);
+ snprintf(name, sizeof(name), "%s-%s", conn->conn->name, conn->conn->id);
end = &conn->end;
if (end->rtp.fd != -1 || end->rtcp.fd != -1) {
diff --git a/src/libosmo-mgcp/mgcp_osmux.c b/src/libosmo-mgcp/mgcp_osmux.c
index 09b2636..5030812 100644
--- a/src/libosmo-mgcp/mgcp_osmux.c
+++ b/src/libosmo-mgcp/mgcp_osmux.c
@@ -573,7 +573,7 @@
if (conn->osmux.state != OSMUX_STATE_ENABLED)
return;
- LOGP(DLMGCP, LOGL_INFO, "Releasing connection %u using Osmux CID %u\n",
+ LOGP(DLMGCP, LOGL_INFO, "Releasing connection %s using Osmux CID %u\n",
conn->conn->id, conn->osmux.cid);
osmux_xfrm_input_close_circuit(conn->osmux.in, conn->osmux.cid);
conn->osmux.state = OSMUX_STATE_DISABLED;
diff --git a/src/libosmo-mgcp/mgcp_protocol.c b/src/libosmo-mgcp/mgcp_protocol.c
index f542745..672a8d4 100644
--- a/src/libosmo-mgcp/mgcp_protocol.c
+++ b/src/libosmo-mgcp/mgcp_protocol.c
@@ -221,7 +221,7 @@
osmux_extension[0] = '\0';
}
- rc = msgb_printf(sdp, "I: %u%s\n\n", conn->conn->id, osmux_extension);
+ rc = msgb_printf(sdp, "I: %s%s\n\n", conn->conn->id, osmux_extension);
if (rc < 0)
goto error;
@@ -443,12 +443,11 @@
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;
struct mgcp_conn_rtp *conn = NULL;
- uint32_t conn_id;
+ const char *conn_id = NULL;
char conn_name[512];
LOGP(DLMGCP, LOGL_NOTICE, "CRCX: creating new connection ...\n");
@@ -469,7 +468,7 @@
callid = (const char *)line + 3;
break;
case 'I':
- ci = (const char *)line + 3;
+ conn_id = (const char *)line + 3;
break;
case 'M':
mode = (const char *)line + 3;
@@ -511,7 +510,7 @@
return create_err_response(endp, 400, "CRCX", p->trans);
}
- if (!ci) {
+ if (!conn_id) {
LOGP(DLMGCP, LOGL_ERROR,
"CRCX: endpoint:%x insufficient parameters, missing connection id\n",
ENDPOINT_NUMBER(endp));
@@ -561,13 +560,6 @@
set_local_cx_options(endp->tcfg->endpoints, &endp->local_options,
local_options);
- if (mgcp_parse_ci(&conn_id, ci)) {
- LOGP(DLMGCP, LOGL_ERROR,
- "CRCX: endpoint:%x insufficient parameters, missing ci (connectionIdentifier)\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,
@@ -583,7 +575,7 @@
}
}
- snprintf(conn_name, sizeof(conn_name), "%s-%u", callid, conn_id);
+ 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);
@@ -664,7 +656,7 @@
}
LOGP(DLMGCP, LOGL_DEBUG,
- "CRCX: endpoint:%x Creating connection: CI: %u port: %u\n",
+ "CRCX: endpoint:%x Creating connection: CI: %s port: %u\n",
ENDPOINT_NUMBER(endp), conn->conn->id, conn->end.local_port);
if (p->cfg->change_cb)
p->cfg->change_cb(tcfg, ENDPOINT_NUMBER(endp), MGCP_ENDP_CRCX);
@@ -695,11 +687,10 @@
int silent = 0;
int have_sdp = 0;
char *line;
- const char *ci = NULL;
const char *local_options = NULL;
const char *mode = NULL;
struct mgcp_conn_rtp *conn = NULL;
- uint32_t conn_id;
+ const char *conn_id = NULL;
LOGP(DLMGCP, LOGL_NOTICE, "MDCX: modifying existing connection ...\n");
@@ -723,8 +714,8 @@
goto error3;
break;
case 'I':
- ci = (const char *)line + 3;
- if (mgcp_verify_ci(endp, ci) != 0)
+ conn_id = (const char *)line + 3;
+ if (mgcp_verify_ci(endp, conn_id) != 0)
goto error3;
break;
case 'L':
@@ -749,7 +740,7 @@
}
mgcp_header_done:
- if (mgcp_parse_ci(&conn_id, ci)) {
+ if (!conn_id) {
LOGP(DLMGCP, LOGL_ERROR,
"MDCX: endpoint:%x insufficient parameters, missing ci (connectionIdentifier)\n",
ENDPOINT_NUMBER(endp));
@@ -849,9 +840,8 @@
int silent = 0;
char *line;
char stats[1048];
- const char *ci = NULL;
+ const char *conn_id = NULL;
struct mgcp_conn_rtp *conn = NULL;
- uint32_t conn_id;
if (p->found != 0)
return create_err_response(NULL, error_code, "DLCX", p->trans);
@@ -877,8 +867,8 @@
goto error3;
break;
case 'I':
- ci = (const char *)line + 3;
- if (mgcp_verify_ci(endp, ci) != 0)
+ conn_id = (const char *)line + 3;
+ if (mgcp_verify_ci(endp, conn_id) != 0)
goto error3;
break;
case 'Z':
@@ -919,7 +909,7 @@
/* When no connection id is supplied, we will interpret this as a
* wildcarded DLCX and drop all connections at once. (See also
* RFC3435 Section F.7) */
- if (!ci) {
+ if (!conn_id) {
LOGP(DLMGCP, LOGL_NOTICE,
"DLCX: endpoint:%x missing ci (connectionIdentifier), will remove all connections at once\n",
ENDPOINT_NUMBER(endp));
@@ -932,14 +922,6 @@
return create_ok_response(endp, 200, "DLCX", p->trans);
}
- /* Parse the connection id */
- if (mgcp_parse_ci(&conn_id, ci)) {
- LOGP(DLMGCP, LOGL_ERROR,
- "DLCX: endpoint:%x insufficient parameters, invalid ci (connectionIdentifier)\n",
- ENDPOINT_NUMBER(endp));
- return create_err_response(endp, 400, "DLCX", p->trans);
- }
-
/* Find the connection */
conn = mgcp_conn_get_rtp(endp, conn_id);
if (!conn)
diff --git a/src/libosmo-mgcp/mgcp_sdp.c b/src/libosmo-mgcp/mgcp_sdp.c
index f45d6e7..666b8c2 100644
--- a/src/libosmo-mgcp/mgcp_sdp.c
+++ b/src/libosmo-mgcp/mgcp_sdp.c
@@ -365,7 +365,7 @@
rc = msgb_printf(sdp,
"v=0\r\n"
- "o=- %u 23 IN IP4 %s\r\n"
+ "o=- %s 23 IN IP4 %s\r\n"
"s=-\r\n"
"c=IN IP4 %s\r\n"
"t=0 0\r\n", conn->conn->id, addr, addr);
diff --git a/src/libosmo-mgcp/mgcp_vty.c b/src/libosmo-mgcp/mgcp_vty.c
index 80396e0..e938391 100644
--- a/src/libosmo-mgcp/mgcp_vty.c
+++ b/src/libosmo-mgcp/mgcp_vty.c
@@ -970,7 +970,7 @@
struct mgcp_trunk_config *trunk;
struct mgcp_endpoint *endp;
struct mgcp_conn_rtp *conn;
- uint32_t conn_id;
+ const char *conn_id = NULL;
trunk = find_trunk(g_cfg, atoi(argv[0]));
if (!trunk) {
@@ -994,11 +994,11 @@
endp = &trunk->endpoints[endp_no];
- conn_id = strtoul(argv[2], NULL, 10);
+ conn_id = argv[2];
conn = mgcp_conn_get_rtp(endp, conn_id);
if (!conn) {
- vty_out(vty, "Conn ID %s/%d is invalid.%s",
- argv[2], conn_id, VTY_NEWLINE);
+ vty_out(vty, "Conn ID %s is invalid.%s",
+ conn_id, VTY_NEWLINE);
return CMD_WARNING;
}