[Home]

Summary:ASTERISK-12074: [patch] SIP Protocol Violation when REFER rejected in sip_transfer (Cisco CCM, post answer), and Transfer application misclaims
Reporter:David Woolley (davidw)Labels:
Date Opened:2008-05-23 12:59:53Date Closed:2011-07-26 15:03:51
Priority:MinorRegression?No
Status:Closed/CompleteComponents:Channels/chan_sip/Transfers
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) badtransfer.txt
( 1) Issue_12713_branch1.6.1_r186382_port.patch
( 2) Issue_12713_branch1.6.2_r186382_port.patch
( 3) Issue_12713_tag1.6.1.0_r186382_port.patch
Description:A call from Cisco CCM was answered and then transferred back to a different extension on the Cisco.

Asterisk first reported a successful TRANSFERSTATUS, then tried to use a REFER method.  This was rejected as unsupported.  Asterisk responded with BYE, which the Cisco accepted.  However it continued to try to send REFERs, which the Cisco, of course, rejected, because the session reference had been deleted by the BYE!

****** ADDITIONAL INFORMATION ******

I'm attaching a trace with sip set debug and core set debug 5 chan_sip.c set.  This is a screen scrape, so has some awkward newlines.

The Asterisk version is 1.4.20-rc3; Mantis doesn't have the release candidate suffixes.

The dialplan was:

exten => 6176,1,Transfer(SIP/6903@192.168.10.10)
exten => 6176,n,Noop(${TRANSFERSTATUS})

exten => 6177,1,Answer
exten => 6177,n,Goto(6176,1)
Comments:By: Mark Michelson (mmichelson) 2008-06-04 10:00:04

I believe this issue is occurring because of Asterisk's inflexibility for supporting multiple transactions within a dialog. The REFER is sent with Cseq 102.  Before the response is received, Asterisk sends a BYE with Cseq 103. When the response for the REFER is received, Asterisk sees that the Cseq is less than the latest Cseq sent for that dialog, and so Asterisk does not handle the response at all. If you have sip history enabled, you would probably see a lot of messages that say "Ignoring this retransmit."

This is similar to issue ASTERISK-9749 (which is now closed) and issue ASTERISK-11308, which suffer similar consequences due to the same behavior.
By: David Woolley (davidw) 2008-06-04 10:21:30

Note that the other aspect of this is that the transfer fails after the dialplan has been told it has worked.  Looking further at this, I notice this issue is highlighted in the comments, although they say they are waiting on some form of return code variable and that now seems to exist.

That means the dialplan cannot simply fall back to using Dial, even it it hadn't completely lost the original call.

In practice transfer didn't prove useful to use for other reasons, which is why I only considered the problem minor.
By: David Woolley (davidw) 2008-06-04 10:49:36

The sip history is as you say.  You can avoid the protocol violation by adding a wait after the transfer, but this just tends to highlight that the transfer application shouldn't be returning until it knows that the transfer either definitely failed, or it has lost control of the call.

If you wait long enough to be sure that the transfer has failed, you can even use a fallback Dial command, but its an unreasonable wait and chan_sip knows sooner.

This is from before the wait was added:

 * SIP Call
