Bug 618496: memoization of PK11SymKey. r=philiKON
authorRichard Newman <rnewman@mozilla.com>
Wed, 02 Mar 2011 15:18:46 -0800
changeset 63306 e88c9b6c4ebd
parent 63305 0bea44f90d6d
child 63307 bf00bb0056ce
child 63337 2ef1943bd05b
push idunknown
push userunknown
push dateunknown
reviewersphiliKON
bugs618496
Bug 618496: memoization of PK11SymKey. 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
@@ -43,17 +43,17 @@ const Cr = Components.results;
 
 Components.utils.import("resource://gre/modules/XPCOMUtils.jsm");
 Components.utils.import("resource://gre/modules/Services.jsm");
 Components.utils.import("resource://gre/modules/ctypes.jsm");
 
 const ALGORITHM                 = Ci.IWeaveCrypto.AES_256_CBC;
 const KEYSIZE_AES_256           = 32;
 const KEY_DERIVATION_ITERATIONS = 4096;   // PKCS#5 recommends at least 1000.
- 
+
 function WeaveCrypto() {
     this.init();
 }
 
 WeaveCrypto.prototype = {
     QueryInterface: XPCOMUtils.generateQI([Ci.IWeaveCrypto]),
 
     prefBranch : null,
@@ -209,17 +209,16 @@ WeaveCrypto.prototype = {
 
 
         // security/nss/lib/util/pkcs11t.h
         this.nss.CKK_RSA = 0x0;
         this.nss.CKM_RSA_PKCS_KEY_PAIR_GEN = 0x0000;
         this.nss.CKM_AES_KEY_GEN           = 0x1080; 
         this.nss.CKA_ENCRYPT = 0x104;
         this.nss.CKA_DECRYPT = 0x105;
-        this.nss.CKA_UNWRAP  = 0x107;
 
         // security/nss/lib/softoken/secmodt.h
         this.nss.PK11_ATTR_SESSION   = 0x02;
         this.nss.PK11_ATTR_PUBLIC    = 0x08;
         this.nss.PK11_ATTR_SENSITIVE = 0x40;
 
         // security/nss/lib/util/secoidt.h
         this.nss.SEC_OID_PKCS5_PBKDF2         = 291;
@@ -417,35 +416,27 @@ WeaveCrypto.prototype = {
     _commonCrypt : function (input, output, symmetricKey, iv, operation) {
         this.log("_commonCrypt() called");
         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 ctx, symKey, slot, ivParam;
+        let ctx, symKey, ivParam;
         try {
             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);
-            if (symKey.isNull())
-                throw Components.Exception("symkey import failed", Cr.NS_ERROR_FAILURE);
-
+            symKey = this.importSymKey(symmetricKey, operation);
             ctx = this.nss.PK11_CreateContextBySymKey(this.padMechanism, operation, symKey, ivParam);
             if (ctx.isNull())
                 throw Components.Exception("couldn't create context for symkey", Cr.NS_ERROR_FAILURE);
 
             let maxOutputSize = output.length;
             let tmpOutputSize = new ctypes.int(); // Note 1: NSS uses a signed int here...
 
             if (this.nss.PK11_CipherOp(ctx, output, tmpOutputSize.address(), maxOutputSize, input, input.length))
@@ -466,25 +457,21 @@ WeaveCrypto.prototype = {
             let newOutput = ctypes.cast(output, ctypes.unsigned_char.array(actualOutputSize));
             return newOutput;
         } catch (e) {
             this.log("_commonCrypt: failed: " + e);
             throw e;
         } finally {
             if (ctx && !ctx.isNull())
                 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);
-            
+
             // Note that we do not free the IV SECItem; we reuse it.
+            // Neither do we free the symKey, because that's memoized.
         }
     },
 
 
     generateRandomKey : function() {
         this.log("generateRandomKey() called");
         let slot, randKey, keydata;
         try {
@@ -525,16 +512,67 @@ WeaveCrypto.prototype = {
         // Temporary buffer to hold the generated data.
         let scratch = new ctypes.ArrayType(ctypes.unsigned_char, byteCount)();
         if (this.nss.PK11_GenerateRandom(scratch, byteCount))
             throw Components.Exception("PK11_GenrateRandom failed", Cr.NS_ERROR_FAILURE);
 
         return this.encodeBase64(scratch.address(), scratch.length);
     },
 
+    //
+    // PK11SymKey memoization.
+    //
+
+    // Memoize the lookup of symmetric keys. We do this by using the base64
+    // string itself as a key -- the overhead of SECItem creation during the
+    // initial population is negligible, so that phase is not memoized.
+    _encryptionSymKeyMemo: {},
+    _decryptionSymKeyMemo: {},
+    importSymKey: function importSymKey(encodedKeyString, operation) {
+        let memo;
+
+        // We use two separate memos for thoroughness: operation is an input to
+        // key import.
+        switch (operation) {
+            case this.nss.CKA_ENCRYPT:
+                memo = this._encryptionSymKeyMemo;
+                break;
+            case this.nss.CKA_DECRYPT:
+                memo = this._decryptionSymKeyMemo;
+                break;
+            default:
+                throw "Unsupported operation in importSymKey.";
+        }
+
+        if (encodedKeyString in memo)
+            return memo[encodedKeyString];
+
+        let keyItem, slot;
+        try {
+            keyItem = this.makeSECItem(encodedKeyString, true);
+            slot    = this.nss.PK11_GetInternalKeySlot();
+            if (slot.isNull())
+                throw Components.Exception("can't get internal key slot",
+                                           Cr.NS_ERROR_FAILURE);
+
+            let symKey = this.nss.PK11_ImportSymKey(slot, this.padMechanism,
+                                                    this.nss.PK11_OriginUnwrap,
+                                                    operation, keyItem, null);
+            if (!symKey || symKey.isNull())
+                throw Components.Exception("symkey import failed",
+                                           Cr.NS_ERROR_FAILURE);
+
+            return memo[encodedKeyString] = symKey;
+        } finally {
+            if (slot && !slot.isNull())
+                this.nss.PK11_FreeSlot(slot);
+            this.freeSECItem(keyItem);
+        }
+    },
+
 
     //
     // 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
@@ -574,46 +612,46 @@ WeaveCrypto.prototype = {
       return expanded;
     },
 
     encodeBase64 : function (data, len) {
         return btoa(this.expandData(data, len));
     },
 
     // Returns a filled SECItem *, as returned by SECITEM_AllocItem.
-    // 
+    //
     // Note that this must be released with freeSECItem, which will also
     // 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.";
-        
+
         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())
@@ -642,17 +680,17 @@ WeaveCrypto.prototype = {
         let prfAlg    = this.nss.SEC_OID_HMAC_SHA1;
 
         let keyLength  = keyLength || 0;    // 0 = Callee will pick.
         let iterations = KEY_DERIVATION_ITERATIONS;
 
         let algid, slot, symKey, keyData;
         try {
             algid = this.nss.PK11_CreatePBEV2AlgorithmID(pbeAlg, cipherAlg, prfAlg,
-                                                         keyLength, iterations, 
+                                                         keyLength, iterations,
                                                          saltItem);
             if (algid.isNull())
                 throw Components.Exception("PK11_CreatePBEV2AlgorithmID failed", Cr.NS_ERROR_FAILURE);
 
             slot = this.nss.PK11_GetInternalSlot();
             if (slot.isNull())
                 throw Components.Exception("couldn't get internal slot", Cr.NS_ERROR_FAILURE);
 
@@ -679,14 +717,14 @@ WeaveCrypto.prototype = {
             throw e;
         } finally {
             if (algid && !algid.isNull())
                 this.nss.SECOID_DestroyAlgorithmID(algid, true);
             if (slot && !slot.isNull())
                 this.nss.PK11_FreeSlot(slot);
             if (symKey && !symKey.isNull())
                 this.nss.PK11_FreeSymKey(symKey);
-            
+
             this.freeSECItem(passItem);
             this.freeSECItem(saltItem);
         }
     },
 };
--- a/services/crypto/tests/unit/test_crypto_crypt.js
+++ b/services/crypto/tests/unit/test_crypto_crypt.js
@@ -15,25 +15,58 @@ function run_test() {
   
   if (this.gczeal) {
     _("Running crypto tests with gczeal(2).");
     gczeal(2);
   }
   test_bug_617650();
   test_encrypt_decrypt();
   test_SECItem_byteCompressInts();
+  test_key_memoization();
   if (this.gczeal)
     gczeal(0);
 }
 
+function test_key_memoization() {
+  let oldImport = cryptoSvc.nss && cryptoSvc.nss.PK11_ImportSymKey;
+  if (!oldImport) {
+    _("Couldn't swizzle PK11_ImportSymKey; returning.");
+    return;
+  }
+
+  let iv  = cryptoSvc.generateRandomIV();
+  let key = cryptoSvc.generateRandomKey();
+  let c   = 0;
+  cryptoSvc.nss.PK11_ImportSymKey = function(slot, type, origin, operation, key, wincx) {
+    c++;
+    return oldImport(slot, type, origin, operation, key, wincx);
+  }
+
+  // Encryption should cause a single counter increment.
+  do_check_eq(c, 0);
+  let cipherText = cryptoSvc.encrypt("Hello, world.", key, iv);
+  do_check_eq(c, 1);
+  let cipherText = cryptoSvc.encrypt("Hello, world.", key, iv);
+  do_check_eq(c, 1);
+
+  // ... as should decryption.
+  cryptoSvc.decrypt(cipherText, key, iv);
+  cryptoSvc.decrypt(cipherText, key, iv);
+  cryptoSvc.decrypt(cipherText, key, iv);
+  do_check_eq(c, 2);
+
+  // Un-swizzle.
+  cryptoSvc.nss.PK11_ImportSymKey = oldImport;
+}
+
 function multiple_decrypts(iterations) {
   let iv = cryptoSvc.generateRandomIV();
   let key = cryptoSvc.generateRandomKey();
   let cipherText = cryptoSvc.encrypt("Hello, world.", key, iv);
-  
+
   for (let i = 0; i < iterations; ++i) {
     let clearText = cryptoSvc.decrypt(cipherText, key, iv);
     do_check_eq(clearText + " " + i, "Hello, world. " + i);
   }
   _("Done with multiple_decrypts.");
 }
 
 function test_bug_617650() {
@@ -48,36 +81,36 @@ function test_bug_617650() {
     _("No gczeal (non-debug build?); attempting 10,000 iterations of multiple_decrypts.");
     multiple_decrypts(10000);
   }
 }
 
 // Just verify that it gets populated with the correct bytes.
 function test_makeSECItem() {
   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;
   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() {