Bug 549638 - Ensure that encrypted payloads haven't been tampered with [r=mconnor r=dolske]

Generate a SHA256 HAMC hex string from the base64 ciphertext and base64 symmetric key. Generate a HMAC key from the symmetric key and cache them together when unwrapping. Refactor the Utils.sha1 to share the same digest code.

--HG--
extra : rebase_source : dae973c62317eb3b738306fc30ea19870555c7df

services/sync/modules/base_records/crypto.js

@@ -65,6 +65,7 @@ CryptoWrapper.prototype = {
     this.IV = Svc.Crypto.generateRandomIV();
     this.ciphertext = Svc.Crypto.encrypt(JSON.stringify(this.cleartext),
                                          symkey, this.IV);
+    this.hmac = Utils.sha256HMAC(this.ciphertext, symkey.hmacKey);
     this.cleartext = null;
   },
 
@@ -75,6 +76,10 @@ CryptoWrapper.prototype = {
     let meta = CryptoMetas.get(this.encryption);
     let symkey = meta.getKey(privkey, passphrase);
 
+    // Authenticate the encrypted blob with the expected HMAC
+    if (Utils.sha256HMAC(this.ciphertext, symkey.hmacKey) != this.hmac)
+      throw "Server attack?! SHA256 HMAC mismatch: " + this.hmac;
+
     this.cleartext = JSON.parse(Svc.Crypto.decrypt(this.ciphertext, symkey,
                                                    this.IV));
     this.ciphertext = null;
@@ -103,7 +108,8 @@ CryptoWrapper.prototype = {
   },
 };
 
-Utils.deferGetSet(CryptoWrapper, "payload", ["ciphertext", "encryption", "IV"]);
+Utils.deferGetSet(CryptoWrapper, "payload", ["ciphertext", "encryption", "IV",
+                                             "hmac"]);
 Utils.deferGetSet(CryptoWrapper, "cleartext", "deleted");
 
 function CryptoMeta(uri) {
@@ -115,10 +121,6 @@ CryptoMeta.prototype = {
   _logName: "Record.CryptoMeta",
 
   getKey: function CryptoMeta_getKey(privkey, passphrase) {
-    // We cache the unwrapped key, as it's expensive to generate
-    if (this._unwrappedKey)
-      return this._unwrappedKey;
-
     // get the uri to our public key
     let pubkeyUri = privkey.publicKeyUri.spec;
 
@@ -131,8 +133,14 @@ CryptoMeta.prototype = {
     if (!wrapped_key)
       throw "keyring doesn't contain a key for " + pubkeyUri;
 
-    return this._unwrappedKey = Svc.Crypto.unwrapSymmetricKey(wrapped_key,
-      privkey.keyData, passphrase.password, privkey.salt, privkey.iv);
+    let unwrappedKey = new String(Svc.Crypto.unwrapSymmetricKey(wrapped_key,
+      privkey.keyData, passphrase.password, privkey.salt, privkey.iv));
+
+    unwrappedKey.hmacKey = Svc.KeyFactory.keyFromString(Ci.nsIKeyObject.HMAC,
+      unwrappedKey);
+
+    // Cache the result after the first get and just return it
+    return (this.getKey = function() unwrappedKey)();
   },
 
   addKey: function CryptoMeta_addKey(new_pubkey, privkey, passphrase) {

services/sync/modules/util.js

@@ -475,26 +475,34 @@ let Utils = {
       throw 'checkStatus failed';
   },
 
-  sha1: function Weave_sha1(string) {
+  digest: function digest(message, hasher) {
     let converter = Cc["@mozilla.org/intl/scriptableunicodeconverter"].
       createInstance(Ci.nsIScriptableUnicodeConverter);
     converter.charset = "UTF-8";
 
-    let hasher = Cc["@mozilla.org/security/hash;1"]
-      .createInstance(Ci.nsICryptoHash);
-    hasher.init(hasher.SHA1);
-
-    let data = converter.convertToByteArray(string, {});
+    let data = converter.convertToByteArray(message, {});
     hasher.update(data, data.length);
-    let rawHash = hasher.finish(false);
 
-    // return the two-digit hexadecimal code for a byte
-    function toHexString(charCode) {
-      return ("0" + charCode.toString(16)).slice(-2);
-    }
+    // Convert each hashed byte into 2-hex strings then combine them
+    return [("0" + byte.charCodeAt().toString(16)).slice(-2) for each (byte in
+      hasher.finish(false))].join("");
+  },
 
-    let hash = [toHexString(rawHash.charCodeAt(i)) for (i in rawHash)].join("");
-    return hash;
+  sha1: function sha1(message) {
+    let hasher = Cc["@mozilla.org/security/hash;1"].
+      createInstance(Ci.nsICryptoHash);
+    hasher.init(hasher.SHA1);
+    return Utils.digest(message, hasher);
+  },
+
+  /**
+   * Generate a sha256 HMAC for a string message and a given nsIKeyObject
+   */
+  sha256HMAC: function sha256HMAC(message, key) {
+    let hasher = Cc["@mozilla.org/security/hmac;1"].
+      createInstance(Ci.nsICryptoHMAC);
+    hasher.init(hasher.SHA256, key);
+    return Utils.digest(message, hasher);
   },
 
   makeURI: function Weave_makeURI(URIString) {
@@ -858,6 +866,7 @@ Svc.Obs = Observers;
  ["History", "@mozilla.org/browser/nav-history-service;1", "nsPIPlacesDatabase"],
  ["Idle", "@mozilla.org/widget/idleservice;1", "nsIIdleService"],
  ["IO", "@mozilla.org/network/io-service;1", "nsIIOService"],
+ ["KeyFactory", "@mozilla.org/security/keyobjectfactory;1", "nsIKeyObjectFactory"],
  ["Login", "@mozilla.org/login-manager;1", "nsILoginManager"],
  ["Memory", "@mozilla.org/xpcom/memory-service;1", "nsIMemory"],
  ["Observer", "@mozilla.org/observer-service;1", "nsIObserverService"],

services/sync/tests/unit/test_records_crypto.js

@@ -110,6 +110,18 @@ function run_test() {
     }
     do_check_eq(error, "Server attack?! Id mismatch: crypted-resource,other");
 
+    log.info("Make sure wrong hmacs cause failures");
+    cryptoWrap.encrypt(passphrase);
+    cryptoWrap.hmac = "foo";
+    error = "";
+    try {
+      cryptoWrap.decrypt(passphrase);
+    }
+    catch(ex) {
+      error = ex;
+    }
+    do_check_eq(error, "Server attack?! SHA256 HMAC mismatch: foo");
+
     log.info("Done!");
   }
   finally { server.stop(); }