[webcrypto] Add JWK import/export of RSA private keys (NSS).

content/child/webcrypto/platform_crypto_nss.cc

Index: content/child/webcrypto/platform_crypto_nss.cc
diff --git a/content/child/webcrypto/platform_crypto_nss.cc b/content/child/webcrypto/platform_crypto_nss.cc
index eebd9e071b445b39baec6ae805a7f52d6456405e..9bd54e8893bce382e882658de2a99e0c2a684685 100644
--- a/content/child/webcrypto/platform_crypto_nss.cc
+++ b/content/child/webcrypto/platform_crypto_nss.cc
@@ -596,10 +596,6 @@ void CopySECItemToVector(const SECItem& item, std::vector<uint8>* out) {
   out->assign(item.data, item.data + item.len);
 }
 
-// The system NSS library doesn't have the new PK11_ExportDERPrivateKeyInfo
-// function yet (https://bugzilla.mozilla.org/show_bug.cgi?id=519255). So we
-// provide a fallback implementation.
-#if defined(USE_NSS)
 // From PKCS#1 [http://tools.ietf.org/html/rfc3447]:
 //
 //    RSAPrivateKey ::= SEQUENCE {
@@ -629,6 +625,10 @@ struct RSAPrivateKey {
   SECItem coefficient;
 };
 
+// The system NSS library doesn't have the new PK11_ExportDERPrivateKeyInfo
+// function yet (https://bugzilla.mozilla.org/show_bug.cgi?id=519255). So we
+// provide a fallback implementation.
+#if defined(USE_NSS)

[Ryan Sleevi, 2014/05/19 00:10:34]

I would prefer a runtime check, rather than the hardcoding.

[eroman, 2014/05/19 18:51:09]

Can you explain a bit more?

This ifdef is guarding code that is not compiled into the binary when using
PK11_ExportDERPrivateKeyInfo(). Are you proposing always compiling this in
instead?

(I had to loosen this ifdef since I am re-using the RSAPrivateKey struct and
helper functions.)

[Ryan Sleevi, 2014/05/19 18:59:27]

Ah, right, missed that bit. No, this is fine.

 const SEC_ASN1Template RSAPrivateKeyTemplate[] = {
     {SEC_ASN1_SEQUENCE, 0, NULL, sizeof(RSAPrivateKey)},
     {SEC_ASN1_INTEGER, offsetof(RSAPrivateKey, version)},
@@ -641,6 +641,7 @@ const SEC_ASN1Template RSAPrivateKeyTemplate[] = {
     {SEC_ASN1_INTEGER, offsetof(RSAPrivateKey, exponent2)},
     {SEC_ASN1_INTEGER, offsetof(RSAPrivateKey, coefficient)},
     {0}};
+#endif  // defined(USE_NSS)
 
 // On success |value| will be filled with data which must be freed by
 // SECITEM_FreeItem(value, PR_FALSE);
@@ -709,7 +710,6 @@ struct FreeRsaPrivateKey {
     SECITEM_FreeItem(&out->coefficient, PR_FALSE);
   }
 };
-#endif  // defined(USE_NSS)
 
 }  // namespace
 
@@ -968,6 +968,37 @@ Status ExportRsaPublicKey(PublicKey* key,
   return Status::Success();
 }
 
