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:11 | Date Closed: | 2011-01-04 12:06:48.000-0600 |
Priority: | Minor | Regression? | No |
Status: | Closed/Complete | Components: | 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 |