From bc39c42ca397899a88cd84095aa6c74957fe822e Mon Sep 17 00:00:00 2001 From: "reed@reedloden.com" Date: Tue, 18 Mar 2008 12:45:40 -0700 Subject: [PATCH] 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 | 15 +++-- security/manager/ssl/public/nsIKeyModule.idl | 1 + security/manager/ssl/src/nsKeyModule.cpp | 26 ++++++-- security/manager/ssl/src/nsNSSComponent.cpp | 33 +++++----- security/manager/ssl/tests/test_hmac.js | 66 +++++++++++++++---- .../src/nsUrlClassifierDBService.cpp | 23 +++++-- .../src/nsUrlClassifierHashCompleter.cpp | 15 ++++- .../tests/unit/test_streamupdater.js | 9 +-- 8 files changed, 138 insertions(+), 50 deletions(-) diff --git a/netwerk/base/public/nsICryptoHMAC.idl b/netwerk/base/public/nsICryptoHMAC.idl index 96608cf2fd4..ce8fcc2e6f8 100644 --- a/netwerk/base/public/nsICryptoHMAC.idl +++ b/netwerk/base/public/nsICryptoHMAC.idl @@ -38,13 +38,14 @@ #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 { /** @@ -69,10 +70,13 @@ interface nsICryptoHMAC : nsISupports * 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. @@ -80,8 +84,7 @@ interface nsICryptoHMAC : nsISupports * 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 diff --git a/security/manager/ssl/public/nsIKeyModule.idl b/security/manager/ssl/public/nsIKeyModule.idl index 612403d4e6f..3b0efdb7bc8 100644 --- a/security/manager/ssl/public/nsIKeyModule.idl +++ b/security/manager/ssl/public/nsIKeyModule.idl @@ -48,6 +48,7 @@ interface nsIKeyObject : nsISupports // 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. diff --git a/security/manager/ssl/src/nsKeyModule.cpp b/security/manager/ssl/src/nsKeyModule.cpp index ee1267e1aac..43301587b98 100644 --- a/security/manager/ssl/src/nsKeyModule.cpp +++ b/security/manager/ssl/src/nsKeyModule.cpp @@ -87,6 +87,7 @@ nsKeyObject::InitKey(PRInt16 aAlgorithm, void * aKey) switch (aAlgorithm) { case nsIKeyObject::RC4: + case nsIKeyObject::HMAC: mSymKey = reinterpret_cast(aKey); if (!mSymKey) { @@ -151,7 +152,7 @@ nsKeyObject::GetType(PRInt16 *_retval) ////////////////////////////////////////////////////////////////////////////// // nsIKeyObjectFactory -NS_IMPL_ISUPPORTS1(nsKeyObjectFactory, nsIKeyObjectFactory) +NS_IMPL_THREADSAFE_ISUPPORTS1(nsKeyObjectFactory, nsIKeyObjectFactory) nsKeyObjectFactory::nsKeyObjectFactory() { @@ -176,9 +177,24 @@ 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 key = do_CreateInstance(NS_KEYMODULEOBJECT_CONTRACTID, &rv); @@ -191,8 +207,6 @@ nsKeyObjectFactory::KeyFromString(PRInt16 aAlgorithm, const nsACString & aKey, 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"); @@ -200,7 +214,7 @@ nsKeyObjectFactory::KeyFromString(PRInt16 aAlgorithm, const nsACString & aKey, } PK11SymKey* symKey = PK11_ImportSymKey(slot, cipherMech, PK11_OriginUnwrap, - CKA_ENCRYPT, &keyItem, nsnull); + cipherOperation, &keyItem, nsnull); // cleanup code if (slot) PK11_FreeSlot(slot); diff --git a/security/manager/ssl/src/nsNSSComponent.cpp b/security/manager/ssl/src/nsNSSComponent.cpp index 2bf64d1d682..38d4813e79f 100644 --- a/security/manager/ssl/src/nsNSSComponent.cpp +++ b/security/manager/ssl/src/nsNSSComponent.cpp @@ -103,7 +103,7 @@ #include "nsICRLManager.h" #include "nsNSSShutDown.h" #include "nsSmartCardEvent.h" -#include "nsICryptoHash.h" +#include "nsIKeyModule.h" #include "nss.h" #include "pk11func.h" @@ -2556,8 +2556,8 @@ nsCryptoHMAC::~nsCryptoHMAC() 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) { @@ -2584,25 +2584,26 @@ NS_IMETHODIMP nsCryptoHMAC::Init(PRUint32 aAlgorithm, const PRUint8 *aKeyData, P 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(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); diff --git a/security/manager/ssl/tests/test_hmac.js b/security/manager/ssl/tests/test_hmac.js index 920b9c2d536..98ead82a9b0 100644 --- a/security/manager/ssl/tests/test_hmac.js +++ b/security/manager/ssl/tests/test_hmac.js @@ -2,45 +2,89 @@ 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); } diff --git a/toolkit/components/url-classifier/src/nsUrlClassifierDBService.cpp b/toolkit/components/url-classifier/src/nsUrlClassifierDBService.cpp index 54e166cc261..a4cb780a9da 100644 --- a/toolkit/components/url-classifier/src/nsUrlClassifierDBService.cpp +++ b/toolkit/components/url-classifier/src/nsUrlClassifierDBService.cpp @@ -51,6 +51,7 @@ #include "nsICryptoHash.h" #include "nsICryptoHMAC.h" #include "nsIDirectoryService.h" +#include "nsIKeyModule.h" #include "nsIObserverService.h" #include "nsIPrefBranch.h" #include "nsIPrefBranch2.h" @@ -2690,17 +2691,31 @@ nsUrlClassifierDBServiceWorker::BeginStream(const nsACString &table, // If we're expecting a MAC, create the nsICryptoHMAC component now. if (!mUpdateClientKey.IsEmpty()) { + nsCOMPtr 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 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(mUpdateClientKey.BeginReading()), - mUpdateClientKey.Length()); + rv = mHMAC->Init(nsICryptoHMAC::SHA1, keyObject); if (NS_FAILED(rv)) { NS_WARNING("Failed to initialize nsICryptoHMAC instance"); mUpdateStatus = rv; diff --git a/toolkit/components/url-classifier/src/nsUrlClassifierHashCompleter.cpp b/toolkit/components/url-classifier/src/nsUrlClassifierHashCompleter.cpp index 0b4d17ae9be..4a8db016c9d 100644 --- a/toolkit/components/url-classifier/src/nsUrlClassifierHashCompleter.cpp +++ b/toolkit/components/url-classifier/src/nsUrlClassifierHashCompleter.cpp @@ -40,6 +40,7 @@ #include "nsIChannel.h" #include "nsICryptoHMAC.h" #include "nsIHttpChannel.h" +#include "nsIKeyModule.h" #include "nsIObserverService.h" #include "nsIUploadChannel.h" #include "nsNetUtil.h" @@ -224,13 +225,21 @@ nsUrlClassifierHashCompleterRequest::HandleMAC(nsACString::const_iterator& begin nsUrlClassifierUtils::UnUrlsafeBase64(serverMAC); nsresult rv; + + nsCOMPtr keyObjectFactory( + do_GetService("@mozilla.org/security/keyobjectfactory;1", &rv)); + NS_ENSURE_SUCCESS(rv, rv); + + nsCOMPtr keyObject; + rv = keyObjectFactory->KeyFromString(nsIKeyObject::HMAC, mClientKey, + getter_AddRefs(keyObject)); + NS_ENSURE_SUCCESS(rv, rv); + nsCOMPtr hmac = do_CreateInstance(NS_CRYPTO_HMAC_CONTRACTID, &rv); NS_ENSURE_SUCCESS(rv, rv); - rv = hmac->Init(nsICryptoHMAC::SHA1, - reinterpret_cast(mClientKey.BeginReading()), - mClientKey.Length()); + rv = hmac->Init(nsICryptoHMAC::SHA1, keyObject); NS_ENSURE_SUCCESS(rv, rv); const nsCSubstring &remaining = Substring(begin, end); diff --git a/toolkit/components/url-classifier/tests/unit/test_streamupdater.js b/toolkit/components/url-classifier/tests/unit/test_streamupdater.js index 1646cf6aa8b..8bd4fdd2d23 100644 --- a/toolkit/components/url-classifier/tests/unit/test_streamupdater.js +++ b/toolkit/components/url-classifier/tests/unit/test_streamupdater.js @@ -8,11 +8,12 @@ function MAC(content, clientKey) var converter = Cc["@mozilla.org/intl/scriptableunicodeconverter"]. createInstance(Ci.nsIScriptableUnicodeConverter); converter.charset = "UTF-8"; - var result = {}; - var data = converter.convertToByteArray(clientKey, result); - hmac.init(Ci.nsICryptoHMAC.SHA1, data, data.length); - result = {}; + var keyObject = Cc["@mozilla.org/security/keyobjectfactory;1"] + .getService(Ci.nsIKeyObjectFactory).keyFromString(Ci.nsIKeyObject.HMAC, clientKey); + hmac.init(Ci.nsICryptoHMAC.SHA1, keyObject); + + var result = {}; data = converter.convertToByteArray(content, result); hmac.update(data, data.length); return hmac.finish(true);