Don't use RSAPrivateKey in NSS integration code.

net/test/cert_test_util.h

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef NET_TEST_CERT_TEST_UTIL_H_
 #define NET_TEST_CERT_TEST_UTIL_H_
 
 #include <string>
 
 #include "base/memory/ref_counted.h"
 #include "net/cert/x509_cert_types.h"
 #include "net/cert/x509_certificate.h"
 
 #if defined(USE_NSS_CERTS)
-#include "base/memory/scoped_ptr.h"
-
 // From <pk11pub.h>
 typedef struct PK11SlotInfoStr PK11SlotInfo;
 #endif
 
 namespace base {
 class FilePath;
 }
 
 namespace crypto {
 class RSAPrivateKey;
 }
 
 namespace net {
 
 class EVRootCAMetadata;
 
 #if defined(USE_NSS_CERTS)
-// Imports a private key from file |key_filename| in |dir|. The file must
-// contain a PKCS#8 PrivateKeyInfo in DER encoding. The key is imported to
-// |slot|.
-scoped_ptr<crypto::RSAPrivateKey> ImportSensitiveKeyFromFile(
-    const base::FilePath& dir,
-    const std::string& key_filename,
-    PK11SlotInfo* slot);
+// Imports a private key from file |key_filename| in |dir| into |slot|. The file
+// must contain a PKCS#8 PrivateKeyInfo in DER encoding. Returns true on success
+// and false on failure.
+bool ImportSensitiveKeyFromFile(const base::FilePath& dir,

[davidben, 2015/04/27 18:07:22]

Nothing ever uses this output except as a boolean. I could make it return a
crypto::ScopedSECKEYPrivateKey, but then net needs to add NSS to
export_dependent_settings/public_deps which seemed unnecessary here.

[Ryan Sleevi, 2015/04/27 19:11:27]

I don't follow this.

[davidben, 2015/04/27 19:53:44]

The only caller of this function just casts the output to a boolean and cares
about the side effect on |slot|. So bool is sufficient for that caller.

If this were to return a crypto::ScopedSECKEYPrivateKey, that means that a
public header file in //net depends on header files from NSS. That means that
targets which depend on //net must also pull in NSS headers. The way one
expresses this is for NSS to be in net's export_dependent_settings (gyp) or
public_deps (gn).

Given that we have no need for the return value anyway, it seemed better to just
not expose that dependency. (Though, honestly, we're pretty bad at exporting
dependencies when we should. If our buildsystem had better control over our
includes, we'd probably have to declare a lot more stuff.)

+                                const std::string& key_filename,
+                                PK11SlotInfo* slot);
 
 bool ImportClientCertToSlot(const scoped_refptr<X509Certificate>& cert,
                             PK11SlotInfo* slot);
 
 scoped_refptr<X509Certificate> ImportClientCertAndKeyFromFile(
     const base::FilePath& dir,
     const std::string& cert_filename,
     const std::string& key_filename,
     PK11SlotInfo* slot);
 #endif
@@ skipping 33 matching lines @@
   ~ScopedTestEVPolicy();
 
  private:
   SHA1HashValue fingerprint_;
   EVRootCAMetadata* const ev_root_ca_metadata_;
 };
 
 }  // namespace net
 
 #endif  // NET_TEST_CERT_TEST_UTIL_H_