res/res_pjsip: Standardize/fix localnet checks across pjsip.

In 2dee95cc (ASTERISK-27024) and 776ffd77 (ASTERISK-26879) there was
confusion about whether the transport_state->localnet ACL has ALLOW or
DENY semantics.

For the record: the localnet has DENY semantics, meaning that "not in
the list" means ALLOW, and the local nets are in the list.

Therefore, checks like this look wrong, but are right:

    /* See if where we are sending this request is local or not, and if
       not that we can get a Contact URI to modify */
    if (ast_apply_ha(transport_state->localnet, &addr) != AST_SENSE_ALLOW) {
        ast_debug(5, "Request is being sent to local address, "
                     "skipping NAT manipulation\n");

(In the list == localnet == DENY == skip NAT manipulation.)

And conversely, other checks that looked right, were wrong.

This change adds two macro's to reduce the confusion and uses those
instead:

    ast_sip_transport_is_nonlocal(transport_state, addr)
    ast_sip_transport_is_local(transport_state, addr)

ASTERISK-27248 #close

Change-Id: Ie7767519eb5a822c4848e531a53c0fd054fae934

diff --git a/include/asterisk/res_pjsip.h b/include/asterisk/res_pjsip.h
index 6d882e339190915af6a3299fd333f2984de0c12f..70b56aefa3fc8857a212bf796ac4c9fd1bb69edc 100644 (file)
--- a/include/asterisk/res_pjsip.h
+++ b/include/asterisk/res_pjsip.h
@@ -98,7 +98,10 @@ struct ast_sip_transport_state {
         */
        pj_ssl_cipher ciphers[SIP_TLS_MAX_CIPHERS];
        /*!
-        * Optional local network information, used for NAT purposes
+        * Optional local network information, used for NAT purposes.
+        * "deny" (set) means that it's in the local network. Use the
+        * ast_sip_transport_is_nonlocal and ast_sip_transport_is_local
+        * macro's.
         * \since 13.8.0
         */
        struct ast_ha *localnet;
@@ -124,6 +127,12 @@ struct ast_sip_transport_state {
        struct ast_sockaddr external_media_address;
 };
 
+#define ast_sip_transport_is_nonlocal(transport_state, addr) \
+       (!transport_state->localnet || ast_apply_ha(transport_state->localnet, addr) == AST_SENSE_ALLOW)
+
+#define ast_sip_transport_is_local(transport_state, addr) \
+       (transport_state->localnet && ast_apply_ha(transport_state->localnet, addr) != AST_SENSE_ALLOW)
+
 /*
  * \brief Transport to bind to
  */

diff --git a/main/acl.c b/main/acl.c
index 9820e8bef2c71a6a858f0a4d8e8304a3c4b375ee..94a242af2c9a94ecbedd08707dea20c40b3d33ae 100644 (file)
--- a/main/acl.c
+++ b/main/acl.c
@@ -739,8 +739,8 @@ enum ast_acl_sense ast_apply_ha(const struct ast_ha *ha, const struct ast_sockad
                char iabuf[INET_ADDRSTRLEN];
                char iabuf2[INET_ADDRSTRLEN];
                /* DEBUG */
-               ast_copy_string(iabuf, ast_inet_ntoa(sin->sin_addr), sizeof(iabuf));
-               ast_copy_string(iabuf2, ast_inet_ntoa(ha->netaddr), sizeof(iabuf2));
+               ast_copy_string(iabuf, ast_sockaddr_stringify(addr), sizeof(iabuf));
+               ast_copy_string(iabuf2, ast_sockaddr_stringify(&current_ha->addr), sizeof(iabuf2));
                ast_debug(1, "##### Testing %s with %s\n", iabuf, iabuf2);
 #endif
                if (ast_sockaddr_is_ipv4(&current_ha->addr)) {

diff --git a/res/res_pjsip/config_transport.c b/res/res_pjsip/config_transport.c
index 5f7eafa1c4deed04111e7449a71e310d5ce2cc33..0c804b82a49f1a9f4f54c24c5b573c738e08e110 100644 (file)
--- a/res/res_pjsip/config_transport.c
+++ b/res/res_pjsip/config_transport.c
@@ -1127,7 +1127,9 @@ static int transport_localnet_handler(const struct aco_option *opt, struct ast_v
                return 0;
        }
 
-       if (!(state->localnet = ast_append_ha("d", var->value, state->localnet, &error))) {
+       /* We use only the ast_apply_ha() which defaults to ALLOW
+        * ("permit"), so we add DENY rules. */
+       if (!(state->localnet = ast_append_ha("deny", var->value, state->localnet, &error))) {
                return -1;
        }
 

diff --git a/res/res_pjsip_nat.c b/res/res_pjsip_nat.c
index 63ae722b4d5f48a6fa9df0255b24bf3164231014..370004a3aa062dc6c0c4a69499e441c3a58f1a07 100644 (file)
--- a/res/res_pjsip_nat.c
+++ b/res/res_pjsip_nat.c
@@ -267,7 +267,7 @@ static pj_status_t nat_on_tx_message(pjsip_tx_data *tdata)
                ast_sockaddr_set_port(&addr, tdata->tp_info.dst_port);
 
                /* See if where we are sending this request is local or not, and if not that we can get a Contact URI to modify */
-               if (ast_apply_ha(transport_state->localnet, &addr) != AST_SENSE_ALLOW) {
+               if (ast_sip_transport_is_local(transport_state, &addr)) {
                        ast_debug(5, "Request is being sent to local address, skipping NAT manipulation\n");
                        return PJ_SUCCESS;
                }

diff --git a/res/res_pjsip_sdp_rtp.c b/res/res_pjsip_sdp_rtp.c
index 72b70441fd61205482db1eff2133711f003001e1..b8ae8c185f8150729a72d360d873ce56bdfca580 100644 (file)
--- a/res/res_pjsip_sdp_rtp.c
+++ b/res/res_pjsip_sdp_rtp.c
@@ -1517,8 +1517,7 @@ static void change_outgoing_sdp_stream_media_address(pjsip_tx_data *tdata, struc
        ast_sockaddr_parse(&addr, host, PARSE_PORT_FORBID);
 
        /* Is the address within the SDP inside the same network? */
-       if (transport_state->localnet
-               && ast_apply_ha(transport_state->localnet, &addr) == AST_SENSE_ALLOW) {
+       if (ast_sip_transport_is_local(transport_state, &addr)) {
                return;
        }
        ast_debug(5, "Setting media address to %s\n", ast_sockaddr_stringify_host(&transport_state->external_media_address));

diff --git a/res/res_pjsip_session.c b/res/res_pjsip_session.c
index 4960b9c1c134e748218e2b920aece82c689a10e8..ab6fce2c8e96ea7fa12563da4541f501adb20d6d 100644 (file)
--- a/res/res_pjsip_session.c
+++ b/res/res_pjsip_session.c
@@ -3195,8 +3195,7 @@ static void session_outgoing_nat_hook(pjsip_tx_data *tdata, struct ast_sip_trans
                ast_copy_pj_str(host, &sdp->conn->addr, sizeof(host));
                ast_sockaddr_parse(&addr, host, PARSE_PORT_FORBID);
 
-               if (!transport_state->localnet
-                       || ast_apply_ha(transport_state->localnet, &addr) != AST_SENSE_ALLOW) {
+               if (ast_sip_transport_is_nonlocal(transport_state, &addr)) {
                        ast_debug(5, "Setting external media address to %s\n", ast_sockaddr_stringify_host(&transport_state->external_media_address));
                        pj_strdup2(tdata->pool, &sdp->conn->addr, ast_sockaddr_stringify_host(&transport_state->external_media_address));
                }

diff --git a/res/res_pjsip_t38.c b/res/res_pjsip_t38.c
index 648c7535450bc809c3dcc82ad25164e96d4473b0..e53b3222578b74173e57a75e206f5f0333afe0f1 100644 (file)
--- a/res/res_pjsip_t38.c
+++ b/res/res_pjsip_t38.c
@@ -881,8 +881,7 @@ static void change_outgoing_sdp_stream_media_address(pjsip_tx_data *tdata, struc
        ast_sockaddr_parse(&addr, host, PARSE_PORT_FORBID);
 
        /* Is the address within the SDP inside the same network? */
-       if (transport_state->localnet
-               && ast_apply_ha(transport_state->localnet, &addr) == AST_SENSE_ALLOW) {
+       if (ast_sip_transport_is_local(transport_state, &addr)) {
                return;
        }
        ast_debug(5, "Setting media address to %s\n", ast_sockaddr_stringify_host(&transport_state->external_media_address));