[Asterisk-code-review] res_pjsip_nat: Restore original contact for REGISTER responses (asterisk[17])

Friendly Automation asteriskteam at digium.com
Mon Dec 16 10:28:07 CST 2019


Friendly Automation has submitted this change. ( https://gerrit.asterisk.org/c/asterisk/+/12826 )

Change subject: res_pjsip_nat: Restore original contact for REGISTER responses
......................................................................

res_pjsip_nat: Restore original contact for REGISTER responses

RFC3261 Section 10 "Registrations", specifically paragraph
"10.2.4: Refreshing Bindings", states that a user agent compares
each contact address (in a 200 REGISTER response) to see if it
created the contact.  If the Asterisk endpoint has the
rewrite_contact option set however, the contact host and port sent
back in the 200 response will be the rewritten one and not the
one sent by the user agent.  This prevents the user agent from
matching its own contact.  Some user agents get very upset when
this happens and will not consider the registration successful.
While this is rare, it is acceptable behavior especially if more
than 1 user agent is allowed to register to a single endpoint/aor.

This commit updates res_pjsip_nat (where rewrite_contact is
implemented) to store the original incoming Contact header in
a new "x-ast-orig-host" URI parameter before rewriting it, and to
restore the original host and port to the Contact headers in the
outgoing response.

This is only done if the request is a REGISTER and rewrite_contact
is enabled.

pjsip_message_filter was also updated to ensure that if a request
comes in with any existing x-ast-* URI parameters, we remove them
so they don't conflict.  Asterisk will never send a request
with those headers in it but someone might just decide to add them
to a request they craft and send to Asterisk.

NOTE: If a device changes its contact address and registers again,
it's a NEW registration.  If the device didn't unregister the
original registration then all existing behavior based
on aor/remove_existing and aor/max_contacts apply.

ASTERISK-28502
Reported-by: Ross Beer

Change-Id: Idc263ad2d2d7bd8faa047e5804d96a5fe1cd282e
---
M res/res_pjsip/pjsip_message_filter.c
M res/res_pjsip_nat.c
2 files changed, 122 insertions(+), 2 deletions(-)

Approvals:
  Joshua C. Colp: Looks good to me, but someone else must approve
  George Joseph: Looks good to me, approved
  Friendly Automation: Approved for Submit



diff --git a/res/res_pjsip/pjsip_message_filter.c b/res/res_pjsip/pjsip_message_filter.c
index b70985b..1826ee2 100644
--- a/res/res_pjsip/pjsip_message_filter.c
+++ b/res/res_pjsip/pjsip_message_filter.c
@@ -400,6 +400,40 @@
 #endif
 }
 
+/*!
+ * /internal
+ *
+ * We want to make sure that any incoming requests don't already
+ * have x-ast-* parameters in any URIs or we may get confused
+ * if symmetric transport (x-ast-txp) or rewrite_contact (x-ast-orig-host)
+ * is used later on.
+ */
+static void remove_x_ast_params(pjsip_uri *header_uri){
+	pjsip_sip_uri *uri;
+	pjsip_param *param;
+
+	if (!header_uri) {
+		return;
+	}
+
+	uri = pjsip_uri_get_uri(header_uri);
+	if (!uri) {
+		return;
+	}
+
+	param = uri->other_param.next;
+
+	while (param != &uri->other_param) {
+		/* We need to save off 'next' because pj_list_erase will remove it */
+		pjsip_param *next = param->next;
+
+		if (pj_strncmp2(&param->name, "x-ast-", 6) == 0) {
+			pj_list_erase(param);
+		}
+		param = next;
+	}
+}
+
 static pj_bool_t on_rx_process_uris(pjsip_rx_data *rdata)
 {
 	pjsip_contact_hdr *contact = NULL;
@@ -414,6 +448,7 @@
 			PJSIP_SC_UNSUPPORTED_URI_SCHEME, NULL, NULL, NULL);
 		return PJ_TRUE;
 	}
+	remove_x_ast_params(rdata->msg_info.msg->line.req.uri);
 
 	if (!is_sip_uri(rdata->msg_info.from->uri)) {
 		print_uri_debug(URI_TYPE_FROM, rdata, (pjsip_hdr *)rdata->msg_info.from);
@@ -421,6 +456,7 @@
 			PJSIP_SC_UNSUPPORTED_URI_SCHEME, NULL, NULL, NULL);
 		return PJ_TRUE;
 	}
+	remove_x_ast_params(rdata->msg_info.from->uri);
 
 	if (!is_sip_uri(rdata->msg_info.to->uri)) {
 		print_uri_debug(URI_TYPE_TO, rdata, (pjsip_hdr *)rdata->msg_info.to);
@@ -428,7 +464,7 @@
 			PJSIP_SC_UNSUPPORTED_URI_SCHEME, NULL, NULL, NULL);
 		return PJ_TRUE;
 	}
-
+	remove_x_ast_params(rdata->msg_info.to->uri);
 
 	contact = (pjsip_contact_hdr *) pjsip_msg_find_hdr(
 		rdata->msg_info.msg, PJSIP_H_CONTACT, NULL);
@@ -448,6 +484,8 @@
 				PJSIP_SC_UNSUPPORTED_URI_SCHEME, NULL, NULL, NULL);
 			return PJ_TRUE;
 		}
+		remove_x_ast_params(contact->uri);
+
 		contact = (pjsip_contact_hdr *) pjsip_msg_find_hdr(
 			rdata->msg_info.msg, PJSIP_H_CONTACT, contact->next);
 	}
