Cleanup: update TLS session pointer types

* rename SSL* pointer to Security::SessionPtr and SSL_Pointer to
  SessionPointer as the smart pointer variant. Matching the model
  designed for TLS context storage.

* update fd_table .ssl member to a SessionPointer for safer session
  pointer deallocation.

* migrate most uses of SSL* to Securit::SessionPtr or auto

diff --git a/src/acl/Certificate.cc b/src/acl/Certificate.cc
index 00c03df3b89555f8877764cef8ff0d435146a7ab..44144a53ae5b605d211f1f31692aae95a3fa766b 100644 (file)
--- a/src/acl/Certificate.cc
+++ b/src/acl/Certificate.cc
@@ -28,7 +28,7 @@ ACLCertificateStrategy::match (ACLData<MatchType> * &data, ACLFilledChecklist *c
 {
     const int fd = checklist->fd();
     const bool goodDescriptor = 0 <= fd && fd <= Biggest_FD;
-    auto ssl = goodDescriptor ? fd_table[fd].ssl : nullptr;
+    auto ssl = goodDescriptor ? fd_table[fd].ssl.get() : nullptr;
     X509 *cert = SSL_get_peer_certificate(ssl);
     const bool res = data->match (cert);
     X509_free(cert);

diff --git a/src/adaptation/icap/Xaction.cc b/src/adaptation/icap/Xaction.cc
index ca4a31ea2540fba0a7f58b8a5fb8c2c594a132f9..de13235f71f324f386a7d9b3ac675d0ed4cae97b 100644 (file)
--- a/src/adaptation/icap/Xaction.cc
+++ b/src/adaptation/icap/Xaction.cc
@@ -60,7 +60,7 @@ public:
         PeerConnector(aServerConn, aCallback, timeout), icapService(service) {}
 
     /* PeerConnector API */
-    virtual Security::SessionPointer initializeSsl();
+    virtual Security::SessionPtr initializeSsl();
     virtual void noteNegotiationDone(ErrorState *error);
     virtual Security::ContextPtr getSslContext() {return icapService->sslContext;}
 
@@ -296,7 +296,7 @@ void Adaptation::Icap::Xaction::noteCommConnected(const CommConnectCbParams &io)
 #if USE_OPENSSL
     // If it is a reused connection and the SSL object is build
     // we should not negotiate new SSL session
-    auto ssl = fd_table[io.conn->fd].ssl;
+    const auto &ssl = fd_table[io.conn->fd].ssl;
     if (!ssl && service().cfg().secure.encryptTransport) {
         CbcPointer<Adaptation::Icap::Xaction> me(this);
         securer = asyncCall(93, 4, "Adaptation::Icap::Xaction::handleSecuredPeer",
@@ -701,7 +701,7 @@ bool Adaptation::Icap::Xaction::fillVirginHttpHeader(MemBuf &) const
 }
 
 #if USE_OPENSSL
-Security::SessionPointer
+Security::SessionPtr
 Ssl::IcapPeerConnector::initializeSsl()
 {
     auto ssl = Ssl::PeerConnector::initializeSsl();
@@ -729,7 +729,7 @@ Ssl::IcapPeerConnector::noteNegotiationDone(ErrorState *error)
         return;
 
     const int fd = serverConnection()->fd;
-    auto ssl = fd_table[fd].ssl;
+    auto ssl = fd_table[fd].ssl.get();
     assert(ssl);
     if (!SSL_session_reused(ssl)) {
         if (icapService->sslSession)

diff --git a/src/client_side.cc b/src/client_side.cc
index b9634c8fa13bd463df318e22c12d4b08ad97b888..8c87acc9e6d33ce206d3f422977f80d91be4a3df 100644 (file)
--- a/src/client_side.cc
+++ b/src/client_side.cc
@@ -3072,7 +3072,7 @@ ConnStateData::requestTimeout(const CommTimeoutCbParams &io)
                 receivedFirstByte();
                 return;
             }
-        } else if (fd_table[io.conn->fd].ssl == NULL)
+        } else if (!fd_table[io.conn->fd].ssl)
 #endif
         {
             const HttpRequestMethod method;
@@ -3266,7 +3266,7 @@ httpAccept(const CommAcceptCbParams &params)
 #if USE_OPENSSL
 
 /** Create SSL connection structure and update fd_table */
-static Security::SessionPointer
+static Security::SessionPtr
 httpsCreate(const Comm::ConnectionPointer &conn, Security::ContextPtr sslContext)
 {
     if (auto ssl = Ssl::CreateServer(sslContext, conn->fd, "client https start")) {
@@ -3288,7 +3288,7 @@ static int
 Squid_SSL_accept(ConnStateData *conn, PF *callback)
 {
     int fd = conn->clientConnection->fd;
-    auto ssl = fd_table[fd].ssl;
+    auto ssl = fd_table[fd].ssl.get();
     int ret;
 
     errno = 0;
@@ -3337,7 +3337,7 @@ clientNegotiateSSL(int fd, void *data)
 {
     ConnStateData *conn = (ConnStateData *)data;
     X509 *client_cert;
-    auto ssl = fd_table[fd].ssl;
+    auto ssl = fd_table[fd].ssl.get();
 
     int ret;
     if ((ret = Squid_SSL_accept(conn, clientNegotiateSSL)) <= 0) {
@@ -3421,7 +3421,7 @@ clientNegotiateSSL(int fd, void *data)
 static void
 httpsEstablish(ConnStateData *connState, Security::ContextPtr sslContext)
 {
-    Security::SessionPointer ssl = nullptr;
+    Security::SessionPtr ssl = nullptr;
     assert(connState);
     const Comm::ConnectionPointer &details = connState->clientConnection;
 
@@ -3551,7 +3551,7 @@ ConnStateData::sslCrtdHandleReply(const Helper::Reply &reply)
                 debugs(33, 5, HERE << "Certificate for " << sslConnectHostOrIp << " was successfully recieved from ssl_crtd");
                 if (sslServerBump && (sslServerBump->act.step1 == Ssl::bumpPeek || sslServerBump->act.step1 == Ssl::bumpStare)) {
                     doPeekAndSpliceStep();
-                    auto ssl = fd_table[clientConnection->fd].ssl;
+                    auto ssl = fd_table[clientConnection->fd].ssl.get();
                     bool ret = Ssl::configureSSLUsingPkeyAndCertFromMemory(ssl, reply_message.getBody().c_str(), *port);
                     if (!ret)
                         debugs(33, 5, "Failed to set certificates to ssl object for PeekAndSplice mode");
@@ -3711,7 +3711,7 @@ ConnStateData::getSslContextStart()
         debugs(33, 5, HERE << "Generating SSL certificate for " << certProperties.commonName);
         if (sslServerBump && (sslServerBump->act.step1 == Ssl::bumpPeek || sslServerBump->act.step1 == Ssl::bumpStare)) {
             doPeekAndSpliceStep();
-            auto ssl = fd_table[clientConnection->fd].ssl;
+            auto ssl = fd_table[clientConnection->fd].ssl.get();
             if (!Ssl::configureSSL(ssl, certProperties, *port))
                 debugs(33, 5, "Failed to set certificates to ssl object for PeekAndSplice mode");
         } else {
@@ -3843,7 +3843,7 @@ static void
 clientPeekAndSpliceSSL(int fd, void *data)
 {
     ConnStateData *conn = (ConnStateData *)data;
-    auto ssl = fd_table[fd].ssl;
+    auto ssl = fd_table[fd].ssl.get();
 
     debugs(83, 5, "Start peek and splice on FD " << fd);
 
@@ -3905,7 +3905,7 @@ void ConnStateData::startPeekAndSplice()
     Comm::SetSelect(clientConnection->fd, COMM_SELECT_READ, clientPeekAndSpliceSSL, this, 0);
     switchedToHttps_ = true;
 
-    auto ssl = fd_table[clientConnection->fd].ssl;
+    auto ssl = fd_table[clientConnection->fd].ssl.get();
     BIO *b = SSL_get_rbio(ssl);
     Ssl::ClientBio *bio = static_cast<Ssl::ClientBio *>(b->ptr);
     bio->hold(true);
@@ -3941,8 +3941,8 @@ void httpsSslBumpStep2AccessCheckDone(allow_t answer, void *data)
 void
 ConnStateData::splice()
 {
-    //Normally we can splice here, because we just got client hello message
-    auto ssl = fd_table[clientConnection->fd].ssl;
+    // normally we can splice here, because we just got client hello message
+    auto ssl = fd_table[clientConnection->fd].ssl.get();
 
     //retrieve received TLS client information
     clientConnection->tlsNegotiations()->fillWith(ssl);
@@ -4001,7 +4001,7 @@ ConnStateData::startPeekAndSpliceDone()
 void
 ConnStateData::doPeekAndSpliceStep()
 {
-    auto ssl = fd_table[clientConnection->fd].ssl;
+    auto ssl = fd_table[clientConnection->fd].ssl.get();
     BIO *b = SSL_get_rbio(ssl);
     assert(b);
     Ssl::ClientBio *bio = static_cast<Ssl::ClientBio *>(b->ptr);
@@ -4559,7 +4559,7 @@ ConnStateData::handleIdleClientPinnedTlsRead()
     // renegotiations. We should close the connection except for the last case.
 
     Must(pinning.serverConnection != nullptr);
-    SSL *ssl = fd_table[pinning.serverConnection->fd].ssl;
+    auto ssl = fd_table[pinning.serverConnection->fd].ssl.get();
     if (!ssl)
         return false;
 

diff --git a/src/client_side_request.cc b/src/client_side_request.cc
index 290f74d0871c93510fcd254e2d4c7e21795d6581..2a7f17c5472f1e52196fb294a9bfd4d39ea1d2fe 100644 (file)
--- a/src/client_side_request.cc
+++ b/src/client_side_request.cc
@@ -174,7 +174,7 @@ ClientHttpRequest::ClientHttpRequest(ConnStateData * aConn) :
 
 #if USE_OPENSSL
     if (aConn->clientConnection != NULL && aConn->clientConnection->isOpen()) {
-        if (auto ssl = fd_table[aConn->clientConnection->fd].ssl)
+        if (auto ssl = fd_table[aConn->clientConnection->fd].ssl.get())
             al->cache.sslClientCert.reset(SSL_get_peer_certificate(ssl));
     }
 #endif
@@ -851,7 +851,7 @@ ClientHttpRequest::noteAdaptationAclCheckDone(Adaptation::ServiceGroupPointer g)
             ih->rfc931 = getConn()->clientConnection->rfc931;
 #if USE_OPENSSL
             if (getConn()->clientConnection->isOpen()) {
-                ih->ssluser = sslGetUserEmail(fd_table[getConn()->clientConnection->fd].ssl);
+                ih->ssluser = sslGetUserEmail(fd_table[getConn()->clientConnection->fd].ssl.get());
             }
 #endif
         }

diff --git a/src/comm.cc b/src/comm.cc
index 25bdb55b3129cd0786b0400394205e262dde62b1..212d6f64a7fcf0441fdcd306e1a3fe686d358711 100644 (file)
--- a/src/comm.cc
+++ b/src/comm.cc
@@ -822,22 +822,18 @@ old_comm_reset_close(int fd)
 void
 commStartSslClose(const FdeCbParams &params)
 {
-    assert(fd_table[params.fd].ssl != NULL);
-    ssl_shutdown_method(fd_table[params.fd].ssl);
+    assert(fd_table[params.fd].ssl);
+    ssl_shutdown_method(fd_table[params.fd].ssl.get());
 }
 #endif
 
 void
 comm_close_complete(const FdeCbParams &params)
 {
-#if USE_OPENSSL
     fde *F = &fd_table[params.fd];
+    F->ssl.reset(nullptr);
 
-    if (F->ssl) {
-        SSL_free(F->ssl);
-        F->ssl = NULL;
-    }
-
+#if USE_OPENSSL
     if (F->dynamicSslContext) {
         SSL_CTX_free(F->dynamicSslContext);
         F->dynamicSslContext = NULL;

diff --git a/src/fd.cc b/src/fd.cc
index 8425ac755b24b97086367549da76928b5ceed387..e3e27ac9a89402530417e8c5430ea56aa4b56d3d 100644 (file)
--- a/src/fd.cc
+++ b/src/fd.cc
@@ -97,7 +97,7 @@ fd_close(int fd)
     F->flags.open = false;
     fdUpdateBiggest(fd, 0);
     --Number_FD;
-    *F = fde();
+    F->clear();
 }
 
 #if _SQUID_WINDOWS_

diff --git a/src/fde.h b/src/fde.h
index fdf682e41afb8db510a271012836604d7c7f1751..35f5387d1f684c98775ea35de3cb8b35ad65b583 100644 (file)
--- a/src/fde.h
+++ b/src/fde.h
@@ -137,7 +137,6 @@ public:
                                             connection, whereas nfmarkToServer is the value to set on packets
                                             *leaving* Squid.   */
 
-private:
     /** Clear the fde class back to NULL equivalent. */
     inline void clear() {
         type = 0;
@@ -168,7 +167,7 @@ private:
         halfClosedReader = NULL;
         read_method = NULL;
         write_method = NULL;
-        ssl = NULL;
+        ssl.reset(nullptr);
         dynamicSslContext = NULL;
 #if _SQUID_WINDOWS_
         win32.handle = (long)NULL;

diff --git a/src/format/Format.cc b/src/format/Format.cc
index 0b656121a0e0e7132528f1c154030299b31d1e7b..674b45af0b094a198055cadc93f28c71a22dcda0 100644 (file)
--- a/src/format/Format.cc
+++ b/src/format/Format.cc
@@ -1174,8 +1174,8 @@ Format::Format::assemble(MemBuf &mb, const AccessLogEntry::Pointer &al, int logS
         case LFT_EXT_ACL_USER_CERT_RAW:
             if (al->request) {
                 ConnStateData *conn = al->request->clientConnectionManager.get();
-                if (conn != NULL && Comm::IsConnOpen(conn->clientConnection)) {
-                    if (SSL *ssl = fd_table[conn->clientConnection->fd].ssl)
+                if (conn && Comm::IsConnOpen(conn->clientConnection)) {
+                    if (auto ssl = fd_table[conn->clientConnection->fd].ssl.get())
                         out = sslGetUserCertificatePEM(ssl);
                 }
             }
@@ -1184,8 +1184,8 @@ Format::Format::assemble(MemBuf &mb, const AccessLogEntry::Pointer &al, int logS
         case LFT_EXT_ACL_USER_CERTCHAIN_RAW:
             if (al->request) {
                 ConnStateData *conn = al->request->clientConnectionManager.get();
-                if (conn != NULL && Comm::IsConnOpen(conn->clientConnection)) {
-                    if (SSL *ssl = fd_table[conn->clientConnection->fd].ssl)
+                if (conn && Comm::IsConnOpen(conn->clientConnection)) {
+                    if (auto ssl = fd_table[conn->clientConnection->fd].ssl.get())
                         out = sslGetUserCertificatePEM(ssl);
                 }
             }
@@ -1194,8 +1194,8 @@ Format::Format::assemble(MemBuf &mb, const AccessLogEntry::Pointer &al, int logS
         case LFT_EXT_ACL_USER_CERT:
             if (al->request) {
                 ConnStateData *conn = al->request->clientConnectionManager.get();
-                if (conn != NULL && Comm::IsConnOpen(conn->clientConnection)) {
-                    if (SSL *ssl = fd_table[conn->clientConnection->fd].ssl)
+                if (conn && Comm::IsConnOpen(conn->clientConnection)) {
+                    if (auto ssl = fd_table[conn->clientConnection->fd].ssl.get())
                         out = sslGetUserAttribute(ssl, format->data.header.header);
                 }
             }
@@ -1204,8 +1204,8 @@ Format::Format::assemble(MemBuf &mb, const AccessLogEntry::Pointer &al, int logS
         case LFT_EXT_ACL_USER_CA_CERT:
             if (al->request) {
                 ConnStateData *conn = al->request->clientConnectionManager.get();
-                if (conn != NULL && Comm::IsConnOpen(conn->clientConnection)) {
-                    if (SSL *ssl = fd_table[conn->clientConnection->fd].ssl)
+                if (conn && Comm::IsConnOpen(conn->clientConnection)) {
+                    if (auto ssl = fd_table[conn->clientConnection->fd].ssl.get())
                         out = sslGetCAAttribute(ssl, format->data.header.header);
                 }
             }

diff --git a/src/security/NegotiationHistory.cc b/src/security/NegotiationHistory.cc
index 3ee071bf4a060edc94c9792cdbace806db392dde..5bad38247d417a5dd56389f3b58a65fd6047a17d 100644 (file)
--- a/src/security/NegotiationHistory.cc
+++ b/src/security/NegotiationHistory.cc
@@ -50,10 +50,10 @@ Security::NegotiationHistory::printTlsVersion(int v) const
 #endif
 }
 
-#if USE_OPENSSL
 void
-Security::NegotiationHistory::fillWith(SSL *ssl)
+Security::NegotiationHistory::fillWith(Security::SessionPtr ssl)
 {
+#if USE_OPENSSL
     if ((cipher = SSL_get_current_cipher(ssl)) != NULL) {
         // Set the negotiated version only if the cipher negotiated
         // else probably the negotiation is not completed and version
@@ -76,8 +76,8 @@ Security::NegotiationHistory::fillWith(SSL *ssl)
     debugs(83, 5, "SSL connection info on FD " << bio->fd() <<
            " SSL version " << version_ <<
            " negotiated cipher " << cipherName());
-}
 #endif
+}
 
 const char *
 Security::NegotiationHistory::cipherName() const

diff --git a/src/security/NegotiationHistory.h b/src/security/NegotiationHistory.h
index 9a7dfd78e1aaaf8080c217d14b66f49f18085282..18f4d313f0df011e26a372169b97d5755d5cd67f 100644 (file)
--- a/src/security/NegotiationHistory.h
+++ b/src/security/NegotiationHistory.h
@@ -9,11 +9,7 @@
 #ifndef SQUID_SRC_SECURITY_NEGOTIATIONHISTORY_H
 #define SQUID_SRC_SECURITY_NEGOTIATIONHISTORY_H
 
-#if USE_OPENSSL
-#if HAVE_OPENSSL_SSL_H
-#include <openssl/ssl.h>
-#endif
-#endif
+#include "security/Session.h"
 
 namespace Security {
 
@@ -21,9 +17,7 @@ class NegotiationHistory
 {
 public:
     NegotiationHistory();
-#if USE_OPENSSL
-    void fillWith(SSL *); ///< Extract negotiation information from TLS object
-#endif
+    void fillWith(Security::SessionPtr); ///< Extract negotiation information from TLS object
     const char *cipherName() const; ///< The name of negotiated cipher
     /// String representation of TLS negotiated version
     const char *negotiatedVersion() const {return printTlsVersion(version_);}

diff --git a/src/security/Session.h b/src/security/Session.h
index 3be6f05e5b41b2baab82057449ae9fca36bba519..f095fbac67b0357ba712002d32a41d14a3dfd4a4 100644 (file)
--- a/src/security/Session.h
+++ b/src/security/Session.h
@@ -9,6 +9,9 @@
 #ifndef SQUID_SRC_SECURITY_SESSION_H
 #define SQUID_SRC_SECURITY_SESSION_H
 
+// LockingPointer.h instead of TidyPointer.h for CtoCpp1()
+#include "security/LockingPointer.h"
+
 #if USE_OPENSSL
 #if HAVE_OPENSSL_SSL_H
 #include <openssl/ssl.h>
@@ -21,17 +24,32 @@
 #endif
 #endif
 
+/*
+ * NOTE: we use TidyPointer for sessions. OpenSSL provides explicit reference
+ * locking mechanisms, but GnuTLS only provides init/deinit. To ensure matching
+ * behaviour we cannot use LockingPointer (yet) and must ensure that there is
+ * no possibility of double-free being used on the raw pointers. That is
+ * currently done by using a TidyPointer in the global fde table so its
+ * lifetime matched the connection.
+ */
+
 namespace Security {
 
 #if USE_OPENSSL
-typedef SSL* SessionPointer;
+typedef SSL* SessionPtr;
+CtoCpp1(SSL_free, SSL *);
+typedef TidyPointer<SSL, Security::SSL_free_cpp> SessionPointer;
 
 #elif USE_GNUTLS
-typedef gnutls_session_t SessionPointer;
+typedef gnutls_session_t SessionPtr;
+CtoCpp1(gnutls_deinit, gnutls_session_t);
+typedef TidyPointer<struct gnutls_session_int, Security::gnutls_deinit_cpp> SessionPointer;
 
 #else
 // use void* so we can check against NULL
-typedef void* SessionPointer;
+typedef void* SessionPtr;
+typedef TidyPointer<void, nullptr> SessionPointer;
+
 #endif
 
 } // namespace Security

diff --git a/src/ssl/PeerConnector.cc b/src/ssl/PeerConnector.cc
index 2f0199d92857db4dcdcfccd42d4cd7fa93960fee..05b8ba52fe2bd4a5abdf1023e2a0881e29d70843 100644 (file)
--- a/src/ssl/PeerConnector.cc
+++ b/src/ssl/PeerConnector.cc
@@ -65,7 +65,7 @@ Ssl::PeerConnector::start()
 {
     AsyncJob::start();
 
-    if (prepareSocket() && (initializeSsl() != NULL))
+    if (prepareSocket() && initializeSsl())
         negotiateSsl();
 }
 
@@ -99,7 +99,7 @@ Ssl::PeerConnector::prepareSocket()
     return true;
 }
 
-SSL *
+Security::SessionPtr
 Ssl::PeerConnector::initializeSsl()
 {
     Security::ContextPtr sslContext(getSslContext());
@@ -107,7 +107,7 @@ Ssl::PeerConnector::initializeSsl()
 
     const int fd = serverConnection()->fd;
 
-    SSL *ssl = Ssl::CreateClient(sslContext, fd, "server https start");
+    auto ssl = Ssl::CreateClient(sslContext, fd, "server https start");
     if (!ssl) {
         ErrorState *anErr = new ErrorState(ERR_SOCKET_FAILURE, Http::scInternalServerError, request.getRaw());
         anErr->xerrno = errno;
@@ -115,7 +115,7 @@ Ssl::PeerConnector::initializeSsl()
 
         noteNegotiationDone(anErr);
         bail(anErr);
-        return NULL;
+        return nullptr;
     }
 
     // If CertValidation Helper used do not lookup checklist for errors,
@@ -153,7 +153,7 @@ Ssl::PeerConnector::negotiateSsl()
         return;
 
     const int fd = serverConnection()->fd;
-    SSL *ssl = fd_table[fd].ssl;
+    Security::SessionPtr ssl = fd_table[fd].ssl.get();
     const int result = SSL_connect(ssl);
     if (result <= 0) {
         handleNegotiateError(result);
@@ -171,7 +171,7 @@ Ssl::PeerConnector::sslFinalized()
 {
     if (Ssl::TheConfig.ssl_crt_validator && useCertValidator_) {
         const int fd = serverConnection()->fd;
-        SSL *ssl = fd_table[fd].ssl;
+        Security::SessionPtr ssl = fd_table[fd].ssl.get();
 
         Ssl::CertValidationRequest validationRequest;
         // WARNING: Currently we do not use any locking for any of the
@@ -248,7 +248,7 @@ Ssl::PeekingPeerConnector::checkForPeekAndSplice()
     acl_checklist->banAction(allow_t(ACCESS_ALLOWED, Ssl::bumpStare));
     acl_checklist->banAction(allow_t(ACCESS_ALLOWED, Ssl::bumpClientFirst));
     acl_checklist->banAction(allow_t(ACCESS_ALLOWED, Ssl::bumpServerFirst));
-    SSL *ssl = fd_table[serverConn->fd].ssl;
+    Security::SessionPtr ssl = fd_table[serverConn->fd].ssl.get();
     BIO *b = SSL_get_rbio(ssl);
     Ssl::ServerBio *srvBio = static_cast<Ssl::ServerBio *>(b->ptr);
     if (!srvBio->canSplice())
@@ -261,7 +261,7 @@ Ssl::PeekingPeerConnector::checkForPeekAndSplice()
 void
 Ssl::PeekingPeerConnector::checkForPeekAndSpliceMatched(const Ssl::BumpMode action)
 {
-    SSL *ssl = fd_table[serverConn->fd].ssl;
+    Security::SessionPtr ssl = fd_table[serverConn->fd].ssl.get();
     BIO *b = SSL_get_rbio(ssl);
     Ssl::ServerBio *srvBio = static_cast<Ssl::ServerBio *>(b->ptr);
     debugs(83,5, "Will check for peek and splice on FD " << serverConn->fd);
@@ -368,7 +368,7 @@ Ssl::PeerConnector::sslCrtvdCheckForErrors(Ssl::CertValidationResponse const &re
     if (acl_access *acl = ::Config.ssl_client.cert_error)
         check = new ACLFilledChecklist(acl, request.getRaw(), dash_str);
 
-    SSL *ssl = fd_table[serverConnection()->fd].ssl;
+    Security::SessionPtr ssl = fd_table[serverConnection()->fd].ssl.get();
     typedef Ssl::CertValidationResponse::RecvdErrors::const_iterator SVCRECI;
     for (SVCRECI i = resp.errors.begin(); i != resp.errors.end(); ++i) {
         debugs(83, 7, "Error item: " << i->error_no << " " << i->error_reason);
@@ -425,7 +425,7 @@ Ssl::PeerConnector::handleNegotiateError(const int ret)
 {
     const int fd = serverConnection()->fd;
     unsigned long ssl_lib_error = SSL_ERROR_NONE;
-    SSL *ssl = fd_table[fd].ssl;
+    Security::SessionPtr ssl = fd_table[fd].ssl.get();
     const int ssl_error = SSL_get_error(ssl, ret);
 
     switch (ssl_error) {
@@ -490,7 +490,7 @@ Ssl::PeerConnector::noteSslNegotiationError(const int ret, const int ssl_error,
         anErr = new ErrorState(ERR_SECURE_CONNECT_FAIL, Http::scServiceUnavailable, NULL);
     anErr->xerrno = sysErrNo;
 
-    SSL *ssl = fd_table[fd].ssl;
+    Security::SessionPtr ssl = fd_table[fd].ssl.get();
     Ssl::ErrorDetail *errFromFailure = (Ssl::ErrorDetail *)SSL_get_ex_data(ssl, ssl_ex_index_ssl_error_detail);
     if (errFromFailure != NULL) {
         // The errFromFailure is attached to the ssl object
@@ -597,12 +597,12 @@ Ssl::BlindPeerConnector::getSslContext()
     return ::Config.ssl_client.sslContext;
 }
 
-SSL *
+Security::SessionPtr
 Ssl::BlindPeerConnector::initializeSsl()
 {
-    SSL *ssl = Ssl::PeerConnector::initializeSsl();
+    auto ssl = Ssl::PeerConnector::initializeSsl();
     if (!ssl)
-        return NULL;
+        return nullptr;
 
     if (const CachePeer *peer = serverConnection()->getPeer()) {
         assert(peer);
@@ -639,7 +639,7 @@ Ssl::BlindPeerConnector::noteNegotiationDone(ErrorState *error)
     }
 
     const int fd = serverConnection()->fd;
-    SSL *ssl = fd_table[fd].ssl;
+    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);
@@ -655,12 +655,12 @@ Ssl::PeekingPeerConnector::getSslContext()
     return ::Config.ssl_client.sslContext;
 }
 
-SSL *
+Security::SessionPtr
 Ssl::PeekingPeerConnector::initializeSsl()
 {
-    SSL *ssl = Ssl::PeerConnector::initializeSsl();
+    auto ssl = Ssl::PeerConnector::initializeSsl();
     if (!ssl)
-        return NULL;
+        return nullptr;
 
     if (ConnStateData *csd = request->clientConnectionManager.valid()) {
 
@@ -674,7 +674,7 @@ Ssl::PeekingPeerConnector::initializeSsl()
         SSL_set_tlsext_status_type(ssl, TLSEXT_STATUSTYPE_ocsp);
 
         // In server-first bumping mode, clientSsl is NULL.
-        if (SSL *clientSsl = fd_table[clientConn->fd].ssl) {
+        if (auto clientSsl = fd_table[clientConn->fd].ssl.get()) {
             BIO *b = SSL_get_rbio(clientSsl);
             cltBio = static_cast<Ssl::ClientBio *>(b->ptr);
             const Ssl::Bio::sslFeatures &features = cltBio->receivedHelloFeatures();
@@ -742,7 +742,7 @@ Ssl::PeekingPeerConnector::initializeSsl()
 void
 Ssl::PeekingPeerConnector::noteNegotiationDone(ErrorState *error)
 {
-    SSL *ssl = fd_table[serverConnection()->fd].ssl;
+    Security::SessionPtr ssl = fd_table[serverConnection()->fd].ssl.get();
 
     // Check the list error with
     if (!request->clientConnectionManager.valid() || ! ssl)
@@ -788,7 +788,7 @@ Ssl::PeekingPeerConnector::noteNegotiationDone(ErrorState *error)
         serverCertificateVerified();
         if (splice) {
             //retrieved received TLS client informations
-            SSL *clientSsl = fd_table[clientConn->fd].ssl;
+            auto clientSsl = fd_table[clientConn->fd].ssl.get();
             clientConn->tlsNegotiations()->fillWith(clientSsl);
             switchToTunnel(request.getRaw(), clientConn, serverConn);
         }
@@ -799,7 +799,7 @@ void
 Ssl::PeekingPeerConnector::noteWantWrite()
 {
     const int fd = serverConnection()->fd;
-    SSL *ssl = fd_table[fd].ssl;
+    Security::SessionPtr ssl = fd_table[fd].ssl.get();
     BIO *b = SSL_get_rbio(ssl);
     Ssl::ServerBio *srvBio = static_cast<Ssl::ServerBio *>(b->ptr);
 
@@ -816,7 +816,7 @@ void
 Ssl::PeekingPeerConnector::noteSslNegotiationError(const int result, const int ssl_error, const int ssl_lib_error)
 {
     const int fd = serverConnection()->fd;
-    SSL *ssl = fd_table[fd].ssl;
+    Security::SessionPtr ssl = fd_table[fd].ssl.get();
     BIO *b = SSL_get_rbio(ssl);
     Ssl::ServerBio *srvBio = static_cast<Ssl::ServerBio *>(b->ptr);
 
@@ -865,7 +865,7 @@ Ssl::PeekingPeerConnector::handleServerCertificate()
 
     if (ConnStateData *csd = request->clientConnectionManager.valid()) {
         const int fd = serverConnection()->fd;
-        SSL *ssl = fd_table[fd].ssl;
+        Security::SessionPtr ssl = fd_table[fd].ssl.get();
         Security::CertPointer serverCert(SSL_get_peer_certificate(ssl));
         if (!serverCert.get())
             return;
@@ -888,7 +888,7 @@ Ssl::PeekingPeerConnector::serverCertificateVerified()
             serverCert.resetAndLock(serverBump->serverCert.get());
         else {
             const int fd = serverConnection()->fd;
-            SSL *ssl = fd_table[fd].ssl;
+            Security::SessionPtr ssl = fd_table[fd].ssl.get();
             serverCert.reset(SSL_get_peer_certificate(ssl));
         }
         if (serverCert.get()) {

diff --git a/src/ssl/PeerConnector.h b/src/ssl/PeerConnector.h
index 3fd0fc4f332a1ebf8be80375e06aff79e2a8301c..c48321e8d48b70a22bfa109cb1db6225e5fbed12 100644 (file)
--- a/src/ssl/PeerConnector.h
+++ b/src/ssl/PeerConnector.h
@@ -99,7 +99,7 @@ protected:
     /// silent server
     void setReadTimeout();
 
-    virtual SSL *initializeSsl(); ///< Initializes SSL state
+    virtual Security::SessionPtr initializeSsl(); ///< Initializes SSL state
 
     /// Performs a single secure connection negotiation step.
     /// It is called multiple times untill the negotiation finish or aborted.
@@ -193,7 +193,7 @@ public:
 
     /// Calls parent initializeSSL, configure the created SSL object to try reuse SSL session
     /// and sets the hostname to use for certificates validation
-    virtual SSL *initializeSsl();
+    virtual Security::SessionPtr initializeSsl();
 
     /// Return the configured Security::ContextPtr object
     virtual Security::ContextPtr getSslContext();
@@ -222,7 +222,7 @@ public:
     }
 
     /* PeerConnector API */
-    virtual SSL *initializeSsl();
+    virtual Security::SessionPtr initializeSsl();
     virtual Security::ContextPtr getSslContext();
     virtual void noteWantWrite();
     virtual void noteSslNegotiationError(const int result, const int ssl_error, const int ssl_lib_error);

diff --git a/src/ssl/bio.cc b/src/ssl/bio.cc
index 84c0592c7fe8f803e8006d451ba347ffaea517ac..55e49fc07cf0644f793a49f4b4c0cd5136d3df89 100644 (file)
--- a/src/ssl/bio.cc
+++ b/src/ssl/bio.cc
@@ -440,7 +440,7 @@ Ssl::ServerBio::write(const char *buf, int size, BIO *table)
             //Hello message is the first message we write to server
             assert(helloMsg.isEmpty());
 
-            SSL *ssl = fd_table[fd_].ssl;
+            auto ssl = fd_table[fd_].ssl.get();
             if (clientFeatures.initialized_ && ssl) {
                 if (bumpMode_ == Ssl::bumpPeek) {
                     if (adjustSSL(ssl, clientFeatures))

diff --git a/src/ssl/gadgets.h b/src/ssl/gadgets.h
index d1111a20088f75cdbfd8b7e6fd91e63531a61e24..30c24b1b861ffb187c35e5a6aad075927d13bc64 100644 (file)
--- a/src/ssl/gadgets.h
+++ b/src/ssl/gadgets.h
@@ -69,9 +69,6 @@ typedef TidyPointer<X509_REQ, X509_REQ_free_cpp> X509_REQ_Pointer;
 CtoCpp1(SSL_CTX_free, SSL_CTX *)
 typedef TidyPointer<SSL_CTX, SSL_CTX_free_cpp> SSL_CTX_Pointer;
 
-CtoCpp1(SSL_free, SSL *)
-typedef TidyPointer<SSL, SSL_free_cpp> SSL_Pointer;
-
 sk_free_wrapper(sk_X509_NAME, STACK_OF(X509_NAME) *, X509_NAME_free)
 typedef TidyPointer<STACK_OF(X509_NAME), sk_X509_NAME_free_wrapper> X509_NAME_STACK_Pointer;
 

diff --git a/src/ssl/support.cc b/src/ssl/support.cc
index d4fc4c8196cac54f141bf2d6db01a1635949e77b..6b9688fb368cefe309ef5b8494d3ec1aecdf93a3 100644 (file)
--- a/src/ssl/support.cc
+++ b/src/ssl/support.cc
@@ -697,8 +697,7 @@ sslCreateClientContext(Security::PeerOptions &peer, long options, long fl)
 int
 ssl_read_method(int fd, char *buf, int len)
 {
-    SSL *ssl = fd_table[fd].ssl;
-    int i;
+    auto ssl = fd_table[fd].ssl.get();
 
 #if DONT_DO_THIS
 
@@ -709,7 +708,7 @@ ssl_read_method(int fd, char *buf, int len)
 
 #endif
 
-    i = SSL_read(ssl, buf, len);
+    int i = SSL_read(ssl, buf, len);
 
     if (i > 0 && SSL_pending(ssl) > 0) {
         debugs(83, 2, "SSL FD " << fd << " is pending");
@@ -724,16 +723,13 @@ ssl_read_method(int fd, char *buf, int len)
 int
 ssl_write_method(int fd, const char *buf, int len)
 {
-    SSL *ssl = fd_table[fd].ssl;
-    int i;
-
+    auto ssl = fd_table[fd].ssl.get();
     if (!SSL_is_init_finished(ssl)) {
         errno = ENOTCONN;
         return -1;
     }
 
-    i = SSL_write(ssl, buf, len);
-
+    int i = SSL_write(ssl, buf, len);
     return i;
 }
 
@@ -1035,7 +1031,7 @@ bool Ssl::verifySslCertificate(Security::ContextPtr sslContext, CertificatePrope
     assert(0);
 #else
     // Temporary ssl for getting X509 certificate from SSL_CTX.
-    Ssl::SSL_Pointer ssl(SSL_new(sslContext));
+    Security::SessionPointer ssl(SSL_new(sslContext));
     X509 * cert = SSL_get_certificate(ssl.get());
 #endif
     if (!cert)
@@ -1303,16 +1299,15 @@ SslCreate(Security::ContextPtr sslContext, const int fd, Ssl::Bio::Type type, co
 
     const char *errAction = NULL;
     int errCode = 0;
-    if (SSL *ssl = SSL_new(sslContext)) {
+    if (auto ssl = SSL_new(sslContext)) {
         // 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 = ssl;
+            fd_table[fd].ssl.reset(ssl);
             fd_table[fd].read_method = &ssl_read_method;
             fd_table[fd].write_method = &ssl_write_method;
             fd_note(fd, squidCtx);
-
             return ssl;
         }
         errCode = ERR_get_error();

diff --git a/src/stat.cc b/src/stat.cc
index c62ae45d6c3d5e7dd3b554ec7956dfc378a1a30c..e3f170b89015f442fc24156457da4c654783097f 100644 (file)
--- a/src/stat.cc
+++ b/src/stat.cc
@@ -1889,10 +1889,8 @@ statClientRequests(StoreEntry * s)
             p = conn->clientConnection->rfc931;
 
 #if USE_OPENSSL
-
         if (!p && conn != NULL && Comm::IsConnOpen(conn->clientConnection))
-            p = sslGetUserEmail(fd_table[conn->clientConnection->fd].ssl);
-
+            p = sslGetUserEmail(fd_table[conn->clientConnection->fd].ssl.get());
 #endif
 
         if (!p)

diff --git a/src/tests/stub_libsecurity.cc b/src/tests/stub_libsecurity.cc
index 6c09cefcc8a39938fd2e95f5c79a8145496f8a66..5d296a6cb3074eb26bd1baf8230be5ec2317b272 100644 (file)
--- a/src/tests/stub_libsecurity.cc
+++ b/src/tests/stub_libsecurity.cc
@@ -37,11 +37,8 @@ Security::ContextPtr Security::ServerOptions::createBlankContext() const STUB
 void Security::ServerOptions::updateContextEecdh(Security::ContextPtr &) STUB
 
 #include "security/NegotiationHistory.h"
-
 Security::NegotiationHistory::NegotiationHistory() STUB
-#if USE_OPENSSL
-void Security::NegotiationHistory::fillWith(SSL *) STUB
-#endif
+void Security::NegotiationHistory::fillWith(Security::SessionPtr) STUB
 const char *Security::NegotiationHistory::cipherName() const STUB
 const char *Security::NegotiationHistory::printTlsVersion(int) const STUB
 

diff --git a/src/tunnel.cc b/src/tunnel.cc
index 961949c340ce66cce811ac5131ef1f408922fffb..2aae972b9a5f443ec8e4a06dc266b40a4a9b3dfc 100644 (file)
--- a/src/tunnel.cc
+++ b/src/tunnel.cc
@@ -1297,7 +1297,7 @@ switchToTunnel(HttpRequest *request, Comm::ConnectionPointer &clientConn, Comm::
     fd_table[srvConn->fd].read_method = &default_read_method;
     fd_table[srvConn->fd].write_method = &default_write_method;
 
-    auto ssl = fd_table[srvConn->fd].ssl;
+    auto ssl = fd_table[srvConn->fd].ssl.get();
     assert(ssl);
     BIO *b = SSL_get_rbio(ssl);
     Ssl::ServerBio *srvBio = static_cast<Ssl::ServerBio *>(b->ptr);