+void AssignVectorFromSecItem(const SECItem& item, std::vector<uint8>* output) {
+  output->assign(item.data, item.data + item.len);
+}
+
+Status ExportRsaPrivateKey(PrivateKey* key,
+                           std::vector<uint8>* modulus,
+                           std::vector<uint8>* public_exponent,
+                           std::vector<uint8>* private_exponent,
+                           std::vector<uint8>* prime1,
+                           std::vector<uint8>* prime2,
+                           std::vector<uint8>* exponent1,
+                           std::vector<uint8>* exponent2,
+                           std::vector<uint8>* coefficient) {
+  RSAPrivateKey key_props = {};
+  scoped_ptr<RSAPrivateKey, FreeRsaPrivateKey> free_private_key(&key_props);
+
+  if (!InitRSAPrivateKey(key->key(), &key_props))
+    return Status::OperationError();
+
+  AssignVectorFromSecItem(key_props.modulus, modulus);
+  AssignVectorFromSecItem(key_props.public_exponent, public_exponent);
+  AssignVectorFromSecItem(key_props.private_exponent, private_exponent);
+  AssignVectorFromSecItem(key_props.prime1, prime1);
+  AssignVectorFromSecItem(key_props.prime2, prime2);
+  AssignVectorFromSecItem(key_props.exponent1, exponent1);
+  AssignVectorFromSecItem(key_props.exponent2, exponent2);
+  AssignVectorFromSecItem(key_props.coefficient, coefficient);
+
+  return Status::Success();
+}
+
 Status ExportKeyPkcs8(PrivateKey* key,
                       const blink::WebCryptoKeyAlgorithm& key_algorithm,
                       std::vector<uint8>* buffer) {
@@ -1494,6 +1525,117 @@ Status ImportRsaPublicKey(const blink::WebCryptoAlgorithm& algorithm,
   return Status::Success();
 }
 
+struct DestroyGenericObject {
+  void operator()(PK11GenericObject* o) const {
+    if (o)
+      PK11_DestroyGenericObject(o);
+  }
+};
+
+typedef scoped_ptr<PK11GenericObject, DestroyGenericObject>
+    ScopedPK11GenericObject;
+
+// Helper to add an attribute to a template.
+void AddAttribute(CK_ATTRIBUTE_TYPE type,
+                  void* value,
+                  unsigned long length,
+                  std::vector<CK_ATTRIBUTE>* templ) {
+  CK_ATTRIBUTE attribute = {type, value, length};
+  templ->push_back(attribute);
+}
+
+// Helper to optionally add an attribute to a template, based on |has_data|.
+void AddOptionalAttribute(CK_ATTRIBUTE_TYPE type,
+                          bool has_data,
+                          const CryptoData& data,
+                          std::vector<CK_ATTRIBUTE>* templ) {
+  if (has_data) {

[Ryan Sleevi, 2014/05/19 00:10:34]

Two thoughts:
1) Short-circuit the error control:
if (!has_data)
  return

2) This doesn't seem to disambiguate when the property is absent, and when the
property is present and (empty or invalid)

Rather than having to track the has_data, I'd say just use if data.byte_length()
!= 0, and remove the bool entirely.

[eroman, 2014/05/19 18:51:09]

I considered this initially, but wasn't sure about the correctness implications.
Certainly if I use the data being empty as a check for optional it simplifies
the ImportRsaPrivateKey() signature since I don't need to pass in all the
boolean values like "has_prime".

Several possibilities, which do you think are correct:

  * Check for specified but empty properties in jwk.cc and fail rather than
passing empty data to ImportRsaPrivateKey(). The failure in this case, should it
be OperationError or DataError?

  * Treat empty input the same as unspecified. There is no evidence of this in
the spec so this is likely wrong.

In terms of whether incorrect data leads to an error or not, it doesn't appear
that NSS validates it (well at least, errors if one is wrong). I presume NSS is
checking the inputs in some order and if it gets stuff working than it proceeds
happily. I was able to do imports with some of the properties being bogus. I
also was unable to do an import with just "n", "e", "d" -- I needed at least one
of "p" and "q" to succeed, is that expected?

[Ryan Sleevi, 2014/05/19 18:58:35]

This is not correct. It's a valid JWK to have these as empty properties, at
least according to the format. (That is, JWK dictates no consistency proofs on
the properties, nor does WebCrypto)
Why make a distinction between the two at all? The spec doesn't say anything
about special handling if they're not present.
See http://mxr.mozilla.org/nss/source/lib/freebl/rsa.c#330

rsa_get_primes_from_exponents, which calculates the various properties, has the
requirement of

e, d, and either p or n, contingent upon hasModulus

So e, n, d "should" be sufficient for the computation of p/q.

This function is called by
http://mxr.mozilla.org/nss/source/lib/freebl/rsa.c#604

Which is documented as requiring either publicExponent or privateExponent

And two more of the following 5 parameters

So publicExponent (e), privateExponent (d), and modulus (n) should be
sufficient.

I suspect we'd need to step through a debugger on that function to find out why
it might be failing to populate the data.

+    CK_ATTRIBUTE attribute = {type, const_cast<unsigned char*>(data.bytes()),
+                              data.byte_length()};
+    templ->push_back(attribute);
+  }
+}
+
+Status ImportRsaPrivateKey(const blink::WebCryptoAlgorithm& algorithm,
+                           bool extractable,
+                           blink::WebCryptoKeyUsageMask usage_mask,
+                           const CryptoData& modulus,
+                           const CryptoData& public_exponent,
+                           const CryptoData& private_exponent,
+                           bool has_prime1,
+                           const CryptoData& prime1,
+                           bool has_prime2,
+                           const CryptoData& prime2,
+                           bool has_exponent1,
+                           const CryptoData& exponent1,
+                           bool has_exponent2,
+                           const CryptoData& exponent2,
+                           bool has_coefficient,
+                           const CryptoData& coefficient,
+                           blink::WebCryptoKey* key) {
+  CK_OBJECT_CLASS obj_class = CKO_PRIVATE_KEY;
+  CK_KEY_TYPE key_type = CKK_RSA;
+  CK_BBOOL ck_false = CK_FALSE;
+
+  std::vector<CK_ATTRIBUTE> key_template;
+
+  AddAttribute(CKA_CLASS, &obj_class, sizeof(obj_class), &key_template);
+  AddAttribute(CKA_KEY_TYPE, &key_type, sizeof(key_type), &key_template);
+  AddAttribute(CKA_TOKEN, &ck_false, sizeof(ck_false), &key_template);
+  AddAttribute(CKA_SENSITIVE, &ck_false, sizeof(ck_false), &key_template);
+  AddAttribute(CKA_PRIVATE, &ck_false, sizeof(ck_false), &key_template);
+
+  // Required properties.
+  AddOptionalAttribute(CKA_MODULUS, true, modulus, &key_template);
+  AddOptionalAttribute(
+      CKA_PUBLIC_EXPONENT, true, public_exponent, &key_template);
+  AddOptionalAttribute(
+      CKA_PRIVATE_EXPONENT, true, private_exponent, &key_template);
+
+  // Optional properties.
+  AddOptionalAttribute(CKA_PRIME_1, has_prime1, prime1, &key_template);
+  AddOptionalAttribute(CKA_PRIME_2, has_prime2, prime2, &key_template);
+  AddOptionalAttribute(CKA_EXPONENT_1, has_exponent1, exponent1, &key_template);
+  AddOptionalAttribute(CKA_EXPONENT_2, has_exponent2, exponent2, &key_template);
+  AddOptionalAttribute(
+      CKA_COEFFICIENT, has_coefficient, coefficient, &key_template);
+
+  crypto::ScopedPK11Slot slot(PK11_GetInternalSlot());
+
+  ScopedPK11GenericObject key_object(PK11_CreateGenericObject(
+      slot.get(), &key_template[0], key_template.size(), PR_FALSE));
+
+  if (!key_object)
+    return Status::OperationError();
+
+  SECItem object_id = {};
+  if (PK11_ReadRawAttribute(
+          PK11_TypeGeneric, key_object.get(), CKA_ID, &object_id) != SECSuccess)

[Ryan Sleevi, 2014/05/19 00:10:34]

Should document that this isn't guaranteed to be set by PKCS#11, but *is*
guaranteed by NSS's softoken -
https://code.google.com/p/chromium/codesearch#chromium-os/src/platform/login_...

[eroman, 2014/05/19 18:51:09]

Done, added comment.

+    return Status::OperationError();
+
+  crypto::ScopedSECKEYPrivateKey private_key(
+      PK11_FindKeyByKeyID(slot.get(), &object_id, NULL));
+  if (!private_key)
+    return Status::OperationError();
+
+  blink::WebCryptoKeyAlgorithm key_algorithm;
+  if (!CreatePrivateKeyAlgorithm(algorithm, private_key.get(), &key_algorithm))
+    return Status::ErrorUnexpected();
+
+  scoped_ptr<PrivateKey> key_handle;
+  Status status =
+      PrivateKey::Create(private_key.Pass(), key_algorithm, &key_handle);
+  if (status.IsError())
+    return status;
+
+  *key = blink::WebCryptoKey::create(key_handle.release(),
+                                     blink::WebCryptoKeyTypePrivate,
+                                     extractable,
+                                     key_algorithm,
+                                     usage_mask);
+  return Status::Success();
+}
+
 Status WrapSymKeyAesKw(SymKey* key,
                        SymKey* wrapping_key,
                        std::vector<uint8>* buffer) {