Skip to content
This repository has been archived by the owner on Feb 20, 2023. It is now read-only.

Use Android secure storage (for 23+, or earlier if possible) for FxA state #5938

Closed
2 tasks
liuche opened this issue Oct 11, 2019 · 14 comments
Closed
2 tasks
Labels
E2 Estimation Point: easy, half a day to 2 days Feature:Logins needs:ac Needs Android Component Work 🙅 waiting Issues that are blocked or has dependencies that are not ready

Comments

@liuche
Copy link
Contributor

liuche commented Oct 11, 2019

Why/User Benefit/User Problem

As an Android 23+ user, Android provides secure storage for keys, and I want my sync key to be stored safely in this manner.

Dependencies

What/Requirements

Use AndroidKeyStore to manage Sync keys.

See Lockwise documentation and implementation

Acceptance Criteria (how do I know when I’m done?)

  • Sync keys are stored securely in Android 23+ in Android secure storage.

┆Issue is synchronized with this Jira Task

@liuche liuche added feature request 🌟 New functionality and improvements Feature:Logins labels Oct 11, 2019
@liuche liuche added this to Fenix P1 in Login Management Oct 11, 2019
@liuche liuche added the P1 Current sprint label Oct 11, 2019
@liuche liuche changed the title Use Android secure storage for 23+ Use Android secure storage for 23+ for sync key Oct 11, 2019
@liuche liuche added the must label Oct 14, 2019
@boek boek added this to Prioritized Eng Backlog in Fenix Sprint Kanban Oct 15, 2019
@vesta0 vesta0 removed P1 Current sprint feature request 🌟 New functionality and improvements labels Oct 16, 2019
@vesta0 vesta0 removed this from Must in Fenix Q4 Feature Backlog Oct 16, 2019
@liuche
Copy link
Contributor Author

liuche commented Oct 16, 2019

Wait it actually looks like AndroidKeyStore is 18+! @jhugman do you know if there's anything specific to the 23+ version of the API that Lockwise is using in the LW library (that we wouldn't
https://developer.android.com/training/articles/keystore

@liuche
Copy link
Contributor Author

liuche commented Oct 16, 2019

@ekager will talk to @linuxwolf about what is possible with AndroidKeyStore, and give an update and then we'll size it.

@linuxwolf
Copy link

will discuss with @ekager.

My understanding is AndroidKeyStore was added in API 18, but only supporting RSA keys; API 23 added support for symmetric keys (e..g, AES).

@ekager
Copy link
Contributor

ekager commented Oct 16, 2019

FYI I think this is a prerequisite for #5544 to land because once we set an encryption key for logins we would have to worry about moving users forward if we change the key which could get messy.

@liuche
Copy link
Contributor Author

liuche commented Oct 16, 2019

It sounds like there are 2 keys here: sync key and login encryption key. Is the login key dependent in some way on how we store the sync key?

Why can't we a) store the sync key normally until we figure what our sync key storage story is, and then b) encrypt it, and decrypt it when we pass that into AS?

@grigoryk grigoryk changed the title Use Android secure storage for 23+ for sync key Use Android secure storage (for 23+, or earlier if possible) for FxA state Oct 17, 2019
@grigoryk
Copy link
Contributor

This is indeed a per-requisite for securely storing logins data.

FxA state contains everything you'd need to fetch logins from a sync server. So, encrypting logins storage at rest is moot if you store FxA state in plaintext.

@grigoryk
Copy link
Contributor

Secure storage support for FxA state is tracked in mozilla-mobile/android-components#3982

@grigoryk grigoryk added the needs:ac Needs Android Component Work label Oct 17, 2019
@liuche
Copy link
Contributor Author

liuche commented Oct 17, 2019

@ekager I just talked to grisha and going to summarize here - this does not need to block #5544 as long as we actually get this in place before we ship (but ideally, much sooner than that!).

Our plan here is, generate a key and store it in #5544, and then encrypt it once we get this issue fixed.

@ekager
Copy link
Contributor

ekager commented Oct 17, 2019

Opened #6066 to track the similar issue for securely storing the logins db key.

AC issue here to track lowering the min API for dataprotect
mozilla-mobile/android-components#4758

@liuche
Copy link
Contributor Author

liuche commented Oct 22, 2019

@ekager when you summarize the approach and discussion you had w/ @linuxwolf can you also give an update in https://bugzilla.mozilla.org/show_bug.cgi?id=1587993 ?

@liuche liuche added the 🙅 waiting Issues that are blocked or has dependencies that are not ready label Nov 1, 2019
@liuche liuche moved this from Prioritized Eng Backlog to Prioritized Feature Backlog in Fenix Sprint Kanban Nov 1, 2019
@vesta0 vesta0 added must E2 Estimation Point: easy, half a day to 2 days and removed P1 Current sprint labels Nov 5, 2019
@liuche
Copy link
Contributor Author

liuche commented Nov 5, 2019

Sizing as E2 because this should primarily be done in AC.

@ekager
Copy link
Contributor

ekager commented Nov 19, 2019

This was merged in mozilla-mobile/android-components#5053
and we can close this ticket now

@ekager ekager closed this as completed Nov 19, 2019
@project-bot project-bot bot moved this from Prioritized Feature Backlog to Sprint 11.2 Done in Fenix Sprint Kanban Nov 19, 2019
@grigoryk
Copy link
Contributor

Support for this merged, but we still need to flip the flag in Fenix.

@grigoryk grigoryk reopened this Nov 20, 2019
Fenix Sprint Kanban automation moved this from Sprint 11.2 Done to In Progress Nov 20, 2019
@liuche
Copy link
Contributor Author

liuche commented Nov 22, 2019

@grigoryk i'm going to close this bc it looks like your change landed.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
E2 Estimation Point: easy, half a day to 2 days Feature:Logins needs:ac Needs Android Component Work 🙅 waiting Issues that are blocked or has dependencies that are not ready
Projects
No open projects
Development

No branches or pull requests

5 participants