diff --git a/res/res_pjsip_nat.c b/res/res_pjsip_nat.c
index bbf3359..dc7d10b 100644
--- a/res/res_pjsip_nat.c
+++ b/res/res_pjsip_nat.c
@@ -32,8 +32,43 @@
 #include "asterisk/module.h"
 #include "asterisk/acl.h"
 
+/*! URI parameter for original host/port */
+#define AST_SIP_X_AST_ORIG_HOST "x-ast-orig-host"
+#define AST_SIP_X_AST_ORIG_HOST_LEN 15
+
+static void save_orig_contact_host(pjsip_rx_data *rdata, pjsip_sip_uri *uri)
+{
+	pjsip_param *x_orig_host;
+	pj_str_t p_value;
+#define COLON_LEN 1
+#define MAX_PORT_LEN 5
+
+	if (rdata->msg_info.msg->type != PJSIP_REQUEST_MSG ||
+		rdata->msg_info.msg->line.req.method.id != PJSIP_REGISTER_METHOD) {
+		return;
+	}
+
+	ast_debug(1, "Saving contact '%.*s:%d'\n",
+		(int)uri->host.slen, uri->host.ptr, uri->port);
+
+	x_orig_host = PJ_POOL_ALLOC_T(rdata->tp_info.pool, pjsip_param);
+	x_orig_host->name = pj_strdup3(rdata->tp_info.pool, AST_SIP_X_AST_ORIG_HOST);
+	p_value.slen = pj_strlen(&uri->host) + COLON_LEN + MAX_PORT_LEN;
+	p_value.ptr = (char*)pj_pool_alloc(rdata->tp_info.pool, p_value.slen + 1);
+	p_value.slen = snprintf(p_value.ptr, p_value.slen + 1, "%.*s:%d", (int)uri->host.slen, uri->host.ptr, uri->port);
+	pj_strassign(&x_orig_host->value, &p_value);
+	pj_list_insert_before(&uri->other_param, x_orig_host);
+
+	return;
+}
+
 static void rewrite_uri(pjsip_rx_data *rdata, pjsip_sip_uri *uri)
 {
+
+	if (pj_strcmp2(&uri->host, rdata->pkt_info.src_name) != 0) {
+		save_orig_contact_host(rdata, uri);
+	}
+
 	pj_cstr(&uri->host, rdata->pkt_info.src_name);
 	uri->port = rdata->pkt_info.src_port;
 	if (!strcasecmp("WSS", rdata->tp_info.transport->type_name)) {
@@ -264,7 +299,44 @@
 	return 0;
 }
 
-static pj_status_t nat_on_tx_message(pjsip_tx_data *tdata)
+static void restore_orig_contact_host(pjsip_tx_data *tdata)
+{
+	pjsip_contact_hdr *contact;
+
+	if (tdata->msg->type != PJSIP_RESPONSE_MSG) {
+		return;
+	}
+
+	contact = pjsip_msg_find_hdr(tdata->msg, PJSIP_H_CONTACT, NULL);
+	while (contact) {
+		pjsip_sip_uri *contact_uri = pjsip_uri_get_uri(contact->uri);
+		pj_str_t x_name = { AST_SIP_X_AST_ORIG_HOST, AST_SIP_X_AST_ORIG_HOST_LEN };
+		pjsip_param *x_orig_host = pjsip_param_find(&contact_uri->other_param, &x_name);
+
+		if (x_orig_host) {
+			char host_port[x_orig_host->value.slen + 1];
+			char *sep;
+
+			ast_debug(1, "Restoring contact %.*s:%d  to %.*s\n", (int)contact_uri->host.slen,
+				contact_uri->host.ptr, contact_uri->port,
+				(int)x_orig_host->value.slen, x_orig_host->value.ptr);
+
+			strncpy(host_port, x_orig_host->value.ptr, x_orig_host->value.slen);
+			host_port[x_orig_host->value.slen] = '\0';
+			sep = strchr(host_port, ':');
+			if (sep) {
+				*sep = '\0';
+				sep++;
+				pj_strdup2(tdata->pool, &contact_uri->host, host_port);
+				contact_uri->port = strtol(sep, NULL, 10);
+			}
+			pj_list_erase(x_orig_host);
+		}
+		contact = pjsip_msg_find_hdr(tdata->msg, PJSIP_H_CONTACT, contact->next);
+	}
+}
+
+static pj_status_t process_nat(pjsip_tx_data *tdata)
 {
 	RAII_VAR(struct ao2_container *, transport_states, NULL, ao2_cleanup);
 	RAII_VAR(struct ast_sip_transport *, transport, NULL, ao2_cleanup);
@@ -364,6 +436,16 @@
 	return PJ_SUCCESS;
 }
 
+static pj_status_t nat_on_tx_message(pjsip_tx_data *tdata) {
+	pj_status_t rc;
+
+	rc = process_nat(tdata);
+	restore_orig_contact_host(tdata);
+
+	return rc;
+}
+
+
 static pjsip_module nat_module = {
 	.name = { "NAT", 3 },
 	.id = -1,

-- 
To view, visit https://gerrit.asterisk.org/c/asterisk/+/12826
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: 17
Gerrit-Change-Id: Idc263ad2d2d7bd8faa047e5804d96a5fe1cd282e
Gerrit-Change-Number: 12826
Gerrit-PatchSet: 10
Gerrit-Owner: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Joshua C. Colp <jcolp at sangoma.com>
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-MessageType: merged
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20191216/e3806873/attachment.html>


More information about the asterisk-code-review mailing list