GnuTLS: support for TLS session resume (part 2)

diff --git a/src/CachePeer.cc b/src/CachePeer.cc
index fac6a3e5ba1da99042476ce452b7db4b798e58ba..964948ad5b15e3df0d6e2db69f84b6ce0b299f74 100644 (file)
--- a/src/CachePeer.cc
+++ b/src/CachePeer.cc
@@ -42,7 +42,6 @@ CachePeer::CachePeer() :
     domain(NULL),
 #if USE_OPENSSL
     sslContext(NULL),
-    sslSession(NULL),
 #endif
     front_end_https(0),
     connection_auth(2 /* auto */)
@@ -102,9 +101,6 @@ CachePeer::~CachePeer()
 #if USE_OPENSSL
     if (sslContext)
         SSL_CTX_free(sslContext);
-
-    if (sslSession)
-        SSL_SESSION_free(sslSession);
 #endif
 }
 

diff --git a/src/CachePeer.h b/src/CachePeer.h
index 848150df31fa8e97c9547029f49b635af64d8361..67a6302ae399c53d83c0da696c3a2404ebda120b 100644 (file)
--- a/src/CachePeer.h
+++ b/src/CachePeer.h
@@ -184,9 +184,7 @@ public:
     /// security settings for peer connection
     Security::PeerOptions secure;
     Security::ContextPtr sslContext;
-#if USE_OPENSSL
-    SSL_SESSION *sslSession;
-#endif
+    Security::SessionStatePointer sslSession;
 
     int front_end_https;
     int connection_auth;

diff --git a/src/Makefile.am b/src/Makefile.am
index 11180a08273ac4759a738febea78998644459e35..7557134021d70e80be2778cc507e30154a05bab6 100644 (file)
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -547,8 +547,8 @@ squid_LDADD = \
        fs/libfs.la \
        DiskIO/libdiskio.la \
        comm/libcomm.la \
-       $(SSL_LIBS) \
        security/libsecurity.la \
+       $(SSL_LIBS) \
        ipc/libipc.la \
        mgr/libmgr.la \
        anyp/libanyp.la \

diff --git a/src/adaptation/icap/ServiceRep.cc b/src/adaptation/icap/ServiceRep.cc
index 7aec5ca4f2f623f7a076f7bb8518d80aab71a8f1..14996a7fa8226e92bd305353b4c7c8cfc97df78d 100644 (file)
--- a/src/adaptation/icap/ServiceRep.cc
+++ b/src/adaptation/icap/ServiceRep.cc
@@ -34,9 +34,6 @@ CBDATA_NAMESPACED_CLASS_INIT(Adaptation::Icap, ServiceRep);
 Adaptation::Icap::ServiceRep::ServiceRep(const ServiceConfigPointer &svcCfg):
     AsyncJob("Adaptation::Icap::ServiceRep"), Adaptation::Service(svcCfg),
     sslContext(NULL),
-#if USE_OPENSSL
-    sslSession(NULL),
-#endif
     theOptions(NULL), theOptionsFetcher(0), theLastUpdate(0),
     theBusyConns(0),
     theAllWaiters(0),

diff --git a/src/adaptation/icap/ServiceRep.h b/src/adaptation/icap/ServiceRep.h
index 39e9bf57b02d0a84477b55c31b271019c302c212..579bfbd574f84126b97d6f65493cf09c717a5bfc 100644 (file)
--- a/src/adaptation/icap/ServiceRep.h
+++ b/src/adaptation/icap/ServiceRep.h
@@ -111,9 +111,7 @@ public: // treat these as private, they are for callbacks only
     virtual void noteAdaptationAnswer(const Answer &answer);
 
     Security::ContextPtr sslContext;
-#if USE_OPENSSL
-    SSL_SESSION *sslSession;
-#endif
+    Security::SessionStatePointer sslSession;
 
 private:
     // stores Prepare() callback info

