Bug 1017433 (part 2) - allow for upload and download of encrypted sync sentinel. r=rnewman
authorMark Hammond <mhammond@skippinet.com.au>
Wed, 10 Dec 2014 13:02:25 +1100
changeset 218980 d90fa41d351ce1a020d61ccbb9aaece5fc44eccd
parent 218979 961e53b507fe37ca2dc145a8e107253b0eac33f6
child 218981 9acf9c028045d10821a74e268959c821f3afd95f
push id27950
push usercbook@mozilla.com
push dateWed, 10 Dec 2014 10:58:50 +0000
treeherderautoland@5b01216f97f8 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersrnewman
bugs1017433
milestone37.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1017433 (part 2) - allow for upload and download of encrypted sync sentinel. r=rnewman
services/sync/modules/FxaMigrator.jsm
services/sync/modules/service.js
services/sync/tests/unit/test_fxa_migration.js
services/sync/tests/unit/test_fxa_migration_sentinel.js
services/sync/tests/unit/xpcshell.ini
--- a/services/sync/modules/FxaMigrator.jsm
+++ b/services/sync/modules/FxaMigrator.jsm
@@ -244,41 +244,33 @@ Migrator.prototype = {
     yield WeaveService.whenLoaded();
     let signedInUser = yield fxAccounts.getSignedInUser();
     let sentinel = {
       email: signedInUser.email,
       uid: signedInUser.uid,
       verified: signedInUser.verified,
       prefs: this._getSentinelPrefs(),
     };
-    if (Weave.Service.setFxaMigrationSentinel) {
-      yield Weave.Service.setFxaMigrationSentinel(sentinel);
-    } else {
-      this.log.warn("Waiting on bug 1017433; no sync sentinel");
-    }
+    yield Weave.Service.setFxAMigrationSentinel(sentinel);
   }),
 
   /* Ask sync to upload the migration sentinal if we (or any other linked device)
      haven't previously written one.
    */
   _setMigrationSentinelIfNecessary: Task.async(function* () {
     if (!(yield this._getSyncMigrationSentinel())) {
       this.log.info("writing the migration sentinel");
       yield this._setSyncMigrationSentinel();
     }
   }),
 
   /* Ask sync to return a migration sentinel if one exists, otherwise return null */
   _getSyncMigrationSentinel: Task.async(function* () {
     yield WeaveService.whenLoaded();
-    if (!Weave.Service.getFxaMigrationSentinel) {
-      this.log.warn("Waiting on bug 1017433; no sync sentinel");
-      return null;
-    }
-    let sentinel = yield Weave.Service.getFxaMigrationSentinel();
+    let sentinel = yield Weave.Service.getFxAMigrationSentinel();
     this.log.debug("got migration sentinel ${}", sentinel);
     return sentinel;
   }),
 
   _getDefaultAccountName: Task.async(function* (sentinel) {
     // Requires looking to see if other devices have written a migration
     // sentinel (eg, see _haveSynchedMigrationSentinel), and if not, see if
     // the legacy account name appears to be a valid email address (via the
--- a/services/sync/modules/service.js
+++ b/services/sync/modules/service.js
@@ -1275,17 +1275,26 @@ Sync11Service.prototype = {
       synchronizer.sync();
       // wait() throws if the first argument is truthy, which is exactly what
       // we want.
       let result = cb.wait();
 
       histogram = Services.telemetry.getHistogramById("WEAVE_COMPLETE_SUCCESS_COUNT");
       histogram.add(1);
 
-      // We successfully synchronized. Now let's update our declined engines.
+      // We successfully synchronized.
+      // Try and fetch the migration sentinel - it will end up in the recordManager
+      // cache, so a sync migration doesn't need a server round-trip.
+      // If we have no clusterURL, we are probably doing a node reassignment
+      // do don't attempt to get the credentials.
+      if (this.clusterURL) {
+        this.recordManager.get(this.storageURL + "meta/fxa_credentials");
+      }
+
+      // Now let's update our declined engines.
       let meta = this.recordManager.get(this.metaURL);
       if (!meta) {
         this._log.warn("No meta/global; can't update declined state.");
         return;
       }
 
       let declinedEngines = new DeclinedEngines(this);
       let didChange = declinedEngines.updateDeclined(meta, this.engineManager);
@@ -1312,16 +1321,102 @@ Sync11Service.prototype = {
     let response = res.put(meta);
     if (!response.success) {
       throw response;
     }
     this.recordManager.set(this.metaURL, meta);
   },
 
   /**
+   * Get a migration sentinel for the Firefox Accounts migration.
+   * Returns a JSON blob - it is up to callers of this to make sense of the
+   * data.
+   *
+   * Returns a promise that resolves with the sentinel, or null.
+   */
+  getFxAMigrationSentinel: function() {
+    if (this._shouldLogin()) {
+      this._log.debug("In getFxAMigrationSentinel: should login.");
+      if (!this.login()) {
+        this._log.debug("Can't get migration sentinel: login returned false.");
+        return Promise.resolve(null);
+      }
+    }
+    if (!this.identity.syncKeyBundle) {
+      this._log.error("Can't get migration sentinel: no syncKeyBundle.");
+      return Promise.resolve(null);
+    }
+    try {
+      let collectionURL = this.storageURL + "meta/fxa_credentials";
+      let cryptoWrapper = this.recordManager.get(collectionURL);
+      if (!cryptoWrapper.payload) {
+        // nothing to decrypt - .decrypt is noisy in that case, so just bail
+        // now.
+        return Promise.resolve(null);
+      }
+      // If the payload has a sentinel it means we must have put back the
+      // decrypted version last time we were called.
+      if (cryptoWrapper.payload.sentinel) {
+        return Promise.resolve(cryptoWrapper.payload.sentinel);
+      }
+      // If decryption fails it almost certainly means the key is wrong - but
+      // it's not clear if we need to take special action for that case?
+      let payload = cryptoWrapper.decrypt(this.identity.syncKeyBundle);
+      // After decrypting the ciphertext is lost, so we just stash the
+      // decrypted payload back into the wrapper.
+      cryptoWrapper.payload = payload;
+      return Promise.resolve(payload.sentinel);
+    } catch (ex) {
+      this._log.error("Failed to fetch the migration sentinel: ${}", ex);
+      return Promise.resolve(null);
+    }
+  },
+
+  /**
+   * Set a migration sentinel for the Firefox Accounts migration.
+   * Accepts a JSON blob - it is up to callers of this to make sense of the
+   * data.
+   *
+   * Returns a promise that resolves with a boolean which indicates if the
+   * sentinel was successfully written.
+   */
+  setFxAMigrationSentinel: function(sentinel) {
+    if (this._shouldLogin()) {
+      this._log.debug("In setFxAMigrationSentinel: should login.");
+      if (!this.login()) {
+        this._log.debug("Can't set migration sentinel: login returned false.");
+        return Promise.resolve(false);
+      }
+    }
+    if (!this.identity.syncKeyBundle) {
+      this._log.error("Can't set migration sentinel: no syncKeyBundle.");
+      return Promise.resolve(false);
+    }
+    try {
+      let collectionURL = this.storageURL + "meta/fxa_credentials";
+      let cryptoWrapper = new CryptoWrapper("meta", "fxa_credentials");
+      cryptoWrapper.cleartext.sentinel = sentinel;
+
+      cryptoWrapper.encrypt(this.identity.syncKeyBundle);
+
+      let res = this.resource(collectionURL);
+      let response = res.put(cryptoWrapper.toJSON());
+
+      if (!response.success) {
+        throw response;
+      }
+      this.recordManager.set(collectionURL, cryptoWrapper);
+    } catch (ex) {
+      this._log.error("Failed to set the migration sentinel: ${}", ex);
+      return Promise.resolve(false);
+    }
+    return Promise.resolve(true);
+  },
+
+  /**
    * If we have a passphrase, rather than a 25-alphadigit sync key,
    * use the provided sync ID to bootstrap it using PBKDF2.
    *
    * Store the new 'passphrase' back into the identity manager.
    *
    * We can check this as often as we want, because once it's done the
    * check will no longer succeed. It only matters that it happens after
    * we decide to bump the server storage version.
--- a/services/sync/tests/unit/test_fxa_migration.js
+++ b/services/sync/tests/unit/test_fxa_migration.js
@@ -98,30 +98,28 @@ add_task(function *testMigration() {
   Assert.deepEqual((yield fxaMigrator._queueCurrentUserState()), null,
                    "no user state when complete");
 
   // Arrange for a legacy sync user and manually bump the migrator
   let [engine, server] = configureLegacySync();
 
   // monkey-patch the migration sentinel code so we know it was called.
   let haveStartedSentinel = false;
-// (This is waiting on bug 1017433)
-/**
-  let origSetFxaMigrationSentinel = Service.setFxaMigrationSentinel;
+  let origSetFxAMigrationSentinel = Service.setFxAMigrationSentinel;
   let promiseSentinelWritten = new Promise((resolve, reject) => {
-    Service.setFxaMigrationSentinel = function(arg) {
+    Service.setFxAMigrationSentinel = function(arg) {
       haveStartedSentinel = true;
-      return origSetFxaMigrationSentinel.call(Service, arg).then(result => {
-        Service.setFxaMigrationSentinel = origSetFxaMigrationSentinel;
+      return origSetFxAMigrationSentinel.call(Service, arg).then(result => {
+        Service.setFxAMigrationSentinel = origSetFxAMigrationSentinel;
         resolve(result);
         return result;
       });
     }
   });
-**/
+
   // We are now configured for legacy sync, but we aren't in an EOL state yet,
   // so should still be not waiting for a user.
   Assert.deepEqual((yield fxaMigrator._queueCurrentUserState()), null,
                    "no user state before server EOL");
 
   // Start a sync - this will cause an EOL notification which the migrator's
   // observer will notice.
   let promise = promiseOneObserver("fxa-migration:state-changed");
@@ -222,20 +220,17 @@ add_task(function *testMigration() {
 
   Assert.ok(wasWaiting, "everything was good while sync was running.")
 
   // The migration is now going to run to completion.
   // sync should still be "blocked"
   Assert.ok(Service.scheduler.isBlocked, "sync is blocked.");
 
   // We should see the migration sentinel written and it should return true.
-// (This is waiting on bug 1017433)
-/**
   Assert.ok((yield promiseSentinelWritten), "wrote the sentinel");
-**/
 
   // And we should see a new sync start
   yield promiseFinalSync;
 
   // and we should be configured for FxA
   let WeaveService = Cc["@mozilla.org/weave/service;1"]
          .getService(Components.interfaces.nsISupports)
          .wrappedJSObject;
new file mode 100644
--- /dev/null
+++ b/services/sync/tests/unit/test_fxa_migration_sentinel.js
@@ -0,0 +1,150 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+// Test the reading and writing of the sync migration sentinel.
+Cu.import("resource://gre/modules/Promise.jsm");
+Cu.import("resource://gre/modules/FxAccounts.jsm");
+Cu.import("resource://gre/modules/FxAccountsCommon.js");
+
+Cu.import("resource://testing-common/services/sync/utils.js");
+Cu.import("resource://testing-common/services/common/logging.js");
+
+Cu.import("resource://services-sync/record.js");
+
+// Set our username pref early so sync initializes with the legacy provider.
+Services.prefs.setCharPref("services.sync.username", "foo");
+
+// Now import sync
+Cu.import("resource://services-sync/service.js");
+
+const USER = "foo";
+const PASSPHRASE = "abcdeabcdeabcdeabcdeabcdea";
+
+function promiseStopServer(server) {
+  return new Promise((resolve, reject) => {
+    server.stop(resolve);
+  });
+}
+
+let numServerRequests = 0;
+
+// Helpers
+function configureLegacySync() {
+  let contents = {
+    meta: {global: {}},
+    crypto: {},
+  };
+
+  setBasicCredentials(USER, "password", PASSPHRASE);
+
+  numServerRequests = 0;
+  let server = new SyncServer({
+    onRequest: () => {
+      ++numServerRequests
+    }
+  });
+  server.registerUser(USER, "password");
+  server.createContents(USER, contents);
+  server.start();
+
+  Service.serverURL = server.baseURI;
+  Service.clusterURL = server.baseURI;
+  Service.identity.username = USER;
+  Service._updateCachedURLs();
+
+  return server;
+}
+
+// Test a simple round-trip of the get/set functions.
+add_task(function *() {
+  // Arrange for a legacy sync user.
+  let server = configureLegacySync();
+
+  Assert.equal((yield Service.getFxAMigrationSentinel()), null, "no sentinel to start");
+
+  let sentinel = {foo: "bar"};
+  yield Service.setFxAMigrationSentinel(sentinel);
+
+  Assert.deepEqual((yield Service.getFxAMigrationSentinel()), sentinel, "got the sentinel back");
+
+  yield promiseStopServer(server);
+});
+
+// Test the records are cached by the record manager.
+add_task(function *() {
+  // Arrange for a legacy sync user.
+  let server = configureLegacySync();
+  Service.login();
+
+  // Reset the request count here as the login would have made some.
+  numServerRequests = 0;
+
+  Assert.equal((yield Service.getFxAMigrationSentinel()), null, "no sentinel to start");
+  Assert.equal(numServerRequests, 1, "first fetch should hit the server");
+
+  let sentinel = {foo: "bar"};
+  yield Service.setFxAMigrationSentinel(sentinel);
+  Assert.equal(numServerRequests, 2, "setting sentinel should hit the server");
+
+  Assert.deepEqual((yield Service.getFxAMigrationSentinel()), sentinel, "got the sentinel back");
+  Assert.equal(numServerRequests, 2, "second fetch should not should hit the server");
+
+  // Clobber the caches and ensure we still get the correct value back when we
+  // do hit the server.
+  Service.recordManager.clearCache();
+  Assert.deepEqual((yield Service.getFxAMigrationSentinel()), sentinel, "got the sentinel back");
+  Assert.equal(numServerRequests, 3, "should have re-hit the server with empty caches");
+
+  yield promiseStopServer(server);
+});
+
+// Test the records are cached by a sync.
+add_task(function* () {
+  let server = configureLegacySync();
+
+  // A first sync clobbers meta/global due to it being empty, so we first
+  // do a sync which forces a good set of data on the server.
+  Service.sync();
+
+  // Now create a sentinel exists on the server.  It's encrypted, so we need to
+  // put an encrypted version.
+  let cryptoWrapper = new CryptoWrapper("meta", "fxa_credentials");
+  let sentinel = {foo: "bar"};
+  cryptoWrapper.cleartext = {
+    id: "fxa_credentials",
+    sentinel: sentinel,
+    deleted: false,
+  }
+  cryptoWrapper.encrypt(Service.identity.syncKeyBundle);
+  let payload = {
+    ciphertext: cryptoWrapper.ciphertext,
+    IV:         cryptoWrapper.IV,
+    hmac:       cryptoWrapper.hmac,
+  };
+
+  server.createContents(USER, {
+    meta: {fxa_credentials: payload},
+    crypto: {},
+  });
+
+  // Another sync - this will cause the encrypted record to be fetched.
+  Service.sync();
+  // Reset the request count here as the sync will have made many!
+  numServerRequests = 0;
+
+  // Asking for the sentinel should use the copy cached in the record manager.
+  Assert.deepEqual((yield Service.getFxAMigrationSentinel()), sentinel, "got it");
+  Assert.equal(numServerRequests, 0, "should not have hit the server");
+
+  // And asking for it again should work (we have to work around the fact the
+  // ciphertext is clobbered on first decrypt...)
+  Assert.deepEqual((yield Service.getFxAMigrationSentinel()), sentinel, "got it again");
+  Assert.equal(numServerRequests, 0, "should not have hit the server");
+
+  yield promiseStopServer(server);
+});
+
+function run_test() {
+  initTestLogging();
+  run_next_test();
+}
--- a/services/sync/tests/unit/xpcshell.ini
+++ b/services/sync/tests/unit/xpcshell.ini
@@ -171,8 +171,9 @@ skip-if = debug
 [test_healthreport.js]
 skip-if = ! healthreport
 
 [test_warn_on_truncated_response.js]
 
 # FxA migration
 [test_block_sync.js]
 [test_fxa_migration.js]
+[test_fxa_migration_sentinel.js]