Closed Bug 1587993 Opened 6 years ago Closed 6 years ago

Unencrypted sync credentials and logins on synced device

Categories

(Firefox for Android :: General, task)

ARM
Android
task
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: liuche, Unassigned)

References

Details

(Keywords: sec-audit)

Vision statement / What / Requirements

Fenix is preparing to add Login Sync. For Android 21-23 users, they can't use Android 23's secure key storage, so sync keys and login encryption keys are vulnerable.

In bug 1568690 FFTV is not syncing logins.

(Android 23+ can use Android's keystore.)

Approach

  • Check on token encryption and file system encryption for Android 21-23 & compare to how other Firefox products do this. FWIW Fennec users (w/o master password) have this same vulnerability.
  • Determine if there are additional steps we need to take to ensure account tokens are secure
  • Determine which of Lockwise secure vulnerabilities are also relevant to Fenix (using Gecko engine) from https://bugzilla.mozilla.org/show_bug.cgi?id=1545220

One proposal made in https://github.com/mozilla-mobile/fenix/issues/5550 was to follow Fennec's pattern for Android 21-23, and leave both the sync key and logins encryption key protected by only the platform file permissions. (Does this limit the vulnerability to rooted Android devices?)

Acceptance criteria

  • Tokens are encrypted in an acceptable way for Android 21-23, or risk has been identified and accepted

https://github.com/mozilla-mobile/fenix/issues/5550

An unrooted user can open Firefox for Android (Fennec) and visit file:///data/data/org.mozilla.firefox/shared_prefs/ then tap on the sync.prefs.org!mozilla!firefox!fxa.[hash].default.1.xml to view the Sync secrets.

Firefox Preview (Fenix) does not support file: URIs so unrooted users would not be able to use this to view the data.

Secure key storage didn't begin at API 23, it just got much more full-featured. It actually goes back to API 18. There were also devices back then with hardware-backed credential storage. This included certain Samsung and Nexus devices, like my trusty, old Nexus 4. It's possible that there may be an alternate key storage solution that works back to at least 21 for the most popular devices.

https://developer.android.com/training/articles/keystore

Are there any GDPR requirements for how we store this data? Given the state of Fennec, I'd guess not but it might be worth checking.

Emily Kager will be talking to Matt Miller about the possibility of using the API 18+ AndroidKeyStore on 10/21. One initial concern there is that it might be AES vs RSA encryption.

Let's make sure Thyla is aware of this bug. Asking fro review via secreview@mozilla.com should normally work to get this in front of the right folks.

We attempted to make the Android KeyStore work for <23 with an AES/RSA workaround but in my research about potential key-loss scenarios I found that there are major reported crashes and the official Google response to those bugs is "don't use KeyStore for <23".

This is one of the issues we saw in the wild after releasing code that uses KeyStore on API < 23 (we were not using setEncryptionRequired(true)): https://issuetracker.google.com/issues/37051017

The official response to that bug is Do not use the old keystore APIs. They're broken and deprecated. Use the new ones added in Marshmallow.. :dumpsterfire: https://issuetracker.google.com/issues/37051017#comment18

Emily, I noticed there's an (alpha) Android security library as part of Jetpack that stores keys in the Android KeyStore: do you think this might address the issues developers were seeing in < API 23? I'm not sure if it's flexible enough for our needs though.

Flags: needinfo?(ekager)

After spending a little time on this, I noticed that their MasterKeys API takes a KeyGenParameterSpec, which was an API added in API 23. Sounds like this won't help us now but might help us later, e.g. if they add all the necessary APIs to Jetpack.

Leaving NI for Emily's awareness but no action needed.

Android security library is min API 23 I thought.

Correct I had previously researched this and their min API is still 23 for the jetpack library.
Here is the issue that is tracking lowering that API (with no action since this summer).
https://issuetracker.google.com/issues/132325342

Flags: needinfo?(ekager)

Hi Paul and Daniel.

Here's a doc that Emily and I have put together with the potential attack vectors, as well as our mitigations, and proposed plan.

We haven't implemented the secure storage parts yet, but we've outlined our current state as well as next steps.

https://docs.google.com/document/d/1GekoMBLlhJXx-7NjHyYosYCAVTK3Br1LMES9EDs_Lms/edit#heading=h.msf1bm8ff2yc

Let us know if these seem like reasonable next steps, or if you have suggestions or other risks that we should consider. We're aiming to release Logins in Fenix at the end of Q4.

Flags: needinfo?(ptheriault)
Flags: needinfo?(dveditz)

It's my last day, so I dont have time to review this properly, but from a skim, your proposal seems reasonable. One thing that might be helpful is to enumerate exactly which scenarios you expect to protect against, which you won't (for example, do we expect to protect keys for an attacker who has managed to gain physical access to a phone and unlock/root the phone via the bootlader?)

I don't know how strong our libdataprotect library is, but presumably it was audited along with the rest of lockbox. Reviewing the items raised by the 3rd party review of android that my team ran might be useful (bugs under https://bugzilla.mozilla.org/show_bug.cgi?id=1545220). Frederik Braun is a good contact there in my absence.

Flags: needinfo?(ptheriault)

Hi Freddie, could you take a look at this proposal? Thanks!

Flags: needinfo?(fbraun)

Thank you for this very thorough analysis.

I agree with your first take-aways. To summarize:

  • we should warn all users that want to use fxa features and do not have device security enabled (similar to lockwise dialog)
  • API 23+: libdataprotect/android keystore for is a good path forward.

The API 21-22 thing is tough. I don't have the required background to suggest a good solution. If we end up doing the same as we do on Fennec, then I don't think this protects us against stolen & rooted devices. Just makes it a tiny bit cumbersome.
I'm not sure whether we'd consider that within our threat model, to be honest.

Flags: needinfo?(fbraun)
Flags: needinfo?(dveditz)
Flags: needinfo?(dveditz)

About API 21-22, there is a solution coming. The Jetpack Security library is being updated to support back to KitKat according to the developer who's doing the work who I spoke to in person at DroidConSF. Jetpack Security is simply a frontend to the open source Tink library, which implements key storage on these APIs using a private, cleartext shared preference. This is likely similar to what we're already doing and was a required workaround to serious bugs in the Keystore prior to API 23.

https://github.com/google/tink/blob/master/docs/KNOWN-ISSUES.md

We accepted this risk that we cannot do anything for API 21-22. We have secure FxA storage for API 23+.

Status: NEW → RESOLVED
Closed: 6 years ago
Flags: needinfo?(dveditz)
Resolution: --- → FIXED
Group: core-security-release
Group: mobile-core-security, core-security-release
Component: Security: Android → General
You need to log in before you can comment on or make changes to this bug.