[webcrypto] Set the error type for failures.

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>
 
 #include "base/basictypes.h"
 #include "base/file_util.h"
 #include "base/json/json_reader.h"
 #include "base/json/json_writer.h"
 #include "base/logging.h"
 #include "base/memory/ref_counted.h"
 #include "base/path_service.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_util.h"
+#include "base/strings/stringprintf.h"
 #include "content/child/webcrypto/crypto_data.h"
 #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/WebArrayBuffer.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"
 
 // 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
 
 // Helper macros to verify the value of a Status.
 #define EXPECT_STATUS_ERROR(code) EXPECT_FALSE((code).IsSuccess())
 #define EXPECT_STATUS(expected, code) \
-  EXPECT_EQ(expected.ToString(), (code).ToString())
+  EXPECT_EQ(StatusToString(expected), StatusToString(code))
 #define ASSERT_STATUS(expected, code) \
-  ASSERT_EQ(expected.ToString(), (code).ToString())
+  ASSERT_EQ(StatusToString(expected), StatusToString(code))
 #define EXPECT_STATUS_SUCCESS(code) EXPECT_STATUS(Status::Success(), code)
 #define ASSERT_STATUS_SUCCESS(code) ASSERT_STATUS(Status::Success(), code)
 
 namespace content {
 
 namespace webcrypto {
 
 namespace {
 
+std::string StatusToString(const Status& status) {
+  if (status.IsSuccess())
+    return "Success";
+  return base::StringPrintf("error_type=%d: %s, error_details=",
+                            status.error_type(),
+                            status.error_details().c_str());
+}

[Ryan Sleevi, 2014/04/24 01:17:01]

This isn't really idiomatic GTest to do this.

The GTest friendly way -

bool StatusAreEqual(const Status& a, const Status& b) {
  if (a.IsSuccess() != b.IsSuccess())
    return false;
  if (a.IsSuccess())
    return true;
  return a.error_type() == b.error_type() && a.error_details ==
b.error_details();
}

void PrintTo(const Status& status, ::std::ostream* os) {
  if (status.IsSuccess())
    *os << "Success";
  else
    *os << "Error type: " << status.error_type() << " Error details: " <<
status.error_details();
}


And then
EXPECT_PRED_FORMAT2(StatusAreEqual, Status::DataError(), status);


If you decide not to use the GTest/GMock friendly comparators, then you've at
least got your format string wrong. Presumably you meant to write
"error_type=%d, error_details=%s" - otherwise, the error_details= seems
superfluous

[eroman, 2014/04/24 02:34:37]

I addressed this by changing the code to using EXPECT_EQ(), ASSERT_EQ() directly
on Status objects. And defining an operator==, operator!=, and PrintTo() for it
in the test file.

+
 // TODO(eroman): For Linux builds using system NSS, AES-GCM support is a
 // runtime dependency. Test it by trying to import a key.
 // TODO(padolph): Consider caching the result of the import key test.
 bool SupportsAesGcm() {
   std::vector<uint8> key_raw(16, 0);
 
   blink::WebCryptoKey key = blink::WebCryptoKey::createNull();
   Status status = ImportKey(blink::WebCryptoKeyFormatRaw,
                             CryptoData(key_raw),
                             CreateAlgorithm(blink::WebCryptoAlgorithmIdAesGcm),
@@ skipping 407 matching lines @@
   blink::WebCryptoAlgorithm algorithm =
       CreateAesGcmAlgorithm(iv, additional_data, tag_length_bits);
 
   blink::WebArrayBuffer output;
   Status status = Encrypt(algorithm, key, CryptoData(plain_text), &output);
   if (status.IsError())
     return status;
 
   if (output.byteLength() * 8 < tag_length_bits) {
     EXPECT_TRUE(false);
-    return Status::Error();
+    return Status::OperationError();
   }
 
   // The encryption result is cipher text with authentication tag appended.
   cipher_text->assign(static_cast<uint8*>(output.data()),
                       static_cast<uint8*>(output.data()) +
                           (output.byteLength() - tag_length_bits / 8));
   authentication_tag->assign(
       static_cast<uint8*>(output.data()) + cipher_text->size(),
       static_cast<uint8*>(output.data()) + output.byteLength());
 
@@ skipping 182 matching lines @@
 }  // namespace
 
 TEST_F(SharedCryptoTest, CheckAesGcm) {
   if (!SupportsAesGcm()) {
     LOG(WARNING) << "AES GCM not supported on this platform, so some tests "
                     "will be skipped. Consider upgrading local NSS libraries";
     return;
   }
 }
 
-TEST_F(SharedCryptoTest, StatusToString) {
-  EXPECT_EQ("Success", Status::Success().ToString());
-  EXPECT_EQ("", Status::Error().ToString());
-  EXPECT_EQ("The requested operation is unsupported",
-            Status::ErrorUnsupported().ToString());
+TEST_F(SharedCryptoTest, Status) {

[Ryan Sleevi, 2014/04/24 01:17:01]

It's not clear what you're trying to test by this test - what the success or
failure conditions are, or why it's necessary.

Please add a comment to this test to indicate what you're testing, and then
comments to each of these sections where you're doing status = "" to indicate
what you're testing.

The way you have it structured now, it seems you're just testing data identical
to what you have hardcoded, and that makes no sense.

[eroman, 2014/04/24 02:34:37]

The relevant distinction is to make sure that errors with no details, but a
different error type, do not compare as equal.

In other words, Status::OperationError() != Status::DataError() !=
Status::Success().

The intent of these tests is to verify that the comparison of errors works. If
it doesn't, then all of the assertions on the error earlier in this test file
mean very little.

Added a comment. Also now that it is testing the equality operator things are
clearer.

+  Status status = Status::Success();
+
+  EXPECT_EQ(false, status.IsError());
+  EXPECT_EQ("", status.error_details());
+
+  status = Status::OperationError();
+  EXPECT_EQ(true, status.IsError());
+  EXPECT_EQ("", status.error_details());
+  EXPECT_EQ(blink::WebCryptoErrorTypeOperation, status.error_type());
+
+  status = Status::DataError();
+  EXPECT_EQ(true, status.IsError());
+  EXPECT_EQ("", status.error_details());
+  EXPECT_EQ(blink::WebCryptoErrorTypeData, Status::DataError().error_type());
+
+  // Even though the error message is the same, these should not be considered
+  // the same by the tests.

[Ryan Sleevi, 2014/04/24 01:17:01]

This doesn't make any sense. Is "error message" the output of StatusToString? If
so, you're testing exactly the opposite of what you say. If not, it's ambiguous.

[eroman, 2014/04/24 02:34:37]

error message, versus error type.

+  EXPECT_NE(StatusToString(Status::DataError()),
+            StatusToString(Status::OperationError()));
+
+  status = Status::ErrorUnsupported();
+  EXPECT_EQ(true, status.IsError());
+  EXPECT_EQ("The requested operation is unsupported", status.error_details());
+  EXPECT_EQ(blink::WebCryptoErrorTypeNotSupported, status.error_type());
+
+  status = Status::ErrorJwkPropertyMissing("kty");
+  EXPECT_EQ(true, status.IsError());
   EXPECT_EQ("The required JWK property \"kty\" was missing",
-            Status::ErrorJwkPropertyMissing("kty").ToString());
+            status.error_details());
+  EXPECT_EQ(blink::WebCryptoErrorTypeData, Status::DataError().error_type());
+
+  status = Status::ErrorJwkPropertyWrongType("kty", "string");
+  EXPECT_EQ(true, status.IsError());
   EXPECT_EQ("The JWK property \"kty\" must be a string",
-            Status::ErrorJwkPropertyWrongType("kty", "string").ToString());
+            status.error_details());
+  EXPECT_EQ(blink::WebCryptoErrorTypeData, Status::DataError().error_type());
+
+  status = Status::ErrorJwkBase64Decode("n");
+  EXPECT_EQ(true, status.IsError());
   EXPECT_EQ("The JWK property \"n\" could not be base64 decoded",
-            Status::ErrorJwkBase64Decode("n").ToString());
+            status.error_details());
+  EXPECT_EQ(blink::WebCryptoErrorTypeData, Status::DataError().error_type());
 }
 
 TEST_F(SharedCryptoTest, DigestSampleSets) {
   scoped_ptr<base::ListValue> tests;
   ASSERT_TRUE(ReadJsonTestFileToList("digest.json", &tests));
 
   for (size_t test_index = 0; test_index < tests->GetSize(); ++test_index) {
     SCOPED_TRACE(test_index);
     base::DictionaryValue* test;
     ASSERT_TRUE(tests->GetDictionary(test_index, &test));
@@ skipping 189 matching lines @@
     EXPECT_STATUS(Status::ErrorDataTooLarge(),
                   Decrypt(CreateAesCbcAlgorithm(iv), key, input, &output));
   }
 
   // Fail importing the key (too few bytes specified)
   {
     std::vector<uint8> key_raw(1);
     std::vector<uint8> iv(16);
 
     blink::WebCryptoKey key = blink::WebCryptoKey::createNull();
-    EXPECT_STATUS(Status::Error(),
+    EXPECT_STATUS(Status::ErrorImportAesKeyLength(),
                   ImportKey(blink::WebCryptoKeyFormatRaw,
                             CryptoData(key_raw),
                             CreateAesCbcAlgorithm(iv),
                             true,
                             blink::WebCryptoKeyUsageEncrypt,
                             &key));
   }
 
   // Fail exporting the key in SPKI and PKCS#8 formats (not allowed for secret
   // keys).
@@ skipping 49 matching lines @@
                           CryptoData(test_cipher_text),
                           &output));
     EXPECT_TRUE(ArrayBufferMatches(test_plain_text, output));
 
     const unsigned int kAesCbcBlockSize = 16;
 
     // Decrypt with a padding error by stripping the last block. This also ends
     // up testing decryption over empty cipher text.
     if (test_cipher_text.size() >= kAesCbcBlockSize) {
       EXPECT_STATUS(
-          Status::Error(),
+          Status::OperationError(),
           Decrypt(CreateAesCbcAlgorithm(test_iv),
                   key,
                   CryptoData(&test_cipher_text[0],
                              test_cipher_text.size() - kAesCbcBlockSize),
                   &output));
     }
 
     // Decrypt cipher text which is not a multiple of block size by stripping
     // a few bytes off the cipher text.
     if (test_cipher_text.size() > 3) {
       EXPECT_STATUS(
-          Status::Error(),
+          Status::OperationError(),
           Decrypt(CreateAesCbcAlgorithm(test_iv),
                   key,
                   CryptoData(&test_cipher_text[0], test_cipher_text.size() - 3),
                   &output));
     }
   }
 }
 
 TEST_F(SharedCryptoTest, MAYBE(GenerateKeyAes)) {
   // Check key generation for each of AES-CBC, AES-GCM, and AES-KW, and for each
@@ skipping 817 matching lines @@
       Status::ErrorImportEmptyKeyData(),
       ImportKey(blink::WebCryptoKeyFormatSpki,
                 CryptoData(std::vector<uint8>()),
                 CreateAlgorithm(blink::WebCryptoAlgorithmIdRsaEsPkcs1v1_5),
                 true,
                 blink::WebCryptoKeyUsageEncrypt,
                 &key));
 
   // Failing case: Bad DER encoding.
   EXPECT_STATUS(
-      Status::Error(),
+      Status::DataError(),
       ImportKey(blink::WebCryptoKeyFormatSpki,
                 CryptoData(HexStringToBytes("618333c4cb")),
                 CreateAlgorithm(blink::WebCryptoAlgorithmIdRsaEsPkcs1v1_5),
                 true,
                 blink::WebCryptoKeyUsageEncrypt,
                 &key));
 
   // Failing case: Import RSA key but provide an inconsistent input algorithm.
