Bug 1253740 - Handle password resets more correctly, r=markh
authorEthan Glasser-Camp <eglassercamp@mozilla.com>
Fri, 30 Sep 2016 12:12:58 -0400
changeset 438343 1c73de6813cd72aff5bd0d390b549a8e49c35a64
parent 438342 aa6abbd36dd156ea933df7c491c7a03af86f617f
child 438344 acd74a4affe15d3dada12918d184e1045cf714e7
push id35679
push userbmo:timdream@gmail.com
push dateMon, 14 Nov 2016 09:29:38 +0000
reviewersmarkh
bugs1253740
milestone52.0a1
Bug 1253740 - Handle password resets more correctly, r=markh MozReview-Commit-ID: 1mSvbsYP9fV
services/sync/modules/engines/extension-storage.js
toolkit/components/extensions/ExtensionStorageSync.jsm
toolkit/components/extensions/test/xpcshell/test_ext_storage_sync.js
--- a/services/sync/modules/engines/extension-storage.js
+++ b/services/sync/modules/engines/extension-storage.js
@@ -222,9 +222,51 @@ class KeyRingEncryptionRemoteTransformer
       let keyMaterial = CryptoUtils.hkdf(kB, undefined,
                                        "identity.mozilla.com/picl/v1/chrome.storage.sync", 2*32);
       let bundle = new BulkKeyBundle();
       // [encryptionKey, hmacKey]
       bundle.keyPair = [keyMaterial.slice(0, 32), keyMaterial.slice(32, 64)];
       return bundle;
     });
   }
+  // Pass through the kbHash field from the unencrypted record. If
+  // encryption fails, we can use this to try to detect whether we are
+  // being compromised or if the record here was encoded with a
+  // different kB.
+  encode(record) {
+    const encodePromise = super.encode(record);
+    return Task.spawn(function* () {
+      const encoded = yield encodePromise;
+      encoded.kbHash = record.kbHash;
+      return encoded;
+    });
+  }
+
+  decode(record) {
+    const decodePromise = super.decode(record);
+    return Task.spawn(function* () {
+      try {
+        return yield decodePromise;
+      } catch (e) {
+        if (Utils.isHMACMismatch(e)) {
+          const currentKBHash = yield ExtensionStorageSync.getKBHash();
+          if (record.kbHash != currentKBHash) {
+            // Some other client encoded this with a kB that we don't
+            // have access to.
+            KeyRingEncryptionRemoteTransformer.throwOutdatedKB(currentKBHash, record.kbHash);
+          }
+        }
+        throw e;
+      }
+    });
+  }
+
+  // Generator and discriminator for KB-is-outdated exceptions.
+  static throwOutdatedKB(shouldBe, is) {
+    throw new Error(`kB hash on record is outdated: should be ${shouldBe}, is ${is}`);
+  }
+
+  static isOutdatedKB(exc) {
+    const kbMessage = "kB hash on record is outdated: ";
+    return exc && exc.message && exc.message.indexOf &&
+      (exc.message.indexOf(kbMessage) == 0);
+  }
 }
