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

content/child/webcrypto/jwk.cc

Index: content/child/webcrypto/jwk.cc
diff --git a/content/child/webcrypto/jwk.cc b/content/child/webcrypto/jwk.cc
index 53dd787d7a36cc5ebe15fc9aedc7370a1d10aff4..2e67ddd6e94c5df7337fe14026a424c3d0520c8d 100644
--- a/content/child/webcrypto/jwk.cc
+++ b/content/child/webcrypto/jwk.cc
@@ -130,11 +130,8 @@
 //
 // 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)
+// carried in the JWK to be imported.
+// // - 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     |
@@ -150,6 +147,39 @@
 //   |       | represented as the base64url encoding of the value's         |
 //   |       | unsigned big endian representation as an octet sequence.     |
 //   +-------+--------------------------------------------------------------+
+// - If key == "RSA" and the "d" parameter is present then it is a private key.
+//   All the parameters above for public keys apply, as well as the following.
+//   (Note that except for "d", all of these are optional):
+//   +-------+--------------------------------------------------------------+
+//   | "d"   | Contains the private exponent value for the RSA private key. |
+//   |       | It is represented as the base64url encoding of the value's   |
+//   |       | unsigned big endian representation as an octet sequence.     |
+//   +-------+--------------------------------------------------------------+
+//   | "p"   | Contains the first prime factor value for the RSA private    |
+//   |       | key.  It is represented as the base64url encoding of the     |
+//   |       | value's                                                      |
+//   |       | unsigned big endian representation as an octet sequence.     |
+//   +-------+--------------------------------------------------------------+
+//   | "q"   | Contains the second prime factor value for the RSA private   |
+//   |       | key.  It is represented as the base64url encoding of the     |
+//   |       | value's unsigned big endian representation as an octet       |
+//   |       | sequence.                                                    |
+//   +-------+--------------------------------------------------------------+
+//   | "dp"  | Contains the first factor CRT exponent value for the RSA     |
+//   |       | private key.  It is represented as the base64url encoding of |
+//   |       | the value's unsigned big endian representation as an octet   |
+//   |       | sequence.                                                    |
+//   +-------+--------------------------------------------------------------+
+//   | "dq"  | Contains the second factor CRT exponent value for the RSA    |
+//   |       | private key.  It is represented as the base64url encoding of |
+//   |       | the value's unsigned big endian representation as an octet   |
+//   |       | sequence.                                                    |
+//   +-------+--------------------------------------------------------------+
+//   | "dq"  | Contains the first CRT coefficient value for the RSA private |
+//   |       | key.  It is represented as the base64url encoding of the     |
+//   |       | value's unsigned big endian representation as an octet       |
+//   |       | sequence.                                                    |
+//   +-------+--------------------------------------------------------------+
 //
 // Consistency and conflict resolution
 // The 'algorithm', 'extractable', and 'usage_mask' input parameters
@@ -430,6 +460,25 @@ Status GetJwkBytes(base::DictionaryValue* dict,
   return Status::Success();
 }
 
