Cleanup: Ssl::Create*() functions

* return value is only used in boolean expressions, so make it bool

* pass Comm::ConnectionPointer instead of raw-FD value

diff --git a/src/client_side.cc b/src/client_side.cc
index 784abea28ae494c22f3550b24ba0ba2c4010e89b..952ecaafed656d75b8e330b49655826b1ebd6263 100644 (file)
--- a/src/client_side.cc
+++ b/src/client_side.cc
@@ -2577,16 +2577,16 @@ httpAccept(const CommAcceptCbParams &params)
 #if USE_OPENSSL
 
 /** Create SSL connection structure and update fd_table */
-static Security::SessionPtr
+static bool
 httpsCreate(const Comm::ConnectionPointer &conn, Security::ContextPtr sslContext)
 {
-    if (auto ssl = Ssl::CreateServer(sslContext, conn->fd, "client https start")) {
+    if (Ssl::CreateServer(sslContext, conn, "client https start")) {
         debugs(33, 5, "will negotate SSL on " << conn);
-        return ssl;
+        return true;
     }
 
     conn->close();
-    return nullptr;
+    return false;
 }
 
 /**
@@ -2732,11 +2732,10 @@ clientNegotiateSSL(int fd, void *data)
 static void
 httpsEstablish(ConnStateData *connState, Security::ContextPtr sslContext)
 {
-    Security::SessionPtr ssl = nullptr;
     assert(connState);
     const Comm::ConnectionPointer &details = connState->clientConnection;
 
-    if (!sslContext || !(ssl = httpsCreate(details, sslContext)))
+    if (!sslContext || !httpsCreate(details, sslContext))
         return;
 
     typedef CommCbMemFunT<ConnStateData, CommTimeoutCbParams> TimeoutDialer;

diff --git a/src/ssl/PeerConnector.cc b/src/ssl/PeerConnector.cc
index 0e22b8ce701bd7e9a9e1d1e7f50cb7671db08864..f4a936d1345e2c3bddde945fc330f507064a87ca 100644 (file)
--- a/src/ssl/PeerConnector.cc
+++ b/src/ssl/PeerConnector.cc
@@ -93,10 +93,7 @@ Ssl::PeerConnector::initializeSsl()
     Security::ContextPtr sslContext(getSslContext());
     assert(sslContext);
 
-    const int fd = serverConnection()->fd;
-
-    auto ssl = Ssl::CreateClient(sslContext, fd, "server https start");
-    if (!ssl) {
+    if (!Ssl::CreateClient(sslContext, serverConnection(), "server https start")) {
         ErrorState *anErr = new ErrorState(ERR_SOCKET_FAILURE, Http::scInternalServerError, request.getRaw());
         anErr->xerrno = errno;
         debugs(83, DBG_IMPORTANT, "Error allocating SSL handle: " << ERR_error_string(ERR_get_error(), NULL));
@@ -106,6 +103,9 @@ Ssl::PeerConnector::initializeSsl()
         return nullptr;
     }
 
+    // A TLS/SSL session has now been created for the connection and stored in fd_table
+    auto &tlsSession = fd_table[serverConnection()->fd].ssl;
+
     // If CertValidation Helper used do not lookup checklist for errors,
     // but keep a list of errors to send it to CertValidator
     if (!Ssl::TheConfig.ssl_crt_validator) {
@@ -115,10 +115,10 @@ Ssl::PeerConnector::initializeSsl()
             ACLFilledChecklist *check = new ACLFilledChecklist(acl, request.getRaw(), dash_str);
             check->al = al;
             // check->fd(fd); XXX: need client FD here
-            SSL_set_ex_data(ssl, ssl_ex_index_cert_error_check, check);
+            SSL_set_ex_data(tlsSession.get(), ssl_ex_index_cert_error_check, check);
         }
     }
-    return ssl;
+    return tlsSession.get();
 }
 
 void

diff --git a/src/ssl/support.cc b/src/ssl/support.cc
index 637ae3e5fe0cf014d8d6bce317f4fe6d1680210a..4a0502c241ca2e55867ba2cc742c1255b6177c68 100644 (file)
--- a/src/ssl/support.cc
+++ b/src/ssl/support.cc
@@ -1298,17 +1298,18 @@ bool Ssl::generateUntrustedCert(Security::CertPointer &untrustedCert, EVP_PKEY_P
     return Ssl::generateSslCertificate(untrustedCert, untrustedPkey, certProperties);
 }
 
-SSL *
-SslCreate(Security::ContextPtr sslContext, const int fd, Ssl::Bio::Type type, const char *squidCtx)
+static bool
+SslCreate(Security::ContextPtr sslContext, const Comm::ConnectionPointer &conn, Ssl::Bio::Type type, const char *squidCtx)
 {
-    if (fd < 0) {
+    if (!Comm::IsConnOpen(conn)) {
         debugs(83, DBG_IMPORTANT, "Gone connection");
-        return NULL;
+        return false;
     }
 
     const char *errAction = NULL;
     int errCode = 0;
     if (auto ssl = SSL_new(sslContext)) {
+        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
@@ -1317,7 +1318,7 @@ SslCreate(Security::ContextPtr sslContext, const int fd, Ssl::Bio::Type type, co
             fd_table[fd].read_method = &ssl_read_method;
             fd_table[fd].write_method = &ssl_write_method;
             fd_note(fd, squidCtx);
-            return ssl;
+            return true;
         }
         errCode = ERR_get_error();
         errAction = "failed to initialize I/O";
@@ -1329,19 +1330,19 @@ SslCreate(Security::ContextPtr sslContext, const int fd, Ssl::Bio::Type type, co
 
     debugs(83, DBG_IMPORTANT, "ERROR: " << squidCtx << ' ' << errAction <<
            ": " << ERR_error_string(errCode, NULL));
-    return NULL;
+    return false;
 }
 
-SSL *
-Ssl::CreateClient(Security::ContextPtr sslContext, const int fd, const char *squidCtx)
+bool
+Ssl::CreateClient(Security::ContextPtr sslContext, const Comm::ConnectionPointer &c, const char *squidCtx)
 {
-    return SslCreate(sslContext, fd, Ssl::Bio::BIO_TO_SERVER, squidCtx);
+    return SslCreate(sslContext, c, Ssl::Bio::BIO_TO_SERVER, squidCtx);
 }
 
-SSL *
-Ssl::CreateServer(Security::ContextPtr sslContext, const int fd, const char *squidCtx)
+bool
+Ssl::CreateServer(Security::ContextPtr sslContext, const Comm::ConnectionPointer &c, const char *squidCtx)
 {
-    return SslCreate(sslContext, fd, Ssl::Bio::BIO_TO_CLIENT, squidCtx);
+    return SslCreate(sslContext, c, Ssl::Bio::BIO_TO_CLIENT, squidCtx);
 }
 
 Ssl::CertError::CertError(ssl_error_t anErr, X509 *aCert, int aDepth): code(anErr), depth(aDepth)

diff --git a/src/ssl/support.h b/src/ssl/support.h
index 0ef586762b48c2f7fdf656bb7e076d95efa985d9..2142beb28f8cfafde3faa4e433aa8ec8317ee943 100644 (file)
--- a/src/ssl/support.h
+++ b/src/ssl/support.h
@@ -14,6 +14,7 @@
 #if USE_OPENSSL
 
 #include "base/CbDataList.h"
+#include "comm/forward.h"
 #include "sbuf/SBuf.h"
 #include "security/forward.h"
 #include "ssl/gadgets.h"
@@ -77,12 +78,12 @@ 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 NULL.
-SSL *CreateClient(Security::ContextPtr sslContext, const int fd, const char *squidCtx);
+/// On errors, emits DBG_IMPORTANT with details and returns false.
+bool CreateClient(Security::ContextPtr sslContext, 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 NULL.
-SSL *CreateServer(Security::ContextPtr sslContext, const int fd, const char *squidCtx);
+/// On errors, emits DBG_IMPORTANT with details and returns false.
+bool CreateServer(Security::ContextPtr sslContext, const Comm::ConnectionPointer &, const char *squidCtx);
 
 /// An SSL certificate-related error.
 /// Pairs an error code with the certificate experiencing the error.