-  EXPECT_STATUS(Status::Error(),
+  EXPECT_STATUS(Status::DataError(),
                 ImportKey(blink::WebCryptoKeyFormatSpki,
                           CryptoData(HexStringToBytes(kPublicKeySpkiDerHex)),
                           CreateAlgorithm(blink::WebCryptoAlgorithmIdAesCbc),
                           true,
                           blink::WebCryptoKeyUsageEncrypt,
                           &key));
 
   // Passing case: Export a previously imported RSA public key in SPKI format
   // and compare to original data.
   blink::WebArrayBuffer output;
@@ skipping 53 matching lines @@
                           CryptoData(std::vector<uint8>()),
                           CreateRsaHashedImportAlgorithm(
                               blink::WebCryptoAlgorithmIdRsaSsaPkcs1v1_5,
                               blink::WebCryptoAlgorithmIdSha1),
                           true,
                           blink::WebCryptoKeyUsageSign,
                           &key));
 
   // Failing case: Bad DER encoding.
   EXPECT_STATUS(
-      Status::Error(),
+      Status::DataError(),
       ImportKey(blink::WebCryptoKeyFormatPkcs8,
                 CryptoData(HexStringToBytes("618333c4cb")),
                 CreateAlgorithm(blink::WebCryptoAlgorithmIdRsaSsaPkcs1v1_5),
                 true,
                 blink::WebCryptoKeyUsageSign,
                 &key));
 
   // Failing case: Import RSA key but provide an inconsistent input algorithm.
