Summary:	ASTERISK-27421: RTP source learning not working with devices that have some clock issues
Reporter:	nappsoft (nappsoft)	Labels:	patch
Date Opened:	2017-11-15 06:43:20.000-0600	Date Closed:	2017-11-27 16:15:45.000-0600
Priority:	Minor	Regression?	Yes
Status:	Closed/Complete	Components:	Resources/res_rtp_asterisk
Versions:	13.17.0 13.17.1 13.17.2 13.18.0 13.18.1 13.18.2	Frequency of Occurrence
Related Issues:	is related to ASTERISK-27440 Strictrtp has issues to qualify video rtp streams
Environment:		Attachments:	( 0) accept_jitter.diff ( 1) accept_jitter2.diff
Description:	I've observed some Softphones that do not send packets perfectly frequent but for example with 30ms between the first and the second packet and between the second and the third and <1 ms between the third and the fourth. While a jitterbuffer can handle this perfectly, the RTP source learning algorithm doesn't accept that: it will reset the learning process on every thourd packet as there are less than 5 ms between these packets. (That's not a theory: I just have observed a client that is exactly behaving as described). I've attached a patch that is changing the implementation in a way that not the time between two packets is taken into account to detect a flood, but the time between the first and the last packet during the learning phase. (A timeout lower than 30ms between 4 packets will not be accepted) what solves the described issue and will still detect flood attacks.
Comments:	By: Asterisk Team (asteriskteam) 2017-11-15 06:43:20.560-0600 Thanks for creating a report! The issue has entered the triage process. That means the issue will wait in this status until a Bug Marshal has an opportunity to review the issue. Once the issue has been reviewed you will receive comments regarding the next steps towards resolution. A good first step is for you to review the [Asterisk Issue Guidelines|https://wiki.asterisk.org/wiki/display/AST/Asterisk+Issue+Guidelines] if you haven't already. The guidelines detail what is expected from an Asterisk issue report. Then, if you are submitting a patch, please review the [Patch Contribution Process|https://wiki.asterisk.org/wiki/display/AST/Patch+Contribution+Process]. By: Richard Mudgett (rmudgett) 2017-11-15 12:50:06.186-0600 If you put the change up on Gerrit then it will be reviewed and included faster. Instructions for doing so are on the wiki[1]. By not doing this you are asking someone else to take it through the complete process and with limited resources this can take some time. [1] https://wiki.asterisk.org/wiki/display/AST/Gerrit+Usage By: nappsoft (nappsoft) 2017-11-16 01:23:35.355-0600 reworked patch By: nappsoft (nappsoft) 2017-11-16 01:26:43.551-0600 Ok, I'll try to submit the patch with gerrit. I've reworked the patch a bit to also cover situations in which users overwrite the default for probation. Btw: I guess I've just found another bug (fixed in the same patch): there is a LOG_WARNING if probation is set to a value bellow one telling the user that the default will be used instead. However learning_min_sequential is not set back to DEFAULT_LEARNING_MIN_SEQUENTIAL after the LOG_WARNING. By: nappsoft (nappsoft) 2017-11-16 01:48:36.588-0600 corrected patch (a ; was missing) By: Friendly Automation (friendly-automation) 2017-11-27 16:15:46.549-0600 Change 7237 merged by Jenkins2: res_rtp_asterisk.c: Fix rtp source address learning for broken clients [https://gerrit.asterisk.org/7237|https://gerrit.asterisk.org/7237] By: Friendly Automation (friendly-automation) 2017-11-27 16:29:04.548-0600 Change 7275 merged by Joshua Colp: res_rtp_asterisk.c: Fix rtp source address learning for broken clients [https://gerrit.asterisk.org/7275|https://gerrit.asterisk.org/7275] By: Friendly Automation (friendly-automation) 2017-11-27 16:34:34.248-0600 Change 7276 merged by Jenkins2: res_rtp_asterisk.c: Fix rtp source address learning for broken clients [https://gerrit.asterisk.org/7276|https://gerrit.asterisk.org/7276] By: Friendly Automation (friendly-automation) 2017-12-05 20:09:37.322-0600 Change 7439 merged by Jenkins2: res_rtp_asterisk.c: Fix rtp source address learning for broken clients [https://gerrit.asterisk.org/7439|https://gerrit.asterisk.org/7439]