[Device Pairing Phase 1] Desktop Auth

RESOLVED FIXED in Firefox 67

Status

()

P1
normal
RESOLVED FIXED
6 months ago
4 days ago

People

(Reporter: eoger, Assigned: eoger)

Tracking

(Depends on: 1 bug, Blocks: 1 bug)

unspecified
Firefox 67
Points:
---
Dependency tree / graph
Bug Flags:
qe-verify +

Firefox Tracking Flags

(firefox65- wontfix, firefox66+ wontfix, firefox67 fixed)

Details

Attachments

(3 attachments, 5 obsolete attachments)

(Assignee)

Description

6 months ago
Posted file Implementation plan
Let's implement a prototype with Firefox Desktop as Auth (supplier of keys).
See the milestone 3 from the "Implementation plan" attachment for more information.
This work will also need a fxa-content-server patch, for which I've filed an issue here:

  https://github.com/mozilla/fxa-content-server/issues/6546
(Assignee)

Comment 2

6 months ago
(In reply to Ryan Kelly [:rfkelly] from comment #1)
> This work will also need a fxa-content-server patch, for which I've filed an
> issue here:
> 
>   https://github.com/mozilla/fxa-content-server/issues/6546

Sounds good, let's update the prototype later once this is ready.
Priority: -- → P1
(Assignee)

Comment 3

6 months ago
Posted file Bug 1490671 - wip prototype (obsolete) —
(Assignee)

Comment 4

6 months ago
(Assignee)

Comment 5

6 months ago
(Assignee)

Comment 7

6 months ago
Depends on D6965
Attachment #9010440 - Attachment is obsolete: true
Attachment #9012294 - Attachment is obsolete: true
Attachment #9012297 - Attachment is obsolete: true
Attachment #9012298 - Attachment description: Bug 1490671 p2 - Add JWK/JWE methods and clean jwcrypto → Bug 1490671 p1 - Add JWK/JWE methods and clean jwcrypto.
Attachment #9012299 - Attachment description: Bug 1490671 p3 - Add FxA device pairing → Bug 1490671 p2 - Add FxA device pairing.
(Assignee)

Comment 8

5 months ago
TODO:
- add "fxaccounts:pair_heartbeat" message handling
- use channelID as base64url bytes.
- On decryption error -> display an error to the user
(Assignee)

Updated

5 months ago
Depends on: 1500292
(Assignee)

Comment 9

5 months ago
Patch updated with all review comments and the comment 8's todos.
Not really asking for review yet, as I'd like to test it with Shane's content server branch first.
(Assignee)

Updated

5 months ago
Summary: [Device Pairing] Desktop Auth → [Device Pairing Phase 1] Desktop Auth
Paul, the "p1" patch in this bug is doing a refactor of some of the crypto-related code for Firefox Sync, and adding some utilities for working with JWEs.  Who should we get to take a look at this from a security perspective?
Flags: needinfo?(ptheriault)
Attachment #9012298 - Attachment description: Bug 1490671 p1 - Add JWK/JWE methods and clean jwcrypto. → Bug 1490671 p1 - Refactor CryptoUtils and add JWK/JWE methods to jwcrypto.
(Assignee)

Comment 11

5 months ago
Thanks for the review Ryan, I've updated P1 with your comments.
P2 also got updated with some stuff but is in a limbo state until we decide on the protocol.
I think it would be fine to land p1 when we're ready in the meantime as it cleans a lot of things up.
(In reply to Ryan Kelly [:rfkelly] from comment #10)
> Paul, the "p1" patch in this bug is doing a refactor of some of the
> crypto-related code for Firefox Sync, and adding some utilities for working
> with JWEs.  Who should we get to take a look at this from a security
> perspective?

Hey Ryan, we can, to  start with at least. We have a security testing bug open for this in 1500024 but I haven't had a chance to look at it yet. I'll get someone on it this week, and bring in other security teams as necessary (we might need someone from  JC's team here perhaps, not sure).
Flags: needinfo?(ptheriault)
Depends on: 1500024
(Assignee)

Comment 13

5 months ago
Things to do before landing:

- Finish UX work: intro screen, error screen, strings.
- Make the pairing flow Nightly only: make it impossible to be enabled on other release channels.
- Cleanup (e.g. Remove the old key-data stuff)

Unless Shane objects I believe these patches are ready for formal review.
(In reply to Edouard Oger [:eoger] from comment #13)

> Unless Shane objects I believe these patches are ready for formal review.

Absolutely no objections from me!
There is a small chance this feature will be turned on in Nightly 65 and will be behind a nightly-only flag.
status-firefox65: --- → affected
tracking-firefox65: --- → +
(Assignee)

Comment 16

4 months ago
Depends on D6966
(Assignee)

Comment 17

4 months ago
Patch updated. Things left to do:
- UX work: intro screen, error screens.
- Strings.
Safe to say this isn't targeting 65 at this point?
Flags: needinfo?(eoger)
(Assignee)

Comment 19

3 months ago
Yup probably 66
Flags: needinfo?(eoger)
status-firefox65: affected → wontfix
status-firefox66: --- → affected
tracking-firefox65: + → -
tracking-firefox66: --- → +
(Assignee)

Comment 20

3 months ago
We're still trying to land this in 66.
(Assignee)

Comment 21

3 months ago
Updated with the new crypto lib (do not land).
Things to do:
- Check compat with content server
- Update tests for both p1 and p2.
(Assignee)

Updated

3 months ago
Depends on: 1518300
Attachment #9025150 - Attachment is obsolete: true
(Assignee)

Comment 22

2 months ago

Patches updated with revamped UX [0] and latest protocol changes [1].
This is ready for final review.

[0] https://www.lucidchart.com/documents/view/9a420c19-1e92-42a5-8eae-908a442c1044/1
[1] https://www.lucidchart.com/documents/edit/1bc1b604-0047-4542-8827-ed8518b0433e

Attachment #9012299 - Attachment description: Bug 1490671 p2 - Add FxA device pairing. → Bug 1490671 - Add FxA device pairing.
(Assignee)

Updated

2 months ago
Attachment #9012298 - Attachment is obsolete: true
Depends on: 1523519

Something that just occurred to me here is, do we need to make any changes to the handling of the "fxa_status" webchannel message as part of this work?

When accounts.firefox.com loads in the browser, it sends an "fxa_status" webchannel message to the browser in order to sync up the login state between the browser and web content. The browser code for handling this is here:

https://dxr.mozilla.org/mozilla-central/rev/c2593a3058afdfeaac5c990e18794ee8257afe99/services/fxaccounts/FxAccountsWebChannel.jsm#183

We currently have some logic that ignores this message when in private browsing mode, except if the user is trying to access account management from the sync settings page:

https://dxr.mozilla.org/mozilla-central/rev/c2593a3058afdfeaac5c990e18794ee8257afe99/services/fxaccounts/FxAccountsWebChannel.jsm#

IIUC, the pairing authority broker will depend on the "fxa_status" webchannel message in order to ensure it's using the same sessionToken as the browser itself, to do things like check the user's 2FA status. Do we need to add any logic to ensure that "fxa_status" is handled correctly when the user does the pairing flow in a private browsing window?

What we definitely do not want, is to have web content using one sessionToken for its side of the authority flow, and the browser using a different sessionToken for its side of the flow.

Vlad, thoughts on what might be necessary here from the web-content side of things?

Flags: needinfo?(vlad)

(In reply to Ryan Kelly [:rfkelly] from comment #23)

We looked at this and it looks pretty good. Basically the plan is to add a signal that it's a pairing flow (just like we signal 'sync' at this moment) for the fxa status message.
I'm tracking this as follow up PR to content server and a small patch to desktop. We also want to make sure to land it before pairing hits release. :eoger said it should be okay to land that fix separately as well.

Flags: needinfo?(vlad)
(Assignee)

Updated

2 months ago
Blocks: 1526026

Comment 25

a month ago
Pushed by eoger@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/74922ea29d44
Add FxA device pairing. r=markh,rfkelly,vladikoff,flod

FYI made a v1.0.1 release of fxa-pairing-channel with some small tweaks based on latest discussions:

https://github.com/mozilla/fxa-pairing-channel/releases/tag/v1.0.1

Comment 28

a month ago
Pushed by eoger@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/9dea142f2cc0
Add FxA device pairing. r=markh,rfkelly,vladikoff,flod
(Assignee)

Comment 30

a month ago

Clearing ni?: Re-landed with test fixes and v1.0.1 of the fxa-pairing-channel.

Flags: needinfo?(eoger)

Comment 31

a month ago
Pushed by eoger@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/050422e845df
(Follow-up) Apply some recent subdialog fixes. r=vladikoff

Comment 32

a month ago
bugherder
Status: ASSIGNED → RESOLVED
Last Resolved: a month ago
status-firefox67: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 67

Is this intended to ride with 67 to release or are you looking for beta uplift?

Flags: needinfo?(eoger)

Wontfixing for now but let me know if you feel strongly that it needs to be in 66.

status-firefox66: affected → wontfix
Flags: qe-verify+
(Assignee)

Comment 37

27 days ago

No uplift scheduled, thank you!

Flags: needinfo?(eoger)
Depends on: 1533758

How exactly am I supposed to use this feature? Where do I find this information?
Information needed for validating this bug.

Flags: needinfo?(eoger)

(In reply to Bodea Daniel [:danibodea] from comment #38)

How exactly am I supposed to use this feature? Where do I find this information?
Information needed for validating this bug.

  1. Download Android Reference Browser builds from the following:

arm: https://s3-us-west-2.amazonaws.com/fxa-dev-bucket/fenix-pair/mar18/app-geckoNightly-arm-release.apk
x86: https://s3-us-west-2.amazonaws.com/fxa-dev-bucket/fenix-pair/mar18/app-geckoNightly-x86-release.apk
arm64: https://s3-us-west-2.amazonaws.com/fxa-dev-bucket/fenix-pair/mar18/app-geckoNightly-aarch64-release.apk

  1. Create a new Firefox Nightly Desktop profile, flip this pref to true: identity.fxaccounts.pairing.enabled

  2. Login to Firefox Sync in Desktop

  3. Open the Reference Browser Android, go to Settings -> Pair.

Here's a video guide from an older build: https://www.youtube.com/watch?v=hN-mpyEC7Pg

Flags: needinfo?(eoger)
You need to log in before you can comment on or make changes to this bug.