[webcrypto] Add JWK import of RSA public key for NSS.

content/renderer/webcrypto/webcrypto_impl_nss.cc

 // Copyright 2013 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 "content/renderer/webcrypto/webcrypto_impl.h"
 
 #include <cryptohi.h>
 #include <pk11pub.h>
 #include <sechash.h>
 
@@ skipping 221 matching lines @@
     size_t reverse_i = data_size - i - 1;
 
     if (reverse_i >= sizeof(unsigned long) && data[i])
       return false;  // Too large for a long.
 
     *result |= data[i] << 8 * reverse_i;
   }
   return true;
 }
 
+typedef scoped_ptr_malloc<

[Ryan Sleevi, 2013/11/21 01:41:34]

scoped_ptr_malloc is DEPRECATED, as scoped_ptr supports the case of passing
custom deleters now.

See
https://code.google.com/p/chromium/codesearch#chromium/src/base/memory/scoped...

[padolph, 2013/11/21 03:38:13]

This typedef is no longer needed because of the change below.

+    PRArenaPool, crypto::NSSDestroyer1<PRArenaPool,
+                                       PORT_FreeArena,
+                                       PR_FALSE> > ScopedPRArenaPool;
+
 }  // namespace
 
 void WebCryptoImpl::Init() {
   crypto::EnsureNSSInit();
 }
 
 bool WebCryptoImpl::EncryptInternal(
     const blink::WebCryptoAlgorithm& algorithm,
     const blink::WebCryptoKey& key,
     const unsigned char* data,
@@ skipping 390 matching lines @@
 
       break;
     }
     default:
       return false;
   }
 
   return true;
 }
 
+bool WebCryptoImpl::ImportRsaPublicKeyInternal(

[eroman, 2013/11/20 23:37:12]

@rsleevi, dark lord of NSS: Can you review this function?

+    const unsigned char* modulus_data,
+    unsigned modulus_size,
+    const unsigned char* exponent_data,
+    unsigned exponent_size,
+    const blink::WebCryptoAlgorithm& algorithm,
+    bool extractable,
+    blink::WebCryptoKeyUsageMask usage_mask,
+    blink::WebCryptoKey* key) {
+
+  DCHECK(modulus_data);
+  DCHECK(modulus_size);
+  DCHECK(exponent_data);
+  DCHECK(exponent_size);

[Ryan Sleevi, 2013/11/21 01:41:34]

Is Blink going to require these four fields be valid? I just want to confirm
they're fine as a DCHECK (eg: Blink guarantees it), and not something we should
be doing as actual checks.

[eroman, 2013/11/21 01:55:41]

The caller here is the JWK parsing code, which validates that none of these are
empty. That said it could be more future proof to fail on empty data instead.

[padolph, 2013/11/21 03:38:13]

Done.

+
+  // NSS does not provide a way to create an RSA public key directly from the
+  // modulus and exponent values. But it can import an DER-encoded ASN.1 blob

[Ryan Sleevi, 2013/11/21 01:41:34]

"values. But" -> "values, but"

[padolph, 2013/11/21 03:38:13]

Done.

+  // with the values and create the key from that. The code below follows the
+  // recommendation described in
+  // https://developer.mozilla.org/en-US/docs/NSS/NSS_Tech_Notes/nss_tech_note7
+
+  ScopedPRArenaPool arena(PORT_NewArena(DER_DEFAULT_CHUNKSIZE));
+  if (!arena.get())
+    return false;
+
+  // Pack the input values into struct compatible with NSS ASN.1 encoding, and
+  // set up the ASN.1 encoder template for it.
+  struct RsaPublicKeyData {
+    SECItem modulus;
+    SECItem exponent;
+  };
+  const RsaPublicKeyData pubkey_in = {
+      {siUnsignedInteger, const_cast<unsigned char*>(modulus_data),
+       modulus_size},
+      {siUnsignedInteger, const_cast<unsigned char*>(exponent_data),
+       exponent_size}};
+  const SEC_ASN1Template RsaPublicKeyTemplate[] = {
+      {SEC_ASN1_SEQUENCE, 0, NULL, sizeof(RsaPublicKeyData)},
+      {SEC_ASN1_INTEGER, offsetof(RsaPublicKeyData, modulus), },
+      {SEC_ASN1_INTEGER, offsetof(RsaPublicKeyData, exponent), },
+      {0, }};

[Ryan Sleevi, 2013/11/21 01:41:34]

nit: I seem to recall that clang-format/the style guide expects spaces following
the leading {

[padolph, 2013/11/21 03:38:13]

The current format is what clang-format -style=Chromium produced. I could not
find info either way in the Chromium style guide. Should I change this?

+
+  // Do the ASN.1 encoding to produce the DER-formatted public key. In this
+  // usage, SEC_ASN1EncodeItem() returns a pointer to the provided output
+  // SECItem (pubkey_der) on success, or NULL on failure.

[Ryan Sleevi, 2013/11/21 01:41:34]

A subtle element of this is the fact that the SECItem is allocated from the
PRArenaPool.

// DER-encode the public key. The returned SECItem* is allocated from
// the specified arena, and thus does not need to be explicitly freed.


I'm not a fan of documenting the function being called (eg: "on success, ... or
NULL of failure". Also, the "Do the ASN.1 encoding" is a bit redundant with the
function call; comments that start with "Do" can usually be reworded like I
tried.

That said, I don't understand why you manually create the arena. The
SEC_ASN1EncodeItem API permits you to supply NULL for the pool, and use the
'easier' APIs like ScopedSECItem - see
http://mxr.mozilla.org/nss/source/lib/util/secasn1.h#118

[padolph, 2013/11/21 03:38:13]

Done.
 
Copy/paste from the NSS tech note. Using the NULL calling method does make
things simpler. Thank you.

+  SECItem pubkey_der = {siBuffer, NULL, 0};
+  SECItem* const check_der_ptr = SEC_ASN1EncodeItem(
+      arena.get(), &pubkey_der, &pubkey_in, RsaPublicKeyTemplate);
+  if (!check_der_ptr)
+    return false;
+  DCHECK_EQ(&pubkey_der, check_der_ptr);
+
+  // Import the DER to create an RSA SECKEYPublicKey.

[Ryan Sleevi, 2013/11/21 01:41:34]

nit: // Import the DER-encoded public key to...

[padolph, 2013/11/21 03:38:13]

Done.

+  crypto::ScopedSECKEYPublicKey pubkey(
+      SECKEY_ImportDERPublicKey(&pubkey_der, CKK_RSA));
+  if (!pubkey.get())
+    return false;
+
+  *key = blink::WebCryptoKey::create(new PublicKeyHandle(pubkey.Pass()),
+                                     blink::WebCryptoKeyTypePublic,
+                                     extractable,
+                                     algorithm,
+                                     usage_mask);
+  return true;
+}
+
 }  // namespace content