Add support for RSA-OAEP when using NSS 3.16.2 or later

content/child/webcrypto/shared_crypto_unittest.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/shared_crypto.h"
 
 #include <algorithm>
 #include <string>
 #include <vector>
 
@@ skipping 11 matching lines @@
 #include "content/child/webcrypto/status.h"
 #include "content/child/webcrypto/webcrypto_util.h"
 #include "content/public/common/content_paths.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "third_party/WebKit/public/platform/WebCryptoAlgorithm.h"
 #include "third_party/WebKit/public/platform/WebCryptoAlgorithmParams.h"
 #include "third_party/WebKit/public/platform/WebCryptoKey.h"
 #include "third_party/WebKit/public/platform/WebCryptoKeyAlgorithm.h"
 #include "third_party/re2/re2/re2.h"
 
+#if defined(USE_NSS)
+#include <nss.h>
+#include <pk11pub.h>
+
+#include "crypto/scoped_nss_types.h"
+#endif
+
 // The OpenSSL implementation of WebCrypto is less complete, so don't run all of
 // the tests: http://crbug.com/267888
 #if defined(USE_OPENSSL)
 #define MAYBE(test_name) DISABLED_##test_name
 #else
 #define MAYBE(test_name) test_name
 #endif
 
 #define EXPECT_BYTES_EQ(expected, actual) \
   EXPECT_EQ(CryptoData(expected), CryptoData(actual))
@@ skipping 57 matching lines @@
                             CreateAlgorithm(blink::WebCryptoAlgorithmIdAesGcm),
                             true,
                             blink::WebCryptoKeyUsageEncrypt,
                             &key);
 
   if (status.IsError())
     EXPECT_EQ(Status::ErrorUnsupported(), status);
   return status.IsSuccess();
 }
 
+bool SupportsRsaOaep() {
+#if defined(USE_OPENSSL)
+  return false;
+#else
+  if (!NSS_VersionCheck("3.16.2"))

[eroman, 2014/05/16 20:29:51]

I prefer to not have this sort of knowledge in unit-tests:

  - Imagine how a comparable LayoutTest would look -- it cannot do these sort of
internal checks.

  - It is good to exercise the actual code users will run when it is
unavailable. I think that is a worthwhile test.

[Ryan Sleevi, 2014/05/16 22:43:05]

I strongly disagree with this.

1) Performance - these tests should be highly performant.
2) Coupling - Your choice of ImportKey unduly couples the thing under test to
specifics of your implementation
3) Correctness - This added check is to ensure that - independent of what the
IUT thinks, *this test* believes OAEP should be supported.

That is, it serves as a check in and of itself whether or not the IUT is
properly checking if RSA-OAEP is supported.

+    return false;
+  crypto::ScopedPK11Slot slot(PK11_GetInternalKeySlot());
+  return !!PK11_DoesMechanism(slot.get(), CKM_RSA_PKCS_OAEP);

[eroman, 2014/05/16 20:29:51]

Same question as earlier, why the !! rather than implicit cast, or a
static_cast<bool>(). Feels like I am reading Javascript.

[Ryan Sleevi, 2014/05/16 22:43:05]

Because PR_Boolean is an int, and bool is a narrower type. This is a standard
pattern for Windows API code, which does the same thing (with FALSE and TRUE),
as an artifact of the lack of a primitive bool type.

