bug 1430906 - don't hold around a test key forever in mozilla::pkix gtests r=franziskus

Before this patch, mozilla::pkix gtests would generate a public/private key pair
and stash it in a global variable. Since this wasn't part of XPCOM nor tracked
by the PSM/NSS shutdown machinery, it wouldn't get released at the appropriate
time. The solution to this is to generate the key and then essentially export it
as data, so no NSS objects are held alive. Since NSS considers private keys
stored in the persistent database sensitive and won't export them in the clear,
we "encrypt" the key material with an empty password so we can import it when
necessary. (While the gtests don't use persistent keys, the test utilties in the
gtests are also used by some xpcshell tests that do use persistent keys, hence
the need to encrypt the key material.)

--HG--
extra : rebase_source : df10c25a462a3ba0396f5ba4a43a52fb924548ff
extra : amend_source : d95722891e49a99c471046cd9c758e914a02838e

security/manager/ssl/tests/unit/tlsserver/lib/OCSPCommon.cpp

@@ -7,19 +7,11 @@
 #include <stdio.h>
 
 #include "pkixtestutil.h"
+#include "pkixtestnss.h"
 #include "TLSServer.h"
 #include "secder.h"
 #include "secerr.h"
 
-namespace mozilla { namespace pkix { namespace test {
-
-// Ownership of privateKey is transfered.
-TestKeyPair* CreateTestKeyPair(const TestPublicKeyAlgorithm publicKeyAlg,
-                               const SECKEYPublicKey& publicKey,
-                               SECKEYPrivateKey* privateKey);
-
-} } } // namespace mozilla::pkix::test
-
 using namespace mozilla;
 using namespace mozilla::pkix;
 using namespace mozilla::pkix::test;
@@ -28,15 +20,15 @@ using namespace mozilla::test;
 static TestKeyPair*
 CreateTestKeyPairFromCert(const UniqueCERTCertificate& cert)
 {
-  UniqueSECKEYPrivateKey privateKey(PK11_FindKeyByAnyCert(cert.get(), nullptr));
+  ScopedSECKEYPrivateKey privateKey(PK11_FindKeyByAnyCert(cert.get(), nullptr));
   if (!privateKey) {
     return nullptr;
   }
-  UniqueSECKEYPublicKey publicKey(CERT_ExtractPublicKey(cert.get()));
+  ScopedSECKEYPublicKey publicKey(CERT_ExtractPublicKey(cert.get()));
   if (!publicKey) {
     return nullptr;
   }
-  return CreateTestKeyPair(RSA_PKCS1(), *publicKey.get(), privateKey.release());
+  return CreateTestKeyPair(RSA_PKCS1(), publicKey, privateKey);
 }
 
 SECItemArray*

security/nss.symbols

@@ -390,6 +390,7 @@ PK11_ImportCert
 PK11_ImportCertForKey
 PK11_ImportCRL
 PK11_ImportDERPrivateKeyInfoAndReturnKey
+PK11_ImportEncryptedPrivateKeyInfoAndReturnKey
 PK11_ImportPublicKey
 PK11_ImportSymKey
 PK11_InitPin

security/pkix/test/lib/pkixtestnss.cpp

@@ -23,6 +23,7 @@
  */
 
 #include "pkixtestutil.h"
+#include "pkixtestnss.h"
 
 #include <limits>
 
