pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-mgw/+/39239?usp=email ) Change subject: mgw: MDCX: Split mgcp header pars parsing into a previous step ...................................................................... mgw: MDCX: 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 MDCX. Further work (commits) will follow for DLCX. Change-Id: I6ecb41fc2cc737c3a161b6bc98bd08ae01909655 --- M include/osmocom/mgcp/mgcp_protocol.h M src/libosmo-mgcp/mgcp_msg.c M src/libosmo-mgcp/mgcp_protocol.c 3 files changed, 45 insertions(+), 90 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-mgw refs/changes/39/39239/1 diff --git a/include/osmocom/mgcp/mgcp_protocol.h b/include/osmocom/mgcp/mgcp_protocol.h index 633f3e9..ffe7ae0 100644 --- a/include/osmocom/mgcp/mgcp_protocol.h +++ b/include/osmocom/mgcp/mgcp_protocol.h @@ -9,6 +9,7 @@ struct mgcp_parse_hdr_pars { const char *local_options; const char *callid; + const char *connid; enum mgcp_connection_mode mode; int remote_osmux_cid; bool have_sdp; @@ -21,6 +22,7 @@ *hpars = (struct mgcp_parse_hdr_pars){ .local_options = NULL, .callid = NULL, + .connid = NULL, .mode = MGCP_CONN_NONE, .remote_osmux_cid = MGCP_PARSE_HDR_PARS_OSMUX_CID_UNSET, .have_sdp = false, diff --git a/src/libosmo-mgcp/mgcp_msg.c b/src/libosmo-mgcp/mgcp_msg.c index 493c473..577914f 100644 --- a/src/libosmo-mgcp/mgcp_msg.c +++ b/src/libosmo-mgcp/mgcp_msg.c @@ -189,10 +189,8 @@ 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; + hp->connid = (const char *)line + 3; + break; case 'M': hp->mode = mgcp_parse_conn_mode((const char *)line + 3); break; diff --git a/src/libosmo-mgcp/mgcp_protocol.c b/src/libosmo-mgcp/mgcp_protocol.c index d592cc9..3751bc1 100644 --- a/src/libosmo-mgcp/mgcp_protocol.c +++ b/src/libosmo-mgcp/mgcp_protocol.c @@ -716,19 +716,6 @@ 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 - * \returns OSMUX CID, -1 for wildcard, -2 on parse error, -3 on osmux initalize error - */ -static int mgcp_osmux_setup(struct mgcp_endpoint *endp, const char *line) -{ - if (mgcp_trunk_osmux_init_if_needed(endp->trunk) < 0) - return -3; - - return mgcp_parse_osmux_cid(line); -} - /* Process codec information contained in CRCX/MDCX */ static int handle_codec_info(struct mgcp_conn_rtp *conn, struct mgcp_request_data *rq, int have_sdp, bool crcx) @@ -843,9 +830,6 @@ 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, 539, "CRCX", pdata->trans); @@ -868,6 +852,15 @@ return create_err_response(endp, endp, 517, "CRCX", pdata->trans); } + /* It is illegal to send a connection identifier + * together with a CRCX, the MGW will assign the + * connection identifier by itself on CRCX */ + if (hpars->connid) { + LOGPENDP(endp, DLMGCP, LOGL_ERROR, "CRCX: 'I: %s' not expected!\n", hpars->connid); + rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_CRCX_FAIL_BAD_ACTION)); + return create_err_response(endp, endp, 523, "CRCX", pdata->trans); + } + /* If osmux is disabled, just skip setting it up */ if (trunk->cfg->osmux.usage == OSMUX_USAGE_OFF) hpars->remote_osmux_cid = MGCP_PARSE_HDR_PARS_OSMUX_CID_UNSET; @@ -1046,17 +1039,12 @@ 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; char new_local_addr[INET6_ADDRSTRLEN]; int error_code = 500; - int have_sdp = 0; - char *line; - const char *local_options = NULL; - enum mgcp_connection_mode mode = MGCP_CONN_NONE; struct mgcp_conn *conn = NULL; struct mgcp_conn_rtp *conn_rtp = NULL; - const char *conn_id = NULL; - int remote_osmux_cid = -2; int rc; LOGPENDP(endp, DLMGCP, LOGL_NOTICE, "MDCX: modifying existing connection ...\n"); @@ -1089,64 +1077,33 @@ return create_err_response(endp, endp, 400, "MDCX", pdata->trans); } - for_each_line(line, pdata->save) { - if (!mgcp_check_param(line)) - continue; - - switch (toupper(line[0])) { - case 'C': - if (mgcp_verify_call_id(endp, line + 3) != 0) { - rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_MDCX_FAIL_INVALID_CALLID)); - error_code = 516; - goto error3; - } - break; - case 'I': - conn_id = (const char *)line + 3; - if ((error_code = mgcp_verify_ci(endp, conn_id))) { - rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_MDCX_FAIL_INVALID_CONNID)); - goto error3; - } - break; - case 'L': - local_options = (const char *)line + 3; - 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 (endp->trunk->cfg->osmux.usage == OSMUX_USAGE_OFF) - break; - remote_osmux_cid = mgcp_osmux_setup(endp, line); - break; - } - /* Ignore unknown X-headers */ - break; - case '\0': - have_sdp = 1; - goto mgcp_header_done; - break; - default: - LOGPENDP(endp, DLMGCP, LOGL_NOTICE, - "MDCX: Unhandled MGCP option: '%c'/%d\n", - line[0], line[0]); - rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_MDCX_FAIL_UNHANDLED_PARAM)); - return create_err_response(rq->trunk, NULL, 539, "MDCX", pdata->trans); - break; - } + rc = mgcp_parse_hdr_pars(pdata); + switch (rc) { + case 0: + break; /* all good, continue below */ + case 539: + rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_CRCX_FAIL_UNHANDLED_PARAM)); + return create_err_response(rq->trunk, NULL, 539, "MDCX", pdata->trans); + default: + return create_err_response(rq->trunk, NULL, rc, "MDCX", pdata->trans); } -mgcp_header_done: - if (!conn_id) { + if (hpars->callid && mgcp_verify_call_id(endp, hpars->callid) < 0) { + rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_MDCX_FAIL_INVALID_CALLID)); + return create_err_response(endp, endp, 516, "MDCX", pdata->trans); + } + + if (!hpars->connid) { LOGPENDP(endp, DLMGCP, LOGL_ERROR, "MDCX: insufficient parameters, missing ci (connectionIdentifier)\n"); rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_MDCX_FAIL_NO_CONNID)); return create_err_response(endp, endp, 515, "MDCX", pdata->trans); + } else if ((error_code = mgcp_verify_ci(endp, hpars->connid)) != 0) { + rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_MDCX_FAIL_INVALID_CONNID)); + return create_err_response(endp, endp, error_code, "MDCX", pdata->trans); } - conn = mgcp_endp_get_conn(endp, conn_id); + conn = mgcp_endp_get_conn(endp, hpars->connid); if (!conn) { rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_MDCX_FAIL_CONN_NOT_FOUND)); return create_err_response(endp, endp, 400, "MDCX", pdata->trans); @@ -1154,20 +1111,18 @@ mgcp_conn_watchdog_kick(conn); - if (mode != MGCP_CONN_NONE) { - if (mgcp_conn_set_mode(conn, mode) < 0) { - rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_MDCX_FAIL_INVALID_MODE)); - error_code = 517; - goto error3; - } - } else { + if (hpars->mode == MGCP_CONN_NONE) { + /* Reset conn mode in case it was tweaked through VTY: */ conn->mode = conn->mode_orig; + } else if (mgcp_conn_set_mode(conn, hpars->mode) < 0) { + rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_MDCX_FAIL_INVALID_MODE)); + return create_err_response(endp, endp, 517, "MDCX", pdata->trans); } /* 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, "MDCX: invalid local connection options!\n"); @@ -1181,7 +1136,7 @@ OSMO_ASSERT(conn_rtp); /* Handle codec information and decide for a suitable codec */ - rc = handle_codec_info(conn_rtp, rq, have_sdp, false); + rc = handle_codec_info(conn_rtp, rq, hpars->have_sdp, false); mgcp_codecset_summary(&conn_rtp->end.cset, mgcp_conn_dump(conn)); if (rc) { error_code = rc; @@ -1195,22 +1150,22 @@ if (mgcp_conn_rtp_is_osmux(conn_rtp)) { OSMO_ASSERT(conn_rtp->osmux.local_cid_allocated); - if (remote_osmux_cid < -1) { + if (hpars->remote_osmux_cid < -1) { LOGPCONN(conn, DLMGCP, LOGL_ERROR, "MDCX: Failed to parse Osmux CID!\n"); goto error3; - } else if (remote_osmux_cid == -1) { + } else if (hpars->remote_osmux_cid == -1) { LOGPCONN(conn, DLMGCP, LOGL_ERROR, "MDCX: wilcard in MDCX is not supported!\n"); goto error3; } else if (conn_rtp->osmux.remote_cid_present && - remote_osmux_cid != conn_rtp->osmux.remote_cid) { + hpars->remote_osmux_cid != conn_rtp->osmux.remote_cid) { LOGPCONN(conn, DLMGCP, LOGL_ERROR, "MDCX: changing already allocated CID is not supported!\n"); goto error3; } else { conn_rtp->osmux.remote_cid_present = true; - conn_rtp->osmux.remote_cid = remote_osmux_cid; + conn_rtp->osmux.remote_cid = hpars->remote_osmux_cid; if (conn_osmux_event_rx_crcx_mdcx(conn_rtp) < 0) { LOGPCONN(conn, DLMGCP, LOGL_ERROR, "MDCX: Osmux handling failed!\n"); -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/39239?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: newchange Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: I6ecb41fc2cc737c3a161b6bc98bd08ae01909655 Gerrit-Change-Number: 39239 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin@sysmocom.de>