-  EXPECT_STATUS(Status::Error(),
+  EXPECT_STATUS(Status::DataError(),
                 ImportKey(blink::WebCryptoKeyFormatPkcs8,
                           CryptoData(HexStringToBytes(kPrivateKeyPkcs8DerHex)),
                           CreateAlgorithm(blink::WebCryptoAlgorithmIdAesCbc),
                           true,
                           blink::WebCryptoKeyUsageSign,
                           &key));
 }
 
 TEST_F(SharedCryptoTest, MAYBE(GenerateKeyPairRsa)) {
   // Note: using unrealistic short key lengths here to avoid bogging down tests.
@@ skipping 294 matching lines @@
   // Fail encrypt with a private key.
   blink::WebArrayBuffer encrypted_data;
   const std::string message_hex_str("0102030405060708090a0b0c0d0e0f");
   const std::vector<uint8> message_hex(HexStringToBytes(message_hex_str));
   EXPECT_STATUS(
       Status::ErrorUnexpectedKeyType(),
       Encrypt(
           algorithm, private_key, CryptoData(message_hex), &encrypted_data));
 
   // Fail encrypt with empty message.
-  EXPECT_STATUS(Status::Error(),
+  EXPECT_STATUS(Status::ErrorDataTooSmall(),
                 Encrypt(algorithm,
                         public_key,
                         CryptoData(std::vector<uint8>()),
                         &encrypted_data));
 
   // Fail encrypt with message too large. 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;
   EXPECT_STATUS(
       Status::ErrorDataTooLarge(),
@@ skipping 13 matching lines @@
       Status::ErrorUnexpectedKeyType(),
       Decrypt(
           algorithm, public_key, CryptoData(encrypted_data), &decrypted_data));
 
   // Corrupt encrypted data; ensure decrypt fails because padding was disrupted.
   std::vector<uint8> corrupted_data(
       static_cast<uint8*>(encrypted_data.data()),
       static_cast<uint8*>(encrypted_data.data()) + encrypted_data.byteLength());
   corrupted_data[corrupted_data.size() / 2] ^= 0x01;
   EXPECT_STATUS(
-      Status::Error(),
+      Status::OperationError(),
       Decrypt(
           algorithm, private_key, CryptoData(corrupted_data), &decrypted_data));
 
   // TODO(padolph): Are there other specific data corruption scenarios to
   // consider?
 
   // Do a successful decrypt with good data just for confirmation.
   EXPECT_STATUS_SUCCESS(Decrypt(
       algorithm, private_key, CryptoData(encrypted_data), &decrypted_data));
   ExpectArrayBufferMatchesHex(message_hex_str, decrypted_data);
@@ skipping 223 matching lines @@
                                   CryptoData(HexStringToBytes(key_raw_hex_in)),
                                   algorithm,
                                   true,
                                   blink::WebCryptoKeyUsageWrapKey,
                                   &key));
   EXPECT_STATUS_SUCCESS(
       ExportKey(blink::WebCryptoKeyFormatRaw, key, &key_raw_out));
   ExpectArrayBufferMatchesHex(key_raw_hex_in, key_raw_out);
 
   // Fail import of 0 length key
