Asterisk
  1. Asterisk
  2. ASTERISK-7210

[patch] allow SIP Spiral to work instead of causing a '482 Loop Detected' condition

    Details

    • Type: Bug Bug
    • Status: Closed
    • Severity: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: None
    • Target Release Version/s: None
    • Labels:
      None
    • SVN Revision Number:
      47646
    • Mantis ID:
      7403
    • Regression:
      No

      Description

      A sip call originating from asterisk causes a '482 Loop Detected' response when forwarded back to asterisk from a external proxy. This should be allowed when the request URI has been changed by the proxy and the call is now targeted at a different user.

                • ADDITIONAL INFORMATION ******

      I have made a small change to the function find_call so that a INVITE request without a to tag is not incorrectly matched with the originating channel on the basis of the callid.

      Now when a spiraled INVITE is received new sip_pvt is created, the to tag for the dialog (call leg) is created by asterisk in the invite response and both dialogs are then uniquely identified by the to and from tags. The call then proceeds correctly.

      Note: this assumes that pedantic = true.

      1. chan_sip_spiral.patch
        3 kB
      2. chan_sip.c.patch2
        12 kB
      3. sip_find_call_1.diff
        5 kB
      4. sip_find_call.diff
        5 kB
      5. sip_spiral.patch
        4 kB
        gautam_dawar
      6. sip_spiral3.patch
        4 kB
      7. spiral_attempt.patch
        2 kB
        Mark Michelson
      8. working_spiral.patch
        10 kB
        Mark Michelson

        Activity

        Hide
        Iñaki Baz Castillo added a comment -

        Hi, I'm trying to patch chan_sip.c (rev 123742) but I get this error:

        /usr/src/asterisk-trunk$ patch -p0 < working_spiral.patch
        patching file channels/chan_sip.c
        Hunk #1 FAILED at 4641.
        Hunk #2 succeeded at 17406 (offset 3642 lines).
        Hunk #3 succeeded at 17734 with fuzz 1 (offset 3658 lines).
        1 out of 3 hunks FAILED – saving rejects to file channels/chan_sip.c.rej

        It's seems that the path is done for this 123742 revision, isn't?

        The content of channels/chan_sip.c.rej is:

        -------------
        ***************

            • 4641,4648 ****
              if (!found && option_debug > 4)
              ast_log(LOG_DEBUG, "= Being pedantic: This is not our match on request: Call ID: %s Ourtag <null> Totag %s Method %s\n", p->callid, totag, sip_methods[req->method].text);
              }
              -
              -
              if (found) { /* Found the call */ ast_mutex_lock(&p->lock); --- 4641,4646 ---- if (!found && option_debug > 4) ast_log(LOG_DEBUG, "= Being pedantic: This is not our match on request: Call ID: %s Ourtag <null> Totag %s Method %s\n", p->callid, totag, sip_methods[req->method].text); }

              if (found)

              { /* Found the call */ ast_mutex_lock(&p->lock); *************** *** 17985,17994 **** being able to call yourself */ /* If pedantic is on, we need to check the tags. If they're different, this is in fact a forked call through a SIP proxy somewhere. */ - transmit_response(p, "482 Loop Detected", req); - p->invitestate = INV_COMPLETED; - sip_scheddestroy(p, DEFAULT_TRANS_TIMEOUT); - return 0; }

        if (!ast_test_flag(req, SIP_PKT_IGNORE) && p->pendinginvite) {
        — 18248,18289 ----
        being able to call yourself */
        /* If pedantic is on, we need to check the tags. If they're different, this is
        in fact a forked call through a SIP proxy somewhere. */
        + int different;
        + if (pedanticsipchecking)
        + different = sip_uri_cmp(p->initreq.rlPart2, req->rlPart2);
        + else
        + different = strcmp(p->initreq.rlPart2, req->rlPart2);
        + if (!different)

        { + transmit_response(p, "482 Loop Detected", req); + p->invitestate = INV_COMPLETED; + sip_scheddestroy(p, DEFAULT_TRANS_TIMEOUT); + return 0; + }

        else {
        + /* This is a spiral. What we need to do is to just change the outgoing INVITE
        + * so that it now routes to the new Request URI. Since we created the INVITE ourselves
        + * that should be all we need to do.
        + */
        + char *uri = ast_strdupa(req->rlPart2);
        + char *at = strchr(uri, '@');
        + char *peerorhost;
        + struct sip_pkt *pkt = NULL;
        + ast_log(LOG_NOTICE, "Spiral detected\n");
        + if (at)

        { + *at = '\0'; + }

        + /* Parse out "sip:" */
        + if ((peerorhost = strchr(uri, ':')))

        { + *peerorhost++ = '\0'; + }

        + create_addr(p, peerorhost);
        + ast_string_field_free(p, theirtag);
        + for (pkt = p->packets; pkt; pkt = pkt->next) {
        + if (pkt->seqno == p->icseq && pkt->method == SIP_INVITE)

        { + AST_SCHED_DEL(sched, pkt->retransid); + }

        + }
        + return transmit_invite(p, SIP_INVITE, 1, 2);
        + }
        }

        if (!ast_test_flag(req, SIP_PKT_IGNORE) && p->pendinginvite) {

        -------------

        Show
        Iñaki Baz Castillo added a comment - Hi, I'm trying to patch chan_sip.c (rev 123742) but I get this error: /usr/src/asterisk-trunk$ patch -p0 < working_spiral.patch patching file channels/chan_sip.c Hunk #1 FAILED at 4641. Hunk #2 succeeded at 17406 (offset 3642 lines). Hunk #3 succeeded at 17734 with fuzz 1 (offset 3658 lines). 1 out of 3 hunks FAILED – saving rejects to file channels/chan_sip.c.rej It's seems that the path is done for this 123742 revision, isn't? The content of channels/chan_sip.c.rej is: ------------- *************** 4641,4648 **** if (!found && option_debug > 4) ast_log(LOG_DEBUG, "= Being pedantic: This is not our match on request: Call ID: %s Ourtag <null> Totag %s Method %s\n", p->callid, totag, sip_methods [req->method] .text); } - - if (found) { /* Found the call */ ast_mutex_lock(&p->lock); --- 4641,4646 ---- if (!found && option_debug > 4) ast_log(LOG_DEBUG, "= Being pedantic: This is not our match on request: Call ID: %s Ourtag <null> Totag %s Method %s\n", p->callid, totag, sip_methods[req->method].text); } if (found) { /* Found the call */ ast_mutex_lock(&p->lock); *************** *** 17985,17994 **** being able to call yourself */ /* If pedantic is on, we need to check the tags. If they're different, this is in fact a forked call through a SIP proxy somewhere. */ - transmit_response(p, "482 Loop Detected", req); - p->invitestate = INV_COMPLETED; - sip_scheddestroy(p, DEFAULT_TRANS_TIMEOUT); - return 0; } if (!ast_test_flag(req, SIP_PKT_IGNORE) && p->pendinginvite) { — 18248,18289 ---- being able to call yourself */ /* If pedantic is on, we need to check the tags. If they're different, this is in fact a forked call through a SIP proxy somewhere. */ + int different; + if (pedanticsipchecking) + different = sip_uri_cmp(p->initreq.rlPart2, req->rlPart2); + else + different = strcmp(p->initreq.rlPart2, req->rlPart2); + if (!different) { + transmit_response(p, "482 Loop Detected", req); + p->invitestate = INV_COMPLETED; + sip_scheddestroy(p, DEFAULT_TRANS_TIMEOUT); + return 0; + } else { + /* This is a spiral. What we need to do is to just change the outgoing INVITE + * so that it now routes to the new Request URI. Since we created the INVITE ourselves + * that should be all we need to do. + */ + char *uri = ast_strdupa(req->rlPart2); + char *at = strchr(uri, '@'); + char *peerorhost; + struct sip_pkt *pkt = NULL; + ast_log(LOG_NOTICE, "Spiral detected\n"); + if (at) { + *at = '\0'; + } + /* Parse out "sip:" */ + if ((peerorhost = strchr(uri, ':'))) { + *peerorhost++ = '\0'; + } + create_addr(p, peerorhost); + ast_string_field_free(p, theirtag); + for (pkt = p->packets; pkt; pkt = pkt->next) { + if (pkt->seqno == p->icseq && pkt->method == SIP_INVITE) { + AST_SCHED_DEL(sched, pkt->retransid); + } + } + return transmit_invite(p, SIP_INVITE, 1, 2); + } } if (!ast_test_flag(req, SIP_PKT_IGNORE) && p->pendinginvite) { -------------
        Hide
        Mark Michelson added a comment -

        Sorry. The patch was made against the 1.4 branch of Asterisk.

        Show
        Mark Michelson added a comment - Sorry. The patch was made against the 1.4 branch of Asterisk.
        Hide
        Iñaki Baz Castillo added a comment -

        Ok, so in which revision of chan_sip.c must I apply the patch? Thanks a lot.

        Show
        Iñaki Baz Castillo added a comment - Ok, so in which revision of chan_sip.c must I apply the patch? Thanks a lot.
        Hide
        Sean Bright added a comment -

        The patch itself was made against revision 123742 of chan_sip.c. As putnopvut mentioned, the patch was made against the 1.4 branch (http://svn.digium.com/svn/asterisk/branches/1.4).

        Show
        Sean Bright added a comment - The patch itself was made against revision 123742 of chan_sip.c. As putnopvut mentioned, the patch was made against the 1.4 branch ( http://svn.digium.com/svn/asterisk/branches/1.4 ).
        Hide
        Mark Michelson added a comment -

        Hmmm, for some reason svnbot didn't close this issue when I fixed this. I tweaked the patch that I had attached after I found some flaws and committed the fix in rev 132790 to the 1.4 branch. Committed to trunk in rev 132795.

        Any further issues with SIP spirals should be opened in new bug reports about specific problems with the now-implemented spiral code.

        Thanks.

        Show
        Mark Michelson added a comment - Hmmm, for some reason svnbot didn't close this issue when I fixed this. I tweaked the patch that I had attached after I found some flaws and committed the fix in rev 132790 to the 1.4 branch. Committed to trunk in rev 132795. Any further issues with SIP spirals should be opened in new bug reports about specific problems with the now-implemented spiral code. Thanks.

          Dates

          • Created:
            Updated:
            Resolved:

            Development