[Home]

Summary:ASTERISK-16753: [patch] Asterisk fails to recognize SUBSCRIBE retransmissions and tries to re-authenticate them, which breaks presence on polyco
Reporter:mdu113 (mdu113)Labels:
Date Opened:2010-09-29 16:16:11Date Closed:2011-01-04 12:06:48.000-0600
Priority:MinorRegression?No
Status:Closed/CompleteComponents:Channels/chan_sip/Subscriptions
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) astconsole.log
( 1) diff.txt
( 2) poly.log
Description:Here's what happens from asterisk point of view:
-> SUBSCRIBE
<- 401 Unauthorized
-> SUBSCRIBE with auth
<- 200 OK (lost)
<- NOTIFY
-> 200 OK for NOTIFY
-> SUBSCRIBE with auth retransmission
<- 401 Unauthorized
At this point polycom phone received 2 "401 Unauthorized" in a row and it completely gives up on resubscribing to this hint until reboot. While polycom behavior is questionable, I believe asterisk should recognize retransmission and resubmit 200OK in this case

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

astconsole.log - is asterisk console log with sip debug enabled
poly.log is the polycom SIP log of the failed subscription.
It's clearly seen from the poly.log that 200 OK has been lost and retransmission is resulted in a new challenge which caused polycom to terminate subscription
The testing is done with latest svn revision of 1.4 branch - r289177
Comments:By: Terry Wilson (twilson) 2010-11-09 12:19:42.000-0600

Please test the attached diff.txt and see if it fixes the issue for you. Thanks!

By: Ramon Peek-Fares (ramonpeek) 2010-11-10 01:42:31.000-0600

Question:
Did the retransmission indeed occur before the "T2 Timer" (4s.) expired?
(Because the attached logs don't show the time)

Assuming it did...
Then its important to know that I've seen this exact behavior occur on REGISTER requests in Asterisk 1.4.37. So I guess we need to fix this for not only SUBSCRIBES but other transactions too. And I would think this should be fixed in a more general location within the source code.
Also interesting to read is this review: https://reviewboard.asterisk.org/r/687/

By: mdu113 (mdu113) 2010-11-10 15:36:37.000-0600

ramonpeek, polycom retransmits subscribe (just tested) in ~500ms.

the attached patch did fix the problem described. now i'm waiting for the discussion at https://reviewboard.asterisk.org/r/1005/ to get somewhere

Thanks for the patch

By: Digium Subversion (svnbot) 2011-01-04 11:11:50.000-0600

Repository: asterisk
Revision: 300216

U   branches/1.4/channels/chan_sip.c

------------------------------------------------------------------------
r300216 | twilson | 2011-01-04 11:11:49 -0600 (Tue, 04 Jan 2011) | 15 lines

Don't authenticate SUBSCRIBE re-transmissions

This only skips authentication on retransmissions that are already
authenticated. A similar method is already used for INVITES. This
is the kind of thing we end up having to do when we don't have a
transaction layer...

(closes issue ASTERISK-16753)
Reported by: mdu113
Patches:
     diff.txt uploaded by twilson (license 396)
Tested by: twilson, mdu113

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

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

http://svn.digium.com/view/asterisk?view=rev&revision=300216

By: Digium Subversion (svnbot) 2011-01-04 11:37:30.000-0600

Repository: asterisk
Revision: 300298

_U  branches/1.6.2/
U   branches/1.6.2/channels/chan_sip.c

------------------------------------------------------------------------
r300298 | twilson | 2011-01-04 11:37:30 -0600 (Tue, 04 Jan 2011) | 22 lines

Merged revisions 300216 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.4

........
 r300216 | twilson | 2011-01-04 11:11:48 -0600 (Tue, 04 Jan 2011) | 15 lines
 
 Don't authenticate SUBSCRIBE re-transmissions
 
 This only skips authentication on retransmissions that are already
 authenticated. A similar method is already used for INVITES. This
 is the kind of thing we end up having to do when we don't have a
 transaction layer...
 
 (closes issue ASTERISK-16753)
 Reported by: mdu113
 Patches:
       diff.txt uploaded by twilson (license 396)
 Tested by: twilson, mdu113
 
 Review: https://reviewboard.asterisk.org/r/1005/
........

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

http://svn.digium.com/view/asterisk?view=rev&revision=300298

By: Digium Subversion (svnbot) 2011-01-04 11:54:43.000-0600

Repository: asterisk
Revision: 300301

_U  branches/1.8/
U   branches/1.8/channels/chan_sip.c

------------------------------------------------------------------------
r300301 | twilson | 2011-01-04 11:54:43 -0600 (Tue, 04 Jan 2011) | 29 lines

Merged revisions 300298 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.6.2

................
 r300298 | twilson | 2011-01-04 11:37:26 -0600 (Tue, 04 Jan 2011) | 22 lines
 
 Merged revisions 300216 via svnmerge from
 https://origsvn.digium.com/svn/asterisk/branches/1.4
 
 ........
   r300216 | twilson | 2011-01-04 11:11:48 -0600 (Tue, 04 Jan 2011) | 15 lines
   
   Don't authenticate SUBSCRIBE re-transmissions
   
   This only skips authentication on retransmissions that are already
   authenticated. A similar method is already used for INVITES. This
   is the kind of thing we end up having to do when we don't have a
   transaction layer...
   
   (closes issue ASTERISK-16753)
   Reported by: mdu113
   Patches:
         diff.txt uploaded by twilson (license 396)
   Tested by: twilson, mdu113
   
   Review: https://reviewboard.asterisk.org/r/1005/
 ........
................

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

http://svn.digium.com/view/asterisk?view=rev&revision=300301

By: Digium Subversion (svnbot) 2011-01-04 12:06:47.000-0600

Repository: asterisk
Revision: 300302

_U  trunk/
U   trunk/channels/chan_sip.c

------------------------------------------------------------------------
r300302 | twilson | 2011-01-04 12:06:47 -0600 (Tue, 04 Jan 2011) | 36 lines

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

................
 r300301 | twilson | 2011-01-04 11:54:41 -0600 (Tue, 04 Jan 2011) | 29 lines
 
 Merged revisions 300298 via svnmerge from
 https://origsvn.digium.com/svn/asterisk/branches/1.6.2
 
 ................
   r300298 | twilson | 2011-01-04 11:37:26 -0600 (Tue, 04 Jan 2011) | 22 lines
   
   Merged revisions 300216 via svnmerge from
   https://origsvn.digium.com/svn/asterisk/branches/1.4
   
   ........
     r300216 | twilson | 2011-01-04 11:11:48 -0600 (Tue, 04 Jan 2011) | 15 lines
     
     Don't authenticate SUBSCRIBE re-transmissions
     
     This only skips authentication on retransmissions that are already
     authenticated. A similar method is already used for INVITES. This
     is the kind of thing we end up having to do when we don't have a
     transaction layer...
     
     (closes issue ASTERISK-16753)
     Reported by: mdu113
     Patches:
           diff.txt uploaded by twilson (license 396)
     Tested by: twilson, mdu113
     
     Review: https://reviewboard.asterisk.org/r/1005/
   ........
 ................
................

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

http://svn.digium.com/view/asterisk?view=rev&revision=300302