1. Rx              INVITE / 101 INVITE / sip:6177@192.168.130.116:5060
2. NewChan         Channel SIP/192.168.10.10-0a0ee5f0 - from 42bcc700-1de1504f-9
db
3. TxResp          SIP/2.0 / 101 INVITE - 100 Trying
4. TxRespRel       SIP/2.0 / 101 INVITE - 200 OK
5. TxReqRel        REFER / 102 REFER - -UNKNOWN-
6. Hangup          Cause Unknown
7. SchedDestroy    32000 ms
8. CancelDestroy  
9. Rx              ACK / 101 ACK / sip:6177@192.168.130.116:5060
10. TxReqRel        BYE / 103 BYE - -UNKNOWN-
11. SchedDestroy    32000 ms
12. Rx              SIP/2.0 / 102 REFER / 405 Method Not Allowed
13. Ignore          Ignoring this retransmit
14. Rx              SIP/2.0 / 103 BYE / 200 OK
15. ReTx            1000 REFER sip:3002@192.168.10.10:5061 SIP/2.0
16. Rx              SIP/2.0 / 102 REFER / 481 Call Leg/Transaction Does Not Exis
t
17. Ignore          Ignoring this retransmit
18. ReTx            2000 REFER sip:3002@192.168.10.10:5061 SIP/2.0
19. Rx              SIP/2.0 / 102 REFER / 481 Call Leg/Transaction Does Not Exis
t
20. Ignore          Ignoring this retransmit
21. ReTx            4000 REFER sip:3002@192.168.10.10:5061 SIP/2.0
22. Rx              SIP/2.0 / 102 REFER / 481 Call Leg/Transaction Does Not Exis
t
23. Ignore          Ignoring this retransmit
24. ReTx            4000 REFER sip:3002@192.168.10.10:5061 SIP/2.0
25. Rx              SIP/2.0 / 102 REFER / 481 Call Leg/Transaction Does Not Exis
By: Olle Johansson (oej) 2008-07-01 08:12:20

The transfer application in the dialplan is totally broken. There's no way for the SIP channel to come back and say "I failed". This needs to be seriously redesigned.
By: Joshua C. Colp (jcolp) 2009-04-02 09:01:06

I have created a branch available at http://svn.digium.com/svn/asterisk/team/file/issue12713 which causes Transfer() to actually provide a TRANSFERSTATUS that represents what actually happened. I will continue testing but would appreciate any other testing.
By: David Woolley (davidw) 2009-04-03 05:38:53

Unfortunately, we've moved on to a later Cisco version which can handle late transfers.  Also, SIP limitations look like meaning that transfers would break mandatory requirements for us, even though we had hoped that they might provide a solution for some optional ones.  As such, I'm not sure that we are in a position to test this any longer.
By: Digium Subversion (svnbot) 2009-04-03 11:47:29

Repository: asterisk
Revision: 186382

U   trunk/channels/chan_iax2.c
U   trunk/channels/chan_sip.c
U   trunk/include/asterisk/frame.h
U   trunk/main/channel.c

------------------------------------------------------------------------
r186382 | file | 2009-04-03 11:47:28 -0500 (Fri, 03 Apr 2009) | 11 lines

Add better support for relaying success or failure of the ast_transfer() API call.

This API call now waits for a special frame from the underlying channel driver to
indicate success or failure. This allows the return value to truly convey whether
the transfer worked or not. In the case of the Transfer() dialplan application this
means the value of the TRANSFERSTATUS dialplan variable is actually true.

(closes issue ASTERISK-12074)
Reported by: davidw
Tested by: file

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

http://svn.digium.com/view/asterisk?view=rev&revision=186382
By: Digium Subversion (svnbot) 2009-04-03 11:48:13

Repository: asterisk
Revision: 186383

_U  branches/1.6.2/

------------------------------------------------------------------------
r186383 | file | 2009-04-03 11:48:13 -0500 (Fri, 03 Apr 2009) | 17 lines

Blocked revisions 186382 via svnmerge

........
 r186382 | file | 2009-04-03 13:47:27 -0300 (Fri, 03 Apr 2009) | 11 lines
 
 Add better support for relaying success or failure of the ast_transfer() API call.
 
 This API call now waits for a special frame from the underlying channel driver to
 indicate success or failure. This allows the return value to truly convey whether
 the transfer worked or not. In the case of the Transfer() dialplan application this
 means the value of the TRANSFERSTATUS dialplan variable is actually true.
 
 (closes issue ASTERISK-12074)
 Reported by: davidw
 Tested by: file
........

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

http://svn.digium.com/view/asterisk?view=rev&revision=186383
By: David Woolley (davidw) 2009-09-07 11:01:13

1) Looking at the code, I think that it is reporting a success on a NOTIFY reporting a status of 1xx, when it should, as the comments say, ignore this.

2) This appears to have failed to merge into 1.6.2, in spite of the svnbot report:

