Move session tls-options= assignment out of CreateSession() function

The original implementation utilized OpenSSL inheritence of
library-specific options from SSL_CTX* to SSL* structures.
This does not work with GnuTLS which requires setting the
options on each session object.

The workaround of looking up the session options inside CreateSession()
was only looking for client connection details. Which is broken when
creating server sessions.

* Extend Security::CreateSession() to take a PeerOptions reference
  containing the specific config details to associate with the new
  session (if any).

* Move selection of CachePeer vs tls_outgoing_* options
  out to Security::CreateClientSession().

* Pass the Any::PortCfg::secure details into the
  Security::CreateServerSession().

diff --git a/src/client_side.cc b/src/client_side.cc
index 4aaf8f6d668122a330d73bc6b7c09ab3fb90656c..0510409760ffd2afdeb397c97ab14c82c7790a38 100644 (file)
--- a/src/client_side.cc
+++ b/src/client_side.cc
@@ -2340,9 +2340,10 @@ httpAccept(const CommAcceptCbParams &params)
 
 /// Create TLS connection structure and update fd_table
 static bool
-httpsCreate(const Comm::ConnectionPointer &conn, const Security::ContextPointer &ctx)
+httpsCreate(const ConnStateData *connState, const Security::ContextPointer &ctx)
 {
-    if (Security::CreateServerSession(ctx, conn, "client https start")) {
+    const auto conn = connState->clientConnection;
+    if (Security::CreateServerSession(ctx, conn, connState->port->secure, "client https start")) {
         debugs(33, 5, "will negotiate TLS on " << conn);
         return true;
     }
@@ -2527,7 +2528,7 @@ httpsEstablish(ConnStateData *connState, const Security::ContextPointer &ctx)
     assert(connState);
     const Comm::ConnectionPointer &details = connState->clientConnection;
 
-    if (!ctx || !httpsCreate(details, ctx))
+    if (!ctx || !httpsCreate(connState, ctx))
         return;
 
     typedef CommCbMemFunT<ConnStateData, CommTimeoutCbParams> TimeoutDialer;
@@ -2891,7 +2892,7 @@ ConnStateData::getSslContextDone(Security::ContextPointer &ctx)
         }
     }
 
-    if (!httpsCreate(clientConnection, ctx))
+    if (!httpsCreate(this, ctx))
         return;
 
     // bumped intercepted conns should already have Config.Timeout.request set
@@ -3116,7 +3117,7 @@ ConnStateData::startPeekAndSplice()
     Security::ContextPointer unConfiguredCTX(Ssl::createSSLContext(port->secure.signingCa.cert, port->secure.signingCa.pkey, port->secure));
     fd_table[clientConnection->fd].dynamicTlsContext = unConfiguredCTX;
 
-    if (!httpsCreate(clientConnection, unConfiguredCTX))
+    if (!httpsCreate(this, unConfiguredCTX))
         return;
 
     switchedToHttps_ = true;

diff --git a/src/security/PeerOptions.cc b/src/security/PeerOptions.cc
index fd9e5968b80a3de6b5521753734051302ea7ea8f..deecc5eb0fcfe4f4699e7a03fd40b00b11e7c7c9 100644 (file)
--- a/src/security/PeerOptions.cc
+++ b/src/security/PeerOptions.cc
@@ -518,8 +518,9 @@ Security::PeerOptions::parseOptions()
     const char *err = nullptr;
     const char *priorities = str.c_str();
     gnutls_priority_t op;
