<p>Joshua Colp <strong>merged</strong> this change.</p><p><a href="https://gerrit.asterisk.org/6397">View Change</a></p><div style="white-space:pre-wrap">Approvals:
Joshua Colp: Looks good to me, but someone else must approve; Approved for Submit
George Joseph: Looks good to me, approved
</div><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;">diff --git a/include/asterisk/res_pjsip.h b/include/asterisk/res_pjsip.h<br>index e2c487a..ad88138 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 3194567..6868ea1 100644<br>--- a/main/acl.c<br>+++ b/main/acl.c<br>@@ -737,8 +737,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(¤t_ha->addr), sizeof(iabuf2));<br> ast_debug(1, "##### Testing %s with %s\n", iabuf, iabuf2);<br> #endif<br> if (ast_sockaddr_is_ipv4(¤t_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 b082b1d..9110a1c 100644<br>--- a/res/res_pjsip_sdp_rtp.c<br>+++ b/res/res_pjsip_sdp_rtp.c<br>@@ -1818,8 +1818,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 f6b3b93..545113f 100644<br>--- a/res/res_pjsip_session.c<br>+++ b/res/res_pjsip_session.c<br>@@ -3968,8 +3968,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 27eff42..2e60493 100644<br>--- a/res/res_pjsip_t38.c<br>+++ b/res/res_pjsip_t38.c<br>@@ -963,8 +963,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/6397">change 6397</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/6397"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: 15 </div>
<div style="display:none"> Gerrit-MessageType: merged </div>
<div style="display:none"> Gerrit-Change-Id: Ie7767519eb5a822c4848e531a53c0fd054fae934 </div>
<div style="display:none"> Gerrit-Change-Number: 6397 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: Walter Doekes <walter+asterisk@wjd.nu> </div>
<div style="display:none"> Gerrit-Reviewer: George Joseph <gjoseph@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins2 </div>
<div style="display:none"> Gerrit-Reviewer: Joshua Colp <jcolp@digium.com> </div>