Bug 618219 - Merge fx-sync to mozilla-central. a=blockers
authorPhilipp von Weitershausen <philipp@weitershausen.de>
Thu, 09 Dec 2010 23:21:08 -0800
changeset 59071 2ae9544cd1984eb80453376607df7318ae7d9ede
parent 59067 70728ca65a0704b1a80ad513e24df9c43cfd5a7c (current diff)
parent 59070 16f9d94814287beb0a9c5ded17c96897f71e6604 (diff)
child 59072 a4544a4b322400eb612ea1fd18c88c6bf08379b3
push id1
push usershaver@mozilla.com
push dateTue, 04 Jan 2011 17:58:04 +0000
reviewersblockers
bugs618219
milestone2.0b8pre
Bug 618219 - Merge fx-sync to mozilla-central. a=blockers
services/sync/modules/constants.js
services/sync/modules/engines.js
services/sync/modules/service.js
--- a/services/sync/modules/constants.js
+++ b/services/sync/modules/constants.js
@@ -71,16 +71,21 @@ SYNC_KEY_DECODED_LENGTH:               1
 SYNC_KEY_HYPHENATED_LENGTH:            31,    // 26 chars, 5 hyphens.
 
 // Sync intervals for various clients configurations
 SINGLE_USER_SYNC:                      24 * 60 * 60 * 1000, // 1 day
 MULTI_DESKTOP_SYNC:                    60 * 60 * 1000, // 1 hour
 MULTI_MOBILE_SYNC:                     5 * 60 * 1000, // 5 minutes
 PARTIAL_DATA_SYNC:                     60 * 1000, // 1 minute
 
+// HMAC event handling timeout.
+// 10 minutes: a compromise between the multi-desktop sync interval
+// and the mobile sync interval.
+HMAC_EVENT_INTERVAL:                   600000,
+
 // 50 is hardcoded here because of URL length restrictions.
 // (GUIDs can be up to 64 chars long)
 MOBILE_BATCH_SIZE:                     50,
 
 // score thresholds for early syncs
 SINGLE_USER_THRESHOLD:                 1000,
 MULTI_DESKTOP_THRESHOLD:               500,
 MULTI_MOBILE_THRESHOLD:                100,
