Security audit for 1545232
Categories
(Firefox Graveyard :: Security: Review Requests, task)
Tracking
(Not tracked)
People
(Reporter: pauljt, Unassigned)
References
Details
Request from PI list:
We would like to ask a security review for the following change:
Add ContentProvider for sharing authentication state
https://bugzilla.mozilla.org/show_bug.cgi?id=1545232This is a patch that will let Fennec share a user's Firefox Account credentials to Fenix, on the same device.
With this patch in place, if you install Fenix next to Fennec, the user can be asked "Do you want to use the same FxA account that you use in Fennec already.". Without additional work from the user, Fenix can then automatically log in the the user's Firefox Account.
We are confident this patch is implemented in a secure way, but we would like to ask for a review nonetheless.
Will get to it ASAP.
Reporter | ||
Comment 1•5 years ago
|
||
Relevant security review notes: https://docs.google.com/document/d/15TJwvNj-hVFNiJ9dxlB5ljZFZ-kuynPjYZ5VOt2-ePM/edit#
Reporter | ||
Comment 2•5 years ago
|
||
See secreview doc for potential testing actions, probably not a high priority but will discuss with team and decide. Grisha will also provide a sample app for testing (as Fenix doesn't implement it's side of the handshake yet).
Comment 3•5 years ago
|
||
See security review doc. https://docs.google.com/document/d/1JpgZIZCm_nODRcBtuMQCWxoCMA0UktWlNa4ByuX8ldo/edit#
The main bit that's unexamined is the way Fenix verifies that Fennec is the legit app from which to request the credentials. The checks going the other direction look fine, and the Android ContentProvider APIs seem well-designed for avoiding obvious flaws like TOCTOU.
Reporter | ||
Comment 4•5 years ago
|
||
Stefan, just a courtesy need-info for you to let us know when the Fenix side of this is complete so we can do end to end testing of the feature.
Updated•5 years ago
|
Comment 5•5 years ago
|
||
Paul, all of the relevant work should now be available for testing in Firefox Preview nightly. Let me know if you need any assistance in testing this.
Comment 6•5 years ago
|
||
Reporter | ||
Updated•5 years ago
|
Reporter | ||
Comment 7•5 years ago
|
||
Thanks, added needinfo so that its in my queue. Ill get to this asap, but probably not until next week.
Reporter | ||
Comment 8•4 years ago
|
||
And then our team was moved, and now I'm gone. I don't know who, if anyone owns mobile security now. This was just left open to perform folllow-up testing after the previous security review. As far as I know this isn't really a thing that we have resources for now, but I'll leave it open in case someone wants to pick it up.
Comment 9•4 years ago
|
||
Closing outdated, unassigned review requests for a non-existent team.
Updated•4 years ago
|
Description
•