Add ContentProvider for sharing authentication state

RESOLVED FIXED in Firefox 67

Status

()

task
P1
normal
RESOLVED FIXED
3 months ago
2 months ago

People

(Reporter: Grisha, Assigned: Grisha)

Tracking

(Depends on 1 bug)

unspecified
Firefox 68
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox67 fixed, firefox68 fixed)

Details

Attachments

(2 attachments)

Our mobile application ecosystem is growing, and we need a mechanism to share our authentication state with other trusted (read: ours) applications on the system. This greatly helps in reducing on-boarding friction, and overall makes for a smoother user experience.

For example, with the upcoming Fenix launch, it would be nice to be able to ask the user if they want to re-use the FxA account they currently have configured in Fennec.

After some discussions, proposed approach to this is an exported ContentProvider which checks its callers against a signature whitelist. This is somewhat similar to a signature-level ContentProvider readPermission, but with quite a bit more operational flexibility since we're performing all of the necessary checks ourselves. Whitelist will be a map from applicationId to a known certificate signature. We will explicitly not support multiple signers or key rotation for the first iteration of this (refusing access if those are detected).

Users of this ContentProvider are expected to do their due diligence and check that the authority they're relying on is signed with the known Fennec's keys.

Assignee: nobody → gkruglov
Type: defect → task
Priority: -- → P1

See comments in the patch for details.

Attachment #9059155 - Attachment description: Bug 1545232 - Add AuthStateProvider r=nalexander → Bug 1545232 - Add AuthStateProvider r=nalexander,sebastian
Pushed by nalexander@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/dc1cbac5213d
Add AuthStateProvider r=nalexander,sebastian

Ugh. https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel%2Crunnable&revision=dc1cbac5213d6027127892e460a852bbf6e683b3&selectedJob=241370169 has the details. This is the case where lint isn't being very helpful. It's flagging our use of newer APIs (which is gated on version checks) and exporting a content provider without permissions (which is protected by manual signature checks)... Need to figure out how to appease lint.

Flags: needinfo?(gkruglov)
Pushed by nalexander@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/9c712980e970
Add AuthStateProvider r=nalexander,sebastian
Status: NEW → RESOLVED
Closed: 3 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 68

Beta/Release Uplift Approval Request

  • User impact if declined: This work is done in preparation for launching Fenix. This patch allows whitelisted application (currently, release channels of Fenix and Lockwise) to securely access Fennec's authentication information, and thus seamlessly migrate their Firefox Account from Fennec to Fenix (or Lockwise).

If uplift is declined, Fenix release in June will not support any account migration from Fennec. That's the biggest impact, and will negatively impact our FxA users as they're trying out the new browser. This may also negatively impact Fenix launch overall.

  • Is this code covered by automated tests?: No
  • Has the fix been verified in Nightly?: No
  • Needs manual test from QE?: No
  • If yes, steps to reproduce: Due to signature whitelist, testing this requires Fenix support. We don't have support for this in Fenix yet. Tested with non-released applications locally, by adjusting hard-coded whitelist. Once Fenix nightly comes with a basic migration support, we will be able to test two nightlies against each other, and QA will be able to verify.
  • List of other uplifts needed: None
  • Risk to taking this patch: Medium
  • Why is the change risky/not risky? (and alternatives if risky): This patch exposes authentication information (enough to synchronize everything that user has stored in Firefox Sync) to a set of whitelisted applications. This is obviously sec-sensitive.

We - myself, reviewers - are confident that the signature-whitelist approach is good enough to share this information securely among our own applications.

Alternatives to this have been deemed to be riskier (complexity wise), more time consuming (non-feasible given the tight timelines), potentially less secure, and less flexible when considering authentication sharing strategies within our app ecosystem.

  • String changes made/needed: none
Attachment #9060178 - Flags: approval-mozilla-beta?
Attachment #9059155 - Flags: approval-mozilla-beta?

Adding to the above, functionality is quite isolated from everything else in Fennec, and should have no impact on the application. It's also written in a defensive style, to help make sure we won't see unexpected problems in the wild. Consuming side (Fenix, Lockwise) will require a similar whitelist as well, to verify they're interacting with a real Fennec.

Grisha, was there a security review done for this patch?

Flags: needinfo?(gkruglov)

We haven't done a formal security review, because it was deemed unnecessary.

We're relying on basic security guarantees of the android application signing mechanism. The system APIs we're using are partially intended for this specific purpose (different packages verifying trust in each other against their internal whitelists). Through some cursory research (externally and internally), this kind of verification seems like a fairly standard practice in the industry.

Additionally, our specific version of signature verification is as conservative as we can make it.

Flags: needinfo?(gkruglov)
Comment on attachment 9060178 [details] [diff] [review]
auth_state_provider.patch

The patch is low risk at this is prep work for Fenix past the 67 release and is rather isolated from Fennec code, we have added a Trello card for this in our Firefox board so as that other stakeholders have this change on their radar, specifically QA. Uplift approved for 67 beta 15, thanks.
Attachment #9060178 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attachment #9059155 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

I filed a PI Request for a security review. That team can assess what kind of attention this patch needs. I understand it is low risk but I think it will be good to get another pair of security-minded eyes on it.

You need to log in before you can comment on or make changes to this bug.