[Asterisk-code-review] pjsip message filter: Fix regression causing bad contact add... (asterisk[master])

[Jenkins2]

Jenkins2 has submitted this change and it was merged. ( https://gerrit.asterisk.org/6610 )

Change subject: pjsip_message_filter: Fix regression causing bad contact address
......................................................................

pjsip_message_filter: Fix regression causing bad contact address

The "res_pjsip:  Filter out non SIP(S) requests" commit moved the
filtering of messages to pjproject's PJSIP_MOD_PRIORITY_TRANSPORT_LAYER
in order to filter out incoming bad uri schemes as early as possible.
Since the change affected outgoing messages as well and the TRANSPORT
layer is the last to be run on outgoing messages, we were overwriting
the setting of external_signaling_address (which is set earlier by
res_pjsip_nat) with an internal address.

* pjsip_message_filter now registers itself as a pjproject module
twice.  Once in the TSX layer for the outgoing messages (as it was
originally), then a second time in the TRANSPORT layer for the
incoming messages to catch the invalid uri schemes.

ASTERISK-27295
Reported by: Sean Bright

Change-Id: I2c90190c43370f8a9d1c4693a19fd65840689c8c
---
M res/res_pjsip/pjsip_message_filter.c
1 file changed, 23 insertions(+), 9 deletions(-)

Approvals:
  Joshua Colp: Looks good to me, but someone else must approve
  Kevin Harwell: Looks good to me, approved
  Jenkins2: Approved for Submit



diff --git a/res/res_pjsip/pjsip_message_filter.c b/res/res_pjsip/pjsip_message_filter.c
index 63b8bd5..d2f9b95 100644
--- a/res/res_pjsip/pjsip_message_filter.c
+++ b/res/res_pjsip/pjsip_message_filter.c
@@ -36,13 +36,19 @@
 	unsigned int disallow_from_domain_modification;
 };
 
-static pjsip_module filter_module = {
-	.name = { "Message Filtering", 17 },
+static pjsip_module filter_module_transport = {
+	.name = { "Message Filtering Transport", 27 },
 	.id = -1,
 	.priority = PJSIP_MOD_PRIORITY_TRANSPORT_LAYER,
+	.on_rx_request = filter_on_rx_message,
+};
+
+static pjsip_module filter_module_tsx = {
+	.name = { "Message Filtering TSX", 21 },
+	.id = -1,
+	.priority = PJSIP_MOD_PRIORITY_TSX_LAYER - 1,
 	.on_tx_request = filter_on_tx_message,
 	.on_tx_response = filter_on_tx_message,
-	.on_rx_request = filter_on_rx_message,
 };
 
 /*! \brief Helper function to get (or allocate if not already present) restrictions on a message */
@@ -50,13 +56,13 @@
 {
 	struct filter_message_restrictions *restrictions;
 
-	restrictions = ast_sip_mod_data_get(tdata->mod_data, filter_module.id, MOD_DATA_RESTRICTIONS);
+	restrictions = ast_sip_mod_data_get(tdata->mod_data, filter_module_tsx.id, MOD_DATA_RESTRICTIONS);
 	if (restrictions) {
 		return restrictions;
 	}
 
 	restrictions = PJ_POOL_ALLOC_T(tdata->pool, struct filter_message_restrictions);
-	ast_sip_mod_data_set(tdata->pool, tdata->mod_data, filter_module.id, MOD_DATA_RESTRICTIONS, restrictions);
+	ast_sip_mod_data_set(tdata->pool, tdata->mod_data, filter_module_tsx.id, MOD_DATA_RESTRICTIONS, restrictions);
 
 	return restrictions;
 }
@@ -217,7 +223,8 @@
 
 static pj_status_t filter_on_tx_message(pjsip_tx_data *tdata)
 {
-	struct filter_message_restrictions *restrictions = ast_sip_mod_data_get(tdata->mod_data, filter_module.id, MOD_DATA_RESTRICTIONS);
+	struct filter_message_restrictions *restrictions =
+		ast_sip_mod_data_get(tdata->mod_data, filter_module_transport.id, MOD_DATA_RESTRICTIONS);
 	pjsip_tpmgr_fla2_param prm;
 	pjsip_cseq_hdr *cseq;
 	pjsip_via_hdr *via;
@@ -277,7 +284,7 @@
 			/* prm.ret_addr is allocated from the tdata pool OR the transport so it is perfectly fine to just do an assignment like this */
 			pj_strassign(&uri->host, &prm.ret_addr);
 			uri->port = prm.ret_port;
-			ast_debug(4, "Re-wrote Contact URI host/port to %.*s:%d\n",
+			ast_debug(5, "Re-wrote Contact URI host/port to %.*s:%d (this may be re-written again later)\n",
 				(int)pj_strlen(&uri->host), pj_strbuf(&uri->host), uri->port);
 
 			if (tdata->tp_info.transport->key.type == PJSIP_TRANSPORT_UDP ||
@@ -498,7 +505,8 @@
 
 void ast_res_pjsip_cleanup_message_filter(void)
 {
-	ast_sip_unregister_service(&filter_module);
+	ast_sip_unregister_service(&filter_module_tsx);
+	ast_sip_unregister_service(&filter_module_transport);
 	ast_sip_unregister_supplement(&filter_supplement);
 	ast_sip_session_unregister_supplement(&filter_session_supplement);
 }
@@ -516,7 +524,13 @@
 		return -1;
 	}
 
-	if (ast_sip_register_service(&filter_module)) {
+	if (ast_sip_register_service(&filter_module_transport)) {
+		ast_log(LOG_ERROR, "Could not register message filter module for incoming and outgoing requests\n");
+		ast_res_pjsip_cleanup_message_filter();
+		return -1;
+	}
+
+	if (ast_sip_register_service(&filter_module_tsx)) {
 		ast_log(LOG_ERROR, "Could not register message filter module for incoming and outgoing requests\n");
 		ast_res_pjsip_cleanup_message_filter();
 		return -1;

-- 
To view, visit https://gerrit.asterisk.org/6610
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I2c90190c43370f8a9d1c4693a19fd65840689c8c
Gerrit-Change-Number: 6610
Gerrit-PatchSet: 2
Gerrit-Owner: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Jenkins2
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20170928/742033a3/attachment-0001.html>