[WebCrypto] Cleanup in LoadTestKeys() in ecdh unit tests

content/child/webcrypto/test/ecdh_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 "base/stl_util.h"
 #include "content/child/webcrypto/algorithm_dispatch.h"
 #include "content/child/webcrypto/crypto_data.h"
 #include "content/child/webcrypto/jwk.h"
 #include "content/child/webcrypto/status.h"
 #include "content/child/webcrypto/test/test_helpers.h"
@@ skipping 108 matching lines @@
     std::vector<uint8_t> expected_bytes =
         GetBytesFromHexString(test, "derived_bytes");
 
     EXPECT_EQ(CryptoData(expected_bytes), CryptoData(derived_bytes));
   }
 }
 
 // Loads up a test ECDH public and private key for P-521. The keys
 // come from different key pairs, and can be used for key derivation of up to
 // 528 bits.
-void LoadTestKeys(blink::WebCryptoKey* public_key,
-                  blink::WebCryptoKey* private_key) {
-  // Assume that the 7th key in the test data is for P-521.
+::testing::AssertionResult LoadTestKeys(blink::WebCryptoKey* public_key,
+                                        blink::WebCryptoKey* private_key) {
   scoped_ptr<base::ListValue> tests;
-  ASSERT_TRUE(ReadJsonTestFileToList("ecdh.json", &tests));
+  if (!ReadJsonTestFileToList("ecdh.json", &tests))
+    return ::testing::AssertionFailure() << "Failed loading ecdh.json";
 
   const base::DictionaryValue* test;
-  ASSERT_TRUE(tests->GetDictionary(6, &test));
+  bool valid_p521_keys = false;
+  for (size_t test_index = 0; test_index < tests->GetSize(); ++test_index) {
+    SCOPED_TRACE(test_index);
+    EXPECT_TRUE(tests->GetDictionary(test_index, &test));
+    test->GetBoolean("valid_p521_keys", &valid_p521_keys);
+    if (valid_p521_keys)
+      break;
+    else if (!valid_p521_keys && tests->GetSize() == test_index + 1) {

[eroman, 2014/12/30 17:41:48]

This is not a good way to do the test. (For instance it will not return a
failure if tests->GetSize() == 0)

Instead move this outside of the "for" loop:

if (!valid_p521_keys) {
  return ::testing::AssertionFailure() << ....;
}

[Habib Virji, 2014/12/31 09:38:24]

Done.

+      return ::testing::AssertionFailure()
+             << "The P-521 test are missing in ecdh.json";
+    }
+  }
 
   ImportKeysFromTest(test, public_key, private_key);
 
-  ASSERT_EQ(blink::WebCryptoNamedCurveP521,
+  EXPECT_EQ(blink::WebCryptoNamedCurveP521,
             public_key->algorithm().ecParams()->namedCurve());
+
+  return ::testing::AssertionSuccess();
 }
 
 // Try deriving an AES key of length 129 bits.
 TEST(WebCryptoEcdhTest, DeriveKeyBadAesLength) {
   if (!SupportsEcdh())
     return;
 
   blink::WebCryptoKey public_key;
   blink::WebCryptoKey base_key;
-  LoadTestKeys(&public_key, &base_key);
+  ASSERT_TRUE(LoadTestKeys(&public_key, &base_key));
 
   blink::WebCryptoKey derived_key;
 
   ASSERT_EQ(Status::ErrorGetAesKeyLength(),
             DeriveKey(CreateEcdhDeriveParams(public_key), base_key,
                       CreateAlgorithm(blink::WebCryptoAlgorithmIdAesGcm),
                       CreateAesGcmDerivedKeyParams(129), true,
                       blink::WebCryptoKeyUsageEncrypt, &derived_key));
 }
 
 // Try deriving an AES key of length 192 bits.
 TEST(WebCryptoEcdhTest, DeriveKeyUnsupportedAesLength) {
   if (!SupportsEcdh())
     return;
 
   blink::WebCryptoKey public_key;
   blink::WebCryptoKey base_key;
-  LoadTestKeys(&public_key, &base_key);
+  ASSERT_TRUE(LoadTestKeys(&public_key, &base_key));
 
   blink::WebCryptoKey derived_key;
 
   ASSERT_EQ(Status::ErrorAes192BitUnsupported(),
             DeriveKey(CreateEcdhDeriveParams(public_key), base_key,
                       CreateAlgorithm(blink::WebCryptoAlgorithmIdAesGcm),
                       CreateAesGcmDerivedKeyParams(192), true,
                       blink::WebCryptoKeyUsageEncrypt, &derived_key));
 }
 
 // Try deriving an HMAC key of length 0 bits.
 TEST(WebCryptoEcdhTest, DeriveKeyZeroLengthHmac) {
   if (!SupportsEcdh())
     return;
 
   blink::WebCryptoKey public_key;
   blink::WebCryptoKey base_key;
-  LoadTestKeys(&public_key, &base_key);
+  ASSERT_TRUE(LoadTestKeys(&public_key, &base_key));
 
   blink::WebCryptoKey derived_key;
 
   const blink::WebCryptoAlgorithm import_algorithm =
       CreateHmacImportAlgorithm(blink::WebCryptoAlgorithmIdSha1, 0);
 
   ASSERT_EQ(Status::ErrorGetHmacKeyLengthZero(),
             DeriveKey(CreateEcdhDeriveParams(public_key), base_key,
                       import_algorithm, import_algorithm, true,
                       blink::WebCryptoKeyUsageSign, &derived_key));
 }
 
 // Derive an HMAC key of length 19 bits.
 TEST(WebCryptoEcdhTest, DeriveKeyHmac19Bits) {
   if (!SupportsEcdh())
     return;
 
   blink::WebCryptoKey public_key;
   blink::WebCryptoKey base_key;
-  LoadTestKeys(&public_key, &base_key);
+  ASSERT_TRUE(LoadTestKeys(&public_key, &base_key));
 
   blink::WebCryptoKey derived_key;
 
   const blink::WebCryptoAlgorithm import_algorithm =
       CreateHmacImportAlgorithm(blink::WebCryptoAlgorithmIdSha1, 19);
 
   ASSERT_EQ(Status::Success(),
             DeriveKey(CreateEcdhDeriveParams(public_key), base_key,
                       import_algorithm, import_algorithm, true,
                       blink::WebCryptoKeyUsageSign, &derived_key));
@@ skipping 12 matching lines @@
   EXPECT_EQ(0, raw_key[raw_key.size() - 1] & 0x1f);
 }
 
 // Derive an HMAC key with no specified length (just the hash of SHA-256).
 TEST(WebCryptoEcdhTest, DeriveKeyHmacSha256NoLength) {
   if (!SupportsEcdh())
     return;
 
   blink::WebCryptoKey public_key;
   blink::WebCryptoKey base_key;
-  LoadTestKeys(&public_key, &base_key);
+  ASSERT_TRUE(LoadTestKeys(&public_key, &base_key));
 
   blink::WebCryptoKey derived_key;
 
   const blink::WebCryptoAlgorithm import_algorithm =
       CreateHmacImportAlgorithmNoLength(blink::WebCryptoAlgorithmIdSha256);
 
   ASSERT_EQ(Status::Success(),
             DeriveKey(CreateEcdhDeriveParams(public_key), base_key,
                       import_algorithm, import_algorithm, true,
                       blink::WebCryptoKeyUsageSign, &derived_key));
@@ skipping 18 matching lines @@
 // In practice, authors won't be directly generating keys from key agreement
 // schemes, as that is frequently insecure, and instead be using KDFs to expand
 // and generate keys. For simplicity of testing, however, test using an HMAC
 // key.
 TEST(WebCryptoEcdhTest, DeriveKeyHmacSha512NoLength) {
   if (!SupportsEcdh())
     return;
 
   blink::WebCryptoKey public_key;
   blink::WebCryptoKey base_key;
-  LoadTestKeys(&public_key, &base_key);
+  ASSERT_TRUE(LoadTestKeys(&public_key, &base_key));
 
   blink::WebCryptoKey derived_key;
 
   const blink::WebCryptoAlgorithm import_algorithm =
       CreateHmacImportAlgorithmNoLength(blink::WebCryptoAlgorithmIdSha512);
 
   ASSERT_EQ(Status::ErrorEcdhLengthTooBig(528),
             DeriveKey(CreateEcdhDeriveParams(public_key), base_key,
                       import_algorithm, import_algorithm, true,
                       blink::WebCryptoKeyUsageSign, &derived_key));
 }
 
 // Try deriving an AES key of length 128 bits.
 TEST(WebCryptoEcdhTest, DeriveKeyAes128) {
   if (!SupportsEcdh())
     return;
 
   blink::WebCryptoKey public_key;
   blink::WebCryptoKey base_key;
-  LoadTestKeys(&public_key, &base_key);
+  ASSERT_TRUE(LoadTestKeys(&public_key, &base_key));
 
   blink::WebCryptoKey derived_key;
 
   ASSERT_EQ(Status::Success(),
             DeriveKey(CreateEcdhDeriveParams(public_key), base_key,
                       CreateAlgorithm(blink::WebCryptoAlgorithmIdAesGcm),
                       CreateAesGcmDerivedKeyParams(128), true,
                       blink::WebCryptoKeyUsageEncrypt, &derived_key));
 
   ASSERT_EQ(blink::WebCryptoAlgorithmIdAesGcm, derived_key.algorithm().id());
@@ skipping 38 matching lines @@
                       CryptoData(MakeJsonVector(*private_key_json)),
                       CreateEcdhImportAlgorithm(curve), true,
                       0, &key));
 }
 
 }  // namespace
 
 }  // namespace webcrypto
 
 }  // namespace content