[Asterisk-code-review] res pjsip: Split type=identify to IP address and SIP header ... (asterisk[15])

Richard Mudgett asteriskteam at digium.com
Thu Jan 11 15:24:36 CST 2018


Richard Mudgett has uploaded this change for review. ( https://gerrit.asterisk.org/7935


Change subject: res_pjsip: Split type=identify to IP address and SIP header matching priorities
......................................................................

res_pjsip: Split type=identify to IP address and SIP header matching priorities

The type=identify endpoint identification method can match by IP address
and by SIP header.  However, the SIP header matching has limited
usefulness because you cannot specify the SIP header matching priority
relative to the IP address matching.  All the matching happens at the same
priority and the order of evaluating the identify sections is
indeterminate.  e.g., If you had two type=identify sections where one
matches by IP address for endpoint alice and the other matches by SIP
header for endpoint bob then you couldn't predict which endpoint is
matched when a request comes in that matches both.

* Extract the SIP header matching criteria into its own "header" endpoint
identification method so the user can specify the relative priority of the
SIP header and the IP address matching criteria in the global
endpoint_identifier_order option.  The "ip" endpoint identification method
now only matches by IP address.

ASTERISK-27491

Change-Id: I9df142a575b7e1e3471b7cda5d3ea156cef08095
---
M CHANGES
M UPGRADE.txt
M configs/samples/pjsip.conf.sample
A contrib/ast-db-manage/config/versions/52798ad97bdf_add_pjsip_identify_by_header.py
M include/asterisk/res_pjsip.h
M res/res_pjsip.c
M res/res_pjsip/pjsip_configuration.c
M res/res_pjsip_endpoint_identifier_ip.c
8 files changed, 166 insertions(+), 31 deletions(-)



  git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/35/7935/1

diff --git a/CHANGES b/CHANGES
index 25e0a86..07bbce1 100644
--- a/CHANGES
+++ b/CHANGES
@@ -16,6 +16,27 @@
 ------------------
  * Removed the unused and incomplete SDP processing modules.
 
+res_pjsip
+------------------
+ * Users who are matching endpoints by SIP header need to reevaluate their
+   global "endpoint_identifier_order" option in light of the "ip" endpoint
+   identifier method split into the "ip" and "header" endpoint identifier
+   methods.
+
+res_pjsip_endpoint_identifier_ip
+------------------
+ * The endpoint identifier "ip" method previously recognized endpoints either
+   by IP address or a matching SIP header.  The "ip" endpoint identifier method
+   is now split into the "ip" and "header" endpoint identifier methods.  The
+   "ip" endpoint identifier method only matches by IP address and the "header"
+   endpoint identifier method only matches by SIP header.  The split allows the
+   user to control the relative priority of the IP address and the SIP header
+   identification methods in the global "endpoint_identifier_order" option.
+   e.g., If you have two type=identify sections where one matches by IP address
+   for endpoint alice and the other matches by SIP header for endpoint bob then
+   you can now predict which endpoint is matched when a request comes in that
+   matches both.
+
 res_sdp_translator_pjmedia
 ------------------
  * Removed the unused and incomplete SDP processing modules.
diff --git a/UPGRADE.txt b/UPGRADE.txt
index 30dc5d0..d47bbe3 100644
--- a/UPGRADE.txt
+++ b/UPGRADE.txt
@@ -23,6 +23,29 @@
 === UPGRADE-14.txt  -- Upgrade info for 13 to 14
 ===========================================================
 
+From 15.2.0 to 15.3.0:
+
+res_pjsip
+------------------
+ * Users who are matching endpoints by SIP header need to reevaluate their
+   global "endpoint_identifier_order" option in light of the "ip" endpoint
+   identifier method split into the "ip" and "header" endpoint identifier
+   methods.
+
+res_pjsip_endpoint_identifier_ip
+------------------
+ * The endpoint identifier "ip" method previously recognized endpoints either
+   by IP address or a matching SIP header.  The "ip" endpoint identifier method
+   is now split into the "ip" and "header" endpoint identifier methods.  The
+   "ip" endpoint identifier method only matches by IP address and the "header"
+   endpoint identifier method only matches by SIP header.  The split allows the
+   user to control the relative priority of the IP address and the SIP header
+   identification methods in the global "endpoint_identifier_order" option.
+   e.g., If you have two type=identify sections where one matches by IP address
+   for endpoint alice and the other matches by SIP header for endpoint bob then
+   you can now predict which endpoint is matched when a request comes in that
+   matches both.
+
 New in 15.0.0:
 
 Build System:
diff --git a/configs/samples/pjsip.conf.sample b/configs/samples/pjsip.conf.sample
index 8499320..a39a867 100644
--- a/configs/samples/pjsip.conf.sample
+++ b/configs/samples/pjsip.conf.sample
@@ -635,6 +635,7 @@
                         ; "username": Identify by the From or To username and domain
                         ; "auth_username": Identify by the Authorization username and realm
                         ; "ip": Identify by the source IP address
+                        ; "header": Identify by a configured SIP header value.
                         ; In the username and auth_username cases, if an exact match
                         ; on both username and domain/realm fails, the match is
                         ; retried with just the username.
@@ -993,11 +994,11 @@
             ; (default: "no")
 ;endpoint_identifier_order=ip,username,anonymous
             ; The order by which endpoint identifiers are given priority.
-            ; Currently, "ip", "username", "auth_username" and "anonymous" are valid
-            ; identifiers as registered by the res_pjsip_endpoint_identifier_* modules.
-            ; Some modules like res_pjsip_endpoint_identifier_user register more than
-            ; one identifier. Use the CLI command "pjsip show identifiers" to see the
-            ; identifiers currently available.
+            ; Currently, "ip", "header", "username", "auth_username" and "anonymous"
+            ; are valid identifiers as registered by the res_pjsip_endpoint_identifier_*
+            ; modules.  Some modules like res_pjsip_endpoint_identifier_user register
+            ; more than one identifier.  Use the CLI command "pjsip show identifiers"
+            ; to see the identifiers currently available.
             ; (default: ip,username,anonymous)
 ;max_initial_qualify_time=4 ; The maximum amount of time (in seconds) from
                             ; startup that qualifies should be attempted on all
diff --git a/contrib/ast-db-manage/config/versions/52798ad97bdf_add_pjsip_identify_by_header.py b/contrib/ast-db-manage/config/versions/52798ad97bdf_add_pjsip_identify_by_header.py
new file mode 100644
index 0000000..d67b277
--- /dev/null
+++ b/contrib/ast-db-manage/config/versions/52798ad97bdf_add_pjsip_identify_by_header.py
@@ -0,0 +1,57 @@
+"""add pjsip identify by header
+
+Revision ID: 52798ad97bdf
+Revises: 041c0d3d1857
+Create Date: 2018-01-08 12:16:02.782277
+
+"""
+
+# revision identifiers, used by Alembic.
+revision = '52798ad97bdf'
+down_revision = '041c0d3d1857'
+
+from alembic import op
+import sqlalchemy as sa
+
+
+def column_upgrade(table_name, column_name, enum_name):
+    if op.get_context().bind.dialect.name != 'postgresql':
+        if op.get_context().bind.dialect.name == 'mssql':
+            op.drop_constraint('ck_ps_endpoints_identify_by_pjsip_identify_by_values',
+                               table_name)
+        op.alter_column(table_name, column_name, type_=sa.String(80))
+        return
+
+    # Postgres requires a few more steps
+    op.execute('ALTER TABLE ' + table_name + ' ALTER COLUMN ' + column_name +
+               ' TYPE varchar(80) USING identify_by::text::' + enum_name)
+
+    op.execute('DROP TYPE ' + enum_name)
+
+
+def column_downgrade(table_name, column_name, enum_name, enum_values):
+    if op.get_context().bind.dialect.name != 'postgresql':
+        op.alter_column(table_name, column_name,
+                        type_=sa.Enum(*enum_values, name=enum_name))
+        return
+
+    # Postgres requires a few more steps
+    updated = sa.Enum(*enum_values, name=enum_name)
+    updated.create(op.get_bind(), checkfirst=False)
+
+    op.execute('ALTER TABLE ' + table_name + ' ALTER COLUMN ' + column_name +
+               ' TYPE ' + enum_name + ' USING identify_by::text::' + enum_name)
+
+
+def upgrade():
+    # The ps_endpoints identify_by column has always been a comma separated
+    # list of enum values.  This is better represented as a string anyway to
+    # avoid database compatibility issues.  Also future changes are likely
+    # to allow loadable endpoint identifier names and negating fixed enum
+    # benefits.
+    column_upgrade('ps_endpoints', 'identify_by', 'pjsip_identify_by_values')
+
+
+def downgrade():
+    column_downgrade('ps_endpoints', 'identify_by', 'pjsip_identify_by_values',
+                     ['username', 'auth_username', 'ip'])
diff --git a/include/asterisk/res_pjsip.h b/include/asterisk/res_pjsip.h
index 1c71957..330e49f 100644
--- a/include/asterisk/res_pjsip.h
+++ b/include/asterisk/res_pjsip.h
@@ -439,6 +439,8 @@
 	AST_SIP_ENDPOINT_IDENTIFY_BY_AUTH_USERNAME = (1 << 1),
 	/*! Identify based on source IP address */
 	AST_SIP_ENDPOINT_IDENTIFY_BY_IP = (1 << 2),
+	/*! Identify based on arbitrary headers */
+	AST_SIP_ENDPOINT_IDENTIFY_BY_HEADER = (1 << 3),
 };
 AST_VECTOR(ast_sip_identify_by_vector, enum ast_sip_endpoint_identifier_type);
 