-    if (gnutls_priority_init(&op, priorities, &err) != GNUTLS_E_SUCCESS) {
-        fatalf("Unknown TLS option '%s'", err);
+    int x = gnutls_priority_init(&op, priorities, &err);
+    if (x != GNUTLS_E_SUCCESS) {
+        fatalf("(%s) in TLS options '%s'", ErrorString(x), err);
     }
     parsedOptions = Security::ParsedOptions(op, [](gnutls_priority_t p) {
         debugs(83, 5, "gnutls_priority_deinit p=" << (void*)p);
@@ -738,6 +739,7 @@ Security::PeerOptions::updateSessionOptions(Security::SessionPointer &s)
 {
     parseOptions();
 #if USE_OPENSSL
+    debugs(83, 5, "set OpenSSL options for session=" << s << ", parsedOptions=" << parsedOptions);
     // XXX: Options already set before (via the context) are not cleared!
     SSL_set_options(s.get(), parsedOptions);
 
@@ -750,13 +752,13 @@ Security::PeerOptions::updateSessionOptions(Security::SessionPointer &s)
         static const SBuf defaults("default");
         errMsg = defaults;
     } else {
-        debugs(83, 5, "set GnuTLS options '" << sslOptions << "' for session=" << s);
+        debugs(83, 5, "set GnuTLS session=" << s << ", options='" << sslOptions << ":" << tlsMinOptions << "'");
         x = gnutls_priority_set(s.get(), parsedOptions.get());
         errMsg = sslOptions;
     }
 
     if (x != GNUTLS_E_SUCCESS) {
-        debugs(83, DBG_IMPORTANT, "ERROR: Failed to set TLS options (" << errMsg << ":" << tlsMinVersion << "). error: " << Security::ErrorString(x));
+        debugs(83, DBG_IMPORTANT, "ERROR: session=" << s << " Failed to set TLS options (" << errMsg << ":" << tlsMinVersion << "). error: " << Security::ErrorString(x));
     }
 #endif
 }

diff --git a/src/security/PeerOptions.h b/src/security/PeerOptions.h
index da7721b91ddc94c45aac8995d022851a2a920a2e..1deeff258f58608bcef28e2d5cb34a12fc90daf3 100644 (file)
--- a/src/security/PeerOptions.h
+++ b/src/security/PeerOptions.h
@@ -105,13 +105,15 @@ protected:
     template<typename T>
     Security::ContextPointer convertContextFromRawPtr(T ctx) const {
 #if USE_OPENSSL
+        debugs(83, 5, "SSL_CTX construct, this=" << (void*)ctx);
         return ContextPointer(ctx, [](SSL_CTX *p) {
-            debugs(83, 5, "SSL_free ctx=" << (void*)p);
+            debugs(83, 5, "SSL_CTX destruct, this=" << (void*)p);
             SSL_CTX_free(p);
         });
 #elif USE_GNUTLS
+        debugs(83, 5, "gnutls_certificate_credentials construct, this=" << (void*)ctx);
         return Security::ContextPointer(ctx, [](gnutls_certificate_credentials_t p) {
-            debugs(83, 5, "gnutls_certificate_free_credentials ctx=" << (void*)p);
+            debugs(83, 0, "gnutls_certificate_credentials destruct this=" << (void*)p);
             gnutls_certificate_free_credentials(p);
         });
 #else

diff --git a/src/security/Session.cc b/src/security/Session.cc
index 9a9d90904c5f261f34e17215868054ef293aba19..6698d26457b58e1fe0cfeb9941d7326436481e03 100644 (file)
--- a/src/security/Session.cc
+++ b/src/security/Session.cc
@@ -106,7 +106,7 @@ Security::NewSessionObject(const Security::ContextPointer &ctx)
 #endif
 
 static bool
-CreateSession(const Security::ContextPointer &ctx, const Comm::ConnectionPointer &conn, Security::Io::Type type, const char *squidCtx)
+CreateSession(const Security::ContextPointer &ctx, const Comm::ConnectionPointer &conn, Security::PeerOptions &opts, Security::Io::Type type, const char *squidCtx)
 {
     if (!Comm::IsConnOpen(conn)) {
         debugs(83, DBG_IMPORTANT, "Gone connection");
@@ -122,6 +122,7 @@ CreateSession(const Security::ContextPointer &ctx, const Comm::ConnectionPointer
     if (!session) {
         errCode = ERR_get_error();
         errAction = "failed to allocate handle";
+        debugs(83, DBG_IMPORTANT, "TLS error: " << errAction << ": " << Security::ErrorString(errCode));
     }
 #elif USE_GNUTLS
     gnutls_session_t tmp;
@@ -134,6 +135,7 @@ CreateSession(const Security::ContextPointer &ctx, const Comm::ConnectionPointer
     if (errCode != GNUTLS_E_SUCCESS) {
         session.reset();
         errAction = "failed to initialize session";
+        debugs(83, DBG_IMPORTANT, "TLS error: " << errAction << ": " << Security::ErrorString(errCode));
     }
 #endif
 
@@ -148,10 +150,7 @@ CreateSession(const Security::ContextPointer &ctx, const Comm::ConnectionPointer
         errCode = gnutls_credentials_set(session.get(), GNUTLS_CRD_CERTIFICATE, ctx.get());
         if (errCode == GNUTLS_E_SUCCESS) {
 
-            if (auto *peer = conn->getPeer())
-                peer->secure.updateSessionOptions(session);
-            else
-                Security::ProxyOutgoingConfig.updateSessionOptions(session);
+            opts.updateSessionOptions(session);
 
             // NP: GnuTLS does not yet support the BIO operations
             //     this does the equivalent of SSL_set_fd() for now.
@@ -184,13 +183,17 @@ CreateSession(const Security::ContextPointer &ctx, const Comm::ConnectionPointer
 bool
 Security::CreateClientSession(const Security::ContextPointer &ctx, const Comm::ConnectionPointer &c, const char *squidCtx)
 {
-    return CreateSession(ctx, c, Security::Io::BIO_TO_SERVER, squidCtx);
+    if (!c || !c->getPeer())
+        return CreateSession(ctx, c, Security::ProxyOutgoingConfig, Security::Io::BIO_TO_SERVER, squidCtx);
+
+    auto *peer = c->getPeer();
+    return CreateSession(ctx, c, peer->secure, Security::Io::BIO_TO_SERVER, squidCtx);
 }
 
 bool
-Security::CreateServerSession(const Security::ContextPointer &ctx, const Comm::ConnectionPointer &c, const char *squidCtx)
+Security::CreateServerSession(const Security::ContextPointer &ctx, const Comm::ConnectionPointer &c, Security::PeerOptions &o, const char *squidCtx)
 {
-    return CreateSession(ctx, c, Security::Io::BIO_TO_CLIENT, squidCtx);
+    return CreateSession(ctx, c, o, Security::Io::BIO_TO_CLIENT, squidCtx);
 }
 
 void

diff --git a/src/security/Session.h b/src/security/Session.h
index 723b9b83c0114f2c98cf077c41eaf638ad10793c..7ba40d89d70b75117f37120e2746ef341b7376c9 100644 (file)
--- a/src/security/Session.h
+++ b/src/security/Session.h
@@ -34,9 +34,11 @@ namespace Security {
 /// On errors, emits DBG_IMPORTANT with details and returns false.
 bool CreateClientSession(const Security::ContextPointer &, const Comm::ConnectionPointer &, const char *squidCtx);
 
+class PeerOptions;
+
 /// Creates TLS Server connection structure (aka 'session' state) and initializes TLS/SSL I/O (Comm and BIO).
 /// On errors, emits DBG_IMPORTANT with details and returns false.
-bool CreateServerSession(const Security::ContextPointer &, const Comm::ConnectionPointer &, const char *squidCtx);
+bool CreateServerSession(const Security::ContextPointer &, const Comm::ConnectionPointer &, Security::PeerOptions &, const char *squidCtx);
 
 #if USE_OPENSSL
 typedef std::shared_ptr<SSL> SessionPointer;

diff --git a/src/tests/stub_libsecurity.cc b/src/tests/stub_libsecurity.cc
index 63de368b369c657fba597eff998ae28485ff4078..ac0d72c1b4068e0c25ead06365574448882a9cdc 100644 (file)
--- a/src/tests/stub_libsecurity.cc
+++ b/src/tests/stub_libsecurity.cc
@@ -109,7 +109,7 @@ void Security::ServerOptions::updateContextSessionId(Security::ContextPointer &)
 #include "security/Session.h"
 namespace Security {
 bool CreateClientSession(const Security::ContextPointer &, const Comm::ConnectionPointer &, const char *) STUB_RETVAL(false)
-bool CreateServerSession(const Security::ContextPointer &, const Comm::ConnectionPointer &, const char *) STUB_RETVAL(false)
+bool CreateServerSession(const Security::ContextPointer &, const Comm::ConnectionPointer &, Security::PeerOptions &, const char *) STUB_RETVAL(false)
 void SessionSendGoodbye(const Security::SessionPointer &) STUB
 bool SessionIsResumed(const Security::SessionPointer &) STUB_RETVAL(false)
 void MaybeGetSessionResumeData(const Security::SessionPointer &, Security::SessionStatePointer &) STUB