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