Add ECPrivateKey for Elliptic Curve keypair generation.

crypto/ec_private_key_nss.cc

Index: crypto/ec_private_key_nss.cc
diff --git a/crypto/ec_private_key_nss.cc b/crypto/ec_private_key_nss.cc
new file mode 100644
index 0000000000000000000000000000000000000000..bee263a97ee37280d0b82148ccaa867b71edfb67
--- /dev/null
+++ b/crypto/ec_private_key_nss.cc
@@ -0,0 +1,303 @@
+// Copyright (c) 2011 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.
+
+#include "crypto/ec_private_key.h"
+
+extern "C" {
+// Work around NSS missing SEC_BEGIN_PROTOS in secmodt.h.  This must come before
+// other NSS headers.
+#include <secmodt.h>
+}
+#include <cryptohi.h>
+#include <keyhi.h>
+#include <pk11pub.h>
+#include <secmod.h>
+
+#include "base/logging.h"
+#include "base/memory/scoped_ptr.h"
+#include "crypto/nss_util.h"
+#include "crypto/nss_util_internal.h"
+#include "crypto/scoped_nss_types.h"
+#include "crypto/third_party/nss/ec.h"
+
+namespace {
+
+// Copied from rsa_private_key_nss.cc.
+static bool ReadAttribute(SECKEYPrivateKey* key,
+                          CK_ATTRIBUTE_TYPE type,
+                          std::vector<uint8>* output) {
+  SECItem item;
+  SECStatus rv;
+  rv = PK11_ReadRawAttribute(PK11_TypePrivKey, key, type, &item);
+  if (rv != SECSuccess) {
+    NOTREACHED() << "error: " << PORT_GetError();
+    return false;
+  }
+
+  output->assign(item.data, item.data + item.len);
+  SECITEM_FreeItem(&item, PR_FALSE);
+  return true;
+}
+
+}  // namespace
+
+namespace crypto {
+
+ECPrivateKey::~ECPrivateKey() {
+  if (key_)
+    SECKEY_DestroyPrivateKey(key_);
+  if (public_key_)
+    SECKEY_DestroyPublicKey(public_key_);

[Ryan Sleevi, 2011/11/04 03:21:25]

Don't need this when using the Scoped types

[mattm, 2011/11/08 02:12:27]

Done.

+}
+
+// static
+ECPrivateKey* ECPrivateKey::Create() {
+  return CreateWithParams(PR_FALSE /* not permanent */,
+                          PR_FALSE /* not sensitive */);
+}
+
+// static
+ECPrivateKey* ECPrivateKey::CreateSensitive() {
+#if defined(USE_NSS)
+  return CreateWithParams(PR_TRUE /* permanent */,
+                          PR_TRUE /* sensitive */);
+#else
+  // If USE_NSS is not defined, we initialize NSS with no databases, so we can't
+  // create permanent keys.
+  NOTIMPLEMENTED();

[Ryan Sleevi, 2011/11/04 03:21:25]

If this will never be implemented, should it be a NOTIMPLEMENTED?

I thought those were for "not quite ready" implementations

Same for line 101

[wtc, 2011/11/04 21:52:49]

I think you're right.  NOTREACHED() seems more appropriate.

[mattm, 2011/11/08 02:12:27]

Done.

+  return NULL;
+#endif
+}
+
+// static
+ECPrivateKey* ECPrivateKey::CreateFromEncryptedPrivateKeyInfo(
+    const std::string& password,
+    const std::vector<uint8>& encrypted_private_key_info,
+    const std::vector<uint8>& subject_public_key_info) {
+  return CreateFromEncryptedPrivateKeyInfoWithParams(
+      password,
+      encrypted_private_key_info,
+      subject_public_key_info,
+      PR_FALSE /* not permanent */,
+      PR_FALSE /* not sensitive */);
+}
+
+// static
+ECPrivateKey* ECPrivateKey::CreateSensitiveFromEncryptedPrivateKeyInfo(
+    const std::string& password,
+    const std::vector<uint8>& encrypted_private_key_info,
+    const std::vector<uint8>& subject_public_key_info) {
+#if defined(USE_NSS)
+  return CreateFromEncryptedPrivateKeyInfoWithParams(
+      password,
+      encrypted_private_key_info,
+      subject_public_key_info,
+      PR_TRUE /* permanent */,
+      PR_TRUE /* sensitive */);
+#else
+  // If USE_NSS is not defined, we initialize NSS with no databases, so we can't
+  // create permanent keys.
+  NOTIMPLEMENTED();
+  return NULL;
+#endif
+}
+
+bool ECPrivateKey::ExportEncryptedPrivateKey(
+    const std::string& password,
+    std::vector<uint8>* encrypted_private_key_info,
+    std::vector<uint8>* subject_public_key_info) {
+  // We export as an EncryptedPrivateKeyInfo bundle instead of a plain PKCS #8
+  // PrivateKeyInfo because PK11_ImportDERPrivateKeyInfoAndReturnKey doesn't
+  // support EC keys.
+  // https://bugzilla.mozilla.org/show_bug.cgi?id=327773
+  SECItem password_secitem = {

[wtc, 2011/11/04 22:41:26]

Nit: shorten the variable name suffix to _item.  (This is the
convention used inside NSS.)

[mattm, 2011/11/08 02:12:27]

Done.

+    siBuffer,
+    reinterpret_cast<unsigned char*>(const_cast<char*>(password.data())),
+    password.size()
+  };
+
+  SECKEYEncryptedPrivateKeyInfo* encrypted = PK11_ExportEncryptedPrivKeyInfo(

[Ryan Sleevi, 2011/11/04 03:21:25]

nit? typedef scoped_ptr_malloc<SECKEYEncryptedPrivateKeyInfo,
SECKEY_DestroyEncryptedPrivateKeyInfo> ScopedSECKEYEncryptedPrivateKeyInfo

[wtc, 2011/11/04 21:52:49]

Unless this function becomes longer, using this
scoped_ptr_malloc seems to be a net loss...

+      NULL, // Slot, optional.
+      SEC_OID_PKCS12_V2_PBE_WITH_SHA1_AND_3KEY_TRIPLE_DES_CBC,
+      &password_secitem,
+      key_,
+      1, // iterations.

[Ryan Sleevi, 2011/11/04 03:21:25]

Could you add a comment here as to why iterations is 1 (it's because it's not
really meant to be 'encrypted', it's just a transport thing.

I'm surprised NSS doesn't complain about low iteration counts - does this work
the same in the FIPS mode?

[mattm, 2011/11/08 02:12:27]

Since this has a password arg now I guess it also makes sense to expose the
iterations as an argument too.  Then using the iteration count of 1 can be moved
up to a higher level where it makes more sense.

+      NULL); // wincx.
+
+  if (!encrypted) {
+    NOTREACHED() << "PK11_ExportEncryptedPrivKeyInfo error: "
+                 << PORT_GetError();

[Ryan Sleevi, 2011/11/04 03:21:25]

Is PORT_GetError correct here, or should it be PR_GetError?

I wonder if we shouldn't surface GetNSSErrorMessage from crypto/nss_util.cc to
public API and use that everywhere here. Perhaps TODO for later.

[wtc, 2011/11/04 21:52:49]

PORT_GetError and PR_GetError are the same:
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/security/nss/lib/util/sec...

In code that uses NSS, it looks nicer to call PORT_GetError.

+    return false;
+  }
+
+  ScopedPLArenaPool arena(PORT_NewArena(DER_DEFAULT_CHUNKSIZE));
+  SECItem der_key = {siBuffer, NULL, 0};
+  SECItem* rv = SEC_ASN1EncodeItem(

[wtc, 2011/11/04 22:41:26]

Nit: name this variable 'encoded_item'.

[mattm, 2011/11/08 02:12:27]

Done.

+      arena.get(),
+      &der_key,
+      encrypted,
+      SEC_ASN1_GET(SECKEY_EncryptedPrivateKeyInfoTemplate));
+  SECKEY_DestroyEncryptedPrivateKeyInfo(encrypted, PR_TRUE);
+  if (!rv) {
+    NOTREACHED() << "SEC_ASN1EncodeItem error: " << PORT_GetError();
+    return false;
+  }
+
+  encrypted_private_key_info->assign(der_key.data, der_key.data + der_key.len);
+
+  ExportPublicKey(subject_public_key_info);

[Ryan Sleevi, 2011/11/04 03:21:25]

This method can fail. Perhaps

return ExportPublicKey(subject_public_key_info);

+
+  return true;
+}
+
+bool ECPrivateKey::ExportPublicKey(std::vector<uint8>* output) {
+  ScopedSECItem der_pubkey(SECKEY_EncodeDERSubjectPublicKeyInfo(public_key_));
+  if (!der_pubkey.get()) {
+    NOTREACHED();

[Ryan Sleevi, 2011/11/04 03:21:25]

Does this need to be NOTREACHED? Seems a valid (handled) edge condition, curious
the motivation.

[wtc, 2011/11/04 21:52:49]

This is copied from crypto/rsa_private_key_nss.cc.  I agree
this NOTREACHED() can be removed.

[mattm, 2011/11/08 02:12:27]

Done.

+    return false;
+  }
+
+  output->assign(der_pubkey->data, der_pubkey->data + der_pubkey->len);
+  return true;
+}
+
+bool ECPrivateKey::ExportValue(std::vector<uint8>* output) {
+  return ReadAttribute(key_, CKA_VALUE, output);
+}
+
+bool ECPrivateKey::ExportECParams(std::vector<uint8>* output) {
+  return ReadAttribute(key_, CKA_EC_PARAMS, output);
+}
+
+ECPrivateKey::ECPrivateKey() : key_(NULL), public_key_(NULL) {
+}

[Ryan Sleevi, 2011/11/04 03:21:25]

nit: Brace on same line

[mattm, 2011/11/08 02:12:27]

Done.

+
+// static
+ECPrivateKey* ECPrivateKey::CreateWithParams(bool permanent,
+                                             bool sensitive) {
+  EnsureNSSInit();
+
+  scoped_ptr<ECPrivateKey> result(new ECPrivateKey);
+
+  ScopedPK11Slot slot(GetPrivateNSSKeySlot());
+  if (!slot.get())
+    return NULL;
+
+  SECOidData* oid_data = SECOID_FindOIDByTag(SEC_OID_SECG_EC_SECP256R1);
+  if (!oid_data) {
+    NOTREACHED();

[Ryan Sleevi, 2011/11/04 03:21:25]

Same here for NOTREACHED()

[mattm, 2011/11/08 02:12:27]

Done.

+    return NULL;
+  }
+
+  SECKEYECParams ec_parameters = {siDEROID, NULL, 0};
+
+  if (!SECITEM_AllocItem(NULL, &ec_parameters, (2 + oid_data->oid.len))) {

[Ryan Sleevi, 2011/11/04 03:21:25]

Comment to explain the magic 2 here and on line 203

[wtc, 2011/11/04 21:52:49]

The magic 2 here is indeed not obvious.  The magic 2 on
line 203 is obvious, following the assignments on lines
201-202.

[wtc, 2011/11/04 22:41:26]

Nit: In C++ you should be able to allocate a variable-sized array on the stack:
  unsigned char parameters_buf[2 + oid_data->oid.len];
  SECKEYECParams ec_parameters = {
    siDEROID, parameters_buf, 2 + oid_data->oid.len
  };

Then you can avoid the SECITEM_AllocItem and SECITEM_FreeItem calls.

[mattm, 2011/11/08 02:12:27]

Done.

[mattm, 2011/11/08 02:12:27]

Hm, seems variable length arrays aren't in (standard) C++, but are in C99.  The
style guide doesn't allow them.  However the idea is good, I switched to using a
vector instead.

+    return NULL;
+  }
+
+  // SECKEYECParams is a SECItem containing the DER encoded ASN.1 ECParameters
+  // value.  For a named curve, that is just the OBJECT IDENTIFIER of the curve.
+  ec_parameters.data[0] = SEC_ASN1_OBJECT_ID;
+  ec_parameters.data[1] = oid_data->oid.len;
+  memcpy(ec_parameters.data + 2, oid_data->oid.data, oid_data->oid.len);
+
+  result->key_ = PK11_GenerateKeyPair(slot.get(),
+                                      CKM_EC_KEY_PAIR_GEN,
+                                      &ec_parameters,
+                                      &result->public_key_,
+                                      permanent,
+                                      sensitive,
+                                      NULL);
+  SECITEM_FreeItem(&ec_parameters, PR_FALSE);

[Ryan Sleevi, 2011/11/04 03:21:25]

Perhaps we should have a StackSECItem to go along with the ScopedSECItem, to
pass PR_FALSE as the last param.

Or you could just template<> the existing ScopedSECItem, then

StackSECItem stack_item;
SECItem item;
stack_item.reset(&item);

Perhaps that's no better, just looking for places where we can RAII (the
templates will optimize out, the only thing would be symbol size)

+  if (!result->key_) {
+    NOTREACHED() << "PK11_GenerateKeyPair error: " << PORT_GetError();
+    return NULL;
+  }
+
+  return result.release();
+}
+
+// static
+ECPrivateKey* ECPrivateKey::CreateFromEncryptedPrivateKeyInfoWithParams(
+    const std::string& password,
+    const std::vector<uint8>& encrypted_private_key_info,
+    const std::vector<uint8>& subject_public_key_info,
+    bool permanent,
+    bool sensitive) {
+  EnsureNSSInit();
+
+  scoped_ptr<ECPrivateKey> result(new ECPrivateKey);
+
+  ScopedPK11Slot slot(GetPrivateNSSKeySlot());
+  if (!slot.get())
+    return NULL;
+
+  SECItem subject_public_key_info_secitem = {

[wtc, 2011/11/04 22:41:26]

Nit: use the _item suffix in the variable name.
You can just name this 'spki_item' or 'encoded_spki',
because you already use the "spki" abbreviation below.

Similarly, encrypted_private_key_info_secitem can be
shortened to epki_item or encoded_epki.

[mattm, 2011/11/08 02:12:27]

Done.

+    siBuffer,
+    const_cast<unsigned char*>(&subject_public_key_info[0]),
+    subject_public_key_info.size()
+  };
+  CERTSubjectPublicKeyInfo* decoded_spki = SECKEY_DecodeDERSubjectPublicKeyInfo(
+      &subject_public_key_info_secitem);
+  if (!decoded_spki) {
+    NOTREACHED() << "SECKEY_DecodeDERSubjectPublicKeyInfo error: "
+                 << PORT_GetError();
+    return NULL;
+  }
+
+  result->public_key_ = SECKEY_ExtractPublicKey(decoded_spki);
+
+  SECKEY_DestroySubjectPublicKeyInfo(decoded_spki);
+
+  if (!result->public_key_) {
+    NOTREACHED() << "SECKEY_ExtractPublicKey error: " << PORT_GetError();
+    return NULL;
+  }
+
+  SECItem encrypted_private_key_info_secitem = {
+    siBuffer,
+    const_cast<unsigned char*>(&encrypted_private_key_info[0]),
+    encrypted_private_key_info.size()
+  };
+  SECKEYEncryptedPrivateKeyInfo epki;
+  memset(&epki, 0, sizeof(epki));
+
+  ScopedPLArenaPool arena(PORT_NewArena(DER_DEFAULT_CHUNKSIZE));
+
+  SECStatus rv = SEC_ASN1DecodeItem(
+      arena.get(),
+      &epki,
+      SEC_ASN1_GET(SECKEY_EncryptedPrivateKeyInfoTemplate),
+      &encrypted_private_key_info_secitem);
+  if (rv != SECSuccess) {
+    NOTREACHED() << "SEC_ASN1DecodeItem error: " << PORT_GetError();
+    return NULL;
+  }
+
+  SECItem password_secitem = {

[wtc, 2011/11/04 22:41:26]

Nit: password_secitem => password_item

[mattm, 2011/11/08 02:12:27]

Done.

+    siBuffer,
+    reinterpret_cast<unsigned char*>(const_cast<char*>(password.data())),
+    password.size()
+  };
+
+  rv = ImportEncryptedECPrivateKeyInfoAndReturnKey(
+      slot.get(),
+      &epki,
+      &password_secitem,
+      NULL,  // nickname
+      &result->public_key_->u.ec.publicValue,
+      permanent,
+      sensitive,
+      &result->key_,
+      NULL);  // wincx
+  if (rv != SECSuccess) {
+    DLOG(ERROR) << "ImportEncryptedECPrivateKeyInfoAndReturnKey error: "
+                << PORT_GetError();
+    return NULL;
+  }
+
+  return result.release();
+}
+
+}  // namespace crypto