Add ECPrivateKey for Elliptic Curve keypair generation.

crypto/ec_private_key_nss.cc

+// 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"
+
+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;
+}
+
+// Like PK11_ImportEncryptedPrivateKeyInfo, but hardcoded for EC, and returns

[wtc, 2011/11/03 02:17:50]

These two functions should be put in a file with an NSS
license header under src/crypto/third_party/nss.

[mattm, 2011/11/04 02:39:14]

Done.

+// the SECKEYPrivateKey.
+// TODO(mattm): make patch to add something like this upstream?
+// See https://bugzilla.mozilla.org/show_bug.cgi?id=211546
+SECStatus ImportEncryptedPrivateKeyInfoAndReturnKey(

[wtc, 2011/11/03 02:17:50]

Ideally this function should simply return SECKEYPrivateKey*
rather than using an output parameter.  But I found two
existing PK11_ImportXXXAndReturnKey functions and they both
use an output parameter like this.  So this should be consistent.
Sigh.

+    PK11SlotInfo* slot,
+    SECKEYEncryptedPrivateKeyInfo* epki,
+    SECItem* password,
+    SECItem* nickname,
+    SECItem* public_value,
+    PRBool permanent,
+    PRBool sensitive,
+    SECKEYPrivateKey** private_key,
+    void* wincx) {
+  SECItem* crypto_param = NULL;
+
+  CK_ATTRIBUTE_TYPE usage = CKA_SIGN;
+
+  PK11SymKey* key = PK11_PBEKeyGen(slot,
+                                   &epki->algorithm,
+                                   password,
+                                   PR_FALSE,  // faulty3DES
+                                   wincx);
+  if (key == NULL) {
+    NOTREACHED() << "PK11_PBEKeyGen error: " << PORT_GetError();
+    return SECFailure;
+  }
+
+  CK_MECHANISM_TYPE cryptoMechType = PK11_GetPBECryptoMechanism(

[wtc, 2011/11/03 02:17:50]

Since you are following Google style, name this variable
crypto_mech_type.

[mattm, 2011/11/04 02:39:14]

Done.

+      &epki->algorithm, &crypto_param, password);
+  if (cryptoMechType == CKM_INVALID_MECHANISM) {
+    NOTREACHED() << "PK11_GetPBECryptoMechanism error: " << PORT_GetError();
+    PK11_FreeSymKey(key);
+    return SECFailure;
+  }
+
+  cryptoMechType = PK11_GetPadMechanism(cryptoMechType);
+
+  *private_key = PK11_UnwrapPrivKey(slot, key, cryptoMechType, crypto_param,
+                                    &epki->encryptedData, nickname,
+                                    public_value, permanent, sensitive, CKK_EC,
+                                    &usage, 1, wincx);
+
+  if (crypto_param != NULL)
+    SECITEM_ZfreeItem(crypto_param, PR_TRUE);
+
+  PK11_FreeSymKey(key);
+
+  if (!*private_key) {
+    NOTREACHED() << "PK11_UnwrapPrivKey error: " << PORT_GetError();
+    return SECFailure;
+  }
+
+  return SECSuccess;
+}
+
+// SECKEY_ConvertToPublicKey doesn't support EC.
+// TODO(mattm): post patch for SECKEY_ConvertToPublicKey upstream?
+SECKEYPublicKey *
+ConvertToPublicKey(SECKEYPrivateKey *privk)
+{
+  SECKEYPublicKey *pubk;
+  PLArenaPool *arena;
+  SECStatus rv;
+
+  arena = PORT_NewArena (DER_DEFAULT_CHUNKSIZE);
+  if (arena == NULL) {
+    return NULL;
+  }
+  pubk = (SECKEYPublicKey *)PORT_ArenaZAlloc(arena,
+                                             sizeof (SECKEYPublicKey));
+  if (pubk == NULL) {
+    PORT_FreeArena(arena,PR_FALSE);
+    return NULL;
+  }
+  pubk->keyType = privk->keyType;
+  pubk->pkcs11Slot = NULL;
+  pubk->pkcs11ID = CK_INVALID_HANDLE;
+  pubk->arena = arena;
+
+  // We have to do a little dance of reading into a SECItem then copying to
+  // another SECItem allocated under the pubkey's arena because
+  // PK11_ReadAttribute isn't public and PK11_ReadRawAttribute doesn't take an
+  // arena arg.
+  SECItem item;
+  switch(privk->keyType) {
+    case ecKey:
+      // See nss/lib/pk11wrap/pk11obj.c#966
+      rv = PK11_ReadRawAttribute(PK11_TypePrivKey, privk,
+                                 CKA_NETSCAPE_DB, &item);

[wtc, 2011/11/03 02:17:50]

The CKA_NETSCAPE_DB attribute is only used by NSS's built-in
soft token, so this method won't work in general.

Fortunately, I believe we don't need the ConvertToPublicKey
function.  See my very last comment in this file.

+      if (rv != SECSuccess) {
+        NOTREACHED() << "error: " << PORT_GetError();
+        break;
+      }
+      rv = SECITEM_CopyItem(arena, &pubk->u.ec.publicValue, &item);
+      SECITEM_FreeItem(&item, PR_FALSE);
+      if (rv != SECSuccess) {
+        NOTREACHED() << "error: " << PORT_GetError();
+        break;
+      }
+
+      rv = PK11_ReadRawAttribute(PK11_TypePrivKey, privk,
+                                 CKA_EC_PARAMS, &item);
+      if (rv != SECSuccess) {
+        NOTREACHED() << "error: " << PORT_GetError();
+        break;
+      }
+      rv = SECITEM_CopyItem(arena, &pubk->u.ec.DEREncodedParams, &item);
+      SECITEM_FreeItem(&item, PR_FALSE);
+      if (rv != SECSuccess) {
+        NOTREACHED() << "error: " << PORT_GetError();
+        break;
+      }
+
+      // Seems to be okay to leave pubk->u.ec.size = 0.

[wtc, 2011/11/03 02:17:50]

Based on code inspection, I believe you're right.  I saw no
NSS code that uses pubk->u.ec.size, and I saw NSS code that
sets pubk->u.ec.size to 0:
http://mxr.mozilla.org/security/search?string=ec.size

But I figured out how to set this field properly:
  pubk->u.ec.size = SECKEY_ECParamsToKeySize(&pubk->u.ec.DEREncodedParams);

Can you find out if this sets pubk->u.ec.size to 256?

[mattm, 2011/11/04 02:39:14]

Yes, that did set it to 256.

+      return pubk;
+    default:
+      break;
+  }
+
+  PORT_FreeArena (arena, PR_FALSE);
+  return NULL;
+}
+
+}  // namespace
+
+namespace crypto {
+
+ECPrivateKey::~ECPrivateKey() {
+  if (key_)
+    SECKEY_DestroyPrivateKey(key_);
+  if (public_key_)
+    SECKEY_DestroyPublicKey(public_key_);
+}
+
+// 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
+  NOTIMPLEMENTED();

[wtc, 2011/11/03 02:17:50]

Please add a comment to explain why this is not implemented
(if USE_NSS is not defined, we initialize NSS with no
databases, so we can't create permanent keys).

Same with CreateSensitiveFromPrivateKeyInfo below.

[mattm, 2011/11/04 02:39:14]

Done.

+  return NULL;
+#endif
+}
+
+// static
+ECPrivateKey* ECPrivateKey::CreateFromPrivateKeyInfo(
+    const std::vector<uint8>& input) {
+  return CreateFromPrivateKeyInfoWithParams(input,
+                                            PR_FALSE /* not permanent */,
+                                            PR_FALSE /* not sensitive */);
+}
+
+// static
+ECPrivateKey* ECPrivateKey::CreateSensitiveFromPrivateKeyInfo(
+    const std::vector<uint8>& input) {
+#if defined(USE_NSS)
+  return CreateFromPrivateKeyInfoWithParams(input,
+                                            PR_TRUE /* permanent */,
+                                            PR_TRUE /* sensitive */);
+#else
+  NOTIMPLEMENTED();
+  return NULL;
+#endif
+}
+
+bool ECPrivateKey::ExportPrivateKey(std::vector<uint8>* output) {
+  // 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 = {siBuffer, NULL, 0};
+
+  SECKEYEncryptedPrivateKeyInfo* encrypted = PK11_ExportEncryptedPrivKeyInfo(
+      NULL, // Slot, optional.
+      SEC_OID_PKCS12_V2_PBE_WITH_SHA1_AND_3KEY_TRIPLE_DES_CBC,
+      &password,
+      key_,
+      1, // iterations.
+      NULL); // wincx.
+
+  if (!encrypted) {
+    NOTREACHED() << "PK11_ExportEncryptedPrivKeyInfo error: "
+                 << PORT_GetError();
+    return false;
+  }
+
+  ScopedPLArenaPool arena(PORT_NewArena(DER_DEFAULT_CHUNKSIZE));
+  SECItem der_key = {siBuffer, NULL, 0};
+  if (!SEC_ASN1EncodeItem(
+      arena.get(),
+      &der_key,
+      encrypted,
+      SEC_ASN1_GET(SECKEY_EncryptedPrivateKeyInfoTemplate))) {
+    NOTREACHED() << "SEC_ASN1EncodeItem error: " << PORT_GetError();
+    SECKEY_DestroyEncryptedPrivateKeyInfo(encrypted, PR_TRUE);
+    return false;
+  }
+  SECKEY_DestroyEncryptedPrivateKeyInfo(encrypted, PR_TRUE);

[wtc, 2011/11/03 02:17:50]

Nit: save the return value of SEC_ASN1EncodeItem in a local
variable, and call SECKEY_DestroyEncryptedPrivateKeyInfo
before testing the return value.  This avoid the duplicate
SECKEY_DestroyEncryptedPrivateKeyInfo calls.

[mattm, 2011/11/04 02:39:14]

Done.

+
+
+  output->clear();
+  // Do a really lame and simple encoding of the necessary information into the
+  // output buffer.  Assumes publicValue is 255 bytes or less.
+  // XXX: do we care about cross build compatibility (like loading keys

[wtc, 2011/11/03 02:17:50]

Replace XXX with TODO.

[mattm, 2011/11/04 02:39:14]

Removed the comment, since I hope there shouldn't be any compatibility issues
now.

+  // generated in NSS with an OpenSSL build, or vice versa)?  The key
+  // should be loadable by OpenSSL (it's a standard PKCS #8
+  // EncryptedPrivateKeyInfo), and I don't think OpenSSL needs the publicValue,
+  // but what about the reverse?
+  DCHECK_LT(public_key_->u.ec.publicValue.len, 256U);
+  output->push_back(public_key_->u.ec.publicValue.len);
+  output->insert(output->end(),
+                 public_key_->u.ec.publicValue.data,
+                 public_key_->u.ec.publicValue.data +
+                 public_key_->u.ec.publicValue.len);
+  output->insert(output->end(),
+                 der_key.data,
+                 der_key.data +
+                 der_key.len);
+
+  return true;
+}
+
+bool ECPrivateKey::ExportPublicKey(std::vector<uint8>* output) {
+  ScopedSECItem der_pubkey(SECKEY_EncodeDERSubjectPublicKeyInfo(public_key_));
+  if (!der_pubkey.get()) {
+    NOTREACHED();
+    return false;
+  }
+
+  for (size_t i = 0; i < der_pubkey->len; ++i)
+    output->push_back(der_pubkey->data[i]);

[wtc, 2011/11/03 02:17:50]

Can we use the assign or insert method to avoid byte-by-byte
copying?  Please fix the original code in
rsa_private_key_nss.cc the same way.

[mattm, 2011/11/04 02:39:14]

done (I'll do rsa_private_key_nss.cc and rsa_private_key.cc (it has some too)
separately)

+
+  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) {
+  EnsureNSSInit();

[wtc, 2011/11/03 02:17:50]

This EnsureNSSInit() call is redundant.

[mattm, 2011/11/04 02:39:14]

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();
+    return NULL;
+  }
+
+  SECKEYECParams ec_parameters = {siDEROID, NULL, 0};
+
+  if (!SECITEM_AllocItem(NULL, &ec_parameters, (2 + oid_data->oid.len))) {
+    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);
+  if (!result->key_) {
+    NOTREACHED() << "PK11_GenerateKeyPair error: " << PORT_GetError();
+    return NULL;
+  }
+
+  return result.release();
+}
+
+// static
+ECPrivateKey* ECPrivateKey::CreateFromPrivateKeyInfoWithParams(
+    const std::vector<uint8>& input, bool permanent, bool sensitive) {
+  EnsureNSSInit();
+
+  scoped_ptr<ECPrivateKey> result(new ECPrivateKey);
+
+  ScopedPK11Slot slot(GetPrivateNSSKeySlot());
+  if (!slot.get())
+    return NULL;
+
+  SECItem public_value = {siBuffer};

[wtc, 2011/11/03 02:17:50]

Nit: initialize this to {siBuffer, NULL, 0} for consistency.

[mattm, 2011/11/04 02:39:14]

Done.

+  SECItem der_key = {siBuffer, NULL, 0};
+  SECKEYEncryptedPrivateKeyInfo epki;
+  memset(&epki, 0, sizeof(epki));
+
+  if (input.size() < 2U)
+    return NULL;
+  unsigned char* buf = const_cast<unsigned char*>(&input[0]);

[wtc, 2011/11/03 02:17:50]

Nit: I prefer moving the const_cast to lines 366 and 369 to
make it clear it's necessitated by NSS's SECItem.

[mattm, 2011/11/04 02:39:14]

done-ish (code is a bit different now)

+
+  public_value.len = *buf++;
+  if (public_value.len >= input.size())

[wtc, 2011/11/03 02:17:50]

I think this should be
  if (1 + public_value.len >= input.size())
to account for byte 0 (the length byte).

+    return NULL;
+
+  public_value.data = buf;
+  buf += public_value.len;
+
+  der_key.data = buf;
+  der_key.len = &input.back() - buf + 1;

[wtc, 2011/11/03 02:17:50]

input.size() - 1 - public_value.len is easier to understand.

+
+  ScopedPLArenaPool arena(PORT_NewArena(DER_DEFAULT_CHUNKSIZE));
+
+  SECStatus rv = SEC_ASN1DecodeItem(
+      arena.get(),
+      &epki,
+      SEC_ASN1_GET(SECKEY_EncryptedPrivateKeyInfoTemplate),
+      &der_key);
+  if (rv != SECSuccess) {
+    NOTREACHED() << "SEC_ASN1DecodeItem error: " << PORT_GetError();
+    return NULL;
+  }
+
+  SECItem password = {siBuffer, NULL, 0};
+
+  rv = ImportEncryptedPrivateKeyInfoAndReturnKey(
+      slot.get(),
+      &epki,
+      &password,
+      NULL,  // nickname
+      &public_value,
+      permanent,
+      sensitive,
+      &result->key_,
+      NULL);  // wincx
+  if (rv != SECSuccess) {
+    NOTREACHED() << "ImportEncryptedPrivateKeyInfoAndReturnKey error: "
+                 << PORT_GetError();
+    return NULL;
+  }
+
+  result->public_key_ = ConvertToPublicKey(result->key_);

[wtc, 2011/11/03 02:17:50]

IMPORTANT: this ConvertToPublicKey call is not necessary
because we have the public_value.

Can we use SECKEY_DecodeDERSubjectPublicKeyInfo
and SECKEY_ExtractPublicKey?  This requires changing
ExportPrivateKey to export an encoded SubjectPublicKeyInfo
instead of just public_key_->u.ec.publicValue.

[mattm, 2011/11/04 02:39:14]

Done.

+  if (!result->public_key_) {
+    NOTREACHED() << "SECKEY_ConvertToPublicKey error: " << PORT_GetError();
+    return NULL;
+  }
+
+  return result.release();
+}
+
+}  // namespace crypto