<p>Walter Doekes has uploaded this change for <strong>review</strong>.</p><p><a href="https://gerrit.asterisk.org/6396">View Change</a></p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">res/res_pjsip: Standardize/fix localnet checks across pjsip.<br><br>In 2dee95cc (ASTERISK-27024) and 776ffd77 (ASTERISK-26879) there was<br>confusion about whether the transport_state->localnet ACL has ALLOW or<br>DENY semantics.<br><br>For the record: the localnet has DENY semantics, meaning that "not in<br>the list" means ALLOW, and the local nets are in the list.<br><br>Therefore, checks like this look wrong, but are right:<br><br>    /* See if where we are sending this request is local or not, and if<br>       not that we can get a Contact URI to modify */<br>    if (ast_apply_ha(transport_state->localnet, &addr) != AST_SENSE_ALLOW) {<br>        ast_debug(5, "Request is being sent to local address, "<br>                     "skipping NAT manipulation\n");<br><br>(In the list == localnet == DENY == skip NAT manipulation.)<br><br>And conversely, other checks that looked right, were wrong.<br><br>This change adds two macro's to reduce the confusion and uses those<br>instead:<br><br>    ast_sip_transport_is_nonlocal(transport_state, addr)<br>    ast_sip_transport_is_local(transport_state, addr)<br><br>ASTERISK-27248 #close<br><br>Change-Id: Ie7767519eb5a822c4848e531a53c0fd054fae934<br>---<br>M include/asterisk/res_pjsip.h<br>M main/acl.c<br>M res/res_pjsip/config_transport.c<br>M res/res_pjsip_nat.c<br>M res/res_pjsip_sdp_rtp.c<br>M res/res_pjsip_session.c<br>M res/res_pjsip_t38.c<br>7 files changed, 19 insertions(+), 11 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/96/6396/1</pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">diff --git a/include/asterisk/res_pjsip.h b/include/asterisk/res_pjsip.h<br>index efc0cd0..18661df 100644<br>--- a/include/asterisk/res_pjsip.h<br>+++ b/include/asterisk/res_pjsip.h<br>@@ -98,7 +98,10 @@<br>        */<br>   pj_ssl_cipher ciphers[SIP_TLS_MAX_CIPHERS];<br>   /*!<br>-   * Optional local network information, used for NAT purposes<br>+  * Optional local network information, used for NAT purposes.<br>+         * "deny" (set) means that it's in the local network. Use the<br>+   * ast_sip_transport_is_nonlocal and ast_sip_transport_is_local<br>+       * macro's.<br>        * \since 13.8.0<br>       */<br>   struct ast_ha *localnet;<br>@@ -124,6 +127,12 @@<br>        struct ast_sockaddr external_media_address;<br> };<br> <br>+#define ast_sip_transport_is_nonlocal(transport_state, addr) \<br>+ (!transport_state->localnet || ast_apply_ha(transport_state->localnet, addr) == AST_SENSE_ALLOW)<br>+<br>+#define ast_sip_transport_is_local(transport_state, addr) \<br>+      (transport_state->localnet && ast_apply_ha(transport_state->localnet, addr) != AST_SENSE_ALLOW)<br>+<br> /*<br>  * \brief Transport to bind to<br>  */<br>diff --git a/main/acl.c b/main/acl.c<br>index 6aeff40..237d77d 100644<br>--- a/main/acl.c<br>+++ b/main/acl.c<br>@@ -739,8 +739,8 @@<br>            char iabuf[INET_ADDRSTRLEN];<br>          char iabuf2[INET_ADDRSTRLEN];<br>                 /* DEBUG */<br>-          ast_copy_string(iabuf, ast_inet_ntoa(sin->sin_addr), sizeof(iabuf));<br>-              ast_copy_string(iabuf2, ast_inet_ntoa(ha->netaddr), sizeof(iabuf2));<br>+              ast_copy_string(iabuf, ast_sockaddr_stringify(addr), sizeof(iabuf));<br>+         ast_copy_string(iabuf2, ast_sockaddr_stringify(&current_ha->addr), sizeof(iabuf2));<br>            ast_debug(1, "##### Testing %s with %s\n", iabuf, iabuf2);<br> #endif<br>                 if (ast_sockaddr_is_ipv4(&current_ha->addr)) {<br>diff --git a/res/res_pjsip/config_transport.c b/res/res_pjsip/config_transport.c<br>index 5f7eafa..0c804b8 100644<br>--- a/res/res_pjsip/config_transport.c<br>+++ b/res/res_pjsip/config_transport.c<br>@@ -1127,7 +1127,9 @@<br>                 return 0;<br>     }<br> <br>- if (!(state->localnet = ast_append_ha("d", var->value, state->localnet, &error))) {<br>+   /* We use only the ast_apply_ha() which defaults to ALLOW<br>+     * ("permit"), so we add DENY rules. */<br>+    if (!(state->localnet = ast_append_ha("deny", var->value, state->localnet, &error))) {<br>                 return -1;<br>    }<br> <br>diff --git a/res/res_pjsip_nat.c b/res/res_pjsip_nat.c<br>index 45b0d7c..e1d56e6 100644<br>--- a/res/res_pjsip_nat.c<br>+++ b/res/res_pjsip_nat.c<br>@@ -267,7 +267,7 @@<br>                ast_sockaddr_set_port(&addr, tdata->tp_info.dst_port);<br> <br>              /* See if where we are sending this request is local or not, and if not that we can get a Contact URI to modify */<br>-           if (ast_apply_ha(transport_state->localnet, &addr) != AST_SENSE_ALLOW) {<br>+              if (ast_sip_transport_is_local(transport_state, &addr)) {<br>                         ast_debug(5, "Request is being sent to local address, skipping NAT manipulation\n");<br>                        return PJ_SUCCESS;<br>            }<br>diff --git a/res/res_pjsip_sdp_rtp.c b/res/res_pjsip_sdp_rtp.c<br>index 850d04d..b77994b 100644<br>--- a/res/res_pjsip_sdp_rtp.c<br>+++ b/res/res_pjsip_sdp_rtp.c<br>@@ -1535,8 +1535,7 @@<br>         ast_sockaddr_parse(&addr, host, PARSE_PORT_FORBID);<br> <br>    /* Is the address within the SDP inside the same network? */<br>- if (transport_state->localnet<br>-             && ast_apply_ha(transport_state->localnet, &addr) == AST_SENSE_ALLOW) {<br>+       if (ast_sip_transport_is_local(transport_state, &addr)) {<br>                 return;<br>       }<br>     ast_debug(5, "Setting media address to %s\n", ast_sockaddr_stringify_host(&transport_state->external_media_address));<br>diff --git a/res/res_pjsip_session.c b/res/res_pjsip_session.c<br>index 42d37fe..e7ee055 100644<br>--- a/res/res_pjsip_session.c<br>+++ b/res/res_pjsip_session.c<br>@@ -3227,8 +3227,7 @@<br>            ast_copy_pj_str(host, &sdp->conn->addr, sizeof(host));<br>              ast_sockaddr_parse(&addr, host, PARSE_PORT_FORBID);<br> <br>-           if (!transport_state->localnet<br>-                    || ast_apply_ha(transport_state->localnet, &addr) != AST_SENSE_ALLOW) {<br>+               if (ast_sip_transport_is_nonlocal(transport_state, &addr)) {<br>                      ast_debug(5, "Setting external media address to %s\n", ast_sockaddr_stringify_host(&transport_state->external_media_address));<br>                       pj_strdup2(tdata->pool, &sdp->conn->addr, ast_sockaddr_stringify_host(&transport_state->external_media_address));<br>                 }<br>diff --git a/res/res_pjsip_t38.c b/res/res_pjsip_t38.c<br>index 9f6ae0c..ac75caf 100644<br>--- a/res/res_pjsip_t38.c<br>+++ b/res/res_pjsip_t38.c<br>@@ -881,8 +881,7 @@<br>   ast_sockaddr_parse(&addr, host, PARSE_PORT_FORBID);<br> <br>    /* Is the address within the SDP inside the same network? */<br>- if (transport_state->localnet<br>-             && ast_apply_ha(transport_state->localnet, &addr) == AST_SENSE_ALLOW) {<br>+       if (ast_sip_transport_is_local(transport_state, &addr)) {<br>                 return;<br>       }<br>     ast_debug(5, "Setting media address to %s\n", ast_sockaddr_stringify_host(&transport_state->external_media_address));<br></pre><p>To view, visit <a href="https://gerrit.asterisk.org/6396">change 6396</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/6396"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: 14 </div>
<div style="display:none"> Gerrit-MessageType: newchange </div>
<div style="display:none"> Gerrit-Change-Id: Ie7767519eb5a822c4848e531a53c0fd054fae934 </div>
<div style="display:none"> Gerrit-Change-Number: 6396 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: Walter Doekes <walter+asterisk@wjd.nu> </div>