Bug 1253740 - Define checkSyncKeyRing() which reuploads keys when passwords change, r=markh

MozReview-Commit-ID: B5Ptj4MGAC

--HG--
extra : rebase_source : 89a01c22937a19ce66c3993838a8491b7677cefa
extra : intermediate-source : a56017082760c6fbd0242298d37414de0a4aa25d
extra : source : d282a924f350b4d6ea21e3e1b4c9ea2ad09d3df6
This commit is contained in:
Ethan Glasser-Camp 2016-10-03 19:19:13 -04:00
Родитель 50314ee3a6
Коммит dc89d37ad4
2 изменённых файлов: 149 добавлений и 0 удалений

Просмотреть файл

@ -224,6 +224,13 @@ const cryptoCollection = this.cryptoCollection = {
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});
}),
upsert: Task.async(function* (record) {
const collection = yield this.getCollection();
yield collection.upsert(record);
@ -348,6 +355,7 @@ this.ExtensionStorageSync = {
return;
}
yield this.ensureKeysFor(extIds);
yield this.checkSyncKeyRing();
const promises = Array.from(extensionContexts.keys(), extension => {
return openCollection(extension).then(coll => {
return this.sync(extension, coll);
@ -472,10 +480,13 @@ 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,
// Add a field for the current kB hash.
kbHash: kbHash,
};
yield cryptoCollection.upsert(newRecord);
const result = yield cryptoCollection.sync();
@ -490,6 +501,87 @@ this.ExtensionStorageSync = {
return newKeys;
}),
/**
* Get the current user's hashed kB.
*
* @returns sha256 of the user's kB as a hex string
*/
getKBHash: Task.async(function* () {
const signedInUser = yield this._fxaService.getSignedInUser();
if (!signedInUser) {
throw new Error("User isn't signed in!");
}
if (!signedInUser.kB) {
throw new Error("User doesn't have kB??");
}
let kBbytes = CommonUtils.hexToBytes(signedInUser.kB);
let hasher = Cc["@mozilla.org/security/hash;1"]
.createInstance(Ci.nsICryptoHash);
hasher.init(hasher.SHA256);
return CommonUtils.bytesAsHex(CryptoUtils.digestBytes(signedInUser.uid + kBbytes, hasher));
}),
/**
* Update the kB in the crypto record.
*/
updateKeyRingKB: Task.async(function* () {
const signedInUser = yield this._fxaService.getSignedInUser();
if (!signedInUser) {
// Although this function is meant to be called on login,
// it's not unreasonable to check any time, even if we aren't
// logged in.
//
// If we aren't logged in, we don't have any information about
// the user's kB, so we can't be sure that the user changed
// their kB, so just return.
return;
}
const thisKBHash = yield this.getKBHash();
yield cryptoCollection.updateKBHash(thisKBHash);
}),
/**
* Make sure the keyring is up to date and synced.
*
* This is called on syncs to make sure that we don't sync anything
* to any collection unless the key for that collection is on the
* server.
*/
checkSyncKeyRing: Task.async(function* () {
yield this.updateKeyRingKB();
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.
//
// 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();
}
}),
/**
* Get the collection for an extension, and register the extension
* as being "in use".

Просмотреть файл

@ -17,9 +17,11 @@ const {
} = Cu.import("resource://gre/modules/ExtensionStorageSync.jsm");
Cu.import("resource://services-sync/engines/extension-storage.js");
Cu.import("resource://services-sync/keys.js");
Cu.import("resource://services-sync/util.js");
/* globals BulkKeyBundle, CommonUtils, EncryptionRemoteTransformer */
/* globals KeyRingEncryptionRemoteTransformer */
/* globals Utils */
function handleCannedResponse(cannedResponse, request, response) {
response.setStatusLine(null, cannedResponse.status.status,
@ -498,6 +500,61 @@ add_task(function* ensureKeysFor_handles_conflicts() {
});
});
add_task(function* checkSyncKeyRing_reuploads_keys() {
// Verify that when keys are present, they are reuploaded with the
// new kB when we call touchKeys().
const extensionId = uuid();
let extensionKey;
yield* withContextAndServer(function* (context, server) {
yield* withSignedInUser(loggedInUser, function* () {
server.installCollection("storage-sync-crypto");
server.etag = 765;
yield cryptoCollection._clear();
// Do an `ensureKeysFor` to generate some keys.
let collectionKeys = yield ExtensionStorageSync.ensureKeysFor([extensionId]);
ok(collectionKeys.hasKeysFor([extensionId]),
`ensureKeysFor should return a keyring that has a key for ${extensionId}`);
extensionKey = collectionKeys.keyForCollection(extensionId).keyPairB64;
equal(server.getPosts().length, 1,
"generating a key that doesn't exist on the server should post it");
});
// The user changes their password. This is their new kB, with
// the last f changed to an e.
const NOVEL_KB = "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdee";
const newUser = Object.assign({}, loggedInUser, {kB: NOVEL_KB});
let postedKeys;
yield* withSignedInUser(newUser, function* () {
yield ExtensionStorageSync.checkSyncKeyRing();
let posts = server.getPosts();
equal(posts.length, 2,
"when kB changes, checkSyncKeyRing should post the keyring reencrypted with the new kB");
postedKeys = posts[1];
assertPostedUpdatedRecord(postedKeys, 765);
let body = yield assertPostedEncryptedKeys(postedKeys);
deepEqual(body.keys.collections[extensionId], extensionKey,
`the posted keyring should have the same key for ${extensionId} as the old one`);
});
// Verify that with the old kB, we can't decrypt the record.
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),
"decrypting the keyring with the old kB should throw an HMAC mismatch");
});
});
});
add_task(function* test_storage_sync_pulls_changes() {
const extensionId = defaultExtensionId;
const collectionId = defaultCollectionId;