Implements openssl session caching

Also fixes up the ssl socket handling of the OpenSSL error stack, and resolves a few TODOs.

BUG=None
TEST=opening https: pages with vlog=3 enabled and expected the log

Review URL: http://codereview.chromium.org/5100010

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@67087 0039d316-1c4b-4281-b951-d872f2087c98

2 files changed, 172 insertions, 56 deletions

diff --git a/net/socket/ssl_client_socket_openssl.cc b/net/socket/ssl_client_socket_openssl.cc
index 62f3dbb..9aaca41 100644
--- a/net/socket/ssl_client_socket_openssl.cc
+++ b/net/socket/ssl_client_socket_openssl.cc
@@ -10,8 +10,10 @@
 #include <openssl/ssl.h>
 #include <openssl/err.h>
 
+#include "base/lock.h"
 #include "base/metrics/histogram.h"
 #include "base/openssl_util.h"
+#include "base/singleton.h"
 #include "net/base/cert_verifier.h"
 #include "net/base/net_errors.h"
 #include "net/base/ssl_connection_status_flags.h"
@@ -31,15 +33,8 @@ namespace {
 #endif
 
 const size_t kMaxRecvBufferSize = 4096;
-
-void MaybeLogSSLError() {
-  int error_num;
-  while ((error_num = ERR_get_error()) != 0) {
-    char buf[128];  // this buffer must be at least 120 chars long.
-    ERR_error_string_n(error_num, buf, arraysize(buf));
-    DVLOG(1) << "SSL error " << error_num << ": " << buf;
-  }
-}
+const int kSessionCacheTimeoutSeconds = 60 * 60;
+const size_t kSessionCacheMaxEntires = 1024;
 
 int MapOpenSSLError(int err) {
   switch (err) {
@@ -48,12 +43,10 @@ int MapOpenSSLError(int err) {
       return ERR_IO_PENDING;
     case SSL_ERROR_SYSCALL:
       DVLOG(1) << "OpenSSL SYSCALL error, errno " << errno;
-      MaybeLogSSLError();
       return ERR_SSL_PROTOCOL_ERROR;
     default:
       // TODO(joth): Implement full mapping.
       LOG(WARNING) << "Unknown OpenSSL error " << err;
-      MaybeLogSSLError();
       return ERR_SSL_PROTOCOL_ERROR;
   }
 }
@@ -65,21 +58,149 @@ int NoOpVerifyCallback(X509_STORE_CTX*, void *) {
   return 1;
 }
 
-struct SSLContextSingletonTraits : public DefaultSingletonTraits<SSL_CTX> {
-  static SSL_CTX* New() {
-    base::EnsureOpenSSLInit();
-    SSL_CTX* self = SSL_CTX_new(SSLv23_client_method());
-    SSL_CTX_set_cert_verify_callback(self, NoOpVerifyCallback, NULL);
-    return self;
+// OpenSSL manages a cache of SSL_SESSION, this class provides the application
+// side policy for that cache about session re-use: we retain one session per
+// unique HostPortPair.
+class SSLSessionCache {
+ public:
+  SSLSessionCache() {}
+
+  void OnSessionAdded(const HostPortPair& host_and_port, SSL_SESSION* session) {
+    // Declare the session cleaner-upper before the lock, so any call into
+    // OpenSSL to free the session will happen after the lock is released.
+    base::ScopedOpenSSL<SSL_SESSION, SSL_SESSION_free> session_to_free;
+    AutoLock lock(lock_);
+
+    DCHECK_EQ(0U, session_map_.count(session));
+    std::pair<HostPortMap::iterator, bool> res =
+        host_port_map_.insert(std::make_pair(host_and_port, session));
+    if (!res.second) {  // Already exists: replace old entry.
+      session_to_free.reset(res.first->second);
+      session_map_.erase(session_to_free.get());
+      res.first->second = session;
+    }
+    DVLOG(2) << "Adding session " << session << " => "
+             << host_and_port.ToString() << ", new entry = " << res.second;
+    DCHECK(host_port_map_[host_and_port] == session);
+    session_map_[session] = res.first;
+    DCHECK_EQ(host_port_map_.size(), session_map_.size());
+    DCHECK_LE(host_port_map_.size(), kSessionCacheMaxEntires);
   }
-  static void Delete(SSL_CTX* self) {
-    SSL_CTX_free(self);
+
+  void OnSessionRemoved(SSL_SESSION* session) {
+    // Declare the session cleaner-upper before the lock, so any call into
+    // OpenSSL to free the session will happen after the lock is released.
+    base::ScopedOpenSSL<SSL_SESSION, SSL_SESSION_free> session_to_free;
+    AutoLock lock(lock_);
+
+    SessionMap::iterator it = session_map_.find(session);
+    if (it == session_map_.end())
+      return;
+    DVLOG(2) << "Remove session " << session << " => "
+             << it->second->first.ToString();
+    DCHECK(it->second->second == session);
+    host_port_map_.erase(it->second);
+    session_map_.erase(it);
+    session_to_free.reset(session);
+    DCHECK_EQ(host_port_map_.size(), session_map_.size());
   }
+
+  // Looks up the host:port in the cache, and if a session is found it is added
+  // to |ssl|, returning true on success.
+  bool SetSSLSession(SSL* ssl, const HostPortPair& host_and_port) {
+    AutoLock lock(lock_);
+    HostPortMap::iterator it = host_port_map_.find(host_and_port);
+    if (it == host_port_map_.end())
+      return false;
+    DVLOG(2) << "Lookup session: " << it->second << " => "
+             << host_and_port.ToString();
+    SSL_SESSION* session = it->second;
+    DCHECK(session);
+    DCHECK(session_map_[session] == it);
+    // Ideally we'd release |lock_| before calling into OpenSSL here, however
+    // that opens a small risk |session| will go out of scope before it is used.
+    // Alternatively we would take a temporary local refcount on |session|,
+    // except OpenSSL does not provide a public API for adding a ref (c.f.
+    // SSL_SESSION_free which decrements the ref).
+    return SSL_set_session(ssl, session) == 1;
+  }
+
+ private:
+  // A pair of maps to allow bi-directional lookups between host:port and an
+  // associated seesion.
+  // TODO(joth): When client certificates are implemented we should key the
+  // cache on the client certificate used in addition to the host-port pair.
+  typedef std::map<HostPortPair, SSL_SESSION*> HostPortMap;
+  typedef std::map<SSL_SESSION*, HostPortMap::iterator> SessionMap;
+  HostPortMap host_port_map_;
+  SessionMap session_map_;
+
+  // Protects access to both the above maps.
+  Lock lock_;
+
+  DISALLOW_COPY_AND_ASSIGN(SSLSessionCache);
 };
 
-SSL_CTX* GetSSLContext() {
-  return Singleton<SSL_CTX, SSLContextSingletonTraits>::get();
-}
+class SSLContext {
+ public:
+  static SSLContext* Get() { return Singleton<SSLContext>::get(); }
+  SSL_CTX* ssl_ctx() { return ssl_ctx_.get(); }
+  SSLSessionCache* session_cache() { return &session_cache_; }
+
+  SSLClientSocketOpenSSL* GetClientSocketFromSSL(SSL* ssl) {
+    DCHECK(ssl);
+    SSLClientSocketOpenSSL* socket = static_cast<SSLClientSocketOpenSSL*>(
+        SSL_get_ex_data(ssl, ssl_socket_data_index_));
+    DCHECK(socket);
+    return socket;
+  }
+
+  bool SetClientSocketForSSL(SSL* ssl, SSLClientSocketOpenSSL* socket) {
+    return SSL_set_ex_data(ssl, ssl_socket_data_index_, socket) != 0;
+  }
+
+ private:
+  friend struct DefaultSingletonTraits<SSLContext>;
+
+  SSLContext() {
+    base::EnsureOpenSSLInit();
+    ssl_socket_data_index_ = SSL_get_ex_new_index(0, 0, 0, 0, 0);
+    DCHECK_NE(ssl_socket_data_index_, -1);
+    ssl_ctx_.reset(SSL_CTX_new(SSLv23_client_method()));
+    SSL_CTX_set_cert_verify_callback(ssl_ctx_.get(), NoOpVerifyCallback, NULL);
+    SSL_CTX_set_session_cache_mode(ssl_ctx_.get(), SSL_SESS_CACHE_CLIENT);
+    SSL_CTX_sess_set_new_cb(ssl_ctx_.get(), NewSessionCallbackStatic);
+    SSL_CTX_sess_set_remove_cb(ssl_ctx_.get(), RemoveSessionCallbackStatic);
+    SSL_CTX_set_timeout(ssl_ctx_.get(), kSessionCacheTimeoutSeconds);
+    SSL_CTX_sess_set_cache_size(ssl_ctx_.get(), kSessionCacheMaxEntires);
+  }
+
+  static int NewSessionCallbackStatic(SSL* ssl, SSL_SESSION* session) {
+    return Get()->NewSessionCallback(ssl, session);
+  }
+
+  int NewSessionCallback(SSL* ssl, SSL_SESSION* session) {
+    SSLClientSocketOpenSSL* socket = GetClientSocketFromSSL(ssl);
+    session_cache_.OnSessionAdded(socket->host_and_port(), session);
+    return 1;  // 1 => We took ownership of |session|.
+  }
+
+  static void RemoveSessionCallbackStatic(SSL_CTX* ctx, SSL_SESSION* session) {
+    return Get()->RemoveSessionCallback(ctx, session);
+  }
+
+  void RemoveSessionCallback(SSL_CTX* ctx, SSL_SESSION* session) {
+    DCHECK(ctx == ssl_ctx());
+    session_cache_.OnSessionRemoved(session);
+  }
+
+  // This is the index used with SSL_get_ex_data to retrieve the owner
+  // SSLClientSocketOpenSSL object from an SSL instance.
+  int ssl_socket_data_index_;
+
+  base::ScopedOpenSSL<SSL_CTX, SSL_CTX_free> ssl_ctx_;
+  SSLSessionCache session_cache_;
+};
 
 }  // namespace
 
@@ -96,6 +217,7 @@ SSLClientSocketOpenSSL::SSLClientSocketOpenSSL(
       user_connect_callback_(NULL),
       user_read_callback_(NULL),
       user_write_callback_(NULL),
+      completed_handshake_(false),
       client_auth_cert_needed_(false),
       ALLOW_THIS_IN_INITIALIZER_LIST(handshake_io_callback_(
           this, &SSLClientSocketOpenSSL::OnHandshakeIOComplete)),
@@ -104,7 +226,7 @@ SSLClientSocketOpenSSL::SSLClientSocketOpenSSL(
       transport_(transport_socket),
       host_and_port_(host_and_port),
       ssl_config_(ssl_config),
-      completed_handshake_(false),
+      trying_cached_session_(false),
       net_log_(transport_socket->socket()->NetLog()) {
 }
 
@@ -116,23 +238,23 @@ bool SSLClientSocketOpenSSL::Init() {
   DCHECK(!ssl_);
   DCHECK(!transport_bio_);
 
-  ssl_ = SSL_new(GetSSLContext());
-  if (!ssl_) {
-    MaybeLogSSLError();
+  SSLContext* context = SSLContext::Get();
+  base::OpenSSLErrStackTracer err_tracer(FROM_HERE);
+
+  ssl_ = SSL_new(context->ssl_ctx());
+  if (!ssl_ || !context->SetClientSocketForSSL(ssl_, this))
     return false;
-  }
 
-  if (!SSL_set_tlsext_host_name(ssl_, host_and_port_.host().c_str())) {
-    MaybeLogSSLError();
+  if (!SSL_set_tlsext_host_name(ssl_, host_and_port_.host().c_str()))
     return false;
-  }
+
+  trying_cached_session_ =
+      context->session_cache()->SetSSLSession(ssl_, host_and_port_);
 
   BIO* ssl_bio = NULL;
-  // TODO(joth): Provide explicit write buffer sizes, rather than use defaults?
-  if (!BIO_new_bio_pair(&ssl_bio, 0, &transport_bio_, 0)) {
-    MaybeLogSSLError();
+  // 0 => use default buffer sizes.
+  if (!BIO_new_bio_pair(&ssl_bio, 0, &transport_bio_, 0))
     return false;
-  }
   DCHECK(ssl_bio);
   DCHECK(transport_bio_);
 
@@ -286,11 +408,11 @@ void SSLClientSocketOpenSSL::Disconnect() {
   user_write_buf_        = NULL;
   user_write_buf_len_    = 0;
 
-  client_certs_.clear();
-  client_auth_cert_needed_ = false;
-
   server_cert_verify_result_.Reset();
   completed_handshake_ = false;
+
+  client_certs_.clear();
+  client_auth_cert_needed_ = false;
 }
 
 int SSLClientSocketOpenSSL::DoHandshakeLoop(int last_io_result) {
@@ -336,10 +458,16 @@ int SSLClientSocketOpenSSL::DoHandshakeLoop(int last_io_result) {
 }
 
 int SSLClientSocketOpenSSL::DoHandshake() {
+  base::OpenSSLErrStackTracer err_tracer(FROM_HERE);
   int net_error = net::OK;
   int rv = SSL_do_handshake(ssl_);
 
   if (rv == 1) {
+    if (trying_cached_session_ && logging::DEBUG_MODE) {
+      DVLOG(2) << "Result of session reuse for " << host_and_port_.ToString()
+               << " is: " << (SSL_session_reused(ssl_) ? "Success" : "Fail");
+    }
+
     // SSL handshake is completed.  Let's verify the certificate.
     const bool got_cert = !!UpdateServerCert();
     DCHECK(got_cert);
@@ -355,7 +483,6 @@ int SSLClientSocketOpenSSL::DoHandshake() {
       LOG(ERROR) << "handshake failed; returned " << rv
                  << ", SSL error code " << ssl_error
                  << ", net_error " << net_error;
-      MaybeLogSSLError();
     }
   }
   return net_error;
@@ -400,28 +527,11 @@ int SSLClientSocketOpenSSL::DoVerifyCertComplete(int result) {
   }
 
   completed_handshake_ = true;
-  // The NSS version has a comment that we may not need this call because it is
-  // now harmless to have a session with a bad cert.
-  // This may or may not apply here, but let's invalidate it anyway.
-  InvalidateSessionIfBadCertificate();
   // Exit DoHandshakeLoop and return the result to the caller to Connect.
   DCHECK_EQ(STATE_NONE, next_handshake_state_);
   return result;
 }
 
-void SSLClientSocketOpenSSL::InvalidateSessionIfBadCertificate() {
-  if (UpdateServerCert() != NULL &&
-      ssl_config_.IsAllowedBadCert(server_cert_)) {
-    // Remove from session cache but don't clear this connection.
-    // TODO(joth): This should be a no-op until we enable session caching,
-    // see SSL_CTX_set_session_cache_mode(SSL_SESS_CACHE_CLIENT).
-    SSL_SESSION* session = SSL_get_session(ssl_);
-    LOG_IF(ERROR, session) << "Connection has a session?? " << session;
-    int rv = SSL_CTX_remove_session(GetSSLContext(), session);
-    LOG_IF(ERROR, rv) << "Session was cached?? " << rv;
-  }
-}
-
 X509Certificate* SSLClientSocketOpenSSL::UpdateServerCert() {
   if (server_cert_)
     return server_cert_;
@@ -736,6 +846,7 @@ bool SSLClientSocketOpenSSL::SetSendBufferSize(int32 size) {
 }
 
 int SSLClientSocketOpenSSL::DoPayloadRead() {
+  base::OpenSSLErrStackTracer err_tracer(FROM_HERE);
   int rv = SSL_read(ssl_, user_read_buf_->data(), user_read_buf_len_);
   // We don't need to invalidate the non-client-authenticated SSL session
   // because the server will renegotiate anyway.
@@ -750,6 +861,7 @@ int SSLClientSocketOpenSSL::DoPayloadRead() {
 }
 
 int SSLClientSocketOpenSSL::DoPayloadWrite() {
+  base::OpenSSLErrStackTracer err_tracer(FROM_HERE);
   int rv = SSL_write(ssl_, user_write_buf_->data(), user_write_buf_len_);
 
   if (rv >= 0)
diff --git a/net/socket/ssl_client_socket_openssl.h b/net/socket/ssl_client_socket_openssl.h
index 783132d..e7bfe3c 100644
--- a/net/socket/ssl_client_socket_openssl.h
+++ b/net/socket/ssl_client_socket_openssl.h
@@ -36,6 +36,8 @@ class SSLClientSocketOpenSSL : public SSLClientSocket {
                          const SSLConfig& ssl_config);
   ~SSLClientSocketOpenSSL();
 
+  const HostPortPair& host_and_port() const { return host_and_port_; }
+
   // SSLClientSocket methods:
   virtual void GetSSLInfo(SSLInfo* ssl_info);
   virtual void GetSSLCertRequestInfo(SSLCertRequestInfo* cert_request_info);
@@ -111,6 +113,7 @@ class SSLClientSocketOpenSSL : public SSLClientSocket {
   // Set when handshake finishes.
   scoped_refptr<X509Certificate> server_cert_;
   CertVerifyResult server_cert_verify_result_;
+  bool completed_handshake_;
 
   // Stores client authentication information between ClientAuthHandler and
   // GetSSLCertRequestInfo calls.
@@ -128,7 +131,8 @@ class SSLClientSocketOpenSSL : public SSLClientSocket {
   const HostPortPair host_and_port_;
   SSLConfig ssl_config_;
 
-  bool completed_handshake_;
+  // Used for session cache diagnostics.
+  bool trying_cached_session_;
 
   enum State {
     STATE_NONE,