Bug 419794 - "work out key API for nsICryptoHMAC" (API use nsIKeyObject + fix nsKeyModule + nsICryptoHMAC usage fix) [p=honzab@allpeers.com (Honza Bambas [mayhemer]) r=rrelyea sr=dveditz a=blocking1.9+]
authorreed@reedloden.com
Tue, 18 Mar 2008 12:45:40 -0700
changeset 13256 20084bc26b5a3a0401afeed9081a7e3e55d6668a
parent 13255 92a792593cbbb4aa0ec07c8ee3d2245696a590c6
child 13257 0d1d6f231081ce6da78a1d6f428bcc6f6898a46e
push idunknown
push userunknown
push dateunknown
reviewersrrelyea, dveditz, blocking1.9
bugs419794
milestone1.9b5pre
Bug 419794 - "work out key API for nsICryptoHMAC" (API use nsIKeyObject + fix nsKeyModule + nsICryptoHMAC usage fix) [p=honzab@allpeers.com (Honza Bambas [mayhemer]) r=rrelyea sr=dveditz a=blocking1.9+]
netwerk/base/public/nsICryptoHMAC.idl
security/manager/ssl/public/nsIKeyModule.idl
security/manager/ssl/src/nsKeyModule.cpp
security/manager/ssl/src/nsNSSComponent.cpp
security/manager/ssl/tests/test_hmac.js
toolkit/components/url-classifier/src/nsUrlClassifierDBService.cpp
toolkit/components/url-classifier/src/nsUrlClassifierHashCompleter.cpp
toolkit/components/url-classifier/tests/unit/test_streamupdater.js
--- a/netwerk/base/public/nsICryptoHMAC.idl
+++ b/netwerk/base/public/nsICryptoHMAC.idl
@@ -33,23 +33,24 @@
  * and other provisions required by the GPL or the LGPL. If you do not delete
  * the provisions above, a recipient may use your version of this file under
  * the terms of any one of the MPL, the GPL or the LGPL.
  *
  * ***** END LICENSE BLOCK ***** */
 
 #include "nsISupports.idl"
 interface nsIInputStream;
+interface nsIKeyObject;
 
 /**
  * nsICryptoHMAC
  * This interface provides HMAC signature algorithms.
  */
 
