Implement VerifySignedData() for ECDSA, RSA PKCS#1 and RSA PSS.

net/cert/internal/verify_signed_data_unittest.cc

+// Copyright 2015 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 "net/cert/internal/verify_signed_data.h"
+
+#include "base/base_paths.h"
+#include "base/files/file_util.h"
+#include "base/path_service.h"
+#include "net/cert/internal/signature_algorithm.h"
+#include "net/cert/pem_tokenizer.h"
+#include "net/der/input.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace net {
+
+namespace {
+
+// Reads a signature verification test file.
+//
+// The test file is a series of PEM blocks (PEM is just base64 data) with
+// headings of:
+//
+//    "PUBLIC KEY" - DER encoding of the SubjectPublicKeyInfo
+//    "ALGORITHM" - DER encoding of the AlgorithmIdentifier for the signature
+//                  algorithm (signatureAlgorithm in x509)

[Ryan Sleevi, 2015/07/07 14:07:31]

s/x509/X.509/

(and throughout)

[eroman, 2015/07/07 18:07:00]

Done.

+//    "DATA" - The data that was signed (tbsCertificate in x509)
+//    "SIGNATURE" - The result of signing DATA.
+::testing::AssertionResult ParseTestDataFile(const std::string& file_data,
+                                             std::string* public_key,
+                                             std::string* algorithm,
+                                             std::string* signed_data,
+                                             std::string* signature_value) {
+  const char kPublicKeyBlock[] = "PUBLIC KEY";
+  const char kAlgorithmBlock[] = "ALGORITHM";
+  const char kSignedDataBlock[] = "DATA";
+  const char kSignatureBlock[] = "SIGNATURE";
+
+  std::vector<std::string> pem_headers;
+  pem_headers.push_back(kPublicKeyBlock);
+  pem_headers.push_back(kAlgorithmBlock);
+  pem_headers.push_back(kSignedDataBlock);
+  pem_headers.push_back(kSignatureBlock);
+
+  PEMTokenizer pem_tok(file_data, pem_headers);

[Ryan Sleevi, 2015/07/07 14:07:31]

nit: s/pem_tok/pem_tokenizer/ - the tok abbreviation is non-obvious

[eroman, 2015/07/07 18:07:00]

Done.

+  for (size_t cur_index = 0; pem_tok.GetNext(); cur_index++) {

[Ryan Sleevi, 2015/07/07 14:07:31]

|cur_index| is unused. Why not

while (pem_tok.GetNext()) {

}

[eroman, 2015/07/07 18:07:00]

Fixed (was a left-over from older version when I cared about the index).

+    if (pem_tok.block_type() == kPublicKeyBlock) {
+      public_key->assign(pem_tok.data());
+    } else if (pem_tok.block_type() == kAlgorithmBlock) {
+      algorithm->assign(pem_tok.data());
+    } else if (pem_tok.block_type() == kSignedDataBlock) {
+      signed_data->assign(pem_tok.data());
+    } else if (pem_tok.block_type() == kSignatureBlock) {
+      signature_value->assign(pem_tok.data());
+    }
+  }
+
+  return ::testing::AssertionSuccess();
+}
+
+// Returns a path to the file |file_name| within the unittest data directory.
+base::FilePath GetTestFilePath(const char* file_name) {
+  base::FilePath src_root;
+  PathService::Get(base::DIR_SOURCE_ROOT, &src_root);
+  return src_root.Append(
+                      FILE_PATH_LITERAL("net/data/verify_signed_data_unittest"))
+      .AppendASCII(file_name);
+}
+
+enum VerifyResult {
+  SUCCESS,
+  FAILURE,
+};
+
+// Reads test data from |file_name| and runs VerifySignedData() over its inputs.
+//
+// If expected_result was SUCCESS then the test will only succeed if
+// VerifySignedData() returns true.
+//
+// If expected_result was FAILURE then the test will only succeed if
+// VerifySignedData() returns false.
+void RunTestCase(VerifyResult expected_result, const char* file_name) {
+#if !defined(USE_OPENSSL)
+  LOG(INFO) << "Skipping test, only implemented for BoringSSL";
+  return;
+#endif
+
+  base::FilePath test_file_path = GetTestFilePath(file_name);
+
+  std::string file_data;
+  if (!base::ReadFileToString(test_file_path, &file_data)) {

[Ryan Sleevi, 2015/07/07 14:07:31]

Why not ASSERT_TRUE(base::ReadFileToString(...)) << "Couldn't read file: " <<
test_file_path.value() ?

[eroman, 2015/07/07 18:07:00]

Done.

(Left-over from when RunTestCase used to have a return value)

+    FAIL() << "Couldn't read file: " << test_file_path.value();
+    return;
+  }
+
+  std::string public_key;
+  std::string algorithm;
+  std::string signed_data;
+  std::string signature_value;
+
+  ASSERT_TRUE(ParseTestDataFile(file_data, &public_key, &algorithm,
+                                &signed_data, &signature_value));
+
+  SignatureAlgorithm signature_algorithm;
+  ASSERT_TRUE(signature_algorithm.ParseDer(der::Input(algorithm)));
+
+  bool expected_result_bool = expected_result == SUCCESS;
+
+  ASSERT_EQ(
+      expected_result_bool,
+      VerifySignedData(signature_algorithm, der::Input(signed_data),
+                       der::Input(signature_value), der::Input(public_key)));
+}
+
+// Read the descriptions in the test files themselves for details on what is
+// being tested.
+
+TEST(VerifySignedDataTest, RsaPkcs1_Sha1) {
+  RunTestCase(SUCCESS, "rsa-pkcs1-sha1.pem");

[Ryan Sleevi, 2015/07/07 14:07:31]

In looking at the enum & such, why not do

EXPECT_TRUE(RunTestCase(...));
EXPECT_FALSE(RunTestCase(...));

?

Is it so you can use the Assert macros?

[eroman, 2015/07/07 18:07:00]

I experimented with this earlier, but found it had some downsides:

 (1) It complicates how tests are disabled. Currently this unit-test is compiled
in all configurations, however the tests only pass on builds using BoringSSL.
Right now this is accomplished by putting an early return in RunTestCase(). This
approach no longer works when the return value of RunTestCase() is changed to a
bool that is asserted by the test.

(2) It slightly weakens the error output on failures. The line number shown is
now the EXPECT_TRUE(RunTestCase()) one rather than the more explicit line. This
can be solved by returning an assertion result.

(3) Makes code more verbose by not being able to use ASSERTions.

(4) More bug prone. For instance, if when failing to read the test file
RunTestCase() were to simply return false, a test for failed signature matching
might ASSERT_FALSE() and end up passing. The reason for the failure however is
not the right one (failed running the test, as opposed to verification failed).
This is easily solved by adding an ADD_FAILURE(), but it does suggest that a
boolean return value can be ambiguous.

In the near future I expect we will switch to using error codes rather than
booleans (or at least some interface for outputting the error details). At which
point the tests will need to be adjusted to check the actual error information,
and we can re-visit the approach.

+}
+
+TEST(VerifySignedDataTest, RsaPkcs1_Sha256) {
+  RunTestCase(SUCCESS, "rsa-pkcs1-sha256.pem");
+}
+
+TEST(VerifySignedDataTest, Ecdsa_Secp384r1_Sha256) {
+  RunTestCase(SUCCESS, "ecdsa-secp384r1-sha256.pem");
+}
+
+TEST(VerifySignedDataTest, Ecdsa_prime256v1_Sha512) {
+  RunTestCase(SUCCESS, "ecdsa-prime256v1-sha512.pem");
+}
+
+TEST(VerifySignedDataTest, RsaPss_Sha1) {
+  RunTestCase(SUCCESS, "rsa-pss-sha1-salt20.pem");
+}
+
+TEST(VerifySignedDataTest, RsaPss_Sha256) {
+  RunTestCase(SUCCESS, "rsa-pss-sha256-salt10.pem");
+}
+
+TEST(VerifySignedDataTest, RsaPss_Sha1_WrongSalt) {
+  RunTestCase(FAILURE, "rsa-pss-sha1-wrong-salt.pem");
+}
+
+TEST(VerifySignedDataTest, Ecdsa_Secp384r1_Sha256_CorruptedData) {
+  RunTestCase(FAILURE, "ecdsa-secp384r1-sha256-corrupted-data.pem");
+}
+
+TEST(VerifySignedDataTest, RsaPkcs1_Sha1_WrongAlgorithm) {
+  RunTestCase(FAILURE, "rsa-pkcs1-sha1-wrong-algorithm.pem");
+}
+
+TEST(VerifySignedDataTest, Ecdsa_prime256v1_Sha512_WrongSignatureFormat) {
+  RunTestCase(FAILURE, "ecdsa-prime256v1-sha512-wrong-signature-format.pem");
+}
+
+TEST(VerifySignedDataTest, EcdsaUsingRsaKey) {
+  RunTestCase(FAILURE, "ecdsa-using-rsa-key.pem");
+}
+
+TEST(VerifySignedDataTest, RsaUsingEcKey) {
+  RunTestCase(FAILURE, "rsa-using-ec-key.pem");
+}
+
+TEST(VerifySignedDataTest, RsaPkcs1_Sha1_BadKeyDerNull) {
+  RunTestCase(FAILURE, "rsa-pkcs1-sha1-bad-key-der-null.pem");
+}
+
+TEST(VerifySignedDataTest, RsaPkcs1_Sha1_BadKeyDerLength) {
+  RunTestCase(FAILURE, "rsa-pkcs1-sha1-bad-key-der-length.pem");
+}
+
+}  // namespace
+
+}  // namespace net