--- a/services/sync/modules/engines.js
+++ b/services/sync/modules/engines.js
@@ -52,16 +52,18 @@ Cu.import("resource://services-sync/ext/
 Cu.import("resource://services-sync/ext/Sync.js");
 Cu.import("resource://services-sync/identity.js");
 Cu.import("resource://services-sync/log4moz.js");
 Cu.import("resource://services-sync/resource.js");
 Cu.import("resource://services-sync/stores.js");
 Cu.import("resource://services-sync/trackers.js");
 Cu.import("resource://services-sync/util.js");
 
+Cu.import("resource://services-sync/main.js");    // So we can get to Service for callbacks.
+
 // Singleton service, holds registered engines
 
 Utils.lazy(this, 'Engines', EngineManagerSvc);
 
 function EngineManagerSvc() {
   this._engines = {};
   this._log = Log4Moz.repository.getLogger("Service.Engines");
   this._log.level = Log4Moz.Level[Svc.Prefs.get(
@@ -89,17 +91,17 @@ EngineManagerSvc.prototype = {
     return engine;
   },
   getAll: function EngMgr_getAll() {
     return [engine for ([name, engine] in Iterator(Engines._engines))];
   },
   getEnabled: function EngMgr_getEnabled() {
     return this.getAll().filter(function(engine) engine.enabled);
   },
-
+  
   /**
    * Register an Engine to the service. Alternatively, give an array of engine
    * objects to register.
    *
    * @param engineObject
    *        Engine object used to get an instance of the engine
    * @return The engine object if anything failed
    */
@@ -457,17 +459,32 @@ SyncEngine.prototype = {
 
       // Track the collection for the WBO.
       item.collection = this.name;
       
       // Remember which records were processed
       handled.push(item.id);
 
       try {
-        item.decrypt();
+        try {
+          item.decrypt();
+        } catch (ex) {
+          if (Utils.isHMACMismatch(ex) &&
+              this.handleHMACMismatch()) {
+            // Let's try handling it.
+            // If the callback returns true, try decrypting again, because
+            // we've got new keys.
+            this._log.info("Trying decrypt again...");
+            item.decrypt();
+          }
+          else {
+            throw ex;
+          }
+        }
+       
         if (this._reconcile(item)) {
           count.applied++;
           this._tracker.ignoreAll = true;
           this._store.applyIncoming(item);
         } else {
           count.reconciled++;
           this._log.trace("Skipping reconciled incoming item " + item.id);
         }
@@ -779,10 +796,14 @@ SyncEngine.prototype = {
 
   _resetClient: function SyncEngine__resetClient() {
     this.resetLastSync();
   },
 
   wipeServer: function wipeServer() {
     new Resource(this.engineURL).delete();
     this._resetClient();
+  },
+  
+  handleHMACMismatch: function handleHMACMismatch() {
+    return Weave.Service.handleHMACEvent();
   }
 };
--- a/services/sync/modules/service.js
+++ b/services/sync/modules/service.js
@@ -245,16 +245,127 @@ WeaveSvc.prototype = {
         ok = true;
 
     } catch (e) {
       this._log.debug("Crypto check failed: " + e);
     }
 
     return ok;
   },
+  
+  /**
+   * Here is a disgusting yet reasonable way of handling HMAC errors deep in
+   * the guts of Sync. The astute reader will note that this is a hacky way of
+   * implementing something like continuable conditions.
+   * 
+   * A handler function is glued to each engine. If the engine discovers an
+   * HMAC failure, we fetch keys from the server and update our keys, just as
+   * we would on startup.
+   * 
+   * If our key collection changed, we signal to the engine (via our return
+   * value) that it should retry decryption.
+   * 
+   * If our key collection did not change, it means that we already had the
+   * correct keys... and thus a different client has the wrong ones. Reupload
+   * the bundle that we fetched, which will bump the modified time on the
+   * server and (we hope) prompt a broken client to fix itself.
+   * 
+   * We keep track of the time at which we last applied this reasoning, because
+   * thrashing doesn't solve anything. We keep a reasonable interval between
+   * these remedial actions.
+   */
+  lastHMACEvent: 0,
+  
+  /*
+   * Returns whether to try again.
+   */
+  handleHMACEvent: function handleHMACEvent() {
+    let now = Date.now();
+    
+    // Leave a sizable delay between HMAC recovery attempts. This gives us
+    // time for another client to fix themselves if we touch the record.
+    if ((now - this.lastHMACEvent) < HMAC_EVENT_INTERVAL)
+      return false;
+    
+    this._log.info("Bad HMAC event detected. Attempting recovery " +
+                   "or signaling to other clients.");
+    
+    // Set the last handled time so that we don't act again.
+    this.lastHMACEvent = now;
+    
+    // Fetch keys.
+    let cryptoKeys = new CryptoWrapper("crypto", "keys");
+    try {
+      let cryptoResp = cryptoKeys.fetch(this.cryptoKeysURL).response;
+      
+      // Save out the ciphertext for when we reupload. If there's a bug in
+      // CollectionKeys, this will prevent us from uploading junk.
+      let cipherText = cryptoKeys.ciphertext;
+      
+      if (!cryptoResp.success) {
+        this._log.warn("Failed to download keys.");
+        return false;
+      }
+      
+      let keysChanged = this.handleFetchedKeys(this.syncKeyBundle,
+                                               cryptoKeys, true);
+      if (keysChanged) {
+        // Did they change? If so, carry on.
+        this._log.info("Suggesting retry.");
+        return true;              // Try again.
+      }
+      
+      // If not, reupload them and continue the current sync.
+      cryptoKeys.ciphertext = cipherText;
+      cryptoKeys.cleartext  = null;
+      
+      let uploadResp = cryptoKeys.upload(this.cryptoKeysURL);
+      if (uploadResp.success)
+        this._log.info("Successfully re-uploaded keys. Continuing sync.");
+      else
+        this._log.warn("Got error response re-uploading keys. " +
+                       "Continuing sync; let's try again later.");
+      
+      return false;            // Don't try again: same keys.
+      
+    } catch (ex) {
+      this._log.warn("Got exception \"" + ex + "\" fetching and handling " +
+                     "crypto keys. Will try again later.");
+      return false;
+    }
+  },
+  
+  handleFetchedKeys: function handleFetchedKeys(syncKey, cryptoKeys, skipReset) {
+    // Don't want to wipe if we're just starting up!
+    // This is largely relevant because we don't persist
+    // CollectionKeys yet: Bug 610913.
+    let wasBlank = CollectionKeys.isClear;
+    let keysChanged = CollectionKeys.updateContents(syncKey, cryptoKeys);
+    
+    if (keysChanged && !wasBlank) {
+      this._log.debug("Keys changed: " + JSON.stringify(keysChanged));
+      
+      if (!skipReset) {
+        this._log.info("Resetting client to reflect key change.");
+
+        if (keysChanged.length) {
+          // Collection keys only. Reset individual engines.
+          this.resetClient(keysChanged);
+        }
+        else {
+          // Default key changed: wipe it all.
+          this.resetClient();
+        }
+
+        this._log.info("Downloaded new keys, client reset. Proceeding.");
+      }
+      return true;
+    }
+    return false;
+  },                
 
   /**
    * Prepare to initialize the rest of Weave after waiting a little bit
    */
   onStartup: function onStartup() {
     this._migratePrefs();
     this._initLogs();
     this._log.info("Loading Weave " + WEAVE_VERSION);
@@ -623,37 +734,17 @@ WeaveSvc.prototype = {
         let cryptoKeys;
         
         if (infoCollections && ('crypto' in infoCollections)) {
           try {
             cryptoKeys = new CryptoWrapper("crypto", "keys");
             let cryptoResp = cryptoKeys.fetch(this.cryptoKeysURL).response;
             
             if (cryptoResp.success) {
-              // Don't want to wipe if we're just starting up!
-              // This is largely relevant because we don't persist
-              // CollectionKeys yet: Bug 610913.
-              let wasBlank = CollectionKeys.isClear;
-              let keysChanged = CollectionKeys.updateContents(syncKey, cryptoKeys);
-              
-              if (keysChanged && !wasBlank) {
-                this._log.debug("Keys changed: " + JSON.stringify(keysChanged));
-                this._log.info("Resetting client to reflect key change.");
-                
-                if (keysChanged.length) {
-                  // Collection keys only. Reset individual engines.
-                  this.resetClient(keysChanged);
-                }
-                else {
-                  // Default key changed: wipe it all.
-                  this.resetClient();
-                }
-                
-                this._log.info("Downloaded new keys, client reset. Proceeding.");
-              }
+              let keysChanged = this.handleFetchedKeys(syncKey, cryptoKeys);
               return true;
             }
             else if (cryptoResp.status == 404) {
               // On failure, ask CollectionKeys to generate new keys and upload them.
               // Fall through to the behavior below.
               this._log.warn("Got 404 for crypto/keys, but 'crypto' in info/collections. Regenerating.");
               cryptoKeys = null;
             }
@@ -1085,29 +1176,47 @@ WeaveSvc.prototype = {
     let reset = false;
 
     this._log.debug("Fetching global metadata record");
     let meta = Records.get(this.metaURL);
     
     // Checking modified time of the meta record.
     if (infoResponse &&
         (infoResponse.obj.meta != this.metaModified) &&
-        !meta.isNew) {
+        (!meta || !meta.isNew)) {
       
       // Delete the cached meta record...
       this._log.debug("Clearing cached meta record. metaModified is " +
           JSON.stringify(this.metaModified) + ", setting to " +
           JSON.stringify(infoResponse.obj.meta));
+      
       Records.del(this.metaURL);
       
       // ... fetch the current record from the server, and COPY THE FLAGS.
       let newMeta       = Records.get(this.metaURL);
-      newMeta.isNew     = meta.isNew;
-      newMeta.changed   = meta.changed;
-      
+ 
+      if (!Records.response.success || !newMeta) {
+        this._log.debug("No meta/global record on the server. Creating one.");
+        newMeta = new WBORecord("meta", "global");
+        newMeta.payload.syncID = this.syncID;
+        newMeta.payload.storageVersion = STORAGE_VERSION;
+ 
+        newMeta.isNew = true;
+ 
+        Records.set(this.metaURL, newMeta);
+        if (!newMeta.upload(this.metaURL).success) {
+          this._log.warn("Unable to upload new meta/global. Failing remote setup.");
+          return false;
+        }
+      } else {
+        // If newMeta, then it stands to reason that meta != null.
+        newMeta.isNew   = meta.isNew;
+        newMeta.changed = meta.changed;
+      }
+        
       // Switch in the new meta object and record the new time.
       meta              = newMeta;
       this.metaModified = infoResponse.obj.meta;
     }
 
     let remoteVersion = (meta && meta.payload.storageVersion)?
       meta.payload.storageVersion : "";
 
new file mode 100644
--- /dev/null
+++ b/services/sync/tests/unit/test_corrupt_keys.js
@@ -0,0 +1,247 @@
+Cu.import("resource://services-sync/main.js");
+Cu.import("resource://services-sync/service.js");
+Cu.import("resource://services-sync/engines.js");
+Cu.import("resource://services-sync/util.js");
+Cu.import("resource://services-sync/status.js");
+Cu.import("resource://services-sync/constants.js");
+Cu.import("resource://services-sync/base_records/wbo.js");      // For Records.
+Cu.import("resource://services-sync/base_records/crypto.js");   // For CollectionKeys.
+Cu.import("resource://services-sync/engines/tabs.js");
+Cu.import("resource://services-sync/engines/history.js");
+Cu.import("resource://services-sync/log4moz.js");
+  
+function test_locally_changed_keys() {
+  let passphrase = "abcdeabcdeabcdeabcdeabcdea";
+
+  // Tracking info/collections.
+  let collectionsHelper = track_collections_helper();
+  let upd = collectionsHelper.with_updated_collection;
+  let collections = collectionsHelper.collections;
+
+  let keysWBO = new ServerWBO("keys");
+  let clients = new ServerCollection();
+  let meta_global = new ServerWBO("global");
+  
+  let history = new ServerCollection();
+  
+  let hmacErrorCount = 0;
+  function counting(f) {
+    return function() {
+      hmacErrorCount++;
+      return f.call(this);
+    };
+  }
+  
+  Weave.Service.handleHMACEvent = counting(Weave.Service.handleHMACEvent);
+  
+  do_test_pending();
+  let server = httpd_setup({
+    // Special.
+    "/1.0/johndoe/storage/meta/global": upd("meta", meta_global.handler()),
+    "/1.0/johndoe/info/collections": collectionsHelper.handler,
+    "/1.0/johndoe/storage/crypto/keys": upd("crypto", keysWBO.handler()),
+      
+    // Track modified times.
+    "/1.0/johndoe/storage/clients": upd("clients", clients.handler()),
+    "/1.0/johndoe/storage/clients/foobar": upd("clients", new ServerWBO("clients").handler()),
+    "/1.0/johndoe/storage/tabs": upd("tabs", new ServerCollection().handler()),
+    
+    // Just so we don't get 404s in the logs.
+    "/1.0/johndoe/storage/bookmarks": new ServerCollection().handler(),
+    "/1.0/johndoe/storage/forms": new ServerCollection().handler(),
+    "/1.0/johndoe/storage/passwords": new ServerCollection().handler(),
+    "/1.0/johndoe/storage/prefs": new ServerCollection().handler(),
+    
+    "/1.0/johndoe/storage/history": upd("history", history.handler()),
+  });
+
+  try {
+    
+    Svc.Prefs.set("registerEngines", "Tab");
+    _("Set up some tabs.");
+    let myTabs = 
+      {windows: [{tabs: [{index: 1,
+                          entries: [{
+                            url: "http://foo.com/",
+                            title: "Title"
+                          }],
+                          attributes: {
+                            image: "image"
+                          },
+                          extData: {
+                            weaveLastUsed: 1
+                          }}]}]};
+    delete Svc.Session;
+    Svc.Session = {
+      getBrowserState: function () JSON.stringify(myTabs)
+    };
+    
+    Weave.Service.username = "johndoe";
+    Weave.Service.password = "ilovejane";
+    Weave.Service.passphrase = passphrase;
+    
+    Weave.Service.serverURL = "http://localhost:8080/";
+    Weave.Service.clusterURL = "http://localhost:8080/";
+    
+    Engines.register(HistoryEngine);
+    Weave.Service._registerEngines();
+    
+    function corrupt_local_keys() {
+      CollectionKeys._default.keyPair = [Svc.Crypto.generateRandomKey(),
+                                         Svc.Crypto.generateRandomKey()];
+    }
+    
+    _("Setting meta.");
+    
+    // Bump version on the server.
+    let m = new WBORecord("meta", "global");
+    m.payload = {"syncID": "foooooooooooooooooooooooooo",
+                 "storageVersion": STORAGE_VERSION};
+    m.upload(Weave.Service.metaURL);
+    
+    _("New meta/global: " + JSON.stringify(meta_global));
+    
+    // Upload keys.
+    CollectionKeys.generateNewKeys();
+    serverKeys = CollectionKeys.asWBO("crypto", "keys");
+    serverKeys.encrypt(Weave.Service.syncKeyBundle);
+    do_check_true(serverKeys.upload(Weave.Service.cryptoKeysURL).success);
+    
+    // Check that login works.
+    do_check_true(Weave.Service.login("johndoe", "ilovejane", passphrase));
+    do_check_true(Weave.Service.isLoggedIn);
+    
+    // Sync should upload records.
+    Weave.Service.sync();
+    
+    // Tabs exist.
+    _("Tabs modified: " + collections.tabs);
+    do_check_true(!!collections.tabs);
+    do_check_true(collections.tabs > 0);
+    
+    let coll_modified = CollectionKeys._lastModified;
+    
+    // Let's create some server side history records.
+    let liveKeys = CollectionKeys.keyForCollection("history");
+    _("Keys now: " + liveKeys.keyPair);
+    let nextHistory = {}
+    let visitType = Ci.nsINavHistoryService.TRANSITION_LINK;
+    for (var i = 0; i < 5; i++) {
+      let id = 'record-no-' + i;
+      let modified = Date.now()/1000 - 60*(i+10);
+      
+      let w = new CryptoWrapper("history", "id");
+      w.cleartext = {
+        id: id,
+        histUri: "http://foo/bar?" + id,
+        title: id,
+        sortindex: i,
+        visits: [{date: (modified - 5), type: visitType}],
+        deleted: false};
+      w.encrypt();
+      
+      let wbo = new ServerWBO(id, {ciphertext: w.ciphertext,
+                                   IV: w.IV,
+                                   hmac: w.hmac});
+      wbo.modified = modified;
+      history.wbos[id] = wbo;
+      server.registerPathHandler("/1.0/johndoe/storage/history/record-no-" + i, upd("history", wbo.handler()));
+    }
+    
+    collections.history = Date.now()/1000;
+    let old_key_time = collections.crypto;
+    _("Old key time: " + old_key_time);
+    
+    // Check that we can decrypt one.
+    let rec = new CryptoWrapper("history", "record-no-0");
+    rec.fetch(Weave.Service.storageURL + "history/record-no-0");
+    _(JSON.stringify(rec));
+    do_check_true(!!rec.decrypt());
+    
+    do_check_eq(hmacErrorCount, 0);
+    
+    // Fill local key cache with bad data.
+    corrupt_local_keys();
+    _("Keys now: " + CollectionKeys.keyForCollection("history").keyPair);
+    
+    do_check_eq(hmacErrorCount, 0);
+    
+    // Add some data.
+    for (let k in nextHistory) {
+      nextHistory[k].modified += 1000;
+      history.wbos[k] = nextHistory[k];
+    }
+    
+    _("HMAC error count: " + hmacErrorCount);
+    // Now syncing should succeed, after one HMAC error.
+    Weave.Service.sync();
+    do_check_eq(hmacErrorCount, 1);
+    _("Keys now: " + CollectionKeys.keyForCollection("history").keyPair);
+    
+    // And look! We downloaded history!
+    do_check_true(Engines.get("history")._store.urlExists("http://foo/bar?record-no-0"));
+    do_check_true(Engines.get("history")._store.urlExists("http://foo/bar?record-no-1"));
+    do_check_true(Engines.get("history")._store.urlExists("http://foo/bar?record-no-2"));
+    do_check_true(Engines.get("history")._store.urlExists("http://foo/bar?record-no-3"));
+    do_check_true(Engines.get("history")._store.urlExists("http://foo/bar?record-no-4"));
+    do_check_eq(hmacErrorCount, 1);
+    
+    _("Busting some new server values.");
+    // Now what happens if we corrupt the HMAC on the server?
+    for (var i = 5; i < 10; i++) {
+      let id = 'record-no-' + i;
+      let modified = 1 + (Date.now()/1000);
+      
+      let w = new CryptoWrapper("history", "id");
+      w.cleartext = {
+        id: id,
+        histUri: "http://foo/bar?" + id,
+        title: id,
+        sortindex: i,
+        visits: [{date: (modified - 5), type: visitType}],
+        deleted: false};
+      w.encrypt();
+      w.hmac = w.hmac.toUpperCase();
+      
+      let wbo = new ServerWBO(id, {ciphertext: w.ciphertext,
+                                   IV: w.IV,
+                                   hmac: w.hmac});
+      wbo.modified = modified;
+      history.wbos[id] = wbo;
+      server.registerPathHandler("/1.0/johndoe/storage/history/record-no-" + i, upd("history", wbo.handler()));
+    }
+    collections.history = Date.now()/1000;
+    
+    _("Server key time hasn't changed.");
+    do_check_eq(collections.crypto, old_key_time);
+    
+    _("Resetting HMAC error timer.");
+    Weave.Service.lastHMACEvent = 0;
+    
+    _("Syncing...");
+    Weave.Service.sync();
+    _("Keys now: " + CollectionKeys.keyForCollection("history").keyPair);
+    _("Server keys have been updated, and we skipped over 5 more HMAC errors without adjusting history.");
+    do_check_true(collections.crypto > old_key_time);
+    do_check_eq(hmacErrorCount, 6);
+    do_check_false(Engines.get("history")._store.urlExists("http://foo/bar?record-no-5"));
+    do_check_false(Engines.get("history")._store.urlExists("http://foo/bar?record-no-6"));
+    do_check_false(Engines.get("history")._store.urlExists("http://foo/bar?record-no-7"));
+    do_check_false(Engines.get("history")._store.urlExists("http://foo/bar?record-no-8"));
+    do_check_false(Engines.get("history")._store.urlExists("http://foo/bar?record-no-9"));
+    
+    // Clean up.
+    Weave.Service.startOver();
+    
+  } finally {
+    Weave.Svc.Prefs.resetBranch("");
+    server.stop(do_test_finished);
+  }
+}
+
+function run_test() {
+  let logger = Log4Moz.repository.rootLogger;
+  Log4Moz.repository.rootLogger.addAppender(new Log4Moz.DumpAppender());
+  
+  test_locally_changed_keys();
+}