[root@centos ~]# svn diff -c 186383  http://svn.digium.com/svn/asterisk/branches/1.6.2
Property changes on: .
___________________________________________________________________
Name: trunk-blocked
  - /trunk:182362,182521,182762,182960,183124,183148,183196,183239,183553-18355
5,184986,185299,185532,185581,185704,185741,185777,186078
  + /trunk:182362,182521,182762,182960,183124,183148,183196,183239,183553-18355
5,184986,185299,185532,185581,185704,185741,185777,186078,186382

[root@centos ~]# svn diff -c 186383  http://svn.digium.com/svn/asterisk/branches/1.6.2/channels/chan_sip.c
[root@centos ~]# svn log -r 186383  http://svn.digium.com/svn/asterisk/branches/1.6.2/channels/chan_sip.c                                                      
------------------------------------------------------------------------

By: Olle Johansson (oej) 2009-09-09 15:06:34

I see the issue - we should not report transfer success on 1xx messages... Will fix.
By: Olle Johansson (oej) 2009-09-09 15:07:03

File: really cool fix. I'm impressed. This is something I've been waiting for a long time.
By: Digium Subversion (svnbot) 2009-09-09 15:10:42

Repository: asterisk
Revision: 217482

U   trunk/channels/chan_sip.c

------------------------------------------------------------------------
r217482 | oej | 2009-09-09 15:10:42 -0500 (Wed, 09 Sep 2009) | 9 lines

Don't report transfer success until we actually know. 1xx messages are not final.

Related to ASTERISK-12074

Patch by oej

A big thank you to file for finally fixing the transfer() dialplan application.
I've been waiting for years for this. Great work!

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

http://svn.digium.com/view/asterisk?view=rev&revision=217482
By: Olle Johansson (oej) 2009-09-09 15:14:58

Please test.

This code was only merged to trunk.

We will have to ask file about that, since this was reported in 1.4 and has been an issue I've complained about ever since 0.9...
By: Digium Subversion (svnbot) 2009-09-09 15:19:56

Repository: asterisk
Revision: 217490

_U  branches/1.6.2/

------------------------------------------------------------------------
r217490 | oej | 2009-09-09 15:19:56 -0500 (Wed, 09 Sep 2009) | 16 lines

Blocked revisions 217482 via svnmerge

........
r217482 | oej | 2009-09-09 22:09:31 +0200 (Ons, 09 Sep 2009) | 9 lines

Don't report transfer success until we actually know. 1xx messages are not final.

Related to ASTERISK-12074

Patch by oej

A big thank you to file for finally fixing the transfer() dialplan application.
I've been waiting for years for this. Great work!

........

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

http://svn.digium.com/view/asterisk?view=rev&revision=217490
By: David Woolley (davidw) 2009-11-27 14:03:28.000-0600

As the svnbot merge of r186382 failed for 1.6.2, and we are in a configuration freeze at 1.6.1.0, I've produced replacement patches against 1.6.1.0, 1.6.1 SVN, and 1.6.2 SVN.

The 1.6.1.0 variant compiles, and will take the r217482 patch cleanly (although as this is already recorded as done in SVN, although it failed, some care may be needed in applying to the repository).  I have not done any testing yet.  The key difference was that the needdestroy variable was replaced by a function.

This patch won't work against 1.6.1 SVN because of conflicts in channel.c (extra entries wrt T38 in two case statements involving the the new frame type), and because of new additions in frame.h, so I have a variant for that, which I haven't even tried to compile.

Neither patch will work for 1.6.2 SVN, because it doesn't require a fixup for needdestroy relative to the the original revision to the trunk, so I've also uploaded a variant for that.
By: David Woolley (davidw) 2009-11-30 11:15:46.000-0600

There are a number of places where the comments say that transfers are broken, but which are no longer true once this patch is applied.

E.g. a simple one is:

       \todo Fix the transfer() dialplan function so that a transfer may fail
By: David Woolley (davidw) 2009-12-01 09:41:03.000-0600

