pespin has uploaded this change for review.

View Change

mgw: Validate CallID before updating endp->x_osmo_ign

This allows already starting to mark a clear line in CRCX handling
function between *read-only parsing + validation* and *read-write
allocation and update* of objects.

This in turn will allow further clean up on the second mentioned step.

Change-Id: Ia41f7383171bb24f48297456935c633536aa7b05
---
M src/libosmo-mgcp/mgcp_msg.c
M src/libosmo-mgcp/mgcp_protocol.c
2 files changed, 22 insertions(+), 11 deletions(-)

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

diff --git a/src/libosmo-mgcp/mgcp_msg.c b/src/libosmo-mgcp/mgcp_msg.c
index ebaa32a..2269a66 100644
--- a/src/libosmo-mgcp/mgcp_msg.c
+++ b/src/libosmo-mgcp/mgcp_msg.c
@@ -276,10 +276,6 @@
 	if (!endp)
 		return -1;
 
-	/* Accept any CallID for "X-Osmo-IGN: C" */
-	if (endp->x_osmo_ign & MGCP_X_OSMO_IGN_CALLID)
-		return 0;
-
 	if (!callid)
 		return -1;
 	if (!endp->callid)
diff --git a/src/libosmo-mgcp/mgcp_protocol.c b/src/libosmo-mgcp/mgcp_protocol.c
index ce397c8..b985a1b 100644
--- a/src/libosmo-mgcp/mgcp_protocol.c
+++ b/src/libosmo-mgcp/mgcp_protocol.c
@@ -928,12 +928,14 @@
 		}
 	}
 
-	/* 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, hpars->callid)) {
+	 * callids match up so that we are sure that this is our call.
+	 * Do check only if endpoint was (or is by current CRCX) configured
+	 * to explicitly ignore it ("X-Osmo-IGN: C").
+	 */
+	if (endp->callid &&
+	    !((endp->x_osmo_ign | hpars->x_osmo_ign) & MGCP_X_OSMO_IGN_CALLID) &&
+	    mgcp_verify_call_id(endp, hpars->callid)) {
 		LOGPENDP(endp, DLMGCP, LOGL_ERROR,
 			 "CRCX: already seized by other call (%s)\n",
 			 endp->callid);
@@ -949,6 +951,14 @@
 		}
 	}
 
+	/*******************************************************************
+	 * Allocate and update endpoint and conn.
+	 * From here on below we start updating endpoing and creating conn:
+	 *******************************************************************/
+
+	/* Update endp->x_osmo_ign: */
+	endp->x_osmo_ign |= hpars->x_osmo_ign;
+
 	if (!endp->callid) {
 		/* Claim endpoint resources. This will also set the callid,
 		 * creating additional connections will only be possible if
@@ -1122,7 +1132,11 @@
 		return create_err_response(rq->trunk, NULL, -rc, "MDCX", pdata->trans);
 	}
 
-	if (hpars->callid && mgcp_verify_call_id(endp, hpars->callid) < 0) {
+	/* 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);
 	}
@@ -1328,7 +1342,8 @@
 			rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_DLCX_FAIL_UNHANDLED_PARAM));
 			return create_err_response(rq->trunk, NULL, 539, "DLCX", pdata->trans);
 		}
-		if (mgcp_verify_call_id(endp, hpars->callid) != 0) {
+		if (!(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_DLCX_FAIL_INVALID_CALLID));
 			return create_err_response(endp, endp, 516, "DLCX", pdata->trans);
 		}

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