From: Amos Jeffries Date: Wed, 17 Aug 2016 14:24:07 +0000 (+1200) Subject: Cleanup: remove many needless references to SSL X-Git-Tag: SQUID_4_0_14~25 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=0166128b48098dd2c666f2b628890b8f81dd2428;p=thirdparty%2Fsquid.git Cleanup: remove many needless references to SSL --- diff --git a/src/Makefile.am b/src/Makefile.am index 40bf2de2e3..11180a0827 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -546,12 +546,12 @@ squid_LDADD = \ ip/libip.la \ fs/libfs.la \ DiskIO/libdiskio.la \ + comm/libcomm.la \ $(SSL_LIBS) \ + security/libsecurity.la \ ipc/libipc.la \ mgr/libmgr.la \ anyp/libanyp.la \ - comm/libcomm.la \ - security/libsecurity.la \ parser/libparser.la \ eui/libeui.la \ icmp/libicmp.la \ diff --git a/src/adaptation/icap/Xaction.cc b/src/adaptation/icap/Xaction.cc index 180bf04574..79ee247440 100644 --- a/src/adaptation/icap/Xaction.cc +++ b/src/adaptation/icap/Xaction.cc @@ -59,10 +59,10 @@ public: AccessLogEntry::Pointer const &alp, const time_t timeout = 0): AsyncJob("Ssl::IcapPeerConnector"), - PeerConnector(aServerConn, aCallback, alp, timeout), icapService(service) {} + Security::PeerConnector(aServerConn, aCallback, alp, timeout), icapService(service) {} /* Security::PeerConnector API */ - virtual bool initializeTls(Security::SessionPointer &); + virtual bool initialize(Security::SessionPointer &); virtual void noteNegotiationDone(ErrorState *error); virtual Security::ContextPtr getSslContext() {return icapService->sslContext;} @@ -301,8 +301,8 @@ void Adaptation::Icap::Xaction::noteCommConnected(const CommConnectCbParams &io) CloseDialer(this,&Adaptation::Icap::Xaction::noteCommClosed)); comm_add_close_handler(io.conn->fd, closer); - // If it is a reused connection and the SSL object is build - // we should not negotiate new SSL session + // If it is a reused connection and the TLS object is built + // we should not negotiate new TLS session const auto &ssl = fd_table[io.conn->fd].ssl; if (!ssl && service().cfg().secure.encryptTransport) { CbcPointer me(this); @@ -705,9 +705,9 @@ bool Adaptation::Icap::Xaction::fillVirginHttpHeader(MemBuf &) const } bool -Ssl::IcapPeerConnector::initializeTls(Security::SessionPointer &serverSession) +Ssl::IcapPeerConnector::initialize(Security::SessionPointer &serverSession) { - if (!Security::PeerConnector::initializeTls(serverSession)) + if (!Security::PeerConnector::initialize(serverSession)) return false; #if USE_OPENSSL @@ -764,13 +764,13 @@ Adaptation::Icap::Xaction::handleSecuredPeer(Security::EncryptorAnswer &answer) if (answer.conn != NULL) answer.conn->close(); debugs(93, 2, typeName << - " SSL negotiation to " << service().cfg().uri << " failed"); + " TLS negotiation to " << service().cfg().uri << " failed"); service().noteConnectionFailed("failure"); detailError(ERR_DETAIL_ICAP_XACT_SSL_START); - throw TexcHere("cannot connect to the SSL ICAP service"); + throw TexcHere("cannot connect to the TLS ICAP service"); } - debugs(93, 5, "SSL negotiation to " << service().cfg().uri << " complete"); + debugs(93, 5, "TLS negotiation to " << service().cfg().uri << " complete"); service().noteConnectionUse(answer.conn); diff --git a/src/security/BlindPeerConnector.cc b/src/security/BlindPeerConnector.cc index 6e5d16fb73..54631f3305 100644 --- a/src/security/BlindPeerConnector.cc +++ b/src/security/BlindPeerConnector.cc @@ -9,6 +9,7 @@ #include "squid.h" #include "CachePeer.h" #include "comm/Connection.h" +#include "errorpage.h" #include "fde.h" #include "HttpRequest.h" #include "neighbors.h" @@ -30,9 +31,9 @@ Security::BlindPeerConnector::getSslContext() } bool -Security::BlindPeerConnector::initializeTls(Security::SessionPointer &serverSession) +Security::BlindPeerConnector::initialize(Security::SessionPointer &serverSession) { - if (!Security::PeerConnector::initializeTls(serverSession)) + if (!Security::PeerConnector::initialize(serverSession)) return false; if (const CachePeer *peer = serverConnection()->getPeer()) { diff --git a/src/security/BlindPeerConnector.h b/src/security/BlindPeerConnector.h index fe529ea425..4db37a03f8 100644 --- a/src/security/BlindPeerConnector.h +++ b/src/security/BlindPeerConnector.h @@ -6,11 +6,13 @@ * Please see the COPYING and CONTRIBUTORS files for details. */ -#ifndef SQUID_SRC_SSL_BLINDPEERCONNECTOR_H -#define SQUID_SRC_SSL_BLINDPEERCONNECTOR_H +#ifndef SQUID_SRC_SECURITY_BLINDPEERCONNECTOR_H +#define SQUID_SRC_SECURITY_BLINDPEERCONNECTOR_H #include "security/PeerConnector.h" +class ErrorState; + namespace Security { @@ -31,20 +33,21 @@ public: /* Security::PeerConnector API */ - /// Calls parent initializeTls(), configure the created TLS session object to - /// try reuse TLS session and sets the hostname to use for certificates validation + /// Calls parent initialize(), configures the created TLS session object + /// to try and reuse a TLS session and sets the hostname to use for + /// certificate validation /// \returns true on successful initialization - virtual bool initializeTls(Security::SessionPointer &); + virtual bool initialize(Security::SessionPointer &); /// Return the configured Security::ContextPtr object virtual Security::ContextPtr getSslContext(); - /// On error calls peerConnectFailed function, on success store the used SSL session - /// for later use - virtual void noteNegotiationDone(ErrorState *error); + /// On error calls peerConnectFailed(). + /// On success store the used TLS session for later use. + virtual void noteNegotiationDone(ErrorState *); }; } // namespace Security -#endif /* SQUID_SRC_SSL_BLINDPEERCONNECTOR_H */ +#endif /* SQUID_SRC_SECURITY_BLINDPEERCONNECTOR_H */ diff --git a/src/security/PeerConnector.cc b/src/security/PeerConnector.cc index cc4c608d8c..30ae857dd2 100644 --- a/src/security/PeerConnector.cc +++ b/src/security/PeerConnector.cc @@ -60,8 +60,8 @@ Security::PeerConnector::start() AsyncJob::start(); Security::SessionPointer tmp; - if (prepareSocket() && initializeTls(tmp)) - negotiateSsl(); + if (prepareSocket() && initialize(tmp)) + negotiate(); else mustStop("Security::PeerConnector TLS socket initialize failed"); } @@ -97,7 +97,7 @@ Security::PeerConnector::prepareSocket() } bool -Security::PeerConnector::initializeTls(Security::SessionPointer &serverSession) +Security::PeerConnector::initialize(Security::SessionPointer &serverSession) { #if USE_OPENSSL Security::ContextPtr sslContext(getSslContext()); @@ -106,7 +106,7 @@ Security::PeerConnector::initializeTls(Security::SessionPointer &serverSession) 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)); + debugs(83, DBG_IMPORTANT, "Error allocating TLS handle: " << ERR_error_string(ERR_get_error(), NULL)); noteNegotiationDone(anErr); bail(anErr); return false; @@ -166,15 +166,17 @@ Security::PeerConnector::recordNegotiationDetails() } void -Security::PeerConnector::negotiateSsl() +Security::PeerConnector::negotiate() { - if (!Comm::IsConnOpen(serverConnection()) || fd_table[serverConnection()->fd].closing()) + if (!Comm::IsConnOpen(serverConnection())) return; -#if USE_OPENSSL const int fd = serverConnection()->fd; - Security::SessionPtr ssl = fd_table[fd].ssl.get(); - const int result = SSL_connect(ssl); + if (fd_table[fd].closing()) + return; + +#if USE_OPENSSL + const int result = SSL_connect(fd_table[fd].ssl.get()); #else const int result = -1; #endif @@ -345,9 +347,9 @@ Security::PeerConnector::sslCrtvdCheckForErrors(Ssl::CertValidationResponse cons void Security::PeerConnector::NegotiateSsl(int, void *data) { - PeerConnector *pc = static_cast(data); + PeerConnector *pc = static_cast(data); // Use job calls to add done() checks and other job logic/protections. - CallJobHere(83, 7, pc, Security::PeerConnector, negotiateSsl); + CallJobHere(83, 7, pc, Security::PeerConnector, negotiate); } void @@ -380,7 +382,7 @@ Security::PeerConnector::handleNegotiateError(const int ret) // Log connection details, if any recordNegotiationDetails(); - noteSslNegotiationError(ret, ssl_error, ssl_lib_error); + noteNegotiationError(ret, ssl_error, ssl_lib_error); #endif } @@ -423,7 +425,7 @@ Security::PeerConnector::noteWantWrite() } void -Security::PeerConnector::noteSslNegotiationError(const int ret, const int ssl_error, const int ssl_lib_error) +Security::PeerConnector::noteNegotiationError(const int ret, const int ssl_error, const int ssl_lib_error) { #if USE_OPENSSL // not used unless OpenSSL enabled. #if defined(EPROTO) @@ -449,7 +451,7 @@ Security::PeerConnector::noteSslNegotiationError(const int ret, const int ssl_er anErr->xerrno = sysErrNo; Security::SessionPtr ssl = fd_table[fd].ssl.get(); - Ssl::ErrorDetail *errFromFailure = (Ssl::ErrorDetail *)SSL_get_ex_data(ssl, ssl_ex_index_ssl_error_detail); + Ssl::ErrorDetail *errFromFailure = static_cast(SSL_get_ex_data(ssl, ssl_ex_index_ssl_error_detail)); if (errFromFailure != NULL) { // The errFromFailure is attached to the ssl object // and will be released when ssl object destroyed. diff --git a/src/security/PeerConnector.h b/src/security/PeerConnector.h index 2480ddf343..b1f7cd220f 100644 --- a/src/security/PeerConnector.h +++ b/src/security/PeerConnector.h @@ -6,13 +6,14 @@ * Please see the COPYING and CONTRIBUTORS files for details. */ -#ifndef SQUID_SRC_SSL_PEERCONNECTOR_H -#define SQUID_SRC_SSL_PEERCONNECTOR_H +#ifndef SQUID_SRC_SECURITY_PEERCONNECTOR_H +#define SQUID_SRC_SECURITY_PEERCONNECTOR_H #include "acl/Acl.h" #include "base/AsyncCbdataCalls.h" #include "base/AsyncJob.h" #include "CommCalls.h" +#include "http/forward.h" #include "security/EncryptorAnswer.h" #include "security/forward.h" #if USE_OPENSSL @@ -22,7 +23,6 @@ #include #include -class HttpRequest; class ErrorState; class AccessLogEntry; typedef RefCount AccessLogEntryPointer; @@ -31,26 +31,28 @@ namespace Security { /** - * Connects Squid to SSL/TLS-capable peers or services. - * Contains common code and interfaces of various specialized PeerConnectors, + * Initiates encryption on a connection to peers or servers. + * Despite its name does not perform any connect(2) operations. + * + * Contains common code and interfaces of various specialized PeerConnector's, * including peer certificate validation code. \par * The caller receives a call back with Security::EncryptorAnswer. If answer.error - * is not nil, then there was an error and the SSL connection to the SSL peer + * is not nil, then there was an error and the encryption to the peer or server * was not fully established. The error object is suitable for error response * generation. \par * The caller must monitor the connection for closure because this * job will not inform the caller about such events. \par - * PeerConnector class curently supports a form of SSL negotiation timeout, - * which accounted only when sets the read timeout from SSL peer. + * PeerConnector class currently supports a form of TLS negotiation timeout, + * which is accounted only when sets the read timeout from encrypted peers/servers. * For a complete solution, the caller must monitor the overall connection * establishment timeout and close the connection on timeouts. This is probably * better than having dedicated (or none at all!) timeouts for peer selection, * DNS lookup, TCP handshake, SSL handshake, etc. Some steps may have their * own timeout, but not all steps should be forced to have theirs. - * XXX: tunnel.cc and probably other subsystems does not have an "overall + * XXX: tunnel.cc and probably other subsystems do not have an "overall * connection establishment" timeout. We need to change their code so that they * start monitoring earlier and close on timeouts. This change may need to be * discussed on squid-dev. @@ -63,7 +65,7 @@ class PeerConnector: virtual public AsyncJob CBDATA_CLASS(PeerConnector); public: - /// Callback dialier API to allow PeerConnector to set the answer. + /// Callback dialer API to allow PeerConnector to set the answer. class CbDialer { public: @@ -72,8 +74,6 @@ public: virtual Security::EncryptorAnswer &answer() = 0; }; - typedef RefCount HttpRequestPointer; - public: PeerConnector(const Comm::ConnectionPointer &aServerConn, AsyncCall::Pointer &aCallback, @@ -104,19 +104,19 @@ protected: void setReadTimeout(); /// \returns true on successful TLS session initialization - virtual bool initializeTls(Security::SessionPointer &); + virtual bool initialize(Security::SessionPointer &); /// Performs a single secure connection negotiation step. - /// It is called multiple times untill the negotiation finish or aborted. - void negotiateSsl(); + /// It is called multiple times untill the negotiation finishes or aborts. + void negotiate(); - /// Called after SSL negotiations have finished. Cleans up SSL state. + /// Called after negotiation has finished. Cleans up TLS/SSL state. /// Returns false if we are now waiting for the certs validation job. /// Otherwise, returns true, regardless of negotiation success/failure. bool sslFinalized(); - /// Called when the SSL negotiation step aborted because data needs to - /// be transferred to/from SSL server or on error. In the first case + /// Called when the negotiation step aborted because data needs to + /// be transferred to/from server or on error. In the first case /// setups the appropriate Comm::SetSelect handler. In second case /// fill an error and report to the PeerConnector caller. void handleNegotiateError(const int result); @@ -148,7 +148,7 @@ protected: /// \param result the SSL_connect return code /// \param ssl_error the error code returned from the SSL_get_error function /// \param ssl_lib_error the error returned from the ERR_Get_Error function - virtual void noteSslNegotiationError(const int result, const int ssl_error, const int ssl_lib_error); + virtual void noteNegotiationError(const int result, const int ssl_error, const int ssl_lib_error); /// Called when the SSL negotiation to the server completed and the certificates /// validated using the cert validator. @@ -156,7 +156,7 @@ protected: virtual void noteNegotiationDone(ErrorState *error) {} /// Must implemented by the kid classes to return the Security::ContextPtr object to use - /// for building the SSL objects. + /// for building the encryption context objects. virtual Security::ContextPtr getSslContext() = 0; /// mimics FwdState to minimize changes to FwdState::initiate/negotiateSsl @@ -210,5 +210,5 @@ private: } // namespace Security -#endif /* SQUID_SRC_SSL_PEERCONNECTOR_H */ +#endif /* SQUID_SRC_SECURITY_PEERCONNECTOR_H */ diff --git a/src/ssl/PeekingPeerConnector.cc b/src/ssl/PeekingPeerConnector.cc index 05ed6f2ad8..d7ddaad526 100644 --- a/src/ssl/PeekingPeerConnector.cc +++ b/src/ssl/PeekingPeerConnector.cc @@ -134,9 +134,9 @@ Ssl::PeekingPeerConnector::getSslContext() } bool -Ssl::PeekingPeerConnector::initializeTls(Security::SessionPointer &serverSession) +Ssl::PeekingPeerConnector::initialize(Security::SessionPointer &serverSession) { - if (!Security::PeerConnector::initializeTls(serverSession)) + if (!Security::PeerConnector::initialize(serverSession)) return false; if (ConnStateData *csd = request->clientConnectionManager.valid()) { @@ -277,7 +277,7 @@ Ssl::PeekingPeerConnector::noteWantWrite() } void -Ssl::PeekingPeerConnector::noteSslNegotiationError(const int result, const int ssl_error, const int ssl_lib_error) +Ssl::PeekingPeerConnector::noteNegotiationError(const int result, const int ssl_error, const int ssl_lib_error) { const int fd = serverConnection()->fd; Security::SessionPtr ssl = fd_table[fd].ssl.get(); @@ -318,7 +318,7 @@ Ssl::PeekingPeerConnector::noteSslNegotiationError(const int result, const int s } // else call parent noteNegotiationError to produce an error page - Security::PeerConnector::noteSslNegotiationError(result, ssl_error, ssl_lib_error); + Security::PeerConnector::noteNegotiationError(result, ssl_error, ssl_lib_error); } void diff --git a/src/ssl/PeekingPeerConnector.h b/src/ssl/PeekingPeerConnector.h index d55af8968a..b4ea5066af 100644 --- a/src/ssl/PeekingPeerConnector.h +++ b/src/ssl/PeekingPeerConnector.h @@ -37,10 +37,10 @@ public: } /* Security::PeerConnector API */ - virtual bool initializeTls(Security::SessionPointer &); + virtual bool initialize(Security::SessionPointer &); virtual Security::ContextPtr getSslContext(); virtual void noteWantWrite(); - virtual void noteSslNegotiationError(const int result, const int ssl_error, const int ssl_lib_error); + virtual void noteNegotiationError(const int result, const int ssl_error, const int ssl_lib_error); virtual void noteNegotiationDone(ErrorState *error); /// Updates associated client connection manager members diff --git a/src/tests/stub_libsecurity.cc b/src/tests/stub_libsecurity.cc index f8c0f2da0b..95f88d2cf6 100644 --- a/src/tests/stub_libsecurity.cc +++ b/src/tests/stub_libsecurity.cc @@ -18,10 +18,11 @@ CBDATA_NAMESPACED_CLASS_INIT(Security, BlindPeerConnector); namespace Security { -bool BlindPeerConnector::initializeTls(Security::SessionPointer &) STUB_RETVAL(false) +bool BlindPeerConnector::initialize(Security::SessionPointer &) STUB_RETVAL(false) Security::ContextPtr BlindPeerConnector::getSslContext() STUB_RETVAL(nullptr) void BlindPeerConnector::noteNegotiationDone(ErrorState *) STUB } + #include "security/EncryptorAnswer.h" Security::EncryptorAnswer::~EncryptorAnswer() {} std::ostream &Security::operator <<(std::ostream &os, const Security::EncryptorAnswer &) STUB_RETVAL(os) @@ -52,13 +53,13 @@ void PeerConnector::commCloseHandler(const CommCloseCbParams &) STUB void PeerConnector::connectionClosed(const char *) STUB bool PeerConnector::prepareSocket() STUB_RETVAL(false) void PeerConnector::setReadTimeout() STUB -bool PeerConnector::initializeTls(Security::SessionPointer &) STUB_RETVAL(false) -void PeerConnector::negotiateSsl() STUB +bool PeerConnector::initialize(Security::SessionPointer &) STUB_RETVAL(false) +void PeerConnector::negotiate() STUB bool PeerConnector::sslFinalized() STUB_RETVAL(false) void PeerConnector::handleNegotiateError(const int) STUB void PeerConnector::noteWantRead() STUB void PeerConnector::noteWantWrite() STUB -void PeerConnector::noteSslNegotiationError(const int, const int, const int) STUB +void PeerConnector::noteNegotiationError(const int, const int, const int) STUB // virtual Security::ContextPtr getSslContext() = 0; void PeerConnector::bail(ErrorState *) STUB void PeerConnector::callBack() STUB diff --git a/src/tunnel.cc b/src/tunnel.cc index 0a76aaa452..a952fecc24 100644 --- a/src/tunnel.cc +++ b/src/tunnel.cc @@ -174,7 +174,7 @@ public: void connectToPeer(); private: - /// Gives PeerConnector access to Answer in the TunnelStateData callback dialer. + /// Gives Security::PeerConnector access to Answer in the TunnelStateData callback dialer. class MyAnswerDialer: public CallDialer, public Security::PeerConnector::CbDialer { public: diff --git a/src/url.cc b/src/url.cc index 8cfe220eb9..6b37a84b45 100644 --- a/src/url.cc +++ b/src/url.cc @@ -804,11 +804,7 @@ urlCheckRequest(const HttpRequest * r) case AnyP::PROTO_HTTPS: #if USE_OPENSSL - rc = 1; - - break; - #else /* * Squid can't originate an SSL connection, so it should @@ -816,8 +812,8 @@ urlCheckRequest(const HttpRequest * r) * CONNECT instead. */ rc = 0; - #endif + break; default: break;