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

[Terry Wilson]

> 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.

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.


> On 2010-05-26 12:29:41, David Vossel wrote:
> > /trunk/channels/chan_iax2.c, lines 5150-5152
> > <https://reviewboard.asterisk.org/r/191/diff/3/?file=9802#file9802line5150>
> >
> >     If the IAX_FORCE_ENCRYPT flag is being set here, then it seems like the iaxs[callno]->encmethods should be verified to be set here as well, otherwise it would be possible to have IAX_FORCE_ENCRYPT set with no possible encryption methods for the iax_pvt.... Use the get_encrypt_methods() function to set the encmethods.

This is by design. We are only setting that the dialplan is requesting encryption on this leg of the call. We are not guaranteeing that it is actually possible (hence we fail the call above in the section you commented on above).


> 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.

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. :-)


- Terry


-----------------------------------------------------------
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
> 
>