[webcrypto] JWK: Updated import(ext, key_ops) and added export of symmetric keys

content/renderer/webcrypto/webcrypto_util.cc

Index: content/renderer/webcrypto/webcrypto_util.cc
diff --git a/content/renderer/webcrypto/webcrypto_util.cc b/content/renderer/webcrypto/webcrypto_util.cc
index 3b565cfab478d97f32cbe1b9b6169187e3b21ce9..f06ad0d7c5a0504343cbdfd000215103cec8bbac 100644
--- a/content/renderer/webcrypto/webcrypto_util.cc
+++ b/content/renderer/webcrypto/webcrypto_util.cc
@@ -6,6 +6,7 @@
 
 #include "base/base64.h"
 #include "base/logging.h"
+#include "base/strings/stringprintf.h"
 #include "third_party/WebKit/public/platform/WebCryptoAlgorithm.h"
 #include "third_party/WebKit/public/platform/WebCryptoAlgorithmParams.h"
 #include "third_party/WebKit/public/platform/WebCryptoKeyAlgorithm.h"
@@ -47,10 +48,10 @@ Status Status::ErrorJwkBase64Decode(const std::string& property) {
                 "\" could not be base64 decoded");
 }
 
-Status Status::ErrorJwkExtractableInconsistent() {
+Status Status::ErrorJwkExtInconsistent() {
   return Status(
-      "The \"extractable\" property of the JWK dictionary is "
-      "inconsistent what that specified by the Web Crypto call");
+      "The \"ext\" property of the JWK dictionary is inconsistent what that "
+      "specified by the Web Crypto call");
 }
 
 Status Status::ErrorJwkUnrecognizedAlgorithm() {
@@ -69,17 +70,34 @@ Status Status::ErrorJwkAlgorithmMissing() {
       "and one wasn't specified by the Web Crypto call");
 }
 
