Disk Based Certificate Cache Implementation

net/http/disk_based_cert_cache_unittest.cc

Index: net/http/disk_based_cert_cache_unittest.cc
diff --git a/net/http/disk_based_cert_cache_unittest.cc b/net/http/disk_based_cert_cache_unittest.cc
new file mode 100644
index 0000000000000000000000000000000000000000..f8340e8b3ababc9985251cf3dde735bf3852a188
--- /dev/null
+++ b/net/http/disk_based_cert_cache_unittest.cc
@@ -0,0 +1,431 @@
+// Copyright (c) 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 "net/http/disk_based_cert_cache.h"
+
+#include "base/bind.h"
+#include "base/callback_helpers.h"
+#include "net/base/completion_callback.h"
+#include "net/base/io_buffer.h"
+#include "net/base/net_errors.h"
+#include "net/base/test_completion_callback.h"
+#include "net/base/test_data_directory.h"
+#include "net/disk_cache/memory/mem_backend_impl.h"
+#include "net/http/mock_http_cache.h"
+#include "net/test/cert_test_util.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace net {
+
+namespace {
+
+// Testing the DiskBasedCertCache requireds constant use of the

[Ryan Sleevi, 2014/06/25 19:31:57]

s/requires/

+// certificates in GetTestCertsDirectory(). The TestCertMetaData
+// struct stores metadata relevant to the DiskBasedCertCache for
+// each used test certificate.
+struct TestCertMetaData {
+  const char* name;

[Ryan Sleevi, 2014/06/25 19:31:57]

s/name/file_name/ - since that's what you're naming.

+  const char* key;

[Ryan Sleevi, 2014/06/25 19:31:57]

s/key/cache_key/

+  const int size;

[Ryan Sleevi, 2014/06/25 19:31:57]

Is this supposed to be the size of the file? For sizes of byte ranges/files, we
use size_t, not int (the IOBuffer is sort of weird in that respect)

Note: We shouldn't bake this (or the contents of the certs) into code. We should
obtain it dynamically.

+};
+
+const TestCertMetaData kCert1{
+    "root_ca_cert.pem", "cert:4C005EF1CF45F80D4A5A2BCFB00D4F198121E8D4", 759};
+
+const TestCertMetaData kCert2{
+    "ok_cert.pem", "cert:9174C7CB9E4919604E7B1BFC430E4929DA45F65F", 888};
+
+// MockTransactions are required to use the MockDiskCache backend.
+// |key| is a cache key, and is equivalent to the key that will be
+// used to store or retrieve certificates in the cache. |test_mode|
+// is an integer that is used to indicate properties of the test
+// transaction, mostly whether or not it is synchronous.
+// For testing the DiskBasedCertcache, other data members of the struct
+// are irrelevant. Only one MockTransaction per certificate can be used
+// at a time.
+MockTransaction GetMockTransaction(const char* key, int test_mode) {
+  return {key,          "", base::Time(), "",   LOAD_NORMAL, "", "",
+          base::Time(), "", test_mode,    NULL, 0,           OK};
+}
+
+// Helper class, for use with DiskBasedCertCache::Get, that will ensure that
+// the returned certificate handle is kept alive after the callback has been
+// executed and allow a user to WaitForResult of DiskBasedCertCache::Get.
+class TestGetCallback {
+ public:
+  TestGetCallback() : cert_handle_(NULL) {}
+  ~TestGetCallback() { X509Certificate::FreeOSCertHandle(cert_handle_); }
+
+  // Blocks until the underlying Get() operation has succeeded.
+  void WaitForResult() { cb_.WaitForResult(); }
+
+  // Returns a Callback suitable for use with DiskBasedCertCache::Get(). The
+  // returned callback is only valid while the TestGetCallback object is still
+  // valid.
+  DiskBasedCertCache::GetCallback callback() {
+    return base::Bind(&TestGetCallback::OnGetComplete, base::Unretained(this));
+  }
+
+  // Returns the associated certificate handle.
+  const X509Certificate::OSCertHandle& cert_handle() const {
+    return cert_handle_;
+  }
+
+ private:
+  void OnGetComplete(const X509Certificate::OSCertHandle handle) {
+    if (handle)
+      cert_handle_ = X509Certificate::DupOSCertHandle(handle);
+    cb_.callback().Run(OK);
+  }
+
+  TestCompletionCallback cb_;
+  X509Certificate::OSCertHandle cert_handle_;
+};
+
+// Helper class, for use with DiskBasedCertCache::Set, that will store the
+// returned key and allow a user to WaitForResult of DiskBasedCertCache::Set.
+class TestSetCallback {
+ public:
+  TestSetCallback() {}
+  ~TestSetCallback() {}
+
+  // Blocks until the underlying Set() operation has succeeded.
+  void WaitForResult() { cb_.WaitForResult(); }
+
+  // Returns a Callback suitable for use with DiskBasedCertCache::Set(). The
+  // returned callback is only valid while the TestSetCallback object is still
+  // valid.
+  DiskBasedCertCache::SetCallback callback() {
+    return base::Bind(&TestSetCallback::OnSetComplete, base::Unretained(this));
+  }
+
+  // Returns the associated certificate handle.
+  const std::string& key() const { return key_; }
+
+ private:
+  void OnSetComplete(const std::string& key) {
+    key_ = key;
+    cb_.callback().Run(OK);
+  }
+
+  TestCompletionCallback cb_;
+  std::string key_;
+};
+
+// Stores the certificate corresponding to |cert_data| in |backend|. If
+// |corrupt_data| is true, the certificate will be imported with errors
+// so as to mimic a corrupted file on disk.
+void ImportCert(disk_cache::Backend* backend,
+                const TestCertMetaData& cert_data,
+                bool corrupt_data) {
+  disk_cache::Entry* entry;
+  TestCompletionCallback callback;
+  int rv = backend->CreateEntry(cert_data.key, &entry, callback.callback());
+  EXPECT_EQ(OK, callback.GetResult(rv));
+  scoped_refptr<X509Certificate> cert(
+      ImportCertFromFile(GetTestCertsDirectory(), cert_data.name));
+  std::string write_data;
+  bool encoded =
+      X509Certificate::GetDEREncoded(cert->os_cert_handle(), &write_data);
+  CHECK(encoded);

[Ryan Sleevi, 2014/06/25 19:31:57]

In unittests, you don't want to CHECK, as that can bring down the whole test
harness.

You'd want to do

ASSERT_TRUE(encoded);

+  if (corrupt_data) {
+    for (int i = 0; i < cert_data.size; i += 20)
+      write_data[i]++;
+  }
+  scoped_refptr<IOBuffer> buffer(new IOBuffer(write_data.size()));
+  memcpy(buffer->data(), write_data.data(), write_data.size());
+  rv = entry->WriteData(0 /* index */,
+                        0 /* offset */,
+                        buffer,
+                        write_data.size(),
+                        callback.callback(),
+                        true /* truncate */);
+  ASSERT_EQ(cert_data.size, callback.GetResult(rv));
+  entry->Close();
+}
+
+// Checks that the the certificate corresponding to |cert_data| is an existing,
+// correctly cached entry in |backend|.
+void CheckCertCached(disk_cache::Backend* backend,
+                     const TestCertMetaData& cert_data) {
+  disk_cache::Entry* entry;
+  TestCompletionCallback callback;
+  int rv = backend->OpenEntry(cert_data.key, &entry, callback.callback());
+  EXPECT_EQ(OK, callback.GetResult(rv));
+  scoped_refptr<X509Certificate> cert(
+      ImportCertFromFile(GetTestCertsDirectory(), cert_data.name));
+  std::string write_data;
+  bool encoded =
+      X509Certificate::GetDEREncoded(cert->os_cert_handle(), &write_data);
+  ASSERT_TRUE(encoded);
+  scoped_refptr<IOBuffer> buffer(new IOBuffer(cert_data.size));
+  rv = entry->ReadData(0 /* index */,
+                       0 /* offset */,
+                       buffer,
+                       cert_data.size,
+                       callback.callback());
+  EXPECT_EQ(cert_data.size, callback.GetResult(rv));
+  X509Certificate::OSCertHandle cached_cert_handle =
+      X509Certificate::CreateOSCertHandleFromBytes(buffer->data(),
+                                                   cert_data.size);
+  EXPECT_TRUE(X509Certificate::IsSameOSCert(cached_cert_handle,
+                                            cert->os_cert_handle()));
+}
+
+}  // namespace
+
+// ----------------------------------------------------------------------------
+
+// Tests that a certificate can be stored in the cache.
+TEST(DiskBasedCertCache, SetCert) {
+  ScopedMockTransaction trans1(
+      GetMockTransaction(kCert1.key, TEST_MODE_NORMAL));
+  MockDiskCache backend;
+  DiskBasedCertCache cache(&backend);
+  scoped_refptr<X509Certificate> cert(
+      ImportCertFromFile(GetTestCertsDirectory(), kCert1.name));
+  ASSERT_TRUE(cert.get());
+  TestSetCallback set_callback;
+
+  cache.Set(cert->os_cert_handle(), set_callback.callback());
+  set_callback.WaitForResult();
+  EXPECT_EQ(kCert1.key, set_callback.key());
+  CheckCertCached(&backend, kCert1);
+}
+
+// Tests that a certificate can be retrieved from the cache.
+TEST(DiskBasedCertCache, GetCert) {
+  ScopedMockTransaction trans1(
+      GetMockTransaction(kCert1.key, TEST_MODE_NORMAL));
+  MockDiskCache backend;
+  ImportCert(&backend, kCert1, false /* not corrupted */);

[Ryan Sleevi, 2014/06/25 19:31:57]

If ImportCert had a failure, this test will continue executing.

In order to have the error propagate from subroutines (both this and
"CheckCertCached"), you need to wrap them in a GTest macro

ASSERT_NO_FATAL_FAILURE(ImportCert(...));
ASSERT_NO_FATAL_FAILURE(CheckCertCached(...));

+  DiskBasedCertCache cache(&backend);
+  TestGetCallback get_callback;
+
+  cache.Get(kCert1.key, get_callback.callback());
+  get_callback.WaitForResult();
+
+  scoped_refptr<X509Certificate> cert(
+      ImportCertFromFile(GetTestCertsDirectory(), kCert1.name));
+  EXPECT_TRUE(X509Certificate::IsSameOSCert(get_callback.cert_handle(),
+                                            cert->os_cert_handle()));
+}
+
+// Tests that the DiskBasedCertCache successfully writes to the cache
+// if the cache acts synchronously
+TEST(DiskBasedCertCache, SyncSet) {
+  ScopedMockTransaction trans1(
+      GetMockTransaction(kCert1.key, TEST_MODE_SYNC_ALL));
+  MockDiskCache backend;
+  DiskBasedCertCache cache(&backend);
+  scoped_refptr<X509Certificate> cert(
+      ImportCertFromFile(GetTestCertsDirectory(), kCert1.name));
+  ASSERT_TRUE(cert.get());
+
+  TestSetCallback set_callback;
+  cache.Set(cert->os_cert_handle(), set_callback.callback());
+  EXPECT_EQ(kCert1.key, set_callback.key());
+  CheckCertCached(&backend, kCert1);
+}
+
+// Tests that the DiskBasedCertCache successfully reads from the cache
+// if the cache acts synchronously
+TEST(DiskBasedCertCache, SyncGet) {
+  ScopedMockTransaction trans1(
+      GetMockTransaction(kCert1.key, TEST_MODE_SYNC_ALL));
+  MockDiskCache backend;
+  ImportCert(&backend, kCert1, false /* not corrupted */);
+  DiskBasedCertCache cache(&backend);
+  scoped_refptr<X509Certificate> cert(
+      ImportCertFromFile(GetTestCertsDirectory(), kCert1.name));
+  ASSERT_TRUE(cert.get());
+
+  TestGetCallback get_callback;
+  cache.Get(kCert1.key, get_callback.callback());
+  EXPECT_EQ(cert->os_cert_handle(), get_callback.cert_handle());
+}
+
+// Tests that Get will fail on a corrupted certificate.
+TEST(DiskBasedCertCache, GetBrokenCert) {
+  ScopedMockTransaction trans1(
+      GetMockTransaction(kCert1.key, TEST_MODE_NORMAL));
+  MockDiskCache backend;
+  ImportCert(&backend, kCert1, true /* corrupted */);
+  DiskBasedCertCache cache(&backend);
+  TestGetCallback get_callback;
+
+  cache.Get(kCert1.key, get_callback.callback());
+  get_callback.WaitForResult();
+
+  scoped_refptr<X509Certificate> cert(
+      ImportCertFromFile(GetTestCertsDirectory(), kCert1.name));
+  EXPECT_FALSE(get_callback.cert_handle());
+}
+
+// Tests that attempting to retrieve a cert that is not in the cache will
+// return NULL.
+TEST(DiskBasedCertCache, GetUncachedCert) {
+  ScopedMockTransaction trans1(
+      GetMockTransaction(kCert1.key, TEST_MODE_NORMAL));
+  MockDiskCache backend;
+  DiskBasedCertCache cache(&backend);
+  TestGetCallback get_callback;
+
+  cache.Get(kCert1.key, get_callback.callback());
+  get_callback.WaitForResult();
+  EXPECT_EQ(NULL, get_callback.cert_handle());
+}
+
+// Issues two requests to store a certificate in the cache
+// (simultaneously), and checks that the DiskBasedCertCache stores the
+// certificate to the cache (in one write rather than two).
+TEST(DiskBasedCertCache, SetMultiple) {
+  ScopedMockTransaction trans1(
+      GetMockTransaction(kCert1.key, TEST_MODE_NORMAL));
+  MockDiskCache backend;
+  DiskBasedCertCache cache(&backend);
+  scoped_refptr<X509Certificate> cert(
+      ImportCertFromFile(GetTestCertsDirectory(), kCert1.name));
+  ASSERT_TRUE(cert.get());
+  TestSetCallback set_callback1, set_callback2;
+
+  cache.Set(cert->os_cert_handle(), set_callback1.callback());
+  cache.Set(cert->os_cert_handle(), set_callback2.callback());

[Ryan Sleevi, 2014/06/25 19:31:57]

Can you add a note that describes how this is working

That is, something like the following (assuming my understanding is correct)

// These two operations will be combined into one operation
// because the cache is operating asynchronously (e.g. the
// first Set() won't proceed until the message loop is pumped,
// such as by WaitForResult())

+  set_callback1.WaitForResult();
+  set_callback2.WaitForResult();
+  EXPECT_EQ(set_callback1.key(), set_callback2.key());
+  CheckCertCached(&backend, kCert1);
+}
+
+// Stores a certificate in the DiskBasedCertCache, then retrieves it
+// and makes sure it was retrieved successfully.
+TEST(DiskBasedCertCache, SimpleSetAndGet) {
+  ScopedMockTransaction trans1(
+      GetMockTransaction(kCert1.key, TEST_MODE_NORMAL));
+  MockDiskCache backend;
+  DiskBasedCertCache cache(&backend);
+  scoped_refptr<X509Certificate> cert(
+      ImportCertFromFile(GetTestCertsDirectory(), kCert1.name));
+  ASSERT_TRUE(cert.get());
+  TestSetCallback set_callback;
+  TestGetCallback get_callback;
+
+  cache.Set(cert->os_cert_handle(), set_callback.callback());
+  set_callback.WaitForResult();
+  cache.Get(set_callback.key(), get_callback.callback());
+  get_callback.WaitForResult();
+  EXPECT_TRUE(X509Certificate::IsSameOSCert(get_callback.cert_handle(),
+                                            cert->os_cert_handle()));
+}
+
+// Tests some basic functionality of the DiskBasedCertCache, with multiple
+// set and get operations.
+TEST(DiskBasedCertCache, BasicUsage) {
+  ScopedMockTransaction trans1(
+      GetMockTransaction(kCert1.key, TEST_MODE_SYNC_CACHE_START));
+  ScopedMockTransaction trans2(
+      GetMockTransaction(kCert2.key, TEST_MODE_NORMAL));
+  MockDiskCache backend;
+  DiskBasedCertCache cache(&backend);
+  scoped_refptr<X509Certificate> cert1(
+      ImportCertFromFile(GetTestCertsDirectory(), kCert1.name));
+  scoped_refptr<X509Certificate> cert2(
+      ImportCertFromFile(GetTestCertsDirectory(), kCert2.name));
+  ASSERT_TRUE(cert1.get());
+  ASSERT_TRUE(cert2.get());
+  ASSERT_FALSE(X509Certificate::IsSameOSCert(cert1->os_cert_handle(),
+                                             cert2->os_cert_handle()));
+  TestSetCallback set_callback1, set_callback2;
+
+  cache.Set(cert1->os_cert_handle(), set_callback1.callback());
+  cache.Set(cert2->os_cert_handle(), set_callback2.callback());
+  set_callback1.WaitForResult();
+  set_callback2.WaitForResult();
+
+  TestGetCallback get_callback1, get_callback2;
+
+  cache.Get(set_callback1.key(), get_callback1.callback());
+  cache.Get(set_callback2.key(), get_callback2.callback());
+
+  get_callback1.WaitForResult();
+  get_callback2.WaitForResult();
+
+  EXPECT_TRUE(X509Certificate::IsSameOSCert(cert1->os_cert_handle(),
+                                            get_callback1.cert_handle()));
+  EXPECT_TRUE(X509Certificate::IsSameOSCert(cert2->os_cert_handle(),
+                                            get_callback2.cert_handle()));
+}
+
+// Test the result of simultaneous requests to store and retrieve a
+// certificate from the cache, with the get operation attempting to
+// open the cache first and therefore failing to open the entry.
+TEST(DiskBasedCertCache, SimultaneousGetSet) {
+  ScopedMockTransaction trans1(
+      GetMockTransaction(kCert1.key, TEST_MODE_SYNC_CACHE_START));
+  MockDiskCache backend;
+  DiskBasedCertCache cache(&backend);
+  scoped_refptr<X509Certificate> cert(
+      ImportCertFromFile(GetTestCertsDirectory(), kCert1.name));
+  ASSERT_TRUE(cert.get());
+
+  TestGetCallback get_callback;
+  TestSetCallback set_callback;
+
+  MockDiskEntry::IgnoreCallbacks(true);
+  cache.Get(kCert1.key, get_callback.callback());
+  cache.Set(cert->os_cert_handle(), set_callback.callback());
+  MockDiskEntry::IgnoreCallbacks(false);
+  get_callback.WaitForResult();
+  set_callback.WaitForResult();
+
+  EXPECT_EQ(NULL, get_callback.cert_handle());
+  EXPECT_EQ(kCert1.key, set_callback.key());
+}
+
+// Test the result of simultaneous requests to store and retrieve a
+// certificate from the cache, with the get operation opening the cache
+// after the set operation, leading to a successful read.
+TEST(DiskBasedCertCache, SimultaneousSetGet) {
+  ScopedMockTransaction trans1(
+      GetMockTransaction(kCert1.key, TEST_MODE_SYNC_CACHE_START));
+  MockDiskCache backend;
+  DiskBasedCertCache cache(&backend);
+  scoped_refptr<X509Certificate> cert(
+      ImportCertFromFile(GetTestCertsDirectory(), kCert1.name));
+  ASSERT_TRUE(cert.get());
+
+  TestSetCallback set_callback;
+  TestGetCallback get_callback;
+
+  MockDiskEntry::IgnoreCallbacks(true);
+  cache.Set(cert->os_cert_handle(), set_callback.callback());
+  cache.Get(kCert1.key, get_callback.callback());
+  MockDiskEntry::IgnoreCallbacks(false);
+  set_callback.WaitForResult();
+  get_callback.WaitForResult();
+
+  EXPECT_EQ(kCert1.key, set_callback.key());
+  EXPECT_TRUE(X509Certificate::IsSameOSCert(cert->os_cert_handle(),
+                                            get_callback.cert_handle()));
+}
+
+// Tests that the DiskBasedCertCache can be deleted without issues when
+// there are pending operations in the disk cache.
+TEST(DiskBasedCertCache, DeletedCertCache) {
+  ScopedMockTransaction trans1(
+      GetMockTransaction(kCert1.key, TEST_MODE_NORMAL));
+  MockDiskCache backend;
+  scoped_ptr<DiskBasedCertCache> cache(new DiskBasedCertCache(&backend));
+  scoped_refptr<X509Certificate> cert(
+      ImportCertFromFile(GetTestCertsDirectory(), kCert1.name));
+  ASSERT_TRUE(cert.get());
+  TestSetCallback set_callback;
+
+  cache->Set(cert->os_cert_handle(), set_callback.callback());
+  cache.reset();
+  set_callback.WaitForResult();
+  EXPECT_EQ(std::string(), set_callback.key());
+}
+
+}  // namespace net