1158343 - Temporarily re-enable TLS_RSA_WITH_AES_128_CBC_SHA for WebRTC

Status: RESOLVED FIXED

(Core :: WebRTC: Networking, defect)

Description

[Martin Thomson [:mt:]]

Facebook's use of WebRTC breaks if we don't.

This is intended to be a temporary fix for Firefox 38 (and maybe 38.0.5) only.

Comment 1

[Martin Thomson [:mt:]]

Attachment: bug1158343.patch

This patch applies cleanly to both release and beta as of this moment.

Comment 2

[Eric Rescorla (:ekr)]

Comment on attachment 8597452 [details] [diff] [review]
bug1158343.patch

Review of attachment 8597452 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM. Suggest that we verify with Facebook on m-c before we uplift.

Comment 3

[Maire Reavy [:mreavy]]

(In reply to Eric Rescorla (:ekr) from comment #2)
> Comment on attachment 8597452 [details] [diff] [review]
> bug1158343.patch
> 
> Review of attachment 8597452 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> LGTM. Suggest that we verify with Facebook on m-c before we uplift.

Just to be precise: we don't want to land this on m-c, just Beta (Fx 38).

Martin -- Can you push a try against Beta?  I'll work to get it tested.

Comment 4

[Eric Rescorla (:ekr)]

(In reply to Maire Reavy [:mreavy] (Plz needinfo me) from comment #3)
> (In reply to Eric Rescorla (:ekr) from comment #2)
> > Comment on attachment 8597452 [details] [diff] [review]
> > bug1158343.patch
> > 
> > Review of attachment 8597452 [details] [diff] [review]:
> > -----------------------------------------------------------------
> > 
> > LGTM. Suggest that we verify with Facebook on m-c before we uplift.
> 
> Just to be precise: we don't want to land this on m-c, just Beta (Fx 38).
> 
> Martin -- Can you push a try against Beta?  I'll work to get it tested.

That's fine with me as well.

Comment 5

[Martin Thomson [:mt:]]

OK Maire, this is all yours now.  I'll get you to request all the approvals.  I assume that you have talked to Sylvestre about this already.

Ideally, you should also ask Facebook to verify that this works for them.  I have triggered a build, they should be able to download a build at: https://ftp-ssl.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-5b5546b70a64

(Sorry about the delays, it took me a while to work out what had gone wrong with my local setup that was preventing me from building and running tests.)

Comment 6

[Maire Reavy [:mreavy]]

[Tracking Requested - why for this release]:
In bug 1052610 we enforced requiring PFS cipher suites; this was scheduled to ship in 37 but was delayed to 38 in Bug 1134437.  Testing shows that Facebook iOS app users will be unable to connect to Firefox 38 due to this.  They are updating their app, but I'm concerned there will be insufficient time to deploy and get uptake of the updated app before 38 ships.  

This patch will temporarily delay blocking one suite until 39, which should provide enough time to mitigate the concern.  We expect to back this out in ESR38.

Comment 7

[Philipp Hancke [:fippo]]

Just tested this with messenger on ios and android calling Firefox on facebook.com, this works again with the build by mt.

Comment 8

[Matthew Robertson]

I haven't been able to get the test build running on my mac. Not sure if I am doing something wrong but I keep getting a "damaged and can’t be opened" error. 

It sounds like Philipp was able to confirm the fix. You just need to make a call from the current version of messenger (Android or iOS) and to firefox. If the issue is still happening the media will not start flowing and the person on mobile will see a "connecting" screen before the call eventually fails.

Comment 9

[Maire Reavy [:mreavy]]

Hi Matthew, Like Fippo (Philipp) I was able to confirm that it worked. I was able to download and run the build with no problems. Not sure what's going on with your machine. Before we invest time debugging why the build doesn't work on your machine, I think it makes sense for you to try it on another machine;  I realize that likely means that you won't be able to try this until you get into work on Monday.  Thanks for trying it on the weekend!

Based on Fippo's and my successful testing (two independent tests), I'm going to put this up for Beta approval.

Matthew -- If you can chime in with the results of your testing (assuming you can get it to work on another machine) -- whenever that is, that would be very much appreciated.  And if you continue to have problems, please reach out again either in this bug or irc or email -- whatever is easiest for you.

Comment 10

[Maire Reavy [:mreavy]]

Comment on attachment 8597452 [details] [diff] [review]
bug1158343.patch

Approval Request Comment
[Feature/regressing bug #]: Bug 1052610 disabled non-PFS cipher suites intentionally for improved security.  In our testing we found that apps like Facebook need to be updated to account for this change.  The Facebook fix will likely be rolled out at the same time that Fx 38 is released or slightly after; so we are temporarily re-enabling TLS_RSA_WITH_AES_128_CBC_SHA (in Fx38 and Fx38.0.5) to give more time for the app fix to propagate.
[User impact if declined]: Without this fix, WebRTC calls (e.g. in Facebook) will fail to complete.  User will see "Connecting" but never get a connected call.
[Describe test coverage new/current, TreeHerder]: Manual testing by me and Fippo (from talky.io); we separately confirmed that the fix is working. Facebook is also testing.
[Risks and why]: Very, very small risk and no risk outside of WebRTC.  As you can see from the patch, we are commenting out TLS_RSA_WITH_AES_128_CBC_SHA so that it is permitted as a cipher suite
[String/UUID change made/needed]: No strings

Comment 11

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8597452 [details] [diff] [review]
bug1158343.patch

[Triage Comment]
Thanks! Approved. It should be in 38 beta 9 (and of course 38.0.5).

Comment 12

[Martin Thomson [:mt:]]

I think that the mac builds need to be run from a certain directory, or with DYLD_LIBRARY_PATH set.  Or maybe it's an issue with the lack of signature over them on some machines.  Hard to know without poking and probably not worth the time to work it out.

Leaving the n-i here so that I can remember to check back on this later (and back it out).

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-release/rev/d10817faa571

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/d10817faa571

Comment 15

[Matthew Robertson]

Ok, I got the build working (it was a code signing thing) and I can confirm that Facebook VOIP calling is working perfectly with mt's patch.

Comment 16

[Liz Henry (:lizzard) (relman/hg->git project)]

An update to clarify this situation. This was only enabled for 38, so perfect forward secrecy will be in effect for 39. We need to know if WebRTC works with FB on 39. If not, we should re-enable this for 39 just like we did in 38 until the issue is resolved.

Comment 17

[Eric Rescorla (:ekr)]

(In reply to Liz Henry (:lizzard) from comment #16)
> An update to clarify this situation. This was only enabled for 38, so
> perfect forward secrecy will be in effect for 39. We need to know if WebRTC
> works with FB on 39. If not, we should re-enable this for 39 just like we
> did in 38 until the issue is resolved.

Why isn't the response in c15 sufficient?

Comment 18

[Eric Rescorla (:ekr)]

(In reply to Eric Rescorla (:ekr) from comment #17)
> (In reply to Liz Henry (:lizzard) from comment #16)
> > An update to clarify this situation. This was only enabled for 38, so
> > perfect forward secrecy will be in effect for 39. We need to know if WebRTC
> > works with FB on 39. If not, we should re-enable this for 39 just like we
> > did in 38 until the issue is resolved.
> 
> Why isn't the response in c15 sufficient?

Never mind, I see your point, but I'm not convinced that this is correct.
We explicitly gave Facebook another 6 weeks to update their software, but
that doesn't mean that if they haven't done so, we should give them another
6 weeks. Brad?

Comment 19

[Brad Lassey [:blassey] (use needinfo?)]

Facebook has confirmed that they've pushed their fix such that their VOIP calling is working with 39. There is no need for an additional 6 weeks.

Comment 20

[Maire Reavy [:mreavy]]

Matthew and I have also manually tested that the Facebook app on iOS and Android work with Fx39.