Closed Bug 591118 Opened 11 years ago Closed 11 years ago

Generate Sync Key

Categories

(Firefox :: Sync, defect)

defect
Not set
normal

Tracking

()

VERIFIED FIXED

People

(Reporter: philikon, Assigned: philikon)

References

Details

(Whiteboard: [strings])

Attachments

(3 files, 4 obsolete files)

Instead of requiring the user to enter a passphrase/Sync Key, we want to auto-generate it. a-z is mobile friendly, 20 char length gives us 94 bits.
Component: Sync → Firefox UI
QA Contact: sync → firefox
Note that hyphens need to be ignored when evaluating a sync key.  In the mockup they are displayed for readability, but users may or may not decide to enter them on additional machines.
Similar interfaces provide the hyphens automatically, but we can't really do that since the user can provide their own.  But either way people have been trained to not type them, so we should expect that a certain number of people won't.

Would taking the same approach with spaces mess up our entropy too much for user created sync keys? (in case they enter spaces instead of hyphens?)
A few related questions:
1. If the user opts to have their Sync Key emailed or copies it to clipboard, will it include the hyphens or not?
2. When they are setting up Sync on Android (or Firefox Home), will they have to enter the hyphen or not?
3. Can we gather aggregate information about how many users prefer to go with an auto generated Sync Key vs. creating their own?
>1. If the user opts to have their Sync Key emailed or copies it to clipboard,
>will it include the hyphens or not?

yes, they are important for readability as the user is doing re-entry

>2. When they are setting up Sync on Android (or Firefox Home), will they have
>to enter the hyphen or not?

they may or may not enter them, so we should assume that a certain percentage are not going to, and evaluate what is entered without hyphens (and potentially without spaces if possible).
This ambiguity worries me a bit: "Here's your Sync key nicely hyphenated into 4 groups. But you don't have to enter it that way. Best is you don't worry about the hyphens because they're hard to enter on mobile. In fact, we don't care about the hyphens at all and when you enter your own key, there will be no hyphens. They're not what you think they are."

I think we should either not show the hyphens at all OR make this clearer in the UI. For instance by separating the autogenerated Sync Key case from the custom passphrase case. The initial view of the setup screen would present the Sync Key, but not in an editable textbox. There's a button next to it that says "I want to provide my own". When clicked that makes the Sync Key and the Email/Print/Save buttons disappear and just shows the regular textbox that allows you to enter your own passphrase. For input we would do the same: present four textboxes a la Windows Product Key with a "I have my custom passphrase" button next to it.
Btw, are the Email/Print/Save options final? Are we dropping Copy To Clipboard?
Attached patch WIP v1 (obsolete) — Splinter Review
Work in progress.

Done:
* Generate 20 character (a-z) passphrase
* Implement Email/Print/Save backup options with preliminary text. Print and Save use an HTML page that needs to be styled (bug 591533).

Todo:
* Add hyphens, do not focus passphrase textbox, select all when focusing
* Fix up other parts of the wizard and pref UI where the passphrase can be entered/set.
Assignee: nobody → philipp
Splitting the patch into parts. Part 1 contains the common methods for generating as well as backing up the passphrase/Sync Key.
Attachment #470134 - Attachment is obsolete: true
Attachment #470665 - Flags: review?(mconnor)
This makes the wizard generate the Sync Key (also changed the terminology from passphrase/secret phrase to Sync Key everywhere). You can't continue until you either choose one of the backup methods or provide your own passphrase.
Attachment #470666 - Flags: review?(mconnor)
Attachment #470666 - Flags: feedback?(faaborg)
Fix up the Change/Update Passphrase dialog in the preference pane. The user can open this dialog manually when they want to change their passphrase. Since we now display the current passphrase/Sync Key in clear text, it's also handy to see what the Sync Key is. The three backup methods are available here as well, so this dialog can also be used to redo the backup. The user can also either enter a new passphrase or have a new Sync Key generated.
Attachment #470668 - Flags: review?(mconnor)
Attachment #470668 - Flags: feedback?(faaborg)
Blocks: 592230
Whiteboard: [strings]
Incorporate faaborg's feedback:
* Move error msg for custom passphrases into the box
* Fix up remaining error messages that still mention the "secret phrase"
Attachment #470666 - Attachment is obsolete: true
Attachment #471870 - Flags: review?(mconnor)
Attachment #470666 - Flags: review?(mconnor)
Attachment #470666 - Flags: feedback?(faaborg)
Incorporate faaborg's feedback:
* Rename the "Change Secret Phrase" button to "My Sync Key" as it now allows you to review the key and back it up again, not just change it.
Attachment #470668 - Attachment is obsolete: true
Attachment #471873 - Flags: review?(mconnor)
Attachment #470668 - Flags: review?(mconnor)
Attachment #470668 - Flags: feedback?(faaborg)
Attachment #470665 - Flags: review?(mconnor) → review+
Comment on attachment 471870 [details] [diff] [review]
Part 2 (v2): Generate the Sync Key in the setup wizard

