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 {
+
+// Creates a der::Input from an std::string. The lifetimes are a bit subtle
+// when using this function:
+//
+// The returned der::Input() is only valid so long as the input string is alive
+// and is not mutated.
+//
+// Note that the input parameter has been made a pointer to prevent callers
+// from accidentally passing an r-value.
+der::Input InputFromString(const std::string* s) {
+  return der::Input(reinterpret_cast<const uint8_t*>(s->data()), s->size());
+}
+
+// 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 X.509)
+//    "DATA" - The data that was signed (tbsCertificate in X.509)
+//    "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_tokenizer(file_data, pem_headers);
+  while (pem_tokenizer.GetNext()) {
+    if (pem_tokenizer.block_type() == kPublicKeyBlock) {
+      public_key->assign(pem_tokenizer.data());
+    } else if (pem_tokenizer.block_type() == kAlgorithmBlock) {
+      algorithm->assign(pem_tokenizer.data());
+    } else if (pem_tokenizer.block_type() == kSignedDataBlock) {
+      signed_data->assign(pem_tokenizer.data());
+    } else if (pem_tokenizer.block_type() == kSignatureBlock) {
+      signature_value->assign(pem_tokenizer.data());
+    }
+  }
+
+  return ::testing::AssertionSuccess();

[davidben, 2015/07/21 16:26:27]

This function never fails. (Did you mean for it to return a failure if any of
public_key through signature_value weren't found? Though you then have the usual
"modifies output parameters on failure" thing unless you also use some
temporaries.)

[eroman, 2015/07/21 19:24:28]

Done  -- I additionally made it fail if a block is multiply defined, or never
defined.

+}
+
+// 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;
+  ASSERT_TRUE(base::ReadFileToString(test_file_path, &file_data))
+      << "Couldn't read file: " << test_file_path.value();
+
+  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));
+
+  scoped_ptr<SignatureAlgorithm> signature_algorithm =
+      SignatureAlgorithm::CreateFromDer(InputFromString(&algorithm));
+  ASSERT_TRUE(signature_algorithm);
+
+  bool expected_result_bool = expected_result == SUCCESS;
+
+  ASSERT_EQ(

[davidben, 2015/07/21 16:26:27]

Nit: I think this can just be EXPECT_EQ.

[eroman, 2015/07/21 19:24:29]

Done.

+      expected_result_bool,
+      VerifySignedData(*signature_algorithm, InputFromString(&signed_data),
+                       InputFromString(&signature_value),
+                       InputFromString(&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");
+}
+
+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_Mgf1_Sha512_Salt33) {
+  RunTestCase(SUCCESS, "rsa-pss-sha256-mgf1-sha512-salt33.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");
+}
+
+TEST(VerifySignedDataTest, RsaPkcs1_Sha256_UsingEcdsaAlgorithm) {
+  RunTestCase(FAILURE, "rsa-pkcs1-sha256-using-ecdsa-algorithm.pem");
+}
+
+TEST(VerifySignedDataTest, Ecdsa_Prim256v1_Sha512_UsingRsaAlgorithm) {
+  RunTestCase(FAILURE, "ecdsa-prime256v1-sha512-using-rsa-algorithm.pem");
+}
+
+TEST(VerifySignedDataTest, Ecdsa_Prim256v1_Sha512_UsingEcdhKey) {
+  RunTestCase(FAILURE, "ecdsa-prime256v1-sha512-using-ecdh-key.pem");
+}
+
+TEST(VerifySignedDataTest, Ecdsa_Prim256v1_Sha512_UsingEcmqvKey) {
+  RunTestCase(FAILURE, "ecdsa-prime256v1-sha512-using-ecmqv-key.pem");
+}
+
+TEST(VerifySignedDataTest, RsaPkcs1_Sha1_KeyParamsAbsent) {
+  RunTestCase(SUCCESS, "rsa-pkcs1-sha1-key-params-absent.pem");
+}
+
+// TODO(eroman): Support rsaPss for the key algorithm.
+TEST(VerifySignedDataTest, DISABLED_RsaPss_Sha1_Salt20_UsingPssKeyNoParams) {
+  RunTestCase(SUCCESS, "rsa-pss-sha1-salt20-using-pss-key-no-params.pem");
+}
+
+TEST(VerifySignedDataTest, RsaPkcs1_Sha1_UsingPssKeyNoParams) {
+  RunTestCase(FAILURE, "rsa-pkcs1-sha1-using-pss-key-no-params.pem");
+}
+
+// TODO(eroman): Support rsaPss for the key algorithm.
+TEST(VerifySignedDataTest,
+     DISABLED_RsaPss_Sha256_Salt10_UsingPssKeyWithParams) {

[davidben, 2015/07/21 16:26:27]

Optional: Do you want to add disabled tests for 2.5.8.1.1 and bad
id-rsaEncryption params as well? Maybe also a non-minimally encoded
SubjectPublicKeyInfo.

[eroman, 2015/07/21 19:24:28]

Yes, good ideas.

I will post another patchset when this is complete, and plan to add at least:
  * Missing params for rsaEncryption (I believe BoringSSL allows this)
  * Non-NULL params for rsaEncryption (for instance an empty sequence, or
INTEGER)
  * A NULL params for rsaEncryption that specifies a length other than 0 (is
this what you meant by non-minimal, or where you referring to length encodings?)
  * An rsa key with OID 2.5.8.1.1 (id-ea-rsa)

Let me know if you have other ideas for tests.

[davidben, 2015/07/21 19:41:14]

I was thinking length encodings. I *think* crypto/asn1 will happily accept
those, though I haven't checked.

Honestly, most of it could be covered by "did you use a real DER parser?". Minor
nuisance is that we're switching parsers and one of them's not a real one.

(Actually there's three parsers involved now. der::Input for the outer stuff.
Then the SPKI by crypto/asn1. But the RSAPublicKey is crypto/bytestring. With
the SPKI eventually being folded into one of the other.)

+  RunTestCase(SUCCESS, "rsa-pss-sha256-salt10-using-pss-key-with-params.pem");
+}
+
+TEST(VerifySignedDataTest, RsaPss_Sha256_Salt10_UsingPssKeyWithWrongParams) {
+  RunTestCase(FAILURE,
+              "rsa-pss-sha256-salt10-using-pss-key-with-wrong-params.pem");
+}
+
+TEST(VerifySignedDataTest, RsaPss_Sha256_Salt12_UsingPssKeyWithNullParams) {
+  RunTestCase(FAILURE,
+              "rsa-pss-sha1-salt20-using-pss-key-with-null-params.pem");
+}
+
+}  // namespace
+
+}  // namespace net