[asterisk-dev] DTLS-SRTP support and WebRTC: a report

[Lorenzo Miniero]

Hi Joshua,

I'm glad you appreciated the effort!
I've added just a couple of comments inline.

2013/5/30 Joshua Colp <jcolp at digium.com>

> Lorenzo Miniero wrote:
>
> <snipped>
>
>
>  1. First of all, from a configuration point of view, 'dtlsverify'
>> needs to be set to 'no'. In fact, all certificates used in WebRTC calls
>> from browsers are apparently generated on the fly or, anyway, self
>> certificates, and so cannot be verified. Besides, I also just tested
>> 'ALL:NULL:eNULL:aNULL' as the 'dtlschipher': not sure whether or not
>> this can be safely narrowed down. Not really a technical issue or
>> anything that needs to be addressed in the code, but I thought it was
>> important to point out, if only to notify this in related documentation
>> files.
>>
>
> There is a wiki page detailing WebRTC and Asterisk, but it's from when
> nothing supported DTLS-SRTP. It could certainly be extended to include
> information about this. Of course since stuff is still in flux it'll
> probably become out of date quickly. Playing with a moving target, always
> fun.
>
>
>  2. Asterisk only enforces DTLS-SRTP when it receives
>> UDP/TLS/RTP/SAVP(F) as the protocol to be used in the SDP, but both
>> Chrome and Firefox negotiate RTP/SAVPF. According to RFC 5764, Asterisk
>> is doing the right thing: anyway, UDP/TLS/RTP/SAVP(F) is a SHALL and not
>> a MUST, and besides both browsers don't seem to "digest" it very well.
>> This means that this needs to be addressed: I did it in the signalling
>> gateway, others may do this somewhere else (e.g., directly in JavaScript
>> after producing an offer and before passing an answer). Probably not
>> needed in chan_sip.c as well, even though I'm not sure how other
>> DTLS-SRTP implementations behave in that sense.
>>
>
> Agreed, I'd rather us not break spec. Hopefully the browsers will fix
> things up eventually and if not we can explore what to do then.
>
>
>  3. Even when forced, through the proper "DtlsSrtpKeyAgreement":true
>> constraint, to negotiate DTLS-SRTP, Chrome also adds SDES crypto
>> attributes as a fallback. As far as I've understood from the chan_sip.c
>> code, this could cause Asterisk to try and setup SRTP using both SDES
>> (first) and DTLS (then), with potentially unpredictable results. I have
>> not tested this, as in my signalling gateway I made sure that only the
>> fingerprint attribute was left in the code, but this may be worth
>> looking at. What was definitely needed in the Chrome case, though, was
>> to add a fake crypto line in the reply anyway (e.g., a=crypto:1
>> AES_CM_128_HMAC_SHA1_80
>> inline:**BAADBAADBAADBAADBAADBAADBAADBA**ADBAADBAAD), as was explained in
>> http://www.webrtc.org/interop (note: this doesn't seem to be on that
>> page anymore, so this may have been fixed, but older Chrome versions may
>> still be affected). Again, I did this in the signalling gateway and not
>> Asterisk, so probably not very important.
>>
>
> I'm sure older ones are impacted but it'll probably change as things
> progress.
>
>
>  4. Neither Chrome nor Asterisk provide any 'setup' attribute when
>> negotiating the DTLS fingerprint. According to RFC 5763, this is a MUST,
>> and in fact Asterisk negotiates it instead: nevertheless, both browsers
>> ignore it and don't negotiate the role there, but use a different
>> approach. Specifically, Chrome uses ICE roles to determine
>> passive/active DTLS parties (ICE-CONTROLLED is active), while Firefox
>> assumes that the caller is going to take the DTLS passive role (DTLS
>> server) and the callee the active one (DTLS client). As both approaches
>> basically lead to the same thing, they are able to interact with each
>> other. In order to get this working in Asterisk as well, I made sure
>> that Asterisk was configured as AST_RTP_DTLS_SETUP_ACTIVE for the
>> incoming calls, by adding a setup:passive attribute in the signalling
>> gateway to have it comply with RFC 5763: in fact, as I anticipated I so
>> far only tested calls from browsers to Asterisk. Nevertheless, using a
>> similar approach for outgoing calls (i.e., making sure Asterisk is
>> configured as passive when negotiating DTLS in a call it generates)
>> should do the trick for those scenarios as well.
>>
>
> *nod* Configuring Asterisk as actpass and manipulating the SDP externally
> would do it. It will automatically change.
>
>
>  5. An important aspect I needed to take care of was the right
>> sequence of steps to accomplish, namely: i) ICE completion, ii) DTLS
>> handshake, iii) RTP/RTCP packets flowing. This is what browsers expect,
>> and what Asterisk failed to comply to: the DTLS handshake, for instance,
>> is by default attempted when activating RTP (ast_rtp_activate in
>> res_rtp_asterisk.c), which happens before ICE even starts to work, thus
>> resulting in the handshake never starting at all. In order to fix this,
>> I had to add, to res_rtp_engine.c, an .on_ice_complete callback
>> triggered by PJNATH. This allowed the module to be aware when ICE
>> completed for a specific RTP instance, so that's where I added a new
>> point to start the DTLS handshake for both RTP and RTCP. I also added a
>> 'dtlsdone' variable to the RTP instance to keep track of when the DTLS
>> handshake successfully completed: this was needed to prevent Asterisk
>> from sending RTP/RTCP packets before time, as it seemed to confuse the
>> browsers.
>>
>
> Seems like a sane enough change. If you can clean it up and submit it I'll
> take a gander.
>
>
>

Of course: I'll try and clean up the patch I have now, especially with
respect to point 6, and submit it.
Is it ok to use the same github repository I've created for the Opus/VP8
patch, or would you rather have it submitted somewhere else?



>  6. About the previous point, one more thing I found out is that
>> both browsers expect DTLS to be used to set up both RTP *and* RTCP. This
>> is needed because, as browsers (Chrome at least) mux RTP and RTCP on the
>> same port, when talking to each other they just need a single DTLS
>> handshake for the purpose. Asterisk doesn't mux them instead, and as
>> this was causing calls to fail, I added some very ugly hacks to enforce
>> this for RTCP as well. This means that I basically had to copy all the
>> DTLS-related variables from ast_rtp to the ast_rtcp structure as well,
>> and replicate the calls that were already there for RTP for RTCP as
>> well. This code in particular is especially "ugly" to see right now, but
>> it seems to work fine, meaning that at least it was the right way to go.
>>
>
> Yeah, muxing and ensuring both RTP and RTCP are set up as the browsers
> expect is certainly an ongoing issue. I don't think there's any way around
> what you have done though without going down the full muxing route.
>
>
>
About this, by talking with Chrome developers on the discuss-webrtc Google
group, I found out another issue related to the separate RTP and RTCP DTSL
setup, which also explains why SRTCP doesn't work when using DTLS to
exchange keys. The fact that muxing is not used, also implies that
different keys are negotiated by the browsers via DTLS for RTP and RTCP:
this doesn't happen for SDES, instead, where the SRTP keys are the same for
both RTP and RTCP.