From 78506d6708239d48991bfdba1cb8d533fef9f0f0 Mon Sep 17 00:00:00 2001 From: David Keeler Date: Fri, 4 Dec 2015 10:36:51 -0800 Subject: [PATCH] bug 1230377 - part 2/2: simplify nsIKeyObject and nsIKeyObjectFactory r=jcj nsIKeyObject and nsIKeyObjectFactory defined an interface that was largely unimplemented. This cuts the interface back to what actually exists in code. --- security/manager/ssl/nsCryptoHash.cpp | 2 +- security/manager/ssl/nsIKeyModule.idl | 34 ++---- security/manager/ssl/nsKeyModule.cpp | 156 +++++++------------------- security/manager/ssl/nsKeyModule.h | 25 ++--- 4 files changed, 63 insertions(+), 154 deletions(-) diff --git a/security/manager/ssl/nsCryptoHash.cpp b/security/manager/ssl/nsCryptoHash.cpp index de2a4e3ef08..b917a2d7856 100644 --- a/security/manager/ssl/nsCryptoHash.cpp +++ b/security/manager/ssl/nsCryptoHash.cpp @@ -301,7 +301,7 @@ nsCryptoHMAC::Init(uint32_t aAlgorithm, nsIKeyObject *aKeyObject) PK11SymKey* key; // GetKeyObj doesn't addref the key - rv = aKeyObject->GetKeyObj((void**)&key); + rv = aKeyObject->GetKeyObj(&key); NS_ENSURE_SUCCESS(rv, rv); SECItem rawData; diff --git a/security/manager/ssl/nsIKeyModule.idl b/security/manager/ssl/nsIKeyModule.idl index 63a775140fa..d9e1d11388e 100644 --- a/security/manager/ssl/nsIKeyModule.idl +++ b/security/manager/ssl/nsIKeyModule.idl @@ -4,46 +4,34 @@ #include "nsISupports.idl" +%{ C++ + /* forward declaration */ + typedef struct PK11SymKeyStr PK11SymKey; +%} +[ptr] native PK11SymKeyPtr(PK11SymKey); + // An opaque key object. -[scriptable, uuid(4b31f4ed-9424-4710-b946-79b7e33cf3a8)] +[scriptable, uuid(ee2dc516-ba7b-4e77-89fe-c43b886ef715)] 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); + [noscript] void initKey(in short aAlgorithm, in PK11SymKeyPtr aKey); - // Return a pointer to the underlying key object - [noscript] voidPtr getKeyObj(); + // Returns a pointer to the underlying key object. + [noscript] PK11SymKeyPtr getKeyObj(); - // Will return NS_ERROR_NOT_INITIALIZED if initKey hasn't been run short getType(); }; -[scriptable, uuid(264eb54d-e20d-49a0-890c-1a5986ea81c4)] +[scriptable, uuid(838bdbf1-8244-448f-8bcd-cede70227d75)] interface nsIKeyObjectFactory : nsISupports { - nsIKeyObject lookupKeyByName(in ACString aName); - - nsIKeyObject unwrapKey(in short aAlgorithm, - [const, array, size_is(aWrappedKeyLen)] in octet aWrappedKey, - in unsigned long aWrappedKeyLen); - - // TODO: deriveKeyFrom* - - - // DO NOT USE - // This is not FIPS compliant and should not be used. nsIKeyObject keyFromString(in short aAlgorithm, in ACString aKey); }; diff --git a/security/manager/ssl/nsKeyModule.cpp b/security/manager/ssl/nsKeyModule.cpp index 9e02fbc4187..c6688a51fc8 100644 --- a/security/manager/ssl/nsKeyModule.cpp +++ b/security/manager/ssl/nsKeyModule.cpp @@ -2,11 +2,10 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ -#include "nsComponentManagerUtils.h" #include "nsCOMPtr.h" +#include "nsComponentManagerUtils.h" #include "nsKeyModule.h" #include "nsString.h" -#include "ScopedNSSTypes.h" using namespace mozilla; using namespace mozilla::psm; @@ -14,8 +13,7 @@ using namespace mozilla::psm; NS_IMPL_ISUPPORTS(nsKeyObject, nsIKeyObject) nsKeyObject::nsKeyObject() - : mKeyType(0), mSymKey(nullptr), mPrivateKey(nullptr), - mPublicKey(nullptr) + : mSymKey(nullptr) { } @@ -38,104 +36,57 @@ nsKeyObject::virtualDestroyNSSReference() void nsKeyObject::destructorSafeDestroyNSSReference() { - switch (mKeyType) { - case nsIKeyObject::SYM_KEY: - PK11_FreeSymKey(mSymKey); - break; - - case nsIKeyObject::PRIVATE_KEY: - PK11_DeleteTokenPrivateKey(mPrivateKey, true /* force */); - break; - - case nsIKeyObject::PUBLIC_KEY: - PK11_DeleteTokenPublicKey(mPublicKey); - break; - - default: - // probably not initialized, do nothing - break; - } - mKeyType = 0; + mSymKey = nullptr; } ////////////////////////////////////////////////////////////////////////////// // nsIKeyObject NS_IMETHODIMP -nsKeyObject::InitKey(int16_t aAlgorithm, void * aKey) +nsKeyObject::InitKey(int16_t aAlgorithm, PK11SymKey* aKey) { + if (!aKey || aAlgorithm != nsIKeyObject::HMAC) { + return NS_ERROR_INVALID_ARG; + } + nsNSSShutDownPreventionLock locker; if (isAlreadyShutDown()) { return NS_ERROR_NOT_AVAILABLE; } - // Clear previous key data if it exists - destructorSafeDestroyNSSReference(); - - switch (aAlgorithm) { - case nsIKeyObject::RC4: - case nsIKeyObject::HMAC: - mSymKey = reinterpret_cast(aKey); - - if (!mSymKey) { - NS_ERROR("no symkey"); - break; - } - mKeyType = nsIKeyObject::SYM_KEY; - break; - - case nsIKeyObject::AES_CBC: - return NS_ERROR_NOT_IMPLEMENTED; - - default: - return NS_ERROR_INVALID_ARG; - } - - // One of these should have been created - if (!mSymKey && !mPrivateKey && !mPublicKey) - return NS_ERROR_FAILURE; - + mSymKey = aKey; return NS_OK; } NS_IMETHODIMP -nsKeyObject::GetKeyObj(void * *_retval) +nsKeyObject::GetKeyObj(PK11SymKey** _retval) { + if (!_retval) { + return NS_ERROR_INVALID_ARG; + } + + *_retval = nullptr; + nsNSSShutDownPreventionLock locker; if (isAlreadyShutDown()) { return NS_ERROR_NOT_AVAILABLE; } - if (mKeyType == 0) + if (!mSymKey) { return NS_ERROR_NOT_INITIALIZED; - - switch (mKeyType) { - case nsIKeyObject::SYM_KEY: - *_retval = (void*)mSymKey; - break; - - case nsIKeyObject::PRIVATE_KEY: - *_retval = (void*)mPublicKey; - break; - - case nsIKeyObject::PUBLIC_KEY: - *_retval = (void*)mPrivateKey; - break; - - default: - // unknown key type? How did that happen? - return NS_ERROR_FAILURE; } + + *_retval = mSymKey; return NS_OK; } NS_IMETHODIMP nsKeyObject::GetType(int16_t *_retval) { - if (mKeyType == 0) - return NS_ERROR_NOT_INITIALIZED; - - *_retval = mKeyType; + if (!_retval) { + return NS_ERROR_INVALID_ARG; + } + *_retval = nsIKeyObject::SYM_KEY; return NS_OK; } @@ -149,50 +100,27 @@ nsKeyObjectFactory::nsKeyObjectFactory() } NS_IMETHODIMP -nsKeyObjectFactory::LookupKeyByName(const nsACString & aName, - nsIKeyObject **_retval) +nsKeyObjectFactory::KeyFromString(int16_t aAlgorithm, const nsACString& aKey, + nsIKeyObject** _retval) { - return NS_ERROR_NOT_IMPLEMENTED; -} - -NS_IMETHODIMP -nsKeyObjectFactory::UnwrapKey(int16_t aAlgorithm, const uint8_t *aWrappedKey, - uint32_t aWrappedKeyLen, nsIKeyObject **_retval) -{ - return NS_ERROR_NOT_IMPLEMENTED; -} + if (!_retval || aAlgorithm != nsIKeyObject::HMAC) { + return NS_ERROR_INVALID_ARG; + } -NS_IMETHODIMP -nsKeyObjectFactory::KeyFromString(int16_t aAlgorithm, const nsACString & aKey, - nsIKeyObject **_retval) -{ nsNSSShutDownPreventionLock locker; if (isAlreadyShutDown()) { return NS_ERROR_NOT_AVAILABLE; } - 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; - } + CK_MECHANISM_TYPE cipherMech = CKM_GENERIC_SECRET_KEY_GEN; + CK_ATTRIBUTE_TYPE cipherOperation = CKA_SIGN; nsresult rv; - nsCOMPtr key = - do_CreateInstance(NS_KEYMODULEOBJECT_CONTRACTID, &rv); - NS_ENSURE_SUCCESS(rv, rv); + nsCOMPtr key( + do_CreateInstance(NS_KEYMODULEOBJECT_CONTRACTID, &rv)); + if (NS_FAILED(rv)) { + return rv; + } // Convert the raw string into a SECItem const nsCString& flatKey = PromiseFlatCString(aKey); @@ -202,18 +130,20 @@ nsKeyObjectFactory::KeyFromString(int16_t aAlgorithm, const nsACString & aKey, ScopedPK11SlotInfo slot(PK11_GetBestSlot(cipherMech, nullptr)); if (!slot) { - NS_ERROR("no slot"); return NS_ERROR_FAILURE; } - PK11SymKey* symKey = PK11_ImportSymKey(slot, cipherMech, PK11_OriginUnwrap, - cipherOperation, &keyItem, nullptr); + ScopedPK11SymKey symKey(PK11_ImportSymKey(slot, cipherMech, + PK11_OriginUnwrap, cipherOperation, + &keyItem, nullptr)); if (!symKey) { return NS_ERROR_FAILURE; } - - rv = key->InitKey(aAlgorithm, (void*)symKey); - NS_ENSURE_SUCCESS(rv, rv); + + rv = key->InitKey(aAlgorithm, symKey.forget()); + if (NS_FAILED(rv)) { + return rv; + } key.swap(*_retval); return NS_OK; diff --git a/security/manager/ssl/nsKeyModule.h b/security/manager/ssl/nsKeyModule.h index 2ca23b1d678..b75e8bc8a5a 100644 --- a/security/manager/ssl/nsKeyModule.h +++ b/security/manager/ssl/nsKeyModule.h @@ -2,22 +2,20 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ -#ifndef _NS_KEYMODULE_H_ -#define _NS_KEYMODULE_H_ +#ifndef nsKeyModule_h +#define nsKeyModule_h -#include "mozilla/Attributes.h" +#include "ScopedNSSTypes.h" #include "nsIKeyModule.h" #include "nsNSSShutDown.h" #include "pk11pub.h" -/* eae599aa-ecef-49c6-a8af-6ddcc6feb484 */ #define NS_KEYMODULEOBJECT_CID \ -{ 0xeae599aa, 0xecef, 0x49c6, {0xa8, 0xaf, 0x6d, 0xdc, 0xc6, 0xfe, 0xb4, 0x84} } +{ 0x9d383ddd, 0x6856, 0x4187, {0x84, 0x85, 0xf3, 0x61, 0x95, 0xb2, 0x9a, 0x0e} } #define NS_KEYMODULEOBJECT_CONTRACTID "@mozilla.org/security/keyobject;1" -/* a39e0e9d-e567-41e3-b12c-5df67f18174d */ #define NS_KEYMODULEOBJECTFACTORY_CID \ -{ 0xa39e0e9d, 0xe567, 0x41e3, {0xb1, 0x2c, 0x5d, 0xf6, 0x7f, 0x18, 0x17, 0x4d} } +{ 0x2a35dd47, 0xb026, 0x4e8d, {0xb6, 0xb7, 0x57, 0x40, 0xf6, 0x1a, 0xb9, 0x02} } #define NS_KEYMODULEOBJECTFACTORY_CONTRACTID \ "@mozilla.org/security/keyobjectfactory;1" @@ -32,18 +30,11 @@ public: private: ~nsKeyObject(); - + // Disallow copy constructor nsKeyObject(nsKeyObject&); - // 0 if not yet set, otherwise one of the nsIKeyObject::*KEY values - uint32_t mKeyType; - - // A union of our possible key types - PK11SymKey* mSymKey; - SECKEYPrivateKey* mPrivateKey; - SECKEYPublicKey* mPublicKey; - + ScopedPK11SymKey mSymKey; virtual void virtualDestroyNSSReference() override; void destructorSafeDestroyNSSReference(); @@ -69,4 +60,4 @@ private: virtual void virtualDestroyNSSReference() override {} }; -#endif // _NS_KEYMODULE_H_ +#endif // nsKeyModule_h