diff --git a/src/adaptation/icap/Xaction.cc b/src/adaptation/icap/Xaction.cc
index 79ee247440d57b85588daf938c1bcbbb37f8c88e..74b03938e371e66460d008655c8731168cf02089 100644 (file)
--- a/src/adaptation/icap/Xaction.cc
+++ b/src/adaptation/icap/Xaction.cc
@@ -710,22 +710,18 @@ Ssl::IcapPeerConnector::initialize(Security::SessionPointer &serverSession)
     if (!Security::PeerConnector::initialize(serverSession))
         return false;
 
-#if USE_OPENSSL
     assert(!icapService->cfg().secure.sslDomain.isEmpty());
+#if USE_OPENSSL
     SBuf *host = new SBuf(icapService->cfg().secure.sslDomain);
     SSL_set_ex_data(serverSession.get(), ssl_ex_index_server, host);
 
     ACLFilledChecklist *check = static_cast<ACLFilledChecklist *>(SSL_get_ex_data(serverSession.get(), ssl_ex_index_cert_error_check));
     if (check)
         check->dst_peer_name = *host;
+#endif
 
-    if (icapService->sslSession)
-        SSL_set_session(serverSession.get(), icapService->sslSession);
-
+    Security::SetSessionResumeData(serverSession, icapService->sslSession);
     return true;
-#else
-    return false;
-#endif
 }
 
 void
@@ -734,16 +730,8 @@ Ssl::IcapPeerConnector::noteNegotiationDone(ErrorState *error)
     if (error)
         return;
 
-#if USE_OPENSSL
     const int fd = serverConnection()->fd;
-    auto ssl = fd_table[fd].ssl.get();
-    assert(ssl);
-    if (!SSL_session_reused(ssl)) {
-        if (icapService->sslSession)
-            SSL_SESSION_free(icapService->sslSession);
-        icapService->sslSession = SSL_get1_session(ssl);
-    }
-#endif
+    Security::MaybeGetSessionResumeData(fd_table[fd].ssl, icapService->sslSession);
 }
 
 void

diff --git a/src/client_side.cc b/src/client_side.cc
index bcc25d4410d4fbe619b074fc064ab7b95546eec7..1d83a43fb003ac59fe7201628431a84734f4015d 100644 (file)
--- a/src/client_side.cc
+++ b/src/client_side.cc
@@ -2663,7 +2663,7 @@ clientNegotiateSSL(int fd, void *data)
         return;
     }
 
