Cert - protobufs to serialize and deserialize CertVerifierCache.

components/cronet/cert/cert_verifier_cache_persister_unittest.cc

Index: components/cronet/cert/cert_verifier_cache_persister_unittest.cc
diff --git a/components/cronet/cert/cert_verifier_cache_persister_unittest.cc b/components/cronet/cert/cert_verifier_cache_persister_unittest.cc
new file mode 100644
index 0000000000000000000000000000000000000000..43512218c153ce1812c3629ea69ddaf90017c59a
--- /dev/null
+++ b/components/cronet/cert/cert_verifier_cache_persister_unittest.cc
@@ -0,0 +1,614 @@
+// Copyright 2016 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 "components/cronet/cert/cert_verifier_cache_persister.h"
+
+#include <memory>
+#include <string>
+
+#include "base/android/path_utils.h"
+#include "base/files/file_path.h"
+#include "base/memory/ptr_util.h"
+#include "base/memory/ref_counted.h"
+#include "net/base/net_errors.h"
+#include "net/base/test_completion_callback.h"
+#include "net/base/test_data_directory.h"
+#include "net/cert/caching_cert_verifier.h"
+#include "net/cert/cert_verifier.h"
+#include "net/cert/cert_verify_result.h"
+#include "net/cert/mock_cert_verifier.h"
+#include "net/cert/x509_certificate.h"
+#include "net/log/net_log.h"
+#include "net/test/cert_test_util.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace cronet {
+
+namespace {
+
+int VerifyCert(scoped_refptr<net::X509Certificate> certificate,

[Ryan Sleevi, 2016/06/15 23:31:32]

Either pass as raw pointer (X509Certificate*) or std::move on line 38. You
shouldn't pass by-val unless you're taking ownership, same as unique_ptr

[ramant (doing other things), 2016/06/17 02:45:15]

Thanks. Missed it.
Done.

+               const std::string& hostname,
+               net::CachingCertVerifier* verifier,
+               net::CertVerifyResult* verify_result) {
+  net::TestCompletionCallback callback;
+  std::unique_ptr<net::CertVerifier::Request> request;
+
+  return callback.GetResult(verifier->Verify(
+      net::CertVerifier::RequestParams(certificate, hostname, 0, std::string(),
+                                       net::CertificateList()),
+      nullptr, verify_result, callback.callback(), &request,
+      net::BoundNetLog()));
+}
+
+cronet_pb::CertVerificationCache VerifyAndSerializeCert(
+    const std::string& cert_name,
+    const std::string& hostname,
+    net::CachingCertVerifier* verifier,
+    net::CertVerifyResult* verify_result) {
+  // Set up server certs.
+  scoped_refptr<net::X509Certificate> cert(
+      net::ImportCertFromFile(net::GetTestCertsDirectory(), cert_name));
+  EXPECT_TRUE(cert.get());

[Ryan Sleevi, 2016/06/15 23:31:33]

This will crash the test if it's false, so you're not gaining anything from
this.

You can only ASSERT_TRUE() if your return is void or a
::testing::AssertionResult

[Ryan Sleevi, 2016/06/15 23:31:33]

STYLE: Don't use .get() for boolean comparisons.

Just
EXPECT_TRUE(cert);

(but really,
ASSERT_TRUE(cert);

[ramant (doing other things), 2016/06/17 02:45:14]

Done.

[ramant (doing other things), 2016/06/17 02:45:15]

Done.

+
+  int error = VerifyCert(cert, hostname, verifier, verify_result);
+  EXPECT_TRUE(net::IsCertificateError(error));

[Ryan Sleevi, 2016/06/15 23:31:33]

Ditto about this doing nothing

[ramant (doing other things), 2016/06/17 02:45:15]

Done.

+
+  cronet_pb::CertVerificationCache cert_cache =
+      SerializeCertVerifierCache(*verifier);
+  DCHECK_EQ(1, cert_cache.cert_entry_size());
+  DCHECK_EQ(1, cert_cache.cache_entry_size());

[Ryan Sleevi, 2016/06/15 23:31:32]

Don't DCHECK in tests. That crashes the test.

My overall recommendation is don't make a utility function like this, for the
reasons demonstrated above. Do the needful in the tests themselves, or
restructure this so you can use ASSERT_NO_FATAL_FAILURE with the subroutines.

[ramant (doing other things), 2016/06/17 02:45:14]

Done.

+  return cert_cache;
+}
+
+}  // namespace
+
+class CertVerifierCachePersisterTest : public ::testing::Test {
+ public:
+  CertVerifierCachePersisterTest()
+      : verifier_(base::MakeUnique<net::MockCertVerifier>()) {}
+  ~CertVerifierCachePersisterTest() override {}
+
+ protected:
+  net::CachingCertVerifier verifier_;

[Ryan Sleevi, 2016/06/15 23:31:32]

This doesn't really need to be a test harness.

[ramant (doing other things), 2016/06/17 02:45:15]

Done.

+};
+
+TEST_F(CertVerifierCachePersisterTest, RestoreEmptyData) {
+  // Restoring empty data should fail.
+  cronet_pb::CertVerificationCache cert_cache;
+  EXPECT_FALSE(DeserializeCertVerifierCache(cert_cache, &verifier_));
+}
+
+TEST_F(CertVerifierCachePersisterTest, SerializeCache) {
+  net::CertVerifyResult verify_result;
+  cronet_pb::CertVerificationCache cert_cache = VerifyAndSerializeCert(
+      "ok_cert.pem", "www.example.com", &verifier_, &verify_result);
+}
+
+TEST_F(CertVerifierCachePersisterTest, RestoreExistingEntry) {
+  net::CertVerifyResult verify_result;
+  cronet_pb::CertVerificationCache cert_cache = VerifyAndSerializeCert(
+      "ok_cert.pem", "www.example.com", &verifier_, &verify_result);
+
+  // Restore the cache data for an existing entry shouldn't fail.

[Ryan Sleevi, 2016/06/15 23:31:33]

GRAMMAR: Restoring?
GRAMMAR: Avoid double negative (should not fail -> should succeed)?

[ramant (doing other things), 2016/06/17 02:45:15]

Done.

+  EXPECT_TRUE(DeserializeCertVerifierCache(cert_cache, &verifier_));

[Ryan Sleevi, 2016/06/15 23:31:32]

This is unclear to me why this test exists.

[ramant (doing other things), 2016/06/17 02:45:14]

My intention was to test if cert verifier cache already had an entry, restoring
an entry from deserialization shouldn't cause issues.

Deleted it.

+}
+
+TEST_F(CertVerifierCachePersisterTest, RestoreDataIntoNewVerifier) {
+  scoped_refptr<net::X509Certificate> ok_cert(
+      net::ImportCertFromFile(net::GetTestCertsDirectory(), "ok_cert.pem"));
+  ASSERT_NE(static_cast<net::X509Certificate*>(NULL), ok_cert.get());

[Ryan Sleevi, 2016/06/15 23:31:32]

STYLE: Don't do this.

ASSERT_TRUE(ok_cert);

[ramant (doing other things), 2016/06/17 02:45:15]

Done.

+
+  int error;
+  std::string example_hostname("www.example.com");
+  net::CertVerifyResult verify_result;
+  net::TestCompletionCallback callback;
+  std::unique_ptr<net::CertVerifier::Request> request;
+
+  error = callback.GetResult(verifier_.Verify(
+      net::CertVerifier::RequestParams(ok_cert, example_hostname, 0,
+                                       std::string(), net::CertificateList()),
+      nullptr, &verify_result, callback.callback(), &request,
+      net::BoundNetLog()));
+  ASSERT_TRUE(net::IsCertificateError(error));
+
+  cronet_pb::CertVerificationCache cert_cache =
+      SerializeCertVerifierCache(verifier_);
+  DCHECK_EQ(1, cert_cache.cert_entry_size());
+  DCHECK_EQ(1, cert_cache.cache_entry_size());
+
+  // Create a new Verifier and restoring the data into it should succeed.

[Ryan Sleevi, 2016/06/15 23:31:32]

Perhaps this comment belongs on line 96 instead

[ramant (doing other things), 2016/06/17 02:45:15]

Done.

+  net::CachingCertVerifier verifier2(base::MakeUnique<net::MockCertVerifier>());
+  EXPECT_TRUE(DeserializeCertVerifierCache(cert_cache, &verifier2));
+
+  net::CertVerifyResult verify_result2;
+  error = callback.GetResult(verifier2.Verify(
+      net::CertVerifier::RequestParams(ok_cert, example_hostname, 0,
+                                       std::string(), net::CertificateList()),
+      nullptr, &verify_result2, callback.callback(), &request,
+      net::BoundNetLog()));
+  // Synchronous completion and verify it is same as serialized data.
+  ASSERT_NE(net::ERR_IO_PENDING, error);

[Ryan Sleevi, 2016/06/15 23:31:33]

You're calling .GetResult (line 125), so it's impossible for this assertion to
ever fail.

[ramant (doing other things), 2016/06/17 02:45:15]

Done.

+  ASSERT_TRUE(net::IsCertificateError(error));
+  ASSERT_FALSE(request);
+  EXPECT_EQ(verify_result2.cert_status, verify_result.cert_status);
+}
+
+TEST_F(CertVerifierCachePersisterTest, RestoreMultipleEntriesIntoNewVerifier) {

[Ryan Sleevi, 2016/06/15 23:31:32]

Why isn't this combined into the previous test?

[ramant (doing other things), 2016/06/17 02:45:15]

Done.

+  scoped_refptr<net::X509Certificate> ok_cert(
+      net::ImportCertFromFile(net::GetTestCertsDirectory(), "ok_cert.pem"));
+  ASSERT_NE(static_cast<net::X509Certificate*>(NULL), ok_cert.get());
+
+  const scoped_refptr<net::X509Certificate> root_cert =
+      net::ImportCertFromFile(net::GetTestCertsDirectory(), "root_ca_cert.pem");
+  ASSERT_TRUE(root_cert.get());
+
+  // Create a certificate that contains both a leaf and an intermediate/root.
+  net::X509Certificate::OSCertHandles chain;
+  chain.push_back(root_cert->os_cert_handle());
+  const scoped_refptr<net::X509Certificate> combined_cert =
+      net::X509Certificate::CreateFromHandle(ok_cert->os_cert_handle(), chain);
+  ASSERT_TRUE(combined_cert.get());
+
+  int error;
+  net::TestCompletionCallback callback;
+  std::unique_ptr<net::CertVerifier::Request> request;
+
+  // Verify www.example.com host's certificate.
+  std::string example_hostname("www.example.com");
+  net::CertVerifyResult verifyier1_result1;
+  error = callback.GetResult(verifier_.Verify(
+      net::CertVerifier::RequestParams(ok_cert, example_hostname, 0,
+                                       std::string(), net::CertificateList()),
+      nullptr, &verifyier1_result1, callback.callback(), &request,
+      net::BoundNetLog()));
+  ASSERT_TRUE(net::IsCertificateError(error));
+
+  // Verify www.example2.com host's certificate.
+  std::string example2_hostname("www.example2.com");
+  net::CertVerifyResult verifier1_result2;
+  error = callback.GetResult(verifier_.Verify(
+      net::CertVerifier::RequestParams(combined_cert, example2_hostname, 0,
+                                       std::string(), net::CertificateList()),
+      nullptr, &verifier1_result2, callback.callback(), &request,
+      net::BoundNetLog()));
+  ASSERT_TRUE(net::IsCertificateError(error));
+
+  cronet_pb::CertVerificationCache cert_cache =
+      SerializeCertVerifierCache(verifier_);
+  DCHECK_EQ(2, cert_cache.cert_entry_size());
+  DCHECK_EQ(2, cert_cache.cache_entry_size());
+
+  // Create a new Verifier and restoring the data into it should succeed.
+  net::CachingCertVerifier verifier2(base::MakeUnique<net::MockCertVerifier>());
+
+  // Populate |verifier2|'s cache.
+  EXPECT_TRUE(DeserializeCertVerifierCache(cert_cache, &verifier2));
+
+  // Verify the cert for www.example.com with |verifier2|.
+  net::CertVerifyResult verifier2_result1;
+  error = callback.GetResult(verifier2.Verify(
+      net::CertVerifier::RequestParams(ok_cert, example_hostname, 0,
+                                       std::string(), net::CertificateList()),
+      nullptr, &verifier2_result1, callback.callback(), &request,
+      net::BoundNetLog()));
+  // Synchronous completion and verify it is same as serialized data for
+  // www.example.com.
+  ASSERT_NE(net::ERR_IO_PENDING, error);
+  ASSERT_TRUE(net::IsCertificateError(error));
+  ASSERT_FALSE(request);
+  EXPECT_EQ(verifier2_result1.cert_status, verifyier1_result1.cert_status);
+
+  // Verify the cert for www.example2.com with |verifier2|.
+  net::CertVerifyResult verifier2_result2;
+  error = callback.GetResult(verifier2.Verify(
+      net::CertVerifier::RequestParams(combined_cert, example2_hostname, 0,
+                                       std::string(), net::CertificateList()),
+      nullptr, &verifier2_result2, callback.callback(), &request,
+      net::BoundNetLog()));
+  // Synchronous completion and verify it is same as serialized data for
+  // www.example2.com.
+  ASSERT_NE(net::ERR_IO_PENDING, error);
+  ASSERT_TRUE(net::IsCertificateError(error));
+  ASSERT_FALSE(request);
+  EXPECT_EQ(verifier2_result2.cert_status, verifier1_result2.cert_status);
+}
+
+TEST_F(CertVerifierCachePersisterTest, DeserializeCorruptedCerts) {

[Ryan Sleevi, 2016/06/15 23:31:32]

But what's the expected state of |verifier2| here? Some certs but not all? Or no
certs.

As it stands, you call AddEntry on each iteration, so if deserialization fails
partially, you'll deserialize bad data into |verifier2|.

It's a test that highlights an undefined part of your API implementation, and
reveals your DeserializeCertVerifierCache has side-effects, which is probably
undesirable.

[ramant (doing other things), 2016/06/17 02:45:14]

Changed the code to populate CachingCertVerifier only if all the data looks
good.

+  net::CertVerifyResult verify_result;
+  cronet_pb::CertVerificationCache cert_cache = VerifyAndSerializeCert(
+      "ok_cert.pem", "www.example.com", &verifier_, &verify_result);
+
+  // Corrupt deserialized certs and verify deserialization failure.
+  cert_cache.clear_cert_entry();
+
+  net::CachingCertVerifier verifier2(base::MakeUnique<net::MockCertVerifier>());
+  EXPECT_FALSE(DeserializeCertVerifierCache(cert_cache, &verifier2));
+}
+
+TEST_F(CertVerifierCachePersisterTest, DeserializeCorruptedCacheEntry) {
+  net::CertVerifyResult verify_result;
+  cronet_pb::CertVerificationCache cert_cache = VerifyAndSerializeCert(
+      "ok_cert.pem", "www.example.com", &verifier_, &verify_result);
+
+  // Corrupt cronet_pb::CertVerificationCacheEntry and verify deserialization
+  // failure.
+  cert_cache.clear_cache_entry();
+
+  net::CachingCertVerifier verifier2(base::MakeUnique<net::MockCertVerifier>());
+  EXPECT_FALSE(DeserializeCertVerifierCache(cert_cache, &verifier2));
+}
+
+TEST_F(CertVerifierCachePersisterTest, DeserializeCorruptedRequestParams) {
+  net::CertVerifyResult verify_result;
+  cronet_pb::CertVerificationCache cert_cache = VerifyAndSerializeCert(
+      "ok_cert.pem", "www.example.com", &verifier_, &verify_result);
+
+  for (int i = 0; i < cert_cache.cache_entry_size(); ++i) {
+    cronet_pb::CertVerificationCacheEntry* cache_entry =
+        cert_cache.mutable_cache_entry(i);
+    // Corrupt request_params and verify deserialization failure.
+    cache_entry->clear_request_params();
+  }
+
+  net::CachingCertVerifier verifier2(base::MakeUnique<net::MockCertVerifier>());
+  EXPECT_FALSE(DeserializeCertVerifierCache(cert_cache, &verifier2));
+}
+
+TEST_F(CertVerifierCachePersisterTest, DeserializeRequestParamsNoCertificate) {
+  net::CertVerifyResult verify_result;
+  cronet_pb::CertVerificationCache cert_cache = VerifyAndSerializeCert(
+      "ok_cert.pem", "www.example.com", &verifier_, &verify_result);
+
+  for (int i = 0; i < cert_cache.cache_entry_size(); ++i) {
+    cronet_pb::CertVerificationCacheEntry* cache_entry =
+        cert_cache.mutable_cache_entry(i);
+    cronet_pb::CertVerificationRequestParams* request_params =
+        cache_entry->mutable_request_params();
+    // Corrupt request_params's certificate and verify deserialization failure.
+    request_params->clear_certificate();
+  }
+
+  net::CachingCertVerifier verifier2(base::MakeUnique<net::MockCertVerifier>());
+  EXPECT_FALSE(DeserializeCertVerifierCache(cert_cache, &verifier2));
+}
+
+TEST_F(CertVerifierCachePersisterTest, DeserializeRequestParamsNoHostname) {
+  net::CertVerifyResult verify_result;
+  cronet_pb::CertVerificationCache cert_cache = VerifyAndSerializeCert(
+      "ok_cert.pem", "www.example.com", &verifier_, &verify_result);
+
+  for (int i = 0; i < cert_cache.cache_entry_size(); ++i) {
+    cronet_pb::CertVerificationCacheEntry* cache_entry =
+        cert_cache.mutable_cache_entry(i);
+    cronet_pb::CertVerificationRequestParams* request_params =
+        cache_entry->mutable_request_params();
+    // Corrupt request_params's hostname and verify deserialization failure.
+    request_params->clear_hostname();
+  }
+
+  net::CachingCertVerifier verifier2(base::MakeUnique<net::MockCertVerifier>());
+  EXPECT_FALSE(DeserializeCertVerifierCache(cert_cache, &verifier2));
+}
+
+TEST_F(CertVerifierCachePersisterTest, DeserializeRequestParamsEmptyHostname) {
+  net::CertVerifyResult verify_result;
+  cronet_pb::CertVerificationCache cert_cache = VerifyAndSerializeCert(
+      "ok_cert.pem", "www.example.com", &verifier_, &verify_result);
+
+  for (int i = 0; i < cert_cache.cache_entry_size(); ++i) {
+    cronet_pb::CertVerificationCacheEntry* cache_entry =
+        cert_cache.mutable_cache_entry(i);
+    cronet_pb::CertVerificationRequestParams* request_params =
+        cache_entry->mutable_request_params();
+    // Set bogus hostname and verify deserialization failure.
+    request_params->set_hostname("");
+  }
+
+  net::CachingCertVerifier verifier2(base::MakeUnique<net::MockCertVerifier>());
+  EXPECT_FALSE(DeserializeCertVerifierCache(cert_cache, &verifier2));
+}
+
+TEST_F(CertVerifierCachePersisterTest, DeserializeRequestParamsNoFlags) {
+  net::CertVerifyResult verify_result;
+  cronet_pb::CertVerificationCache cert_cache = VerifyAndSerializeCert(
+      "ok_cert.pem", "www.example.com", &verifier_, &verify_result);
+
+  for (int i = 0; i < cert_cache.cache_entry_size(); ++i) {
+    cronet_pb::CertVerificationCacheEntry* cache_entry =
+        cert_cache.mutable_cache_entry(i);
+    cronet_pb::CertVerificationRequestParams* request_params =
+        cache_entry->mutable_request_params();
+    // Corrupt request_params's flags and verify deserialization failure.
+    request_params->clear_flags();
+  }
+
+  net::CachingCertVerifier verifier2(base::MakeUnique<net::MockCertVerifier>());
+  EXPECT_FALSE(DeserializeCertVerifierCache(cert_cache, &verifier2));
+}
+
+TEST_F(CertVerifierCachePersisterTest, DeserializeRequestParamsNoOcspResponse) {
+  net::CertVerifyResult verify_result;
+  cronet_pb::CertVerificationCache cert_cache = VerifyAndSerializeCert(
+      "ok_cert.pem", "www.example.com", &verifier_, &verify_result);
+
+  for (int i = 0; i < cert_cache.cache_entry_size(); ++i) {
+    cronet_pb::CertVerificationCacheEntry* cache_entry =
+        cert_cache.mutable_cache_entry(i);
+    cronet_pb::CertVerificationRequestParams* request_params =
+        cache_entry->mutable_request_params();
+    // Corrupt request_params's flags and verify deserialization failure.
+    request_params->clear_ocsp_response();
+  }
+
+  net::CachingCertVerifier verifier2(base::MakeUnique<net::MockCertVerifier>());
+  EXPECT_FALSE(DeserializeCertVerifierCache(cert_cache, &verifier2));
+}
+
+TEST_F(CertVerifierCachePersisterTest,
+       DeserializeRequestParamsCertificateNoCertNumbers) {
+  net::CertVerifyResult verify_result;
+  cronet_pb::CertVerificationCache cert_cache = VerifyAndSerializeCert(
+      "ok_cert.pem", "www.example.com", &verifier_, &verify_result);
+
+  for (int i = 0; i < cert_cache.cache_entry_size(); ++i) {
+    cronet_pb::CertVerificationCacheEntry* cache_entry =
+        cert_cache.mutable_cache_entry(i);
+    cronet_pb::CertVerificationRequestParams* request_params =
+        cache_entry->mutable_request_params();
+    cronet_pb::CertVerificationCertificate* certificate =
+        request_params->mutable_certificate();
+    // Corrupt request_params's certificate and verify deserialization failure.
+    certificate->clear_cert_numbers();
+  }
+
+  net::CachingCertVerifier verifier2(base::MakeUnique<net::MockCertVerifier>());
+  EXPECT_FALSE(DeserializeCertVerifierCache(cert_cache, &verifier2));
+}
+
+TEST_F(CertVerifierCachePersisterTest,
+       DeserializeCorruptedRequestParamsCertNumbers) {

[Ryan Sleevi, 2016/06/15 23:31:33]

Do lines 217-370 need to be structured as they are? Couldn't you use an
iterative approach with in-place protobuf structures that you deserialize?

That is, something like
const SomeTestData data[] = { {a, a, a},
  {b, b, b},
  {c, c, c},
  {d, d, d} };
for (const auto& test_data : data) {
  SCOPED_TEST(...);
  ASSERT_FALSE(Deserialize....);
}

That is, the above assumes you can do struct-initialization of the protobufs,
and I don't know if you can, but if you can, it sure beats the hell out of the
serialize-mangle-deserialize.

[ramant (doing other things), 2016/06/17 02:45:15]

I agree. All the protobuf related tests, I had seen were doing set... methods.

+  net::CertVerifyResult verify_result;
+  cronet_pb::CertVerificationCache cert_cache = VerifyAndSerializeCert(
+      "ok_cert.pem", "www.example.com", &verifier_, &verify_result);
+
+  for (int i = 0; i < cert_cache.cache_entry_size(); ++i) {
+    cronet_pb::CertVerificationCacheEntry* cache_entry =
+        cert_cache.mutable_cache_entry(i);
+    cronet_pb::CertVerificationRequestParams* request_params =
+        cache_entry->mutable_request_params();
+    cronet_pb::CertVerificationCertificate* certificate =
+        request_params->mutable_certificate();
+    // Set bogus certificate and verify deserialization failure.
+    certificate->set_cert_numbers(0, 100);
+  }
+
+  net::CachingCertVerifier verifier2(base::MakeUnique<net::MockCertVerifier>());
+  EXPECT_FALSE(DeserializeCertVerifierCache(cert_cache, &verifier2));
+}
+
+TEST_F(CertVerifierCachePersisterTest,
+       DeserializeRequestParamsCertificateNoTrustAnchors) {
+  net::CertificateList ca_cert_list = net::CreateCertificateListFromFile(
+      net::GetTestCertsDirectory(), "root_ca_cert.pem",
+      net::X509Certificate::FORMAT_AUTO);
+  ASSERT_EQ(1U, ca_cert_list.size());
+  scoped_refptr<net::X509Certificate> ca_cert(ca_cert_list[0]);
+
+  net::CertificateList cert_list = net::CreateCertificateListFromFile(
+      net::GetTestCertsDirectory(), "ok_cert.pem",
+      net::X509Certificate::FORMAT_AUTO);
+  ASSERT_EQ(1U, cert_list.size());
+  scoped_refptr<net::X509Certificate> cert(cert_list[0]);
+
+  // Now add the |ca_cert| to the |trust_anchors|, and verification should pass.
+  net::CertificateList trust_anchors;
+  trust_anchors.push_back(ca_cert);
+
+  int error;
+  net::CertVerifyResult verify_result;
+  net::TestCompletionCallback callback;
+  std::unique_ptr<net::CertVerifier::Request> request;
+
+  error = callback.GetResult(verifier_.Verify(
+      net::CertVerifier::RequestParams(cert, "www.example.com", 0,
+                                       std::string(), trust_anchors),
+      nullptr, &verify_result, callback.callback(), &request,
+      net::BoundNetLog()));
+  EXPECT_TRUE(net::IsCertificateError(error));
+
+  cronet_pb::CertVerificationCache cert_cache =
+      SerializeCertVerifierCache(verifier_);
+  DCHECK_EQ(2, cert_cache.cert_entry_size());
+  DCHECK_EQ(1, cert_cache.cache_entry_size());
+
+  for (int i = 0; i < cert_cache.cache_entry_size(); ++i) {
+    cronet_pb::CertVerificationCacheEntry* cache_entry =
+        cert_cache.mutable_cache_entry(i);
+    cronet_pb::CertVerificationRequestParams* request_params =
+        cache_entry->mutable_request_params();
+    for (int j = 0; j < request_params->additional_trust_anchors_size(); ++j) {
+      cronet_pb::CertVerificationCertificate* certificate =
+          request_params->mutable_additional_trust_anchors(j);
+      // Corrupt the certificate number in |additional_trust_anchors| and verify
+      // deserialization failure.w
+      certificate->clear_cert_numbers();
+    }
+  }
+
+  net::CachingCertVerifier verifier2(base::MakeUnique<net::MockCertVerifier>());
+  EXPECT_FALSE(DeserializeCertVerifierCache(cert_cache, &verifier2));
+}
+
+TEST_F(CertVerifierCachePersisterTest, DeserializeCorruptedCachedResult) {
+  net::CertVerifyResult verify_result;
+  cronet_pb::CertVerificationCache cert_cache = VerifyAndSerializeCert(
+      "ok_cert.pem", "www.example.com", &verifier_, &verify_result);
+
+  for (int i = 0; i < cert_cache.cache_entry_size(); ++i) {
+    cronet_pb::CertVerificationCacheEntry* cache_entry =
+        cert_cache.mutable_cache_entry(i);
+    // Corrupt the |cached_result| and verify deserialization failure.
+    cache_entry->clear_cached_result();
+  }
+
+  net::CachingCertVerifier verifier2(base::MakeUnique<net::MockCertVerifier>());
+  EXPECT_FALSE(DeserializeCertVerifierCache(cert_cache, &verifier2));
+}
+
+TEST_F(CertVerifierCachePersisterTest, DeserializeCachedResultNoError) {
+  net::CertVerifyResult verify_result;
+  cronet_pb::CertVerificationCache cert_cache = VerifyAndSerializeCert(
+      "ok_cert.pem", "www.example.com", &verifier_, &verify_result);
+
+  for (int i = 0; i < cert_cache.cache_entry_size(); ++i) {
+    cronet_pb::CertVerificationCacheEntry* cache_entry =
+        cert_cache.mutable_cache_entry(i);
+    cronet_pb::CertVerificationCachedResult* cached_result =
+        cache_entry->mutable_cached_result();
+    // Corrupt the |error| and verify deserialization failure.
+    cached_result->clear_error();
+  }
+
+  net::CachingCertVerifier verifier2(base::MakeUnique<net::MockCertVerifier>());
+  EXPECT_FALSE(DeserializeCertVerifierCache(cert_cache, &verifier2));
+}
+
+TEST_F(CertVerifierCachePersisterTest, DeserializeCachedResultNoResult) {
+  net::CertVerifyResult verify_result;
+  cronet_pb::CertVerificationCache cert_cache = VerifyAndSerializeCert(
+      "ok_cert.pem", "www.example.com", &verifier_, &verify_result);
+
+  for (int i = 0; i < cert_cache.cache_entry_size(); ++i) {
+    cronet_pb::CertVerificationCacheEntry* cache_entry =
+        cert_cache.mutable_cache_entry(i);
+    cronet_pb::CertVerificationCachedResult* cached_result =
+        cache_entry->mutable_cached_result();
+    // Corrupt the |cached_result| and verify deserialization failure.
+    cached_result->clear_result();
+  }
+
+  net::CachingCertVerifier verifier2(base::MakeUnique<net::MockCertVerifier>());
+  EXPECT_FALSE(DeserializeCertVerifierCache(cert_cache, &verifier2));
+}
+
+TEST_F(CertVerifierCachePersisterTest, DeserializeCachedResultNoCertStatus) {
+  net::CertVerifyResult verify_result;
+  cronet_pb::CertVerificationCache cert_cache = VerifyAndSerializeCert(
+      "ok_cert.pem", "www.example.com", &verifier_, &verify_result);
+
+  for (int i = 0; i < cert_cache.cache_entry_size(); ++i) {
+    cronet_pb::CertVerificationCacheEntry* cache_entry =
+        cert_cache.mutable_cache_entry(i);
+    cronet_pb::CertVerificationCachedResult* cached_result =
+        cache_entry->mutable_cached_result();
+    cronet_pb::CertVerificationResult* result = cached_result->mutable_result();
+    // Corrupt the |cert_status| and verify deserialization failure.
+    result->clear_cert_status();
+  }
+
+  net::CachingCertVerifier verifier2(base::MakeUnique<net::MockCertVerifier>());
+  EXPECT_FALSE(DeserializeCertVerifierCache(cert_cache, &verifier2));
+}
+
+TEST_F(CertVerifierCachePersisterTest, DeserializeCachedResultNoVerifiedCert) {
+  net::CertVerifyResult verify_result;
+  cronet_pb::CertVerificationCache cert_cache = VerifyAndSerializeCert(
+      "ok_cert.pem", "www.example.com", &verifier_, &verify_result);
+
+  for (int i = 0; i < cert_cache.cache_entry_size(); ++i) {
+    cronet_pb::CertVerificationCacheEntry* cache_entry =
+        cert_cache.mutable_cache_entry(i);
+    cronet_pb::CertVerificationCachedResult* cached_result =
+        cache_entry->mutable_cached_result();
+    cronet_pb::CertVerificationResult* result = cached_result->mutable_result();
+    // Corrupt the |verified_cert| and verify deserialization failure.
+    result->clear_verified_cert();
+  }
+
+  net::CachingCertVerifier verifier2(base::MakeUnique<net::MockCertVerifier>());
+  EXPECT_FALSE(DeserializeCertVerifierCache(cert_cache, &verifier2));
+}
+
+TEST_F(CertVerifierCachePersisterTest,
+       DeserializeCachedResultNoVerifiedCertNumber) {
+  net::CertVerifyResult verify_result;
+  cronet_pb::CertVerificationCache cert_cache = VerifyAndSerializeCert(
+      "ok_cert.pem", "www.example.com", &verifier_, &verify_result);
+
+  for (int i = 0; i < cert_cache.cache_entry_size(); ++i) {
+    cronet_pb::CertVerificationCacheEntry* cache_entry =
+        cert_cache.mutable_cache_entry(i);
+    cronet_pb::CertVerificationCachedResult* cached_result =
+        cache_entry->mutable_cached_result();
+    cronet_pb::CertVerificationResult* result = cached_result->mutable_result();
+    result->clear_verified_cert();
+    // Corrupt the verified certificate and verify deserialization failure.
+    cronet_pb::CertVerificationCertificate* certificate =

[Ryan Sleevi, 2016/06/15 23:31:32]

All of these corrupt tests seem to share the common pattern I mentioned above.

[ramant (doing other things), 2016/06/17 02:45:15]

Acknowledged.

+        result->mutable_verified_cert();
+    certificate->clear_cert_numbers();
+  }
+
+  net::CachingCertVerifier verifier2(base::MakeUnique<net::MockCertVerifier>());
+  EXPECT_FALSE(DeserializeCertVerifierCache(cert_cache, &verifier2));
+}
+
+TEST_F(CertVerifierCachePersisterTest,
+       DeserializeCorruptedCachedResultVerifiedCertNumber) {
+  net::CertVerifyResult verify_result;
+  cronet_pb::CertVerificationCache cert_cache = VerifyAndSerializeCert(
+      "ok_cert.pem", "www.example.com", &verifier_, &verify_result);
+
+  for (int i = 0; i < cert_cache.cache_entry_size(); ++i) {
+    cronet_pb::CertVerificationCacheEntry* cache_entry =
+        cert_cache.mutable_cache_entry(i);
+    cronet_pb::CertVerificationCachedResult* cached_result =
+        cache_entry->mutable_cached_result();
+    cronet_pb::CertVerificationResult* result = cached_result->mutable_result();
+    cronet_pb::CertVerificationCertificate* certificate =
+        result->mutable_verified_cert();
+    // Set bogus certificate number and verify deserialization failure.
+    certificate->set_cert_numbers(0, 100);
+  }
+
+  net::CachingCertVerifier verifier2(base::MakeUnique<net::MockCertVerifier>());
+  EXPECT_FALSE(DeserializeCertVerifierCache(cert_cache, &verifier2));
+}
+
+TEST_F(CertVerifierCachePersisterTest,
+       DeserializeCorruptedCachedResultPublicKeyHashes) {
+  net::CertVerifyResult verify_result;
+  cronet_pb::CertVerificationCache cert_cache = VerifyAndSerializeCert(
+      "ok_cert.pem", "www.example.com", &verifier_, &verify_result);
+
+  for (int i = 0; i < cert_cache.cache_entry_size(); ++i) {
+    cronet_pb::CertVerificationCacheEntry* cache_entry =
+        cert_cache.mutable_cache_entry(i);
+    cronet_pb::CertVerificationCachedResult* cached_result =
+        cache_entry->mutable_cached_result();
+    cronet_pb::CertVerificationResult* result = cached_result->mutable_result();
+    // Set bogus |public_key_hashes| and verify deserialization failure.
+    result->add_public_key_hashes("");
+  }
+
+  net::CachingCertVerifier verifier2(base::MakeUnique<net::MockCertVerifier>());
+  EXPECT_FALSE(DeserializeCertVerifierCache(cert_cache, &verifier2));
+}
+
+TEST_F(CertVerifierCachePersisterTest, DeserializeCorruptedVerificationTime) {
+  net::CertVerifyResult verify_result;
+  cronet_pb::CertVerificationCache cert_cache = VerifyAndSerializeCert(
+      "ok_cert.pem", "www.example.com", &verifier_, &verify_result);
+
+  for (int i = 0; i < cert_cache.cache_entry_size(); ++i) {
+    cronet_pb::CertVerificationCacheEntry* cache_entry =
+        cert_cache.mutable_cache_entry(i);
+    // Corrupt |verification_time| and verify deserialization failure.
+    cache_entry->clear_verification_time();
+  }
+
+  net::CachingCertVerifier verifier2(base::MakeUnique<net::MockCertVerifier>());
+  EXPECT_FALSE(DeserializeCertVerifierCache(cert_cache, &verifier2));
+}
+
+}  // namespace cronet