Bug 636677 - Don't create a new nsICryptoHMAC object for each HMAC verification. r=rnewman a=blocking-fennec

services/sync/modules/jpakeclient.js

@@ -475,7 +475,8 @@ JPAKEClient.prototype = {
     }
 
     this._crypto_key = aes256Key.value;
-    this._hmac_key = Utils.makeHMACKey(Utils.safeAtoB(hmac256Key.value));
+    let hmac_key = Utils.makeHMACKey(Utils.safeAtoB(hmac256Key.value));
+    this._hmac_hasher = Utils.makeHMACHasher(Ci.nsICryptoHMAC.SHA256, hmac_key);
 
     callback();
   },
@@ -523,7 +524,7 @@ JPAKEClient.prototype = {
     try {
       iv = Svc.Crypto.generateRandomIV();
       ciphertext = Svc.Crypto.encrypt(this._data, this._crypto_key, iv);
-      hmac = Utils.sha256HMAC(ciphertext, this._hmac_key);
+      hmac = Utils.bytesAsHex(Utils.digestUTF8(ciphertext, this._hmac_hasher));
     } catch (ex) {
       this._log.error("Failed to encrypt data.");
       this.abort(JPAKE_ERROR_INTERNAL);
@@ -545,7 +546,8 @@ JPAKEClient.prototype = {
     }
     let step3 = this._incoming.payload;
     try {
-      let hmac = Utils.sha256HMAC(step3.ciphertext, this._hmac_key);
+      let hmac = Utils.bytesAsHex(
+        Utils.digestUTF8(step3.ciphertext, this._hmac_hasher));
       if (hmac != step3.hmac)
         throw "HMAC validation failed!";
     } catch (ex) {

services/sync/modules/record.js

@@ -190,11 +190,11 @@ CryptoWrapper.prototype = {
   _logName: "Record.CryptoWrapper",
 
   ciphertextHMAC: function ciphertextHMAC(keyBundle) {
-    let hmacKey = keyBundle.hmacKeyObject;
+    let hasher = keyBundle.sha256HMACHasher;
-    if (!hmacKey)
+    if (!hasher)
-      throw "Cannot compute HMAC with null key.";
+      throw "Cannot compute HMAC without an HMAC key.";
-    
+
-    return Utils.sha256HMAC(this.ciphertext, hmacKey);
+    return Utils.bytesAsHex(Utils.digestUTF8(this.ciphertext, hasher));
   },
 
   /*
@@ -207,7 +207,6 @@ CryptoWrapper.prototype = {
    * Optional key bundle overrides the collection key lookup.
    */
   encrypt: function encrypt(keyBundle) {
-
     keyBundle = keyBundle || CollectionKeys.keyForCollection(this.collection);
     if (!keyBundle)
       throw new Error("Key bundle is null for " + this.uri.spec);
@@ -221,7 +220,6 @@ CryptoWrapper.prototype = {
 
   // Optional key bundle.
   decrypt: function decrypt(keyBundle) {
-    
     if (!this.ciphertext) {
       throw "No ciphertext: nothing to decrypt?";
     }
@@ -238,14 +236,14 @@ CryptoWrapper.prototype = {
     }
 
     // Handle invalid data here. Elsewhere we assume that cleartext is an object.
-    let json_result = JSON.parse(Svc.Crypto.decrypt(this.ciphertext,
+    let cleartext = Svc.Crypto.decrypt(this.ciphertext,
-                                                    keyBundle.encryptionKey, this.IV));
+                                       keyBundle.encryptionKey, this.IV);
+    let json_result = JSON.parse(cleartext);
     
     if (json_result && (json_result instanceof Object)) {
       this.cleartext = json_result;
-    this.ciphertext = null;
+      this.ciphertext = null;
-    }
+    } else {
-    else {
       throw "Decryption failed: result is <" + json_result + ">, not an object.";
     }
 
@@ -536,15 +534,14 @@ function KeyBundle(realm, collectionName, keyStr) {
     throw "KeyBundle given non-string key.";
   
   Identity.call(this, realm, collectionName, keyStr);
-  this._hmac    = null;
-  this._encrypt = null;
-  
-  // Cache the key object.
-  this._hmacObj = null;
 }
-
 KeyBundle.prototype = {
   __proto__: Identity.prototype,
+
+  _encrypt: null,
+  _hmac: null,
+  _hmacObj: null,
+  _sha256HMACHasher: null,
   
   equals: function equals(bundle) {
     return bundle &&
@@ -570,12 +567,18 @@ KeyBundle.prototype = {
   set hmacKey(value) {
     this._hmac = value;
     this._hmacObj = value ? Utils.makeHMACKey(value) : null;
+    this._sha256HMACHasher = value ? Utils.makeHMACHasher(
+      Ci.nsICryptoHMAC.SHA256, this._hmacObj) : null;
   },
   
   get hmacKeyObject() {
     return this._hmacObj;
   },
-}
+
+  get sha256HMACHasher() {
+    return this._sha256HMACHasher;
+  }
+};
 
 function BulkKeyBundle(realm, collectionName) {
   let log = Log4Moz.repository.getLogger("BulkKeyBundle");
@@ -612,8 +615,8 @@ BulkKeyBundle.prototype = {
     }
     else {
       throw "Invalid keypair";
+    }
   }
-  },
 };
 
 function SyncKeyBundle(realm, collectionName, syncKey) {
@@ -640,6 +643,7 @@ SyncKeyBundle.prototype = {
     this._hmac    = null;
     this._hmacObj = null;
     this._encrypt = null;
+    this._sha256HMACHasher = null;
   },
   
   /*
@@ -666,7 +670,13 @@ SyncKeyBundle.prototype = {
       this.generateEntry();
     return this._hmacObj;
   },
-  
+
+  get sha256HMACHasher() {
+    if (!this._sha256HMACHasher)
+      this.generateEntry();
+    return this._sha256HMACHasher;
+  },
+
   /*
    * If we've got a string, hash it into keys and store them.
    */
@@ -687,6 +697,8 @@ SyncKeyBundle.prototype = {
     // Individual sets: cheaper than calling parent setter.
     this._hmac = hmac;
     this._hmacObj = Utils.makeHMACKey(hmac);
+    this._sha256HMACHasher = Utils.makeHMACHasher(
+      Ci.nsICryptoHMAC.SHA256, this._hmacObj);
   }
 };
 

services/sync/modules/util.js

@@ -588,27 +588,49 @@ let Utils = {
       throw 'checkStatus failed';
   },
 
-  digest: function digest(message, hasher) {
+  /**
-    let converter = Cc["@mozilla.org/intl/scriptableunicodeconverter"].
+   * UTF8-encode a message and hash it with the given hasher. Returns a
-      createInstance(Ci.nsIScriptableUnicodeConverter);
+   * string containing bytes. The hasher is reset if it's an HMAC hasher.
-    converter.charset = "UTF-8";
+   */
-
+  digestUTF8: function digestUTF8(message, hasher) {
-    let data = converter.convertToByteArray(message, {});
+    let data = this._utf8Converter.convertToByteArray(message, {});
     hasher.update(data, data.length);
-    return hasher.finish(false);
+    let result = hasher.finish(false);
+    if (hasher instanceof Ci.nsICryptoHMAC) {
+      hasher.reset();
+    }
+    return result;
+  },
+
+  /**
+   * Treat the given message as a bytes string and hash it with the given
+   * hasher. Returns a string containing bytes. The hasher is reset if it's
+   * an HMAC hasher.
+   */
+  digestBytes: function digestBytes(message, hasher) {
+    // No UTF-8 encoding for you, sunshine.
+    let bytes = [b.charCodeAt() for each (b in message)];
+    hasher.update(bytes, bytes.length);
+    let result = hasher.finish(false);
+    if (hasher instanceof Ci.nsICryptoHMAC) {
+      hasher.reset();
+    }
+    return result;
   },
 
   bytesAsHex: function bytesAsHex(bytes) {
-    // Convert each hashed byte into 2-hex strings then combine them
+    let hex = "";
-    return [("0" + byte.charCodeAt().toString(16)).slice(-2)
+    for (let i = 0; i < bytes.length; i++) {
-            for each (byte in bytes)].join("");
+      hex += ("0" + bytes[i].charCodeAt().toString(16)).slice(-2);
+    }
+    return hex;
   },
 
   _sha256: function _sha256(message) {
     let hasher = Cc["@mozilla.org/security/hash;1"].
       createInstance(Ci.nsICryptoHash);
     hasher.init(hasher.SHA256);
-    return Utils.digest(message, hasher);
+    return Utils.digestUTF8(message, hasher);
   },
 
   sha256: function sha256(message) {
@@ -623,7 +645,7 @@ let Utils = {
     let hasher = Cc["@mozilla.org/security/hash;1"].
       createInstance(Ci.nsICryptoHash);
     hasher.init(hasher.SHA1);
-    return Utils.digest(message, hasher);
+    return Utils.digestUTF8(message, hasher);
   },
 
   sha1: function sha1(message) {
@@ -634,6 +656,10 @@ let Utils = {
     return Utils.encodeBase32(Utils._sha1(message));
   },
   
+  sha1Base64: function (message) {
+    return btoa(Utils._sha1(message));
+  },
+
   /**
    * Produce an HMAC key object from a key string.
    */
@@ -642,61 +668,33 @@ let Utils = {
   },
     
   /**
-   * Produce an HMAC hasher.
+   * Produce an HMAC hasher and initialize it with the given HMAC key.
    */
-  makeHMACHasher: function makeHMACHasher() {
+  makeHMACHasher: function makeHMACHasher(type, key) {
-    return Cc["@mozilla.org/security/hmac;1"]
+    let hasher = Cc["@mozilla.org/security/hmac;1"]
-             .createInstance(Ci.nsICryptoHMAC);
+                   .createInstance(Ci.nsICryptoHMAC);
-  },
+    hasher.init(type, key);
-
+    return hasher;
-  sha1Base64: function (message) {
-    return btoa(Utils._sha1(message));
   },
 
   /**
-   * Generate a sha1 HMAC for a message, not UTF-8 encoded,
+   * Some HMAC convenience functions for tests and backwards compatibility:
-   * and a given nsIKeyObject.
+   * 
-   * Optionally provide an existing hasher, which will be 
+   *   sha1HMACBytes: hashes byte string, returns bytes string
-   * initialized and reused.
+   *   sha256HMAC: hashes UTF-8 encoded string, returns hex string
+   *   sha256HMACBytes: hashes byte string, returns bytes string
    */
-  sha1HMACBytes: function sha1HMACBytes(message, key, hasher) {
+  sha1HMACBytes: function sha1HMACBytes(message, key) {
-    let h = hasher || this.makeHMACHasher();
+    let h = Utils.makeHMACHasher(Ci.nsICryptoHMAC.SHA1, key);
-    h.init(h.SHA1, key);
+    return Utils.digestBytes(message, h);
-    
-    // No UTF-8 encoding for you, sunshine.
-    let bytes = [b.charCodeAt() for each (b in message)];
-    h.update(bytes, bytes.length);
-    return h.finish(false);
   },
-  
+  sha256HMAC: function sha256HMAC(message, key) {
-  /**
+    let h = Utils.makeHMACHasher(Ci.nsICryptoHMAC.SHA256, key);
-   * Generate a sha256 HMAC for a string message and a given nsIKeyObject.
+    return Utils.bytesAsHex(Utils.digestUTF8(message, h));
-   * Optionally provide an existing hasher, which will be
-   * initialized and reused.
-   *
-   * Returns hex output.
-   */
-  sha256HMAC: function sha256HMAC(message, key, hasher) {
-    let h = hasher || this.makeHMACHasher();
-    h.init(h.SHA256, key);
-    return Utils.bytesAsHex(Utils.digest(message, h));
   },
-  
+  sha256HMACBytes: function sha256HMACBytes(message, key) {
-  
+    let h = Utils.makeHMACHasher(Ci.nsICryptoHMAC.SHA256, key);
-  /**
+    return Utils.digestBytes(message, h);
-   * Generate a sha256 HMAC for a string message, not UTF-8 encoded,
-   * and a given nsIKeyObject.
-   * Optionally provide an existing hasher, which will be
-   * initialized and reused.
-   */
-  sha256HMACBytes: function sha256HMACBytes(message, key, hasher) {
-    let h = hasher || this.makeHMACHasher();
-    h.init(h.SHA256, key);
-
-    // No UTF-8 encoding for you, sunshine.
-    let bytes = [b.charCodeAt() for each (b in message)];
-    h.update(bytes, bytes.length);
-    return h.finish(false);
   },
 
   /**
@@ -704,13 +702,13 @@ let Utils = {
    */
   hkdfExpand: function hkdfExpand(prk, info, len) {
     const BLOCKSIZE = 256 / 8;
-    let h = Utils.makeHMACHasher();
+    let h = Utils.makeHMACHasher(Ci.nsICryptoHMAC.SHA256,
+                                 Utils.makeHMACKey(prk));
     let T = "";
     let Tn = "";
     let iterations = Math.ceil(len/BLOCKSIZE);
     for (let i = 0; i < iterations; i++) {
-      Tn = Utils.sha256HMACBytes(Tn + info + String.fromCharCode(i + 1),
+      Tn = Utils.digestBytes(Tn + info + String.fromCharCode(i + 1), h);
-                                 Utils.makeHMACKey(prk), h);
       T += Tn;
     }
     return T.slice(0, len);
@@ -736,7 +734,6 @@ let Utils = {
    * can encode as you wish.
    */
   pbkdf2Generate : function pbkdf2Generate(P, S, c, dkLen) {
-    
     // We don't have a default in the algo itself, as NSS does.
     // Use the constant.
     if (!dkLen)
@@ -745,7 +742,7 @@ let Utils = {
     /* For HMAC-SHA-1 */
     const HLEN = 20;
     
-    function F(PK, S, c, i, h) {
+    function F(S, c, i, h) {
     
       function XOR(a, b, isA) {
         if (a.length != b.length) {
@@ -774,9 +771,9 @@ let Utils = {
       I[2] = String.fromCharCode((i >> 8) & 0xff);
       I[3] = String.fromCharCode(i & 0xff);
 
-      U[0] = Utils.sha1HMACBytes(S + I.join(''), PK, h);
+      U[0] = Utils.digestBytes(S + I.join(''), h);
       for (let j = 1; j < c; j++) {
-        U[j] = Utils.sha1HMACBytes(U[j - 1], PK, h);
+        U[j] = Utils.digestBytes(U[j - 1], h);
       }
 
       ret = U[0];
@@ -791,12 +788,11 @@ let Utils = {
     let r = dkLen - ((l - 1) * HLEN);
 
     // Reuse the key and the hasher. Remaking them 4096 times is 'spensive.
-    let PK = Utils.makeHMACKey(P);
+    let h = Utils.makeHMACHasher(Ci.nsICryptoHMAC.SHA1, Utils.makeHMACKey(P));
-    let h = Utils.makeHMACHasher();
     
     T = [];
     for (let i = 0; i < l;) {
-      T[i] = F(PK, S, c, ++i, h);
+      T[i] = F(S, c, ++i, h);
     }
 
     let ret = '';
@@ -1153,10 +1149,7 @@ let Utils = {
     let fos = Cc["@mozilla.org/network/safe-file-output-stream;1"]
                 .createInstance(Ci.nsIFileOutputStream);
     fos.init(file, MODE_WRONLY | MODE_CREATE | MODE_TRUNCATE, PERMS_FILE, 0);
-    let converter = Cc["@mozilla.org/intl/scriptableunicodeconverter"]
+    let is = this._utf8Converter.convertToInputStream(out);
-                      .createInstance(Ci.nsIScriptableUnicodeConverter);
-    converter.charset = "UTF-8";
-    let is = converter.convertToInputStream(out);
     NetUtil.asyncCopy(is, fos, function (result) {
       if (typeof callback == "function") {
         callback.call(that);        
@@ -1289,11 +1282,8 @@ let Utils = {
 
   encodeUTF8: function(str) {
     try {
-      var unicodeConverter = Cc["@mozilla.org/intl/scriptableunicodeconverter"]
+      str = this._utf8Converter.ConvertFromUnicode(str);
-                             .createInstance(Ci.nsIScriptableUnicodeConverter);
+      return str + this._utf8Converter.Finish();
-      unicodeConverter.charset = "UTF-8";
-      str = unicodeConverter.ConvertFromUnicode(str);
-      return str + unicodeConverter.Finish();
     } catch(ex) {
       return null;
     }
@@ -1301,11 +1291,8 @@ let Utils = {
 
   decodeUTF8: function(str) {
     try {
-      var unicodeConverter = Cc["@mozilla.org/intl/scriptableunicodeconverter"]
+      str = this._utf8Converter.ConvertToUnicode(str);
-                             .createInstance(Ci.nsIScriptableUnicodeConverter);
+      return str + this._utf8Converter.Finish();
-      unicodeConverter.charset = "UTF-8";
-      str = unicodeConverter.ConvertToUnicode(str);
-      return str + unicodeConverter.Finish();
     } catch(ex) {
       return null;
     }
@@ -1645,6 +1632,12 @@ let FakeSvc = {
     isFake: true
   }
 };
+Utils.lazy2(Utils, "_utf8Converter", function() {
+  let converter = Cc["@mozilla.org/intl/scriptableunicodeconverter"]
+                    .createInstance(Ci.nsIScriptableUnicodeConverter);
+  converter.charset = "UTF-8";
+  return converter;
+});
 
 /*
  * Commonly-used services

services/sync/tests/unit/head_helpers.js

@@ -187,10 +187,13 @@ function FakeCryptoService() {
   delete Svc.Crypto;  // get rid of the getter first
   Svc.Crypto = this;
   Utils.sha256HMAC = this.sha256HMAC;
+
+  Cu.import("resource://services-sync/record.js");
+  CryptoWrapper.prototype.ciphertextHMAC = this.ciphertextHMAC;
 }
 FakeCryptoService.prototype = {
 
-  sha256HMAC: function(message, key) {
+  sha256HMAC: function Utils_sha256HMAC(message, hasher) {
      message = message.substr(0, 64);
      while (message.length < 64) {
        message += " ";
@@ -198,6 +201,10 @@ FakeCryptoService.prototype = {
      return message;
   },
 
+  ciphertextHMAC: function CryptoWrapper_ciphertextHMAC(keyBundle) {
+    return Utils.sha256HMAC(this.ciphertext);
+  },
+
   encrypt: function(aClearText, aSymmetricKey, aIV) {
     return aClearText;
   },

services/sync/tests/unit/test_utils_sha256HMAC.js

@@ -2,8 +2,8 @@ _("Make sure sha256 hmac works with various messages and keys");
 Cu.import("resource://services-sync/util.js");
 
 function run_test() {
-  let key1 = Svc.KeyFactory.keyFromString(Ci.nsIKeyObject.HMAC, "key1");
+  let key1 = Utils.makeHMACKey("key1");
-  let key2 = Svc.KeyFactory.keyFromString(Ci.nsIKeyObject.HMAC, "key2");
+  let key2 = Utils.makeHMACKey("key2");
 
   let mes1 = "message 1";
   let mes2 = "message 2";