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

9 files changed, 165 insertions, 42 deletions

diff --git a/CHANGES b/CHANGES
index ec533e831..a05345c1a 100644
--- a/CHANGES
+++ b/CHANGES
@@ -57,6 +57,31 @@ cdr_syslog
    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 30dc5d04c..d47bbe38a 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 b2c75fbb5..8f45742af 100644
--- a/UPGRADE.txt
+++ b/UPGRADE.txt
@@ -43,19 +43,3 @@ ARI:
 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 849932069..a39a8675a 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 000000000..ab5939cfe
--- /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 6c48d2e84..66b99b8f7 100644
--- a/include/asterisk/res_pjsip.h
+++ b/include/asterisk/res_pjsip.h
@@ -443,6 +443,8 @@ enum ast_sip_endpoint_identifier_type {
 	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 3f9574d2a..07fc980ef 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 c79877a85..d1bfdfe01 100644
--- a/res/res_pjsip/pjsip_configuration.c
+++ b/res/res_pjsip/pjsip_configuration.c
@@ -594,6 +594,9 @@ static const char *sip_endpoint_identifier_type2str(enum ast_sip_endpoint_identi
 	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 @@ static int sip_endpoint_identifier_str2type(const char *str)
 		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 58e462460..ee47e4d4e 100644
--- a/res/res_pjsip_endpoint_identifier_ip.c
+++ b/res/res_pjsip_endpoint_identifier_ip.c
@@ -186,7 +186,7 @@ static int header_identify_match_check(void *obj, void *arg, int flags)
 		return 0;
 	}
 
-	return CMP_MATCH | CMP_STOP;
+	return CMP_MATCH;
 }
 
 /*! \brief Comparator function for matching an object by IP address */
@@ -201,7 +201,7 @@ static int ip_identify_match_check(void *obj, void *arg, int flags)
 		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,48 +210,62 @@ static int ip_identify_match_check(void *obj, void *arg, int flags)
 	}
 }
 
-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;
 	}
 
-	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);
+	match = ao2_callback(candidates, 0, identify_match_cb, arg);
 	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;
-		}
+		return NULL;
 	}
 
-	endpoint = ast_sorcery_retrieve_by_id(ast_sip_get_sorcery(), "endpoint", match->endpoint_name);
+	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));
+		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 section '%s' points to endpoint '%s' but endpoint could not be looked up\n",
-				ast_sorcery_object_get_id(match), match->endpoint_name);
+		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);
+
+	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 */
 static int ip_identify_match_host_lookup(struct ip_identify_match *identify, const char *host)
 {
@@ -720,6 +734,7 @@ static int load_module(void)
 	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);