Backed out changeset a73da05c87ba (bug 1253740)

This commit is contained in:
Sebastian Hengst 2016-11-02 16:39:42 +01:00
parent b9e0fc97c3
commit 55e6f64d89
3 changed files with 19 additions and 379 deletions

View File

@ -227,46 +227,4 @@ class KeyRingEncryptionRemoteTransformer extends EncryptionRemoteTransformer {
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);
}
}

View File

@ -52,8 +52,6 @@ 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",
@ -236,17 +234,7 @@ const cryptoCollection = this.cryptoCollection = {
getKeyRingRecord: Task.async(function* () {
const collection = yield this._kintoCollectionPromise;
const cryptoKeyRecord = yield collection.getAny(STORAGE_SYNC_CRYPTO_KEYRING_RECORD_ID);
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;
return cryptoKeyRecord.data;
}),
/**
@ -257,15 +245,13 @@ const cryptoCollection = this.cryptoCollection = {
getKeyRing: Task.async(function* () {
const cryptoKeyRecord = yield this.getKeyRingRecord();
const collectionKeys = new CollectionKeyManager();
if (cryptoKeyRecord.keys) {
if (cryptoKeyRecord) {
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;
}),
@ -292,15 +278,6 @@ const cryptoCollection = this.cryptoCollection = {
});
}),
/**
* Reset sync status for ALL collections by directly
* accessing the FirefoxAdapter.
*/
resetSyncStatus: Task.async(function* () {
const coll = yield this._kintoCollectionPromise;
yield coll.db.resetSyncStatus();
}),
// Used only for testing.
_clear: Task.async(function* () {
const collection = yield this._kintoCollectionPromise;
@ -558,20 +535,6 @@ this.ExtensionStorageSync = {
}
}),
/**
* 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
@ -592,12 +555,11 @@ this.ExtensionStorageSync = {
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 this._syncKeyRing(newRecord);
const result = yield cryptoCollection.sync();
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
@ -678,73 +640,25 @@ this.ExtensionStorageSync = {
// 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.
//
// 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();
// Any open collections might have a lastModified; we need
// to wipe that too.
for (let [, cPromise] of collectionPromises) {
const coll = yield cPromise;
// FIXME: should there be a method here?
coll._lastModified = null;
}
// 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;
// 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();
}
}),

View File

@ -56,8 +56,6 @@ class KintoServer {
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 = [];
@ -74,10 +72,6 @@ class KintoServer {
return this.posts;
}
getDeletedBuckets() {
return this.deletedBuckets;
}
installConfigPath() {
const configPath = "/v1/";
const responseBody = JSON.stringify({
@ -193,17 +187,9 @@ class KintoServer {
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({
"data": data,
// Can't JSON a Set directly, so convert to Array
"data": Array.from(this.collections.get(collectionId)),
});
response.write(body);
}
@ -211,35 +197,6 @@ class KintoServer {
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");
@ -260,18 +217,6 @@ class KintoServer {
});
}
// 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 => {
@ -323,9 +268,6 @@ function* withSignedInUser(user, f) {
getOAuthToken() {
return Promise.resolve("some-access-token");
},
sessionStatus() {
return Promise.resolve(true);
},
};
try {
@ -617,186 +559,12 @@ add_task(function* checkSyncKeyRing_reuploads_keys() {
error = e;
}
ok(error, "decrypting the keyring with the old kB should fail");
ok(Utils.isHMACMismatch(error) || KeyRingEncryptionRemoteTransformer.isOutdatedKB(error),
ok(Utils.isHMACMismatch(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* () {
// Prompt ExtensionStorageSync to initialize crypto
yield ExtensionStorageSync.get({id: extensionId}, "random-key", context);
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* () {
// Prompt ExtensionStorageSync to initialize crypto
yield ExtensionStorageSync.get(extension, "random-key", context);
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() {
yield* withContextAndServer(function* (context, server) {
yield* withSignedInUser(loggedInUser, function* () {