[webcrypto] Validate JWK import of AES keys: key length must match algorithm.

content/renderer/webcrypto/webcrypto_impl.cc

 // Copyright (c) 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 <algorithm>
 #include <functional>
 #include <map>
 #include "base/json/json_reader.h"
@@ skipping 20 matching lines @@
 }
 
 bool IsAlgorithmAsymmetric(const blink::WebCryptoAlgorithm& algorithm) {
   // TODO(padolph): include all other asymmetric algorithms once they are
   // defined, e.g. EC and DH.
   return (algorithm.id() == blink::WebCryptoAlgorithmIdRsaEsPkcs1v1_5 ||
           algorithm.id() == blink::WebCryptoAlgorithmIdRsaSsaPkcs1v1_5 ||
           algorithm.id() == blink::WebCryptoAlgorithmIdRsaOaep);
 }
 
-// Binds a specific key length value to a compatible factory function.
-typedef blink::WebCryptoAlgorithm (*AlgFactoryFuncWithOneShortArg)(
-    unsigned short);
-template <AlgFactoryFuncWithOneShortArg func, unsigned short key_length>
-blink::WebCryptoAlgorithm BindAlgFactoryWithKeyLen() {
-  return func(key_length);
-}
+typedef blink::WebCryptoAlgorithm (*AlgorithmCreationFunc)();
 
