709321 - Generate and upload keys

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: Android Sync, defect, P2)

Comment 1

[Richard Newman [:rnewman]]

See Bug 709313 for a similar situation.

Comment 2

[Nick Alexander :nalexander [he/him] (PTO August 22-September 12)]

I can't guarantee it works, but we have code for it.

Comment 3

[Nick Alexander :nalexander [he/him] (PTO August 22-September 12)]

https://github.com/mozilla-services/android-sync/pull/166

Comment 4

[Damon Sicore (:damons)]

What exactly is this bug for?  Can we get a detailed description?  Why do we need it?  Really strange seeing a blocker with no content.

Comment 5

[Nick Alexander :nalexander [he/him] (PTO August 22-September 12)]

(In reply to Damon Sicore (:damons) from comment #4)
> What exactly is this bug for?  Can we get a detailed description?  Why do we
> need it?  Really strange seeing a blocker with no content.

At the moment, Android Sync does not correctly initialize a blank server.  This ticket implements generating new keys and uploading the "crypto/keys" record to the server if it is not already present.

We need this ticket to be a feature complete sync client that can operate without a companion client that prepares the server for us.

Comment 6

[Richard Newman [:rnewman]]

(In reply to Damon Sicore (:damons) from comment #4)
> What exactly is this bug for?  Can we get a detailed description?  Why do we
> need it?  Really strange seeing a blocker with no content.

Again, see Bug 745430 and Bug 745431 for descriptions of what this is for.

Sync clients encounter blank servers on a routine basis.

Comment 7

[Richard Newman [:rnewman]]

Last iteration through the review cycle.

Comment 8

[Richard Newman [:rnewman]]

Reviewed and queued to land once it can be tested with other work as part of Bug 745430, Bug 745431.

Testing snippets:

16:26:32 < rnewman> https://wiki.mozilla.org/Services/Sync/Snippets#Partially_corrupt_a_server
16:26:47 < rnewman> if you delete meta/global, you should see us freshStart (DELETE /storage) and start over
16:27:14 < rnewman> if you delete crypto/keys, we should generate new keys and blow away data, perhaps by freshStart
16:27:36 < rnewman> use this one: https://wiki.mozilla.org/Services/Sync/Snippets#Bump_meta.2Fglobal.27s_modified_time
16:27:45 < rnewman> to bump the modified time of crypto/keys without changing contents
16:27:48 < rnewman> we should handle that quietly
16:30:28 < rnewman> https://wiki.mozilla.org/Services/Sync/Snippets#Generate_new_keys

Comment 9

[Richard Newman [:rnewman]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/4c42dd1bf025

Let's plan on uplifting this along with the rest of Bug 745430/Bug 745431.

Comment 10

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/4c42dd1bf025

Comment 11

[Nick Alexander :nalexander [he/him] (PTO August 22-September 12)]

Putting this here for lack of a better place.  While testing crypto/keys upload in m-c:
   - sync device to existing account
   - bumped crypto/keys modified time using
Components.utils.import("resource://services-sync/main.js");
Components.utils.import("resource://services-sync/resource.js");
let r = new Resource(Weave.Service.storageURL + "crypto/keys");
let g = r.get();
r.put(g);
   - sync completed just fine, and correctly fetched fresh keys quietly:
I GlobalSession(13112)        Running next stage ensureKeysStage (org.mozilla.gecko.sync.stage.EnsureCrypto5KeysStage@411f9870)...
I EnsureC5KeysStage(13112)    Fetching fresh collection keys for this session.
D dalvikvm(13112)             GC_CONCURRENT freed 556K, 10% free 8254K/9095K, paused 2ms+4ms
I EnsureC5KeysStage(13112)    New default key is the same as old default key; returning changed individual keys.
   - deleted crypto keys using
Components.utils.import("resource://services-sync/main.js");
Components.utils.import("resource://services-sync/resource.js");
function deletePath(path) {
  let resource = new Resource(Weave.Service.storageURL + path);
  resource.setHeader("X-Confirm-Delete", "1");
  return resource.delete();
}
deletePath("crypto/keys");
   - despite misleading logs, we did upload new keys correctly.
I GlobalSession(13112)        Running next stage ensureKeysStage (org.mozilla.gecko.sync.stage.EnsureCrypto5KeysStage@411152a8)...
I EnsureC5KeysStage(13112)    Fetching fresh collection keys for this session.
W SSResourceDelegate(13112)   HTTP request failed.
W SSResourceDelegate(13112)   HTTP response body: 404 Not Found
W SSResourceDelegate(13112)
W SSResourceDelegate(13112)   The resource could not be found.
I EnsureC5KeysStage(13112)    Trying to use persisted collection keys for this session.
I EnsureC5KeysStage(13112)    Using persisted collection keys for this session.
I SyncAdapter(13112)          Stage completed: ensureKeysStage
   - subsequent sync recognizes that there are changed keys:
I GlobalSession(13112)        Running next stage ensureKeysStage (org.mozilla.gecko.sync.stage.EnsureCrypto5KeysStage@411664b0)...
I EnsureC5KeysStage(13112)    Fetching fresh collection keys for this session.
D class ch.boye.httpclientandroidlib.impl.conn.tsccm.ThreadSafeClientConnManager(13112) Shutting down
I EnsureC5KeysStage(13112)    New default key is not the same as old default key.
I EnsureC5KeysStage(13112)    Fetched keys are not the same as persisted keys; setting fetched keys for this session before resetting changed engines.
... resetting stuff ...
W GlobalSession(13112)        Aborting sync: crypto/keys changed on server.
   - but we definitely have problems with clearing our persisted keys,
     and eventually we get HMAC errors.
I GlobalSession(13112)        Running next stage ensureKeysStage (org.mozilla.gecko.sync.stage.EnsureCrypto5KeysStage@41197f20)...
I EnsureC5KeysStage(13112)    Trying to use persisted collection keys for this session.
I EnsureC5KeysStage(13112)    Using persisted collection keys for this session.
I SyncAdapter(13112)          Stage completed: ensureKeysStage
I GlobalSession(13112)        Running next stage syncClientsEngine (org.mozilla.gecko.sync.stage.SyncClientsEngineStage@41165140)...
D dalvikvm(13112)             GC_CONCURRENT freed 501K, 10% free 8272K/9095K, paused 2ms+3ms
W GlobalSession(13112)        Aborting sync: Exception handling client WBO.
W GlobalSession(13112)        org.mozilla.gecko.sync.crypto.HMACVerificationException

Comment 12

[Allison Naaktgeboren :ally]

Sync team: Definitely want to uplift to beta, but not during the first one. We are considering uplifting  709321/323 all together with other dependencies of 745430/431.

Comment 13

[Richard Newman [:rnewman]]

Attachment: Aurora uplift.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 
User impact if declined: 
Testing completed (on m-c, etc.): 
Risk to taking this patch (and alternatives if risky): 
String or UUID changes made by this patch:

Comment 14

[Allison Naaktgeboren :ally]

Comment on attachment 624876 [details] [diff] [review]
Aurora uplift.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 
User impact if declined: 
Testing completed (on m-c, etc.): 
Risk to taking this patch (and alternatives if risky): 
String or UUID changes made by this patch: 

part of the meta/global implementation bundle of release blockers

Comment 15

[Richard Newman [:rnewman]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/6eb02cb3be12

Comment 16

[Tracy Walker [:tracy]]

I got a bunch of warnings on desktop when bumping crypto/keys modified time. But this appears to be ok.