NSS: {EC,RSA}PrivateKey shouldn't call crypto::GetPublicNSSKeySlot or GetPrivateNSSKeySlot.

crypto/rsa_private_key.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 CRYPTO_RSA_PRIVATE_KEY_H_
 #define CRYPTO_RSA_PRIVATE_KEY_H_
 
 #include "build/build_config.h"
 
 #include <list>
 #include <vector>
 
 #include "base/basictypes.h"
 #include "crypto/crypto_export.h"
 
 #if defined(USE_NSS)
 #include "base/gtest_prod_util.h"
 #endif
 
+#if !defined(USE_OPENSSL)
+#include "crypto/scoped_nss_types.h"
+#endif
+
 #if defined(USE_OPENSSL)
 // Forward declaration for openssl/*.h
 typedef struct evp_pkey_st EVP_PKEY;
 #elif defined(USE_NSS) || defined(OS_WIN) || defined(OS_MACOSX)

[wtc, 2013/11/11 20:56:25]

Optional: This can become #else. Then you can move the
    #include "crypto/scoped_nss_types.h"
into this block.

Note: there are two other instances of this in this file. I pointed them out
below.

[mattm, 2013/11/12 02:42:44]

Done.

 // Forward declaration.
 typedef struct SECKEYPrivateKeyStr SECKEYPrivateKey;
 typedef struct SECKEYPublicKeyStr SECKEYPublicKey;
 #endif
 
 
 namespace crypto {
 
 // Used internally by RSAPrivateKey for serializing and deserializing
 // PKCS #8 PrivateKeyInfo and PublicKeyInfo.
@@ skipping 140 matching lines @@
   static RSAPrivateKey* Create(uint16 num_bits);
 
   // Create a new instance by importing an existing private key. The format is
   // an ASN.1-encoded PrivateKeyInfo block from PKCS #8. This can return NULL if
   // initialization fails.
   static RSAPrivateKey* CreateFromPrivateKeyInfo(
       const std::vector<uint8>& input);
 
 #if defined(USE_NSS)
   // Create a new random instance. Can return NULL if initialization fails.
   // The created key is permanent and is not exportable in plaintext form.

[wtc, 2013/11/11 20:56:25]

We should mention the new |slot| argument. I guess only the methods that create
permanent keys need a |slot| argument.

[mattm, 2013/11/12 02:42:44]

Done.

-  static RSAPrivateKey* CreateSensitive(uint16 num_bits);
+  static RSAPrivateKey* CreateSensitive(ScopedPK11Slot slot, uint16 num_bits);

[wtc, 2013/11/11 20:56:25]

Is it necessary to pass ScopedPK11Slot rather than a plain PK11Slot* pointer?

What does it imply to pass ScopedPK11Slot? That the function is given a
reference to the PK11Slot?

[mattm, 2013/11/12 02:42:44]

Yeah, however it isn't really necessary for this case.  The "pass a bare pointer
and the function can take a ref if it wants one" convention is pretty clear I
think. Changed it so the callers just use a ScopedPK11Slot locally to hold the
reference while they call this.

 
   // Create a new instance by importing an existing private key. The format is
   // an ASN.1-encoded PrivateKeyInfo block from PKCS #8. This can return NULL if
   // initialization fails.
   // The created key is permanent and is not exportable in plaintext form.
   static RSAPrivateKey* CreateSensitiveFromPrivateKeyInfo(
+      ScopedPK11Slot slot,
       const std::vector<uint8>& input);
 
   // Create a new instance by referencing an existing private key
   // structure.  Does not import the key.
   static RSAPrivateKey* CreateFromKey(SECKEYPrivateKey* key);
 
   // Import an existing public key, and then search for the private
   // half in the key database. The format of the public key blob is is
   // an X509 SubjectPublicKeyInfo block. This can return NULL if
   // initialization fails or the private key cannot be found.  The
   // caller takes ownership of the returned object, but nothing new is
   // created in the key database.
   static RSAPrivateKey* FindFromPublicKeyInfo(
       const std::vector<uint8>& input);
 #endif
 
 #if defined(USE_OPENSSL)
   EVP_PKEY* key() { return key_; }
 #elif defined(USE_NSS) || defined(OS_WIN) || defined(OS_MACOSX)

[wtc, 2013/11/11 20:56:25]

This can become #else.

[mattm, 2013/11/12 02:42:44]

Done.

   SECKEYPrivateKey* key() { return key_; }
   SECKEYPublicKey* public_key() { return public_key_; }
 #endif
 
   // Creates a copy of the object.
   RSAPrivateKey* Copy() const;
 
   // Exports the private key to a PKCS #1 PrivateKey block.
   bool ExportPrivateKey(std::vector<uint8>* output) const;
 
   // Exports the public key to an X509 SubjectPublicKeyInfo block.
   bool ExportPublicKey(std::vector<uint8>* output) const;
 
  private:
 #if defined(USE_NSS)
   FRIEND_TEST_ALL_PREFIXES(RSAPrivateKeyNSSTest, FindFromPublicKey);
   FRIEND_TEST_ALL_PREFIXES(RSAPrivateKeyNSSTest, FailedFindFromPublicKey);
 #endif
 
   // Constructor is private. Use one of the Create*() or Find*()
   // methods above instead.
   RSAPrivateKey();
 
+#if !defined(USE_OPENSSL)
   // Shared helper for Create() and CreateSensitive().
   // TODO(cmasone): consider replacing |permanent| and |sensitive| with a
   //                flags arg created by ORing together some enumerated values.
   // Note: |permanent| is only supported when USE_NSS is defined.
-  static RSAPrivateKey* CreateWithParams(uint16 num_bits,
+  static RSAPrivateKey* CreateWithParams(ScopedPK11Slot slot,
+                                         uint16 num_bits,
                                          bool permanent,
                                          bool sensitive);
 
   // Shared helper for CreateFromPrivateKeyInfo() and
   // CreateSensitiveFromPrivateKeyInfo().
   // Note: |permanent| is only supported when USE_NSS is defined.
   static RSAPrivateKey* CreateFromPrivateKeyInfoWithParams(
+      ScopedPK11Slot slot,
       const std::vector<uint8>& input,
       bool permanent,
       bool sensitive);
+#endif
 
 #if defined(USE_OPENSSL)
   EVP_PKEY* key_;
 #elif defined(USE_NSS) || defined(OS_WIN) || defined(OS_MACOSX)

[wtc, 2013/11/11 20:56:25]

This can become #else.

[mattm, 2013/11/12 02:42:44]

Done.

   SECKEYPrivateKey* key_;
   SECKEYPublicKey* public_key_;
 #endif
 
   DISALLOW_COPY_AND_ASSIGN(RSAPrivateKey);
 };
 
 }  // namespace crypto
 
 #endif  // CRYPTO_RSA_PRIVATE_KEY_H_