Remove some duplicated code, and make rsa_key_nss.cc more closely match rsa_key_openssl.cc.

content/child/webcrypto/nss/rsa_key_nss.cc

Index: content/child/webcrypto/nss/rsa_key_nss.cc
diff --git a/content/child/webcrypto/nss/rsa_key_nss.cc b/content/child/webcrypto/nss/rsa_key_nss.cc
index 99c32bc3d336f681c2f98a225a022b25787729a3..5f11801b41c2667bb43c1d11ad93a07a33fcaf36 100644
--- a/content/child/webcrypto/nss/rsa_key_nss.cc
+++ b/content/child/webcrypto/nss/rsa_key_nss.cc
@@ -21,39 +21,6 @@ namespace webcrypto {
 
 namespace {
 
-bool CreatePublicKeyAlgorithm(const blink::WebCryptoAlgorithm& algorithm,
-                              SECKEYPublicKey* key,
-                              blink::WebCryptoKeyAlgorithm* key_algorithm) {
-  // TODO(eroman): What about other key types rsaPss, rsaOaep.
-  if (!key || key->keyType != rsaKey)
-    return false;
-
-  unsigned int modulus_length_bits = SECKEY_PublicKeyStrength(key) * 8;
-  CryptoData public_exponent(key->u.rsa.publicExponent.data,
-                             key->u.rsa.publicExponent.len);
-
-  switch (algorithm.paramsType()) {
-    case blink::WebCryptoAlgorithmParamsTypeRsaHashedImportParams:
-    case blink::WebCryptoAlgorithmParamsTypeRsaHashedKeyGenParams:
-      *key_algorithm = blink::WebCryptoKeyAlgorithm::createRsaHashed(
-          algorithm.id(),
-          modulus_length_bits,
-          public_exponent.bytes(),
-          public_exponent.byte_length(),
-          GetInnerHashAlgorithm(algorithm).id());
-      return true;
-    default:
-      return false;
-  }
-}
-
-bool CreatePrivateKeyAlgorithm(const blink::WebCryptoAlgorithm& algorithm,
-                               SECKEYPrivateKey* key,
-                               blink::WebCryptoKeyAlgorithm* key_algorithm) {
-  crypto::ScopedSECKEYPublicKey public_key(SECKEY_ConvertToPublicKey(key));
-  return CreatePublicKeyAlgorithm(algorithm, public_key.get(), key_algorithm);
-}
-
 #if defined(USE_NSS) && !defined(OS_CHROMEOS)
 Status ErrorRsaPrivateKeyImportNotSupported() {
   return Status::ErrorUnsupported(
@@ -95,7 +62,8 @@ Status NssSupportsRsaPrivateKeyImport() {
 #endif
 
 bool CreateRsaHashedPublicKeyAlgorithm(
-    const blink::WebCryptoAlgorithm& algorithm,
+    blink::WebCryptoAlgorithmId rsa_algorithm,
+    blink::WebCryptoAlgorithmId hash_algorithm,
     SECKEYPublicKey* key,
     blink::WebCryptoKeyAlgorithm* key_algorithm) {
   // TODO(eroman): What about other key types rsaPss, rsaOaep.
@@ -106,30 +74,25 @@ bool CreateRsaHashedPublicKeyAlgorithm(
   CryptoData public_exponent(key->u.rsa.publicExponent.data,
                              key->u.rsa.publicExponent.len);
 
-  switch (algorithm.paramsType()) {
-    case blink::WebCryptoAlgorithmParamsTypeRsaHashedImportParams:
-    case blink::WebCryptoAlgorithmParamsTypeRsaHashedKeyGenParams:
-      *key_algorithm = blink::WebCryptoKeyAlgorithm::createRsaHashed(
-          algorithm.id(),
-          modulus_length_bits,
-          public_exponent.bytes(),
-          public_exponent.byte_length(),
-          GetInnerHashAlgorithm(algorithm).id());
-      return true;
-    default:
-      return false;
-  }
+  *key_algorithm = blink::WebCryptoKeyAlgorithm::createRsaHashed(
+      rsa_algorithm,

[Ryan Sleevi, 2014/08/27 17:14:09]

Is the assumption that |rsa_algorithm| holds label / saltLength, but not
hash_algorithm?

[eroman, 2014/08/27 17:27:38]

The containing function (CreateRsaHashedPublicKeyAlgorithm) is responsible for
creating the Key.algorithm object given an existing NSS key, and the
"rsa_algorithm" and "hash_algorithm".

These two parameters (rsa_algorithm and hash_algorithm) correspond with the
properties common to both RsaHashedImportParams and RsaHashedKeyGenParams, which
are the two possible entry points for creating the key (unwrapping would also be
given an RsaHashedImportParams).

Note that "rsa_algorithm" is the ID for the algorithm, and not a generic
WebCryptoAlgorithm object. I think this is where the confusion lies.

Therefore label/saltLength are not applicable, as those are operation parameters
and not key creation parameters (label is a parameter to encrypt/decrypt for
RSA-OAEP, and saltLen is a parameter to sign/verify RSA-PSS).

tl;dr; Before what was happening is a generic "key creation algorithm" was being
passed in, and then there was a switch statement (GetInnerHashAlgorithm) to type
cast it and extract the hash. With this re-organization I have removed the
generic switch, and instead ImportKey() and GenerateKey() are responsible for
extracting the hash and passing it into this function (the parameters type is
well known at those entry points).

This matches the organization in the OpenSSL code, since I want to remove big
switch statements as much as possible.

+      modulus_length_bits,
+      public_exponent.bytes(),
+      public_exponent.byte_length(),
+      hash_algorithm);
+  return true;
 }
 
 bool CreateRsaHashedPrivateKeyAlgorithm(
-    const blink::WebCryptoAlgorithm& algorithm,
+    blink::WebCryptoAlgorithmId rsa_algorithm,
+    blink::WebCryptoAlgorithmId hash_algorithm,
     SECKEYPrivateKey* key,
     blink::WebCryptoKeyAlgorithm* key_algorithm) {
   crypto::ScopedSECKEYPublicKey public_key(SECKEY_ConvertToPublicKey(key));
   if (!public_key)
     return false;
   return CreateRsaHashedPublicKeyAlgorithm(
-      algorithm, public_key.get(), key_algorithm);
+      rsa_algorithm, hash_algorithm, public_key.get(), key_algorithm);
 }
 
 // From PKCS#1 [http://tools.ietf.org/html/rfc3447]:
@@ -425,8 +388,13 @@ Status ImportRsaPrivateKey(const blink::WebCryptoAlgorithm& algorithm,
     return Status::OperationError();
 
   blink::WebCryptoKeyAlgorithm key_algorithm;
-  if (!CreatePrivateKeyAlgorithm(algorithm, private_key.get(), &key_algorithm))
+  if (!CreateRsaHashedPrivateKeyAlgorithm(
+          algorithm.id(),
+          algorithm.rsaHashedImportParams()->hash().id(),
+          private_key.get(),
+          &key_algorithm)) {
     return Status::ErrorUnexpected();
+  }
 
   std::vector<uint8_t> pkcs8_data;
   status = ExportKeyPkcs8Nss(private_key.get(), &pkcs8_data);
@@ -511,8 +479,13 @@ Status ImportRsaPublicKey(const blink::WebCryptoAlgorithm& algorithm,
     return Status::OperationError();
 
   blink::WebCryptoKeyAlgorithm key_algorithm;
-  if (!CreatePublicKeyAlgorithm(algorithm, pubkey.get(), &key_algorithm))
+  if (!CreateRsaHashedPublicKeyAlgorithm(
+          algorithm.id(),
+          algorithm.rsaHashedImportParams()->hash().id(),
+          pubkey.get(),
+          &key_algorithm)) {
     return Status::ErrorUnexpected();
+  }
 
   std::vector<uint8_t> spki_data;
   Status status = ExportKeySpkiNss(pubkey.get(), &spki_data);
@@ -594,8 +567,13 @@ Status RsaHashedAlgorithm::GenerateKeyPair(
     return Status::OperationError();
 
   blink::WebCryptoKeyAlgorithm key_algorithm;
-  if (!CreatePublicKeyAlgorithm(algorithm, sec_public_key, &key_algorithm))
+  if (!CreateRsaHashedPublicKeyAlgorithm(
+          algorithm.id(),
+          algorithm.rsaHashedKeyGenParams()->hash().id(),
+          sec_public_key,
+          &key_algorithm)) {
     return Status::ErrorUnexpected();
+  }
 
   std::vector<uint8_t> spki_data;
   status = ExportKeySpkiNss(sec_public_key, &spki_data);
@@ -682,8 +660,12 @@ Status RsaHashedAlgorithm::ImportKeyPkcs8(
 
   blink::WebCryptoKeyAlgorithm key_algorithm;
   if (!CreateRsaHashedPrivateKeyAlgorithm(
-          algorithm, private_key.get(), &key_algorithm))
+          algorithm.id(),
+          algorithm.rsaHashedImportParams()->hash().id(),
+          private_key.get(),
+          &key_algorithm)) {
     return Status::ErrorUnexpected();
+  }
 
   // TODO(eroman): This is probably going to be the same as the input.
   std::vector<uint8_t> pkcs8_data;
@@ -731,8 +713,12 @@ Status RsaHashedAlgorithm::ImportKeySpki(
 
   blink::WebCryptoKeyAlgorithm key_algorithm;
   if (!CreateRsaHashedPublicKeyAlgorithm(
-          algorithm, sec_public_key.get(), &key_algorithm))
+          algorithm.id(),
+          algorithm.rsaHashedImportParams()->hash().id(),
+          sec_public_key.get(),
+          &key_algorithm)) {
     return Status::ErrorUnexpected();
+  }
 
   // TODO(eroman): This is probably going to be the same as the input.
   std::vector<uint8_t> spki_data;