Disk Based Certificate Cache Implementation

net/http/disk_based_cert_cache.cc

Index: net/http/disk_based_cert_cache.cc
diff --git a/net/http/disk_based_cert_cache.cc b/net/http/disk_based_cert_cache.cc
new file mode 100644
index 0000000000000000000000000000000000000000..b6c5b7e2731b5ede6215fefe0f2ffceba679f5e6
--- /dev/null
+++ b/net/http/disk_based_cert_cache.cc
@@ -0,0 +1,547 @@
+// 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 <vector>
+
+#include "base/bind.h"
+#include "base/callback_helpers.h"
+#include "base/memory/ref_counted.h"
+#include "base/stl_util.h"
+#include "base/strings/string_number_conversions.h"
+#include "net/base/io_buffer.h"
+#include "net/base/net_errors.h"
+#include "net/disk_cache/disk_cache.h"
+
+namespace net {
+
+namespace {
+
+// Used to obtain a unique cache key for a certificate in the form of
+// "cert:<hash>".
+std::string GetCacheKeyToCert(const X509Certificate::OSCertHandle cert_handle) {
+  SHA1HashValue fingerprint =
+      X509Certificate::CalculateFingerprint(cert_handle);
+
+  return "cert:" +
+         base::HexEncode(fingerprint.data, arraysize(fingerprint.data));
+}
+
+}  // namespace
+
+// WriteWorkers represent pending Set jobs in the DiskBasedCertCache. Each
+// certificate requested to be cached is assigned a Writeworker on a one-to-one
+// basis. The same certificate should not have multiple WriteWorkers at the same
+// time; instead, add a user callback to the existing WriteWorker.
+class DiskBasedCertCache::WriteWorker {
+ public:
+  // |backend| is the backend to store |certificate| in, using
+  // |key| as the key for the disk_cache::Entry.
+  // |cleanup_callback| is called to clean up this ReadWorker,
+  // regardless of success or failure.
+  WriteWorker(disk_cache::Backend* backend,
+              const std::string& key,
+              X509Certificate::OSCertHandle cert_handle,
+              const base::Closure& cleanup_callback);
+
+  ~WriteWorker();

[rvargas (doing something else), 2014/06/27 19:00:09]

nit: shouldn't this be private?

+
+  // Writes the given certificate to the cache. On completion, will invoke all
+  // user callbacks.
+  void Start();
+
+  // Adds a callback to the set of callbacks to be run when this
+  // WriteWorker finishes processing.
+  void AddCallback(const SetCallback& user_callback);
+
+  // Signals the WriteWorker to abort early. The WriteWorker will be destroyed
+  // upon the completion of any pending callbacks. User callbacks will be
+  // invoked with an empty string.
+  void Cancel();
+
+ private:
+  enum State {
+    STATE_CREATE,
+    STATE_CREATE_COMPLETE,
+    STATE_OPEN,
+    STATE_OPEN_COMPLETE,
+    STATE_WRITE,
+    STATE_WRITE_COMPLETE,
+    STATE_NONE
+  };
+
+  void OnIOComplete(int rv);
+  int DoLoop(int rv);
+
+  int DoCreate();
+  int DoCreateComplete(int rv);
+  int DoOpen();
+  int DoOpenComplete(int rv);
+  int DoWrite();
+  int DoWriteComplete(int rv);
+
+  void Finish(int rv);
+
+  // Invokes all of the |user_callbacks_|
+  void RunCallbacks(int rv);
+
+  disk_cache::Backend* backend_;
+  const X509Certificate::OSCertHandle cert_handle_;
+  std::string key_;
+  bool canceled_;
+
+  disk_cache::Entry* entry_;
+  State state_;
+  scoped_refptr<IOBuffer> buffer_;
+  int io_buf_len_;
+
+  base::Closure cleanup_callback_;
+  std::vector<SetCallback> user_callbacks_;
+  CompletionCallback io_callback_;
+};
+
+DiskBasedCertCache::WriteWorker::WriteWorker(
+    disk_cache::Backend* backend,
+    const std::string& key,
+    X509Certificate::OSCertHandle cert_handle,
+    const base::Closure& cleanup_callback)
+    : backend_(backend),
+      cert_handle_(cert_handle),
+      key_(key),
+      canceled_(false),
+      entry_(NULL),
+      state_(STATE_NONE),
+      io_buf_len_(0),
+      cleanup_callback_(cleanup_callback),
+      io_callback_(
+          base::Bind(&WriteWorker::OnIOComplete, base::Unretained(this))) {
+}
+
+void DiskBasedCertCache::WriteWorker::Start() {
+  DCHECK_EQ(STATE_NONE, state_);
+  state_ = STATE_CREATE;
+  int rv = DoLoop(OK);
+
+  if (rv == ERR_IO_PENDING)
+    return;
+
+  Finish(rv);
+}
+
+void DiskBasedCertCache::WriteWorker::AddCallback(
+    const SetCallback& user_callback) {
+  user_callbacks_.push_back(user_callback);
+}
+
+void DiskBasedCertCache::WriteWorker::OnIOComplete(int rv) {
+  if (canceled_) {
+    Finish(ERR_FAILED);
+    return;
+  }
+
+  rv = DoLoop(rv);
+
+  if (rv == ERR_IO_PENDING)
+    return;
+
+  Finish(rv);
+}
+
+int DiskBasedCertCache::WriteWorker::DoLoop(int rv) {
+  do {
+    State next_state = state_;
+    state_ = STATE_NONE;
+    switch (next_state) {
+      case STATE_CREATE:
+        rv = DoCreate();
+        break;
+      case STATE_CREATE_COMPLETE:
+        rv = DoCreateComplete(rv);
+        break;
+      case STATE_OPEN:
+        rv = DoOpen();
+        break;
+      case STATE_OPEN_COMPLETE:
+        rv = DoOpenComplete(rv);
+        break;
+      case STATE_WRITE:
+        rv = DoWrite();
+        break;
+      case STATE_WRITE_COMPLETE:
+        rv = DoWriteComplete(rv);
+        break;
+      case STATE_NONE:
+        NOTREACHED();
+        break;
+    }
+  } while (rv != ERR_IO_PENDING && state_ != STATE_NONE);
+
+  return rv;
+}
+
+int DiskBasedCertCache::WriteWorker::DoCreate() {
+  state_ = STATE_CREATE_COMPLETE;
+
+  return backend_->CreateEntry(key_, &entry_, io_callback_);
+}
+
+int DiskBasedCertCache::WriteWorker::DoCreateComplete(int rv) {
+  // An error here usually signifies that the entry already exists.
+  // If this occurs, it is necessary to instead open the previously
+  // existing entry.
+  if (rv < 0) {
+    state_ = STATE_OPEN;
+    return OK;
+  }
+
+  state_ = STATE_WRITE;
+  return OK;
+}
+
+int DiskBasedCertCache::WriteWorker::DoOpen() {
+  state_ = STATE_OPEN_COMPLETE;
+  return backend_->OpenEntry(key_, &entry_, io_callback_);
+}
+
+int DiskBasedCertCache::WriteWorker::DoOpenComplete(int rv) {
+  if (rv < 0) {
+    state_ = STATE_NONE;

[rvargas (doing something else), 2014/06/27 19:00:09]

nit: This should not be needed outside of DoLoop

[brandonsalmon, 2014/06/27 19:20:15]

Can you clarify?

[rvargas (doing something else), 2014/06/27 19:55:31]

DoLoop sets state_ to STATE_NONE and then calls the method that implements a
given state... so that method doesn't have to set state_ again (to the same)
value to indicate that the loop should break.

+    return rv;
+  }
+  state_ = STATE_WRITE;
+  return OK;
+}
+
+int DiskBasedCertCache::WriteWorker::DoWrite() {
+  std::string write_data;
+  bool encoded = X509Certificate::GetDEREncoded(cert_handle_, &write_data);
+
+  if (!encoded) {
+    state_ = STATE_NONE;
+    return ERR_FAILED;
+  }
+
+  buffer_ = new IOBuffer(write_data.size());
+  io_buf_len_ = write_data.size();
+  memcpy(buffer_->data(), write_data.data(), io_buf_len_);
+
+  state_ = STATE_WRITE_COMPLETE;
+
+  return entry_->WriteData(0 /* index */,
+                           0 /* offset */,
+                           buffer_,
+                           write_data.size(),
+                           io_callback_,
+                           true /* truncate */);
+}
+
+int DiskBasedCertCache::WriteWorker::DoWriteComplete(int rv) {
+  state_ = STATE_NONE;
+  if (rv < io_buf_len_)
+    return ERR_FAILED;
+
+  return OK;
+}
+
+void DiskBasedCertCache::WriteWorker::RunCallbacks(int rv) {
+  std::string key;
+  if (rv >= 0)
+    key = key_;
+
+  for (std::vector<SetCallback>::const_iterator it = user_callbacks_.begin();
+       it != user_callbacks_.end();
+       ++it) {
+    it->Run(key);
+  }
+  user_callbacks_.clear();
+}
+
+void DiskBasedCertCache::WriteWorker::Finish(int rv) {
+  cleanup_callback_.Run();
+  cleanup_callback_.Reset();
+  RunCallbacks(rv);
+  delete this;
+}
+
+void DiskBasedCertCache::WriteWorker::Cancel() {
+  canceled_ = true;
+}
+
+DiskBasedCertCache::WriteWorker::~WriteWorker() {

[rvargas (doing something else), 2014/06/27 19:00:09]

nit: follow the same order of methods between declaration and definition

+  if (entry_)
+    entry_->Close();
+}
+
+// ReadWorkers represent pending Get jobs in the DiskBasedCertCache. Each
+// certificate requested to be retrieved from the cache is assigned a ReadWorker
+// on a one-to-one basis. The same |key| should not have multiple ReadWorkers
+// at the same time; instead, call AddCallback to add a user_callback_ to
+// the the existing ReadWorker.
+class DiskBasedCertCache::ReadWorker {
+ public:
+  // |backend| is the backend to read |certificate| from, using
+  // |key| as the key for the disk_cache::Entry.
+  // |cleanup_callback| is called to clean up this ReadWorker,
+  // regardless of success or failure.
+  ReadWorker(disk_cache::Backend* backend,
+             const std::string& key,
+             const base::Closure& cleanup_callback);
+
+  ~ReadWorker();
+
+  // Reads the given certificate from the cache. On completion, will invoke all
+  // user callbacks.
+  void Start();
+
+  // Adds a callback to the set of callbacks to be run when this
+  // ReadWorker finishes processing.
+  void AddCallback(const GetCallback& user_callback);
+
+  // Signals the ReadWorker to abort early. The ReadWorker will be destroyed
+  // upon the completion of any pending callbacks. User callbacks will be
+  // invoked with a NULL cert handle.
+  void Cancel();
+
+ private:
+  enum State {
+    STATE_OPEN,
+    STATE_OPEN_COMPLETE,
+    STATE_READ,
+    STATE_READ_COMPLETE,
+    STATE_NONE
+  };
+
+  void OnIOComplete(int rv);
+  int DoLoop(int rv);
+  int DoOpen();
+  int DoOpenComplete(int rv);
+  int DoRead();
+  int DoReadComplete(int rv);
+  void Finish(int rv);
+
+  // Invokes all of |user_callbacks_|
+  void RunCallbacks();
+
+  disk_cache::Backend* backend_;
+  X509Certificate::OSCertHandle cert_handle_;
+  std::string key_;
+  bool canceled_;
+
+  disk_cache::Entry* entry_;
+
+  State state_;
+  scoped_refptr<IOBuffer> buffer_;
+  int io_buf_len_;
+
+  base::Closure cleanup_callback_;
+  std::vector<GetCallback> user_callbacks_;
+  CompletionCallback io_callback_;
+};
+
+DiskBasedCertCache::ReadWorker::ReadWorker(
+    disk_cache::Backend* backend,
+    const std::string& key,
+    const base::Closure& cleanup_callback)
+    : backend_(backend),
+      cert_handle_(NULL),
+      key_(key),
+      canceled_(false),
+      entry_(NULL),
+      state_(STATE_NONE),
+      io_buf_len_(0),
+      cleanup_callback_(cleanup_callback),
+      io_callback_(
+          base::Bind(&ReadWorker::OnIOComplete, base::Unretained(this))) {
+}
+
+void DiskBasedCertCache::ReadWorker::Start() {
+  DCHECK_EQ(STATE_NONE, state_);
+  state_ = STATE_OPEN;
+  int rv = DoLoop(OK);
+
+  if (rv == ERR_IO_PENDING)
+    return;
+
+  Finish(rv);
+}
+
+void DiskBasedCertCache::ReadWorker::AddCallback(
+    const GetCallback& user_callback) {
+  user_callbacks_.push_back(user_callback);
+}
+
+void DiskBasedCertCache::ReadWorker::OnIOComplete(int rv) {
+  if (canceled_) {
+    Finish(ERR_FAILED);
+    return;
+  }
+
+  rv = DoLoop(rv);
+
+  if (rv == ERR_IO_PENDING)
+    return;
+
+  Finish(rv);
+}
+
+int DiskBasedCertCache::ReadWorker::DoLoop(int rv) {
+  do {
+    State next_state = state_;
+    state_ = STATE_NONE;
+    switch (next_state) {
+      case STATE_OPEN:
+        rv = DoOpen();
+        break;
+      case STATE_OPEN_COMPLETE:
+        rv = DoOpenComplete(rv);
+        break;
+      case STATE_READ:
+        rv = DoRead();
+        break;
+      case STATE_READ_COMPLETE:
+        rv = DoReadComplete(rv);
+        break;
+      case STATE_NONE:
+        NOTREACHED();
+        break;
+    }
+  } while (rv != ERR_IO_PENDING && state_ != STATE_NONE);
+
+  return rv;
+}
+
+int DiskBasedCertCache::ReadWorker::DoOpen() {
+  state_ = STATE_OPEN_COMPLETE;
+  return backend_->OpenEntry(key_, &entry_, io_callback_);
+}
+
+int DiskBasedCertCache::ReadWorker::DoOpenComplete(int rv) {
+  if (rv < 0) {
+    state_ = STATE_NONE;
+    return rv;
+  }
+  state_ = STATE_READ;
+  return OK;
+}
+
+int DiskBasedCertCache::ReadWorker::DoRead() {
+  state_ = STATE_READ_COMPLETE;
+  io_buf_len_ = entry_->GetDataSize(0 /* index */);
+  buffer_ = new IOBuffer(io_buf_len_);
+  return entry_->ReadData(
+      0 /* index */, 0 /* offset */, buffer_, io_buf_len_, io_callback_);

[rvargas (doing something else), 2014/06/27 19:00:09]

nit: this is a list of arguments where none of them is long enough to prevent
indentation from the function call so they should probably start on the previous
line and continue under the first one when reaching 80 cols:
   ->Foo(bar, stuff, variable,
         something_else_that_doesnt_fit);

+}
+
+int DiskBasedCertCache::ReadWorker::DoReadComplete(int rv) {
+  state_ = STATE_NONE;
+  if (rv < io_buf_len_)
+    return ERR_FAILED;
+
+  cert_handle_ = X509Certificate::CreateOSCertHandleFromBytes(buffer_->data(),
+                                                              io_buf_len_);
+  if (!cert_handle_)
+    return ERR_FAILED;
+
+  return OK;
+}
+
+void DiskBasedCertCache::ReadWorker::RunCallbacks() {
+  for (std::vector<GetCallback>::const_iterator it = user_callbacks_.begin();
+       it != user_callbacks_.end();
+       ++it) {
+    it->Run(cert_handle_);
+  }
+  user_callbacks_.clear();
+}
+
+void DiskBasedCertCache::ReadWorker::Finish(int rv) {
+  cleanup_callback_.Run();
+  cleanup_callback_.Reset();
+  RunCallbacks();
+  delete this;
+}
+
+void DiskBasedCertCache::ReadWorker::Cancel() {
+  canceled_ = true;
+}
+
+DiskBasedCertCache::ReadWorker::~ReadWorker() {
+  if (entry_)
+    entry_->Close();
+  if (cert_handle_)
+    X509Certificate::FreeOSCertHandle(cert_handle_);
+}
+
+DiskBasedCertCache::DiskBasedCertCache(disk_cache::Backend* backend)
+    : backend_(backend), weak_factory_(this) {
+  DCHECK(backend_);
+}
+
+DiskBasedCertCache::~DiskBasedCertCache() {
+  for (WriteWorkerMap::iterator it = write_worker_map_.begin();
+       it != write_worker_map_.end();
+       ++it) {
+    it->second->Cancel();
+  }
+  for (ReadWorkerMap::iterator it = read_worker_map_.begin();
+       it != read_worker_map_.end();
+       ++it) {
+    it->second->Cancel();
+  }
+}
+
+void DiskBasedCertCache::Get(const std::string& key, const GetCallback& cb) {
+  DCHECK(!key.empty());
+
+  ReadWorkerMap::iterator it = read_worker_map_.find(key);
+
+  if (it == read_worker_map_.end()) {
+    ReadWorker* worker =
+        new ReadWorker(backend_,
+                       key,
+                       base::Bind(&DiskBasedCertCache::FinishedReadOperation,
+                                  weak_factory_.GetWeakPtr(),
+                                  key));
+    read_worker_map_[key] = worker;

[rvargas (doing something else), 2014/06/27 19:00:09]

nit: this pattern (map_.find() + map_[]=) searches the map twice. It can be
replaced with map_.insert()

[brandonsalmon, 2014/06/27 19:20:14]

Does this still apply even when the key isn't found, and .find() returns the end
of the map?

[rvargas (doing something else), 2014/06/27 19:55:31]

I'm not sure I understand the question. If find() returns an entry there is no
overhead. However, when find() returns end(), it has to search the map in order
to do so, and then the insertion here has to go through the map again to find
out where the new entry should go.

insert() can do both things with a single lookup. It returns either where the
new entry was inserted, or the existing entry. Creating a new worker can be
postponed until insert() returns and we know that there was no match.

+    worker->AddCallback(cb);
+    worker->Start();
+  } else {
+    it->second->AddCallback(cb);
+  }
+}
+
+void DiskBasedCertCache::Set(const X509Certificate::OSCertHandle cert_handle,
+                             const SetCallback& cb) {
+  DCHECK(!cb.is_null());
+  DCHECK(cert_handle);
+  std::string key = GetCacheKeyToCert(cert_handle);
+
+  WriteWorkerMap::iterator it = write_worker_map_.find(key);
+
+  if (it == write_worker_map_.end()) {
+    WriteWorker* worker =
+        new WriteWorker(backend_,
+                        key,
+                        cert_handle,
+                        base::Bind(&DiskBasedCertCache::FinishedWriteOperation,
+                                   weak_factory_.GetWeakPtr(),
+                                   key));
+    write_worker_map_[key] = worker;
+    worker->AddCallback(cb);
+    worker->Start();
+  } else {
+    it->second->AddCallback(cb);
+  }
+}
+
+void DiskBasedCertCache::FinishedWriteOperation(const std::string& key) {
+  write_worker_map_.erase(key);
+}
+
+void DiskBasedCertCache::FinishedReadOperation(const std::string& key) {
+  read_worker_map_.erase(key);
+}
+
+}  // namespace net