pespin has submitted this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/39224?usp=email )
Change subject: mgw: CRCX: Split mgcp header pars parsing into a previous step ......................................................................
mgw: CRCX: Split mgcp header pars parsing into a previous step
Do most of the initial parsing and verification in a prior step, filling in a "parsed" struct which simplifies logic coming after for different message types. This commit only modifies stuff enough to work for CRCX. Further work (commits) will follow for MDCX and DLCX.
Change-Id: I3ee5158c254213203830fe9c38de11c15b4b19c1 --- M include/osmocom/mgcp/mgcp_msg.h M include/osmocom/mgcp/mgcp_protocol.h M src/libosmo-mgcp/mgcp_msg.c M src/libosmo-mgcp/mgcp_protocol.c 4 files changed, 171 insertions(+), 109 deletions(-)
Approvals: Jenkins Builder: Verified neels: Looks good to me, approved pespin: Looks good to me, approved
diff --git a/include/osmocom/mgcp/mgcp_msg.h b/include/osmocom/mgcp/mgcp_msg.h index d11fb85..631a4ae 100644 --- a/include/osmocom/mgcp/mgcp_msg.h +++ b/include/osmocom/mgcp/mgcp_msg.h @@ -39,10 +39,11 @@ enum mgcp_connection_mode mgcp_parse_conn_mode(const char *msg);
int mgcp_parse_header(struct mgcp_parse_data *pdata, char *data); +int mgcp_parse_hdr_pars(struct mgcp_parse_data *pdata);
int mgcp_parse_osmux_cid(const char *line);
-bool mgcp_check_param(const struct mgcp_endpoint *endp, struct mgcp_trunk *trunk, const char *line); +bool mgcp_check_param(const char *line);
int mgcp_verify_call_id(struct mgcp_endpoint *endp, const char *callid);
diff --git a/include/osmocom/mgcp/mgcp_protocol.h b/include/osmocom/mgcp/mgcp_protocol.h index 77f17d5..9cda8d6 100644 --- a/include/osmocom/mgcp/mgcp_protocol.h +++ b/include/osmocom/mgcp/mgcp_protocol.h @@ -3,12 +3,39 @@ #include <osmocom/core/utils.h> #include <osmocom/mgcp/mgcp_common.h>
+#define MGCP_PARSE_HDR_PARS_OSMUX_CID_UNSET (-2) +#define MGCP_PARSE_HDR_PARS_OSMUX_CID_WILDCARD (-1) + +struct mgcp_parse_hdr_pars { + const char *local_options; + const char *callid; + enum mgcp_connection_mode mode; + int remote_osmux_cid; + bool have_sdp; + /*! MGCP_X_OSMO_IGN_* flags from 'X-Osmo-IGN:' header */ + uint32_t x_osmo_ign; +}; + +static inline void mgcp_parse_hdr_pars_init(struct mgcp_parse_hdr_pars *hpars) +{ + *hpars = (struct mgcp_parse_hdr_pars){ + .local_options = NULL, + .callid = NULL, + .mode = MGCP_CONN_NONE, + .remote_osmux_cid = MGCP_PARSE_HDR_PARS_OSMUX_CID_UNSET, + .have_sdp = false, + .x_osmo_ign = 0, + }; +} + /* Internal structure while parsing a request */ struct mgcp_parse_data { struct mgcp_config *cfg; + char *save; + /* MGCP Header: */ char *epname; char *trans; - char *save; + struct mgcp_parse_hdr_pars hpars; };
/* Local connection options */ diff --git a/src/libosmo-mgcp/mgcp_msg.c b/src/libosmo-mgcp/mgcp_msg.c index f8ef50b..bcbe258 100644 --- a/src/libosmo-mgcp/mgcp_msg.c +++ b/src/libosmo-mgcp/mgcp_msg.c @@ -23,6 +23,7 @@ */
#include <limits.h> +#include <ctype.h>
#include <osmocom/mgcp/mgcp.h> #include <osmocom/mgcp/osmux.h> @@ -33,6 +34,10 @@ #include <osmocom/mgcp/mgcp_endp.h> #include <osmocom/mgcp/mgcp_trunk.h>
+/* (same fmt as LOGPENDP()) */ +#define LOG_MGCP_PDATA(PDATA, LEVEL, FMT, ARGS...) \ + LOGP(DLMGCP, LEVEL, "endpoint:%s " FMT, (PDATA) ? ((PDATA)->epname ? : "null-epname") : "null-pdata", ##ARGS) + /*! Display an mgcp message on the log output. * \param[in] message mgcp message string * \param[in] len message mgcp message string length @@ -122,8 +127,7 @@ break; case 2: if (strcasecmp("MGCP", elem)) { - LOGP(DLMGCP, LOGL_ERROR, - "MGCP header parsing error\n"); + LOG_MGCP_PDATA(pdata, LOGL_ERROR, "MGCP header parsing error\n"); return -510; } break; @@ -136,13 +140,89 @@ }
if (i != 4) { - LOGP(DLMGCP, LOGL_ERROR, "MGCP status line too short.\n"); + LOG_MGCP_PDATA(pdata, LOGL_ERROR, "MGCP status line too short.\n"); return -510; }
return 0; }
+static bool parse_x_osmo_ign(struct mgcp_parse_data *pdata, char *line) +{ + char *saveptr = NULL; + + if (strncasecmp(line, MGCP_X_OSMO_IGN_HEADER, strlen(MGCP_X_OSMO_IGN_HEADER))) + return false; + line += strlen(MGCP_X_OSMO_IGN_HEADER); + + while (1) { + char *token = strtok_r(line, " ", &saveptr); + line = NULL; + if (!token) + break; + + if (!strcasecmp(token, "C")) + pdata->hpars.x_osmo_ign |= MGCP_X_OSMO_IGN_CALLID; + else + LOG_MGCP_PDATA(pdata, LOGL_ERROR, "received unknown X-Osmo-IGN item '%s'\n", token); + } + + return true; +} + +/*! Analyze and parse the the header of an MGCP message string. + * \param[inout] pdata caller provided memory to store the parsing results. + * \returns 0 when parsing was successful, negative (MGCP cause code) on error. */ +int mgcp_parse_hdr_pars(struct mgcp_parse_data *pdata) +{ + struct mgcp_parse_hdr_pars *hp = &pdata->hpars; + char *line; + + mgcp_parse_hdr_pars_init(hp); + + for_each_line(line, pdata->save) { + if (!mgcp_check_param(line)) { + LOG_MGCP_PDATA(pdata, LOGL_NOTICE, "wrong MGCP option format: '%s'\n", line); + continue; + } + + switch (toupper(line[0])) { + case 'L': + hp->local_options = (const char *)line + 3; + break; + case 'C': + hp->callid = (const char *)line + 3; + break; + case 'I': + /* It is illegal to send a connection identifier + * together with a CRCX, the MGW will assign the + * connection identifier by itself on CRCX */ + return -523; + case 'M': + hp->mode = mgcp_parse_conn_mode((const char *)line + 3); + break; + case 'X': + if (strncasecmp("Osmux: ", line + 2, strlen("Osmux: ")) == 0) { + hp->remote_osmux_cid = mgcp_parse_osmux_cid(line); + break; + } + if (parse_x_osmo_ign(pdata, line)) + break; + /* Ignore unknown X-headers */ + break; + case '\0': + hp->have_sdp = true; + goto mgcp_header_done; + default: + LOG_MGCP_PDATA(pdata, LOGL_NOTICE, "CRCX: unhandled option: '%c'/%d\n", *line, *line); + return -539; + } + } + +mgcp_header_done: + return 0; +} + /*! Extract OSMUX CID from an MGCP parameter line (string). * \param[in] line single parameter line from the MGCP message * \returns OSMUX CID, -1 wildcard, -2 on error */ @@ -153,19 +233,19 @@
if (strcasecmp(line + 2, "Osmux: *") == 0) { LOGP(DLMGCP, LOGL_DEBUG, "Parsed wilcard Osmux CID\n"); - return -1; + return MGCP_PARSE_HDR_PARS_OSMUX_CID_WILDCARD; }
if (sscanf(line + 2 + 7, "%u", &osmux_cid) != 1) { LOGP(DLMGCP, LOGL_ERROR, "Failed parsing Osmux in MGCP msg line: %s\n", line); - return -2; + return MGCP_PARSE_HDR_PARS_OSMUX_CID_UNSET; }
if (osmux_cid > OSMUX_CID_MAX) { LOGP(DLMGCP, LOGL_ERROR, "Osmux ID too large: %u > %u\n", osmux_cid, OSMUX_CID_MAX); - return -2; + return MGCP_PARSE_HDR_PARS_OSMUX_CID_UNSET; } LOGP(DLMGCP, LOGL_DEBUG, "MGCP client offered Osmux CID %u\n", osmux_cid);
@@ -173,20 +253,13 @@ }
/*! Check MGCP parameter line (string) for plausibility. - * \param[in] endp pointer to endpoint (only used for log output, may be NULL) - * \param[in] trunk pointer to trunk (only used for log output, may be NULL if endp is not NULL) * \param[in] line single parameter line from the MGCP message * \returns true when line seems plausible, false on error */ -bool mgcp_check_param(const struct mgcp_endpoint *endp, struct mgcp_trunk *trunk, const char *line) +bool mgcp_check_param(const char *line) { const size_t line_len = strlen(line); - if (line[0] != '\0' && line_len < 2) { - if (endp) - LOGPENDP(endp, DLMGCP, LOGL_NOTICE, "wrong MGCP option format: '%s'\n", line); - else - LOGPTRUNK(trunk, DLMGCP, LOGL_NOTICE, "wrong MGCP option format: '%s'\n", line); + if (line[0] != '\0' && line_len < 2) return false; - }
/* FIXME: A couple more checks wouldn't hurt... */
diff --git a/src/libosmo-mgcp/mgcp_protocol.c b/src/libosmo-mgcp/mgcp_protocol.c index 32fbd43..b185435 100644 --- a/src/libosmo-mgcp/mgcp_protocol.c +++ b/src/libosmo-mgcp/mgcp_protocol.c @@ -702,6 +702,20 @@ codec->frame_duration_den; }
+static int mgcp_trunk_osmux_init_if_needed(struct mgcp_trunk *trunk) +{ + if (trunk->cfg->osmux.initialized) + return 0; + + if (osmux_init(trunk) < 0) { + LOGPTRUNK(trunk, DOSMUX, LOGL_ERROR, "Cannot init OSMUX\n"); + return -3; + } + LOGPTRUNK(trunk, DOSMUX, LOGL_NOTICE, "OSMUX socket has been set up\n"); + + return 0; +} + /*! Initializes osmux socket if not yet initialized. Parses Osmux CID from MGCP line. * \param[in] endp Endpoint willing to initialize osmux * \param[in] line Line X-Osmux from MGCP header msg to parse @@ -709,13 +723,8 @@ */ static int mgcp_osmux_setup(struct mgcp_endpoint *endp, const char *line) { - if (!endp->trunk->cfg->osmux.initialized) { - if (osmux_init(endp->trunk) < 0) { - LOGPENDP(endp, DOSMUX, LOGL_ERROR, "Cannot init OSMUX\n"); - return -3; - } - LOGPENDP(endp, DOSMUX, LOGL_NOTICE, "OSMUX socket has been set up\n"); - } + if (mgcp_trunk_osmux_init_if_needed(endp->trunk) < 0) + return -3;
return mgcp_parse_osmux_cid(line); } @@ -791,42 +800,16 @@ return 534; }
-static bool parse_x_osmo_ign(struct mgcp_endpoint *endp, char *line) -{ - char *saveptr = NULL; - - if (strncasecmp(line, MGCP_X_OSMO_IGN_HEADER, strlen(MGCP_X_OSMO_IGN_HEADER))) - return false; - line += strlen(MGCP_X_OSMO_IGN_HEADER); - - while (1) { - char *token = strtok_r(line, " ", &saveptr); - line = NULL; - if (!token) - break; - - if (!strcasecmp(token, "C")) - endp->x_osmo_ign |= MGCP_X_OSMO_IGN_CALLID; - else - LOGPENDP(endp, DLMGCP, LOGL_ERROR, "received unknown X-Osmo-IGN item '%s'\n", token); - } - - return true; -} - /* CRCX command handler, processes the received command */ static struct msgb *handle_create_con(struct mgcp_request_data *rq) { struct mgcp_parse_data *pdata = rq->pdata; struct mgcp_trunk *trunk = rq->trunk; struct mgcp_endpoint *endp = rq->endp; + struct mgcp_parse_hdr_pars *hpars = &pdata->hpars; struct rate_ctr_group *rate_ctrs; int error_code = 400; - const char *local_options = NULL; - const char *callid = NULL; - enum mgcp_connection_mode mode = MGCP_CONN_NONE; - char *line; - int have_sdp = 0, remote_osmux_cid = -2; + int remote_osmux_cid; struct mgcp_conn *conn = NULL; struct mgcp_conn_rtp *conn_rtp = NULL; char conn_name[512]; @@ -857,69 +840,44 @@ }
/* parse CallID C: and LocalParameters L: */ - for_each_line(line, pdata->save) { - if (!mgcp_check_param(endp, trunk, line)) - continue; - - switch (toupper(line[0])) { - case 'L': - local_options = (const char *)line + 3; - break; - case 'C': - callid = (const char *)line + 3; - break; - case 'I': - /* It is illegal to send a connection identifier - * together with a CRCX, the MGW will assign the - * connection identifier by itself on CRCX */ - rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_CRCX_FAIL_BAD_ACTION)); - return create_err_response(rq->trunk, NULL, 523, "CRCX", pdata->trans); - break; - case 'M': - mode = mgcp_parse_conn_mode((const char *)line + 3); - break; - case 'X': - if (strncasecmp("Osmux: ", line + 2, strlen("Osmux: ")) == 0) { - /* If osmux is disabled, just skip setting it up */ - if (rq->endp->trunk->cfg->osmux.usage == OSMUX_USAGE_OFF) - break; - remote_osmux_cid = mgcp_osmux_setup(endp, line); - break; - } - - if (parse_x_osmo_ign(endp, line)) - break; - - /* Ignore unknown X-headers */ - break; - case '\0': - have_sdp = 1; - goto mgcp_header_done; - default: - LOGPENDP(endp, DLMGCP, LOGL_NOTICE, - "CRCX: unhandled option: '%c'/%d\n", *line, *line); - rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_CRCX_FAIL_UNHANDLED_PARAM)); - return create_err_response(rq->trunk, NULL, 539, "CRCX", pdata->trans); - break; - } + rc = mgcp_parse_hdr_pars(pdata); + switch (rc) { + case 0: + break; /* all good, continue below */ + case -523: + rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_CRCX_FAIL_BAD_ACTION)); + return create_err_response(rq->trunk, NULL, -rc, "CRCX", pdata->trans); + case -539: + rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_CRCX_FAIL_UNHANDLED_PARAM)); + return create_err_response(rq->trunk, NULL, -rc, "CRCX", pdata->trans); + default: + return create_err_response(rq->trunk, NULL, -rc, "CRCX", pdata->trans); }
-mgcp_header_done: /* Check parameters */ - if (!callid) { + if (!hpars->callid) { LOGPENDP(endp, DLMGCP, LOGL_ERROR, "CRCX: insufficient parameters, missing callid\n"); rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_CRCX_FAIL_MISSING_CALLID)); return create_err_response(endp, endp, 516, "CRCX", pdata->trans); }
- if (mode == MGCP_CONN_NONE) { + if (hpars->mode == MGCP_CONN_NONE) { LOGPENDP(endp, DLMGCP, LOGL_ERROR, "CRCX: insufficient parameters, invalid mode\n"); rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_CRCX_FAIL_INVALID_MODE)); return create_err_response(endp, endp, 517, "CRCX", pdata->trans); }
+ remote_osmux_cid = hpars->remote_osmux_cid; + /* If osmux is disabled, just skip setting it up */ + if (trunk->cfg->osmux.usage == OSMUX_USAGE_OFF) + remote_osmux_cid = MGCP_PARSE_HDR_PARS_OSMUX_CID_UNSET; + + /* Make sure osmux is setup: */ + if (remote_osmux_cid != MGCP_PARSE_HDR_PARS_OSMUX_CID_UNSET) + mgcp_trunk_osmux_init_if_needed(trunk); + /* Check if we are able to accept the creation of another connection */ if (mgcp_endp_is_full(endp)) { LOGPENDP(endp, DLMGCP, LOGL_ERROR, @@ -939,9 +897,12 @@ } }
+ /* Update endp->x_osmo_ign: */ + endp->x_osmo_ign |= hpars->x_osmo_ign; + /* Check if this endpoint already serves a call, if so, check if the * callids match up so that we are sure that this is our call */ - if (endp->callid && mgcp_verify_call_id(endp, callid)) { + if (endp->callid && mgcp_verify_call_id(endp, hpars->callid)) { LOGPENDP(endp, DLMGCP, LOGL_ERROR, "CRCX: already seized by other call (%s)\n", endp->callid); @@ -961,14 +922,14 @@ /* Claim endpoint resources. This will also set the callid, * creating additional connections will only be possible if * the callid matches up (see above). */ - rc = mgcp_endp_claim(endp, callid); + rc = mgcp_endp_claim(endp, hpars->callid); if (rc != 0) { rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_CRCX_FAIL_CLAIM)); return create_err_response(endp, endp, 502, "CRCX", pdata->trans); } }
- snprintf(conn_name, sizeof(conn_name), "%s", callid); + snprintf(conn_name, sizeof(conn_name), "%s", hpars->callid); conn = mgcp_conn_alloc(trunk->endpoints, endp, MGCP_CONN_TYPE_RTP, conn_name); if (!conn) { LOGPENDP(endp, DLMGCP, LOGL_ERROR, @@ -977,7 +938,7 @@ goto error2; }
- if (mgcp_conn_set_mode(conn, mode) < 0) { + if (mgcp_conn_set_mode(conn, hpars->mode) < 0) { error_code = 517; rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_CRCX_FAIL_INVALID_MODE)); goto error2; @@ -1004,9 +965,9 @@ }
/* Set local connection options, if present */ - if (local_options) { + if (hpars->local_options) { rc = set_local_cx_options(trunk->endpoints, - &endp->local_options, local_options); + &endp->local_options, hpars->local_options); if (rc != 0) { LOGPCONN(conn, DLMGCP, LOGL_ERROR, "CRCX: invalid local connection options!\n"); @@ -1017,7 +978,7 @@ }
/* Handle codec information and decide for a suitable codec */ - rc = handle_codec_info(conn_rtp, rq, have_sdp, true); + rc = handle_codec_info(conn_rtp, rq, hpars->have_sdp, true); mgcp_codecset_summary(&conn_rtp->end.cset, mgcp_conn_dump(conn)); if (rc) { error_code = rc; @@ -1131,7 +1092,7 @@ }
for_each_line(line, pdata->save) { - if (!mgcp_check_param(endp, trunk, line)) + if (!mgcp_check_param(line)) continue;
switch (toupper(line[0])) { @@ -1357,7 +1318,7 @@ }
for_each_line(line, pdata->save) { - if (!mgcp_check_param(endp, trunk, line)) + if (!mgcp_check_param(line)) continue;
switch (toupper(line[0])) {