From: Amos Jeffries Date: Mon, 11 Jul 2016 10:57:11 +0000 (+1200) Subject: Cleanup: Ssl::Create*() functions X-Git-Tag: SQUID_4_0_13~36 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=157c5ace5e19cb95f7b62fd5e34a72935cf76c4d;p=thirdparty%2Fsquid.git 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 784abea28a..952ecaafed 100644 --- a/src/client_side.cc +++ b/src/client_side.cc @@ -2577,16 +2577,16 @@ httpAccept(const CommAcceptCbParams ¶ms) #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 TimeoutDialer; diff --git a/src/ssl/PeerConnector.cc b/src/ssl/PeerConnector.cc index 0e22b8ce70..f4a936d134 100644 --- 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 637ae3e5fe..4a0502c241 100644 --- 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 0ef586762b..2142beb28f 100644 --- 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 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.