[asterisk-bugs] [JIRA] (ASTERISK-27248) [patch]external_media_address and external_signaling_address don't always honor localnet

[Friendly Automation (JIRA)]

    [ https://issues.asterisk.org/jira/browse/ASTERISK-27248?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=238623#comment-238623 ] 

Friendly Automation commented on ASTERISK-27248:
------------------------------------------------

Change 6469 merged by Joshua Colp:
res/res_pjsip: Fix localnet checks in pjsip, part 2.

[https://gerrit.asterisk.org/6469|https://gerrit.asterisk.org/6469]

> [patch]external_media_address and external_signaling_address don't always honor localnet
> ----------------------------------------------------------------------------------------
>
>                 Key: ASTERISK-27248
>                 URL: https://issues.asterisk.org/jira/browse/ASTERISK-27248
>             Project: Asterisk
>          Issue Type: Bug
>      Security Level: None
>          Components: Channels/chan_pjsip
>    Affects Versions: 13.17.1
>            Reporter: Walter Doekes
>         Attachments: ASTERISK-27248.patch, ASTERISK-27248_undo.patch
>
>
> Let's say I have this pjsip config:
> {noformat}
> external_signaling_address=127.1.1.1
> external_media_address=127.2.2.2
> {noformat}
> Then I want my outgoing invites to look like this:
> {noformat}
> INVITE sip:bob at DEST:9284;transport=UDP SIP/2.0
> Via: SIP/2.0/UDP 127.1.1.1:5060;rport;branch=z9hG4bKPjce5a5266-b624-4a52-b420-3648f073ec6d
> From: <sip:alice at SOURCE>;tag=904cc2dd-1f73-4d2c-b712-78c99761bc0f
> To: <sip:bob at SOURCE>
> Contact: <sip:asterisk at 127.1.1.1:5060>
> ...
> o=- 1018431938 1018431938 IN IP4 SOURCE
> s=Asterisk
> c=IN IP4 127.2.2.2
> {noformat}
> If I add an unrelated localnet setting, then it should not affect those values. For example:
> {noformat}
> local_net=127.255.255.255/32
> local_net=255.255.255.255/32
> {noformat}
> However, in Asterisk 13.17.1 it does differ, because of this code:
> {noformat}
>                 if (!transport_state->localnet
>                         || ast_apply_ha(transport_state->localnet, &addr) != AST_SENSE_ALLOW) {
>                         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));
>                 }
> {noformat}
> The ha struct stores the values in (default) "deny" order: if it's *not* found, then it's ALLOWed. If it *is* found, it returns DENY.
> Thus:
> {noformat}
> ast_apply_ha(transport_state->localnet, &addr) == AST_SENSE_ALLOW)
> {noformat}
> means: it's NOT in the local net
> and:
> {noformat}
> ast_apply_ha(transport_state->localnet, &addr) != AST_SENSE_ALLOW)
> {noformat}
> means: it IS in the local net.
> Logically, you would have it return DENY if it's NOT in the list, and ALLOW if it's in the list, but that's not how ast_apply_ha() works.
> If we check the latest 13.x, we see this:
> {noformat}
> $ wgrep . -B1 -A3 localnet.*SENSE
> {noformat}
> {noformat}
> ./res/res_pjsip_session.c-		if (!transport_state->localnet
> ./res/res_pjsip_session.c:			|| ast_apply_ha(transport_state->localnet, &addr) != AST_SENSE_ALLOW) {
> ./res/res_pjsip_session.c-			ast_debug(5, "Setting external media address to %s\n", ast_sockaddr_stringify_host(&transport_state->external_media_address));
> ./res/res_pjsip_session.c-			pj_strdup2(tdata->pool, &sdp->conn->addr, ast_sockaddr_stringify_host(&transport_state->external_media_address));
> ./res/res_pjsip_session.c-		}
> {noformat}
> DENY -> is local -> setting media to external because local??
> {noformat}
> ./res/res_pjsip_nat.c-		/* See if where we are sending this request is local or not, and if not that we can get a Contact URI to modify */
> ./res/res_pjsip_nat.c:		if (ast_apply_ha(transport_state->localnet, &addr) != AST_SENSE_ALLOW) {
> ./res/res_pjsip_nat.c-			ast_debug(5, "Request is being sent to local address, skipping NAT manipulation\n");
> ./res/res_pjsip_nat.c-			return PJ_SUCCESS;
> ./res/res_pjsip_nat.c-		}
> {noformat}
> DENY -> is local -> OK
> {noformat}
> ./res/res_pjsip_sdp_rtp.c-	if (transport_state->localnet
> ./res/res_pjsip_sdp_rtp.c:		&& ast_apply_ha(transport_state->localnet, &addr) == AST_SENSE_ALLOW) {
> ./res/res_pjsip_sdp_rtp.c-		return;
> ./res/res_pjsip_sdp_rtp.c-	}
> ./res/res_pjsip_sdp_rtp.c-	ast_debug(5, "Setting media address to %s\n", ast_sockaddr_stringify_host(&transport_state->external_media_address));
> {noformat}
> ALLOW -> is not local -> return -> not setting external IP because non-local??
> {noformat}
> ./res/res_pjsip_t38.c-	if (transport_state->localnet
> ./res/res_pjsip_t38.c:		&& ast_apply_ha(transport_state->localnet, &addr) == AST_SENSE_ALLOW) {
> ./res/res_pjsip_t38.c-		return;
> ./res/res_pjsip_t38.c-	}
> ./res/res_pjsip_t38.c-	ast_debug(5, "Setting media address to %s\n", ast_sockaddr_stringify_host(&transport_state->external_media_address));
> {noformat}
> ALLOW -> is not local -> return -> not setting external IP because non-local??
> It appears to me that 3/4 checks are wrong.
> I'd check the regression box, because a customer noticed this after 13.13.1, but I'm not sure how the changes interact. It appears that some of this was already broken before that change.



--
This message was sent by Atlassian JIRA
(v6.2#6252)