pespin has submitted this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/39738?usp=email )
Change subject: mgw: Split MDCX read-only validations into its own function ......................................................................
mgw: Split MDCX read-only validations into its own function
The SDP parsing is moved to a step beforehand, so that validation of full message can be done in a subsequent step. That validation step is moved into a function to give some air to handle_modify_con() and easily spot the logic.
Change-Id: I469413dbe89cc4795deb50d76b434655cba9b776 --- M src/libosmo-mgcp/mgcp_protocol.c 1 file changed, 47 insertions(+), 26 deletions(-)
Approvals: osmith: Looks good to me, but someone else must approve Jenkins Builder: Verified laforge: Looks good to me, but someone else must approve pespin: Looks good to me, approved
diff --git a/src/libosmo-mgcp/mgcp_protocol.c b/src/libosmo-mgcp/mgcp_protocol.c index ef0aad4..79a647a 100644 --- a/src/libosmo-mgcp/mgcp_protocol.c +++ b/src/libosmo-mgcp/mgcp_protocol.c @@ -849,6 +849,48 @@ return create_err_response(endp, endp, error_code, "CRCX", pdata->trans); }
+/* Read-only checks for parsed MDCX request. + * Returns negative MGCP error code on failure, 0 on scucess. */ +static int validate_parsed_mdcx(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 = trunk->ratectr.mgcp_mdcx_ctr_group; + int error_code; + + if (mgcp_endp_num_conns(endp) <= 0) { + LOGPENDP(endp, DLMGCP, LOGL_ERROR, + "MDCX: endpoint is not holding a connection.\n"); + rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_MDCX_FAIL_NO_CONN)); + return -400; + } + + /* If a CallID is provided during MDCX, validate (unless endp was explicitly configured to ignore + * it through "X-Osmo-IGN: C") that it matches the one previously set. */ + if (hpars->callid && + !(endp->x_osmo_ign & MGCP_X_OSMO_IGN_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 -516; + } + + 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 -515; + } + 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 -error_code; + } + + /* Everything fine, continue */ + return 0; +} + /* MDCX command handler, processes the received command */ static struct msgb *handle_modify_con(struct mgcp_request_data *rq) { @@ -870,7 +912,7 @@ LOGP(DLMGCP, LOGL_ERROR, "MDCX: Not allowed in 'null' endpoint!\n"); return create_err_response(rq->cfg, NULL, 502, "MDCX", pdata->trans); } - + /* rq->trunk is available (non-null) from here on. */ rate_ctrs = trunk->ratectr.mgcp_mdcx_ctr_group;
/* Prohibit wildcarded requests */ @@ -886,36 +928,11 @@ LOGPENDP(endp, DLMGCP, LOGL_ERROR, "MDCX: selected endpoint not available!\n"); return create_err_response(rq->trunk, NULL, 501, "MDCX", pdata->trans); } - if (mgcp_endp_num_conns(endp) <= 0) { - LOGPENDP(endp, DLMGCP, LOGL_ERROR, - "MDCX: endpoint is not holding a connection.\n"); - rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_MDCX_FAIL_NO_CONN)); - return create_err_response(endp, endp, 400, "MDCX", pdata->trans); - }
rc = mgcp_parse_hdr_pars(pdata); if (rc < 0) return create_err_response(rq->trunk, NULL, -rc, "MDCX", pdata->trans);
- /* If a CallID is provided during MDCX, validate (unless endp was explicitly configured to ignore it - * through "X-Osmo-IGN: C") that it matches the one previously set. */ - if (hpars->callid && - !(endp->x_osmo_ign & MGCP_X_OSMO_IGN_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); - } - /* Parse SDP if found: */ if (hpars->have_sdp) { rc = mgcp_parse_sdp_data(pdata); @@ -926,6 +943,10 @@ } }
+ rc = validate_parsed_mdcx(rq); + if (rc < 0) + return create_err_response(rq->trunk, NULL, -rc, "MDCX", pdata->trans); + 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));