Most or all exits from the transfer logic seem to set needdestroy on the SIP private structure, but a failed refer should not result in the call going down and certainly is no guarantee that the calling party will clear the call, so could end up with a stuck call on the calling party.  Even the dead leg of a successful transfer needs taking down cleanly, with a BYE, which would typically come from the transferee.

Do I need to raise this as a new issue?
By: David Woolley (davidw) 2009-12-02 08:44:18.000-0600

In handle_request_notify, this is not Ringing, although it needs the same handling!

               case 183:       /* Ringing: */
                       /* Don't do anything yet */
                       success = -1;   /* Wait */
                       break;

More generally, the code should act on the first digit, even if it doesn't recognize the exact code.
By: David Woolley (davidw) 2009-12-02 08:48:51.000-0600

Also status 200 should give success (I at first thought it was working because I got success from the 180 Ringing!)
By: David Woolley (davidw) 2009-12-02 09:38:18.000-0600

487 Session Cancelled needs handling, but I'm not quite sure how yet.  If you cancel the transferred call on a Polycom (PolycomSoundPointIP-SPIP_330-UA/2.2.2.0084), before answer, it will keep the original SIP session up, but you cannot access it from the Polycom user interface, and it won't send RTP, even when connected.

One could argue that this is a success, because the transfer didn't fail because of a party B problem, but I wonder if there are any phones that don't close the call, and do allow it to be retrieved from the user interface.

I wonder if it requires a third state.
By: Digium Subversion (svnbot) 2010-08-18 08:10:38

Repository: asterisk
Revision: 282639

U   branches/1.8/channels/chan_sip.c

------------------------------------------------------------------------
r282639 | mnicholson | 2010-08-18 08:10:37 -0500 (Wed, 18 Aug 2010) | 13 lines

Properly handle 200 and unknown responses conatined in NOTIFY requests received in response to REFER requests.

This patch fixes the way asterisk handles NOTIFY requests received in response to REFER requests.  These changes to NOTIFY handler were first introduced in r217482.  This new change properly handles the 200 response by queueing an AST_TRANSFER_SUCCESS control frame and also prevents that control frame from being queued when provisional and unknown responses are received.

(issue ASTERISK-16226)
Reported by: davidw
Tested by: mnicholson

(issue ASTERISK-12074)
Reported by: davidw

Review: https://reviewboard.asterisk.org/r/860/

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

http://svn.digium.com/view/asterisk?view=rev&revision=282639
By: Digium Subversion (svnbot) 2010-08-18 08:11:37

Repository: asterisk
Revision: 282640

_U  trunk/
U   trunk/channels/chan_sip.c

------------------------------------------------------------------------
r282640 | mnicholson | 2010-08-18 08:11:36 -0500 (Wed, 18 Aug 2010) | 20 lines

Merged revisions 282639 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.8

........
 r282639 | mnicholson | 2010-08-18 08:10:39 -0500 (Wed, 18 Aug 2010) | 13 lines
 
 Properly handle 200 and unknown responses conatined in NOTIFY requests received in response to REFER requests.
 
 This patch fixes the way asterisk handles NOTIFY requests received in response to REFER requests.  These changes to NOTIFY handler were first introduced in r217482.  This new change properly handles the 200 response by queueing an AST_TRANSFER_SUCCESS control frame and also prevents that control frame from being queued when provisional and unknown responses are received.
 
 (issue ASTERISK-16226)
 Reported by: davidw
 Tested by: mnicholson
 
 (issue ASTERISK-12074)
 Reported by: davidw
 
 Review: https://reviewboard.asterisk.org/r/860/
........

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

http://svn.digium.com/view/asterisk?view=rev&revision=282640
By: Russell Bryant (russell) 2011-07-26 15:03:43.809-0500

Per the Asterisk maintenance timeline page at http://www.asterisk.org/asterisk-versions maintenance (bug) support for the 1.4 and 1.6.x branches has ended. For continued maintenance support please move to the 1.8 branch which is a long term support (LTS) branch. For more information about branch support, please see https://wiki.asterisk.org/wiki/display/AST/Asterisk+Versions

If this is still an issue, please open a new issue so it can be re-triaged appropriately. Thanks!