Rewrite session cache in OpenSSL ports.

net/ssl/ssl_client_session_cache_openssl.cc

Index: net/ssl/ssl_client_session_cache_openssl.cc
diff --git a/net/ssl/ssl_client_session_cache_openssl.cc b/net/ssl/ssl_client_session_cache_openssl.cc
new file mode 100644
index 0000000000000000000000000000000000000000..7f273cebeddc84d418edfa3e1ec8a196548c7a58
--- /dev/null
+++ b/net/ssl/ssl_client_session_cache_openssl.cc
@@ -0,0 +1,104 @@
+// 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/ssl/ssl_client_session_cache_openssl.h"
+
+#include <utility>
+
+#include "base/logging.h"
+#include "base/time/clock.h"
+#include "base/time/default_clock.h"
+
+namespace net {
+
+SSLClientSessionCacheOpenSSL::SSLClientSessionCacheOpenSSL(const Config& config)
+    : clock_(new base::DefaultClock),
+      config_(config),
+      cache_(config.max_entries),
+      lookups_since_flush_(0) {
+}
+
+SSLClientSessionCacheOpenSSL::~SSLClientSessionCacheOpenSSL() {
+  // TODO(davidben): The session cache is currently a singleton, so it is
+  // destroyed on a different thread than the one it's created on. When
+  // https://crbug.com/458365 is fixed, this will no longer be an issue.
+  thread_checker_.DetachFromThread();
+
+  Flush();
+}
+
+size_t SSLClientSessionCacheOpenSSL::size() const {
+  return cache_.size();
+}
+
+SSL_SESSION* SSLClientSessionCacheOpenSSL::Lookup(
+    const std::string& cache_key) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+
+  // Expire stale sessions.
+  lookups_since_flush_++;
+  if (lookups_since_flush_ >= config_.expiration_check_count) {
+    lookups_since_flush_ = 0;
+    FlushExpiredSessions();
+  }
+
+  CacheEntryMap::iterator iter = cache_.Get(cache_key);
+  if (iter == cache_.end())
+    return nullptr;
+  if (iter->second->IsExpired(clock_->Now())) {
+    cache_.Erase(iter);
+    return nullptr;
+  }
+  return iter->second->session.get();
+}
+
+void SSLClientSessionCacheOpenSSL::Insert(const std::string& cache_key,
+                                          SSL_SESSION* session) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+
+  // Make a new entry.
+  CacheEntry* entry = new CacheEntry;
+  entry->session.reset(SSL_SESSION_up_ref(session));
+  entry->expiration = clock_->Now() + config_.timeout;
+
+  // Takes ownership.
+  cache_.Put(cache_key, entry);
+}
+
+void SSLClientSessionCacheOpenSSL::Flush() {
+  DCHECK(thread_checker_.CalledOnValidThread());
+
+  cache_.Clear();
+}
+
+void SSLClientSessionCacheOpenSSL::SetClockForTesting(
+    scoped_ptr<base::Clock> clock) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+
+  clock_ = clock.Pass();
+}
+
+SSLClientSessionCacheOpenSSL::CacheEntry::CacheEntry() {
+}
+
+SSLClientSessionCacheOpenSSL::CacheEntry::~CacheEntry() {
+}
+
+bool SSLClientSessionCacheOpenSSL::CacheEntry::IsExpired(base::Time now) {
+  return now > expiration;

[Ryan Sleevi, 2015/04/02 06:53:15]

I still can't help but feel like this is going to lead to a bad time - possibly
even a security bug!

Does it make sense to store either creation time / tick counts with it, to
ensure that things are sane? I'd rather pay the memory cost overhead of getting
the clocks right than run the risk of some sort of session replay *handwave*
thing

[davidben, 2015/04/02 07:21:45]

We could, but I'm not sure I see the difference. The check is now >
creation_time + timeout. Both of those are constant, so we may as well store the
precomputed sum. Is the worry that you might double-insert a session and refresh
the creation time? Storing the creation time would have the same failure mode.
Either we get confused and refresh things or we don't. I think that's best
avoided by only inserting fresh SSL_SESSIONs into the cache, which is what the
SSL_session_reused check in ssl_client_socket_openssl.cc buys.

(I'm confused what session replay means. There's nothing saying you can't offer
to resume a session multiple times. Replay is what the client_random is for. The
only concern I can imagine is if you want to promise that session key material
doesn't last longer than $TIMEOUT since resumption doesn't have forward secrecy.
This cache does manage that by virtue of line 49. The old one, incidentally,
didn't because it only evaluated expiration on FlushExpiredSessions. OpenSSL's
internal cache is similar.)

[Ryan Sleevi, 2015/04/02 07:46:55]

I meant re-using a session beyond the timelimit configured that we've deemed
acceptable. That is, there's nothing (server side) guaranteeing that we aren't
re-using a session for > 24 hours, for example.

I'm thinking of scenarios like
https://code.google.com/p/chromium/codesearch#chromium/src/net/cert/multi_thr...

which was a reaction to http://crbug.com/132124

TL;DR: Store the time we stored the session. If Now < that time, treat it as
expired (since we can no longer reason about the time difference). If Now > that
time, then make sure Now < expiration time. Otherwise, treat it as expired
(since it truly is expired)

[davidben, 2015/04/02 19:05:10]

Ah, okay. Done.

+}
+
+void SSLClientSessionCacheOpenSSL::FlushExpiredSessions() {
+  base::Time now = clock_->Now();
+  CacheEntryMap::iterator iter = cache_.begin();
+  while (iter != cache_.end()) {
+    if (iter->second->IsExpired(now)) {
+      iter = cache_.Erase(iter);
+    } else {
+      ++iter;
+    }
+  }
+}
+
+}  // namespace net