@@ -42,17 +43,18 @@ namespace mozilla { namespace pkix { namespace test {
 
 namespace {
 
-typedef ScopedPtr<SECKEYPublicKey, SECKEY_DestroyPublicKey>
-  ScopedSECKEYPublicKey;
-typedef ScopedPtr<SECKEYPrivateKey, SECKEY_DestroyPrivateKey>
-  ScopedSECKEYPrivateKey;
-
 inline void
 SECITEM_FreeItem_true(SECItem* item)
 {
   SECITEM_FreeItem(item, true);
 }
 
+inline void
+SECKEY_DestroyEncryptedPrivateKeyInfo_true(SECKEYEncryptedPrivateKeyInfo* e)
+{
+  SECKEY_DestroyEncryptedPrivateKeyInfo(e, true);
+}
+
 typedef mozilla::pkix::ScopedPtr<SECItem, SECITEM_FreeItem_true> ScopedSECItem;
 
 TestKeyPair* GenerateKeyPairInner();
@@ -78,12 +80,15 @@ InitReusedKeyPair()
 class NSSTestKeyPair final : public TestKeyPair
 {
 public:
-  // NSSTestKeyPair takes ownership of privateKey.
   NSSTestKeyPair(const TestPublicKeyAlgorithm& publicKeyAlg,
                  const ByteString& spk,
-                 SECKEYPrivateKey* privateKey)
+                 const ByteString& encryptedPrivateKey,
+                 const ByteString& encryptionAlgorithm,
+                 const ByteString& encryptionParams)
     : TestKeyPair(publicKeyAlg, spk)
-    , privateKey(privateKey)
+    , encryptedPrivateKey(encryptedPrivateKey)
+    , encryptionAlgorithm(encryptionAlgorithm)
+    , encryptionParams(encryptionParams)
   {
   }
 
@@ -121,10 +126,50 @@ public:
       abort();
     }
 
+    ScopedPtr<PK11SlotInfo, PK11_FreeSlot> slot(PK11_GetInternalSlot());
+    if (!slot) {
+      return MapPRErrorCodeToResult(PR_GetError());
+    }
+    SECItem encryptedPrivateKeyInfoItem = {
+      siBuffer,
+      const_cast<uint8_t*>(encryptedPrivateKey.data()),
+      static_cast<unsigned int>(encryptedPrivateKey.length())
+    };
+    SECItem encryptionAlgorithmItem = {
+      siBuffer,
+      const_cast<uint8_t*>(encryptionAlgorithm.data()),
+      static_cast<unsigned int>(encryptionAlgorithm.length())
+    };
+    SECItem encryptionParamsItem = {
+      siBuffer,
+      const_cast<uint8_t*>(encryptionParams.data()),
+      static_cast<unsigned int>(encryptionParams.length())
+    };
+    SECKEYEncryptedPrivateKeyInfo encryptedPrivateKeyInfo = {
+      nullptr,
+      { encryptionAlgorithmItem, encryptionParamsItem },
+      encryptedPrivateKeyInfoItem
+    };
+    SECItem passwordItem = { siBuffer, nullptr, 0 };
+    SECItem publicValueItem = {
+      siBuffer,
+      const_cast<uint8_t*>(subjectPublicKey.data()),
+      static_cast<unsigned int>(subjectPublicKey.length())
+    };
+    SECKEYPrivateKey* privateKey;
+    // This should always be an RSA key (we'll have aborted above if we're not
+    // doing an RSA signature).
+    if (PK11_ImportEncryptedPrivateKeyInfoAndReturnKey(
+          slot.get(), &encryptedPrivateKeyInfo, &passwordItem, nullptr,
+          &publicValueItem, false, false, rsaKey, KU_ALL, &privateKey,
+          nullptr) != SECSuccess) {
+      return MapPRErrorCodeToResult(PR_GetError());
+    }
+    ScopedSECKEYPrivateKey scopedPrivateKey(privateKey);
     SECItem signatureItem;
     if (SEC_SignData(&signatureItem, tbs.data(),
                      static_cast<int>(tbs.length()),
-                     privateKey.get(), oidTag) != SECSuccess) {
+                     scopedPrivateKey.get(), oidTag) != SECSuccess) {
       return MapPRErrorCodeToResult(PR_GetError());
     }
     signature.assign(signatureItem.data, signatureItem.len);
@@ -134,40 +179,64 @@ public:
 
   TestKeyPair* Clone() const override
   {
-    ScopedSECKEYPrivateKey
-      privateKeyCopy(SECKEY_CopyPrivateKey(privateKey.get()));
-    if (!privateKeyCopy) {
-      return nullptr;
-    }
     return new (std::nothrow) NSSTestKeyPair(publicKeyAlg,
                                              subjectPublicKey,
-                                             privateKeyCopy.release());
+                                             encryptedPrivateKey,
+                                             encryptionAlgorithm,
+                                             encryptionParams);
   }
 
 private:
-  ScopedSECKEYPrivateKey privateKey;
+  const ByteString encryptedPrivateKey;
+  const ByteString encryptionAlgorithm;
+  const ByteString encryptionParams;
 };
 
 } // namespace
 
 // This private function is also used by Gecko's PSM test framework
 // (OCSPCommon.cpp).
