Shuffle TLS session creation to libsecurity

diff --git a/src/client_side.cc b/src/client_side.cc
index 4987fc407aeaa9955a26a3c86e9c741827f2a02e..8175ee1a0a6b5eec0ca399835142ac3f783e51d2 100644 (file)
--- a/src/client_side.cc
+++ b/src/client_side.cc
@@ -2577,13 +2577,12 @@ httpAccept(const CommAcceptCbParams &params)
 }
 
 #if USE_OPENSSL
-
-/** Create SSL connection structure and update fd_table */
+/// Create TLS connection structure and update fd_table
 static bool
 httpsCreate(const Comm::ConnectionPointer &conn, const Security::ContextPointer &ctx)
 {
-    if (Ssl::CreateServer(ctx, conn, "client https start")) {
-        debugs(33, 5, "will negotate SSL on " << conn);
+    if (Security::CreateServerSession(ctx, conn, "client https start")) {
+        debugs(33, 5, "will negotate TLS on " << conn);
         return true;
     }
 

diff --git a/src/security/PeerConnector.cc b/src/security/PeerConnector.cc
index 24b6f5b9486144f0489e2f1b44134d5a42fdcc7f..513bb9b5ec227e56b2b7e7f115c55c5bc1dbe337 100644 (file)
--- a/src/security/PeerConnector.cc
+++ b/src/security/PeerConnector.cc
@@ -103,7 +103,7 @@ Security::PeerConnector::initialize(Security::SessionPointer &serverSession)
     Security::ContextPointer ctx(getTlsContext());
     assert(ctx);
 
-    if (!Ssl::CreateClient(ctx, serverConnection(), "server https start")) {
+    if (!Security::CreateClientSession(ctx, serverConnection(), "server https start")) {
         ErrorState *anErr = new ErrorState(ERR_SOCKET_FAILURE, Http::scInternalServerError, request.getRaw());
         anErr->xerrno = errno;
         debugs(83, DBG_IMPORTANT, "Error allocating TLS handle: " << ERR_error_string(ERR_get_error(), NULL));

diff --git a/src/security/Session.cc b/src/security/Session.cc
index be777348585f358699381bdc1abd7e32540d8d99..259f647c56b350b506e0fbaf3055133e5d2ea568 100644 (file)
--- a/src/security/Session.cc
+++ b/src/security/Session.cc
@@ -12,13 +12,65 @@
 #include "anyp/PortCfg.h"
 #include "base/RunnersRegistry.h"
 #include "Debug.h"
+#include "fde.h"
 #include "ipc/MemMap.h"
 #include "security/Session.h"
 #include "SquidConfig.h"
+#include "ssl/bio.h"
 
 #define SSL_SESSION_ID_SIZE 32
 #define SSL_SESSION_MAX_SIZE 10*1024
 
+static bool
+CreateSession(const Security::ContextPointer &ctx, const Comm::ConnectionPointer &conn, Security::Io::Type type, const char *squidCtx)
+{
+    if (!Comm::IsConnOpen(conn)) {
+        debugs(83, DBG_IMPORTANT, "Gone connection");
+        return false;
+    }
+
+    const char *errAction = "with no TLS/SSL library";
+#if USE_OPENSSL
+    int errCode = 0;
+    if (auto ssl = SSL_new(ctx.get())) {
+        const int fd = conn->fd;
+        // without BIO, we would call SSL_set_fd(ssl, fd) instead
+        if (BIO *bio = Ssl::Bio::Create(fd, type)) {
+            Ssl::Bio::Link(ssl, bio); // cannot fail
+
+            fd_table[fd].ssl.resetWithoutLocking(ssl);
+            fd_table[fd].read_method = &ssl_read_method;
+            fd_table[fd].write_method = &ssl_write_method;
+            fd_note(fd, squidCtx);
+            return true;
+        }
+        errCode = ERR_get_error();
+        errAction = "failed to initialize I/O";
+        SSL_free(ssl);
+    } else {
+        errCode = ERR_get_error();
+        errAction = "failed to allocate handle";
+    }
+    debugs(83, DBG_IMPORTANT, "ERROR: " << squidCtx << ' ' << errAction <<
+           ": " << ERR_error_string(errCode, nullptr));
+#else
+    debugs(83, DBG_IMPORTANT, "ERROR: " << squidCtx << ' ' << errAction);
+#endif
+    return false;
+}
+
+bool
+Security::CreateClientSession(const Security::ContextPointer &ctx, const Comm::ConnectionPointer &c, const char *squidCtx)
+{
+    return CreateSession(ctx, c, Security::Io::BIO_TO_SERVER, squidCtx);
+}
+
+bool
+Security::CreateServerSession(const Security::ContextPointer &ctx, const Comm::ConnectionPointer &c, const char *squidCtx)
+{
+    return CreateSession(ctx, c, Security::Io::BIO_TO_CLIENT, squidCtx);
+}
+
 bool
 Security::SessionIsResumed(const Security::SessionPointer &s)
 {

diff --git a/src/security/Session.h b/src/security/Session.h
index 7e5dd27f2d0d3b93175cfe3d4735f20deb6c0933..5f477e5bee0e9900961e40bbd597f722dc0c561b 100644 (file)
--- a/src/security/Session.h
+++ b/src/security/Session.h
@@ -10,6 +10,7 @@
 #define SQUID_SRC_SECURITY_SESSION_H
 
 #include "base/HardFun.h"
+#include "comm/forward.h"
 #include "security/LockingPointer.h"
 
 #include <memory>
@@ -28,6 +29,14 @@
 
 namespace Security {
 
+/// Creates TLS Client 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 CreateClientSession(const Security::ContextPointer &, const Comm::ConnectionPointer &, const char *squidCtx);
+
+/// 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);
+
 #if USE_OPENSSL
 CtoCpp1(SSL_free, SSL *);
 typedef LockingPointer<SSL, Security::SSL_free_cpp, CRYPTO_LOCK_SSL> SessionPointer;

diff --git a/src/security/forward.h b/src/security/forward.h
index 8f96b8ef4794c9b974474be6c7d28bbfb821f009..49f9314a14a9e192d650dcccfaadabd0d47a44ec 100644 (file)
--- a/src/security/forward.h
+++ b/src/security/forward.h
@@ -88,6 +88,15 @@ typedef int ErrorCode;
 /// \note using std::unordered_set ensures values are unique, with fast lookup
 typedef std::unordered_set<Security::ErrorCode> Errors;
 
+namespace Io
+{
+    enum Type {
+        BIO_TO_CLIENT = 6000,
+        BIO_TO_SERVER
+    };
+
+} // namespace Io
+
 class KeyData;
 class PeerConnector;
 class PeerOptions;

diff --git a/src/ssl/bio.cc b/src/ssl/bio.cc
index aad25584882e1a1782f2cb0e3946331901e27839..221b8756a7baccf94a6f4f71d45dea64bd4c7170 100644 (file)
--- a/src/ssl/bio.cc
+++ b/src/ssl/bio.cc
@@ -58,7 +58,7 @@ static BIO_METHOD SquidMethods = {
 };
 
 BIO *
-Ssl::Bio::Create(const int fd, Ssl::Bio::Type type)
+Ssl::Bio::Create(const int fd, Security::Io::Type type)
 {
     if (BIO *bio = BIO_new(&SquidMethods)) {
         BIO_int_ctrl(bio, BIO_C_SET_FD, type, fd);
@@ -562,7 +562,7 @@ squid_bio_ctrl(BIO *table, int cmd, long arg1, void *arg2)
         assert(arg2);
         const int fd = *static_cast<int*>(arg2);
         Ssl::Bio *bio;
-        if (arg1 == Ssl::Bio::BIO_TO_SERVER)
+        if (arg1 == Security::Io::BIO_TO_SERVER)
             bio = new Ssl::ServerBio(fd);
         else
             bio = new Ssl::ClientBio(fd);

diff --git a/src/ssl/bio.h b/src/ssl/bio.h
index f5612aefaafde0e08b0808fc1326876caa53425d..3fcc8c03011c5ea83ed3b9334f9fb36918d41b20 100644 (file)
--- a/src/ssl/bio.h
+++ b/src/ssl/bio.h
@@ -9,6 +9,8 @@
 #ifndef SQUID_SSL_BIO_H
 #define SQUID_SSL_BIO_H
 
+#if USE_OPENSSL
+
 #include "fd.h"
 #include "security/Handshake.h"
 
@@ -27,11 +29,6 @@ namespace Ssl
 class Bio
 {
 public:
-    enum Type {
-        BIO_TO_CLIENT = 6000,
-        BIO_TO_SERVER
-    };
-
     explicit Bio(const int anFd);
     virtual ~Bio();
 
@@ -53,7 +50,7 @@ public:
 
     /// Creates a low-level BIO table, creates a high-level Ssl::Bio object
     /// for a given socket, and then links the two together via BIO_C_SET_FD.
-    static BIO *Create(const int fd, Type type);
+    static BIO *Create(const int fd, Security::Io::Type type);
     /// Tells ssl connection to use BIO and monitor state via stateChanged()
     static void Link(SSL *ssl, BIO *bio);
 
@@ -196,5 +193,6 @@ private:
 void
 applyTlsDetailsToSSL(SSL *ssl, Security::TlsDetails::Pointer const &details, Ssl::BumpMode bumpMode);
 
+#endif /* USE_OPENSSL */
 #endif /* SQUID_SSL_BIO_H */
 

diff --git a/src/ssl/support.cc b/src/ssl/support.cc
index 26eeb17d21ce014f090c0dfda9ff1561f101ff51..1d14010315f6bad79fc440fa1318b50585c7bb43 100644 (file)
--- a/src/ssl/support.cc
+++ b/src/ssl/support.cc
@@ -1401,53 +1401,6 @@ bool Ssl::generateUntrustedCert(Security::CertPointer &untrustedCert, EVP_PKEY_P
     return Ssl::generateSslCertificate(untrustedCert, untrustedPkey, certProperties);
 }
 
-static bool
-SslCreate(const Security::ContextPointer &ctx, const Comm::ConnectionPointer &conn, Ssl::Bio::Type type, const char *squidCtx)
-{
-    if (!Comm::IsConnOpen(conn)) {
-        debugs(83, DBG_IMPORTANT, "Gone connection");
-        return false;
-    }
-
-    const char *errAction = NULL;
-    int errCode = 0;
-    if (auto ssl = SSL_new(ctx.get())) {
-        const int fd = conn->fd;
-        // without BIO, we would call SSL_set_fd(ssl, fd) instead
-        if (BIO *bio = Ssl::Bio::Create(fd, type)) {
-            Ssl::Bio::Link(ssl, bio); // cannot fail
-
-            fd_table[fd].ssl.resetWithoutLocking(ssl);
-            fd_table[fd].read_method = &ssl_read_method;
-            fd_table[fd].write_method = &ssl_write_method;
-            fd_note(fd, squidCtx);
-            return true;
-        }
-        errCode = ERR_get_error();
-        errAction = "failed to initialize I/O";
-        SSL_free(ssl);
-    } else {
-        errCode = ERR_get_error();
-        errAction = "failed to allocate handle";
-    }
-
-    debugs(83, DBG_IMPORTANT, "ERROR: " << squidCtx << ' ' << errAction <<
-           ": " << ERR_error_string(errCode, NULL));
-    return false;
-}
-
-bool
-Ssl::CreateClient(const Security::ContextPointer &ctx, const Comm::ConnectionPointer &c, const char *squidCtx)
-{
-    return SslCreate(ctx, c, Ssl::Bio::BIO_TO_SERVER, squidCtx);
-}
-
-bool
-Ssl::CreateServer(const Security::ContextPointer &ctx, const Comm::ConnectionPointer &c, const char *squidCtx)
-{
-    return SslCreate(ctx, c, Ssl::Bio::BIO_TO_CLIENT, squidCtx);
-}
-
 static int
 store_session_cb(SSL *ssl, SSL_SESSION *session)
 {

diff --git a/src/ssl/support.h b/src/ssl/support.h
index 42c981530715de1258c3760b7b7cb2c21f4bdce8..e0cf3f56c4d599d0bf55b9052433faa601db0c87 100644 (file)
--- a/src/ssl/support.h
+++ b/src/ssl/support.h
@@ -73,14 +73,6 @@ class ErrorDetail;
 class CertValidationResponse;
 typedef RefCount<CertValidationResponse> CertValidationResponsePointer;
 
-/// Creates SSL Client connection structure and initializes SSL I/O (Comm and BIO).
-/// On errors, emits DBG_IMPORTANT with details and returns false.
-bool CreateClient(const Security::ContextPointer &, const Comm::ConnectionPointer &, const char *squidCtx);
-
-/// Creates SSL Server connection structure and initializes SSL I/O (Comm and BIO).
-/// On errors, emits DBG_IMPORTANT with details and returns false.
-bool CreateServer(const Security::ContextPointer &, const Comm::ConnectionPointer &, const char *squidCtx);
-
 void SetSessionCallbacks(Security::ContextPointer &);
 extern Ipc::MemMap *SessionCache;
 extern const char *SessionCacheName;

diff --git a/src/tests/stub_libsecurity.cc b/src/tests/stub_libsecurity.cc
index b9707985d20dcd3ada8a6fd3d683f62e9f748c3b..9796f6f7e79f8616a8c243103e83eb02c1b47980 100644 (file)
--- a/src/tests/stub_libsecurity.cc
+++ b/src/tests/stub_libsecurity.cc
@@ -89,6 +89,8 @@ void Security::ServerOptions::updateContextEecdh(Security::ContextPointer &) STU
 
 #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 SessionIsResumed(const Security::SessionPointer &) STUB_RETVAL(false)
 void MaybeGetSessionResumeData(const Security::SessionPointer &, Security::SessionStatePointer &) STUB
 void SetSessionResumeData(const Security::SessionPointer &, const Security::SessionStatePointer &) STUB