Bug 618496: Revisit memoization of SECItems. r=philiKON
authorRichard Newman <rnewman@mozilla.com>
Wed, 02 Mar 2011 15:09:28 -0800
changeset 63305 0bea44f90d6d
parent 63304 ab322645172b
child 63306 e88c9b6c4ebd
push idunknown
push userunknown
push dateunknown
reviewersphiliKON
bugs618496
Bug 618496: Revisit memoization of SECItems. r=philiKON
services/crypto/modules/WeaveCrypto.js
services/crypto/tests/unit/test_crypto_crypt.js
--- a/services/crypto/modules/WeaveCrypto.js
+++ b/services/crypto/modules/WeaveCrypto.js
@@ -82,16 +82,17 @@ WeaveCrypto.prototype = {
             this.prefBranch = Services.prefs.getBranch("services.sync.log.");
             this.prefBranch.QueryInterface(Ci.nsIPrefBranch2);
             this.prefBranch.addObserver("cryptoDebug", this.observer, false);
             this.observer._self = this;
             this.debug = this.prefBranch.getBoolPref("cryptoDebug");
 
             this.initNSS();
             this.initAlgorithmSettings();   // Depends on NSS.
+            this.initIVSECItem();
         } catch (e) {
             this.log("init failed: " + e);
             throw e;
         }
     },
 
     /**
      * Set a bunch of NSS values once, at init-time. These are:
@@ -390,39 +391,50 @@ WeaveCrypto.prototype = {
 
         let inputUCS2 = "";
         if (cipherText.length)
             inputUCS2 = atob(cipherText);
 
         // We can't have js-ctypes create the buffer directly from the string
         // (as in encrypt()), because we do _not_ want it to do UTF8
         // conversion... We've got random binary data in the input's low byte.
-        let input = new ctypes.ArrayType(ctypes.unsigned_char, inputUCS2.length)();
-        this.byteCompress(inputUCS2, input);
+        // 
+        // Compress a JS string (2-byte chars) into a normal C string (1-byte chars).
+        let len   = inputUCS2.length;
+        let input = new ctypes.ArrayType(ctypes.unsigned_char, len)();
+        let ints  = ctypes.cast(input, ctypes.uint8_t.array(len));
+        this.byteCompressInts(inputUCS2, ints, len);
 
         let outputBuffer = new ctypes.ArrayType(ctypes.unsigned_char, input.length)();
 
         outputBuffer = this._commonCrypt(input, outputBuffer, symmetricKey, iv, this.nss.CKA_DECRYPT);
 
         // outputBuffer contains UTF-8 data, let js-ctypes autoconvert that to a JS string.
         // XXX Bug 573842: wrap the string from ctypes to get a new string, so
         // we don't hit bug 573841.
         return "" + outputBuffer.readString() + "";
     },
 
 
     _commonCrypt : function (input, output, symmetricKey, iv, operation) {
         this.log("_commonCrypt() called");
-        // Get rid of the base64 encoding and convert to SECItems.
+        iv = atob(iv);
+
+        // We never want an IV longer than the block size, which is 16 bytes
+        // for AES.
+        if (iv.length > this.blockSize)
+            iv = iv.slice(0, this.blockSize);
+        
+        // We use a single IV SECItem for the sake of efficiency. Fill it here.
+        this.byteCompressInts(iv, this._ivSECItemContents, iv.length);
+
         let keyItem = this.makeSECItem(symmetricKey, true);
-        let ivItem  = this.makeSECItem(iv, true);
-
         let ctx, symKey, slot, ivParam;
         try {
-            ivParam = this.nss.PK11_ParamFromIV(this.padMechanism, ivItem);
+            ivParam = this.nss.PK11_ParamFromIV(this.padMechanism, this._ivSECItem);
             if (ivParam.isNull())
                 throw Components.Exception("can't convert IV to param", Cr.NS_ERROR_FAILURE);
 
             slot = this.nss.PK11_GetInternalKeySlot();
             if (slot.isNull())
                 throw Components.Exception("can't get internal key slot", Cr.NS_ERROR_FAILURE);
 
             symKey = this.nss.PK11_ImportSymKey(slot, this.padMechanism, this.nss.PK11_OriginUnwrap, operation, keyItem, null);
@@ -461,17 +473,18 @@ WeaveCrypto.prototype = {
                 this.nss.PK11_DestroyContext(ctx, true);
             if (symKey && !symKey.isNull())
                 this.nss.PK11_FreeSymKey(symKey);
             if (slot && !slot.isNull())
                 this.nss.PK11_FreeSlot(slot);
             if (ivParam && !ivParam.isNull())
                 this.nss.SECITEM_FreeItem(ivParam, true);
             this.freeSECItem(keyItem);
-            this.freeSECItem(ivItem);
+            
+            // Note that we do not free the IV SECItem; we reuse it.
         }
     },
 
 
     generateRandomKey : function() {
         this.log("generateRandomKey() called");
         let slot, randKey, keydata;
         try {
@@ -517,23 +530,32 @@ WeaveCrypto.prototype = {
         return this.encodeBase64(scratch.address(), scratch.length);
     },
 
 
     //
     // Utility functions
     //
 
+    /**
+     * Compress a JS string into a C uint8 array. count is the number of
+     * elements in the destination array. If the array is smaller than the
+     * string, the string is effectively truncated. If the string is smaller
+     * than the array, the array is 0-padded.
+     */
+    byteCompressInts : function byteCompressInts (jsString, intArray, count) {
+        let len = jsString.length;
+        let end = Math.min(len, count);
 
-    // Compress a JS string (2-byte chars) into a normal C string (1-byte chars)
-    // EG, for "ABC",  0x0041, 0x0042, 0x0043 --> 0x41, 0x42, 0x43
-    byteCompress : function (jsString, charArray) {
-        let intArray = ctypes.cast(charArray, ctypes.uint8_t.array(charArray.length));
-        for (let i = 0; i < jsString.length; i++)
+        for (let i = 0; i < end; i++)
             intArray[i] = jsString.charCodeAt(i) % 256; // convert to bytes
+
+        // Must zero-pad.
+        for (let i = len; i < count; i++)
+            intArray[i] = 0;
     },
 
     // Expand a normal C string (1-byte chars) into a JS string (2-byte chars)
     // EG, for "ABC",  0x41, 0x42, 0x43 --> 0x0041, 0x0042, 0x0043
     byteExpand : function (charArray) {
         let expanded = "";
         let len = charArray.length;
         let intData = ctypes.cast(charArray, ctypes.uint8_t.array(len));
@@ -562,28 +584,54 @@ WeaveCrypto.prototype = {
     // deallocate the internal buffer.
     makeSECItem : function(input, isEncoded) {
         if (isEncoded)
             input = atob(input);
         
         let len = input.length;
         let item = this.nss.SECITEM_AllocItem(null, null, len);
         if (item.isNull())
-          throw "SECITEM_AllocItem failed.";
+            throw "SECITEM_AllocItem failed.";
         
-        let dest = ctypes.cast(item.contents.data, ctypes.unsigned_char.array(len).ptr);
-        this.byteCompress(input, dest.contents);
+        let ptr  = ctypes.cast(item.contents.data,
+                               ctypes.unsigned_char.array(len).ptr);
+        let dest = ctypes.cast(ptr.contents, ctypes.uint8_t.array(len));
+        this.byteCompressInts(input, dest, len);
         return item;
     },
     
     freeSECItem : function(zap) {
         if (zap && !zap.isNull())
             this.nss.SECITEM_ZfreeItem(zap, true);
     },
 
+    // We only ever handle one IV at a time, and they're always different.
+    // Consequently, we maintain a single SECItem, and a handy pointer into its
+    // contents to avoid repetitive and expensive casts.
+    _ivSECItem: null,
+    _ivSECItemContents: null,
+    
+    initIVSECItem: function initIVSECItem() {
+        if (this._ivSECItem) {
+            this._ivSECItemContents = null;
+            this.freeSECItem(this._ivSECItem);
+        }
+
+        let item = this.nss.SECITEM_AllocItem(null, null, this.blockSize);
+        if (item.isNull())
+            throw "SECITEM_AllocItem failed.";
+
+        let ptr = ctypes.cast(item.contents.data,
+                              ctypes.unsigned_char.array(this.blockSize).ptr);
+        let contents = ctypes.cast(ptr.contents,
+                                   ctypes.uint8_t.array(this.blockSize));
+        this._ivSECItem = item;
+        this._ivSECItemContents = contents;
+    },
+
     /**
      * Returns the expanded data string for the derived key.
      */
     deriveKeyFromPassphrase : function deriveKeyFromPassphrase(passphrase, salt, keyLength) {
         this.log("deriveKeyFromPassphrase() called.");
         let passItem = this.makeSECItem(passphrase, false);
         let saltItem = this.makeSECItem(salt, true);
 
--- a/services/crypto/tests/unit/test_crypto_crypt.js
+++ b/services/crypto/tests/unit/test_crypto_crypt.js
@@ -14,16 +14,17 @@ function run_test() {
     test_makeSECItem();
   
   if (this.gczeal) {
     _("Running crypto tests with gczeal(2).");
     gczeal(2);
   }
   test_bug_617650();
   test_encrypt_decrypt();
+  test_SECItem_byteCompressInts();
   if (this.gczeal)
     gczeal(0);
 }
 
 function multiple_decrypts(iterations) {
   let iv = cryptoSvc.generateRandomIV();
   let key = cryptoSvc.generateRandomKey();
   let cipherText = cryptoSvc.encrypt("Hello, world.", key, iv);
@@ -55,16 +56,34 @@ function test_makeSECItem() {
   
   let item1 = cryptoSvc.makeSECItem("abcdefghi", false);
   do_check_true(!item1.isNull());
   let intData = ctypes.cast(item1.contents.data, ctypes.uint8_t.array(8).ptr).contents;
   for (let i = 0; i < 8; ++i)
     do_check_eq(intData[i], "abcdefghi".charCodeAt(i));
 }
 
+function test_SECItem_byteCompressInts() {
+  Components.utils.import("resource://gre/modules/ctypes.jsm");
+  
+  let item1 = cryptoSvc.makeSECItem("abcdefghi", false);
+  do_check_true(!item1.isNull());
+  let intData = ctypes.cast(item1.contents.data, ctypes.uint8_t.array(8).ptr).contents;
+  
+  // Fill it too short.
+  cryptoSvc.byteCompressInts("MMM", intData, 8);
+  for (let i = 0; i < 8; ++i)
+    do_check_eq(intData[i], [77, 77, 77, 0, 0, 0, 0, 0, 0][i]);
+  
+  // Fill it too much. Doesn't buffer overrun.
+  cryptoSvc.byteCompressInts("NNNNNNNNNNNNNNNN", intData, 8);
+  for (let i = 0; i < 8; ++i)
+    do_check_eq(intData[i], "NNNNNNNNNNNNNNNN".charCodeAt(i));
+}
+
 function test_encrypt_decrypt() {
 
   // First, do a normal run with expected usage... Generate a random key and
   // iv, encrypt and decrypt a string.
   var iv = cryptoSvc.generateRandomIV();
   do_check_eq(iv.length, 24);
 
   var key = cryptoSvc.generateRandomKey();
@@ -200,10 +219,9 @@ function test_encrypt_decrypt() {
 
   try {
     failure = false;
     clearText = cryptoSvc.decrypt(badcipher, key, iv);
   } catch (e) {
     failure = true;
   }
   do_check_true(failure);
-
 }