-  EXPECT_STATUS(Status::Error(),
+  EXPECT_STATUS(Status::ErrorImportAesKeyLength(),
                 ImportKey(blink::WebCryptoKeyFormatRaw,
                           CryptoData(HexStringToBytes("")),
                           algorithm,
                           true,
                           blink::WebCryptoKeyUsageWrapKey,
                           &key));
 
   // Fail import of 124-bit KEK
   key_raw_hex_in = "3e4566a2bdaa10cb68134fa66c15ddb";
-  EXPECT_STATUS(Status::Error(),
+  EXPECT_STATUS(Status::ErrorImportAesKeyLength(),
                 ImportKey(blink::WebCryptoKeyFormatRaw,
                           CryptoData(HexStringToBytes(key_raw_hex_in)),
                           algorithm,
                           true,
                           blink::WebCryptoKeyUsageWrapKey,
                           &key));
 
   // Fail import of 200-bit KEK
   key_raw_hex_in = "0a1d88608a5ad9fec64f1ada269ebab4baa2feeb8d95638c0e";
-  EXPECT_STATUS(Status::Error(),
+  EXPECT_STATUS(Status::ErrorImportAesKeyLength(),
                 ImportKey(blink::WebCryptoKeyFormatRaw,
                           CryptoData(HexStringToBytes(key_raw_hex_in)),
                           algorithm,
                           true,
                           blink::WebCryptoKeyUsageWrapKey,
                           &key));
 
   // Fail import of 260-bit KEK
   key_raw_hex_in =
       "72d4e475ff34215416c9ad9c8281247a4d730c5f275ac23f376e73e3bce8d7d5a";
-  EXPECT_STATUS(Status::Error(),
+  EXPECT_STATUS(Status::ErrorImportAesKeyLength(),
                 ImportKey(blink::WebCryptoKeyFormatRaw,
                           CryptoData(HexStringToBytes(key_raw_hex_in)),
                           algorithm,
                           true,
                           blink::WebCryptoKeyUsageWrapKey,
                           &key));
 }
 
 TEST_F(SharedCryptoTest, MAYBE(UnwrapFailures)) {
   // This test exercises the code path common to all unwrap operations.
@@ skipping 173 matching lines @@
   // Import the wrapping key.
   blink::WebCryptoKey wrapping_key = ImportSecretKeyFromRaw(
       test_kek,
       wrapping_algorithm,
       blink::WebCryptoKeyUsageWrapKey | blink::WebCryptoKeyUsageUnwrapKey);
 
   // Unwrap of a corrupted version of the known ciphertext should fail, due to
   // AES-KW's built-in integrity check.
   blink::WebCryptoKey unwrapped_key = blink::WebCryptoKey::createNull();
   EXPECT_STATUS(
-      Status::Error(),
+      Status::OperationError(),
       UnwrapKey(blink::WebCryptoKeyFormatRaw,
                 CryptoData(Corrupted(test_ciphertext)),
                 wrapping_key,
                 wrapping_algorithm,
                 webcrypto::CreateAlgorithm(blink::WebCryptoAlgorithmIdAesCbc),
                 true,
                 blink::WebCryptoKeyUsageEncrypt,
                 &unwrapped_key));
 }
 
@@ skipping 114 matching lines @@
     EXPECT_STATUS_SUCCESS(AesGcmDecrypt(key,
                                         test_iv,
                                         test_additional_data,
                                         test_tag_size_bits,
                                         test_cipher_text,
                                         test_authentication_tag,
                                         &plain_text));
     EXPECT_TRUE(ArrayBufferMatches(test_plain_text, plain_text));
 
     // Decryption should fail if any of the inputs are tampered with.
-    EXPECT_STATUS(Status::Error(),
+    EXPECT_STATUS(Status::OperationError(),
                   AesGcmDecrypt(key,
                                 Corrupted(test_iv),
                                 test_additional_data,
                                 test_tag_size_bits,
                                 test_cipher_text,
                                 test_authentication_tag,
                                 &plain_text));
-    EXPECT_STATUS(Status::Error(),
+    EXPECT_STATUS(Status::OperationError(),
                   AesGcmDecrypt(key,
                                 test_iv,
                                 Corrupted(test_additional_data),
                                 test_tag_size_bits,
                                 test_cipher_text,
                                 test_authentication_tag,
                                 &plain_text));
-    EXPECT_STATUS(Status::Error(),
+    EXPECT_STATUS(Status::OperationError(),
                   AesGcmDecrypt(key,
                                 test_iv,
                                 test_additional_data,
                                 test_tag_size_bits,
                                 Corrupted(test_cipher_text),
                                 test_authentication_tag,
                                 &plain_text));
-    EXPECT_STATUS(Status::Error(),
+    EXPECT_STATUS(Status::OperationError(),
                   AesGcmDecrypt(key,
                                 test_iv,
                                 test_additional_data,
                                 test_tag_size_bits,
                                 test_cipher_text,
                                 Corrupted(test_authentication_tag),
                                 &plain_text));
 
     // Try different incorrect tag lengths
     uint8 kAlternateTagLengths[] = {0, 8, 96, 120, 128, 160, 255};
@@ skipping 341 matching lines @@
       ImportKey(blink::WebCryptoKeyFormatPkcs8,
                 CryptoData(HexStringToBytes(kPrivateKeyPkcs8DerHex)),
                 algorithm,
                 false,
                 blink::WebCryptoKeyUsageUnwrapKey,
                 &private_wrapping_key));
 
   // Treat the ciphertext as a wrapped key and try to unwrap it. Ensure a
   // generic error is received.
   blink::WebCryptoKey unwrapped_key = blink::WebCryptoKey::createNull();
-  EXPECT_STATUS(Status::Error(),
+  EXPECT_STATUS(Status::OperationError(),
                 UnwrapKey(blink::WebCryptoKeyFormatJwk,
                           CryptoData(ciphertext),
                           private_wrapping_key,
                           algorithm,
                           CreateAesCbcAlgorithm(std::vector<uint8>(0, 16)),
                           true,
                           blink::WebCryptoKeyUsageEncrypt,
                           &unwrapped_key));
 }
 
 }  // namespace webcrypto
 
 }  // namespace content