-// Binds a WebCryptoAlgorithmId value to a compatible factory function.
-typedef blink::WebCryptoAlgorithm (*AlgFactoryFuncWithWebCryptoAlgIdArg)(
-    blink::WebCryptoAlgorithmId);
-template <AlgFactoryFuncWithWebCryptoAlgIdArg func,
-          blink::WebCryptoAlgorithmId algorithm_id>
-blink::WebCryptoAlgorithm BindAlgFactoryAlgorithmId() {
-  return func(algorithm_id);
-}
+class JwkAlgorithmInfo {
+ public:
+  JwkAlgorithmInfo() :

[Ryan Sleevi, 2014/02/04 02:36:35]

style: 
: creation_func_(NULL),
  required_key_length_bytes(...)

[eroman, 2014/02/04 02:46:03]

Done.

+      creation_func_(NULL),
+      required_key_length_bytes_(NO_KEY_SIZE_REQUIREMENT) {
 
-// Defines a map between a JWK 'alg' string ID and a corresponding Web Crypto
-// factory function.
-typedef blink::WebCryptoAlgorithm (*AlgFactoryFuncNoArgs)();
-typedef std::map<std::string, AlgFactoryFuncNoArgs> JwkAlgFactoryMap;
-
-class JwkAlgorithmFactoryMap {
- public:
-  JwkAlgorithmFactoryMap() {
-    map_["HS256"] =
-        &BindAlgFactoryAlgorithmId<webcrypto::CreateHmacAlgorithmByHashId,
-                                   blink::WebCryptoAlgorithmIdSha256>;
-    map_["HS384"] =
-        &BindAlgFactoryAlgorithmId<webcrypto::CreateHmacAlgorithmByHashId,
-                                   blink::WebCryptoAlgorithmIdSha384>;
-    map_["HS512"] =
-        &BindAlgFactoryAlgorithmId<webcrypto::CreateHmacAlgorithmByHashId,
-                                   blink::WebCryptoAlgorithmIdSha512>;
-    map_["RS256"] =
-        &BindAlgFactoryAlgorithmId<webcrypto::CreateRsaSsaAlgorithm,
-                                   blink::WebCryptoAlgorithmIdSha256>;
-    map_["RS384"] =
-        &BindAlgFactoryAlgorithmId<webcrypto::CreateRsaSsaAlgorithm,
-                                   blink::WebCryptoAlgorithmIdSha384>;
-    map_["RS512"] =
-        &BindAlgFactoryAlgorithmId<webcrypto::CreateRsaSsaAlgorithm,
-                                   blink::WebCryptoAlgorithmIdSha512>;
-    map_["RSA1_5"] =
-        &BindAlgFactoryAlgorithmId<webcrypto::CreateAlgorithm,
-                                   blink::WebCryptoAlgorithmIdRsaEsPkcs1v1_5>;
-    map_["RSA-OAEP"] =
-        &BindAlgFactoryAlgorithmId<webcrypto::CreateRsaOaepAlgorithm,
-                                   blink::WebCryptoAlgorithmIdSha1>;
-    // TODO(padolph): The Web Crypto spec does not enumerate AES-KW 128 yet
-    map_["A128KW"] = &blink::WebCryptoAlgorithm::createNull;
-    // TODO(padolph): The Web Crypto spec does not enumerate AES-KW 256 yet
-    map_["A256KW"] = &blink::WebCryptoAlgorithm::createNull;
-    map_["A128GCM"] =
-        &BindAlgFactoryAlgorithmId<webcrypto::CreateAlgorithm,
-                                   blink::WebCryptoAlgorithmIdAesGcm>;
-    map_["A256GCM"] =
-        &BindAlgFactoryAlgorithmId<webcrypto::CreateAlgorithm,
-                                   blink::WebCryptoAlgorithmIdAesGcm>;
-    map_["A128CBC"] =
-        &BindAlgFactoryAlgorithmId<webcrypto::CreateAlgorithm,
-                                   blink::WebCryptoAlgorithmIdAesCbc>;
-    map_["A192CBC"] =
-        &BindAlgFactoryAlgorithmId<webcrypto::CreateAlgorithm,
-                                   blink::WebCryptoAlgorithmIdAesCbc>;
-    map_["A256CBC"] =
-        &BindAlgFactoryAlgorithmId<webcrypto::CreateAlgorithm,
-                                   blink::WebCryptoAlgorithmIdAesCbc>;
   }
 
-  blink::WebCryptoAlgorithm CreateAlgorithmFromName(const std::string& alg_id)
-      const {
-    const JwkAlgFactoryMap::const_iterator pos = map_.find(alg_id);
-    if (pos == map_.end())
-      return blink::WebCryptoAlgorithm::createNull();
-    return pos->second();
+  explicit JwkAlgorithmInfo(AlgorithmCreationFunc algorithm_creation_func)
+      : creation_func_(algorithm_creation_func),
+        required_key_length_bytes_(NO_KEY_SIZE_REQUIREMENT) {
+  }
+
+  JwkAlgorithmInfo(AlgorithmCreationFunc algorithm_creation_func,
+                   unsigned int required_key_length_bits)
+      : creation_func_(algorithm_creation_func),
+        required_key_length_bytes_(required_key_length_bits / 8) {
+    DCHECK((required_key_length_bits % 8) == 0);
+  }
+
+  bool CreateAlgorithm(blink::WebCryptoAlgorithm* algorithm) const {
+    *algorithm = creation_func_();
+    return !algorithm->isNull();
+  }
+
+  bool IsInvalidKeyByteLength(size_t byte_length) const {
+    if (required_key_length_bytes_ == NO_KEY_SIZE_REQUIREMENT)
+      return false;
+    return required_key_length_bytes_  != byte_length;
   }
 
  private:
-  JwkAlgFactoryMap map_;
+  enum {NO_KEY_SIZE_REQUIREMENT = UINT_MAX};

[Ryan Sleevi, 2014/02/04 02:36:35]

style: spaces after { / before }

[eroman, 2014/02/04 02:46:03]

Done.

+
+  AlgorithmCreationFunc creation_func_;
+
+  // The expected key size for the algorithm or NO_KEY_SIZE_REQUIREMENT.
+  unsigned int required_key_length_bytes_;
+
 };
 
-base::LazyInstance<JwkAlgorithmFactoryMap> jwk_alg_factory =
+typedef std::map<std::string, JwkAlgorithmInfo> JwkAlgorithmInfoMap;
+
+class JwkAlgorithmRegistry {
+ public:
+  JwkAlgorithmRegistry() {
+    // TODO(eroman):
+    // http://tools.ietf.org/html/draft-ietf-jose-json-web-algorithms-20
+    // says HMAC with SHA-2 should have a key size at least as large as the
+    // hash output.
+    alg_to_info_["HS256"] = JwkAlgorithmInfo(
+        &BindAlgorithmId<webcrypto::CreateHmacAlgorithmByHashId,
+                        blink::WebCryptoAlgorithmIdSha256>);
+    alg_to_info_["HS384"] = JwkAlgorithmInfo(
+        &BindAlgorithmId<webcrypto::CreateHmacAlgorithmByHashId,
+                        blink::WebCryptoAlgorithmIdSha384>);
+    alg_to_info_["HS512"] = JwkAlgorithmInfo(
+        &BindAlgorithmId<webcrypto::CreateHmacAlgorithmByHashId,
+                         blink::WebCryptoAlgorithmIdSha512>);
+    alg_to_info_["RS256"] = JwkAlgorithmInfo(
+        &BindAlgorithmId<webcrypto::CreateRsaSsaAlgorithm,
+                         blink::WebCryptoAlgorithmIdSha256>);
+    alg_to_info_["RS384"] = JwkAlgorithmInfo(
+        &BindAlgorithmId<webcrypto::CreateRsaSsaAlgorithm,
+                         blink::WebCryptoAlgorithmIdSha384>);
+    alg_to_info_["RS512"] = JwkAlgorithmInfo(
+        &BindAlgorithmId<webcrypto::CreateRsaSsaAlgorithm,
+                         blink::WebCryptoAlgorithmIdSha512>);
+    alg_to_info_["RSA1_5"] = JwkAlgorithmInfo(
+        &BindAlgorithmId<webcrypto::CreateAlgorithm,
+                         blink::WebCryptoAlgorithmIdRsaEsPkcs1v1_5>);
+    alg_to_info_["RSA-OAEP"] = JwkAlgorithmInfo(
+        &BindAlgorithmId<webcrypto::CreateRsaOaepAlgorithm,
+                         blink::WebCryptoAlgorithmIdSha1>);
+    // TODO(padolph): The Web Crypto spec does not enumerate AES-KW 128 yet
+    alg_to_info_["A128KW"] =
+        JwkAlgorithmInfo(&blink::WebCryptoAlgorithm::createNull, 128);
+    // TODO(padolph): The Web Crypto spec does not enumerate AES-KW 256 yet
+    alg_to_info_["A256KW"] =
+        JwkAlgorithmInfo(&blink::WebCryptoAlgorithm::createNull, 256);
+    alg_to_info_["A128GCM"] = JwkAlgorithmInfo(
+        &BindAlgorithmId<webcrypto::CreateAlgorithm,
+                         blink::WebCryptoAlgorithmIdAesGcm>, 128);
+    alg_to_info_["A256GCM"] = JwkAlgorithmInfo(
+        &BindAlgorithmId<webcrypto::CreateAlgorithm,
+                         blink::WebCryptoAlgorithmIdAesGcm>, 256);
+    alg_to_info_["A128CBC"] = JwkAlgorithmInfo(
+        &BindAlgorithmId<webcrypto::CreateAlgorithm,
+                         blink::WebCryptoAlgorithmIdAesCbc>, 128);
+    alg_to_info_["A192CBC"] = JwkAlgorithmInfo(
+        &BindAlgorithmId<webcrypto::CreateAlgorithm,
+                         blink::WebCryptoAlgorithmIdAesCbc>, 192);
+    alg_to_info_["A256CBC"] = JwkAlgorithmInfo(
+        &BindAlgorithmId<webcrypto::CreateAlgorithm,
+                         blink::WebCryptoAlgorithmIdAesCbc>, 256);
+  }
+
+  // Returns NULL if the algorithm name was not registered.
+  const JwkAlgorithmInfo* GetAlgorithmInfo(const std::string& jwk_alg) const {
+    const JwkAlgorithmInfoMap::const_iterator pos = alg_to_info_.find(jwk_alg);
+    if (pos == alg_to_info_.end())
+      return NULL;
+    return &pos->second;
+  }
+
+ private:
+  // Binds a WebCryptoAlgorithmId value to a compatible factory function.
+  typedef blink::WebCryptoAlgorithm (*FuncWithWebCryptoAlgIdArg)(
+      blink::WebCryptoAlgorithmId);
+  template <FuncWithWebCryptoAlgIdArg func,
+            blink::WebCryptoAlgorithmId algorithm_id>
+  static blink::WebCryptoAlgorithm BindAlgorithmId() {
+    return func(algorithm_id);
+  }
+
+  JwkAlgorithmInfoMap alg_to_info_;
+};
+
+base::LazyInstance<JwkAlgorithmRegistry> jwk_alg_registry =
     LAZY_INSTANCE_INITIALIZER;
 
 bool WebCryptoAlgorithmsConsistent(const blink::WebCryptoAlgorithm& alg1,
                                    const blink::WebCryptoAlgorithm& alg2) {
   DCHECK(!alg1.isNull());
   DCHECK(!alg2.isNull());
   if (alg1.id() != alg2.id())
     return false;
   switch (alg1.id()) {
     case blink::WebCryptoAlgorithmIdHmac:
@@ skipping 20 matching lines @@
       break;
   }
   return false;
 }
 
 bool GetDecodedUrl64ValueByKey(
     const base::DictionaryValue& dict,
     const std::string& key,
     std::string* decoded) {
   std::string value_url64;
-  if (!dict.GetString(key, &value_url64) ||
-      !webcrypto::Base64DecodeUrlSafe(value_url64, decoded) ||
-      !decoded->size()) {
-    return false;
-  }
-  return true;
+  return dict.GetString(key, &value_url64) &&
+         webcrypto::Base64DecodeUrlSafe(value_url64, decoded);
 }
 
 }  // namespace
 
 WebCryptoImpl::WebCryptoImpl() {
   Init();
 }
 
 void WebCryptoImpl::encrypt(
     const blink::WebCryptoAlgorithm& algorithm,
@@ skipping 343 matching lines @@
   // 1. JWK alg present but unrecognized: error
   // 2. JWK alg valid AND input algorithm isNull: use JWK value
   // 3. JWK alg valid AND input algorithm specified, but JWK value
   //      inconsistent with input: error
   // 4. JWK alg valid AND input algorithm specified, both consistent: use
   //      input value (because it has potentially more details)
   // 5. JWK alg missing AND input algorithm isNull: error
   // 6. JWK alg missing AND input algorithm specified: use input value
   // TODO(eroman): Should error if "alg" was specified but not a string.
   blink::WebCryptoAlgorithm algorithm = blink::WebCryptoAlgorithm::createNull();
+  const JwkAlgorithmInfo* algorithm_info = NULL;
   std::string jwk_alg_value;
   if (dict_value->GetString("alg", &jwk_alg_value)) {
     // JWK alg present
 
     // TODO(padolph): Validate alg vs kty. For example kty="RSA" implies alg can
     // only be from the RSA family.
 
-    const blink::WebCryptoAlgorithm jwk_algorithm =
-        jwk_alg_factory.Get().CreateAlgorithmFromName(jwk_alg_value);
-    if (jwk_algorithm.isNull()) {
-      // JWK alg unrecognized
+    blink::WebCryptoAlgorithm jwk_algorithm =
+        blink::WebCryptoAlgorithm::createNull();
+    algorithm_info = jwk_alg_registry.Get().GetAlgorithmInfo(jwk_alg_value);
+    if (!algorithm_info || !algorithm_info->CreateAlgorithm(&jwk_algorithm))
       return Status::ErrorJwkUnrecognizedAlgorithm();  // case 1
-    }
+
     // JWK alg valid
     if (algorithm_or_null.isNull()) {
       // input algorithm not specified
       algorithm = jwk_algorithm;  // case 2
     } else {
       // input algorithm specified
       if (!WebCryptoAlgorithmsConsistent(jwk_algorithm, algorithm_or_null))
         return Status::ErrorJwkAlgorithmInconsistent();  // case 3
       algorithm = algorithm_or_null;  // case 4
     }
@@ skipping 28 matching lines @@
     }
   }
 
   // JWK keying material --> ImportKeyInternal()
   if (jwk_kty_value == "oct") {
 
     std::string jwk_k_value;
     if (!GetDecodedUrl64ValueByKey(*dict_value, "k", &jwk_k_value))
       return Status::ErrorJwkDecodeK();
 
-    // TODO(padolph): Some JWK alg ID's embed information about the key length
-    // in the alg ID string. For example "A128" implies the JWK carries 128 bits
-    // of key material, and "HS512" implies the JWK carries _at least_ 512 bits
-    // of key material. For such keys validate the actual key length against the
-    // value in the ID.
+    // Some JWK alg ID's embed information about the key length in the alg ID
+    // string. For example "A128CBC" implies the JWK carries 128 bits
+    // of key material. For such keys validate that enough bytes were provided.
+    // If this validation is not done, then it would be possible to select a
+    // different algorithm by passing a different lengthed key, since that is
+    // how WebCrypto interprets things.
+    if (algorithm_info &&
+        algorithm_info->IsInvalidKeyByteLength(jwk_k_value.size())) {
+      return Status::ErrorJwkIncorrectKeyLength();
+    }
 
     return ImportKeyInternal(blink::WebCryptoKeyFormatRaw,
                              reinterpret_cast<const uint8*>(jwk_k_value.data()),
                              jwk_k_value.size(),
                              algorithm,
                              extractable,
                              usage_mask,
                              key);
   } else if (jwk_kty_value == "RSA") {
 
@@ skipping 27 matching lines @@
         key);
 
   } else {
     return Status::ErrorJwkUnrecognizedKty();
   }
 
   return Status::Success();
 }
 
 }  // namespace content