[asterisk-dev] [Code Review]: Add a configurable termination threshold to strictrtp's learning mode.

[jrose]

> On Jan. 10, 2012, 7:51 p.m., Matt Jordan wrote:
> > /branches/1.8/res/res_rtp_asterisk.c, lines 2091-2092
> > <https://reviewboard.asterisk.org/r/1663/diff/1/?file=23038#file23038line2091>
> >
> >     Playing devil's advocate here, but this does seem to open up a scenario where we enter into learning mode and never get out.  Say we have Source A and Source B, and the RTP engine has given us a signal to enter into STRICT_RTP_LEARN.  If the RTP packets come in alternating with each other, you'd reset the count each time and, assuming that neither source stops sending packets, never leave it.
> >     
> >     While that's not entirely likely in the context of the various SIP UA's playing nicely, alternatively, you could have an attacker start sending RTP packets at the same time that Source B starts.  Even if Source A stops sending RTP packets, we'd still end up in a situation where we wouldn't leave learning mode, and we'd accept packets from a malicious source indefinitely.
> >     
> >     One way you could alleviate it (to some extent) would be to tie the hit counts to each observed IP address.  You could then maintain separate counts for both new and old IP addresses, as well as flag (through a security event maybe?) if you ever receive RTP packets from more then two "new" sources.
> >     
> >     Not sure if going to all that trouble is worth it or not however.

Yeah, I was worried about something like that.  At the same time though, it should already be theoretically possible to for a miscellaneous party to hijack the socket address in this manner when it first enters learning mode I imagine. 

I'll start tomorrow by trying to add a configurable timeout to learning mode which will revert to the last setting if it fails to learn anything and also track to see if more than two sources get in the conversation loop and log that in the security log... and generate a security event if I can figure that out quickly.


> On Jan. 10, 2012, 7:51 p.m., Matt Jordan wrote:
> > /branches/1.8/res/res_rtp_asterisk.c, lines 2926-2927
> > <https://reviewboard.asterisk.org/r/1663/diff/1/?file=23038#file23038line2926>
> >
> >     Also, this is funny.

Eh, that's RTCP.  I don't think it has anything to do with what I'm working on.


> On Jan. 10, 2012, 7:51 p.m., Matt Jordan wrote:
> > /branches/1.8/res/res_rtp_asterisk.c, line 2953
> > <https://reviewboard.asterisk.org/r/1663/diff/1/?file=23038#file23038line2953>
> >
> >     Don't use atoi - use sscanf instead

Oh, ok.  I was just using another example from nearby to go off of.


> On Jan. 10, 2012, 7:51 p.m., Matt Jordan wrote:
> > /branches/1.8/res/res_rtp_asterisk.c, line 2955
> > <https://reviewboard.asterisk.org/r/1663/diff/1/?file=23038#file23038line2955>
> >
> >     If the user provides an invalid value, you may just want to set strictlearningthreshold back to its default value

Will do.


- jrose


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/1663/#review5156
-----------------------------------------------------------


On Jan. 10, 2012, 4:37 p.m., jrose wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1663/
> -----------------------------------------------------------
> 
> (Updated Jan. 10, 2012, 4:37 p.m.)
> 
> 
> Review request for Asterisk Developers, Kevin Fleming, Matt Jordan, and jcolp.
> 
> 
> Summary
> -------
> 
> The purpose of this patch is to resolve some problems that can occur with peers using a mix of directmedia=yes and no occuring between multiple asterisk servers when strictrtp is enabled in rtp.conf.  When attempting to set the strictrtppeer during a reinvite, res_rtp_asterisk will ignore the requested sockaddr and instead set learning mode. Prior to this patch, learning mode would simply set the sockaddr owning the next related RTP packet received and then exit learning mode.  Now, learning mode will track how many times a particular socket address sent rtp packets without being interrupted by another socket address until it reaches the learning mode hit threshold value (set in rtp.conf) and then resume closed mode.
> 
> 
> Diffs
> -----
> 
>   /branches/1.8/configs/rtp.conf.sample 350307 
>   /branches/1.8/res/res_rtp_asterisk.c 350307 
> 
> Diff: https://reviewboard.asterisk.org/r/1663/diff
> 
> 
> Testing
> -------
> 
> I tested this against two scenarios with the following configurations... always with strictrtp=yes
> 
> s1
> Phone 1 <-- DirectMedia=yes --> Asterisk 1 <-- DirectMedia=Yes --> Asterisk 2 <-- DirectMedia=No --> Phone 3
> 
> and
> 
> s2
> Phone 1 <-- DirectMedia=yes --> Asterisk 1 <-- DirectMedia=Yes --> Phone 2
> and then doing a blind transfer with:
> Phone 2 <-- DirectMedia=yes --> Asterisk 1 <-- DirectMedia=Yes --> Asterisk 2 <-- DirectMedia=No --> Phone 3
> 
> Prior to this test, in s1 there would be one way audio from phone 3 to phone 1 because RTP coming from phone 1 would be rejected by Asterisk 2.
> 
> In s2, audio would work normally from phone 1 to phone 2, but after the transfer the result would be the same with one way audio from phone 3 to phone 1 due to rejected
> packets from phone 1.
> 
> 
> After this patch is applied, both of these scenarios worked as expected as long as I used a threshold above 3.  Phones 1 and 3 were polycom SP430s while Phone 2 was a Grandstream GXP-2020.
> 
> 
> Thanks,
> 
> jrose
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20120111/78ccf249/attachment.htm>