-Status Status::ErrorJwkUnrecognizedUsage() {
+Status Status::ErrorJwkUnrecognizedUse() {
   return Status("The JWK \"use\" property could not be parsed");
 }
 
-Status Status::ErrorJwkUsageInconsistent() {
+Status Status::ErrorJwkUnrecognizedKeyop() {
+  return Status("The JWK \"key_ops\" property could not be parsed");
+}
+
+Status Status::ErrorJwkUseInconsistent() {
   return Status(
       "The JWK \"use\" property was inconsistent with that specified "
       "by the Web Crypto call. The JWK usage must be a superset of "
       "those requested");
 }
 
+Status Status::ErrorJwkKeyopsInconsistent() {
+  return Status(
+      "The JWK \"key_ops\" property was inconsistent with that "
+      "specified by the Web Crypto call. The JWK usage must be a "
+      "superset of those requested");
+}
+
+Status Status::ErrorJwkUseAndKeyopsInconsistent() {
+  return Status(
+      "The JWK \"use\" and \"key_ops\" properties were both found "
+      "but are inconsistent with each other.");
+}
+
 Status Status::ErrorJwkRsaPrivateKeyUnsupported() {
   return Status(
       "JWK RSA key contained \"d\" property: Private key import is "
@@ -219,6 +237,65 @@ bool Base64DecodeUrlSafe(const std::string& input, std::string* output) {
   return base::Base64Decode(base64EncodedText, output);
 }
 
+// Produces an unpadded 'base64url' encoding of the input data, using the
+// inverse of the process above.
+void Base64EncodeUrlSafe(const base::StringPiece& input, std::string* output) {

[eroman, 2014/03/07 19:33:06]

[optional] Since no error can be returned, you might also consider directly
returning an std::string.

The compiler should be able to do "return value optimization" here.

[padolph, 2014/03/09 22:06:36]

Done.

+  base::Base64Encode(input, output);
+  std::replace(output->begin(), output->end(), '+', '-');
+  std::replace(output->begin(), output->end(), '/', '_');
+  output->erase(std::remove(output->begin(), output->end(), '='),
+                output->end());
+}
+
+struct JwkToWebCryptoUsage {
+  const char* const jwk_key_op;
+  const blink::WebCryptoKeyUsage webcrypto_usage;
+};
+
+const JwkToWebCryptoUsage kJwkWebCryptoUsageMap[] = {
+    {"encrypt", blink::WebCryptoKeyUsageEncrypt},
+    {"decrypt", blink::WebCryptoKeyUsageDecrypt},
+    {"deriveKey", blink::WebCryptoKeyUsageDeriveKey},
+    // TODO(padolph): Add 'deriveBits' once supported by Blink.
+    {"sign", blink::WebCryptoKeyUsageSign},
+    {"unwrapKey", blink::WebCryptoKeyUsageUnwrapKey},
+    {"verify", blink::WebCryptoKeyUsageVerify},
+    {"wrapKey", blink::WebCryptoKeyUsageWrapKey}};
+
+// Composes a Web Crypto usage mask from an array of JWK key_ops values.
+Status GetWebCryptoUsagesFromJwkKeyOps(
+    base::ListValue* jwk_key_ops_value,

[eroman, 2014/03/07 19:33:06]

can this parameter be "const"?

[padolph, 2014/03/09 22:06:36]

Yes. Done.

+    blink::WebCryptoKeyUsageMask* usage_mask) {
+  *usage_mask = 0;
+  for (size_t i = 0; i < jwk_key_ops_value->GetSize(); ++i) {

[eroman, 2014/03/07 19:33:06]

nit: ++i to match other code in webcrypto

[padolph, 2014/03/09 22:06:36]

I don't understand the comment. The code already has '++i'.

[eroman, 2014/03/10 21:55:34]

My mistake, please disregard!

+    std::string key_op;
+    if (!jwk_key_ops_value->GetString(i, &key_op))

[eroman, 2014/03/07 19:33:06]

Please add a curly brace around this (because of line break it is no longer a
"single-line if statement")

[padolph, 2014/03/09 22:06:36]

Done.

+      return Status::ErrorJwkPropertyWrongType(
+          base::StringPrintf("key_ops[%d]", static_cast<int>(i)), "string");
+    size_t j;
+    const size_t map_size = ARRAYSIZE_UNSAFE(kJwkWebCryptoUsageMap);

[eroman, 2014/03/07 19:33:06]

Is the UNSAFE variant needed here or does it work with ARRAYSIZE()?

[padolph, 2014/03/09 22:06:36]

Done.

+    for (j = 0; j < map_size; ++j) {

[eroman, 2014/03/07 19:33:06]

I have been burned by mixups on "i" vs "j" in the past, and encourage you to
pull this out to a helper function, or choose different names for "i" and "j".

For instance:

bool JwkKeyOpToWebCryptoUsage(const std::string key_op, ...* usage) {
  for (size_t i = 0; i < ARRAY_SIZE(kJwkWebCryptoUsageMap); ++i) {
    if (kJwkWebCryptoUsageMap[i].jwk_key_op == key_op) {
      *usage = ....;
      return true;
    }
  }
  return false;
}

[padolph, 2014/03/09 22:06:36]

Done.

+      if (key_op == kJwkWebCryptoUsageMap[j].jwk_key_op) {
+        *usage_mask |= kJwkWebCryptoUsageMap[j].webcrypto_usage;
+        break;
+      }
+    }
+    if (j == map_size)
+      return Status::ErrorJwkUnrecognizedKeyop();
+  }
+  return Status::Success();
+}
+
+// Composes a JWK key_ops List from a Web Crypto usage mask.
+void GetJwkKeyOpsFromWebCryptoUsages(blink::WebCryptoKeyUsageMask usage_mask,

[eroman, 2014/03/07 19:33:06]

I would call this "Fill" rather than "Get".

Or alternately, call it "Create..." and have it return a new base::ListValue.

[padolph, 2014/03/09 22:06:36]

Done.

+                                     base::ListValue* jwk_key_ops) {
+  jwk_key_ops->Clear();
+  for (size_t i = 0; i < ARRAYSIZE_UNSAFE(kJwkWebCryptoUsageMap); ++i) {
+    if (usage_mask & kJwkWebCryptoUsageMap[i].webcrypto_usage)
+      jwk_key_ops->AppendString(kJwkWebCryptoUsageMap[i].jwk_key_op);
+  }
+}
+
 bool IsHashAlgorithm(blink::WebCryptoAlgorithmId alg_id) {
   return alg_id == blink::WebCryptoAlgorithmIdSha1 ||
          alg_id == blink::WebCryptoAlgorithmIdSha224 ||