-[scriptable, uuid(B8E1AC84-E10D-47fe-9D03-F0AF2E943E71)]
+[scriptable, uuid(8FEB4C7C-1641-4a7b-BC6D-1964E2099497)]
 interface nsICryptoHMAC : nsISupports
 {
     /**
      * Hashing Algorithms.  These values are to be used by the
      * |init| method to indicate which hashing function to
      * use.  These values map onto the values defined in
      * mozilla/security/nss/lib/softoken/pkcs11t.h and are 
      * switched to CKM_*_HMAC constant.
@@ -64,29 +65,31 @@ interface nsICryptoHMAC : nsISupports
     /**
      * Initialize the hashing object. This method may be
      * called multiple times with different algorithm types.
      *
      * @param aAlgorithm the algorithm type to be used.
      *        This value must be one of the above valid
      *        algorithm types.
      *
-     * @param aKeyData the raw key data used for the HMAC
-     *        computation
+     * @param aKeyObject
+     *        Object holding a key. To create the key object use for instance:
+     *        var keyObject = Components.classes["@mozilla.org/security/keyobjectfactory;1"]
+     *            .getService(Components.interfaces.nsIKeyObjectFactory)
+     *              .keyFromString(Components.interfaces.nsIKeyObject.HMAC, rawKeyData);
      *
-     * @param aKeyLength length of the key in bytes
+     * WARNING: This approach is not FIPS compliant.
      *
      * @throws NS_ERROR_INVALID_ARG if an unsupported algorithm
      *        type is passed.
      *
      * NOTE: This method must be called before any other method 
      *        on this interface is called.
      */
-    void init(in unsigned long aAlgorithm, [const, array, size_is(aKeyLen)] in octet aKeyData, 
-        in unsigned long aKeyLen);
+    void init(in unsigned long aAlgorithm, in nsIKeyObject aKeyObject);
 
     /**
      * @param aData a buffer to calculate the hash over
      *
      * @param aLen the length of the buffer |aData|
      *
      * @throws NS_ERROR_NOT_INITIALIZED if |init| has not been 
      *         called.
--- a/security/manager/ssl/public/nsIKeyModule.idl
+++ b/security/manager/ssl/public/nsIKeyModule.idl
@@ -43,16 +43,17 @@ interface nsIKeyObject : nsISupports
   // Key types
   const short SYM_KEY = 1;
   const short PRIVATE_KEY = 2;
   const short PUBLIC_KEY = 3;
 
   // Algorithm types
   const short RC4 = 1;
   const short AES_CBC = 2;
+  const short HMAC = 257;
 
   // aAlgorithm is an algorithm type
   // aKey is either a PK11SymKey, SECKEYPublicKey, or a SECKEYPrivateKey.
   // The nsIKeyObject will take ownership of the key and be responsible
   // for freeing the key memory when destroyed.
   [noscript] void initKey(in short aAlgorithm, in voidPtr aKey);
 
   // Return a pointer to the underlying key object
--- a/security/manager/ssl/src/nsKeyModule.cpp
+++ b/security/manager/ssl/src/nsKeyModule.cpp
@@ -82,16 +82,17 @@ nsKeyObject::CleanUp()
 NS_IMETHODIMP
 nsKeyObject::InitKey(PRInt16 aAlgorithm, void * aKey)
 {
   // Clear previous key data if it exists
   CleanUp();
 
   switch (aAlgorithm) {
     case nsIKeyObject::RC4:
+    case nsIKeyObject::HMAC:
       mSymKey = reinterpret_cast<PK11SymKey*>(aKey);
 
       if (!mSymKey) {
         NS_ERROR("no symkey");
         break;
       }
       mKeyType = nsIKeyObject::SYM_KEY;
       break;
@@ -146,17 +147,17 @@ nsKeyObject::GetType(PRInt16 *_retval)
 
   *_retval = mKeyType;
   return NS_OK;
 }
 
 //////////////////////////////////////////////////////////////////////////////
 // nsIKeyObjectFactory
 
-NS_IMPL_ISUPPORTS1(nsKeyObjectFactory, nsIKeyObjectFactory)
+NS_IMPL_THREADSAFE_ISUPPORTS1(nsKeyObjectFactory, nsIKeyObjectFactory)
 
 nsKeyObjectFactory::nsKeyObjectFactory()
 {
 }
 
 /* nsIKeyObject lookupKeyByName (in ACString aName); */
 NS_IMETHODIMP
 nsKeyObjectFactory::LookupKeyByName(const nsACString & aName,
@@ -171,41 +172,54 @@ nsKeyObjectFactory::UnwrapKey(PRInt16 aA
 {
   return NS_ERROR_NOT_IMPLEMENTED;
 }
 
 NS_IMETHODIMP
 nsKeyObjectFactory::KeyFromString(PRInt16 aAlgorithm, const nsACString & aKey,
                                   nsIKeyObject **_retval)
 {
-  if (aAlgorithm != nsIKeyObject::RC4)
+  CK_MECHANISM_TYPE cipherMech;
+  CK_ATTRIBUTE_TYPE cipherOperation;
+  switch (aAlgorithm)
+  {
+  case nsIKeyObject::HMAC:
+    cipherMech = CKM_GENERIC_SECRET_KEY_GEN;
+    cipherOperation = CKA_SIGN;
+    break;
+
+  case nsIKeyObject::RC4:
+    cipherMech = CKM_RC4;
+    cipherOperation = CKA_ENCRYPT;
+    break;
+
+  default:
     return NS_ERROR_INVALID_ARG;
-  
+  }
+
   nsresult rv;
   nsCOMPtr<nsIKeyObject> key =
       do_CreateInstance(NS_KEYMODULEOBJECT_CONTRACTID, &rv);
   NS_ENSURE_SUCCESS(rv, rv);
 
   // Convert the raw string into a SECItem
   const nsCString& flatKey = PromiseFlatCString(aKey);
   SECItem keyItem;
   keyItem.data = (unsigned char*)flatKey.get();
   keyItem.len = flatKey.Length();
 
   PK11SlotInfo *slot = nsnull;
-  CK_MECHANISM_TYPE cipherMech;
-  cipherMech = CKM_RC4;
   slot = PK11_GetBestSlot(cipherMech, nsnull);
   if (!slot) {
     NS_ERROR("no slot");
     return NS_ERROR_FAILURE;
   }
 
   PK11SymKey* symKey = PK11_ImportSymKey(slot, cipherMech, PK11_OriginUnwrap,
-                                         CKA_ENCRYPT, &keyItem, nsnull);
+                                         cipherOperation, &keyItem, nsnull);
   // cleanup code
   if (slot)
     PK11_FreeSlot(slot);
 
   if (!symKey) {
     return NS_ERROR_FAILURE;
   }
   
--- a/security/manager/ssl/src/nsNSSComponent.cpp
+++ b/security/manager/ssl/src/nsNSSComponent.cpp
@@ -98,17 +98,17 @@
 #include "nsIEntropyCollector.h"
 #include "nsIBufEntropyCollector.h"
 #include "nsIServiceManager.h"
 #include "nsILocalFile.h"
 #include "nsITokenPasswordDialogs.h"
 #include "nsICRLManager.h"
 #include "nsNSSShutDown.h"
 #include "nsSmartCardEvent.h"
-#include "nsICryptoHash.h"
+#include "nsIKeyModule.h"
 
 #include "nss.h"
 #include "pk11func.h"
 #include "ssl.h"
 #include "sslproto.h"
 #include "secmod.h"
 #include "sechash.h"
 #include "secmime.h"
@@ -2551,18 +2551,18 @@ nsCryptoHMAC::nsCryptoHMAC()
 }
 
 nsCryptoHMAC::~nsCryptoHMAC()
 {
   if (mHMACContext)
     PK11_DestroyContext(mHMACContext, PR_TRUE);
 }
 
-/* void init (in unsigned long aAlgorithm, in octet aKeyData, in long aKeyLength); */
-NS_IMETHODIMP nsCryptoHMAC::Init(PRUint32 aAlgorithm, const PRUint8 *aKeyData, PRUint32 aKeyLen)
+/* void init (in unsigned long aAlgorithm, in nsIKeyObject aKeyObject); */
+NS_IMETHODIMP nsCryptoHMAC::Init(PRUint32 aAlgorithm, nsIKeyObject *aKeyObject)
 {
   if (mHMACContext)
   {
     PK11_DestroyContext(mHMACContext, PR_TRUE);
     mHMACContext = nsnull;
   }
 
   CK_MECHANISM_TYPE HMACMechType;
@@ -2579,35 +2579,36 @@ NS_IMETHODIMP nsCryptoHMAC::Init(PRUint3
   case nsCryptoHMAC::SHA384:
     HMACMechType = CKM_SHA384_HMAC; break;
   case nsCryptoHMAC::SHA512:
     HMACMechType = CKM_SHA512_HMAC; break;
   default:
     return NS_ERROR_INVALID_ARG;
   }
 
-  PK11SlotInfo *slot = PK11_GetBestSlot(HMACMechType, nsnull);
-  NS_ENSURE_TRUE(slot, NS_ERROR_FAILURE);
+  NS_ENSURE_ARG_POINTER(aKeyObject);
+
+  nsresult rv;
+
+  PRInt16 keyType;
+  rv = aKeyObject->GetType(&keyType);
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  NS_ENSURE_TRUE(keyType == nsIKeyObject::SYM_KEY, NS_ERROR_INVALID_ARG);
+
+  PK11SymKey* key;
+  // GetKeyObj doesn't addref the key
+  rv = aKeyObject->GetKeyObj((void**)&key);
+  NS_ENSURE_SUCCESS(rv, rv);
 
   SECItem rawData;
-  rawData.data = const_cast<unsigned char*>(aKeyData);
-  rawData.len = aKeyLen;
-
-  PK11SymKey* key = PK11_ImportSymKey(
-      slot, HMACMechType, PK11_OriginUnwrap, CKA_SIGN, &rawData, nsnull);
-  PK11_FreeSlot(slot);
-
-  NS_ENSURE_TRUE(key, NS_ERROR_FAILURE);
-
   rawData.data = 0;
   rawData.len = 0;
   mHMACContext = PK11_CreateContextBySymKey(
       HMACMechType, CKA_SIGN, key, &rawData);
-  PK11_FreeSymKey(key);
-
   NS_ENSURE_TRUE(mHMACContext, NS_ERROR_FAILURE);
 
   SECStatus ss = PK11_DigestBegin(mHMACContext);
   NS_ENSURE_TRUE(ss == SECSuccess, NS_ERROR_FAILURE);
 
   return NS_OK;
 }
 
--- a/security/manager/ssl/tests/test_hmac.js
+++ b/security/manager/ssl/tests/test_hmac.js
@@ -1,46 +1,90 @@
 var ScriptableUnicodeConverter =
   Components.Constructor("@mozilla.org/intl/scriptableunicodeconverter",
                          "nsIScriptableUnicodeConverter"); 
                          
-function getHMAC(data, key)
+function getHMAC(data, key, alg)
 {
   var converter = new ScriptableUnicodeConverter(); 
   converter.charset = 'utf8'; 
-  var keyarray = converter.convertToByteArray(key, {});
   var dataarray = converter.convertToByteArray(data, {});
   
+  var keyObject = Components.classes["@mozilla.org/security/keyobjectfactory;1"]
+    .getService(Components.interfaces.nsIKeyObjectFactory)
+      .keyFromString(Components.interfaces.nsIKeyObject.HMAC, key);
+  
   var cryptoHMAC = Components.classes["@mozilla.org/security/hmac;1"]
     .createInstance(Components.interfaces.nsICryptoHMAC);
     
-  cryptoHMAC.init(Components.interfaces.nsICryptoHMAC.SHA1, keyarray, keyarray.length);
+  cryptoHMAC.init(alg, keyObject);
   cryptoHMAC.update(dataarray, dataarray.length);
-  var digest1 = cryptoHMAC.finish(true);
+  var digest1 = cryptoHMAC.finish(false);
   
   cryptoHMAC.reset();
   cryptoHMAC.update(dataarray, dataarray.length);
-  var digest2 = cryptoHMAC.finish(true);
+  var digest2 = cryptoHMAC.finish(false);
   
   do_check_eq(digest1, digest2);
   
   return digest1;
 }
 
-function run_test() {
+function testHMAC(alg) {
   const key1 = 'MyKey_ABCDEFGHIJKLMN';
   const key2 = 'MyKey_01234567890123';
   
   const dataA = "Secret message";
   const dataB = "Secres message";
   
-  var diegest1a = getHMAC(key1, dataA);
-  var diegest2 = getHMAC(key2, dataA);
-  var diegest1b = getHMAC(key1, dataA);
+  var diegest1a = getHMAC(key1, dataA, alg);
+  var diegest2 = getHMAC(key2, dataA, alg);
+  var diegest1b = getHMAC(key1, dataA, alg);
   
   do_check_eq(diegest1a, diegest1b);
   do_check_neq(diegest1a, diegest2);
   
-  var diegest1 = getHMAC(key1, dataA);
-  diegest2 = getHMAC(key1, dataB);
+  var diegest1 = getHMAC(key1, dataA, alg);
+  diegest2 = getHMAC(key1, dataB, alg);
   
   do_check_neq(diegest1, diegest2);
+  
+  return diegest1;
 }
+
+function hexdigest(data) {
+  return [("0" + data.charCodeAt(i).toString(16)).slice(-2) for (i in data)].join("");
+}
+
+function testVectors() {
+  // These are test vectors taken from RFC 4231, section 4.3. (Test Case 2)
+  
+  const keyTestVector = "Jefe";
+  const dataTestVector = "what do ya want for nothing?";
+
+  var diegest;
+  /*
+  Bug 356713    
+  diegest = hexdigest(getHMAC(dataTestVector, keyTestVector, 
+          Components.interfaces.nsICryptoHMAC.SHA224));
+  do_check_eq(diegest, "a30e01098bc6dbbf45690f3a7e9e6d0f8bbea2a39e6148008fd05e44");
+  */
+
+  diegest = hexdigest(getHMAC(dataTestVector, keyTestVector, 
+          Components.interfaces.nsICryptoHMAC.SHA256));
+  do_check_eq(diegest, "5bdcc146bf60754e6a042426089575c75a003f089d2739839dec58b964ec3843"); 
+
+  diegest = hexdigest(getHMAC(dataTestVector, keyTestVector, 
+          Components.interfaces.nsICryptoHMAC.SHA384));
+  do_check_eq(diegest, "af45d2e376484031617f78d2b58a6b1b9c7ef464f5a01b47e42ec3736322445e8e2240ca5e69e2c78b3239ecfab21649"); 
+
+  diegest = hexdigest(getHMAC(dataTestVector, keyTestVector, 
+          Components.interfaces.nsICryptoHMAC.SHA512));
+  do_check_eq(diegest, "164b7a7bfcf819e2e395fbe73b56e0a387bd64222e831fd610270cd7ea2505549758bf75c05a994a6d034f65f8f0e6fdcaeab1a34d4a6b4b636e070a38bce737"); 
+}
+
+function run_test() {
+  testVectors();
+
+  testHMAC(Components.interfaces.nsICryptoHMAC.SHA1);
+  testHMAC(Components.interfaces.nsICryptoHMAC.SHA512);
+  testHMAC(Components.interfaces.nsICryptoHMAC.MD5);
+}
--- a/toolkit/components/url-classifier/src/nsUrlClassifierDBService.cpp
+++ b/toolkit/components/url-classifier/src/nsUrlClassifierDBService.cpp
@@ -46,16 +46,17 @@
 #include "mozStorageHelper.h"
 #include "mozStorageCID.h"
 #include "nsAppDirectoryServiceDefs.h"
 #include "nsAutoLock.h"
 #include "nsCRT.h"
 #include "nsICryptoHash.h"
 #include "nsICryptoHMAC.h"
 #include "nsIDirectoryService.h"
+#include "nsIKeyModule.h"
 #include "nsIObserverService.h"
 #include "nsIPrefBranch.h"
 #include "nsIPrefBranch2.h"
 #include "nsIPrefService.h"
 #include "nsIProperties.h"
 #include "nsIProxyObjectManager.h"
 #include "nsToolkitCompsCID.h"
 #include "nsIUrlClassifierUtils.h"
@@ -2685,27 +2686,41 @@ nsUrlClassifierDBServiceWorker::BeginStr
   NS_ENSURE_STATE(!mInStream);
 
   mInStream = PR_TRUE;
 
   nsresult rv;
 
   // If we're expecting a MAC, create the nsICryptoHMAC component now.
   if (!mUpdateClientKey.IsEmpty()) {
+    nsCOMPtr<nsIKeyObjectFactory> keyObjectFactory(do_GetService(
+        "@mozilla.org/security/keyobjectfactory;1", &rv));
+    if (NS_FAILED(rv)) {
+      NS_WARNING("Failed to get nsIKeyObjectFactory service");
+      mUpdateStatus = rv;
+      return mUpdateStatus;
+    }
+
+    nsCOMPtr<nsIKeyObject> keyObject;
+    rv = keyObjectFactory->KeyFromString(nsIKeyObject::HMAC, mUpdateClientKey, 
+        getter_AddRefs(keyObject));
+    if (NS_FAILED(rv)) {
+      NS_WARNING("Failed to create key object, maybe not FIPS compliant?");
+      mUpdateStatus = rv;
+      return mUpdateStatus;
+    }
+
     mHMAC = do_CreateInstance(NS_CRYPTO_HMAC_CONTRACTID, &rv);
     if (NS_FAILED(rv)) {
       NS_WARNING("Failed to create nsICryptoHMAC instance");
       mUpdateStatus = rv;
       return mUpdateStatus;
     }
-    NS_ENSURE_SUCCESS(rv, rv);
-
-    rv = mHMAC->Init(nsICryptoHMAC::SHA1,
-                     reinterpret_cast<const PRUint8*>(mUpdateClientKey.BeginReading()),
-                     mUpdateClientKey.Length());
+
+    rv = mHMAC->Init(nsICryptoHMAC::SHA1, keyObject);
     if (NS_FAILED(rv)) {
       NS_WARNING("Failed to initialize nsICryptoHMAC instance");
       mUpdateStatus = rv;
       return mUpdateStatus;
     }
   }
 
   mServerMAC = serverMAC;
--- a/toolkit/components/url-classifier/src/nsUrlClassifierHashCompleter.cpp
+++ b/toolkit/components/url-classifier/src/nsUrlClassifierHashCompleter.cpp
@@ -35,16 +35,17 @@
  * the terms of any one of the MPL, the GPL or the LGPL.
  *
  * ***** END LICENSE BLOCK ***** */
 
 #include "nsUrlClassifierHashCompleter.h"
 #include "nsIChannel.h"
 #include "nsICryptoHMAC.h"
 #include "nsIHttpChannel.h"
+#include "nsIKeyModule.h"
 #include "nsIObserverService.h"
 #include "nsIUploadChannel.h"
 #include "nsNetUtil.h"
 #include "nsStreamUtils.h"
 #include "nsStringStream.h"
 #include "nsServiceManagerUtils.h"
 #include "nsThreadUtils.h"
 #include "nsUrlClassifierDBService.h"
@@ -219,23 +220,31 @@ nsUrlClassifierHashCompleterRequest::Han
 
     // Let the hash completer know that we need a new key.
     return mCompleter->RekeyRequested();
   }
 
   nsUrlClassifierUtils::UnUrlsafeBase64(serverMAC);
 
   nsresult rv;
+
+  nsCOMPtr<nsIKeyObjectFactory> keyObjectFactory(
+    do_GetService("@mozilla.org/security/keyobjectfactory;1", &rv));
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  nsCOMPtr<nsIKeyObject> keyObject;
+  rv = keyObjectFactory->KeyFromString(nsIKeyObject::HMAC, mClientKey,
+      getter_AddRefs(keyObject));
+  NS_ENSURE_SUCCESS(rv, rv);
+
   nsCOMPtr<nsICryptoHMAC> hmac =
     do_CreateInstance(NS_CRYPTO_HMAC_CONTRACTID, &rv);
   NS_ENSURE_SUCCESS(rv, rv);
 
-  rv = hmac->Init(nsICryptoHMAC::SHA1,
-                  reinterpret_cast<const PRUint8*>(mClientKey.BeginReading()),
-                  mClientKey.Length());
+  rv = hmac->Init(nsICryptoHMAC::SHA1, keyObject);
   NS_ENSURE_SUCCESS(rv, rv);
 
   const nsCSubstring &remaining = Substring(begin, end);
   rv = hmac->Update(reinterpret_cast<const PRUint8*>(remaining.BeginReading()),
                     remaining.Length());
   NS_ENSURE_SUCCESS(rv, rv);
 
   nsCAutoString clientMAC;
--- a/toolkit/components/url-classifier/tests/unit/test_streamupdater.js
+++ b/toolkit/components/url-classifier/tests/unit/test_streamupdater.js
@@ -3,21 +3,22 @@ var gClientKeyRaw="TESTCLIENTKEY";
 var gClientKey = "VEVTVENMSUVOVEtFWQ==";
 
 function MAC(content, clientKey)
 {
   var hmac = Cc["@mozilla.org/security/hmac;1"].createInstance(Ci.nsICryptoHMAC);
   var converter = Cc["@mozilla.org/intl/scriptableunicodeconverter"].
                   createInstance(Ci.nsIScriptableUnicodeConverter);
   converter.charset = "UTF-8";
+
+  var keyObject = Cc["@mozilla.org/security/keyobjectfactory;1"]
+    .getService(Ci.nsIKeyObjectFactory).keyFromString(Ci.nsIKeyObject.HMAC, clientKey);
+  hmac.init(Ci.nsICryptoHMAC.SHA1, keyObject);
+
   var result = {};
-  var data = converter.convertToByteArray(clientKey, result);
-  hmac.init(Ci.nsICryptoHMAC.SHA1, data, data.length);
-
-  result = {};
   data = converter.convertToByteArray(content, result);
   hmac.update(data, data.length);
   return hmac.finish(true);
 }
 
 function doTest(updates, assertions, expectError, clientKey)
 {
   if (expectError) {