+// Extracts the optional string property with key |path| from |dict| and saves
+// the base64url-decoded bytes to |*result|. If the property exist and is not a
+// string, or could not be base64url-decoded, returns an error.
+Status GetOptionalJwkBytes(base::DictionaryValue* dict,
+                           const std::string& path,
+                           std::string* result,
+                           bool* property_exists) {
+  std::string base64_string;
+  Status status =
+      GetOptionalJwkString(dict, path, &base64_string, property_exists);
+  if (!*property_exists || status.IsError())

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

nit: Check status.IsError() before doing *property_exists

I would expect that if status.IsError() is true, GetOptionalJwkString may not
have updated property_exists (under the normal Chromium idiom of not mutating
return values on error). Now, this may not be true - you may always be updating
property_exists, but that would force me as a reader to check whether or not
that precondition is true (something i'll likely forget), whereas the logical
re-ordering provides some degree of reasonable expectation.

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

Good point! Done.

Reading the boolean first could be an uninitialized memory read as the API is
currently defined (although not current implementation).

+    return status;
+
+  if (!Base64DecodeUrlSafe(base64_string, result))
+    return Status::ErrorJwkBase64Decode(path);
+
+  return Status::Success();
+}
+
 // Extracts the optional boolean property with key |path| from |dict| and saves
 // the result to |*result| if it was found. If the property exists and is not a
 // boolean, returns an error. Otherwise returns success, and sets
@@ -480,6 +529,53 @@ void WriteRsaPublicKey(const std::vector<uint8>& modulus,
   jwk_dict->SetString("e", Base64EncodeUrlSafe(public_exponent));
 }
 
+// Writes an RSA private key to a JWK dictionary
+Status ExportRsaPrivateKeyJwk(const blink::WebCryptoKey& key,
+                              base::DictionaryValue* jwk_dict) {
+  platform::PrivateKey* private_key;
+  Status status = ToPlatformPrivateKey(key, &private_key);
+  if (status.IsError())
+    return status;
+
+  // TODO(eroman): Copying the key properties to temporary vectors is
+  // inefficient. Once there aren't two implementations of platform_crypto this
+  // and other code will be easier to streamline.
+  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;
+
+  status = platform::ExportRsaPrivateKey(private_key,
+                                         &modulus,
+                                         &public_exponent,
+                                         &private_exponent,
+                                         &prime1,
+                                         &prime2,
+                                         &exponent1,
+                                         &exponent2,
+                                         &coefficient);
+  if (status.IsError())
+    return status;
+
+  jwk_dict->SetString("kty", "RSA");
+  jwk_dict->SetString("n", Base64EncodeUrlSafe(modulus));
+  jwk_dict->SetString("e", Base64EncodeUrlSafe(public_exponent));
+  jwk_dict->SetString("d", Base64EncodeUrlSafe(private_exponent));
+  // Although these are "optional", the JWA spec says that producers
+  // SHOULD include them.

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

Doesn't matter what JWA says. The WebCrypto spec is explicit that they MUST be
included.

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

Done, updated the comment.

+  jwk_dict->SetString("p", Base64EncodeUrlSafe(prime1));
+  jwk_dict->SetString("q", Base64EncodeUrlSafe(prime2));
+  jwk_dict->SetString("dp", Base64EncodeUrlSafe(exponent1));
+  jwk_dict->SetString("dq", Base64EncodeUrlSafe(exponent2));
+  jwk_dict->SetString("qi", Base64EncodeUrlSafe(coefficient));
+
+  return Status::Success();
+}
+
 // Writes a Web Crypto usage mask to a JWK dictionary.
 void WriteKeyOps(blink::WebCryptoKeyUsageMask key_usages,
                  base::DictionaryValue* jwk_dict) {
@@ -592,26 +688,16 @@ Status WriteAlg(const blink::WebCryptoKeyAlgorithm& algorithm,
   return Status::Success();
 }
 
-bool IsRsaPublicKey(const blink::WebCryptoKey& key) {
-  if (key.type() != blink::WebCryptoKeyTypePublic)
-    return false;
+bool IsRsaKey(const blink::WebCryptoKey& key) {
   const blink::WebCryptoAlgorithmId algorithm_id = key.algorithm().id();
   return algorithm_id == blink::WebCryptoAlgorithmIdRsaEsPkcs1v1_5 ||
          algorithm_id == blink::WebCryptoAlgorithmIdRsaSsaPkcs1v1_5 ||
          algorithm_id == blink::WebCryptoAlgorithmIdRsaOaep;
 }
 
-// TODO(padolph): This function is duplicated in shared_crypto.cc
-Status ToPlatformPublicKey(const blink::WebCryptoKey& key,
-                           platform::PublicKey** out) {
-  *out = static_cast<platform::Key*>(key.handle())->AsPublicKey();
-  if (!*out)
-    return Status::ErrorUnexpectedKeyType();
-  return Status::Success();
-}
-
 }  // namespace
 
+// TODO(eroman): Split this up into smaller functions.

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

I would rather see RSA Private key import split out in this CL

Lines 836-912 seem like they can be fully encapsulated stand-alone.

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

Done.

 Status ImportKeyJwk(const CryptoData& key_data,
                     const blink::WebCryptoAlgorithm& algorithm,
                     bool extractable,
@@ -753,13 +839,6 @@ Status ImportKeyJwk(const CryptoData& key_data,
     // (private exponent) entry.
     // See http://tools.ietf.org/html/draft-ietf-jose-json-web-algorithms-18,
     // section 6.3.
-
-    // RSA private key import is not currently supported, so fail here if a "d"
-    // entry is found.
-    // TODO(padolph): Support RSA private key import.
-    if (dict_value->HasKey("d"))
-      return Status::ErrorJwkRsaPrivateKeyUnsupported();
-
     std::string jwk_n_value;
     status = GetJwkBytes(dict_value, "n", &jwk_n_value);
     if (status.IsError())
@@ -769,13 +848,68 @@ Status ImportKeyJwk(const CryptoData& key_data,
     if (status.IsError())
       return status;
 
-    return platform::ImportRsaPublicKey(algorithm,
-                                        extractable,
-                                        usage_mask,
-                                        CryptoData(jwk_n_value),
-                                        CryptoData(jwk_e_value),
-                                        key);
+    if (!dict_value->HasKey("d")) {
+      return platform::ImportRsaPublicKey(algorithm,
+                                          extractable,
+                                          usage_mask,
+                                          CryptoData(jwk_n_value),
+                                          CryptoData(jwk_e_value),
+                                          key);
+    }
+
+    std::string jwk_d_value;
+    status = GetJwkBytes(dict_value, "d", &jwk_d_value);
+    if (status.IsError())
+      return status;
+
+    std::string jwk_p_value;
+    bool has_p;
+    status = GetOptionalJwkBytes(dict_value, "p", &jwk_p_value, &has_p);
+    if (status.IsError())
+      return status;
+
+    std::string jwk_q_value;
+    bool has_q;
+    status = GetOptionalJwkBytes(dict_value, "q", &jwk_q_value, &has_q);
+    if (status.IsError())
+      return status;
 
+    std::string jwk_dp_value;
+    bool has_dp;
+    status = GetOptionalJwkBytes(dict_value, "dp", &jwk_dp_value, &has_dp);
+    if (status.IsError())
+      return status;
+
+    std::string jwk_dq_value;
+    bool has_dq;
+    status = GetOptionalJwkBytes(dict_value, "dq", &jwk_dq_value, &has_dq);
+    if (status.IsError())
+      return status;
+
+    std::string jwk_qi_value;
+    bool has_qi;
+    status = GetOptionalJwkBytes(dict_value, "qi", &jwk_qi_value, &has_qi);
+    if (status.IsError())
+      return status;
+
+    return platform::ImportRsaPrivateKey(
+        algorithm,
+        extractable,
+        usage_mask,
+        CryptoData(jwk_n_value),  // modulus
+        CryptoData(jwk_e_value),  // public_exponent
+        CryptoData(jwk_d_value),  // private_exponent
+        has_p,
+        CryptoData(jwk_p_value),  // prime1
+        has_q,
+        CryptoData(jwk_q_value),  // prime2
+        has_dp,
+        CryptoData(jwk_dp_value),  // exponent1
+        has_dq,
+        CryptoData(jwk_dq_value),  // exponent2
+        has_qi,
+        CryptoData(jwk_qi_value),  // coefficient
+        key);
   }
 
   return Status::ErrorJwkUnrecognizedKty();
@@ -797,8 +931,8 @@ Status ExportKeyJwk(const blink::WebCryptoKey& key,
       break;
     }
     case blink::WebCryptoKeyTypePublic: {
-      // Currently only RSA public key export is supported.
-      if (!IsRsaPublicKey(key))
+      // TODO(eroman): Update when there are asymmetric keys other than RSA.
+      if (!IsRsaKey(key))
         return Status::ErrorUnsupported();
       platform::PublicKey* public_key;
       status = ToPlatformPublicKey(key, &public_key);
@@ -813,7 +947,17 @@ Status ExportKeyJwk(const blink::WebCryptoKey& key,
       WriteRsaPublicKey(modulus, public_exponent, &jwk_dict);
       break;
     }
-    case blink::WebCryptoKeyTypePrivate:  // TODO(padolph)
+    case blink::WebCryptoKeyTypePrivate: {
+      // TODO(eroman): Update when there are asymmetric keys other than RSA.
+      if (!IsRsaKey(key))
+        return Status::ErrorUnsupported();
+
+      status = ExportRsaPrivateKeyJwk(key, &jwk_dict);
+      if (status.IsError())
+        return status;
+      break;
+    }
+
     default:
       return Status::ErrorUnsupported();
   }