From 52db8608c2ac5732af52a19291d020b1500503ae Mon Sep 17 00:00:00 2001 From: Richard Newman Date: Wed, 2 Mar 2011 15:18:46 -0800 Subject: [PATCH] Bug 618496: memoization of PK11SymKey. r=philiKON --- services/crypto/modules/WeaveCrypto.js | 90 +++++++++++++------ .../crypto/tests/unit/test_crypto_crypt.js | 43 +++++++-- 2 files changed, 102 insertions(+), 31 deletions(-) diff --git a/services/crypto/modules/WeaveCrypto.js b/services/crypto/modules/WeaveCrypto.js index eafb9a09bc12..be87099f7ec4 100644 --- a/services/crypto/modules/WeaveCrypto.js +++ b/services/crypto/modules/WeaveCrypto.js @@ -48,7 +48,7 @@ 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(); } @@ -214,7 +214,6 @@ WeaveCrypto.prototype = { 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; @@ -422,25 +421,17 @@ WeaveCrypto.prototype = { // 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); @@ -471,15 +462,11 @@ WeaveCrypto.prototype = { } 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. } }, @@ -530,6 +517,57 @@ WeaveCrypto.prototype = { 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 @@ -579,25 +617,25 @@ WeaveCrypto.prototype = { }, // 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); @@ -608,7 +646,7 @@ WeaveCrypto.prototype = { // contents to avoid repetitive and expensive casts. _ivSECItem: null, _ivSECItemContents: null, - + initIVSECItem: function initIVSECItem() { if (this._ivSECItem) { this._ivSECItemContents = null; @@ -647,7 +685,7 @@ WeaveCrypto.prototype = { 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); @@ -684,7 +722,7 @@ WeaveCrypto.prototype = { this.nss.PK11_FreeSlot(slot); if (symKey && !symKey.isNull()) this.nss.PK11_FreeSymKey(symKey); - + this.freeSECItem(passItem); this.freeSECItem(saltItem); } diff --git a/services/crypto/tests/unit/test_crypto_crypt.js b/services/crypto/tests/unit/test_crypto_crypt.js index 6373d640139b..72530b278413 100644 --- a/services/crypto/tests/unit/test_crypto_crypt.js +++ b/services/crypto/tests/unit/test_crypto_crypt.js @@ -20,15 +20,48 @@ function run_test() { 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); @@ -53,7 +86,7 @@ function test_bug_617650() { // 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; @@ -63,16 +96,16 @@ function test_makeSECItem() { 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)