Bug 552134 - Ensure that keyring/symmetric key haven't been tampered with [r=mconnor]

Store a HMAC with the encrypted symmetric key instead of just the wrapped key and verify that the HMAC matches before unwrapping. Test that normal getting works and a tampered payload/HMAC fails but succeeds on restoring the correct HMAC.

services/sync/modules/base_records/crypto.js

@@ -41,6 +41,7 @@ const Ci = Components.interfaces;
 const Cr = Components.results;
 const Cu = Components.utils;
 
+Cu.import("resource://weave/identity.js");
 Cu.import("resource://weave/util.js");
 Cu.import("resource://weave/base_records/wbo.js");
 Cu.import("resource://weave/base_records/keys.js");
@@ -133,8 +134,21 @@ CryptoMeta.prototype = {
     if (!wrapped_key)
       throw "keyring doesn't contain a key for " + pubkeyUri;
 
-    let unwrappedKey = new String(Svc.Crypto.unwrapSymmetricKey(wrapped_key,
-      privkey.keyData, passphrase.password, privkey.salt, privkey.iv));
+    // Make sure the wrapped key hasn't been tampered with
+    let localHMAC = Utils.sha256HMAC(wrapped_key.wrapped, this.hmacKey);
+    if (localHMAC != wrapped_key.hmac)
+      throw "Server attack?! SHA256 HMAC key fail: " + wrapped_key.hmac;
+
+    // Decrypt the symmetric key and make it a String object to add properties
+    let unwrappedKey = new String(
+      Svc.Crypto.unwrapSymmetricKey(
+        wrapped_key.wrapped,
+        privkey.keyData,
+        passphrase.password,
+        privkey.salt,
+        privkey.iv
+      )
+    );
 
     unwrappedKey.hmacKey = Svc.KeyFactory.keyFromString(Ci.nsIKeyObject.HMAC,
       unwrappedKey);
@@ -160,8 +174,17 @@ CryptoMeta.prototype = {
         delete this.keyring[relUri];
     }
 
-    this.keyring[new_pubkey.uri.spec] =
-      Svc.Crypto.wrapSymmetricKey(symkey, new_pubkey.keyData);
+    // Wrap the symmetric key and generate a HMAC for it
+    let wrapped = Svc.Crypto.wrapSymmetricKey(symkey, new_pubkey.keyData);
+    this.keyring[new_pubkey.uri.spec] = {
+      wrapped: wrapped,
+      hmac: Utils.sha256HMAC(wrapped, this.hmacKey)
+    };
+  },
+
+  get hmacKey() {
+    let passphrase = ID.get("WeaveCryptoID").password;
+    return Svc.KeyFactory.keyFromString(Ci.nsIKeyObject.HMAC, passphrase);
   }
 };
 

services/sync/tests/unit/head_first.js

@@ -330,3 +330,9 @@ function SyncTestingInfrastructure(engineFactory) {
  * @usage _(1, 2, 3) -> prints "1 2 3"
  */
 let _ = function(some, debug, text, to) print(Array.slice(arguments).join(" "));
+
+_("Setting the identity for passphrase");
+Cu.import("resource://weave/identity.js");
+let passphrase = ID.set("WeaveCryptoID", new Identity());
+passphrase.password = "passphrase";
+

services/sync/tests/unit/test_clients_escape.js

@@ -4,7 +4,6 @@ Cu.import("resource://weave/base_records/keys.js");
 Cu.import("resource://weave/base_records/crypto.js");
 
 function run_test() {
-  let passphrase = { password: "passphrase" };
   let baseUri = "http://fakebase/";
   let pubUri = baseUri + "pubkey";
   let privUri = baseUri + "privkey";

services/sync/tests/unit/test_records_crypto.js

@@ -59,7 +59,6 @@ function run_test() {
     log.info("Generating keypair + symmetric key");
 
     PubKeys.defaultKeyUri = "http://localhost:8080/pubkey";
-    let passphrase = { password: "my passphrase" };
     keys = PubKeys.createKeypair(passphrase,
                                  "http://localhost:8080/pubkey",
                                  "http://localhost:8080/privkey");

services/sync/tests/unit/test_records_cryptometa.js

@@ -0,0 +1,43 @@
+Cu.import("resource://weave/util.js");
+Cu.import("resource://weave/base_records/crypto.js");
+Cu.import("resource://weave/base_records/keys.js");
+
+function run_test() {
+  _("Generating keypair to encrypt/decrypt symkeys");
+  let {pubkey, privkey} = PubKeys.createKeypair(
+    passphrase,
+    "http://site/pubkey",
+    "http://site/privkey"
+  );
+  PubKeys.set(pubkey.uri, pubkey);
+  PrivKeys.set(privkey.uri, privkey);
+
+  _("Generating a crypto meta with a random key");
+  let crypto = new CryptoMeta("http://site/crypto");
+  let symkey = Svc.Crypto.generateRandomKey();
+  crypto.addUnwrappedKey(pubkey, symkey);
+
+  _("Verifying correct HMAC by getting the key");
+  crypto.getKey(privkey, passphrase);
+
+  _("Generating a new crypto meta as the previous caches the unwrapped key");
+  let crypto = new CryptoMeta("http://site/crypto");
+  let symkey = Svc.Crypto.generateRandomKey();
+  crypto.addUnwrappedKey(pubkey, symkey);
+
+  _("Changing the HMAC to force a mismatch");
+  let goodHMAC = crypto.keyring[pubkey.uri.spec].hmac;
+  crypto.keyring[pubkey.uri.spec].hmac = "failme!";
+  let error = "";
+  try {
+    crypto.getKey(privkey, passphrase);
+  }
+  catch(ex) {
+    error = ex;
+  }
+  do_check_eq(error, "Server attack?! SHA256 HMAC key fail: failme!");
+
+  _("Switching back to the correct HMAC and trying again");
+  crypto.keyring[pubkey.uri.spec].hmac = goodHMAC;
+  crypto.getKey(privkey, passphrase);
+}