bug 1230377 - part 2/2: simplify nsIKeyObject and nsIKeyObjectFactory draft
authorDavid Keeler <dkeeler@mozilla.com>
Fri, 04 Dec 2015 10:36:51 -0800
changeset 317976 c2a56810f56c226eb0dae0e9093f9f261de9595b
parent 317972 0c86aa7ddb6b7555d1514d43d90111684bb7e278
child 512390 b69aecd2f62ba24e60dee9ebf57ed587cc5275c0
push id8798
push userdkeeler@mozilla.com
push dateWed, 30 Dec 2015 00:12:41 +0000
bugs1230377
milestone46.0a1
bug 1230377 - part 2/2: simplify nsIKeyObject and nsIKeyObjectFactory 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
security/manager/ssl/nsIKeyModule.idl
security/manager/ssl/nsKeyModule.cpp
security/manager/ssl/nsKeyModule.h
--- a/security/manager/ssl/nsCryptoHash.cpp
+++ b/security/manager/ssl/nsCryptoHash.cpp
@@ -296,17 +296,17 @@ nsCryptoHMAC::Init(uint32_t aAlgorithm, 
   int16_t 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);
+  rv = aKeyObject->GetKeyObj(&key);
   NS_ENSURE_SUCCESS(rv, rv);
 
   SECItem rawData;
   rawData.data = 0;
   rawData.len = 0;
   mHMACContext = PK11_CreateContextBySymKey(
       HMACMechType, CKA_SIGN, key, &rawData);
   NS_ENSURE_TRUE(mHMACContext, NS_ERROR_FAILURE);
--- a/security/manager/ssl/nsIKeyModule.idl
+++ b/security/manager/ssl/nsIKeyModule.idl
@@ -1,49 +1,37 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * 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 "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);
 };
--- a/security/manager/ssl/nsKeyModule.cpp
+++ b/security/manager/ssl/nsKeyModule.cpp
@@ -1,26 +1,24 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * 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 "nsCOMPtr.h"
 #include "nsComponentManagerUtils.h"
-#include "nsCOMPtr.h"
 #include "nsKeyModule.h"
 #include "nsString.h"
-#include "ScopedNSSTypes.h"
 
 using namespace mozilla;
 using namespace mozilla::psm;
 
 NS_IMPL_ISUPPORTS(nsKeyObject, nsIKeyObject)
 
 nsKeyObject::nsKeyObject()
-  : mKeyType(0), mSymKey(nullptr), mPrivateKey(nullptr),
-    mPublicKey(nullptr)
+  : mSymKey(nullptr)
 {
 }
 
 nsKeyObject::~nsKeyObject()
 {
   nsNSSShutDownPreventionLock locker;
   if (isAlreadyShutDown()) {
     return;
@@ -33,188 +31,120 @@ void
 nsKeyObject::virtualDestroyNSSReference()
 {
   destructorSafeDestroyNSSReference();
 }
 
 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<PK11SymKey*>(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;
 }
 
 //////////////////////////////////////////////////////////////////////////////
 // nsIKeyObjectFactory
 
 NS_IMPL_ISUPPORTS(nsKeyObjectFactory, nsIKeyObjectFactory)
 
 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<nsIKeyObject> key =
-      do_CreateInstance(NS_KEYMODULEOBJECT_CONTRACTID, &rv);
-  NS_ENSURE_SUCCESS(rv, rv);
+  nsCOMPtr<nsIKeyObject> 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);
   SECItem keyItem;
   keyItem.data = (unsigned char*)flatKey.get();
   keyItem.len = flatKey.Length();
 
   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;
 }
--- a/security/manager/ssl/nsKeyModule.h
+++ b/security/manager/ssl/nsKeyModule.h
@@ -1,54 +1,45 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * 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"
 
 class nsKeyObject final : public nsIKeyObject
                         , public nsNSSShutDownObject
 {
 public:
   nsKeyObject();
 
   NS_DECL_ISUPPORTS
   NS_DECL_NSIKEYOBJECT
 
 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();
 };
 
 
 class nsKeyObjectFactory final : public nsIKeyObjectFactory
                                , public nsNSSShutDownObject
@@ -64,9 +55,9 @@ private:
 
   // Disallow copy constructor
   nsKeyObjectFactory(nsKeyObjectFactory&);
 
   // No NSS resources to release.
   virtual void virtualDestroyNSSReference() override {}
 };
 
-#endif // _NS_KEYMODULE_H_
+#endif // nsKeyModule_h