pespin has uploaded this change for review.

View Change

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(-)

git pull ssh://gerrit.osmocom.org:29418/osmo-mgw refs/changes/38/39738/1

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));

To view, visit change 39738. To unsubscribe, or for help writing mail filters, visit settings.