[asterisk-dev] [Code Review] SRTP support for Asterisk

[David Vossel]

> On 2010-05-26 12:29:41, David Vossel wrote:
> > /trunk/channels/chan_iax2.c, lines 13640-13641
> > <https://reviewboard.asterisk.org/r/191/diff/3/?file=9802#file9802line13640>
> >
> >     I'm not sure if this matters or not, but it is possible for the pvt->encmethods to be set but encryption to not be used for the call.  Unless the IAX_FORCE_ENCRYPT flag is set, encryption only occurs if both sides of the call support it.
> 
> Terry Wilson wrote:
>     By the time the call is actually set up (and thus accessible to be queried via the dialplan), would not the calls where merge_encryption() is called and then encmethods set to 0 not ensure that encmethods would only be set if the call itself was using encryption (independent of the settings for the peers)?
>     
>     The encryption stuff in IAX2 is a little hairy. I may be misreading stuff. :-)

This encryption stuff is very hairy.  merge_encryption() is only called when there is an authentication method used, but when we receive an IAX_COMMAND_NEW the pvt's encmethods are guaranteed to be set to 0 regardless.  The only time this may not be the case is if we initiate a call using a guest account and have 'encryption=yes' set in the global space.  If this happens it may be possible for the encmethods to be set and never cleared (or even looked at) because we never request authentication of any kind.  This only applies to outgoing calls.

Since we are guaranteed the call is going to be setup before this is called, you could just use the IAX_CALLENCRYPTED macro to verify the call is encrypted.  This checks to make sure an encryption key was generated which only happens when encryption is actually taking place on the call.


> On 2010-05-26 12:29:41, David Vossel wrote:
> > /trunk/channels/chan_iax2.c, lines 4898-4904
> > <https://reviewboard.asterisk.org/r/191/diff/3/?file=9802#file9802line4898>
> >
> >     I'm not sure why this is necessary.  It should be impossible for this condition to ever happen.   cai.encmethods is set to the iax2_encryption variable, and iax2_encryption is guaranteed to be set if the IAX_FORCE_ENCRYPT flag is set.
> 
> Terry Wilson wrote:
>     If someone calls Set(CHANNEL(secure_bridge_signaling)=1) from the dialplan, then via the channel setoption callback, IAX_FORCE_ENCRYPT will be set. If the call is not set up to actually use encryption, then the call will fail.

Ah, you are correct.  This is necessary because of the changes you have made.  Sorry for the confusion.


- David


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


On 2010-05-04 20:07:15, Terry Wilson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/191/
> -----------------------------------------------------------
> 
> (Updated 2010-05-04 20:07:15)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> SRTP support for Asterisk using Sdescriptions. This has been sitting around for a while, so I figured that it should at least get some review.  Full description of setup at http://lists.digium.com/pipermail/asterisk-dev/2009-January/036029.html
> 
> 
> This addresses bug 5413.
>     https://issues.asterisk.org/view.php?id=5413
> 
> 
> Diffs
> -----
> 
>   /trunk/CHANGES 259665 
>   /trunk/build_tools/menuselect-deps.in 259665 
>   /trunk/channels/chan_iax2.c 259665 
>   /trunk/channels/chan_sip.c 259665 
>   /trunk/channels/sip/dialplan_functions.c 259665 
>   /trunk/channels/sip/include/sdp_crypto.h PRE-CREATION 
>   /trunk/channels/sip/include/sip.h 259665 
>   /trunk/channels/sip/include/srtp.h PRE-CREATION 
>   /trunk/channels/sip/sdp_crypto.c PRE-CREATION 
>   /trunk/channels/sip/srtp.c PRE-CREATION 
>   /trunk/configure UNKNOWN 
>   /trunk/configure.ac 259665 
>   /trunk/funcs/func_channel.c 259665 
>   /trunk/include/asterisk/autoconfig.h.in 259665 
>   /trunk/include/asterisk/frame.h 259665 
>   /trunk/include/asterisk/global_datastores.h 259665 
>   /trunk/include/asterisk/res_srtp.h PRE-CREATION 
>   /trunk/include/asterisk/rtp_engine.h 259665 
>   /trunk/main/asterisk.exports.in 259665 
>   /trunk/main/channel.c 259665 
>   /trunk/main/global_datastores.c 259665 
>   /trunk/main/rtp_engine.c 259665 
>   /trunk/makeopts.in 259665 
>   /trunk/res/res_rtp_asterisk.c 259665 
>   /trunk/res/res_srtp.c PRE-CREATION 
>   /trunk/res/res_srtp.exports.in PRE-CREATION 
> 
> Diff: https://reviewboard.asterisk.org/r/191/diff
> 
> 
> Testing
> -------
> 
> 4 external tests written covering:
> Running with res_srtp noloaded to emulate a user not having libsrtp installed (to make sure we don't accidentally rely on SRTP support)
> Making a call with a user with encrypted=yes when libsrtp support is not enabled fails
> Making a call with encrypted=yes when libsrtp present results in an encrypted call (which also tests the CHANNEL(secure_media) function
> Using CHANNEL(secure_bridge_media) results in the outgoing call attempting to use encryption
> 
> In addition, I have tested a Polycom VVX-1500 to ensure that video + audio SRTP works.
> 
> 
> Thanks,
> 
> Terry
> 
>