Summary:	ASTERISK-03269: [patch] Normalization of RFC2833 events is not robust
Reporter:	danevans (danevans)	Labels:
Date Opened:	2005-01-14 09:37:44.000-0600	Date Closed:	2008-01-15 15:22:29.000-0600
Priority:	Minor	Regression?	No
Status:	Closed/Complete	Components:	Core/General
Versions:		Frequency of Occurrence
Related Issues:
Environment:		Attachments:	( 0) rtp-fix.diff
Description:	DTMF keys are sent according to RFC2833 as sequences of events that are meant to emulate the press and release of a DTMF key, and provide an indication of how long the key was pressed.  For instance, a SIP phone may send RFC2833 key events to Asterisk that look like this: -- DTMF event 01100000 (key 1, end 16 duration 0) -- DTMF event 01100000 (key 1, end 16 duration 0) -- DTMF event 01100000 (key 1, end 16 duration 0) -- DTMF event 011000a0 (key 1, end 16 duration 160) -- DTMF event 01100140 (key 1, end 16 duration 320) -- DTMF event 011001e0 (key 1, end 16 duration 480) -- DTMF event 01900320 (key 1, end 144 duration 800) -- DTMF event 01900320 (key 1, end 144 duration 800) -- DTMF event 01900320 (key 1, end 144 duration 800) The key, end, and duration values are the decimal values of byte 0, byte 1, and bytes 2 and 3.  This nine event sequence represents one press of the 1 key.  The transition from down to up is represented by the 0x80 bit in byte 1.  Asterisk handles incoming sequences like this very well, but it does not pass them through.  Instead, it "normalizes" them.  Asterisk sends, for a DTMF 1 key, the event sequence: -- DTMF event 010a0000 (key 1, end 10 duration 0) -- DTMF event 018a0320 (key 1, end 138 duration 800) -- DTMF event 018a0320 (key 1, end 138 duration 800) -- DTMF event 018a0320 (key 1, end 138 duration 800) This is one down event and three up events.  This sequence is almost an Asterisk signature, one down and three ups.  The problem with this normalized sequence is that it is not robust.  Since these are RTP packets, if the single down event is dropped, the key sequence is not recognized.  The multiple up events at the end are there for exactly this reason.  If there were only one up event, the same potential problem would exist. ****** ADDITIONAL INFORMATION ****** A diff -u file for a proposed fix is attached.  The fix is a two line change and adds two down events, making the normalized RFC2833 event sequence 3 downs followed by 3 ups.
Comments:	By: Brian West (bkw918) 2005-01-14 10:01:16.000-0600 Not major.   bkw By: danevans (danevans) 2005-01-14 10:40:17.000-0600 At the risk of losing more karma, I'd like to point out that I did not frivolously make the severity "major".  We have a high probablility of losing customers because they cannot reliably enter account numbers, etc., using DTMF (I know, get a better IP connection).  If it were just a change I could make to my Asterisk, and be done with it, fine.  BUT, the change actually needs to be propagated out to providers such as Nufone and Voicepulse that are using Asterisk, and who are sending the DTMF events to companies such as ours. By: nick (nick) 2005-01-14 12:33:40.000-0600 You don't loose karma for unpopular opinions, you loose it for not following the Bug Guidelines. Those guidelines give a clear definition of major, so that we can empirically (that is, both objectively and repeatably) say what is major, rather than have each person choose whether a bug is major to them or not: "MAJOR: A bug which completely prevents Asterisk from operating in a method that it normally is expected to operate -- and particularly if it cannot be reasonably worked around -- is MAJOR. Significant protocol violations that are not simply policy decisions are MAJOR."   If we can empirically classify bugs, developers can easily see which problems most desperately need attention; conversly, if everybody files their bug as major, then an objective system really doesn't exist and developers (with limited time) must read through misclassified bugs and bug marshals (also with limited time) must spend time reclassifying bugs, rather than the testing, coordination, and advaocay that we would otherwise be doing. I just wanted to clear this up for you, because I understand that a lot of new reporters are confused by the guidelines. If you want to discuss this further, there is an #asterisk-bugs channel on IRC where you can always find a bug marshal who will be willing help you choose the right project and severity before you file a bug. Nick By: Mark Spencer (markster) 2005-01-15 02:09:02.000-0600 I have searched the RFC and can find nothing in RFC2833 to suggest that we should be sending > 1 packet other than the section which states we should retransmit the "End" packet 3 times, as we currently do. Do you have a paragraph to support the idea that we have to send at least three downs and three ups? By: nick (nick) 2005-01-15 14:56:22.000-0600 "3.6 Sending Event Packets [snippage] If there has been no new event in the last interval, the event SHOULD be retransmitted three times or until the next event is recognized. This ensures that the duration of the event can be recognized correctly even if the last packet for        an event is lost." Nick By: nick (nick) 2005-01-15 14:57:19.000-0600 Also, do you have a disclaimer? By: danevans (danevans) 2005-01-15 15:15:45.000-0600 Yes, did sign and FAX disclaimer Jan 14, 2005. By: Mark Spencer (markster) 2005-01-15 16:57:06.000-0600 Okay, and we do retransmit at least 3 times until the next event is received, so what's the problem? By: danevans (danevans) 2005-01-17 12:06:34.000-0600 Just want to make sure we don't have a semantic problem.  In RFC2833, retransmitting, since the underlying protocol is UDP, is "sending several times".  Asterisk does retransmit the up (end) event 2 times, providing a total of 3 up events, but does not do the same to the initial down event.  If the down event were also retransmitted as is the up event, it would enable the DTMF key press which is encoded by the events to be recognized even if one or two of the down events were dropped (or the up events), as suggested by the RFC, making the traffic transmitted by Asterisk more robust in the event of packet loss. By: Mark Spencer (markster) 2005-01-17 21:40:42.000-0600 By your own quote: "If there has been no new event in the last interval, the event SHOULD be retransmitted three times or until the next event is recognized. This ensures that the duration of the event can be recognized correctly even if the last packet for an event is lost." So in the first interval we have the "down" event, then we have a "new" event which is the up, immediately following, and then no more events and thus we retransmit twice (perhaps we should *retransmit* a third time), but fundamentally, my reading is that we've done this properly and the other implementation should be able to detect the tone by the "up" event only even in the absense of the down. By: danevans (danevans) 2005-01-17 22:42:44.000-0600 If the single down event that is currently sent is dropped, then there is no transition, no state change.  In this case, a sequence of two digits that are the same is not detectable (i.e. when the down event of the second digit is dropped).  The example in the description came from a SIP phone, and there is more than enough redundancy in the event sequence.  When Asterisk is acting as a proxy, normalization processing eliminates the redundancy in the incoming stream, and makes the output event sequence more prone to error. By: Mark Spencer (markster) 2005-01-18 08:13:01.000-0600 Okay.  The attached patch didn't work, and in any case I think it missed one spot, so feel free to reopen if this isn't it. By: Digium Subversion (svnbot) 2008-01-15 15:22:21.000-0600 Repository: asterisk Revision: 4834 U   trunk/rtp.c ------------------------------------------------------------------------ r4834 | markster | 2008-01-15 15:22:21 -0600 (Tue, 15 Jan 2008) | 2 lines Send a few more rfc2833 events on hte "down" (bug ASTERISK-3269) ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=4834 By: Digium Subversion (svnbot) 2008-01-15 15:22:29.000-0600 Repository: asterisk Revision: 4843 U   branches/v1-0/rtp.c ------------------------------------------------------------------------ r4843 | russell | 2008-01-15 15:22:28 -0600 (Tue, 15 Jan 2008) | 2 lines Send a few more rfc2833 events on hte "down" (bug ASTERISK-3269) ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=4843