* please just remove the "deprecated" strings.  Cruft is bad.

>+<!ENTITY setup.newSyncKeyPage.description.label "To ensure your total privacy, all of your data is encrypted prior to being uploaded. The key to decrypt your data is not uploaded.">

not quite technically true, at this point.  We generate insanely powerful keys to encrypt data, and wrap those keys with the key we don't update... we really don't want to get into a "well, the UI claims X, but X is a lie" state.
Attachment #471870 - Flags: review?(mconnor) → review+
Blocks: 593427
(In reply to comment #13)
> * please just remove the "deprecated" strings.  Cruft is bad.

Not a good idea since that will break Fx's integrated UI on the next merge. We should remove them only once the integrated UI has caught up with this. Filed bug 593427 as a follow-up.

> >+<!ENTITY setup.newSyncKeyPage.description.label "To ensure your total privacy, all of your data is encrypted prior to being uploaded. The key to decrypt your data is not uploaded.">
> 
> not quite technically true, at this point.  We generate insanely powerful keys
> to encrypt data, and wrap those keys with the key we don't update... we really
> don't want to get into a "well, the UI claims X, but X is a lie" state.

Right. I think what Faaborg means is "The Sync Key which is necessary to decrypt your data is not uploaded." Would that be ok?
Right I understand how it works (so that we only have to reencrpyt the actual key, not the whole chunk of data) bit ultimately it's the sync key that is used to encpyt access to your data.  Text seems acurate enough for me, but we can tweak it.
That's the historical reason that it works that way, though we no longer do key-rewrapping in the general case.  We can massage the wording, but saying something that isn't true feels like lighting a fuse.

cc-ing johnath, in the hopes that he has a better suggestion.
Comment on attachment 471873 [details] [diff] [review]
Part 3 (v2): Fix up Change Passphrase dialog


>+  doGeneratePassphrase: function () {
>+    let passphrase = gWeaveCommon.generatePassphrase();
>+    let el = document.getElementById("passphraseBox");
>+    el.value = gWeaveCommon.hyphenatePassphrase(passphrase);
>+    this._dialog.getButton("accept").disabled = false;
>+  },

I'm actually now wondering if we should move generatePassphrase and friends into Utils.  File a followup to discuss?

>-  validate: function (event) {
>+  validate: function() {

uber-nit: space before ()
Attachment #471873 - Flags: review?(mconnor) → review+
Blocks: 593820
(In reply to comment #17)
> I'm actually now wondering if we should move generatePassphrase and friends
> into Utils.  File a followup to discuss?

Filed bug 593820.
Small rev to address review comment:
Changed setup.newSyncKeyPage.description.label to "The Sync Key which is necessary to decrypt your data is not uploaded." as proposed by myself.
Attachment #471870 - Attachment is obsolete: true
Blocks: 590633
Blocks: 594190
Blocks: 590805
Blocks: 594514
When I enter my keyphrase/key I see what I type in the field. Before, when it was called a passphrase, it wasn't like that.
(In reply to comment #21)
> When I enter my keyphrase/key I see what I type in the field. Before, when it
> was called a passphrase, it wasn't like that.

This is intended. We want to encourage users to back up their Sync Key in a safe manner. Copy'n'paste is a common option which wouldn't work with a password field.
Blocks: 597026
Verified fix on Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:2.0b7pre) Gecko/20100918 Firefox/4.0b7pre
Status: RESOLVED → VERIFIED
Component: Firefox Sync: UI → Sync
Product: Cloud Services → Firefox
You need to log in before you can comment on or make changes to this bug.