diff --git a/res/res_pjsip.c b/res/res_pjsip.c
index 7988315..90a273c 100644
--- a/res/res_pjsip.c
+++ b/res/res_pjsip.c
@@ -323,6 +323,17 @@
 								endpoint identification.
 								</para>
 							</enum>
+							<enum name="header">
+								<para>Matches the endpoint based on a configured SIP header
+								value.
+								</para>
+								<para>This method of identification is not configured here
+								but simply allowed by this configuration option.  See the
+								documentation for the <literal>identify</literal>
+								configuration section for more details on this method of
+								endpoint identification.
+								</para>
+							</enum>
 						</enumlist>
 					</description>
 				</configOption>
diff --git a/res/res_pjsip/pjsip_configuration.c b/res/res_pjsip/pjsip_configuration.c
index c79877a..d1bfdfe 100644
--- a/res/res_pjsip/pjsip_configuration.c
+++ b/res/res_pjsip/pjsip_configuration.c
@@ -594,6 +594,9 @@
 	case AST_SIP_ENDPOINT_IDENTIFY_BY_IP:
 		str = "ip";
 		break;
+	case AST_SIP_ENDPOINT_IDENTIFY_BY_HEADER:
+		str = "header";
+		break;
 	}
 	return str;
 }
@@ -617,6 +620,8 @@
 		method = AST_SIP_ENDPOINT_IDENTIFY_BY_AUTH_USERNAME;
 	} else if (!strcasecmp(str, "ip")) {
 		method = AST_SIP_ENDPOINT_IDENTIFY_BY_IP;
+	} else if (!strcasecmp(str, "header")) {
+		method = AST_SIP_ENDPOINT_IDENTIFY_BY_HEADER;
 	} else {
 		method = -1;
 	}
