[asterisk-dev] [Code Review] 4419: SDES-SRTP: Handle SRTP keys negotiated with key lifetime/MKI (oej branch lingon-srtp-key-lifetime-1.8) - Asterisk 11

[Olle E Johansson]

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


Thank you for doing this, Matt! This code has been heavily tested in my 1.8 version. I will compare your code with mine before I press "ship it" if there's been any late changes, but I don't think so. 
A note: All the examples I have in my code could be added as a doxygen comment - I think they are useful for reference. This is not an easy topic and having examples in the source helps anyone wanting to modify bugs we miss in the future.

Hopefully I can backport part of your forward port to my branch :-)

- Olle E Johansson


On Feb. 14, 2015, 4:26 a.m., Matt Jordan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/4419/
> -----------------------------------------------------------
> 
> (Updated Feb. 14, 2015, 4:26 a.m.)
> 
> 
> Review request for Asterisk Developers and Olle E Johansson.
> 
> 
> Bugs: ASTERISK-17721, ASTERISK-17899 and ASTERISK-22748
>     https://issues.asterisk.org/jira/browse/ASTERISK-17721
>     https://issues.asterisk.org/jira/browse/ASTERISK-17899
>     https://issues.asterisk.org/jira/browse/ASTERISK-22748
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> Note that this patch is a forward port of oej's lingon-srtp-key-lifetime-1.8 branch, with a few small modifications to reduce block indentation and a few improvements made to surrounding existing code.
> 
> To quote Olle from ASTERISK-17721:
> 
> {quote}
> The Lingon branch now handle lifetime and MKI parameters.
> 
> We only accept lifetimes up to max for the crypto and higher than 10 hours for packetization of 20 ms (50 pps).
> 
> We only handle MKI with index 1.
> 
> We do not really bother with counting packets and reinviting at end of lifetime, so the min of 10 hours kind of takes care of most calls. If there are longer ones, we rely on the other side for re-invites.
> 
> It's still not perfect, but I personally think this is an improvement. A configuration option for minimum lifetime accepted could be added.
> {quote}
> 
> I personally don't think the minimum lifetime option is needed, as in practice, every instance of an SDP offer for SDES-SRTP with lifetime I've seen offers a lifetime of 2^32 - but it could be added in the future if necessary.
> 
> Note that since the sdp_crypto code was moved to the core in 12+, a separate review (r4418) has been made for the Asterisk 13 variant. It is essentially identical to this review.
> 
> 
> Diffs
> -----
> 
>   /branches/11/channels/sip/sdp_crypto.c 431750 
> 
> Diff: https://reviewboard.asterisk.org/r/4419/diff/
> 
> 
> Testing
> -------
> 
> Tests were added for chan_sip and updated for chan_pjsip - see https://reviewboard.asterisk.org/r/4420 . This includes both nominal and off-nominal offers negotiating SDES-SRTP.
> 
> 
> Thanks,
> 
> Matt Jordan
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20150214/b6901adc/attachment.html>