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

 // Copyright 2014 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/child/webcrypto/nss/rsa_key_nss.h"
 
 #include "base/logging.h"
 #include "content/child/webcrypto/crypto_data.h"
 #include "content/child/webcrypto/jwk.h"
 #include "content/child/webcrypto/nss/key_nss.h"
 #include "content/child/webcrypto/nss/util_nss.h"
 #include "content/child/webcrypto/status.h"
 #include "content/child/webcrypto/webcrypto_util.h"
 #include "crypto/scoped_nss_types.h"
 #include "third_party/WebKit/public/platform/WebCryptoAlgorithmParams.h"
 #include "third_party/WebKit/public/platform/WebCryptoKeyAlgorithm.h"
 
 namespace content {
 
 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(
       "NSS version must be at least 3.16.2 for RSA private key import. See "
       "http://crbug.com/380424");
 }
 
 // Prior to NSS 3.16.2 RSA key parameters were not validated. This is
 // a security problem for RSA private key import from JWK which uses a
 // CKA_ID based on the public modulus to retrieve the private key.
@@ skipping 21 matching lines @@
 
   return ErrorRsaPrivateKeyImportNotSupported();
 }
 #else
 Status NssSupportsRsaPrivateKeyImport() {
   return Status::Success();
 }
 #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.
   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;
-  }
+  *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]:
 //
 //    RSAPrivateKey ::= SEQUENCE {
 //      version           Version,
 //      modulus           INTEGER,  -- n
 //      publicExponent    INTEGER,  -- e
 //      privateExponent   INTEGER,  -- d
 //      prime1            INTEGER,  -- p
@@ skipping 275 matching lines @@
 
   // PK11_FindKeyByKeyID() may return a handle to an existing key, rather than
   // the object created by PK11_CreateGenericObject().
   crypto::ScopedSECKEYPrivateKey private_key(
       SECKEY_CopyPrivateKey(private_key_tmp.get()));
 
   if (!private_key)
     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);
   if (status.IsError())
     return status;
 
   scoped_ptr<PrivateKeyNss> key_handle(
       new PrivateKeyNss(private_key.Pass(), CryptoData(pkcs8_data)));
 
   *key = blink::WebCryptoKey::create(key_handle.release(),
@@ skipping 64 matching lines @@
   if (!pubkey_der)
     return Status::OperationError();
 
   // Import the DER-encoded public key to create an RSA SECKEYPublicKey.
   crypto::ScopedSECKEYPublicKey pubkey(
       SECKEY_ImportDERPublicKey(pubkey_der.get(), CKK_RSA));
   if (!pubkey)
     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);
   if (status.IsError())
     return status;
 
   scoped_ptr<PublicKeyNss> key_handle(
       new PublicKeyNss(pubkey.Pass(), CryptoData(spki_data)));
 
   *key = blink::WebCryptoKey::create(key_handle.release(),
@@ skipping 61 matching lines @@
                                       &rsa_gen_params,
                                       &sec_public_key,
                                       attribute_flags,
                                       generate_flags_,
                                       operation_flags_mask,
                                       NULL));
   if (!scoped_sec_private_key)
     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);
   if (status.IsError())
     return status;
 
   scoped_ptr<PublicKeyNss> public_key_handle(new PublicKeyNss(
       crypto::ScopedSECKEYPublicKey(sec_public_key), CryptoData(spki_data)));
 
   std::vector<uint8_t> pkcs8_data;
@@ skipping 66 matching lines @@
   }
   DCHECK(seckey_private_key);
   crypto::ScopedSECKEYPrivateKey private_key(seckey_private_key);
 
   const KeyType sec_key_type = SECKEY_GetPrivateKeyType(private_key.get());
   if (sec_key_type != rsaKey)
     return Status::DataError();
 
   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;
   status = ExportKeyPkcs8Nss(private_key.get(), &pkcs8_data);
   if (status.IsError())
     return status;
 
   scoped_ptr<PrivateKeyNss> key_handle(
       new PrivateKeyNss(private_key.Pass(), CryptoData(pkcs8_data)));
 
@@ skipping 27 matching lines @@
       SECKEY_ExtractPublicKey(spki.get()));
   if (!sec_public_key)
     return Status::DataError();
 
   const KeyType sec_key_type = SECKEY_GetPublicKeyType(sec_public_key.get());
   if (sec_key_type != rsaKey)
     return Status::DataError();
 
   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;
   Status status = ExportKeySpkiNss(sec_public_key.get(), &spki_data);
   if (status.IsError())
     return status;
 
   scoped_ptr<PublicKeyNss> key_handle(
       new PublicKeyNss(sec_public_key.Pass(), CryptoData(spki_data)));
 
@@ skipping 105 matching lines @@
       return Status::Success();
     }
     default:
       return Status::ErrorUnexpected();
   }
 }
 
 }  // namespace webcrypto
 
 }  // namespace content