591118 - Generate Sync Key

Status: VERIFIED FIXED

(Firefox :: Sync, defect)

Description

[Philipp von Weitershausen [:philikon]]

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.

Comment 1

[Alex Faaborg [:faaborg] (Firefox UX)]

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.

Comment 2

[Alex Faaborg [:faaborg] (Firefox UX)]

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?)

Comment 3

[Ragavan S [:rags]]

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?

Comment 4

[Alex Faaborg [:faaborg] (Firefox UX)]

>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).

Comment 5

[Philipp von Weitershausen [:philikon]]

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.

Comment 6

[Philipp von Weitershausen [:philikon]]

Btw, are the Email/Print/Save options final? Are we dropping Copy To Clipboard?

Comment 7

[Philipp von Weitershausen [:philikon]]

Attachment: WIP v1

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.

Comment 8

[Philipp von Weitershausen [:philikon]]

Attachment: Part 1 (v1): Implement key generation and backup (email/print/save)

Splitting the patch into parts. Part 1 contains the common methods for generating as well as backing up the passphrase/Sync Key.

Comment 9

[Philipp von Weitershausen [:philikon]]

Attachment: Part 2 (v1): Generate the Sync Key in the setup wizard

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.

Comment 10

[Philipp von Weitershausen [:philikon]]

Attachment: Part 3 (v1): Fix up Change Passphrase dialog

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.

Comment 11

[Philipp von Weitershausen [:philikon]]

Attachment: Part 2 (v2): Generate the Sync Key in the setup wizard

Incorporate faaborg's feedback:
* Move error msg for custom passphrases into the box
* Fix up remaining error messages that still mention the "secret phrase"

Comment 12

[Philipp von Weitershausen [:philikon]]

Attachment: Part 3 (v2): Fix up Change Passphrase dialog

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.

Comment 13

[Mike Connor [:mconnor]]

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.

Comment 14

[Philipp von Weitershausen [:philikon]]

(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?

Comment 15

[Alex Faaborg [:faaborg] (Firefox UX)]

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.

Comment 16

[Mike Connor [:mconnor]]

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 17

[Mike Connor [:mconnor]]

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 ()

Comment 18

[Philipp von Weitershausen [:philikon]]

(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.

Comment 19

[Philipp von Weitershausen [:philikon]]

Attachment: Part 2 (v2.1): Generate the Sync Key in the setup wizard

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.

Comment 20

[Philipp von Weitershausen [:philikon]]

Part 1: http://hg.mozilla.org/services/fx-sync/rev/0e2776059306
Part 2: http://hg.mozilla.org/services/fx-sync/rev/a5ee66c16761
Part 3: http://hg.mozilla.org/services/fx-sync/rev/6a615de6c55f

Comment 21

[juan becerra [:juanb]]

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.

Comment 22

[Philipp von Weitershausen [:philikon]]

(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.

Comment 23

[Tony Chung [:tchung]]

Verified fix on Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:2.0b7pre) Gecko/20100918 Firefox/4.0b7pre