bug 1233853 - make nsSyncJPAKE aware of NSS shutdown r=jcj

Also fixes a few preexisting leaks and uses scoped types so that sort of thing
will be less likely to happen in the future.

--HG--
extra : rebase_source : c7e0c09f3fae7ee7b702c6f85db3206ff2239c30

services/crypto/component/nsSyncJPAKE.cpp

@@ -132,6 +132,11 @@ NS_IMETHODIMP nsSyncJPAKE::Round1(const nsACString & aSignerID,
                                   nsACString & aGV2,
                                   nsACString & aR2)
 {
+  nsNSSShutDownPreventionLock locker;
+  if (isAlreadyShutDown()) {
+    return NS_ERROR_NOT_AVAILABLE;
+  }
+
   NS_ENSURE_STATE(round == JPAKENotStarted);
   NS_ENSURE_STATE(key == nullptr);
 
@@ -141,9 +146,9 @@ NS_IMETHODIMP nsSyncJPAKE::Round1(const nsACString & aSignerID,
     CKM_NSS_JPAKE_FINAL_SHA256
   };
 
-  PK11SlotInfo * slot = PK11_GetBestSlotMultiple(mechanisms,
-                                                 NUM_ELEM(mechanisms),
-                                                 nullptr);
+  ScopedPK11SlotInfo slot(PK11_GetBestSlotMultiple(mechanisms,
+                                                   NUM_ELEM(mechanisms),
+                                                   nullptr));
   NS_ENSURE_STATE(slot != nullptr);
     
   CK_BYTE pBuf[(NUM_ELEM(p) - 1) / 2];
@@ -208,6 +213,11 @@ NS_IMETHODIMP nsSyncJPAKE::Round2(const nsACString & aPeerID,
                                   nsACString & aGVA,
                                   nsACString & aRA)
 {
+  nsNSSShutDownPreventionLock locker;
+  if (isAlreadyShutDown()) {
+    return NS_ERROR_NOT_AVAILABLE;
+  }
+
   NS_ENSURE_STATE(round == JPAKEBeforeRound2);
   NS_ENSURE_STATE(key != nullptr);
   NS_ENSURE_ARG(!aPeerID.IsEmpty());
@@ -267,28 +277,27 @@ NS_IMETHODIMP nsSyncJPAKE::Round2(const nsACString & aPeerID,
     { CKA_NSS_JPAKE_PEERID, (CK_BYTE *) aPeerID.Data(), aPeerID.Length(), },
     { CKA_KEY_TYPE, &keyType, sizeof keyType }
   };
-  PK11SymKey * newKey = PK11_DeriveWithTemplate(key,
-                                                CKM_NSS_JPAKE_ROUND2_SHA256,
-                                                &paramsItem,
-                                                CKM_NSS_JPAKE_FINAL_SHA256,
-                                                CKA_DERIVE, 0,
-                                                keyTemplate,
-                                                NUM_ELEM(keyTemplate),
-                                                false);
+  ScopedPK11SymKey newKey(PK11_DeriveWithTemplate(key,
+                                                  CKM_NSS_JPAKE_ROUND2_SHA256,
+                                                  &paramsItem,
+                                                  CKM_NSS_JPAKE_FINAL_SHA256,
+                                                  CKA_DERIVE, 0,
+                                                  keyTemplate,
+                                                  NUM_ELEM(keyTemplate),
+                                                  false));
   if (newKey != nullptr) {
     if (toHexString(rp.A.pGX, rp.A.ulGXLen, aA) &&
         toHexString(rp.A.pGV, rp.A.ulGVLen, aGVA) &&
         toHexString(rp.A.pR, rp.A.ulRLen, aRA)) {
       round = JPAKEAfterRound2;
-      PK11_FreeSymKey(key);
-      key = newKey;
+      key = newKey.forget();
       return NS_OK;
     } else {
-      PK11_FreeSymKey(newKey);
       rv = NS_ERROR_OUT_OF_MEMORY;
     }
-  } else
+  } else {
     rv = mapErrno();
+  }
 
   return rv;
 }
@@ -304,10 +313,10 @@ setBase64(const unsigned char * data, unsigned len, nsACString & out)
     if (out.SetCapacity(len, fallible)) {
       out.SetLength(0);
       out.Append(base64, len);
-      PORT_Free((void*) base64);
     } else {
       rv = NS_ERROR_OUT_OF_MEMORY;
     }
+    PORT_Free((void*) base64);
   } else {
     rv = NS_ERROR_OUT_OF_MEMORY;
   }
