From 4aa4fe56e17fe783df9bcfe9f4ed4a368d262aac Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Thu, 28 Apr 2016 22:25:55 +0900 Subject: [PATCH] nghttpx: Destroy SSL object, and always lookup TLS session cache --- src/shrpx_config.h | 8 -------- src/shrpx_connection.cc | 17 ++++------------- src/shrpx_http2_session.cc | 23 ++++++++++------------- src/shrpx_http_downstream_connection.cc | 5 +++-- src/shrpx_live_check.cc | 5 +++-- src/shrpx_ssl.cc | 14 +++++--------- src/shrpx_ssl.h | 14 +++++++++++--- src/shrpx_worker.h | 3 ++- 8 files changed, 38 insertions(+), 51 deletions(-) diff --git a/src/shrpx_config.h b/src/shrpx_config.h index 30463568..fb7f9f2e 100644 --- a/src/shrpx_config.h +++ b/src/shrpx_config.h @@ -319,14 +319,6 @@ struct UpstreamAddr { int fd; }; -struct TLSSessionCache { - // ASN1 representation of SSL_SESSION object. See - // i2d_SSL_SESSION(3SSL). - std::vector session_data; - // The last time stamp when this cache entry is created or updated. - ev_tstamp last_updated; -}; - struct DownstreamAddrConfig { Address addr; // backend address. If |host_unix| is true, this is UNIX domain diff --git a/src/shrpx_connection.cc b/src/shrpx_connection.cc index 8a5814bc..428d98c4 100644 --- a/src/shrpx_connection.cc +++ b/src/shrpx_connection.cc @@ -81,13 +81,7 @@ Connection::Connection(struct ev_loop *loop, int fd, SSL *ssl, } } -Connection::~Connection() { - disconnect(); - - if (tls.ssl) { - SSL_free(tls.ssl); - } -} +Connection::~Connection() { disconnect(); } void Connection::disconnect() { if (tls.ssl) { @@ -104,12 +98,9 @@ void Connection::disconnect() { tls.cached_session_lookup_req = nullptr; } - // To reuse SSL/TLS session, we have to shutdown, and don't free - // tls.ssl. - if (SSL_shutdown(tls.ssl) != 1) { - SSL_free(tls.ssl); - tls.ssl = nullptr; - } + SSL_shutdown(tls.ssl); + SSL_free(tls.ssl); + tls.ssl = nullptr; tls.wbuf.reset(); tls.rbuf.reset(); diff --git a/src/shrpx_http2_session.cc b/src/shrpx_http2_session.cc index 55f601ce..4211565f 100644 --- a/src/shrpx_http2_session.cc +++ b/src/shrpx_http2_session.cc @@ -351,19 +351,15 @@ int Http2Session::initiate_connection() { SSLOG(INFO, this) << "Connecting to downstream server"; } if (ssl_ctx_) { - // We are establishing TLS connection. If conn_.tls.ssl, we may - // reuse the previous session. - if (!conn_.tls.ssl) { - auto ssl = ssl::create_ssl(ssl_ctx_); - if (!ssl) { - return -1; - } - - ssl::setup_downstream_http2_alpn(ssl); - - conn_.set_ssl(ssl); + auto ssl = ssl::create_ssl(ssl_ctx_); + if (!ssl) { + return -1; } + ssl::setup_downstream_http2_alpn(ssl); + + conn_.set_ssl(ssl); + auto sni_name = !get_config()->tls.backend_sni_name.empty() ? StringRef(get_config()->tls.backend_sni_name) : StringRef(addr_->host); @@ -375,7 +371,7 @@ int Http2Session::initiate_connection() { SSL_set_tlsext_host_name(conn_.tls.ssl, sni_name.c_str()); } - auto tls_session = ssl::reuse_tls_session(addr_); + auto tls_session = ssl::reuse_tls_session(addr_->tls_session_cache); if (tls_session) { SSL_set_session(conn_.tls.ssl, tls_session); SSL_SESSION_free(tls_session); @@ -1845,7 +1841,8 @@ int Http2Session::tls_handshake() { if (!SSL_session_reused(conn_.tls.ssl)) { auto tls_session = SSL_get0_session(conn_.tls.ssl); if (tls_session) { - ssl::try_cache_tls_session(addr_, tls_session, ev_now(conn_.loop)); + ssl::try_cache_tls_session(addr_->tls_session_cache, addr_->addr, + tls_session, ev_now(conn_.loop)); } } diff --git a/src/shrpx_http_downstream_connection.cc b/src/shrpx_http_downstream_connection.cc index 2b1fd267..ed7191f0 100644 --- a/src/shrpx_http_downstream_connection.cc +++ b/src/shrpx_http_downstream_connection.cc @@ -246,7 +246,7 @@ int HttpDownstreamConnection::attach_downstream(Downstream *downstream) { SSL_set_tlsext_host_name(conn_.tls.ssl, sni_name.c_str()); } - auto session = ssl::reuse_tls_session(addr_); + auto session = ssl::reuse_tls_session(addr_->tls_session_cache); if (session) { SSL_set_session(conn_.tls.ssl, session); SSL_SESSION_free(session); @@ -916,7 +916,8 @@ int HttpDownstreamConnection::tls_handshake() { if (!SSL_session_reused(conn_.tls.ssl)) { auto session = SSL_get0_session(conn_.tls.ssl); if (session) { - ssl::try_cache_tls_session(addr_, session, ev_now(conn_.loop)); + ssl::try_cache_tls_session(addr_->tls_session_cache, addr_->addr, session, + ev_now(conn_.loop)); } } diff --git a/src/shrpx_live_check.cc b/src/shrpx_live_check.cc index f523016c..a71a352b 100644 --- a/src/shrpx_live_check.cc +++ b/src/shrpx_live_check.cc @@ -199,7 +199,7 @@ int LiveCheck::initiate_connection() { SSL_set_tlsext_host_name(conn_.tls.ssl, sni_name.c_str()); } - auto session = ssl::reuse_tls_session(addr_); + auto session = ssl::reuse_tls_session(addr_->tls_session_cache); if (session) { SSL_set_session(conn_.tls.ssl, session); SSL_SESSION_free(session); @@ -277,7 +277,8 @@ int LiveCheck::tls_handshake() { if (!SSL_session_reused(conn_.tls.ssl)) { auto tls_session = SSL_get0_session(conn_.tls.ssl); if (tls_session) { - ssl::try_cache_tls_session(addr_, tls_session, ev_now(conn_.loop)); + ssl::try_cache_tls_session(addr_->tls_session_cache, addr_->addr, + tls_session, ev_now(conn_.loop)); } } diff --git a/src/shrpx_ssl.cc b/src/shrpx_ssl.cc index 624e0dbc..4a73d694 100644 --- a/src/shrpx_ssl.cc +++ b/src/shrpx_ssl.cc @@ -1445,13 +1445,11 @@ std::vector serialize_ssl_session(SSL_SESSION *session) { } } // namespace -void try_cache_tls_session(DownstreamAddr *addr, SSL_SESSION *session, - ev_tstamp t) { - auto &cache = addr->tls_session_cache; - +void try_cache_tls_session(TLSSessionCache &cache, const Address &addr, + SSL_SESSION *session, ev_tstamp t) { if (cache.last_updated + 1_min > t) { if (LOG_ENABLED(INFO)) { - LOG(INFO) << "Cache for addr=" << util::to_numeric_addr(&addr->addr) + LOG(INFO) << "Cache for addr=" << util::to_numeric_addr(&addr) << " is still host. Not updating."; } return; @@ -1459,7 +1457,7 @@ void try_cache_tls_session(DownstreamAddr *addr, SSL_SESSION *session, if (LOG_ENABLED(INFO)) { LOG(INFO) << "Update cache entry for SSL_SESSION=" << session - << ", addr=" << util::to_numeric_addr(&addr->addr) + << ", addr=" << util::to_numeric_addr(&addr) << ", timestamp=" << std::fixed << std::setprecision(6) << t; } @@ -1467,9 +1465,7 @@ void try_cache_tls_session(DownstreamAddr *addr, SSL_SESSION *session, cache.last_updated = t; } -SSL_SESSION *reuse_tls_session(const DownstreamAddr *addr) { - auto &cache = addr->tls_session_cache; - +SSL_SESSION *reuse_tls_session(const TLSSessionCache &cache) { if (cache.session_data.empty()) { return nullptr; } diff --git a/src/shrpx_ssl.h b/src/shrpx_ssl.h index eced6479..ea3a47cd 100644 --- a/src/shrpx_ssl.h +++ b/src/shrpx_ssl.h @@ -51,6 +51,14 @@ struct UpstreamAddr; namespace ssl { +struct TLSSessionCache { + // ASN1 representation of SSL_SESSION object. See + // i2d_SSL_SESSION(3SSL). + std::vector session_data; + // The last time stamp when this cache entry is created or updated. + ev_tstamp last_updated; +}; + // This struct stores the additional information per SSL_CTX. This is // attached to SSL_CTX using SSL_CTX_set_app_data(). struct TLSContextData { @@ -229,12 +237,12 @@ bool tls_hostname_match(const StringRef &pattern, const StringRef &hostname); // |session| is serialized into ASN1 representation, and stored. |t| // is used as a time stamp. Depending on the existing cache's time // stamp, |session| might not be cached. -void try_cache_tls_session(DownstreamAddr *addr, SSL_SESSION *session, - ev_tstamp t); +void try_cache_tls_session(TLSSessionCache &cache, const Address &addr, + SSL_SESSION *session, ev_tstamp t); // Returns cached session associated |addr|. If no cache entry is // found associated to |addr|, nullptr will be returned. -SSL_SESSION *reuse_tls_session(const DownstreamAddr *addr); +SSL_SESSION *reuse_tls_session(const TLSSessionCache &addr); } // namespace ssl diff --git a/src/shrpx_worker.h b/src/shrpx_worker.h index 7e9c8c5d..0f3b1d34 100644 --- a/src/shrpx_worker.h +++ b/src/shrpx_worker.h @@ -45,6 +45,7 @@ #include "shrpx_config.h" #include "shrpx_downstream_connection_pool.h" #include "memchunk.h" +#include "shrpx_ssl.h" using namespace nghttp2; @@ -84,7 +85,7 @@ struct DownstreamAddr { size_t fall; size_t rise; // Client side TLS session cache - TLSSessionCache tls_session_cache; + ssl::TLSSessionCache tls_session_cache; // Http2Session object created for this address. This list chains // all Http2Session objects that is not in group scope // http2_avail_freelist, and is not reached in maximum concurrency.