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

Jenkins2 asteriskteam at digium.com
Wed Jan 17 11:42:04 CST 2018


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

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-15.txt
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
9 files changed, 170 insertions(+), 47 deletions(-)

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



diff --git a/CHANGES b/CHANGES
index ec533e8..a05345c 100644
--- a/CHANGES
+++ b/CHANGES
@@ -57,6 +57,31 @@
    built.
 
 ------------------------------------------------------------------------------
+--- Functionality changes from Asterisk 15.2.0 to Asterisk 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.
+
+------------------------------------------------------------------------------
 --- Functionality changes from Asterisk 15.1.0 to Asterisk 15.2.0 ------------
 ------------------------------------------------------------------------------
 
diff --git a/UPGRADE-15.txt b/UPGRADE-15.txt
index 30dc5d0..d47bbe3 100644
--- a/UPGRADE-15.txt
+++ b/UPGRADE-15.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/UPGRADE.txt b/UPGRADE.txt
index b2c75fb..8f45742 100644
--- a/UPGRADE.txt
+++ b/UPGRADE.txt
@@ -43,19 +43,3 @@
 cdr_syslog:
  - The cdr_syslog module is now deprecated and by default it is no longer
    built.
-
-New in 15.0.0:
-
-Build System:
- - '--with-pjproject-bundled' is now the default when running ./configure
-   It can be disabled with '--without-pjproject-bundled'.
-
-Core:
- - Multi-stream support has been added so a channel can have multiple
-   streams of the same type such as audio and video.
-
- - The 'Data Retrieval API' has been removed. This API was not actively
-   maintained, was not added to new modules (such as res_pjsip), and there
-   exist better alternatives to acquire the same information, such as the
-   ARI. As a result, the 'DataGet' AMI action as well as the 'data get'
-   CLI command have been removed.
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..ab5939c
--- /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: e2f04d309071
+Create Date: 2018-01-08 12:16:02.782277
+
+"""
+
+# revision identifiers, used by Alembic.
+revision = '52798ad97bdf'
+down_revision = 'e2f04d309071'
+
+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 6c48d2e..66b99b8 100644
--- a/include/asterisk/res_pjsip.h
+++ b/include/asterisk/res_pjsip.h
@@ -443,6 +443,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 3f9574d..07fc980 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 58e4624..ee47e4d 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/7936
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I9df142a575b7e1e3471b7cda5d3ea156cef08095
Gerrit-Change-Number: 7936
Gerrit-PatchSet: 2
Gerrit-Owner: Richard Mudgett <rmudgett at digium.com>
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Jenkins2
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20180117/8f0afe21/attachment-0001.html>


More information about the asterisk-code-review mailing list