<p>Jenkins2 <strong>merged</strong> this change.</p><p><a href="https://gerrit.asterisk.org/7846">View Change</a></p><div style="white-space:pre-wrap">Approvals:
Corey Farrell: Looks good to me, but someone else must approve
Joshua Colp: Looks good to me, approved
Jenkins2: Approved for Submit
</div><pre style="font-family: monospace,monospace; white-space: pre-wrap;">res_pjsip_endpoint_identifier_ip.c: Fix apply identify validation.<br><br>The ip_identify_apply() did not validate the configuration for simple<br>static configuration errors or deal well with address resolution errors.<br><br>* Added missing configuration validation checks.<br>* Fixed address resolution error handling.<br>* Demoted an error message to a warning since it does not fail applying<br>the identify object configuration.<br><br>Change-Id: I8b519607263fe88e8ce964f526a45359fd362b6e<br>---<br>M res/res_pjsip_endpoint_identifier_ip.c<br>1 file changed, 45 insertions(+), 18 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">diff --git a/res/res_pjsip_endpoint_identifier_ip.c b/res/res_pjsip_endpoint_identifier_ip.c<br>index 8b92cef..e40f9bf 100644<br>--- a/res/res_pjsip_endpoint_identifier_ip.c<br>+++ b/res/res_pjsip_endpoint_identifier_ip.c<br>@@ -105,7 +105,7 @@<br> struct ast_ha *matches;<br> /*! \brief Perform SRV resolution of hostnames */<br> unsigned int srv_lookups;<br>- /*! \brief Hosts to be resolved after applying configuration */<br>+ /*! \brief Hosts to be resolved when applying configuration */<br> struct ao2_container *hosts;<br> };<br> <br>@@ -150,8 +150,8 @@<br> c_header = ast_strdupa(identify->match_header);<br> c_value = strchr(c_header, ':');<br> if (!c_value) {<br>- ast_log(LOG_WARNING, "Identify '%s' has invalid header_match: No ':' separator found!\n",<br>- ast_sorcery_object_get_id(identify));<br>+ /* This should not be possible. The object cannot be created if so. */<br>+ ast_assert(0);<br> return 0;<br> }<br> *c_value = '\0';<br>@@ -161,17 +161,19 @@<br> pj_header_name = pj_str(c_header);<br> header = pjsip_msg_find_hdr_by_name(rdata->msg_info.msg, &pj_header_name, NULL);<br> if (!header) {<br>- ast_debug(3, "SIP message does not contain header '%s'\n", c_header);<br>+ ast_debug(3, "Identify '%s': SIP message does not have header '%s'\n",<br>+ ast_sorcery_object_get_id(identify),<br>+ c_header);<br> return 0;<br> }<br> <br> pj_header_value = pj_str(c_value);<br> if (pj_strcmp(&pj_header_value, &header->hvalue)) {<br>- ast_debug(3, "SIP message contains header '%s' but value '%.*s' does not match value '%s' for endpoint '%s'\n",<br>+ ast_debug(3, "Identify '%s': SIP message has header '%s' but value '%.*s' does not match '%s'\n",<br>+ ast_sorcery_object_get_id(identify),<br> c_header,<br> (int) pj_strlen(&header->hvalue), pj_strbuf(&header->hvalue),<br>- c_value,<br>- identify->endpoint_name);<br>+ c_value);<br> return 0;<br> }<br> <br>@@ -261,7 +263,7 @@<br> }<br> <br> for (i = 0; i < num_addrs; ++i) {<br>- /* Check if the address is already in the list, if so don't bother adding it again */<br>+ /* Check if the address is already in the list, if so don't add it again */<br> if (identify->matches && (ast_apply_ha(identify->matches, &addrs[i]) != AST_SENSE_ALLOW)) {<br> continue;<br> }<br>@@ -283,14 +285,13 @@<br> }<br> <br> /*! \brief Helper function which performs an SRV lookup and then resolves the hostname */<br>-static int ip_identify_match_srv_lookup(struct ip_identify_match *identify, const char *prefix, const char *host)<br>+static int ip_identify_match_srv_lookup(struct ip_identify_match *identify, const char *prefix, const char *host, int results)<br> {<br> char service[NI_MAXHOST];<br> struct srv_context *context = NULL;<br> int srv_ret;<br> const char *srvhost;<br> unsigned short srvport;<br>- int results = 0;<br> <br> snprintf(service, sizeof(service), "%s.%s", prefix, host);<br> <br>@@ -372,10 +373,33 @@<br> char *current_string;<br> struct ao2_iterator i;<br> <br>+ /* Validate the identify object configuration */<br>+ if (ast_strlen_zero(identify->endpoint_name)) {<br>+ ast_log(LOG_ERROR, "Identify '%s' missing required endpoint name.\n",<br>+ ast_sorcery_object_get_id(identify));<br>+ return -1;<br>+ }<br>+ if (ast_strlen_zero(identify->match_header) /* No header to match */<br>+ /* and no static IP addresses with a mask */<br>+ && !identify->matches<br>+ /* and no addresses to resolve */<br>+ && (!identify->hosts || !ao2_container_count(identify->hosts))) {<br>+ ast_log(LOG_ERROR, "Identify '%s' is not configured to match anything.\n",<br>+ ast_sorcery_object_get_id(identify));<br>+ return -1;<br>+ }<br>+ if (!ast_strlen_zero(identify->match_header)<br>+ && !strchr(identify->match_header, ':')) {<br>+ ast_log(LOG_ERROR, "Identify '%s' missing ':' separator in match_header '%s'.\n",<br>+ ast_sorcery_object_get_id(identify), identify->match_header);<br>+ return -1;<br>+ }<br>+<br> if (!identify->hosts) {<br> return 0;<br> }<br> <br>+ /* Resolve the match addresses now */<br> i = ao2_iterator_init(identify->hosts, 0);<br> while ((current_string = ao2_iterator_next(&i))) {<br> struct ast_sockaddr address;<br>@@ -383,26 +407,29 @@<br> <br> /* If the provided string is not an IP address perform SRV resolution on it */<br> if (identify->srv_lookups && !ast_sockaddr_parse(&address, current_string, 0)) {<br>- results = ip_identify_match_srv_lookup(identify, "_sip._udp", current_string);<br>+ results = ip_identify_match_srv_lookup(identify, "_sip._udp", current_string,<br>+ results);<br> if (results != -1) {<br>- results += ip_identify_match_srv_lookup(identify, "_sip._tcp", current_string);<br>+ results = ip_identify_match_srv_lookup(identify, "_sip._tcp",<br>+ current_string, results);<br> }<br> if (results != -1) {<br>- results += ip_identify_match_srv_lookup(identify, "_sips._tcp", current_string);<br>+ results = ip_identify_match_srv_lookup(identify, "_sips._tcp",<br>+ current_string, results);<br> }<br> }<br> <br>- /* If SRV falls fall back to a normal lookup on the host itself */<br>+ /* If SRV fails fall back to a normal lookup on the host itself */<br> if (!results) {<br> results = ip_identify_match_host_lookup(identify, current_string);<br> }<br> <br> if (results == 0) {<br>- ast_log(LOG_ERROR, "Address '%s' provided on ip endpoint identifier '%s' did not resolve to any address\n",<br>- current_string, ast_sorcery_object_get_id(obj));<br>+ ast_log(LOG_WARNING, "Identify '%s' provided address '%s' did not resolve to any address\n",<br>+ ast_sorcery_object_get_id(identify), current_string);<br> } else if (results == -1) {<br>- ast_log(LOG_ERROR, "An error occurred when adding resolution results of '%s' on '%s'\n",<br>- current_string, ast_sorcery_object_get_id(obj));<br>+ ast_log(LOG_ERROR, "Identify '%s' failed when adding resolution results of '%s'\n",<br>+ ast_sorcery_object_get_id(identify), current_string);<br> ao2_ref(current_string, -1);<br> ao2_iterator_destroy(&i);<br> return -1;<br></pre><p>To view, visit <a href="https://gerrit.asterisk.org/7846">change 7846</a>. To unsubscribe, visit <a href="https://gerrit.asterisk.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.asterisk.org/7846"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-MessageType: merged </div>
<div style="display:none"> Gerrit-Change-Id: I8b519607263fe88e8ce964f526a45359fd362b6e </div>
<div style="display:none"> Gerrit-Change-Number: 7846 </div>
<div style="display:none"> Gerrit-PatchSet: 2 </div>
<div style="display:none"> Gerrit-Owner: Richard Mudgett <rmudgett@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: Corey Farrell <git@cfware.com> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins2 </div>
<div style="display:none"> Gerrit-Reviewer: Joshua Colp <jcolp@digium.com> </div>