@@ -355,6 +364,11 @@ NS_IMETHODIMP nsSyncJPAKE::Final(const nsACString & aB,
                                  nsACString & aAES256Key,
                                  nsACString & aHMAC256Key)
 {
+  nsNSSShutDownPreventionLock locker;
+  if (isAlreadyShutDown()) {
+    return NS_ERROR_NOT_AVAILABLE;
+  }
+
   static const unsigned AES256_KEY_SIZE = 256 / 8;
   static const unsigned HMAC_SHA256_KEY_SIZE = 256 / 8;
   CK_EXTRACT_PARAMS aesBitPosition = 0;
@@ -377,10 +391,10 @@ NS_IMETHODIMP nsSyncJPAKE::Final(const nsACString & aB,
   SECItem paramsItem;
   paramsItem.data = (unsigned char *) &rp;
   paramsItem.len = sizeof rp;
-  PK11SymKey * keyMaterial = PK11_Derive(key, CKM_NSS_JPAKE_FINAL_SHA256,
-                                         &paramsItem, CKM_NSS_HKDF_SHA256,
-                                         CKA_DERIVE, 0);
-  PK11SymKey * keyBlock = nullptr;
+  ScopedPK11SymKey keyMaterial(PK11_Derive(key, CKM_NSS_JPAKE_FINAL_SHA256,
+                                           &paramsItem, CKM_NSS_HKDF_SHA256,
+                                           CKA_DERIVE, 0));
+  ScopedPK11SymKey keyBlock;
 
   if (keyMaterial == nullptr)
     rv = mapErrno();
@@ -413,16 +427,11 @@ NS_IMETHODIMP nsSyncJPAKE::Final(const nsACString & aB,
 
   if (rv == NS_OK) {
     SECStatus srv = PK11_ExtractKeyValue(keyMaterial);
-    NS_ENSURE_TRUE(srv == SECSuccess, NS_ERROR_UNEXPECTED); // XXX leaks
+    NS_ENSURE_TRUE(srv == SECSuccess, NS_ERROR_UNEXPECTED);
     SECItem * keyMaterialBytes = PK11_GetKeyData(keyMaterial);
     NS_ENSURE_TRUE(keyMaterialBytes != nullptr, NS_ERROR_UNEXPECTED);
   }
 
-  if (keyBlock != nullptr)
-    PK11_FreeSymKey(keyBlock);
-  if (keyMaterial != nullptr)
-    PK11_FreeSymKey(keyMaterial);
-
   return rv;
 }
 
@@ -433,8 +442,24 @@ nsSyncJPAKE::nsSyncJPAKE() : round(JPAKENotStarted), key(nullptr) { }
 
 nsSyncJPAKE::~nsSyncJPAKE()
 {
-  if (key != nullptr)
-    PK11_FreeSymKey(key);
+  nsNSSShutDownPreventionLock locker;
+  if (isAlreadyShutDown()) {
+    return;
+  }
+  destructorSafeDestroyNSSReference();
+  shutdown(calledFromObject);
+}
+
+void
+nsSyncJPAKE::virtualDestroyNSSReference()
+{
+  destructorSafeDestroyNSSReference();
+}
+
+void
+nsSyncJPAKE::destructorSafeDestroyNSSReference()
+{
+  key.dispose();
 }
 
 static const mozilla::Module::CIDEntry kServicesCryptoCIDs[] = {

services/crypto/component/nsSyncJPAKE.h

@@ -4,7 +4,9 @@
 #ifndef nsSyncJPAKE_h__
 #define nsSyncJPAKE_h__
 
+#include "ScopedNSSTypes.h"
 #include "nsISyncJPAKE.h"
+#include "nsNSSShutDown.h"
 
 #define NS_SYNCJPAKE_CONTRACTID \
   "@mozilla.org/services-crypto/sync-jpake;1"
@@ -12,9 +14,10 @@
 #define NS_SYNCJPAKE_CID \
   {0x0b9721c0, 0x1805, 0x47c3, {0x86, 0xce, 0x68, 0x13, 0x79, 0x5a, 0x78, 0x3f}}
 
-typedef struct PK11SymKeyStr PK11SymKey;
+using namespace mozilla;
 
 class nsSyncJPAKE : public nsISyncJPAKE
+                  , public nsNSSShutDownObject
 {
 public:
   NS_DECL_ISUPPORTS
@@ -23,8 +26,11 @@ public:
 protected:
   virtual ~nsSyncJPAKE();
 private:
+  virtual void virtualDestroyNSSReference() override;
+  void destructorSafeDestroyNSSReference();
+
   enum { JPAKENotStarted, JPAKEBeforeRound2, JPAKEAfterRound2 } round;
-  PK11SymKey * key;
+  ScopedPK11SymKey key;
 };
 
 NS_IMPL_ISUPPORTS(nsSyncJPAKE, nsISyncJPAKE)