Rather than coercing an int to a bool (which itself is confusing to reason about
- since now you're wondering about what bits are set, what to do with -1, etc),
the !! ensures that any non-zero value is properly and sanely converted.

+#endif
+}
+
 blink::WebCryptoAlgorithm CreateRsaKeyGenAlgorithm(
     blink::WebCryptoAlgorithmId algorithm_id,
     unsigned int modulus_length,
     const std::vector<uint8>& public_exponent) {
   DCHECK_EQ(blink::WebCryptoAlgorithmIdRsaEsPkcs1v1_5, algorithm_id);
   return blink::WebCryptoAlgorithm::adoptParamsAndCreate(
       algorithm_id,
       new blink::WebCryptoRsaKeyGenParams(
           modulus_length,
           webcrypto::Uint8VectorStart(public_exponent),
@@ skipping 10 matching lines @@
   DCHECK(blink::WebCryptoAlgorithm::isHash(hash_id));
   return blink::WebCryptoAlgorithm::adoptParamsAndCreate(
       algorithm_id,
       new blink::WebCryptoRsaHashedKeyGenParams(
           CreateAlgorithm(hash_id),
           modulus_length,
           webcrypto::Uint8VectorStart(public_exponent),
           public_exponent.size()));
 }
 
+// Creates an RSA-OAEP algorithm

[eroman, 2014/05/16 20:29:51]

Might want to mention "for encryption/decryption/wrap/unwrap".

+blink::WebCryptoAlgorithm CreateRsaOaepAlgorithm(
+    const std::vector<uint8>& label) {
+  return blink::WebCryptoAlgorithm::adoptParamsAndCreate(
+      blink::WebCryptoAlgorithmIdRsaOaep,
+      new blink::WebCryptoRsaOaepParams(
+          !label.empty(), Uint8VectorStart(label), label.size()));
+}
+
 // Creates an AES-CBC algorithm.
 blink::WebCryptoAlgorithm CreateAesCbcAlgorithm(const std::vector<uint8>& iv) {
   return blink::WebCryptoAlgorithm::adoptParamsAndCreate(
       blink::WebCryptoAlgorithmIdAesCbc,
       new blink::WebCryptoAesCbcParams(Uint8VectorStart(iv), iv.size()));
 }
 
 // Creates and AES-GCM algorithm.
 blink::WebCryptoAlgorithm CreateAesGcmAlgorithm(
     const std::vector<uint8>& iv,
@@ skipping 247 matching lines @@
     "3b6dcd3eda8e6443024100b69dca1cf7d4d7ec81e75b90fcca874abcde12"
     "3fd2700180aa90479b6e48de8d67ed24f9f19d85ba275874f542cd20dc72"
     "3e6963364a1f9425452b269a6799fd024028fa13938655be1f8a159cbaca"
     "5a72ea190c30089e19cd274a556f36c4f6e19f554b34c077790427bbdd8d"
     "d3ede2448328f385d81b30e8e43b2fffa02786197902401a8b38f398fa71"
     "2049898d7fb79ee0a77668791299cdfa09efc0e507acb21ed74301ef5bfd"
     "48be455eaeb6e1678255827580a8e4e8e14151d1510a82a3f2e729024027"
     "156aba4126d24a81f3a528cbfb27f56886f840a9f6e86e17a44b94fe9319"
     "584b8e22fdde1e5a2e3bd8aa5ba8d8584194eb2190acf832b847f13a3d24"
     "a79f4d";
+// The modulus and exponent (in hex) of kPublicKeySpkiDerHex
+const char* const kPublicKeyModulusHex =
+    "A56E4A0E701017589A5187DC7EA841D156F2EC0E36AD52A44DFEB1E61F7AD991D8C51056"

[eroman, 2014/05/16 20:29:51]

[optional] OCD nit: The hex string above uses lower case whereas this uses upper
case. Do you hate me?

[Ryan Sleevi, 2014/05/16 22:43:05]

This was your string - from 1445.

+    "FFEDB162B4C0F283A12A88A394DFF526AB7291CBB307CEABFCE0B1DFD5CD9508096D5B2B"
+    "8B6DF5D671EF6377C0921CB23C270A70E2598E6FF89D19F105ACC2D3F0CB35F29280E138"
+    "6B6F64C4EF22E1E1F20D0CE8CFFB2249BD9A2137";
+const char* const kPublicKeyExponentHex = "010001";

[eroman, 2014/05/16 20:29:51]

Consider adding a TODO to replace the existing usages with this constant.

[Ryan Sleevi, 2014/05/16 22:43:05]

I already fixed the existing usages. I'm not sure what you're asking here.

[eroman, 2014/05/19 19:04:33]

There are other occurrences of "010001" in the file, which are from tests using
kPrivateKeyPkcs8DerHex / kPublicKeySpkiDerHex. I believe they could be switched
over to the constant as well (although not as part of this CL).

 
 class SharedCryptoTest : public testing::Test {
  protected:
   virtual void SetUp() OVERRIDE { Init(); }
 };
 
 blink::WebCryptoKey ImportSecretKeyFromRaw(
     const std::vector<uint8>& key_raw,
     const blink::WebCryptoAlgorithm& algorithm,
     blink::WebCryptoKeyUsageMask usage) {
@@ skipping 16 matching lines @@
   return key;
 }
 
 void ImportRsaKeyPair(const std::vector<uint8>& spki_der,
                       const std::vector<uint8>& pkcs8_der,
                       const blink::WebCryptoAlgorithm& algorithm,
                       bool extractable,
                       blink::WebCryptoKeyUsageMask usage_mask,
                       blink::WebCryptoKey* public_key,
                       blink::WebCryptoKey* private_key) {
-  EXPECT_EQ(Status::Success(),
+  ASSERT_EQ(Status::Success(),

[Ryan Sleevi, 2014/05/16 05:17:22]

Ran into CHECK failures here when testing and I didn't have the OAEP stuff wired
up in NSS.

Specifically, line 484, but I figure it's better to do the check here.

             ImportKey(blink::WebCryptoKeyFormatSpki,
                       CryptoData(spki_der),
                       algorithm,
                       true,
                       usage_mask,
                       public_key));
   EXPECT_FALSE(public_key->isNull());
   EXPECT_TRUE(public_key->handle());
   EXPECT_EQ(blink::WebCryptoKeyTypePublic, public_key->type());
   EXPECT_EQ(algorithm.id(), public_key->algorithm().id());
   EXPECT_EQ(extractable, extractable);
   EXPECT_EQ(usage_mask, public_key->usages());
 
-  EXPECT_EQ(Status::Success(),
+  ASSERT_EQ(Status::Success(),
             ImportKey(blink::WebCryptoKeyFormatPkcs8,
                       CryptoData(pkcs8_der),
                       algorithm,
                       extractable,
                       usage_mask,
                       private_key));
   EXPECT_FALSE(private_key->isNull());
   EXPECT_TRUE(private_key->handle());
   EXPECT_EQ(blink::WebCryptoKeyTypePrivate, private_key->type());
   EXPECT_EQ(algorithm.id(), private_key->algorithm().id());
@@ skipping 965 matching lines @@
 
   // Fail on k actual length (192 bits) inconsistent with the embedded JWK alg
   // value (128) for an AES key.
   dict.SetString("k", "dGhpcyAgaXMgIDI0ICBieXRlcyBsb25n");
   EXPECT_EQ(Status::ErrorJwkIncorrectKeyLength(),
             ImportKeyJwkFromDict(dict, algorithm, false, usage_mask, &key));
   RestoreJwkOctDictionary(&dict);
 }
 
 TEST_F(SharedCryptoTest, MAYBE(ImportExportJwkRsaPublicKey)) {
-  // This test uses kPublicKeySpkiDerHex as the RSA key. The data below
-  // represents the modulus and public exponent extracted from this SPKI blob.
-  // These values appear explicitly in the JWK rendering of the key.
-  const std::string n_hex =
-      "A56E4A0E701017589A5187DC7EA841D156F2EC0E36AD52A44DFEB1E61F7AD991D8C51056"
-      "FFEDB162B4C0F283A12A88A394DFF526AB7291CBB307CEABFCE0B1DFD5CD9508096D5B2B"
-      "8B6DF5D671EF6377C0921CB23C270A70E2598E6FF89D19F105ACC2D3F0CB35F29280E138"
-      "6B6F64C4EF22E1E1F20D0CE8CFFB2249BD9A2137";
-  const std::string e_hex = "010001";
+  const bool supports_rsa_oaep = SupportsRsaOaep();
+  if (!supports_rsa_oaep) {
+    LOG(WARNING) << "RSA-OAEP not supported on this platform. Skipping some"

[eroman, 2014/05/16 20:29:51]

sometests --> some tests

+                 << "tests.";
+  }
 
   struct TestCase {
     const blink::WebCryptoAlgorithm algorithm;
     const blink::WebCryptoKeyUsageMask usage;
     const char* const jwk_alg;
   };
   const TestCase kTests[] = {
       // RSAES-PKCS1-v1_5
       {CreateAlgorithm(blink::WebCryptoAlgorithmIdRsaEsPkcs1v1_5),
        blink::WebCryptoKeyUsageEncrypt, "RSA1_5"},
       // RSASSA-PKCS1-v1_5 SHA-1
       {CreateRsaHashedImportAlgorithm(
            blink::WebCryptoAlgorithmIdRsaSsaPkcs1v1_5,
            blink::WebCryptoAlgorithmIdSha1),
        blink::WebCryptoKeyUsageSign, "RS1"},
       // RSASSA-PKCS1-v1_5 SHA-256
       {CreateRsaHashedImportAlgorithm(
            blink::WebCryptoAlgorithmIdRsaSsaPkcs1v1_5,
            blink::WebCryptoAlgorithmIdSha256),
        blink::WebCryptoKeyUsageSign, "RS256"},
       // RSASSA-PKCS1-v1_5 SHA-384
       {CreateRsaHashedImportAlgorithm(
            blink::WebCryptoAlgorithmIdRsaSsaPkcs1v1_5,
            blink::WebCryptoAlgorithmIdSha384),
        blink::WebCryptoKeyUsageSign, "RS384"},
       // RSASSA-PKCS1-v1_5 SHA-512
       {CreateRsaHashedImportAlgorithm(
            blink::WebCryptoAlgorithmIdRsaSsaPkcs1v1_5,
            blink::WebCryptoAlgorithmIdSha512),
-       blink::WebCryptoKeyUsageSign, "RS512"}};
+       blink::WebCryptoKeyUsageSign, "RS512"},
+      // RSA-OAEP with SHA-1 and MGF-1 / SHA-1
+      {CreateRsaHashedImportAlgorithm(blink::WebCryptoAlgorithmIdRsaOaep,
+                                      blink::WebCryptoAlgorithmIdSha1),
+       blink::WebCryptoKeyUsageEncrypt, "RSA-OAEP"},
+      // RSA-OAEP with SHA-256 and MGF-1 / SHA-256
+      {CreateRsaHashedImportAlgorithm(blink::WebCryptoAlgorithmIdRsaOaep,
+                                      blink::WebCryptoAlgorithmIdSha256),
+       blink::WebCryptoKeyUsageEncrypt, "RSA-OAEP-256"},
+      // RSA-OAEP with SHA-384 and MGF-1 / SHA-384
+      {CreateRsaHashedImportAlgorithm(blink::WebCryptoAlgorithmIdRsaOaep,
+                                      blink::WebCryptoAlgorithmIdSha384),
+       blink::WebCryptoKeyUsageEncrypt, "RSA-OAEP-384"},
+      // RSA-OAEP with SHA-512 and MGF-1 / SHA-512
+      {CreateRsaHashedImportAlgorithm(blink::WebCryptoAlgorithmIdRsaOaep,
+                                      blink::WebCryptoAlgorithmIdSha512),
+       blink::WebCryptoKeyUsageEncrypt, "RSA-OAEP-512"}};
 
   for (size_t test_index = 0; test_index < ARRAYSIZE_UNSAFE(kTests);
        ++test_index) {
     SCOPED_TRACE(test_index);
     const TestCase& test = kTests[test_index];
+    if (!supports_rsa_oaep &&
+        test.algorithm.id() == blink::WebCryptoAlgorithmIdRsaOaep) {
+      continue;

[eroman, 2014/05/16 20:29:51]

Consider adding the skip warning here. The outter log has higher chance of
becoming obsolete during some refactor.

[Ryan Sleevi, 2014/05/16 22:43:05]

I generally oppose logging skipped tests on principle, because they create
significant log spew on any bot that doesn't implement them, and they provide no
visibility in terms of tree closing/test failures that something is wrong.

The better way to handle such tests is to use GTest generators, so that the
number of parameterized tests can change based on platform, providing a clearer
signal about tests that changed.

+    }
 
     // Import the spki to create a public key
     blink::WebCryptoKey public_key = blink::WebCryptoKey::createNull();
     ASSERT_EQ(Status::Success(),
               ImportKey(blink::WebCryptoKeyFormatSpki,
                         CryptoData(HexStringToBytes(kPublicKeySpkiDerHex)),
                         test.algorithm,
                         true,
                         test.usage,
                         &public_key));
 
     // Export the public key as JWK and verify its contents
     std::vector<uint8> jwk;
     ASSERT_EQ(Status::Success(),
               ExportKey(blink::WebCryptoKeyFormatJwk, public_key, &jwk));
-    EXPECT_TRUE(VerifyPublicJwk(jwk, test.jwk_alg, n_hex, e_hex, test.usage));
+    EXPECT_TRUE(VerifyPublicJwk(jwk,
+                                test.jwk_alg,
+                                kPublicKeyModulusHex,
+                                kPublicKeyExponentHex,
+                                test.usage));
 
     // Import the JWK back in to create a new key
     blink::WebCryptoKey public_key2 = blink::WebCryptoKey::createNull();
-    EXPECT_EQ(
+    ASSERT_EQ(
         Status::Success(),
         ImportKeyJwk(
             CryptoData(jwk), test.algorithm, true, test.usage, &public_key2));
-    EXPECT_TRUE(public_key2.handle());
+    ASSERT_TRUE(public_key2.handle());
     EXPECT_EQ(blink::WebCryptoKeyTypePublic, public_key2.type());
     EXPECT_EQ(true, public_key2.extractable());
     EXPECT_EQ(test.algorithm.id(), public_key2.algorithm().id());
 
-    // Export the new key as spki and compare to the original.
-    std::vector<uint8> spki;
-    ASSERT_EQ(Status::Success(),
-              ExportKey(blink::WebCryptoKeyFormatSpki, public_key2, &spki));
-    EXPECT_BYTES_EQ_HEX(kPublicKeySpkiDerHex, CryptoData(spki));
+    // Only perform SPKI consistency test for RSA-ES and RSA-SSA, as their
+    // export format is the same as kPublicKeySpkiDerHex

[Ryan Sleevi, 2014/05/16 05:17:22]

These should probably be moved into the JSON proper at a later time.

[eroman, 2014/05/16 20:29:51]

What is the difference in format, just the OID? Feels like there is a TODO
missing here.

[Ryan Sleevi, 2014/05/16 22:43:05]

You already filed the bug -
https://code.google.com/p/chromium/issues/detail?id=373545

+    if (test.algorithm.id() == blink::WebCryptoAlgorithmIdRsaSsaPkcs1v1_5 ||
+        test.algorithm.id() == blink::WebCryptoAlgorithmIdRsaEsPkcs1v1_5) {
+      // Export the new key as spki and compare to the original.
+      std::vector<uint8> spki;
+      ASSERT_EQ(Status::Success(),
+                ExportKey(blink::WebCryptoKeyFormatSpki, public_key2, &spki));
+      EXPECT_BYTES_EQ_HEX(kPublicKeySpkiDerHex, CryptoData(spki));
+    }
   }
 }
 
 TEST_F(SharedCryptoTest, MAYBE(ImportJwkRsaFailures)) {
   base::DictionaryValue dict;
   RestoreJwkRsaDictionary(&dict);
   blink::WebCryptoAlgorithm algorithm =
       CreateAlgorithm(blink::WebCryptoAlgorithmIdRsaEsPkcs1v1_5);
   blink::WebCryptoKeyUsageMask usage_mask = blink::WebCryptoKeyUsageEncrypt;
   blink::WebCryptoKey key = blink::WebCryptoKey::createNull();
@@ skipping 444 matching lines @@
       ImportKey(blink::WebCryptoKeyFormatSpki,
                 CryptoData(HexStringToBytes(kPublicKeySpkiDerHex)),
                 CreateAlgorithm(blink::WebCryptoAlgorithmIdRsaEsPkcs1v1_5),
                 false,
                 blink::WebCryptoKeyUsageEncrypt,
                 &key));
   EXPECT_TRUE(key.handle());
   EXPECT_FALSE(key.extractable());
   EXPECT_EQ(Status::ErrorKeyNotExtractable(),
             ExportKey(blink::WebCryptoKeyFormatSpki, key, &output));
+
+  // TODO(eroman): Failing test: Import a SPKI with an unrecognized hash OID
+  // TODO(eroman): Failing test: Import a SPKI with invalid algorithm params
+  // TODO(eroman): Failing test: Import a SPKI with inconsistent parameters
+  // (e.g. SHA-1 in OID, SHA-256 in params)
+  // TODO(eroman): Failing test: Import a SPKI for RSA-SSA, but with params
+  // as OAEP/PSS

[Ryan Sleevi, 2014/05/16 05:17:22]

Just documenting various SPKI failures.

[eroman, 2014/05/16 20:29:51]

why TODO(eroman) and not TODO(rsleevi)? 

FYI TODO(name) roughly means "todo written by name" not "name will do":

http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml?showone=TODO_C...

[Ryan Sleevi, 2014/05/16 22:43:05]

Because you filed the bug on Import/Export SPKI and PKCS8.

 }
 
 TEST_F(SharedCryptoTest, MAYBE(ImportExportPkcs8)) {
   // Passing case: Import a valid RSA key in PKCS#8 format.
   blink::WebCryptoKey key = blink::WebCryptoKey::createNull();
   ASSERT_EQ(Status::Success(),
             ImportKey(blink::WebCryptoKeyFormatPkcs8,
                       CryptoData(HexStringToBytes(kPrivateKeyPkcs8DerHex)),
                       CreateRsaHashedImportAlgorithm(
                           blink::WebCryptoAlgorithmIdRsaSsaPkcs1v1_5,
@@ skipping 234 matching lines @@
   EXPECT_EQ(Status::ErrorUnexpectedKeyType(),
             ExportKey(blink::WebCryptoKeyFormatSpki, private_key, &output));
 }
 
 TEST_F(SharedCryptoTest, MAYBE(RsaEsRoundTrip)) {
   // Import a key pair.
   blink::WebCryptoAlgorithm algorithm =
       CreateAlgorithm(blink::WebCryptoAlgorithmIdRsaEsPkcs1v1_5);
   blink::WebCryptoKey public_key = blink::WebCryptoKey::createNull();
   blink::WebCryptoKey private_key = blink::WebCryptoKey::createNull();
-  ImportRsaKeyPair(
+  ASSERT_NO_FATAL_FAILURE(ImportRsaKeyPair(

[Ryan Sleevi, 2014/05/16 05:17:22]

*slightly* unrelated, but the OAEP correction (to ASSERT_) made me decide to
wrap all of these as well, since everything else crashes if this import fails.

       HexStringToBytes(kPublicKeySpkiDerHex),
       HexStringToBytes(kPrivateKeyPkcs8DerHex),
       algorithm,
       false,
       blink::WebCryptoKeyUsageEncrypt | blink::WebCryptoKeyUsageDecrypt,
       &public_key,
-      &private_key);
+      &private_key));
 
   // Make a maximum-length data message. RSAES can operate on messages up to
   // length of k - 11 bytes, where k is the octet length of the RSA modulus.
   const unsigned int kMaxMsgSizeBytes = kModulusLengthBits / 8 - 11;
   // There are two hex chars for each byte.
   const unsigned int kMsgHexSize = kMaxMsgSizeBytes * 2;
   char max_data_hex[kMsgHexSize + 1];
   std::fill(&max_data_hex[0], &max_data_hex[0] + kMsgHexSize, 'a');
   max_data_hex[kMsgHexSize] = '\0';
 
@@ skipping 40 matching lines @@
       GetBytesFromHexString(test, "rsa_pkcs8_der");
   const std::vector<uint8> ciphertext =
       GetBytesFromHexString(test, "ciphertext");
   const std::vector<uint8> cleartext = GetBytesFromHexString(test, "cleartext");
 
   // Import the key pair.
   blink::WebCryptoAlgorithm algorithm =
       CreateAlgorithm(blink::WebCryptoAlgorithmIdRsaEsPkcs1v1_5);
   blink::WebCryptoKey public_key = blink::WebCryptoKey::createNull();
   blink::WebCryptoKey private_key = blink::WebCryptoKey::createNull();
-  ImportRsaKeyPair(
+  ASSERT_NO_FATAL_FAILURE(ImportRsaKeyPair(
       rsa_spki_der,
       rsa_pkcs8_der,
       algorithm,
       false,
       blink::WebCryptoKeyUsageEncrypt | blink::WebCryptoKeyUsageDecrypt,
       &public_key,
-      &private_key);
+      &private_key));
 
   // Decrypt the known-good ciphertext with the private key. As a check we must
   // get the known original cleartext.
   std::vector<uint8> decrypted_data;
   ASSERT_EQ(
       Status::Success(),
       Decrypt(algorithm, private_key, CryptoData(ciphertext), &decrypted_data));
   EXPECT_BYTES_EQ(cleartext, decrypted_data);
 
   // Encrypt this decrypted data with the public key.
@@ skipping 13 matching lines @@
           algorithm, private_key, CryptoData(encrypted_data), &decrypted_data));
   EXPECT_EQ(cleartext, decrypted_data);
 }
 
 TEST_F(SharedCryptoTest, MAYBE(RsaEsFailures)) {
   // Import a key pair.
   blink::WebCryptoAlgorithm algorithm =
       CreateAlgorithm(blink::WebCryptoAlgorithmIdRsaEsPkcs1v1_5);
   blink::WebCryptoKey public_key = blink::WebCryptoKey::createNull();
   blink::WebCryptoKey private_key = blink::WebCryptoKey::createNull();
-  ImportRsaKeyPair(
+  ASSERT_NO_FATAL_FAILURE(ImportRsaKeyPair(
       HexStringToBytes(kPublicKeySpkiDerHex),
       HexStringToBytes(kPrivateKeyPkcs8DerHex),
       algorithm,
       false,
       blink::WebCryptoKeyUsageEncrypt | blink::WebCryptoKeyUsageDecrypt,
       &public_key,
-      &private_key);
+      &private_key));
 
   // Fail encrypt with a private key.
   std::vector<uint8> encrypted_data;
   const std::string message_hex_str("0102030405060708090a0b0c0d0e0f");
   const std::vector<uint8> message_hex(HexStringToBytes(message_hex_str));
   EXPECT_EQ(
       Status::ErrorUnexpectedKeyType(),
       Encrypt(
           algorithm, private_key, CryptoData(message_hex), &encrypted_data));
 
@@ skipping 45 matching lines @@
 
 TEST_F(SharedCryptoTest, MAYBE(RsaSsaSignVerifyFailures)) {
   // Import a key pair.
   blink::WebCryptoKeyUsageMask usage_mask =
       blink::WebCryptoKeyUsageSign | blink::WebCryptoKeyUsageVerify;
   blink::WebCryptoAlgorithm importAlgorithm =
       CreateRsaHashedImportAlgorithm(blink::WebCryptoAlgorithmIdRsaSsaPkcs1v1_5,
                                      blink::WebCryptoAlgorithmIdSha1);
   blink::WebCryptoKey public_key = blink::WebCryptoKey::createNull();
   blink::WebCryptoKey private_key = blink::WebCryptoKey::createNull();
-  ImportRsaKeyPair(HexStringToBytes(kPublicKeySpkiDerHex),
-                   HexStringToBytes(kPrivateKeyPkcs8DerHex),
-                   importAlgorithm,
-                   false,
-                   usage_mask,
-                   &public_key,
-                   &private_key);
+  ASSERT_NO_FATAL_FAILURE(
+      ImportRsaKeyPair(HexStringToBytes(kPublicKeySpkiDerHex),
+                       HexStringToBytes(kPrivateKeyPkcs8DerHex),
+                       importAlgorithm,
+                       false,
+                       usage_mask,
+                       &public_key,
+                       &private_key));
 
   blink::WebCryptoAlgorithm algorithm =
       CreateAlgorithm(blink::WebCryptoAlgorithmIdRsaSsaPkcs1v1_5);
 
   std::vector<uint8> signature;
   bool signature_match;
 
   // Compute a signature.
   const std::vector<uint8> data = HexStringToBytes("010203040506070809");
   ASSERT_EQ(Status::Success(),
@@ skipping 116 matching lines @@
 TEST_F(SharedCryptoTest, MAYBE(RsaSignVerifyKnownAnswer)) {
   scoped_ptr<base::ListValue> tests;
   ASSERT_TRUE(ReadJsonTestFileToList("pkcs1v15_sign.json", &tests));
 
   // Import the key pair.
   blink::WebCryptoAlgorithm importAlgorithm =
       CreateRsaHashedImportAlgorithm(blink::WebCryptoAlgorithmIdRsaSsaPkcs1v1_5,
                                      blink::WebCryptoAlgorithmIdSha1);
   blink::WebCryptoKey public_key = blink::WebCryptoKey::createNull();
   blink::WebCryptoKey private_key = blink::WebCryptoKey::createNull();
-  ImportRsaKeyPair(
+  ASSERT_NO_FATAL_FAILURE(ImportRsaKeyPair(
       HexStringToBytes(kPublicKeySpkiDerHex),
       HexStringToBytes(kPrivateKeyPkcs8DerHex),
       importAlgorithm,
       false,
       blink::WebCryptoKeyUsageSign | blink::WebCryptoKeyUsageVerify,
       &public_key,
-      &private_key);
+      &private_key));
 
   blink::WebCryptoAlgorithm algorithm =
       CreateAlgorithm(blink::WebCryptoAlgorithmIdRsaSsaPkcs1v1_5);
 
   // Validate the signatures are computed and verified as expected.
   std::vector<uint8> signature;
   for (size_t test_index = 0; test_index < tests->GetSize(); ++test_index) {
     SCOPED_TRACE(test_index);
 
     base::DictionaryValue* test;
@@ skipping 553 matching lines @@
       GetBytesFromHexString(test, "ciphertext");
   const std::vector<uint8> cleartext = GetBytesFromHexString(test, "cleartext");
   blink::WebCryptoAlgorithm key_algorithm =
       CreateHmacImportAlgorithm(blink::WebCryptoAlgorithmIdSha256);
 
   // Import the RSA key pair.
   blink::WebCryptoAlgorithm algorithm =
       CreateAlgorithm(blink::WebCryptoAlgorithmIdRsaEsPkcs1v1_5);
   blink::WebCryptoKey public_key = blink::WebCryptoKey::createNull();
   blink::WebCryptoKey private_key = blink::WebCryptoKey::createNull();
-  ImportRsaKeyPair(
+  ASSERT_NO_FATAL_FAILURE(ImportRsaKeyPair(
       rsa_spki_der,
       rsa_pkcs8_der,
       algorithm,
       false,
       blink::WebCryptoKeyUsageWrapKey | blink::WebCryptoKeyUsageUnwrapKey,
       &public_key,
-      &private_key);
+      &private_key));
 
   // Import the symmetric key.
   blink::WebCryptoKey key = blink::WebCryptoKey::createNull();
   ASSERT_EQ(Status::Success(),
             ImportKey(blink::WebCryptoKeyFormatRaw,
                       CryptoData(cleartext),
                       key_algorithm,
                       true,
                       blink::WebCryptoKeyUsageSign,
                       &key));
@@ skipping 49 matching lines @@
 TEST_F(SharedCryptoTest, MAYBE(RsaEsRawSymkeyWrapUnwrapErrors)) {
   const std::vector<uint8> data(64, 0);
   blink::WebCryptoAlgorithm key_algorithm =
       CreateHmacImportAlgorithm(blink::WebCryptoAlgorithmIdSha256);
 
   // Import the RSA key pair.
   blink::WebCryptoAlgorithm wrapping_algorithm =
       CreateAlgorithm(blink::WebCryptoAlgorithmIdRsaEsPkcs1v1_5);
   blink::WebCryptoKey public_key = blink::WebCryptoKey::createNull();
   blink::WebCryptoKey private_key = blink::WebCryptoKey::createNull();
-  ImportRsaKeyPair(
+  ASSERT_NO_FATAL_FAILURE(ImportRsaKeyPair(
       HexStringToBytes(kPublicKeySpkiDerHex),
       HexStringToBytes(kPrivateKeyPkcs8DerHex),
       wrapping_algorithm,
       false,
       blink::WebCryptoKeyUsageWrapKey | blink::WebCryptoKeyUsageUnwrapKey,
       &public_key,
-      &private_key);
+      &private_key));
 
   // Import the symmetric key.
   blink::WebCryptoKey key = blink::WebCryptoKey::createNull();
   ASSERT_EQ(Status::Success(),
             ImportKey(blink::WebCryptoKeyFormatRaw,
                       CryptoData(data),
                       key_algorithm,
                       true,
                       blink::WebCryptoKeyUsageSign,
                       &key));
@@ skipping 125 matching lines @@
   ASSERT_EQ(
       Status::Success(),
       GenerateSecretKey(
           gen_algorithm, true, blink::WebCryptoKeyUsageEncrypt, &key_to_wrap));
 
   // Import the wrapping key pair.
   const blink::WebCryptoAlgorithm wrapping_algorithm =
       CreateAlgorithm(blink::WebCryptoAlgorithmIdRsaEsPkcs1v1_5);
   blink::WebCryptoKey public_wrapping_key = blink::WebCryptoKey::createNull();
   blink::WebCryptoKey private_wrapping_key = blink::WebCryptoKey::createNull();
-  ImportRsaKeyPair(
+  ASSERT_NO_FATAL_FAILURE(ImportRsaKeyPair(
       HexStringToBytes(kPublicKeySpkiDerHex),
       HexStringToBytes(kPrivateKeyPkcs8DerHex),
       wrapping_algorithm,
       false,
       blink::WebCryptoKeyUsageWrapKey | blink::WebCryptoKeyUsageUnwrapKey,
       &public_wrapping_key,
-      &private_wrapping_key);
+      &private_wrapping_key));
 
   // Wrap the symkey in JWK format, using the public wrapping key.
   std::vector<uint8> wrapped_data;
   ASSERT_EQ(Status::Success(),
             WrapKey(blink::WebCryptoKeyFormatJwk,
                     key_to_wrap,
                     public_wrapping_key,
                     wrapping_algorithm,
                     &wrapped_data));
 
@@ skipping 81 matching lines @@
             UnwrapKey(blink::WebCryptoKeyFormatJwk,
                       CryptoData(ciphertext),
                       private_wrapping_key,
                       algorithm,
                       CreateAesCbcAlgorithm(std::vector<uint8>(0, 16)),
                       true,
                       blink::WebCryptoKeyUsageEncrypt,
                       &unwrapped_key));
 }
 
+class SharedCryptoRsaOaepTest : public ::testing::Test {
+ public:
+  SharedCryptoRsaOaepTest() { Init(); }

[eroman, 2014/05/16 20:29:51]

Would inheriting from SharedCryptoTest instead work?

+
+  scoped_ptr<base::DictionaryValue> CreatePublicKeyJwkDict() {

[eroman, 2014/05/16 20:29:51]

why introduce a new test fixture, can't this be a function?

[Ryan Sleevi, 2014/05/16 22:43:05]

It can. A new fixture was created, because in my ideal testing world, we would
have files per algorithm being tested - not a 10,000 line test file.

Creating a free function also creates the risk (as evidenced by my own 'misuse')
that it will be used by other tests. I intended this function to only be used by
these tests, because their tests explicitly depend on these values being what
they are.

Creating a free function creates more opportunity for unnecessary coupling.

[eroman, 2014/05/19 19:04:33]

Ok

+    scoped_ptr<base::DictionaryValue> jwk(new base::DictionaryValue());
+    jwk->SetString("kty", "RSA");
+    jwk->SetString("n",
+                   Base64EncodeUrlSafe(HexStringToBytes(kPublicKeyModulusHex)));
+    jwk->SetString(
+        "e", Base64EncodeUrlSafe(HexStringToBytes(kPublicKeyExponentHex)));
+    return jwk.Pass();
+  }
+};
+
+// Import a PKCS#8 private key that uses RSAPrivateKey with the
+// id-rsaEncryption OID.
+TEST_F(SharedCryptoRsaOaepTest, ImportPkcs8WithRsaEncryption) {

[Ryan Sleevi, 2014/05/16 05:17:22]

More tests needed: Variations of SPKI param handling. I'm having to look into
NSS first though.

[eroman, 2014/05/16 20:29:51]

These tests are going to fail when run on Android no? Wrap the function name in
MAYBE().

[Ryan Sleevi, 2014/05/16 22:43:05]

No. Because SupportsRsaOaep().

+  if (!SupportsRsaOaep()) {
+    LOG(WARNING) << "RSA-OAEP support not present; skipping.";
+    return;
+  }
+
+  blink::WebCryptoKey private_key = blink::WebCryptoKey::createNull();
+  ASSERT_EQ(Status::Success(),
+            ImportKey(blink::WebCryptoKeyFormatPkcs8,
+                      CryptoData(HexStringToBytes(kPrivateKeyPkcs8DerHex)),
+                      CreateRsaHashedImportAlgorithm(
+                          blink::WebCryptoAlgorithmIdRsaOaep,
+                          blink::WebCryptoAlgorithmIdSha1),
+                      true,
+                      blink::WebCryptoKeyUsageDecrypt,
+                      &private_key));
+}
+
+TEST_F(SharedCryptoRsaOaepTest, ImportJwkWithNoAlg) {
+  if (!SupportsRsaOaep()) {
+    LOG(WARNING) << "RSA-OAEP support not present; skipping.";
+    return;
+  }
+
+  scoped_ptr<base::DictionaryValue> jwk(CreatePublicKeyJwkDict());
+
+  blink::WebCryptoKey public_key = blink::WebCryptoKey::createNull();
+  ASSERT_EQ(Status::Success(),
+            ImportKeyJwkFromDict(*jwk.get(),
+                                 CreateRsaHashedImportAlgorithm(
+                                     blink::WebCryptoAlgorithmIdRsaOaep,
+                                     blink::WebCryptoAlgorithmIdSha1),
+                                 true,
+                                 blink::WebCryptoKeyUsageEncrypt,
+                                 &public_key));

[eroman, 2014/05/16 20:29:51]

Assert that the resulting key.algorithm() matches?

+}
+
+TEST_F(SharedCryptoRsaOaepTest, ImportJwkWithMatchingAlg) {
+  if (!SupportsRsaOaep()) {
+    LOG(WARNING) << "RSA-OAEP support not present; skipping.";
+    return;
+  }
+
+  scoped_ptr<base::DictionaryValue> jwk(CreatePublicKeyJwkDict());
+  jwk->SetString("alg", "RSA-OAEP");
+
+  blink::WebCryptoKey public_key = blink::WebCryptoKey::createNull();
+  ASSERT_EQ(Status::Success(),
+            ImportKeyJwkFromDict(*jwk.get(),
+                                 CreateRsaHashedImportAlgorithm(
+                                     blink::WebCryptoAlgorithmIdRsaOaep,
+                                     blink::WebCryptoAlgorithmIdSha1),
+                                 true,
+                                 blink::WebCryptoKeyUsageEncrypt,
+                                 &public_key));
+}
+
+TEST_F(SharedCryptoRsaOaepTest, ImportJwkWithMismatchedAlgFails) {
+  if (!SupportsRsaOaep()) {
+    LOG(WARNING) << "RSA-OAEP support not present; skipping.";
+    return;
+  }
+
+  scoped_ptr<base::DictionaryValue> jwk(CreatePublicKeyJwkDict());
+  jwk->SetString("alg", "RSA-OAEP-512");
+
+  blink::WebCryptoKey public_key = blink::WebCryptoKey::createNull();
+  ASSERT_EQ(Status::ErrorJwkAlgorithmInconsistent(),
+            ImportKeyJwkFromDict(*jwk.get(),
+                                 CreateRsaHashedImportAlgorithm(
+                                     blink::WebCryptoAlgorithmIdRsaOaep,
+                                     blink::WebCryptoAlgorithmIdSha1),
+                                 true,
+                                 blink::WebCryptoKeyUsageEncrypt,
+                                 &public_key));
+}
+
+TEST_F(SharedCryptoRsaOaepTest, ImportJwkWithMismatchedTypFails) {
+  if (!SupportsRsaOaep()) {
+    LOG(WARNING) << "RSA-OAEP support not present; skipping.";
+    return;
+  }
+
+  scoped_ptr<base::DictionaryValue> jwk(CreatePublicKeyJwkDict());
+  jwk->SetString("kty", "oct");
+  jwk->SetString("alg", "RSA-OAEP");
+
+  blink::WebCryptoKey public_key = blink::WebCryptoKey::createNull();
+  ASSERT_EQ(Status::ErrorJwkPropertyMissing("k"),
+            ImportKeyJwkFromDict(*jwk.get(),
+                                 CreateRsaHashedImportAlgorithm(
+                                     blink::WebCryptoAlgorithmIdRsaOaep,
+                                     blink::WebCryptoAlgorithmIdSha1),
+                                 true,
+                                 blink::WebCryptoKeyUsageEncrypt,
+                                 &public_key));
+}
+
+TEST_F(SharedCryptoRsaOaepTest, ExportJwk) {

[eroman, 2014/05/16 20:29:51]

Include "Public" somewhere in the name.

+  if (!SupportsRsaOaep()) {
+    LOG(WARNING) << "RSA-OAEP support not present; skipping.";
+    return;
+  }
+
+  struct TestData {
+    blink::WebCryptoAlgorithmId hash_alg;
+    const char* expected_jwk_alg;
+  } kTestData[] = {{blink::WebCryptoAlgorithmIdSha1, "RSA-OAEP"},
+                   {blink::WebCryptoAlgorithmIdSha256, "RSA-OAEP-256"},
+                   {blink::WebCryptoAlgorithmIdSha384, "RSA-OAEP-384"},
+                   {blink::WebCryptoAlgorithmIdSha512, "RSA-OAEP-512"}};
+  for (size_t i = 0; i < ARRAYSIZE_UNSAFE(kTestData); ++i) {
+    const TestData& test_data = kTestData[i];
+    SCOPED_TRACE(test_data.expected_jwk_alg);
+
+    scoped_ptr<base::DictionaryValue> jwk(CreatePublicKeyJwkDict());
+    jwk->SetString("alg", test_data.expected_jwk_alg);
+
+    // Import the key in a known-good format
+    blink::WebCryptoKey public_key = blink::WebCryptoKey::createNull();
+    ASSERT_EQ(Status::Success(),
+              ImportKeyJwkFromDict(
+                  *jwk.get(),
+                  CreateRsaHashedImportAlgorithm(
+                      blink::WebCryptoAlgorithmIdRsaOaep, test_data.hash_alg),
+                  true,
+                  blink::WebCryptoKeyUsageEncrypt,
+                  &public_key));
+
+    // Now export the key as JWK and verify its contents
+    std::vector<uint8> jwk_data;
+    ASSERT_EQ(Status::Success(),
+              ExportKey(blink::WebCryptoKeyFormatJwk, public_key, &jwk_data));
+    EXPECT_TRUE(VerifyPublicJwk(jwk_data,
+                                test_data.expected_jwk_alg,
+                                kPublicKeyModulusHex,
+                                kPublicKeyExponentHex,
+                                blink::WebCryptoKeyUsageEncrypt));
+  }
+}
+
+TEST_F(SharedCryptoRsaOaepTest, EncryptDecryptKnownAnswerTest) {
+  if (!SupportsRsaOaep()) {
+    LOG(WARNING) << "RSA-OAEP support not present; skipping.";
+    return;
+  }
+
+  scoped_ptr<base::ListValue> tests;
+  ASSERT_TRUE(ReadJsonTestFileToList("rsa_oaep.json", &tests));
+
+  for (size_t test_index = 0; test_index < tests->GetSize(); ++test_index) {
+    SCOPED_TRACE(test_index);
+
+    base::DictionaryValue* test = NULL;
+    ASSERT_TRUE(tests->GetDictionary(test_index, &test));
+
+    blink::WebCryptoAlgorithm digest_algorithm =
+        GetDigestAlgorithm(test, "hash");
+    ASSERT_FALSE(digest_algorithm.isNull());
+    std::vector<uint8> public_key_der =
+        GetBytesFromHexString(test, "public_key");
+    std::vector<uint8> private_key_der =
+        GetBytesFromHexString(test, "private_key");
+    std::vector<uint8> ciphertext = GetBytesFromHexString(test, "ciphertext");
+    std::vector<uint8> plaintext = GetBytesFromHexString(test, "plaintext");
+    std::vector<uint8> label = GetBytesFromHexString(test, "label");
+
+    blink::WebCryptoAlgorithm import_algorithm = CreateRsaHashedImportAlgorithm(
+        blink::WebCryptoAlgorithmIdRsaOaep, digest_algorithm.id());
+    blink::WebCryptoKey public_key = blink::WebCryptoKey::createNull();
+    blink::WebCryptoKey private_key = blink::WebCryptoKey::createNull();
+
+    ASSERT_NO_FATAL_FAILURE(ImportRsaKeyPair(
+        public_key_der,
+        private_key_der,
+        import_algorithm,
+        false,
+        blink::WebCryptoKeyUsageEncrypt | blink::WebCryptoKeyUsageDecrypt,
+        &public_key,
+        &private_key));
+
+    blink::WebCryptoAlgorithm op_algorithm = CreateRsaOaepAlgorithm(label);
+    std::vector<uint8> decrypted_data;
+    ASSERT_EQ(Status::Success(),
+              Decrypt(op_algorithm,
+                      private_key,
+                      CryptoData(ciphertext),
+                      &decrypted_data));
+    EXPECT_BYTES_EQ(plaintext, decrypted_data);
+    std::vector<uint8> encrypted_data;
+    ASSERT_EQ(
+        Status::Success(),
+        Encrypt(
+            op_algorithm, public_key, CryptoData(plaintext), &encrypted_data));
+    std::vector<uint8> redecrypted_data;
+    ASSERT_EQ(Status::Success(),
+              Decrypt(op_algorithm,
+                      private_key,
+                      CryptoData(encrypted_data),
+                      &redecrypted_data));
+    EXPECT_BYTES_EQ(plaintext, redecrypted_data);
+  }
+}
+
+TEST_F(SharedCryptoRsaOaepTest, EncryptWithLargeMessageFails) {
+  if (!SupportsRsaOaep()) {
+    LOG(WARNING) << "RSA-OAEP support not present; skipping.";
+    return;
+  }
+
+  FAIL();

[eroman, 2014/05/16 20:29:51]

Wut? Either fill these out, make them into TODOs, or TODO+bug.

+}
+
+TEST_F(SharedCryptoRsaOaepTest, DecryptWithLargeMessageFails) {
+  if (!SupportsRsaOaep()) {
+    LOG(WARNING) << "RSA-OAEP support not present; skipping.";
+    return;
+  }
+
+  FAIL();
+}
+
+TEST_F(SharedCryptoRsaOaepTest, DecryptWithCorruptedHashFails) {
+  if (!SupportsRsaOaep()) {
+    LOG(WARNING) << "RSA-OAEP support not present; skipping.";
+    return;
+  }
+
+  FAIL();
+}
+
+TEST_F(SharedCryptoRsaOaepTest, DecryptWithCorruptedPaddingFails) {
+  if (!SupportsRsaOaep()) {
+    LOG(WARNING) << "RSA-OAEP support not present; skipping.";
+    return;
+  }
+
+  FAIL();
+}
+
+TEST_F(SharedCryptoRsaOaepTest, WrapUnwrapRawKey) {
+  if (!SupportsRsaOaep()) {
+    LOG(WARNING) << "RSA-OAEP support not present; skipping.";
+    return;
+  }
+
+  FAIL();
+}
+
+TEST_F(SharedCryptoRsaOaepTest, WrapUnwrapJwkSymKey) {
+  if (!SupportsRsaOaep()) {
+    LOG(WARNING) << "RSA-OAEP support not present; skipping.";
+    return;
+  }
+
+  FAIL();
+}
+
+TEST_F(SharedCryptoRsaOaepTest, WrapWithLargeKeyFails) {
+  if (!SupportsRsaOaep()) {
+    LOG(WARNING) << "RSA-OAEP support not present; skipping.";
+    return;
+  }
+
+  FAIL();
+}
+
+TEST_F(SharedCryptoRsaOaepTest, UnwrapWithCorruptedKeyFails) {
+  if (!SupportsRsaOaep()) {
+    LOG(WARNING) << "RSA-OAEP support not present; skipping.";
+    return;
+  }
+
+  FAIL();
+}
+
+TEST_F(SharedCryptoRsaOaepTest, UnwrapWithInvalidKeyFails) {
+  if (!SupportsRsaOaep()) {
+    LOG(WARNING) << "RSA-OAEP support not present; skipping.";
+    return;
+  }
+
+  FAIL();
+}
+
 }  // namespace webcrypto
 
 }  // namespace content