1568810 - Crash on Sync, on Android Q

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Android Sync, defect)

Description

[Stefan Arentz | :st3fan | ⏰ EST | he/him]

We received the following report from Google:

Steps to reproduce the issue:

1. Download and install the app.
2. Open Firefox
3. Sign in to existing Firefox account & verify log-in from email
4. Open Menu > Settings > Firefox Profile
5. Tap "Sync Now"
6. Observe

Observed Results:

• Application crashes on launch.

Expected Results:

• Button flashes to say syncing for a split second, then back to saying "Sync Now / Last sync: never".

Possible root cause :

• Firefox isn't intersecting their desired set of cipher suites with the result of SSLSocket.getSupportedCipherSuites(), you're trying to enable a cipher suite that is not supported in Q. We had earlier reached out to you regarding this issue and you might have fixed that bug in the launch page. However it seems you haven't applied the fix in the account syncing code.

Comment 1

[Nick Alexander :nalexander [he/him]]

(In reply to Stefan Arentz [:st3fan] from comment #0)

We received the following report from Google:

Steps to reproduce the issue:

1. Download and install the app.
2. Open Firefox
3. Sign in to existing Firefox account & verify log-in from email
4. Open Menu > Settings > Firefox Profile
5. Tap "Sync Now"
6. Observe

Observed Results:

• Application crashes on launch.

Expected Results:

• Button flashes to say syncing for a split second, then back to saying "Sync Now / Last sync: never".

Possible root cause :

• Firefox isn't intersecting their desired set of cipher suites with the result of SSLSocket.getSupportedCipherSuites(), you're trying to enable a cipher suite that is not supported in Q. We had earlier reached out to you regarding this issue and you might have fixed that bug in the launch page. However it seems you haven't applied the fix in the account syncing code.

Way way way back in the day, enumerating the supported cipher suites was so slow that we felt we could not do it, and instead we hard-coded a list. That choice is not working well as the set of cipher suites evolves. We should do as Google suggest and intersect the set we accept with the set the OS provides. We'll want to bump up the logging around this, and perhaps throw a very rich exception, so that we have some hope of debugging this in even less standard configurations.

Petru, I see you handled Bug 1537701, which is in this area. Can you work up a patch to intersect these sets, or delegate to somebody else who can? Thanks!

Comment 2

[Petru-Mugurel Lingurar [:petru]]

Sure, I'll look into it.

Comment 3

[Petru-Mugurel Lingurar [:petru]]

SSLContext.getDefault().getSocketFactory().getSupportedCipherSuites() on a real Pixel 3 with Android Q returns a list which contains all the ones we've enabled and indeed also many others.
I also tried on other emulators with different system images and I couldn't reproduce.

I don't think the issue is with the ciphers we use.

@Stefan, can we get more info about

• the device & Firefox version on which this issue was observed
• and also a logcat for the error?

Comment 4

[Petru-Mugurel Lingurar [:petru]]

Attachment: Bug 1568810 - Intersect platform TLS ciphers with our desired ones; r?nalexander

This would ensure we will not use ciphers not supported by the platform.

Comment 5

[Petru-Mugurel Lingurar [:petru]]

Intersecting our desired TLS ciphers with the ones the platform supports is a nice optimization but I'm still not sure that this was the issue which has caused the crash described in the report.
There is already some logging in the application and I don't think we really need more given that if there is a crash we will probably have a very nice stack trace to follow through to the root cause.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4b92429efe56
Intersect platform TLS ciphers with our desired ones; r=nalexander

Comment 7

[Alexandru Michis [:malexandru]]

https://hg.mozilla.org/mozilla-central/rev/4b92429efe56

Comment 8

[Chris Peterson [:cpeterson]]

Petru, do we need to uplift your cipher crash fix to Fennec ESR 68?

Comment 9

[Petru-Mugurel Lingurar [:petru]]

Comment on attachment 9081198 [details]
Bug 1568810 - Intersect platform TLS ciphers with our desired ones; r?nalexander

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: Speculative crash fix.
• User impact if declined: The app may crash upon reopening after enabling Sync
• Fix Landed on Version: 70
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Small, targeted code change
• String or UUID changes made by this patch:

Comment 10

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9081198 [details]
Bug 1568810 - Intersect platform TLS ciphers with our desired ones; r?nalexander

Potential crash fix for Fennec running on Android Q. Approved for Fennec 68.1b5 and landing now for the 2-Aug nightly builds.

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/43c35ee16e3e

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr68/rev/be125c707b49

Comment 13

[Laurentiu Apahidean]

Tried to reproduce the issue before the fix but I couldn't. As such I am not able to confirm the patch indeed resolved it or not. Will leave the ticket status as FIXED but will not mark it as VERIFIED"

Comment 14

[Andrei Jolțan [:Andi]]

Hi, I wasn't able to reproduce this issue on Firefox 68.1b7 using a Google Pixel XL.