Disk Based Certificate Cache Implementation

net/http/disk_based_cert_cache_unittest.cc

+// 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 requires constant use of the
+// certificates in GetTestCertsDirectory(). The TestCertMetaData
+// struct stores metadata relevant to the DiskBasedCertCache for
+// each used test certificate.
+struct TestCertMetaData {
+  const char* file_name;
+  const char* key;

[Ryan Sleevi, 2014/06/25 23:24:01]

s/key/cache_key/

+};
+
+const TestCertMetaData kCert1{"root_ca_cert.pem",
+                              "cert:4C005EF1CF45F80D4A5A2BCFB00D4F198121E8D4"};
+
+const TestCertMetaData kCert2{"ok_cert.pem",
+                              "cert:9174C7CB9E4919604E7B1BFC430E4929DA45F65F"};
+
+// 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 CreateMockTransaction(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.file_name));
+  std::string write_data;
+  bool encoded =
+      X509Certificate::GetDEREncoded(cert->os_cert_handle(), &write_data);
+  ASSERT_TRUE(encoded);
+  if (corrupt_data) {
+    for (unsigned int i = 0; i < write_data.size(); i += 20)

[Ryan Sleevi, 2014/06/25 23:24:01]

s/unsigned int/size_t/

Always use the appropriately sized type, and match the type as it's defined
(this will fail to compile on at least one platform)

[Ryan Sleevi, 2014/06/25 23:24:01]

nit: This loop is "dangerous", and were it not test code, would be worth fixing.
because |i| is unsigned, it wraps. If .size() were >
std::numeric_limits<unsigned int>::max() - 20, this loop would run on forever.

These can alternatively be written out as:
for (size_t i = 0; i < write_data.size(); ++i) {
  if (!(i % 20))
    ++write_data[i];
}

That said, it's pedantic here, because we know it will be <= int (line 137/143),
it's just more a "More you know"

+      write_data[i]++;

[Ryan Sleevi, 2014/06/25 23:24:01]

nit: ++write_data[i] (slightly more performant)

+  }
+  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(static_cast<int>(write_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.file_name));
+  std::string write_data;
+  bool encoded =
+      X509Certificate::GetDEREncoded(cert->os_cert_handle(), &write_data);
+  ASSERT_TRUE(encoded);
+  int entry_size = entry->GetDataSize(0 /* index */);
+  scoped_refptr<IOBuffer> buffer(new IOBuffer(entry_size));
+  rv = entry->ReadData(
+      0 /* index */, 0 /* offset */, buffer, entry_size, callback.callback());
+  EXPECT_EQ(entry_size, callback.GetResult(rv));
+  X509Certificate::OSCertHandle cached_cert_handle =
+      X509Certificate::CreateOSCertHandleFromBytes(buffer->data(), entry_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(
+      CreateMockTransaction(kCert1.key, TEST_MODE_NORMAL));
+  MockDiskCache backend;
+  DiskBasedCertCache cache(&backend);
+  scoped_refptr<X509Certificate> cert(
+      ImportCertFromFile(GetTestCertsDirectory(), kCert1.file_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());
+  ASSERT_NO_FATAL_FAILURE(CheckCertCached(&backend, kCert1));
+}
+
+// Tests that a certificate can be retrieved from the cache.
+TEST(DiskBasedCertCache, GetCert) {
+  ScopedMockTransaction trans1(
+      CreateMockTransaction(kCert1.key, TEST_MODE_NORMAL));
+  MockDiskCache backend;
+  ASSERT_NO_FATAL_FAILURE(
+      ImportCert(&backend, kCert1, false /* not corrupted */));
+  DiskBasedCertCache cache(&backend);
+  TestGetCallback get_callback;
+
+  cache.Get(kCert1.key, get_callback.callback());
+  get_callback.WaitForResult();
+
+  scoped_refptr<X509Certificate> cert(
+      ImportCertFromFile(GetTestCertsDirectory(), kCert1.file_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(
+      CreateMockTransaction(kCert1.key, TEST_MODE_SYNC_ALL));
+  MockDiskCache backend;
+  DiskBasedCertCache cache(&backend);
+  scoped_refptr<X509Certificate> cert(
+      ImportCertFromFile(GetTestCertsDirectory(), kCert1.file_name));
+  ASSERT_TRUE(cert.get());
+
+  TestSetCallback set_callback;
+  cache.Set(cert->os_cert_handle(), set_callback.callback());
+  EXPECT_EQ(kCert1.key, set_callback.key());
+  ASSERT_NO_FATAL_FAILURE(CheckCertCached(&backend, kCert1));
+}
+
+// Tests that the DiskBasedCertCache successfully reads from the cache
+// if the cache acts synchronously
+TEST(DiskBasedCertCache, SyncGet) {
+  ScopedMockTransaction trans1(
+      CreateMockTransaction(kCert1.key, TEST_MODE_SYNC_ALL));
+  MockDiskCache backend;
+  ASSERT_NO_FATAL_FAILURE(
+      (ImportCert(&backend, kCert1, false /* not corrupted */)));
+  DiskBasedCertCache cache(&backend);
+  scoped_refptr<X509Certificate> cert(
+      ImportCertFromFile(GetTestCertsDirectory(), kCert1.file_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(
+      CreateMockTransaction(kCert1.key, TEST_MODE_NORMAL));
+  MockDiskCache backend;
+  ASSERT_NO_FATAL_FAILURE(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.file_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(
+      CreateMockTransaction(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(
+      CreateMockTransaction(kCert1.key, TEST_MODE_NORMAL));
+  MockDiskCache backend;
+  DiskBasedCertCache cache(&backend);
+  scoped_refptr<X509Certificate> cert(
+      ImportCertFromFile(GetTestCertsDirectory(), kCert1.file_name));
+  ASSERT_TRUE(cert.get());
+  TestSetCallback set_callback1, set_callback2;
+
+  // Behind the scenes, these two operations will be combined
+  // into one operation. IgnoreCallbacks guarantees that the
+  // first Set operation is not yet complete when the second Set is
+  // called, and then IgnoreCallbacks(false) continues the
+  // (combined) operation in the |cache|.
+  MockDiskEntry::IgnoreCallbacks(true);
+  cache.Set(cert->os_cert_handle(), set_callback1.callback());
+  cache.Set(cert->os_cert_handle(), set_callback2.callback());
+  MockDiskEntry::IgnoreCallbacks(false);
+
+  set_callback1.WaitForResult();
+  set_callback2.WaitForResult();
+  EXPECT_EQ(set_callback1.key(), set_callback2.key());
+  ASSERT_NO_FATAL_FAILURE(CheckCertCached(&backend, kCert1));
+}
+
+// Issues two requests to store a certificate in the cache
+// because the first transaction finishes before the second
+// one is issued, the first cache write is overwritten.
+TEST(DiskBasedCertCache, SetOverwrite) {
+  ScopedMockTransaction trans1(
+      CreateMockTransaction(kCert1.key, TEST_MODE_NORMAL));
+  MockDiskCache backend;
+  backend.set_double_create_check(false);
+  DiskBasedCertCache cache(&backend);
+  scoped_refptr<X509Certificate> cert(
+      ImportCertFromFile(GetTestCertsDirectory(), kCert1.file_name));
+  ASSERT_TRUE(cert.get());
+  TestSetCallback set_callback1, set_callback2;
+
+  cache.Set(cert->os_cert_handle(), set_callback1.callback());
+  set_callback1.WaitForResult();
+  cache.Set(cert->os_cert_handle(), set_callback2.callback());
+  set_callback2.WaitForResult();
+
+  EXPECT_EQ(set_callback1.key(), set_callback2.key());
+  ASSERT_NO_FATAL_FAILURE(CheckCertCached(&backend, kCert1));
+}
+
+// Stores a certificate in the DiskBasedCertCache, then retrieves it
+// and makes sure it was retrieved successfully.
+TEST(DiskBasedCertCache, SimpleSetAndGet) {
+  ScopedMockTransaction trans1(
+      CreateMockTransaction(kCert1.key, TEST_MODE_NORMAL));
+  MockDiskCache backend;
+  DiskBasedCertCache cache(&backend);
+  scoped_refptr<X509Certificate> cert(
+      ImportCertFromFile(GetTestCertsDirectory(), kCert1.file_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(
+      CreateMockTransaction(kCert1.key, TEST_MODE_SYNC_CACHE_START));
+  ScopedMockTransaction trans2(
+      CreateMockTransaction(kCert2.key, TEST_MODE_NORMAL));
+  MockDiskCache backend;
+  DiskBasedCertCache cache(&backend);
+  scoped_refptr<X509Certificate> cert1(
+      ImportCertFromFile(GetTestCertsDirectory(), kCert1.file_name));
+  scoped_refptr<X509Certificate> cert2(
+      ImportCertFromFile(GetTestCertsDirectory(), kCert2.file_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;
+
+  MockDiskEntry::IgnoreCallbacks(true);

[Ryan Sleevi, 2014/06/25 23:24:01]

Document why you're doing these ignore callbacks.

+  cache.Set(cert1->os_cert_handle(), set_callback1.callback());
+  cache.Set(cert2->os_cert_handle(), set_callback2.callback());
+  MockDiskEntry::IgnoreCallbacks(false);
+  set_callback1.WaitForResult();
+  set_callback2.WaitForResult();
+
+  TestGetCallback get_callback1, get_callback2;
+
+  MockDiskEntry::IgnoreCallbacks(true);
+  cache.Get(set_callback1.key(), get_callback1.callback());
+  cache.Get(set_callback2.key(), get_callback2.callback());
+  MockDiskEntry::IgnoreCallbacks(false);
+  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(
+      CreateMockTransaction(kCert1.key, TEST_MODE_SYNC_CACHE_START));
+  MockDiskCache backend;
+  DiskBasedCertCache cache(&backend);
+  scoped_refptr<X509Certificate> cert(
+      ImportCertFromFile(GetTestCertsDirectory(), kCert1.file_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(
+      CreateMockTransaction(kCert1.key, TEST_MODE_SYNC_CACHE_START));
+  MockDiskCache backend;
+  DiskBasedCertCache cache(&backend);
+  scoped_refptr<X509Certificate> cert(
+      ImportCertFromFile(GetTestCertsDirectory(), kCert1.file_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(
+      CreateMockTransaction(kCert1.key, TEST_MODE_NORMAL));
+  MockDiskCache backend;
+  scoped_ptr<DiskBasedCertCache> cache(new DiskBasedCertCache(&backend));
+  scoped_refptr<X509Certificate> cert(
+      ImportCertFromFile(GetTestCertsDirectory(), kCert1.file_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