-    if (SSL_session_reused(ssl)) {
+    if (Security::SessionIsResumed(fd_table[fd].ssl)) {
         debugs(83, 2, "clientNegotiateSSL: Session " << SSL_get_session(ssl) <<
                " reused on FD " << fd << " (" << fd_table[fd].ipaddr << ":" << (int)fd_table[fd].remote_port << ")");
     } else {

diff --git a/src/security/BlindPeerConnector.cc b/src/security/BlindPeerConnector.cc
index 54631f33050592fa9ab8f279fee5385ff596febe..578e0e18cd5ef516d3c179af7313113fd662e937 100644 (file)
--- a/src/security/BlindPeerConnector.cc
+++ b/src/security/BlindPeerConnector.cc
@@ -47,8 +47,7 @@ Security::BlindPeerConnector::initialize(Security::SessionPointer &serverSession
         SBuf *host = new SBuf(peer->secure.sslDomain);
         SSL_set_ex_data(serverSession.get(), ssl_ex_index_server, host);
 
-        if (peer->sslSession)
-            SSL_set_session(serverSession.get(), peer->sslSession);
+        Security::SetSessionResumeData(serverSession, peer->sslSession);
     } else {
         SBuf *hostName = new SBuf(request->url.host());
         SSL_set_ex_data(serverSession.get(), ssl_ex_index_server, (void*)hostName);
@@ -71,15 +70,9 @@ Security::BlindPeerConnector::noteNegotiationDone(ErrorState *error)
         return;
     }
 
-#if USE_OPENSSL
-    const int fd = serverConnection()->fd;
-    Security::SessionPtr ssl = fd_table[fd].ssl.get();
-    if (serverConnection()->getPeer() && !SSL_session_reused(ssl)) {
-        if (serverConnection()->getPeer()->sslSession)
-            SSL_SESSION_free(serverConnection()->getPeer()->sslSession);
-
-        serverConnection()->getPeer()->sslSession = SSL_get1_session(ssl);
+    if (auto *peer = serverConnection()->getPeer()) {
+        const int fd = serverConnection()->fd;
+        Security::MaybeGetSessionResumeData(fd_table[fd].ssl, peer->sslSession);
     }
-#endif
 }
 

diff --git a/src/security/Session.cc b/src/security/Session.cc
index 38480c14385b0f560c5b517e5ba149eeb440ee4f..cf717926f77fabb8be6fffebcd866d2ebbd67454 100644 (file)
--- a/src/security/Session.cc
+++ b/src/security/Session.cc
@@ -6,9 +6,12 @@
  * Please see the COPYING and CONTRIBUTORS files for details.
  */
 
+/* DEBUG: section 83    TLS session management */
+
 #include "squid.h"
 #include "anyp/PortCfg.h"
 #include "base/RunnersRegistry.h"
+#include "Debug.h"
 #include "ipc/MemMap.h"
 #include "security/Session.h"
 #include "SquidConfig.h"
@@ -16,6 +19,66 @@
 #define SSL_SESSION_ID_SIZE 32
 #define SSL_SESSION_MAX_SIZE 10*1024
 
+bool
+Security::SessionIsResumed(const Security::SessionPointer &s)
+{
+    bool result = false;
+#if USE_OPENSSL
+    result = SSL_session_reused(s.get()) == 1;
+#elif USE_GNUTLS
+    result = gnutls_session_is_resumed(s.get()) != 0;
+#endif
+    debugs(83, 7, "session=" << (void*)s.get() << ", query? answer: " << (result ? 'T' : 'F') );
+    return result;
+}
+
+void
+Security::MaybeGetSessionResumeData(const Security::SessionPointer &s, Security::SessionStatePointer &data)
+{
+    if (!SessionIsResumed(s)) {
+#if USE_OPENSSL
+        // nil is valid for SSL_get1_session(), it cannot fail.
+        data.reset(SSL_get1_session(s.get()));
+#elif USE_GNUTLS
+        gnutls_datum_t *tmp = nullptr;
+        const auto x = gnutls_session_get_data2(s.get(), tmp);
+        if (x != GNUTLS_E_SUCCESS) {
+            debugs(83, 3, "session=" << (void*)s.get() << " error: " << gnutls_strerror(x));
+        }
+        data.reset(tmp);
+#endif
+        debugs(83, 5, "session=" << (void*)s.get() << " data=" << (void*)data.get());
+    } else {
+        debugs(83, 5, "session=" << (void*)s.get() << " data=" << (void*)data.get() << ", do nothing.");
+    }
+}
+
+void
+Security::SetSessionResumeData(const Security::SessionPointer &s, const Security::SessionStatePointer &data)
+{
+    if (data) {
+#if USE_OPENSSL
+        if (!SSL_set_session(s.get(), data.get())) {
+            const auto ssl_error = ERR_get_error();
+            debugs(83, 3, "session=" << (void*)s.get() << " data=" << (void*)data.get() <<
+                   " resume error: " << ERR_error_string(ssl_error, nullptr));
+        }
+#elif USE_GNUTLS
+        const auto x = gnutls_session_set_data(s.get(), data->data, data->size);
+        if (x != GNUTLS_E_SUCCESS) {
+            debugs(83, 3, "session=" << (void*)s.get() << " data=" << (void*)data.get() <<
+                   " resume error: " << gnutls_strerror(x));
+        }
+#else
+        // critical because, how did it get here?
+        debugs(83, DBG_CRITICAL, "no TLS library. session=" << (void*)s.get() << " data=" << (void*)data.get());
+#endif
+        debugs(83, 5, "session=" << (void*)s.get() << " data=" << (void*)data.get());
+    } else {
+        debugs(83, 5, "session=" << (void*)s.get() << " no resume data");
+    }
+}
+
 static bool
 isTlsServer()
 {

diff --git a/src/security/Session.h b/src/security/Session.h
index de9c3b785d494ccab838ffe36de797d623a9c058..e7ce105709ec4884745ff2b3c9ccbd6096bb63b9 100644 (file)
--- a/src/security/Session.h
+++ b/src/security/Session.h
@@ -9,6 +9,7 @@
 #ifndef SQUID_SRC_SECURITY_SESSION_H
 #define SQUID_SRC_SECURITY_SESSION_H
 
+#include "base/HardFun.h"
 #include "security/LockingPointer.h"
 
 #include <memory>
@@ -32,6 +33,8 @@ typedef SSL* SessionPtr;
 CtoCpp1(SSL_free, SSL *);
 typedef LockingPointer<SSL, Security::SSL_free_cpp, CRYPTO_LOCK_SSL> SessionPointer;
 
+typedef std::unique_ptr<SSL_SESSION, HardFun<void, SSL_SESSION*, &SSL_SESSION_free>> SessionStatePointer;
+
 #elif USE_GNUTLS
 typedef gnutls_session_t SessionPtr;
 // Locks can be implemented attaching locks counter to gnutls_session_t
@@ -40,14 +43,38 @@ typedef gnutls_session_t SessionPtr;
 CtoCpp1(gnutls_deinit, gnutls_session_t);
 typedef LockingPointer<struct gnutls_session_int, gnutls_deinit_cpp, -1> SessionPointer;
 
+// wrapper function to get around gnutls_free being a typedef
+inline void squid_gnutls_free(void *d) {gnutls_free(d);}
+typedef std::unique_ptr<gnutls_datum_t, HardFun<void, void*, &Security::squid_gnutls_free>> SessionStatePointer;
+
 #else
 // use void* so we can check against NULL
 typedef void* SessionPtr;
 CtoCpp1(xfree, SessionPtr);
 typedef LockingPointer<void, xfree_cpp, -1> SessionPointer;
 
+typedef std::unique_ptr<int> SessionStatePointer;
+
 #endif
 
+/// whether the session is a resumed one
+bool SessionIsResumed(const Security::SessionPointer &);
+
+/**
+ * When the session is not a resumed session, retrieve the details needed to
+ * resume a later connection and store them in 'data'. This may result in 'data'
+ * becoming a nil Pointer if no details exist or an error occurs.
+ *
+ * When the session is already a resumed session, do nothing and leave 'data'
+ * unhanged.
+ * XXX: is this latter behaviour always correct?
+ */
+void MaybeGetSessionResumeData(const Security::SessionPointer &, Security::SessionStatePointer &data);
+
+/// Set the data for resuming a previous session.
+/// Needs to be done before using the SessionPointer for a handshake.
+void SetSessionResumeData(const Security::SessionPointer &, const Security::SessionStatePointer &);
+
 } // namespace Security
 
 #endif /* SQUID_SRC_SECURITY_SESSION_H */

diff --git a/src/tests/stub_libsecurity.cc b/src/tests/stub_libsecurity.cc
index 95f88d2cf6860767c4e040ca3b886ade4970eb70..304d2bb17c7413365cc2600105a54dae720402f8 100644 (file)
--- a/src/tests/stub_libsecurity.cc
+++ b/src/tests/stub_libsecurity.cc
@@ -87,3 +87,10 @@ Security::ContextPtr Security::ServerOptions::createBlankContext() const STUB
 bool Security::ServerOptions::createStaticServerContext(AnyP::PortCfg &) STUB_RETVAL(false)
 void Security::ServerOptions::updateContextEecdh(Security::ContextPtr &) STUB
 
+#include "security/Session.h"
+namespace Security {
+bool SessionIsResumed(const Security::SessionPointer &) STUB_RETVAL(false)
+void MaybeGetSessionResumeData(const Security::SessionPointer &, Security::SessionStatePointer &) STUB
+void SetSessionResumeData(const Security::SessionPointer &, const Security::SessionStatePointer &) STUB
+} // namespace Security
+