1288246 - Missing DHE ciphers prevent FF 50 from connecting to Spark

Status: RESOLVED FIXED

(Core :: WebRTC, defect, P1)

Description

[Nils Ohlmeier [:drno]]

Cisco Spark calls between Firefox:
- 48 <-> 49 works
- 49 <-> 49 works
- 50 <-> 49 fails

As soon as one side of the call uses Firefox 50 the video rendering of the remote participant on both ends (!) stays grey. And after a couple of seconds Spark terminates the call, without the users hanging up.

Comment 1

[Nils Ohlmeier [:drno]]

Looks like an ICE failure in Nightly. Looking into it...

Comment 2

[Nils Ohlmeier [:drno]]

ICE failure was a user error (to aggressive local firewall). With that properly adjusted the problem remains though.

Comment 3

[Nils Ohlmeier [:drno]]

According to mozregression this is the pushlog which causes the problem:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=b69a5bbb5e40bd426e35222baa600b481e50d265&tochange=0edb9df3c97ff7d205a99f8af607f99a858f140b

Not sure I see in there the obvious commit causing this...

Comment 4

[Nils Ohlmeier [:drno]]

I see DTLS handshake problems in network traces. So I'm guessing that this:
  https://hg.mozilla.org/mozilla-central/rev/1a1d7ef3cb0e
from bug 1279479 is causing the problem.

Comment 5

[Masatoshi Kimura [:emk]]

Can you reproduce this problem on Firefox 49 with DHE cipher suites disabled? (Install <https://addons.mozilla.org/ja/firefox/addon/disable-dhe/> to disable DHE.)

Comment 6

[Nils Ohlmeier [:drno]]

Yes I can repro after installing the 'disbale DHE' addon. In wireshark trace I also see that with FF 49 Spark chooses TLS_DHE_RSA_WITH_AES_128_CBC_SHA as the cipher. Where in FF 50 that cipher is no longer offered and the handshake fails.

As WebRTC enforces PFS ciphers since several releases already we are then left with ECDHE only, which I assume will cause quite a bit of interop problems.

:mt, :ekr what are your thoughts on removing DHE from DTLS?

Comment 7

[Masatoshi Kimura [:emk]]

Chrome dropped the DHE support recently. Will Cisco Spark work with Chrome Canary? If so, do you know what Chrome is using? (QUIC?)

Comment 8

[Nils Ohlmeier [:drno]]

Cisco Spark does not support Chrome.

Comment 9

[Nils Ohlmeier [:drno]]

(In reply to Masatoshi Kimura [:emk] from comment #7)
> Chrome dropped the DHE support recently. Will Cisco Spark work with Chrome
> Canary? If so, do you know what Chrome is using? (QUIC?)

BTW in which version of Chrome is that?
Because I just manually verified with Wireshark that Chrome 52 still offer TLS_DHE_RSA_WITH_AES_128_CBC_SHA plus a whole set of other ciphers in their DTLS Client Hello message send by their WebRTC implementation.

Comment 10

[Nils Ohlmeier [:drno]]

Attachment: Screen Shot 2016-07-21 at 15.38.18.png

And even Chrome 54 Canary still offers TLS_DHE_RSA_WITH_AES_128 and even TLS_RSA_WITH_AES_128_CBC_SHA as you can see in this screenshot.

Comment 11

[Masatoshi Kimura [:emk]]

(In reply to Nils Ohlmeier [:drno] from comment #10)
> And even Chrome 54 Canary still offers TLS_DHE_RSA_WITH_AES_128 and even
> TLS_RSA_WITH_AES_128_CBC_SHA as you can see in this screenshot.

Wow, we will have to offer DHE for DTLS until Cisco or Google change their mind :(

Comment 12

[Masatoshi Kimura [:emk]]

WebRTC has an override list to force-enable some cipher suites:
https://dxr.mozilla.org/mozilla-central/rev/4c05938a64a7fde3ac2d7f4493aee1c5f2ad8a0a/media/mtransport/transportlayerdtls.cpp#660

We can add TLS_DHE_RSA_WITH_AES_128_CBC_SHA to the list.

Comment 13

[Nils Ohlmeier [:drno]]

Attachment: Bug 1288246: added DHE_RSA ciphers back into DTLS handshake.

Review commit: https://reviewboard.mozilla.org/r/66328/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/66328/

Comment 14

[Nils Ohlmeier [:drno]]

(In reply to Masatoshi Kimura [:emk] from comment #12)
> WebRTC has an override list to force-enable some cipher suites:
> https://dxr.mozilla.org/mozilla-central/rev/
> 4c05938a64a7fde3ac2d7f4493aee1c5f2ad8a0a/media/mtransport/transportlayerdtls.
> cpp#660
> 
> We can add TLS_DHE_RSA_WITH_AES_128_CBC_SHA to the list.

Thanks for finding these. That helps a lot. I verified already that adding the ciphers back into the list fixes this issue with Spark.

Comment 15

[Eric Rescorla (:ekr)]

I think instead we should back out the original patch.

Comment 16

[Nils Ohlmeier [:drno]]

(In reply to Eric Rescorla (:ekr) from comment #15)
> I think instead we should back out the original patch.

Sure, I would prefer that. I'll just leave this here as backup, until the original patch is backed out.

Comment 17

[Martin Thomson [:mt:]]

Comment on attachment 8773595 [details]
Bug 1288246: added DHE_RSA ciphers back into DTLS handshake.

https://reviewboard.mozilla.org/r/66328/#review63120

I know that this addresses the problem, but as ekr said, we should look at backing out the change that marked DHE as bad.

Comment 18

[Masatoshi Kimura [:emk]]

Is bug 1227519 WONTFIX, then?

Comment 19

[Eric Rescorla (:ekr)]

That seems like a different question that we can address in the future after we have fixed this bustage.

Comment 20

[Masatoshi Kimura [:emk]]

Attachment: Bug 1288246 - Don't disable fallback cipher suites globally.

The PSM fallback logic should not affect outside PSM. This was not a problem when RC4 was the fallback cipher because DTLS prohibited RC4 from day one.

Review commit: https://reviewboard.mozilla.org/r/66480/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/66480/

Comment 21

[Eric Rescorla (:ekr)]

EMK, there was not agreement to make this change even for HTTPS. Please back out the entire patch and we can discuss afterwards.

Comment 22

[Richard Barnes [:rbarnes]]

Patch has been backed out.

https://bugzilla.mozilla.org/show_bug.cgi?id=1279479#c21

Comment 23

[Nils Ohlmeier [:drno]]

Verified that everything is back to working.