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

[Matt Jordan]

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


Update CHANGES and rtp.conf.sample as well


/branches/1.8/res/res_rtp_asterisk.c
<https://reviewboard.asterisk.org/r/1663/#comment9608>

    Add a comment here as to the purpose of strictlearningthreshold (might as well document strictrtp as well)



/branches/1.8/res/res_rtp_asterisk.c
<https://reviewboard.asterisk.org/r/1663/#comment9611>

    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.



/branches/1.8/res/res_rtp_asterisk.c
<https://reviewboard.asterisk.org/r/1663/#comment9612>

    Also, this is funny.



/branches/1.8/res/res_rtp_asterisk.c
<https://reviewboard.asterisk.org/r/1663/#comment9609>

    Don't use atoi - use sscanf instead



/branches/1.8/res/res_rtp_asterisk.c
<https://reviewboard.asterisk.org/r/1663/#comment9610>

    If the user provides an invalid value, you may just want to set strictlearningthreshold back to its default value


- Matt


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/4fe008aa/attachment-0001.htm>