[asterisk-bugs] [JIRA] (ASTERISK-18263) sip directmedia nonat handling / unreachable code

[Joshua Colp (JIRA)]

     [ https://issues.asterisk.org/jira/browse/ASTERISK-18263?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Joshua Colp updated ASTERISK-18263:
-----------------------------------

    Affects Version/s: 13.18.4

> sip directmedia nonat handling / unreachable code
> -------------------------------------------------
>
>                 Key: ASTERISK-18263
>                 URL: https://issues.asterisk.org/jira/browse/ASTERISK-18263
>             Project: Asterisk
>          Issue Type: Bug
>      Security Level: None
>          Components: Channels/chan_sip/General
>    Affects Versions: 1.8.4, 13.18.4
>            Reporter: Maarten Bezemer
>
> While trying to find out why asterisk does a remote bridge on a call although directmediapermit/deny rules should forbid that (but directmediaha appears empty at the time it is applied), I browsed through the code and found something in chan_sip.c I don't understand.
> Maybe it's just me, but it doesn't seem quite right.
> (line numbers from trunk, copy/pasted from doxygen)
> {code}
> 26679    } else if (!strcasecmp(v->name, "directmedia") || !strcasecmp(v->name, "canreinvite")) {
> 26680       ast_set_flag(&mask[0], SIP_REINVITE);
> 26681       ast_clear_flag(&flags[0], SIP_REINVITE);
> 26682       if (ast_true(v->value)) {
> 26683          ast_set_flag(&flags[0], SIP_DIRECT_MEDIA | SIP_DIRECT_MEDIA_NAT);
> 26684       } else if (!ast_false(v->value)) {
> 26685          char buf[64];
> 26686          char *word, *next = buf;
> 26687
> 26688          ast_copy_string(buf, v->value, sizeof(buf));
> 26689          while ((word = strsep(&next, ","))) {
> 26690             if (!strcasecmp(word, "update")) {
> 26691                ast_set_flag(&flags[0], SIP_REINVITE_UPDATE | SIP_DIRECT_MEDIA);
> 26692             } else if (!strcasecmp(word, "nonat")) {
> 26693                ast_set_flag(&flags[0], SIP_DIRECT_MEDIA);
> 26694                ast_clear_flag(&flags[0], SIP_DIRECT_MEDIA_NAT);
> 26695             } else {
> 26696                ast_log(LOG_WARNING, "Unknown directmedia mode '%s' on line %d\n", v->value, v->lineno);
> 26697             }
> 26698          }
> 26699       }
> 26700    } else if (!strcasecmp(v->name, "insecure")) {
> {code}
> If I read this correctly, SIP_DIRECT_MEDIA_NAT will only be set if
> SIP_DIRECT_MEDIA is also set.
> However, now look at this code:
> {code}
> 28985    if (ast_test_flag(&p->flags[0], SIP_DIRECT_MEDIA)) {
> 28986       res = AST_RTP_GLUE_RESULT_REMOTE;
> 28987       if (!apply_directmedia_ha(p, "audio")) {
> 28988          res = AST_RTP_GLUE_RESULT_FORBID;
> 28989       }
> 28990    } else if (ast_test_flag(&p->flags[0], SIP_DIRECT_MEDIA_NAT)) {
> 28991       res = AST_RTP_GLUE_RESULT_REMOTE;
> 28992    } else if (ast_test_flag(&global_jbconf, AST_JB_FORCED)) {
> 28993       res = AST_RTP_GLUE_RESULT_FORBID;
> 28994    }
> {code}
> It seems there is no way in which line 28991 can be reached...
> If SIP_DIRECT_MEDIA is set, the first part will be executed, and if not, SIP_DIRECT_MEDIA_NAT will never be true so that part will always be skipped.
> This code has been this way since introduction of directmediaha, but it seems that now the entire 'nonat' value for the directmedia option seems a bit bogus. If SIP_DIRECT_MEDIA is set, and directmediaha allows it, the bridge will be remote, regardless of SIP_DIRECT_MEDIA_NAT flag. (Or, it will be adjusted later on, of course).
> Aside from that, directmediaha doesn't seem to work as expected (or my understanding of the idea is incorrect and directmediaha would be checked only for one of the peers instead of both), but that's something I'm still digging into. Possibly resulting in another bug report..



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