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

[George Joseph]

George Joseph has uploaded this change for review. ( https://gerrit.asterisk.org/6607


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

The change thoughThis affected both incoming and outgoing messages

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



  git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/07/6607/1

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/6607
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-MessageType: newchange
Gerrit-Change-Id: I2c90190c43370f8a9d1c4693a19fd65840689c8c
Gerrit-Change-Number: 6607
Gerrit-PatchSet: 1
Gerrit-Owner: George Joseph <gjoseph at digium.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20170926/7808aa09/attachment-0001.html>