--- a/toolkit/components/extensions/ExtensionStorageSync.jsm
+++ b/toolkit/components/extensions/ExtensionStorageSync.jsm
@@ -49,16 +49,18 @@ XPCOMUtils.defineLazyModuleGetter(this, 
 XPCOMUtils.defineLazyModuleGetter(this, "CryptoUtils",
                                   "resource://services-crypto/utils.js");
 XPCOMUtils.defineLazyModuleGetter(this, "EncryptionRemoteTransformer",
                                   "resource://services-sync/engines/extension-storage.js");
 XPCOMUtils.defineLazyModuleGetter(this, "ExtensionStorage",
                                   "resource://gre/modules/ExtensionStorage.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "fxAccounts",
                                   "resource://gre/modules/FxAccounts.jsm");
+XPCOMUtils.defineLazyModuleGetter(this, "KintoHttpClient",
+                                  "resource://services-common/kinto-http-client.js");
 XPCOMUtils.defineLazyModuleGetter(this, "loadKinto",
                                   "resource://services-common/kinto-offline-client.js");
 XPCOMUtils.defineLazyModuleGetter(this, "Log",
                                   "resource://gre/modules/Log.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "Observers",
                                   "resource://services-common/observers.js");
 XPCOMUtils.defineLazyModuleGetter(this, "Sqlite",
                                   "resource://gre/modules/Sqlite.jsm");
@@ -198,34 +200,46 @@ const cryptoCollection = this.cryptoColl
    * You can use this if you want to check metadata on the keyring
    * record rather than use the keyring itself.
    *
    * @returns {Promise<Object>}
    */
   getKeyRingRecord: Task.async(function* () {
     const collection = yield this.getCollection();
     const cryptoKeyRecord = yield collection.getAny(STORAGE_SYNC_CRYPTO_KEYRING_RECORD_ID);
-    return cryptoKeyRecord.data;
+
+    let data = cryptoKeyRecord.data;
+    if (!data) {
+      // This is a new keyring. Invent an ID for this record. If this
+      // changes, it means a client replaced the keyring, so we need to
+      // reupload everything.
+      const uuidgen = Cc["@mozilla.org/uuid-generator;1"].getService(Ci.nsIUUIDGenerator);
+      const uuid = uuidgen.generateUUID();
+      data = {uuid};
+    }
+    return data;
   }),
 
   /**
    * Retrieve the actual keyring from the crypto collection.
    *
    * @returns {Promise<CollectionKeyManager>}
    */
   getKeyRing: Task.async(function* () {
     const cryptoKeyRecord = yield this.getKeyRingRecord();
     const collectionKeys = new CollectionKeyManager();
-    if (cryptoKeyRecord) {
+    if (cryptoKeyRecord.keys) {
       collectionKeys.setContents(cryptoKeyRecord.keys, cryptoKeyRecord.last_modified);
     } else {
       // We never actually use the default key, so it's OK if we
       // generate one multiple times.
       collectionKeys.generateDefaultKey();
     }
+    // Pass through uuid field so that we can save it if we need to.
+    collectionKeys.uuid = cryptoKeyRecord.uuid;
     return collectionKeys;
   }),
 
   updateKBHash: Task.async(function* (kbHash) {
     const coll = yield this.getCollection();
     yield coll.update({id: STORAGE_SYNC_CRYPTO_KEYRING_RECORD_ID,
                        kbHash: kbHash},
                       {patch: true});
@@ -238,16 +252,25 @@ const cryptoCollection = this.cryptoColl
 
   sync: Task.async(function* () {
     const collection = yield this.getCollection();
     return yield ExtensionStorageSync._syncCollection(collection, {
       strategy: "server_wins",
     });
   }),
 
+  /**
+   * Reset sync status for ALL collections by directly
+   * accessing the FirefoxAdapter.
+   */
+  resetSyncStatus: Task.async(function* () {
+    const coll = yield this.getCollection();
+    yield coll.db.resetSyncStatus();
+  }),
+
   // Used only for testing.
   _clear: Task.async(function* () {
     const collection = yield this.getCollection();
     yield collection.clear();
   }),
 };
 
 /**
@@ -461,16 +484,30 @@ this.ExtensionStorageSync = {
         return yield f(newToken);
       }
       // Otherwise, we don't know how to handle this error, so just reraise.
       throw e;
     }
   }),
 
   /**
+   * Helper similar to _syncCollection, but for deleting the user's bucket.
+   */
+  _deleteBucket: Task.async(function* () {
+    return yield this._requestWithToken("Clearing server", function* (token) {
+      const headers = {Authorization: "Bearer " + token};
+      const kintoHttp = new KintoHttpClient(prefStorageSyncServerURL, {
+        headers: headers,
+        timeout: KINTO_REQUEST_TIMEOUT,
+      });
+      return yield kintoHttp.deleteBucket("default");
+    });
+  }),
+
+  /**
    * Recursive promise that terminates when our local collectionKeys,
    * as well as that on the server, have keys for all the extensions
    * in extIds.
    *
    * @param {Array<string>} extIds
    *                        The IDs of the extensions which need keys.
    * @returns {Promise<CollectionKeyManager>}
    */
@@ -480,21 +517,22 @@ this.ExtensionStorageSync = {
       return collectionKeys;
     }
 
     const kbHash = yield this.getKBHash();
     const newKeys = yield collectionKeys.ensureKeysFor(extIds);
     const newRecord = {
       id: STORAGE_SYNC_CRYPTO_KEYRING_RECORD_ID,
       keys: newKeys.asWBO().cleartext,
+      uuid: collectionKeys.uuid,
       // Add a field for the current kB hash.
       kbHash: kbHash,
     };
     yield cryptoCollection.upsert(newRecord);
-    const result = yield cryptoCollection.sync();
+    const result = yield this._syncKeyRing(newRecord);
     if (result.resolved.length != 0) {
       // We had a conflict which was automatically resolved. We now
       // have a new keyring which might have keys for the
       // collections. Recurse.
       return yield this.ensureKeysFor(extIds);
     }
 
     // No conflicts. We're good.
@@ -555,35 +593,76 @@ this.ExtensionStorageSync = {
 
     const cryptoKeyRecord = yield cryptoCollection.getKeyRingRecord();
     if (cryptoKeyRecord && cryptoKeyRecord._status !== "synced") {
       // We haven't successfully synced the keyring since the last
       // change. This could be because kB changed and we touched the
       // keyring, or it could be because we failed to sync after
       // adding a key. Either way, take this opportunity to sync the
       // keyring.
+      yield this._syncKeyRing(cryptoKeyRecord);
+    }
+  }),
+
+  _syncKeyRing: Task.async(function* (cryptoKeyRecord) {
+    try {
+      // Try to sync using server_wins.
       //
       // We use server_wins here because whatever is on the server is
       // at least consistent with itself -- the crypto in the keyring
       // matches the crypto on the collection records. This is because
       // we generate and upload keys just before syncing data.
       //
-      // We can also get into the unhappy situation where we need to
-      // resolve conflicts with a record uploaded by a client with a
-      // different password. In this case, we will fail (throw an
-      // exception) because we can't decrypt the record. This behavior
-      // is correct because we have absolutely no chance of resolving
-      // conflicts intelligently -- in particular, we can't prove that
-      // uploading our key won't erase keys that have already been
-      // used on remote data. If this happens, hopefully the user will
-      // relogin so that all devices have a consistent kB; this will
-      // ensure that the most recent version on the server is
-      // encrypted with the same kB that other devices have, and they
-      // will sync the keyring successfully on subsequent syncs.
-      yield cryptoCollection.sync();
+      // It's possible that we can't decode the version on the server.
+      // This can happen if a user is locked out of their account, and
+      // does a "reset password" to get in on a new device. In this
+      // case, we are in a bind -- we can't decrypt the record on the
+      // server, so we can't merge keys. If this happens, we try to
+      // figure out if we're the one with the correct (new) kB or if
+      // we just got locked out because we have the old kB. If we're
+      // the one with the correct kB, we wipe the server and reupload
+      // everything, including a new keyring.
+      //
+      // If another device has wiped the server, we need to reupload
+      // everything we have on our end too, so we detect this by
+      // adding a UUID to the keyring. UUIDs are preserved throughout
+      // the lifetime of a keyring, so the only time a keyring UUID
+      // changes is when a new keyring is uploaded, which only happens
+      // after a server wipe. So when we get a "conflict" (resolved by
+      // server_wins), we check whether the server version has a new
+      // UUID. If so, reset our sync status, so that we'll reupload
+      // everything.
+      const result = yield cryptoCollection.sync();
+      if (result.resolved.length > 0) {
+        if (result.resolved[0].uuid != cryptoKeyRecord.uuid) {
+          log.info("Detected a new UUID. Reseting sync status for everything.");
+          yield cryptoCollection.resetSyncStatus();
+
+          // Server version is now correct. Return that result.
+          return result;
+        }
+      }
+      // No conflicts, or conflict was just someone else adding keys.
+      return result;
+    } catch (e) {
+      if (KeyRingEncryptionRemoteTransformer.isOutdatedKB(e)) {
+        // Check if our token is still valid, or if we got locked out
+        // between starting the sync and talking to Kinto.
+        const isSessionValid = yield this._fxaService.sessionStatus();
+        if (isSessionValid) {
+          yield this._deleteBucket();
+          yield cryptoCollection.resetSyncStatus();
+
+          // Reupload our keyring, which is the only new keyring.
+          // We don't want client_wins here because another device
+          // could have uploaded another keyring in the meantime.
+          return yield cryptoCollection.sync();
+        }
+      }
+      throw e;
     }
   }),
 
   /**
    * Get the collection for an extension, and register the extension
    * as being "in use".
    *
    * @param {Extension} extension
--- a/toolkit/components/extensions/test/xpcshell/test_ext_storage_sync.js
+++ b/toolkit/components/extensions/test/xpcshell/test_ext_storage_sync.js
@@ -51,32 +51,38 @@ class KintoServer {
     this.collections = new Map();
 
     // ETag to serve with responses
     this.etag = 1;
 
     this.port = this.httpServer.identity.primaryPort;
     // POST requests we receive from the client go here
     this.posts = [];
+    // DELETEd buckets will go here.
+    this.deletedBuckets = [];
     // Anything in here will force the next POST to generate a conflict
     this.conflicts = [];
 
     this.installConfigPath();
     this.installBatchPath();
     this.installCatchAll();
   }
 
   clearPosts() {
     this.posts = [];
   }
 
   getPosts() {
     return this.posts;
   }
 
+  getDeletedBuckets() {
+    return this.deletedBuckets;
+  }
+
   installConfigPath() {
     const configPath = "/v1/";
     const responseBody = JSON.stringify({
       "settings": {"batch_max_requests": 25},
       "url": `http://localhost:${this.port}/v1/`,
       "documentation": "https://kinto.readthedocs.org/",
       "version": "1.5.1",
       "commit": "cbc6f58",
@@ -182,26 +188,63 @@ class KintoServer {
         do_throw(`only GET is supported on ${remoteRecordsPath}`);
       }
 
       response.setStatusLine(null, 200, "OK");
       response.setHeader("Content-Type", "application/json; charset=UTF-8");
       response.setHeader("Date", (new Date()).toUTCString());
       response.setHeader("ETag", this.etag.toString());
 
+      const records = this.collections.get(collectionId);
+      // Can't JSON a Set directly, so convert to Array
+      const data = Array.from(records);
+      for (const record of records) {
+        if (record._onlyOnce) {
+          records.delete(record);
+        }
+      }
+
       const body = JSON.stringify({
-        // Can't JSON a Set directly, so convert to Array
-        "data": Array.from(this.collections.get(collectionId)),
+        "data": data,
       });
       response.write(body);
     }
 
     this.httpServer.registerPathHandler(remoteRecordsPath, handleGetRecords.bind(this));
   }
 
+  installDeleteBucket() {
+    this.httpServer.registerPrefixHandler("/v1/buckets/", (request, response) => {
+      if (request.method != "DELETE") {
+        dump(`got a non-delete action on bucket: ${request.method} ${request.path}\n`);
+        return;
+      }
+
+      const noPrefix = request.path.slice("/v1/buckets/".length);
+      const [bucket, afterBucket] = noPrefix.split("/", 1);
+      if (afterBucket && afterBucket != "") {
+        dump(`got a delete for a non-bucket: ${request.method} ${request.path}\n`);
+      }
+
+      this.deletedBuckets.push(bucket);
+      // Fake like this actually deletes the records.
+      for (const [, set] of this.collections) {
+        set.clear();
+      }
+
+      response.write(JSON.stringify({
+        data: {
+          deleted: true,
+          last_modified: 1475161309026,
+          id: "b09f1618-d789-302d-696e-74ec53ee18a8", // FIXME
+        },
+      }));
+    });
+  }
+
   // Utility function to install a keyring at the start of a test.
   installKeyRing(keysData, etag, {conflict = false} = {}) {
     this.installCollection("storage-sync-crypto");
     const keysRecord = {
       "id": "keys",
       "keys": keysData,
       "last_modified": etag,
     };
@@ -212,16 +255,28 @@ class KintoServer {
   }
 
   encryptAndAddRecord(transformer, collectionId, record) {
     return transformer.encode(record).then(encrypted => {
       this.collections.get(collectionId).add(encrypted);
     });
   }
 
+  // Like encryptAndAddRecord, but add a flag that will only serve
+  // this record once.
+  //
+  // Since in real life, Kinto only serves a record as part of a changes feed
+  // once, this can be useful for testing complicated syncing logic.
+  encryptAndAddRecordOnlyOnce(transformer, collectionId, record) {
+    return transformer.encode(record).then(encrypted => {
+      encrypted._onlyOnce = true;
+      this.collections.get(collectionId).add(encrypted);
+    });
+  }
+
   // Conflicts block the next push and then appear in the collection specified.
   encryptAndAddRecordWithConflict(transformer, collectionId, record) {
     return transformer.encode(record).then(encrypted => {
       this.conflicts.push({collectionId, encrypted});
     });
   }
 
   clearCollection(collectionId) {
@@ -263,16 +318,19 @@ function* withSignedInUser(user, f) {
   const oldERTFxAccounts = EncryptionRemoteTransformer.prototype._fxaService;
   ExtensionStorageSync._fxaService = EncryptionRemoteTransformer.prototype._fxaService = {
     getSignedInUser() {
       return Promise.resolve(user);
     },
     getOAuthToken() {
       return Promise.resolve("some-access-token");
     },
+    sessionStatus() {
+      return Promise.resolve(true);
+    },
   };
 
   try {
     yield* f();
   } finally {
     ExtensionStorageSync._fxaService = oldESSFxAccounts;
     EncryptionRemoteTransformer.prototype._fxaService = oldERTFxAccounts;
   }
@@ -544,22 +602,192 @@ add_task(function* checkSyncKeyRing_reup
     yield* withSignedInUser(loggedInUser, function* () {
       let error;
       try {
         yield new KeyRingEncryptionRemoteTransformer().decode(postedKeys.body.data);
       } catch (e) {
         error = e;
       }
       ok(error, "decrypting the keyring with the old kB should fail");
-      ok(Utils.isHMACMismatch(error),
+      ok(Utils.isHMACMismatch(error) || KeyRingEncryptionRemoteTransformer.isOutdatedKB(error),
          "decrypting the keyring with the old kB should throw an HMAC mismatch");
     });
   });
 });
 
+add_task(function* checkSyncKeyRing_overwrites_on_conflict() {
+  // If there is already a record on the server that was encrypted
+  // with a different kB, we wipe the server, clear sync state, and
+  // overwrite it with our keys.
+  const extensionId = uuid();
+  const transformer = new KeyRingEncryptionRemoteTransformer();
+  let extensionKey;
+  yield* withSyncContext(function* (context) {
+    yield* withServer(function* (server) {
+      // The old device has this kB, which is very similar to the
+      // current kB but with the last f changed to an e.
+      const NOVEL_KB = "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdee";
+      const oldUser = Object.assign({}, loggedInUser, {kB: NOVEL_KB});
+      server.installCollection("storage-sync-crypto");
+      server.installDeleteBucket();
+      server.etag = 765;
+      yield* withSignedInUser(oldUser, function* () {
+        const FAKE_KEYRING = {
+          id: "keys",
+          keys: {},
+          uuid: "abcd",
+          kbHash: "abcd",
+        };
+        yield server.encryptAndAddRecord(transformer, "storage-sync-crypto", FAKE_KEYRING);
+      });
+
+      // Now we have this new user with a different kB.
+      yield* withSignedInUser(loggedInUser, function* () {
+        yield cryptoCollection._clear();
+
+        // Do an `ensureKeysFor` to generate some keys.
+        // This will try to sync, notice that the record is
+        // undecryptable, and clear the server.
+        let collectionKeys = yield ExtensionStorageSync.ensureKeysFor([extensionId]);
+        ok(collectionKeys.hasKeysFor([extensionId]),
+           `ensureKeysFor should always return a keyring with a key for ${extensionId}`);
+        extensionKey = collectionKeys.keyForCollection(extensionId).keyPairB64;
+
+        deepEqual(server.getDeletedBuckets(), ["default"],
+                  "Kinto server should have been wiped when keyring was thrown away");
+
+        let posts = server.getPosts();
+        equal(posts.length, 1,
+             "new keyring should have been uploaded");
+        const postedKeys = posts[0];
+        // The POST was to an empty server, so etag shouldn't be respected
+        equal(postedKeys.headers.Authorization, "Bearer some-access-token",
+              "keyring upload should be authorized");
+        equal(postedKeys.headers["If-None-Match"], "*",
+              "keyring upload should be to empty Kinto server");
+        equal(postedKeys.path, collectionRecordsPath("storage-sync-crypto") + "/keys",
+              "keyring upload should be to keyring path");
+
+        let body = yield new KeyRingEncryptionRemoteTransformer().decode(postedKeys.body.data);
+        ok(body.uuid, "new keyring should have a UUID");
+        notEqual(body.uuid, "abcd",
+                 "new keyring should not have the same UUID as previous keyring");
+        ok(body.keys,
+           "new keyring should have a keys attribute");
+        ok(body.keys.default, "new keyring should have a default key");
+        // We should keep the extension key that was in our uploaded version.
+        deepEqual(extensionKey, body.keys.collections[extensionId],
+                  "ensureKeysFor should have returned keyring with the same key that was uploaded");
+
+        // This should be a no-op; the keys were uploaded as part of ensurekeysfor
+        yield ExtensionStorageSync.checkSyncKeyRing();
+        equal(server.getPosts().length, 1,
+              "checkSyncKeyRing should not need to post keys after they were reuploaded");
+      });
+    });
+  });
+});
+
+add_task(function* checkSyncKeyRing_flushes_on_uuid_change() {
+  // If we can decrypt the record, but the UUID has changed, that
+  // means another client has wiped the server and reuploaded a
+  // keyring, so reset sync state and reupload everything.
+  const extensionId = uuid();
+  const extension = {id: extensionId};
+  const collectionId = extensionIdToCollectionId(loggedInUser, extensionId);
+  const transformer = new KeyRingEncryptionRemoteTransformer();
+  yield* withSyncContext(function* (context) {
+    yield* withServer(function* (server) {
+      server.installCollection("storage-sync-crypto");
+      server.installCollection(collectionId);
+      server.installDeleteBucket();
+      yield* withSignedInUser(loggedInUser, function* () {
+        yield cryptoCollection._clear();
+
+        // Do an `ensureKeysFor` to get access to keys.
+        let collectionKeys = yield ExtensionStorageSync.ensureKeysFor([extensionId]);
+        ok(collectionKeys.hasKeysFor([extensionId]),
+           `ensureKeysFor should always return a keyring that has a key for ${extensionId}`);
+        const extensionKey = collectionKeys.keyForCollection(extensionId).keyPairB64;
+
+        // Set something to make sure that it gets re-uploaded when
+        // uuid changes.
+        yield ExtensionStorageSync.set(extension, {"my-key": 5}, context);
+        yield ExtensionStorageSync.syncAll();
+
+        let posts = server.getPosts();
+        equal(posts.length, 2,
+              "should have posted a new keyring and an extension datum");
+        const postedKeys = posts[0];
+        equal(postedKeys.path, collectionRecordsPath("storage-sync-crypto") + "/keys",
+              "should have posted keyring to /keys");
+
+        let body = yield transformer.decode(postedKeys.body.data);
+        ok(body.uuid,
+           "keyring should have a UUID");
+        ok(body.keys,
+           "keyring should have a keys attribute");
+        ok(body.keys.default,
+           "keyring should have a default key");
+        deepEqual(extensionKey, body.keys.collections[extensionId],
+                  "new keyring should have the same key that we uploaded");
+
+        // Another client comes along and replaces the UUID.
+        // In real life, this would mean changing the keys too, but
+        // this test verifies that just changing the UUID is enough.
+        const newKeyRingData = Object.assign({}, body, {
+          uuid: "abcd",
+          // Technically, last_modified should be served outside the
+          // object, but the transformer will pass it through in
+          // either direction, so this is OK.
+          last_modified: 765,
+        });
+        server.clearCollection("storage-sync-crypto");
+        server.etag = 765;
+        yield server.encryptAndAddRecordOnlyOnce(transformer, "storage-sync-crypto", newKeyRingData);
+
+        // Fake adding another extension just so that the keyring will
+        // really get synced.
+        const newExtension = uuid();
+        const newKeyRing = yield ExtensionStorageSync.ensureKeysFor([newExtension]);
+
+        // This should have detected the UUID change and flushed everything.
+        // The keyring should, however, be the same, since we just
+        // changed the UUID of the previously POSTed one.
+        deepEqual(newKeyRing.keyForCollection(extensionId).keyPairB64, extensionKey,
+                  "ensureKeysFor should have pulled down a new keyring with the same keys");
+
+        // Syncing should reupload the data for the extension.
+        yield ExtensionStorageSync.syncAll();
+        posts = server.getPosts();
+        equal(posts.length, 4,
+              "should have posted keyring for new extension and reuploaded extension data");
+
+        const finalKeyRingPost = posts[2];
+        const reuploadedPost = posts[3];
+
+        equal(finalKeyRingPost.path, collectionRecordsPath("storage-sync-crypto") + "/keys",
+              "keyring for new extension should have been posted to /keys");
+        let finalKeyRing = yield transformer.decode(finalKeyRingPost.body.data);
+        equal(finalKeyRing.uuid, "abcd",
+              "newly uploaded keyring should preserve UUID from replacement keyring");
+
+        // Confirm that the data got reuploaded
+        equal(reuploadedPost.path, collectionRecordsPath(collectionId) + "/key-my_2D_key",
+              "extension data should be posted to path corresponding to its key");
+        let reuploadedData = yield new CollectionKeyEncryptionRemoteTransformer(extensionId).decode(reuploadedPost.body.data);
+        equal(reuploadedData.key, "my-key",
+              "extension data should have a key attribute corresponding to the extension data key");
+        equal(reuploadedData.data, 5,
+              "extension data should have a data attribute corresponding to the extension data value");
+      });
+    });
+  });
+});
+
 add_task(function* test_storage_sync_pulls_changes() {
   const extensionId = defaultExtensionId;
   const collectionId = defaultCollectionId;
   const extension = defaultExtension;
   yield* withContextAndServer(function* (context, server) {
     yield* withSignedInUser(loggedInUser, function* () {
       let transformer = new CollectionKeyEncryptionRemoteTransformer(extensionId);
       server.installCollection(collectionId);