-//
-// Ownership of privateKey is transfered.
 TestKeyPair* CreateTestKeyPair(const TestPublicKeyAlgorithm publicKeyAlg,
-                               const SECKEYPublicKey& publicKey,
-                               SECKEYPrivateKey* privateKey)
+                               const ScopedSECKEYPublicKey& publicKey,
+                               const ScopedSECKEYPrivateKey& privateKey)
 {
   ScopedPtr<CERTSubjectPublicKeyInfo, SECKEY_DestroySubjectPublicKeyInfo>
-    spki(SECKEY_CreateSubjectPublicKeyInfo(&publicKey));
+    spki(SECKEY_CreateSubjectPublicKeyInfo(publicKey.get()));
   if (!spki) {
     return nullptr;
   }
   SECItem spkDER = spki->subjectPublicKey;
   DER_ConvertBitString(&spkDER); // bits to bytes
-  return new (std::nothrow) NSSTestKeyPair(publicKeyAlg,
-                                           ByteString(spkDER.data, spkDER.len),
-                                           privateKey);
+  ScopedPtr<PK11SlotInfo, PK11_FreeSlot> slot(PK11_GetInternalSlot());
+  if (!slot) {
+    return nullptr;
+  }
+  // Because NSSTestKeyPair isn't tracked by XPCOM and won't otherwise be aware
+  // of shutdown, we don't have a way to release NSS resources at the
+  // appropriate time. To work around this, NSSTestKeyPair doesn't hold on to
+  // NSS resources. Instead, we export the generated private key part as an
+  // encrypted blob (with an empty password and fairly lame encryption). When we
+  // need to use it (e.g. to sign something), we decrypt it and create a
+  // temporary key object.
+  SECItem passwordItem = { siBuffer, nullptr, 0 };
+  ScopedPtr<SECKEYEncryptedPrivateKeyInfo,
+            SECKEY_DestroyEncryptedPrivateKeyInfo_true> encryptedPrivateKey(
+    PK11_ExportEncryptedPrivKeyInfo(
+      slot.get(), SEC_OID_PKCS12_V2_PBE_WITH_SHA1_AND_3KEY_TRIPLE_DES_CBC,
+      &passwordItem, privateKey.get(), 1, nullptr));
+  if (!encryptedPrivateKey) {
+    return nullptr;
+  }
+
+  return new (std::nothrow) NSSTestKeyPair(
+    publicKeyAlg,
+    ByteString(spkDER.data, spkDER.len),
+    ByteString(encryptedPrivateKey->encryptedData.data,
+               encryptedPrivateKey->encryptedData.len),
+    ByteString(encryptedPrivateKey->algorithm.algorithm.data,
+               encryptedPrivateKey->algorithm.algorithm.len),
+    ByteString(encryptedPrivateKey->algorithm.parameters.data,
+               encryptedPrivateKey->algorithm.parameters.len));
 }
 
 namespace {
@@ -194,7 +263,7 @@ GenerateKeyPairInner()
                                       nullptr));
     ScopedSECKEYPublicKey publicKey(publicKeyTemp);
     if (privateKey) {
-      return CreateTestKeyPair(RSA_PKCS1(), *publicKey, privateKey.release());
+      return CreateTestKeyPair(RSA_PKCS1(), publicKey, privateKey);
     }
 
     assert(!publicKeyTemp);
@@ -275,7 +344,7 @@ GenerateDSSKeyPair()
     return nullptr;
   }
   ScopedSECKEYPublicKey publicKey(publicKeyTemp);
-  return CreateTestKeyPair(DSS(), *publicKey, privateKey.release());
+  return CreateTestKeyPair(DSS(), publicKey, privateKey);
 }
 
 Result

security/pkix/test/lib/pkixtestnss.h

@@ -0,0 +1,49 @@
+/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+/* vim: set ts=8 sts=2 et sw=2 tw=80: */
+/* This code is made available to you under your choice of the following sets
+ * of licensing terms:
+ */
+/* 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/.
+ */
+/* Copyright 2018 Mozilla Contributors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+// This file provides some implementation-specific test utilities. This is only
+// necessary because some PSM xpcshell test utilities overlap in functionality
+// with these test utilities, so the underlying implementation is shared.
+
+#ifndef mozilla_pkix_test_pkixtestnss_h
+#define mozilla_pkix_test_pkixtestnss_h
+
+#include "keyhi.h"
+#include "keythi.h"
+#include "pkixtestutil.h"
+
+namespace mozilla { namespace pkix { namespace test {
+
+typedef ScopedPtr<SECKEYPublicKey, SECKEY_DestroyPublicKey>
+  ScopedSECKEYPublicKey;
+typedef ScopedPtr<SECKEYPrivateKey, SECKEY_DestroyPrivateKey>
+  ScopedSECKEYPrivateKey;
+
+TestKeyPair* CreateTestKeyPair(const TestPublicKeyAlgorithm publicKeyAlg,
+                               const ScopedSECKEYPublicKey& publicKey,
+                               const ScopedSECKEYPrivateKey& privateKey);
+
+} } } // namespace mozilla::pkix::test
+
+#endif // mozilla_pkix_test_pkixtestnss_h

security/pkix/test/lib/pkixtestutil.h

@@ -22,8 +22,8 @@
  * limitations under the License.
  */
 
-#ifndef mozilla_pkix_test_pkixtestutils_h
-#define mozilla_pkix_test_pkixtestutils_h
+#ifndef mozilla_pkix_test_pkixtestutil_h
+#define mozilla_pkix_test_pkixtestutil_h
 
 #include <ctime>
 #include <stdint.h> // Some Mozilla-supported compilers lack <cstdint>
@@ -445,4 +445,4 @@ ByteString CreateEncodedOCSPResponse(OCSPResponseContext& context);
 
 } } } // namespace mozilla::pkix::test
 
-#endif // mozilla_pkix_test_pkixtestutils_h
+#endif // mozilla_pkix_test_pkixtestutil_h