diff --git a/res/res_pjsip_endpoint_identifier_ip.c b/res/res_pjsip_endpoint_identifier_ip.c
index a672b33..1977ab7 100644
--- a/res/res_pjsip_endpoint_identifier_ip.c
+++ b/res/res_pjsip_endpoint_identifier_ip.c
@@ -186,7 +186,7 @@
 		return 0;
 	}
 
-	return CMP_MATCH | CMP_STOP;
+	return CMP_MATCH;
 }
 
 /*! \brief Comparator function for matching an object by IP address */
@@ -201,7 +201,7 @@
 		ast_debug(3, "Source address %s matches identify '%s'\n",
 				ast_sockaddr_stringify(addr),
 				ast_sorcery_object_get_id(identify));
-		return CMP_MATCH | CMP_STOP;
+		return CMP_MATCH;
 	} else {
 		ast_debug(3, "Source address %s does not match identify '%s'\n",
 				ast_sockaddr_stringify(addr),
@@ -210,46 +210,60 @@
 	}
 }
 
-static struct ast_sip_endpoint *ip_identify(pjsip_rx_data *rdata)
+static struct ast_sip_endpoint *common_identify(ao2_callback_fn *identify_match_cb, void *arg)
 {
-	struct ast_sockaddr addr = { { 0, } };
 	RAII_VAR(struct ao2_container *, candidates, NULL, ao2_cleanup);
-	RAII_VAR(struct ip_identify_match *, match, NULL, ao2_cleanup);
+	struct ip_identify_match *match;
 	struct ast_sip_endpoint *endpoint;
 
 	/* If no possibilities exist return early to save some time */
-	if (!(candidates = ast_sorcery_retrieve_by_fields(ast_sip_get_sorcery(), "identify", AST_RETRIEVE_FLAG_MULTIPLE | AST_RETRIEVE_FLAG_ALL, NULL)) ||
-		!ao2_container_count(candidates)) {
+	candidates = ast_sorcery_retrieve_by_fields(ast_sip_get_sorcery(), "identify",
+		AST_RETRIEVE_FLAG_MULTIPLE | AST_RETRIEVE_FLAG_ALL, NULL);
+	if (!candidates || !ao2_container_count(candidates)) {
 		ast_debug(3, "No identify sections to match against\n");
 		return NULL;
 	}
 
+	match = ao2_callback(candidates, 0, identify_match_cb, arg);
+	if (!match) {
+		return NULL;
+	}
+
+	endpoint = ast_sorcery_retrieve_by_id(ast_sip_get_sorcery(), "endpoint",
+		match->endpoint_name);
+	if (endpoint) {
+		ast_debug(3, "Identify '%s' SIP message matched to endpoint %s\n",
+			ast_sorcery_object_get_id(match), match->endpoint_name);
+	} else {
+		ast_log(LOG_WARNING, "Identify '%s' points to endpoint '%s' but endpoint could not be found\n",
+			ast_sorcery_object_get_id(match), match->endpoint_name);
+	}
+
+	ao2_ref(match, -1);
+	return endpoint;
+}
+
+static struct ast_sip_endpoint *ip_identify(pjsip_rx_data *rdata)
+{
+	struct ast_sockaddr addr = { { 0, } };
+
 	ast_sockaddr_parse(&addr, rdata->pkt_info.src_name, PARSE_PORT_FORBID);
 	ast_sockaddr_set_port(&addr, rdata->pkt_info.src_port);
 
-	match = ao2_callback(candidates, 0, ip_identify_match_check, &addr);
-	if (!match) {
-		ast_debug(3, "Identify checks by IP address failed to find match: '%s' did not match any identify section rules\n",
-				ast_sockaddr_stringify(&addr));
-		match = ao2_callback(candidates, 0, header_identify_match_check, rdata);
-		if (!match) {
-			return NULL;
-		}
-	}
-
-	endpoint = ast_sorcery_retrieve_by_id(ast_sip_get_sorcery(), "endpoint", match->endpoint_name);
-	if (endpoint) {
-		ast_debug(3, "Retrieved endpoint %s\n", ast_sorcery_object_get_id(endpoint));
-	} else {
-		ast_log(LOG_WARNING, "Identify section '%s' points to endpoint '%s' but endpoint could not be looked up\n",
-				ast_sorcery_object_get_id(match), match->endpoint_name);
-	}
-
-	return endpoint;
+	return common_identify(ip_identify_match_check, &addr);
 }
 
 static struct ast_sip_endpoint_identifier ip_identifier = {
 	.identify_endpoint = ip_identify,
+};
+
+static struct ast_sip_endpoint *header_identify(pjsip_rx_data *rdata)
+{
+	return common_identify(header_identify_match_check, rdata);
+}
+
+static struct ast_sip_endpoint_identifier header_identifier = {
+	.identify_endpoint = header_identify,
 };
 
 /*! \brief Helper function which performs a host lookup and adds result to identify match */
@@ -720,6 +734,7 @@
 	ast_sorcery_load_object(ast_sip_get_sorcery(), "identify");
 
 	ast_sip_register_endpoint_identifier_with_name(&ip_identifier, "ip");
+	ast_sip_register_endpoint_identifier_with_name(&header_identifier, "header");
 	ast_sip_register_endpoint_formatter(&endpoint_identify_formatter);
 
 	cli_formatter = ao2_alloc(sizeof(struct ast_sip_cli_formatter_entry), NULL);

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

Gerrit-Project: asterisk
Gerrit-Branch: 15
Gerrit-MessageType: newchange
Gerrit-Change-Id: I9df142a575b7e1e3471b7cda5d3ea156cef08095
Gerrit-Change-Number: 7935
Gerrit-PatchSet: 1
Gerrit-Owner: Richard Mudgett <rmudgett at digium.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20180111/75dc28ad/attachment-0001.html>


More information about the asterisk-code-review mailing list