diff --git a/services/sync/modules/engines.js b/services/sync/modules/engines.js index 2440f5987e0..f7daf4be9d3 100644 --- a/services/sync/modules/engines.js +++ b/services/sync/modules/engines.js @@ -717,7 +717,7 @@ SyncEngine.prototype = { try { item.decrypt(); } catch (ex if (Utils.isHMACMismatch(ex) && - this.handleHMACMismatch())) { + this.handleHMACMismatch(item))) { // Let's try handling it. // If the callback returns true, try decrypting again, because // we've got new keys. @@ -1083,7 +1083,7 @@ SyncEngine.prototype = { this._resetClient(); }, - handleHMACMismatch: function handleHMACMismatch() { + handleHMACMismatch: function handleHMACMismatch(item) { return Weave.Service.handleHMACEvent(); } }; diff --git a/services/sync/modules/engines/clients.js b/services/sync/modules/engines/clients.js index 24748338c41..fb817292f05 100644 --- a/services/sync/modules/engines/clients.js +++ b/services/sync/modules/engines/clients.js @@ -192,6 +192,20 @@ ClientEngine.prototype = { _wipeClient: function _wipeClient() { SyncEngine.prototype._resetClient.call(this); this._store.wipe(); + }, + + // Override the default behavior to delete bad records from the server. + handleHMACMismatch: function handleHMACMismatch(item) { + this._log.debug("Handling HMAC mismatch for " + item.id); + if (SyncEngine.prototype.handleHMACMismatch.call(this, item)) + return true; + + // It's a bad client record. Save it to be deleted at the end of the sync. + this._log.debug("Bad client record detected. Scheduling for deletion."); + this._deleteId(item.id); + + // Don't try again. + return false; } }; diff --git a/services/sync/tests/unit/head_http_server.js b/services/sync/tests/unit/head_http_server.js index 9dd6419afc6..e6844465750 100644 --- a/services/sync/tests/unit/head_http_server.js +++ b/services/sync/tests/unit/head_http_server.js @@ -139,6 +139,17 @@ ServerCollection.prototype = { && (!options.newer || (wbo.modified > options.newer)); }, + count: function(options) { + options = options || {}; + let c = 0; + for (let [id, wbo] in Iterator(this.wbos)) { + if (wbo.modified && this._inResultSet(wbo, options)) { + c++; + } + } + return c; + }, + get: function(options) { let result; if (options.full) { diff --git a/services/sync/tests/unit/test_clients_engine.js b/services/sync/tests/unit/test_clients_engine.js index ce1a283f63d..4f2f237d37f 100644 --- a/services/sync/tests/unit/test_clients_engine.js +++ b/services/sync/tests/unit/test_clients_engine.js @@ -3,10 +3,86 @@ Cu.import("resource://services-sync/record.js"); Cu.import("resource://services-sync/identity.js"); Cu.import("resource://services-sync/util.js"); Cu.import("resource://services-sync/engines/clients.js"); +Cu.import("resource://services-sync/service.js"); const MORE_THAN_CLIENTS_TTL_REFRESH = 691200; // 8 days const LESS_THAN_CLIENTS_TTL_REFRESH = 86400; // 1 day +function test_bad_hmac() { + _("Ensure that Clients engine deletes corrupt records."); + let global = new ServerWBO('global', + {engines: {clients: {version: Clients.version, + syncID: Clients.syncID}}}); + let clientsColl = new ServerCollection({}, true); + let keysWBO = new ServerWBO("keys"); + + let collectionsHelper = track_collections_helper(); + let upd = collectionsHelper.with_updated_collection; + let collections = collectionsHelper.collections; + + // Watch for deletions in the given collection. + let deleted = false; + function trackDeletedHandler(coll, handler) { + let u = upd(coll, handler); + return function(request, response) { + if (request.method == "DELETE") + deleted = true; + + return u(request, response); + }; + } + + let handlers = { + "/1.0/foo/info/collections": collectionsHelper.handler, + "/1.0/foo/storage/meta/global": upd("meta", global.handler()), + "/1.0/foo/storage/crypto/keys": upd("crypto", keysWBO.handler()), + "/1.0/foo/storage/clients": trackDeletedHandler("crypto", clientsColl.handler()) + }; + + let server = httpd_setup(handlers); + do_test_pending(); + + try { + let passphrase = "abcdeabcdeabcdeabcdeabcdea"; + Service.serverURL = "http://localhost:8080/"; + Service.clusterURL = "http://localhost:8080/"; + Service.login("foo", "ilovejane", passphrase); + + CollectionKeys.generateNewKeys(); + + _("First sync, client record is uploaded"); + do_check_eq(0, clientsColl.count()); + do_check_eq(Clients.lastRecordUpload, 0); + Clients.sync(); + do_check_eq(1, clientsColl.count()); + do_check_true(Clients.lastRecordUpload > 0); + deleted = false; // Initial setup can wipe the server, so clean up. + + _("Records now: " + clientsColl.get({})); + _("Change our keys and our client ID, reupload keys."); + Clients.localID = Utils.makeGUID(); + Clients.resetClient(); + CollectionKeys.generateNewKeys(); + let serverKeys = CollectionKeys.asWBO("crypto", "keys"); + serverKeys.encrypt(Weave.Service.syncKeyBundle); + do_check_true(serverKeys.upload(Weave.Service.cryptoKeysURL).success); + + _("Sync."); + do_check_true(!deleted); + Clients.sync(); + + _("Old record was deleted, new one uploaded."); + do_check_true(deleted); + do_check_eq(1, clientsColl.count()); + _("Records now: " + clientsColl.get({})); + + } finally { + server.stop(do_test_finished); + Svc.Prefs.resetBranch(""); + Records.clearCache(); + } +} + function test_properties() { try { _("Test lastRecordUpload property"); @@ -74,6 +150,9 @@ function test_sync() { function run_test() { + initTestLogging("Trace"); + Log4Moz.repository.getLogger("Engine.Clients").level = Log4Moz.Level.Trace; + test_bad_hmac(); // Needs to run first: doesn't use fake service! test_properties(); test_sync(); }