[webcrypto] Add JWK import for HMAC and AES-CBC key.

content/renderer/webcrypto/webcrypto_impl.cc

Index: content/renderer/webcrypto/webcrypto_impl.cc
diff --git a/content/renderer/webcrypto/webcrypto_impl.cc b/content/renderer/webcrypto/webcrypto_impl.cc
index f2510100eb755c3f7d4b7929c6227a4bb7571a67..002b337941c897e5b35e863a2b2269a71b469d70 100644
--- a/content/renderer/webcrypto/webcrypto_impl.cc
+++ b/content/renderer/webcrypto/webcrypto_impl.cc
@@ -4,35 +4,103 @@
 
 #include "content/renderer/webcrypto/webcrypto_impl.h"
 
+#include <algorithm>
+#include <functional>
+#include <map>
+#include "base/json/json_reader.h"
+#include "base/lazy_instance.h"
 #include "base/logging.h"
 #include "base/memory/scoped_ptr.h"
-#include "third_party/WebKit/public/platform/WebArrayBuffer.h"
+#include "base/strings/string_piece.h"
+#include "base/values.h"
+#include "content/renderer/webcrypto/webcrypto_util.h"
 #include "third_party/WebKit/public/platform/WebCryptoAlgorithm.h"
+#include "third_party/WebKit/public/platform/WebCryptoAlgorithmParams.h"
 #include "third_party/WebKit/public/platform/WebCryptoKey.h"
 
 namespace content {
 
+namespace {
+
+// Binds a specific key length value to a compatible factory function.
+typedef WebKit::WebCryptoAlgorithm (*AlgFactoryFuncWithOneShortArg)(
+    unsigned short key_length);
+template <AlgFactoryFuncWithOneShortArg func, unsigned short key_length>
+WebKit::WebCryptoAlgorithm BindAlgFactoryWithKeyLen() {

[eroman, 2013/10/28 23:02:00]

Ah, clever!

I had been thinking something dumber -- defining a separate function for each
one, which then passes the key length as parameter.

... But I like your way better.

Even though I don't generally like templates, I think this is a good trade-off.

[padolph, 2013/10/29 02:25:40]

The problem was that the create functions don't all have the same signature
(e.g. not all take a key length). Reducing all to a simple no-argument signature
with this type of non-type template binding was the only way to avoid making a
bunch new create functions where some had ugly unused arguments. BTW I agree
with you on template use.

+  return func(key_length);
+}
+
+// Defines a map between a JWK 'alg' string ID and the corresponding Web Crypto
+// factory function. The signature of the factory function in the map must be
+// WebKit::WebCryptoAlgorithm func();

[eroman, 2013/10/28 23:02:00]

[optional] Could reference the function typedef name instead.

[padolph, 2013/10/29 02:25:40]

I just removed that part of the comment since it is obvious from the code.

+typedef WebKit::WebCryptoAlgorithm (*AlgFactoryFuncNoArgs)();
+typedef std::map<std::string, AlgFactoryFuncNoArgs> JwkAlgFactoryMap;
+
+class JwkAlgorithmFactoryMap {
+public:
+  JwkAlgorithmFactoryMap() {
+    map_["HS256"] = &BindAlgFactoryWithKeyLen<CreateHmacAlgorithmByKeyLen, 256>;
+    map_["HS256"] = &BindAlgFactoryWithKeyLen<CreateHmacAlgorithmByKeyLen, 256>;
+    map_["HS384"] = &BindAlgFactoryWithKeyLen<CreateHmacAlgorithmByKeyLen, 384>;
+    map_["HS384"] = &BindAlgFactoryWithKeyLen<CreateHmacAlgorithmByKeyLen, 512>;
+    map_["RS256"] =
+        &BindAlgFactoryWithKeyLen<CreateRsaSsaAlgorithmByKeyLen, 256>;
+    map_["RS384"] =
+        &BindAlgFactoryWithKeyLen<CreateRsaSsaAlgorithmByKeyLen, 384>;
+    map_["RS512"] =
+        &BindAlgFactoryWithKeyLen<CreateRsaSsaAlgorithmByKeyLen, 512>;
+    map_["RSA1_5"] = &CreateRsaEsAlgorithm;
+    map_["RSA-OAEP"] =
+        &BindAlgFactoryWithKeyLen<CreateRsaOaepAlgorithmByKeyLen, 512>;
+    map_["A128KW"] = &WebKit::WebCryptoAlgorithm::createNull;
+    map_["A256KW"] = &WebKit::WebCryptoAlgorithm::createNull;
+    map_["A128GCM"] =
+        &BindAlgFactoryWithKeyLen<CreateAesGcmKeyGenAlgorithm, 256>;
+    map_["A256GCM"] =
+        &BindAlgFactoryWithKeyLen<CreateAesGcmKeyGenAlgorithm, 256>;
+    map_["A128CBC"] =
+        &BindAlgFactoryWithKeyLen<CreateAesCbcKeyGenAlgorithm, 128>;
+    map_["A256CBC"] =
+        &BindAlgFactoryWithKeyLen<CreateAesCbcKeyGenAlgorithm, 256>;
+    map_["A384CBC"] =
+        &BindAlgFactoryWithKeyLen<CreateAesCbcKeyGenAlgorithm, 384>;
+    map_["A512CBC"] =
+        &BindAlgFactoryWithKeyLen<CreateAesCbcKeyGenAlgorithm, 512>;
+  }
+  JwkAlgFactoryMap& Map() { return map_; }

[eroman, 2013/10/28 23:02:00]

Please make this a const-reference. (For mutable parameters chromium style
prefers pointers).

Also for accessors chromium style uses hacker_style(), so you can call this
"map()"

[padolph, 2013/10/29 02:25:40]

Done.

+
+ private:
+  JwkAlgFactoryMap map_;
+};
+base::LazyInstance<JwkAlgorithmFactoryMap> jwk_alg_factory_map =
+    LAZY_INSTANCE_INITIALIZER;
+
+// TODO(padolph) Verify this logic is sufficient to judge algorithm

[eroman, 2013/10/28 23:02:00]

nit: semicolon after closing parenthesis.

[padolph, 2013/10/29 02:25:40]

Done.

+// "consistency" for JWK import, and for all supported algorithms.
+bool WebCryptoAlgorithmsConsistent(const WebKit::WebCryptoAlgorithm& lhs,
+                                   const WebKit::WebCryptoAlgorithm& rhs) {
+  if (lhs.id() == rhs.id()) {
+    if (lhs.id() == WebKit::WebCryptoAlgorithmIdHmac ||
+        lhs.id() == WebKit::WebCryptoAlgorithmIdRsaSsaPkcs1v1_5 ||
+        lhs.id() == WebKit::WebCryptoAlgorithmIdRsaOaep) {
+      if (WebCryptoAlgorithmsConsistent(lhs.hmacParams()->hash(),

[eroman, 2013/10/28 23:02:00]

Unfortunately getting the hash() is not this easy :(

Because all the parameters are strongly typed following the normalization, there
is a different type corresponding to each algorithm.

for instance the parameter type could be any of:
 WebCryptoHmacParams
 WebCryptoHmacKeyParams
 WebCryptoRsaSsaParams
 WebCryptoRsaOaepParams

Written as is, this could return NULL for hmacParams and subsequently crash.

I suggest adding a helper function in the utils such as say:
  WebCryptoAlgorithm GetHashAlgorithm(const WebCryptoAgorithm&)

Which will explore the correct parameter type.

Could you add a regression test?

[padolph, 2013/10/29 02:25:40]

Oops missed that. Thank you.
Done and done.

+                                        rhs.hmacParams()->hash())) {
+        return true;
+      }
+      return false;
+    }
+    return true;
+  }
+  return false;
+}
+
+}  // namespace
+
 WebCryptoImpl::WebCryptoImpl() {
   Init();
 }
 
-// static
-// TODO(eroman): This works by re-allocating a new buffer. It would be better if
-//               the WebArrayBuffer could just be truncated instead.
-void WebCryptoImpl::ShrinkBuffer(
-    WebKit::WebArrayBuffer* buffer,
-    unsigned new_size) {
-  DCHECK_LE(new_size, buffer->byteLength());
-
-  if (new_size == buffer->byteLength())
-    return;
-
-  WebKit::WebArrayBuffer new_buffer =
-      WebKit::WebArrayBuffer::create(new_size, 1);
-  DCHECK(!new_buffer.isNull());
-  memcpy(new_buffer.data(), buffer->data(), new_size);
-  *buffer = new_buffer;
-}
+WebCryptoImpl::~WebCryptoImpl() {}

[eroman, 2013/10/28 23:02:00]

Doesn't seem like this is needed.

[padolph, 2013/10/29 02:25:40]

I think the same, but clang complained loudly; not sure why.

[eroman, 2013/11/04 21:01:15]

What is the error you get without this?

[padolph, 2013/11/05 03:30:54]

I rebuilt again with clang and don't see the error now. Not sure what happened.

 
 void WebCryptoImpl::encrypt(
     const WebKit::WebCryptoAlgorithm& algorithm,
@@ -103,20 +171,30 @@ void WebCryptoImpl::importKey(
   WebKit::WebCryptoKeyType type;
   scoped_ptr<WebKit::WebCryptoKeyHandle> handle;
 
-  if (!ImportKeyInternal(format,
-                         key_data,
-                         key_data_size,
-                         algorithm,
-                         usage_mask,
-                         &handle,
-                         &type)) {
-    result.completeWithError();
-    return;
+  if (format == WebKit::WebCryptoKeyFormatJwk) {
+    if (!ImportKeyJwk(key_data,
+                      key_data_size,
+                      extractable,
+                      algorithm,
+                      usage_mask,
+                      &handle,
+                      &type)) {
+      result.completeWithError();
+    }
+  } else {
+    if (!ImportKeyInternal(format,
+                           key_data,
+                           key_data_size,
+                           algorithm,
+                           usage_mask,
+                           &handle,
+                           &type)) {
+      result.completeWithError();
+    }
   }
 
-  WebKit::WebCryptoKey key(
-      WebKit::WebCryptoKey::create(
-          handle.release(), type, extractable, algorithm, usage_mask));
+  WebKit::WebCryptoKey key(WebKit::WebCryptoKey::create(
+      handle.release(), type, extractable, algorithm, usage_mask));
 
   result.completeWithKey(key);
 }
@@ -157,4 +235,205 @@ void WebCryptoImpl::verifySignature(
   }
 }
 
+bool WebCryptoImpl::ImportKeyJwk(
+    const unsigned char* key_data,
+    unsigned key_data_size,
+    bool extractable,
+    const WebKit::WebCryptoAlgorithm& algorithm,
+    WebKit::WebCryptoKeyUsageMask usage_mask,
+    scoped_ptr<WebKit::WebCryptoKeyHandle>* handle,
+    WebKit::WebCryptoKeyType* type) {
+
+  // JSON Web Key Format (JWK)
+  // http://tools.ietf.org/html/draft-ietf-jose-json-web-key-16
+  // TODO(padolph) Not all possible values are handled by this code right now
+  //
+  // A JWK is a simple JSON dictionary with the following entries
+  // - "kty" (Key Type) Parameter, REQUIRED
+  // - <kty-specific parameters, see below>, REQUIRED
+  // - "use" (Key Use) Parameter, OPTIONAL
+  // - "alg" (Algorithm) Parameter, OPTIONAL
+  // - "extractable" (Key Exportability), OPTIONAL [NOTE: not yet part of JOSE]
+  // (all other entries are ignored)
+  //
+  // Input key_data contains the JWK. To build a Web Crypto Key, the JWK values
+  // are parsed out and used as follows:
+  // Web Crypto Key type            <-- (deduced)
+  // Web Crypto Key extractable     <-- extractable
+  // Web Crypto Key algorithm       <-- alg
+  // Web Crypto Key keyUsage        <-- usage
+  // Web Crypto Key keying material <-- kty-specific parameters
+  //
+  // Values for each entry are case-sensitive and defined in
+  // http://tools.ietf.org/html/draft-ietf-jose-json-web-algorithms-16.
+  // Note that not all values specified by JOSE are handled by this code. Only
+  // handled values are listed.
+  // - kty (Key Type)
+  //   +-------+--------------------------------------------------------------+
+  //   | "RSA" | RSA [RFC3447]                                                |
+  //   | "oct" | Octet sequence (used to represent symmetric keys)            |
+  //   +-------+--------------------------------------------------------------+
+  // - use (Key Use)
+  //   +-------+--------------------------------------------------------------+
+  //   | "enc" | encrypt and decrypt operations                               |
+  //   | "sig" | sign and verify (MAC) operations                             |
+  //   | "wrap"| key wrap and unwrap [not yet part of JOSE]                   |
+  //   +-------+--------------------------------------------------------------+
+  // - extractable (Key Exportability)
+  //   +-------+--------------------------------------------------------------+
+  //   | true  | Key may be exported from the trusted environment             |
+  //   | false | Key cannot exit the trusted environment                      |
+  //   +-------+--------------------------------------------------------------+
+  // - alg (Algorithm)
+  //   See http://tools.ietf.org/html/draft-ietf-jose-json-web-algorithms-16
+  //   +--------------+-------------------------------------------------------+
+  //   | Digital Signature or MAC Algorithm                                   |
+  //   +--------------+-------------------------------------------------------+
+  //   | "HS256"      | HMAC using SHA-256 hash algorithm                     |
+  //   | "HS384"      | HMAC using SHA-384 hash algorithm                     |
+  //   | "HS512"      | HMAC using SHA-512 hash algorithm                     |
+  //   | "RS256"      | RSASSA using SHA-256 hash algorithm                   |
+  //   | "RS384"      | RSASSA using SHA-384 hash algorithm                   |
+  //   | "RS512"      | RSASSA using SHA-512 hash algorithm                   |
+  //   +--------------+-------------------------------------------------------|
+  //   | Key Management Algorithm                                             |
+  //   +--------------+-------------------------------------------------------+
+  //   | "RSA1_5"     | RSAES-PKCS1-V1_5 [RFC3447]                            |
+  //   | "RSA-OAEP"   | RSAES using Optimal Asymmetric Encryption Padding     |
+  //   |              | (OAEP) [RFC3447], with the default parameters         |
+  //   |              | specified by RFC3447 in Section A.2.1                 |
+  //   | "A128KW"     | Advanced Encryption Standard (AES) Key Wrap Algorithm |
+  //   |              | [RFC3394] using 128 bit keys                          |
+  //   | "A256KW"     | AES Key Wrap Algorithm using 256 bit keys             |
+  //   | "A128GCM"    | AES in Galois/Counter Mode (GCM) [NIST.800-38D] using |
+  //   |              | 128 bit keys                                          |
+  //   | "A256GCM"    | AES GCM using 256 bit keys                            |
+  //   | "A128CBC"    | AES in Cipher Block Chaining Mode (CBC) with PKCS #5  |
+  //   |              | padding [NIST.800-38A] [not yet part of JOSE]         |
+  //   | "A256CBC"    | AES CBC using 256 bit keys [not yet part of JOSE]     |
+  //   | "A384CBC"    | AES CBC using 384 bit keys [not yet part of JOSE]     |
+  //   | "A512CBC"    | AES CBC using 512 bit keys [not yet part of JOSE]     |
+  //   +--------------+-------------------------------------------------------+
+  //
+  // kty-specific parameters
+  // The value of kty determines the type and content of the keying material
+  // carried in the JWK to be imported. Currently only two possibilities are
+  // supported: a raw key or an RSA public key. RSA private keys are not
+  // supported because typical applications seldom need to import a private key,
+  // and the large number of JWK parameters required to describe one.
+  // - kty == "oct" (symmetric or other raw key)
+  //   +-------+--------------------------------------------------------------+
+  //   | "k"   | Contains the value of the symmetric (or other single-valued) |
+  //   |       | key.  It is represented as the base64url encoding of the     |
+  //   |       | octet sequence containing the key value.                     |
+  //   +-------+--------------------------------------------------------------+
+  // - kty == "RSA" (RSA public key)
+  //   +-------+--------------------------------------------------------------+
+  //   | "n"   | Contains the modulus value for the RSA public key.  It is    |
+  //   |       | represented as the base64url encoding of the value's         |
+  //   |       | unsigned big endian representation as an octet sequence.     |
+  //   +-------+--------------------------------------------------------------+
+  //   | "e"   | Contains the exponent value for the RSA public key.  It is   |
+  //   |       | represented as the base64url encoding of the value's         |
+  //   |       | unsigned big endian representation as an octet sequence.     |
+  //   +-------+--------------------------------------------------------------+
+  //
+  // Conflict resolution
+  // The type, algorithm, extractable, and usage_mask input parameters may be
+  // different from similar values inside the JWK. The Web Crypto spec says that
+  // if a JWK value is present, but is inconsistent with the input value, it is
+  // an error and the operation must fail.
+
+  // Parse the incoming JWK JSON.
+  base::StringPiece json_string(reinterpret_cast<const char*>(key_data),
+                                key_data_size);
+  scoped_ptr<base::Value> value(base::JSONReader::Read(json_string));
+  // Note, bare pointer dict_value is ok since it points into scoped value.
+  base::DictionaryValue* dict_value = NULL;
+  if (!value.get() || !value->GetAsDictionary(&dict_value) || !dict_value)
+    return false;
+
+  // JWK "kty". Exit early if this required JWK parameter is missing.
+  std::string jwk_kty_value;
+  if (!dict_value->GetString("kty", &jwk_kty_value))
+    return false;
+
+  // JWK "extractable" --> extractable parameter
+  bool jwk_extractable_value;
+  if (dict_value->GetBoolean("extractable", &jwk_extractable_value)) {
+    if (jwk_extractable_value != extractable)
+      return false;
+  }
+
+  // JWK "alg" --> algorithm parameter
+  std::string jwk_alg_value;
+  if (dict_value->GetString("alg", &jwk_alg_value)) {
+    const JwkAlgFactoryMap::const_iterator pos =
+        jwk_alg_factory_map.Get().Map().find(jwk_alg_value);

[eroman, 2013/10/28 23:02:00]

How about moving this logic into the singleton class itself, instead of exposing
the std::map directly:

WebCryptoAlgorithm webcrypto_algorithm_from_jwk =
jwk_alg_factory_.Get().CreateAlgorithmFromName(jwk_alg_value);

if (algorithm_from_jkw.isNull())
  return false;


(I also shortened the name of the singleton in the above example).

[padolph, 2013/10/29 02:25:40]

Good idea, thank you. Done.

+    if (pos == jwk_alg_factory_map.Get().Map().end())
+      return false;
+    WebKit::WebCryptoAlgorithm webcrypto_algorithm_from_jwk = pos->second();
+    if (webcrypto_algorithm_from_jwk.isNull()) {
+      return false;
+    }
+    if (!WebCryptoAlgorithmsConsistent(webcrypto_algorithm_from_jwk, algorithm))

[eroman, 2013/10/28 23:02:00]

Now that we have made WebCryptoAlgorithm nullable, it will be possible for
"algorithm.isNull()".

So it is necessary to either check for that somewhere in ImportKeyJwk(), or for
WebCryptoAlgorithmsConsistent() to handle it.

[padolph, 2013/10/29 02:25:40]

Done. Added input parameter sanity checks and corresponding unit tests.

+      return false;
+  }
+
+  // JWK "use" --> usage_mask parameter
+  std::string jwk_use_value;
+  if (dict_value->GetString("use", &jwk_use_value)) {
+    unsigned jwk_usage_mask;
+    if (jwk_use_value == "enc") {
+      jwk_usage_mask =
+          WebKit::WebCryptoKeyUsageEncrypt | WebKit::WebCryptoKeyUsageDecrypt;
+    } else if (jwk_use_value == "sig") {
+      jwk_usage_mask =
+          WebKit::WebCryptoKeyUsageSign | WebKit::WebCryptoKeyUsageVerify;
+    } else if (jwk_use_value == "wrap") {
+      jwk_usage_mask =
+          WebKit::WebCryptoKeyUsageWrapKey | WebKit::WebCryptoKeyUsageUnwrapKey;
+    } else {
+      return false;
+    }
+    if ((jwk_usage_mask & usage_mask) != usage_mask) {
+      // usage_mask must be a subset of jwk_usage_mask.

[eroman, 2013/10/28 23:02:00]

Please capitalize comments.

[padolph, 2013/10/29 02:25:40]

Done.

+      return false;
+    }
+  }
+
+  // JWK keying material --> ImportKeyInternal()
+  if (jwk_kty_value == "oct") {
+    if (*type != WebKit::WebCryptoKeyTypeSecret)
+      return false;
+    std::string jwk_k_value_url64;
+    if (!dict_value->GetString("k", &jwk_k_value_url64))
+      return false;
+    std::string jwk_k_value;
+    if (!Base64DecodeUrlSafe(jwk_k_value_url64, &jwk_k_value) ||
+        !jwk_k_value.size()) {
+      return false;
+    }
+    if (!ImportKeyInternal(WebKit::WebCryptoKeyFormatRaw,
+                           reinterpret_cast<const uint8*>(jwk_k_value.data()),
+                           jwk_k_value.size(),
+                           algorithm,
+                           usage_mask,
+                           handle,
+                           type)) {
+      return false;
+    }
+  } else if (jwk_kty_value == "RSA") {
+    // Only an RSA public key is currently handled.
+    if (*type != WebKit::WebCryptoKeyTypePublic)
+      return false;
+    // TODO(padolph): JWK import RSA public key
+    return false;
+  } else {
+    return false;
+  }
+
+  return true;
+}
+
 }  // namespace content