From: Amos Jeffries Date: Fri, 20 Mar 2015 15:10:07 +0000 (-0700) Subject: Move Ssl::PeerConnectorAnswer to Security::EncryptorAnswer X-Git-Tag: merge-candidate-3-v1~205^2 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=fcfdf7f9147f5aacc176fe2cba68ebd3245c8b8e;p=thirdparty%2Fsquid.git Move Ssl::PeerConnectorAnswer to Security::EncryptorAnswer This class was not actually depending on OpenSSL API symbols and by abstracting it out we can unify the callback handlers for encrypted and non-encrypted logic paths for several classes that setup connections. --- diff --git a/src/FwdState.cc b/src/FwdState.cc index 18a967bc38..8a23111de1 100644 --- a/src/FwdState.cc +++ b/src/FwdState.cc @@ -58,6 +58,8 @@ #include "ssl/PeerConnector.h" #include "ssl/ServerBump.h" #include "ssl/support.h" +#else +#include "security/EncryptorAnswer.h" #endif #include @@ -78,7 +80,7 @@ CBDATA_CLASS_INIT(FwdState); class FwdStatePeerAnswerDialer: public CallDialer, public Ssl::PeerConnector::CbDialer { public: - typedef void (FwdState::*Method)(Ssl::PeerConnectorAnswer &); + typedef void (FwdState::*Method)(Security::EncryptorAnswer &); FwdStatePeerAnswerDialer(Method method, FwdState *fwd): method_(method), fwd_(fwd), answer_() {} @@ -91,12 +93,12 @@ public: } /* Ssl::PeerConnector::CbDialer API */ - virtual Ssl::PeerConnectorAnswer &answer() { return answer_; } + virtual Security::EncryptorAnswer &answer() { return answer_; } private: Method method_; CbcPointer fwd_; - Ssl::PeerConnectorAnswer answer_; + Security::EncryptorAnswer answer_; }; #endif @@ -701,16 +703,13 @@ FwdState::connectDone(const Comm::ConnectionPointer &conn, Comm::Flag status, in } #endif - // should reach ConnStateData before the dispatched Client job starts - CallJobHere1(17, 4, request->clientConnectionManager, ConnStateData, - ConnStateData::notePeerConnection, serverConnection()); - - dispatch(); + // if not encrypting just run the post-connect actions + Security::EncryptorAnswer nil; + connectedToPeer(nil); } -#if USE_OPENSSL void -FwdState::connectedToPeer(Ssl::PeerConnectorAnswer &answer) +FwdState::connectedToPeer(Security::EncryptorAnswer &answer) { if (ErrorState *error = answer.error.get()) { fail(error); @@ -719,9 +718,12 @@ FwdState::connectedToPeer(Ssl::PeerConnectorAnswer &answer) return; } + // should reach ConnStateData before the dispatched Client job starts + CallJobHere1(17, 4, request->clientConnectionManager, ConnStateData, + ConnStateData::notePeerConnection, serverConnection()); + dispatch(); } -#endif void FwdState::connectTimeout(int fd) diff --git a/src/FwdState.h b/src/FwdState.h index 988a3a0eb5..ff47632f28 100644 --- a/src/FwdState.h +++ b/src/FwdState.h @@ -16,6 +16,7 @@ #include "fde.h" #include "http/StatusCode.h" #include "ip/Address.h" +#include "security/forward.h" #if USE_OPENSSL #include "ssl/support.h" #endif @@ -34,7 +35,6 @@ namespace Ssl { class ErrorDetail; class CertValidationResponse; -class PeerConnectorAnswer; }; #endif @@ -114,9 +114,7 @@ private: void completed(); void retryOrBail(); ErrorState *makeConnectingError(const err_type type) const; -#if USE_OPENSSL - void connectedToPeer(Ssl::PeerConnectorAnswer &answer); -#endif + void connectedToPeer(Security::EncryptorAnswer &answer); static void RegisterWithCacheManager(void); /// stops monitoring server connection for closure and updates pconn stats diff --git a/src/PeerPoolMgr.cc b/src/PeerPoolMgr.cc index 80f4cfb9c3..52d9bb236e 100644 --- a/src/PeerPoolMgr.cc +++ b/src/PeerPoolMgr.cc @@ -24,21 +24,23 @@ #include "SquidTime.h" #if USE_OPENSSL #include "ssl/PeerConnector.h" +#else +#include "security/EncryptorAnswer.h" #endif CBDATA_CLASS_INIT(PeerPoolMgr); #if USE_OPENSSL /// Gives Ssl::PeerConnector access to Answer in the PeerPoolMgr callback dialer. -class MyAnswerDialer: public UnaryMemFunT, +class MyAnswerDialer: public UnaryMemFunT, public Ssl::PeerConnector::CbDialer { public: MyAnswerDialer(const JobPointer &aJob, Method aMethod): - UnaryMemFunT(aJob, aMethod, Ssl::PeerConnectorAnswer()) {} + UnaryMemFunT(aJob, aMethod, Security::EncryptorAnswer()) {} /* Ssl::PeerConnector::CbDialer API */ - virtual Ssl::PeerConnectorAnswer &answer() { return arg1; } + virtual Security::EncryptorAnswer &answer() { return arg1; } }; #endif @@ -146,9 +148,8 @@ PeerPoolMgr::pushNewConnection(const Comm::ConnectionPointer &conn) // push() will trigger a checkpoint() } -#if USE_OPENSSL void -PeerPoolMgr::handleSecuredPeer(Ssl::PeerConnectorAnswer &answer) +PeerPoolMgr::handleSecuredPeer(Security::EncryptorAnswer &answer) { Must(securer != NULL); securer = NULL; @@ -190,7 +191,6 @@ PeerPoolMgr::handleSecureClosure(const CommCloseCbParams ¶ms) // allow the closing connection to fully close before we check again Checkpoint(this, "conn closure while securing"); } -#endif void PeerPoolMgr::openNewConnection() diff --git a/src/PeerPoolMgr.h b/src/PeerPoolMgr.h index d199d8b113..5a6f0eb33d 100644 --- a/src/PeerPoolMgr.h +++ b/src/PeerPoolMgr.h @@ -11,18 +11,12 @@ #include "base/AsyncJob.h" #include "comm/forward.h" +#include "security/forward.h" class HttpRequest; class CachePeer; class CommConnectCbParams; -#if USE_OPENSSL -namespace Ssl -{ -class PeerConnectorAnswer; -} -#endif - /// Maintains an fixed-size "standby" PconnPool for a single CachePeer. class PeerPoolMgr: public AsyncJob { @@ -56,12 +50,13 @@ protected: /// Comm::ConnOpener calls this when done opening a connection for us void handleOpenedConnection(const CommConnectCbParams ¶ms); -#if USE_OPENSSL + /// Ssl::PeerConnector callback - void handleSecuredPeer(Ssl::PeerConnectorAnswer &answer); + void handleSecuredPeer(Security::EncryptorAnswer &answer); + /// called when the connection we are trying to secure is closed by a 3rd party void handleSecureClosure(const CommCloseCbParams ¶ms); -#endif + /// the final step in connection opening (and, optionally, securing) sequence void pushNewConnection(const Comm::ConnectionPointer &conn); diff --git a/src/security/EncryptorAnswer.cc b/src/security/EncryptorAnswer.cc new file mode 100644 index 0000000000..1220a6f47b --- /dev/null +++ b/src/security/EncryptorAnswer.cc @@ -0,0 +1,24 @@ +/* + * Copyright (C) 1996-2015 The Squid Software Foundation and contributors + * + * Squid software is distributed under GPLv2+ license and includes + * contributions from numerous individuals and organizations. + * Please see the COPYING and CONTRIBUTORS files for details. + */ + +#include "squid.h" +#include "comm/Connection.h" +#include "errorpage.h" +#include "security/EncryptorAnswer.h" + +Security::EncryptorAnswer::~EncryptorAnswer() +{ + delete error.get(); +} + +std::ostream & +Security::operator <<(std::ostream &os, const Security::EncryptorAnswer &answer) +{ + return os << answer.conn << ", " << answer.error; +} + diff --git a/src/security/EncryptorAnswer.h b/src/security/EncryptorAnswer.h new file mode 100644 index 0000000000..a943eccf34 --- /dev/null +++ b/src/security/EncryptorAnswer.h @@ -0,0 +1,37 @@ +/* + * Copyright (C) 1996-2015 The Squid Software Foundation and contributors + * + * Squid software is distributed under GPLv2+ license and includes + * contributions from numerous individuals and organizations. + * Please see the COPYING and CONTRIBUTORS files for details. + */ + +#ifndef SQUID_SECURITY_ENCRYPTORANSWER_H +#define SQUID_SECURITY_ENCRYPTORANSWER_H + +#include "base/CbcPointer.h" +#include "comm/forward.h" + +class ErrorState; + +namespace Security { + +/// Peer encrypted connection setup results (supplied via a callback). +/// The connection to peer was secured if and only if the error member is nil. +class EncryptorAnswer +{ +public: + ~EncryptorAnswer(); ///< deletes error if it is still set + Comm::ConnectionPointer conn; ///< peer connection (secured on success) + + /// answer recepients must clear the error member in order to keep its info + /// XXX: We should refcount ErrorState instead of cbdata-protecting it. + CbcPointer error; ///< problem details (nil on success) +}; + +std::ostream &operator <<(std::ostream &, const Security::EncryptorAnswer &); + +} // namepace Security + +#endif /* SQUID_SECURITY_ENCRYPTORANSWER_H */ + diff --git a/src/security/Makefile.am b/src/security/Makefile.am index 9292a5a5fd..b483e8a2fb 100644 --- a/src/security/Makefile.am +++ b/src/security/Makefile.am @@ -12,5 +12,8 @@ noinst_LTLIBRARIES = libsecurity.la libsecurity_la_SOURCES= \ Context.h \ + EncryptorAnswer.cc \ + EncryptorAnswer.h \ + forward.h \ PeerOptions.cc \ PeerOptions.h diff --git a/src/security/PeerOptions.h b/src/security/PeerOptions.h index 7100298573..c88732f512 100644 --- a/src/security/PeerOptions.h +++ b/src/security/PeerOptions.h @@ -11,7 +11,7 @@ #include "ConfigParser.h" #include "SBuf.h" -#include "security/Context.h" +#include "security/forward.h" namespace Security { diff --git a/src/security/forward.h b/src/security/forward.h new file mode 100644 index 0000000000..c686bcea7c --- /dev/null +++ b/src/security/forward.h @@ -0,0 +1,24 @@ +/* + * Copyright (C) 1996-2015 The Squid Software Foundation and contributors + * + * Squid software is distributed under GPLv2+ license and includes + * contributions from numerous individuals and organizations. + * Please see the COPYING and CONTRIBUTORS files for details. + */ + +#ifndef SQUID_SRC_SECURITY_FORWARD_H +#define SQUID_SRC_SECURITY_FORWARD_H + +#include "security/Context.h" + +/// Network/connection security abstraction layer +namespace Security +{ + +class EncryptorAnswer; +class PeerOptions; + +} // namespace Security + +#endif /* SQUID_SRC_SECURITY_FORWARD_H */ + diff --git a/src/ssl/PeerConnector.cc b/src/ssl/PeerConnector.cc index fb218be9ba..484acc7cf8 100644 --- a/src/ssl/PeerConnector.cc +++ b/src/ssl/PeerConnector.cc @@ -696,16 +696,3 @@ Ssl::PeerConnector::status() const return buf.content(); } -/* PeerConnectorAnswer */ - -Ssl::PeerConnectorAnswer::~PeerConnectorAnswer() -{ - delete error.get(); -} - -std::ostream & -Ssl::operator <<(std::ostream &os, const Ssl::PeerConnectorAnswer &answer) -{ - return os << answer.conn << ", " << answer.error; -} - diff --git a/src/ssl/PeerConnector.h b/src/ssl/PeerConnector.h index 005dde9ded..740e32e189 100644 --- a/src/ssl/PeerConnector.h +++ b/src/ssl/PeerConnector.h @@ -12,6 +12,7 @@ #include "acl/Acl.h" #include "base/AsyncCbdataCalls.h" #include "base/AsyncJob.h" +#include "security/EncryptorAnswer.h" #include "ssl/support.h" #include @@ -24,19 +25,6 @@ namespace Ssl class ErrorDetail; class CertValidationResponse; -/// PeerConnector results (supplied via a callback). -/// The connection to peer was secured if and only if the error member is nil. -class PeerConnectorAnswer -{ -public: - ~PeerConnectorAnswer(); ///< deletes error if it is still set - Comm::ConnectionPointer conn; ///< peer connection (secured on success) - - /// answer recepients must clear the error member in order to keep its info - /// XXX: We should refcount ErrorState instead of cbdata-protecting it. - CbcPointer error; ///< problem details (nil on success) -}; - /** \par * Connects Squid client-side to an SSL peer (cache_peer ... ssl). @@ -44,7 +32,7 @@ public: * Used by TunnelStateData, FwdState, and PeerPoolMgr to start talking to an * SSL peer. \par - * The caller receives a call back with PeerConnectorAnswer. If answer.error + * 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 * was not fully established. The error object is suitable for error response * generation. @@ -78,7 +66,7 @@ public: public: virtual ~CbDialer() {} /// gives PeerConnector access to the in-dialer answer - virtual PeerConnectorAnswer &answer() = 0; + virtual Security::EncryptorAnswer &answer() = 0; }; typedef RefCount HttpRequestPointer; @@ -175,8 +163,6 @@ private: bool splice; ///< Whether we are going to splice or not }; -std::ostream &operator <<(std::ostream &os, const Ssl::PeerConnectorAnswer &a); - } // namespace Ssl #endif /* SQUID_PEER_CONNECTOR_H */ diff --git a/src/tests/stub_libsecurity.cc b/src/tests/stub_libsecurity.cc index 4f70cf8cf0..ec7903251e 100644 --- a/src/tests/stub_libsecurity.cc +++ b/src/tests/stub_libsecurity.cc @@ -7,10 +7,15 @@ */ #include "squid.h" +#include "comm/Connection.h" #define STUB_API "security/libsecurity.la" #include "tests/STUB.h" +#include "security/EncryptorAnswer.h" +Security::EncryptorAnswer::~EncryptorAnswer() {} +std::ostream &Security::operator <<(std::ostream &os, const Security::EncryptorAnswer &) STUB_RETVAL(os) + #include "security/PeerOptions.h" Security::PeerOptions Security::ProxyOutgoingConfig; void Security::PeerOptions::parse(char const*) STUB diff --git a/src/tunnel.cc b/src/tunnel.cc index 9dfcb7dd8d..3fff90e481 100644 --- a/src/tunnel.cc +++ b/src/tunnel.cc @@ -39,6 +39,8 @@ #include "ssl/bio.h" #include "ssl/PeerConnector.h" #include "ssl/ServerBump.h" +#else +#include "security/EncryptorAnswer.h" #endif #include "tools.h" #if USE_DELAY_POOLS @@ -170,7 +172,7 @@ private: class MyAnswerDialer: public CallDialer, public Ssl::PeerConnector::CbDialer { public: - typedef void (TunnelStateData::*Method)(Ssl::PeerConnectorAnswer &); + typedef void (TunnelStateData::*Method)(Security::EncryptorAnswer &); MyAnswerDialer(Method method, TunnelStateData *tunnel): method_(method), tunnel_(tunnel), answer_() {} @@ -183,17 +185,18 @@ private: } /* Ssl::PeerConnector::CbDialer API */ - virtual Ssl::PeerConnectorAnswer &answer() { return answer_; } + virtual Security::EncryptorAnswer &answer() { return answer_; } private: Method method_; CbcPointer tunnel_; - Ssl::PeerConnectorAnswer answer_; + Security::EncryptorAnswer answer_; }; - - void connectedToPeer(Ssl::PeerConnectorAnswer &answer); #endif + /// callback handler after connection setup (including any encryption) + void connectedToPeer(Security::EncryptorAnswer &answer); + public: bool keepGoingAfterRead(size_t len, Comm::Flag errcode, int xerrno, Connection &from, Connection &to); void copy(size_t len, Connection &from, Connection &to, IOCB *); @@ -1012,29 +1015,26 @@ tunnelStart(ClientHttpRequest * http, int64_t * size_ptr, int *status_ptr, const void TunnelStateData::connectToPeer() { - const Comm::ConnectionPointer &srv = server.conn; - #if USE_OPENSSL - if (CachePeer *p = srv->getPeer()) { + if (CachePeer *p = server.conn->getPeer()) { if (p->secure.encryptTransport) { AsyncCall::Pointer callback = asyncCall(5,4, "TunnelStateData::ConnectedToPeer", MyAnswerDialer(&TunnelStateData::connectedToPeer, this)); Ssl::PeerConnector *connector = - new Ssl::PeerConnector(request, srv, client.conn, callback); + new Ssl::PeerConnector(request, server.conn, client.conn, callback); AsyncJob::Start(connector); // will call our callback return; } } #endif - tunnelRelayConnectRequest(srv, this); + Security::EncryptorAnswer nil; + connectedToPeer(nil); } -#if USE_OPENSSL -/// Ssl::PeerConnector callback void -TunnelStateData::connectedToPeer(Ssl::PeerConnectorAnswer &answer) +TunnelStateData::connectedToPeer(Security::EncryptorAnswer &answer) { if (ErrorState *error = answer.error.get()) { *status_ptr = error->httpStatus; @@ -1047,7 +1047,6 @@ TunnelStateData::connectedToPeer(Ssl::PeerConnectorAnswer &answer) tunnelRelayConnectRequest(server.conn, this); } -#endif static void tunnelRelayConnectRequest(const Comm::ConnectionPointer &srv, void *data)