Hi all,
Currently FwdState::negotiateSSL() operates on a TCP connection without
a timeout. If, for example, the server never responds to Squid SSL
Hello, the connection getstuck forever. This happens in real world when,
for example, a client is trying to establish an SSL connection through
bumping Squid to an HTTP server that does not speak SSL and does not
detect initial request garbage (from HTTP point of view)
Moreover, if the client closes the connection while Squid is fruitlessly
waiting for server SSL negotiation, the client connection will get into
the CLOSE_WAIT state with a 1 day client_lifetime timeout. This patch
does not address that CLOSE_WAIT problem directly.
This patch adds an SSL negotiation timeout for the server SSL connection
and try to not exceed forword_timeout or peer_timeout while connecting
to an SSL server.
Some notes:
- In this patch still the timeouts used for Ssl::PeerConnector are not
accurate, they may be 5 secs more then the forward timeout or 1 second
more than peer_connect timeout, but I think are enough reasonable.
- Please check and comment the new
Comm::Connection::startTime()/::noteStart() mechanism.
Now the Comm::Connection::startTime_ computed in Comm::Connection
constructor and resets in Comm::ConnOpener::start() and
Comm::TcpAcceptor::start()
This is a Measurement Factory project.
SSL Server connect I/O timeout
FwdState::negotiateSSL() operates on a TCP connection without a timeout. If,
for example, the server never responds to Squid SSL Hello, the connection get
stuck forever. This happens in real world when, for example, a client is trying
to establish an SSL connection through bumping Squid to an HTTP server that
does not speak SSL and does not detect initial request garbage (from HTTP point
of view)
This patch adds support for timeout to SSL negotiation procedure and sets this
timeout so that it does not exceed peer_connect or forward_timeout.
This is a Measurement Factory project
=== modified file 'src/FwdState.cc'
--- src/FwdState.cc 2014-06-24 22:52:53 +0000
+++ src/FwdState.cc 2014-06-27 13:56:41 +0000
@@ -692,42 +692,44 @@
serverConn = conn;
flags.connected_okay = true;
debugs(17, 3, HERE << serverConnection() << ": '" << entry->url() << "'" );
comm_add_close_handler(serverConnection()->fd, fwdServerClosedWrapper, this);
if (serverConnection()->getPeer())
peerConnectSucceded(serverConnection()->getPeer());
#if USE_OPENSSL
if (!request->flags.pinned) {
if ((serverConnection()->getPeer() && serverConnection()->getPeer()->use_ssl) ||
(!serverConnection()->getPeer() && request->url.getScheme() == AnyP::PROTO_HTTPS) ||
request->flags.sslPeek) {
HttpRequest::Pointer requestPointer = request;
AsyncCall::Pointer callback = asyncCall(17,4,
"FwdState::ConnectedToPeer",
FwdStatePeerAnswerDialer(&FwdState::connectedToPeer, this));
+ // Use positive timeout when less than one second is left.
+ const time_t sslNegotiationTimeout = max(static_cast<time_t>(1), timeLeft());
Ssl::PeerConnector *connector =
- new Ssl::PeerConnector(requestPointer, serverConnection(), callback);
+ new Ssl::PeerConnector(requestPointer, serverConnection(), callback, sslNegotiationTimeout);
AsyncJob::Start(connector); // will call our callback
return;
}
}
#endif
dispatch();
}
#if USE_OPENSSL
void
FwdState::connectedToPeer(Ssl::PeerConnectorAnswer &answer)
{
if (ErrorState *error = answer.error.get()) {
fail(error);
answer.error.clear(); // preserve error for errorSendComplete()
self = NULL;
return;
}
@@ -740,71 +742,77 @@
{
debugs(17, 2, "fwdConnectTimeout: FD " << fd << ": '" << entry->url() << "'" );
assert(serverDestinations[0] != NULL);
assert(fd == serverDestinations[0]->fd);
if (entry->isEmpty()) {
ErrorState *anErr = new ErrorState(ERR_CONNECT_FAIL, Http::scGatewayTimeout, request);
anErr->xerrno = ETIMEDOUT;
fail(anErr);
/* This marks the peer DOWN ... */
if (serverDestinations[0]->getPeer())
peerConnectFailed(serverDestinations[0]->getPeer());
}
if (Comm::IsConnOpen(serverDestinations[0])) {
serverDestinations[0]->close();
}
}
-/**
- * Called after Forwarding path selection (via peer select) has taken place.
- * And whenever forwarding needs to attempt a new connection (routing failover)
- * We have a vector of possible localIP->remoteIP paths now ready to start being connected.
- */
-void
-FwdState::connectStart()
+time_t
+FwdState::timeLeft() const
{
- assert(serverDestinations.size() > 0);
-
- debugs(17, 3, "fwdConnectStart: " << entry->url());
-
- if (!request->hier.first_conn_start.tv_sec) // first attempt
- request->hier.first_conn_start = current_time;
-
/* connection timeout */
int ctimeout;
if (serverDestinations[0]->getPeer()) {
ctimeout = serverDestinations[0]->getPeer()->connect_timeout > 0 ?
serverDestinations[0]->getPeer()->connect_timeout : Config.Timeout.peer_connect;
} else {
ctimeout = Config.Timeout.connect;
}
/* calculate total forwarding timeout ??? */
int ftimeout = Config.Timeout.forward - (squid_curtime - start_t);
if (ftimeout < 0)
ftimeout = 5;
if (ftimeout < ctimeout)
- ctimeout = ftimeout;
+ return (time_t)ftimeout;
+ else
+ return (time_t)ctimeout;
+}
+
+/**
+ * Called after forwarding path selection (via peer select) has taken place
+ * and whenever forwarding needs to attempt a new connection (routing failover).
+ * We have a vector of possible localIP->remoteIP paths now ready to start being connected.
+ */
+void
+FwdState::connectStart()
+{
+ assert(serverDestinations.size() > 0);
+
+ debugs(17, 3, "fwdConnectStart: " << entry->url());
+
+ if (!request->hier.first_conn_start.tv_sec) // first attempt
+ request->hier.first_conn_start = current_time;
if (serverDestinations[0]->getPeer() && request->flags.sslBumped) {
debugs(50, 4, "fwdConnectStart: Ssl bumped connections through parent proxy are not allowed");
ErrorState *anErr = new ErrorState(ERR_CANNOT_FORWARD, Http::scServiceUnavailable, request);
fail(anErr);
self = NULL; // refcounted
return;
}
request->flags.pinned = false; // XXX: what if the ConnStateData set this to flag existing credentials?
// XXX: answer: the peer selection *should* catch it and give us only the pinned peer. so we reverse the =0 step below.
// XXX: also, logs will now lie if pinning is broken and leads to an error message.
if (serverDestinations[0]->peerType == PINNED) {
ConnStateData *pinned_connection = request->pinnedConnection();
debugs(17,7, "pinned peer connection: " << pinned_connection);
// pinned_connection may become nil after a pconn race
if (pinned_connection)
serverConn = pinned_connection->validatePinnedConnection(request, serverDestinations[0]->getPeer());
else
serverConn = NULL;
@@ -866,41 +874,41 @@
}
#endif
dispatch();
return;
}
// We will try to open a new connection, possibly to the same destination.
// We reset serverDestinations[0] in case we are using it again because
// ConnOpener modifies its destination argument.
serverDestinations[0]->local.port(0);
serverConn = NULL;
#if URL_CHECKSUM_DEBUG
entry->mem_obj->checkUrlChecksum();
#endif
GetMarkingsToServer(request, *serverDestinations[0]);
calls.connector = commCbCall(17,3, "fwdConnectDoneWrapper", CommConnectCbPtrFun(fwdConnectDoneWrapper, this));
- Comm::ConnOpener *cs = new Comm::ConnOpener(serverDestinations[0], calls.connector, ctimeout);
+ Comm::ConnOpener *cs = new Comm::ConnOpener(serverDestinations[0], calls.connector, timeLeft());
if (host)
cs->setHost(host);
AsyncJob::Start(cs);
}
void
FwdState::dispatch()
{
debugs(17, 3, clientConn << ": Fetching " << request->method << ' ' << entry->url());
/*
* Assert that server_fd is set. This is to guarantee that fwdState
* is attached to something and will be deallocated when server_fd
* is closed.
*/
assert(Comm::IsConnOpen(serverConn));
fd_note(serverConnection()->fd, entry->url());
fd_table[serverConnection()->fd].noteUse();
=== modified file 'src/FwdState.h'
--- src/FwdState.h 2014-06-24 22:52:53 +0000
+++ src/FwdState.h 2014-06-27 13:56:21 +0000
@@ -58,40 +58,41 @@
static void Start(const Comm::ConnectionPointer &client, StoreEntry *, HttpRequest *, const AccessLogEntryPointer &alp);
/// Same as Start() but no master xaction info (AccessLogEntry) available.
static void fwdStart(const Comm::ConnectionPointer &client, StoreEntry *, HttpRequest *);
/// This is the real beginning of server connection. Call it whenever
/// the forwarding server destination has changed and a new one needs to be opened.
/// Produces the cannot-forward error on fail if no better error exists.
void startConnectionOrFail();
void fail(ErrorState *err);
void unregister(Comm::ConnectionPointer &conn);
void unregister(int fd);
void complete();
void handleUnregisteredServerEnd();
int reforward();
bool reforwardableStatus(const Http::StatusCode s) const;
void serverClosed(int fd);
void connectStart();
void connectDone(const Comm::ConnectionPointer & conn, Comm::Flag status, int xerrno);
void connectTimeout(int fd);
+ time_t timeLeft() const; ///< the time left before the forwarding timeout expired
bool checkRetry();
bool checkRetriable();
void dispatch();
/// Pops a connection from connection pool if available. If not
/// checks the peer stand-by connection pool for available connection.
Comm::ConnectionPointer pconnPop(const Comm::ConnectionPointer &dest, const char *domain);
void pconnPush(Comm::ConnectionPointer & conn, const char *domain);
bool dontRetry() { return flags.dont_retry; }
void dontRetry(bool val) { flags.dont_retry = val; }
/** return a ConnectionPointer to the current server connection (may or may not be open) */
Comm::ConnectionPointer const & serverConnection() const { return serverConn; };
private:
// hidden for safer management of self; use static fwdStart
FwdState(const Comm::ConnectionPointer &client, StoreEntry *, HttpRequest *, const AccessLogEntryPointer &alp);
void start(Pointer aSelf);
=== modified file 'src/PeerPoolMgr.cc'
--- src/PeerPoolMgr.cc 2014-06-05 14:57:58 +0000
+++ src/PeerPoolMgr.cc 2014-06-27 14:09:00 +0000
@@ -1,35 +1,36 @@
#include "squid.h"
#include "base/AsyncJobCalls.h"
#include "base/RunnersRegistry.h"
#include "CachePeer.h"
#include "comm/Connection.h"
#include "comm/ConnOpener.h"
#include "Debug.h"
#include "fd.h"
#include "FwdState.h"
#include "globals.h"
#include "HttpRequest.h"
#include "neighbors.h"
#include "pconn.h"
#include "PeerPoolMgr.h"
#include "SquidConfig.h"
+#include "SquidTime.h"
#if USE_OPENSSL
#include "ssl/PeerConnector.h"
#endif
CBDATA_CLASS_INIT(PeerPoolMgr);
#if USE_OPENSSL
/// Gives Ssl::PeerConnector access to Answer in the PeerPoolMgr callback dialer.
class MyAnswerDialer: public UnaryMemFunT<PeerPoolMgr, Ssl::PeerConnectorAnswer, Ssl::PeerConnectorAnswer&>,
public Ssl::PeerConnector::CbDialer
{
public:
MyAnswerDialer(const JobPointer &aJob, Method aMethod):
UnaryMemFunT<PeerPoolMgr, Ssl::PeerConnectorAnswer, Ssl::PeerConnectorAnswer&>(aJob, aMethod, Ssl::PeerConnectorAnswer()) {}
/* Ssl::PeerConnector::CbDialer API */
virtual Ssl::PeerConnectorAnswer &answer() { return arg1; }
};
#endif
@@ -95,42 +96,48 @@
/* it might have been a timeout with a partially open link */
if (params.conn != NULL)
params.conn->close();
peerConnectFailed(peer);
checkpoint("conn opening failure"); // may retry
return;
}
Must(params.conn != NULL);
#if USE_OPENSSL
// Handle SSL peers.
if (peer->use_ssl) {
typedef CommCbMemFunT<PeerPoolMgr, CommCloseCbParams> CloserDialer;
closer = JobCallback(48, 3, CloserDialer, this,
PeerPoolMgr::handleSecureClosure);
comm_add_close_handler(params.conn->fd, closer);
securer = asyncCall(48, 4, "PeerPoolMgr::handleSecuredPeer",
MyAnswerDialer(this, &PeerPoolMgr::handleSecuredPeer));
+
+ const int peerTimeout = peer->connect_timeout > 0 ?
+ peer->connect_timeout : Config.Timeout.peer_connect;
+ const int timeUsed = squid_curtime - params.conn->startTime();
+ // Use positive timeout when less than one second is left for conn.
+ const int timeLeft = max(1, (peerTimeout - timeUsed));
Ssl::PeerConnector *connector =
- new Ssl::PeerConnector(request, params.conn, securer);
+ new Ssl::PeerConnector(request, params.conn, securer, timeLeft);
AsyncJob::Start(connector); // will call our callback
return;
}
#endif
pushNewConnection(params.conn);
}
void
PeerPoolMgr::pushNewConnection(const Comm::ConnectionPointer &conn)
{
Must(validPeer());
Must(Comm::IsConnOpen(conn));
peer->standby.pool->push(conn, NULL /* domain */);
// push() will trigger a checkpoint()
}
#if USE_OPENSSL
void
PeerPoolMgr::handleSecuredPeer(Ssl::PeerConnectorAnswer &answer)
=== modified file 'src/comm/ConnOpener.cc'
--- src/comm/ConnOpener.cc 2014-06-11 10:25:38 +0000
+++ src/comm/ConnOpener.cc 2014-06-27 14:32:42 +0000
@@ -210,40 +210,41 @@
{
Must(conn_ != NULL);
Must(temporaryFd_ >= 0);
cleanFd();
conn_->fd = temporaryFd_;
temporaryFd_ = -1;
}
void
Comm::ConnOpener::start()
{
Must(conn_ != NULL);
/* outbound sockets have no need to be protocol agnostic. */
if (!(Ip::EnableIpv6&IPV6_SPECIAL_V4MAPPING) && conn_->remote.isIPv4()) {
conn_->local.setIPv4();
}
+ conn_->noteStart();
if (createFd())
doConnect();
}
/// called at the end of Comm::ConnOpener::DelayedConnectRetry event
void
Comm::ConnOpener::restart()
{
debugs(5, 5, conn_ << " restarting after sleep");
calls_.sleep_ = false;
if (createFd())
doConnect();
}
/// Create a socket for the future connection or return false.
/// If false is returned, done() is guaranteed to return true and end the job.
bool
Comm::ConnOpener::createFd()
{
=== modified file 'src/comm/Connection.cc'
--- src/comm/Connection.cc 2014-04-30 10:50:09 +0000
+++ src/comm/Connection.cc 2014-06-27 14:39:25 +0000
@@ -5,68 +5,70 @@
#include "comm/Connection.h"
#include "fde.h"
#include "neighbors.h"
#include "SquidTime.h"
class CachePeer;
bool
Comm::IsConnOpen(const Comm::ConnectionPointer &conn)
{
return conn != NULL && conn->isOpen();
}
Comm::Connection::Connection() :
local(),
remote(),
peerType(HIER_NONE),
fd(-1),
tos(0),
nfmark(0),
flags(COMM_NONBLOCKING),
- peer_(NULL)
+ peer_(NULL),
+ startTime_(squid_curtime)
{
*rfc931 = 0; // quick init the head. the rest does not matter.
}
static int64_t lost_conn = 0;
Comm::Connection::~Connection()
{
if (fd >= 0) {
debugs(5, 4, "BUG #3329: Orphan Comm::Connection: " << *this);
debugs(5, 4, "NOTE: " << ++lost_conn << " Orphans since last started.");
close();
}
cbdataReferenceDone(peer_);
}
Comm::ConnectionPointer
Comm::Connection::copyDetails() const
{
ConnectionPointer c = new Comm::Connection;
c->local = local;
c->remote = remote;
c->peerType = peerType;
c->tos = tos;
c->nfmark = nfmark;
c->flags = flags;
+ c->startTime_ = startTime_;
// ensure FD is not open in the new copy.
c->fd = -1;
// ensure we have a cbdata reference to peer_ not a straight ptr copy.
c->peer_ = cbdataReference(getPeer());
return c;
}
void
Comm::Connection::close()
{
if (isOpen()) {
comm_close(fd);
fd = -1;
if (CachePeer *p=getPeer())
peerConnClosed(p);
}
}
=== modified file 'src/comm/Connection.h'
--- src/comm/Connection.h 2014-02-21 10:46:19 +0000
+++ src/comm/Connection.h 2014-06-27 14:29:46 +0000
@@ -30,40 +30,41 @@
* Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111, USA.
*
*
* Copyright (c) 2003, Robert Collins <robe...@squid-cache.org>
* Copyright (c) 2010, Amos Jeffries <amosjeffr...@squid-cache.org>
*/
#ifndef _SQUIDCONNECTIONDETAIL_H_
#define _SQUIDCONNECTIONDETAIL_H_
#include "comm/forward.h"
#include "defines.h"
#include "hier_code.h"
#include "ip/Address.h"
#include "MemPool.h"
#include "typedefs.h"
#if USE_SQUID_EUI
#include "eui/Eui48.h"
#include "eui/Eui64.h"
#endif
+#include "SquidTime.h"
#include <iosfwd>
#include <ostream>
class CachePeer;
namespace Comm
{
/* TODO: make these a struct of boolean flags members in the connection instead of a bitmap.
* we can't do that until all non-comm code uses Commm::Connection objects to create FD
* currently there is code still using comm_open() and comm_openex() synchronously!!
*/
#define COMM_UNSET 0x00
#define COMM_NONBLOCKING 0x01 // default flag.
#define COMM_NOCLOEXEC 0x02
#define COMM_REUSEADDR 0x04 // shared FD may be both accept()ing and read()ing
#define COMM_DOBIND 0x08 // requires a bind()
#define COMM_TRANSPARENT 0x10 // arrived via TPROXY
#define COMM_INTERCEPTION 0x20 // arrived via NAT
@@ -97,79 +98,86 @@
*/
ConnectionPointer copyDetails() const;
/** Close any open socket. */
void close();
/** determine whether this object describes an active connection or not. */
bool isOpen() const { return (fd >= 0); }
/** retrieve the CachePeer pointer for use.
* The caller is responsible for all CBDATA operations regarding the
* used of the pointer returned.
*/
CachePeer * getPeer() const;
/** alter the stored CachePeer pointer.
* Perform appropriate CBDATA operations for locking the CachePeer pointer
*/
void setPeer(CachePeer * p);
+ /** The time the connection started */
+ time_t startTime() const {return startTime_;}
+
+ void noteStart() {startTime_ = squid_curtime;}
private:
/** These objects may not be exactly duplicated. Use copyDetails() instead. */
Connection(const Connection &c);
/** These objects may not be exactly duplicated. Use copyDetails() instead. */
Connection & operator =(const Connection &c);
public:
/** Address/Port for the Squid end of a TCP link. */
Ip::Address local;
/** Address for the Remote end of a TCP link. */
Ip::Address remote;
/** Hierarchy code for this connection link */
hier_code peerType;
/** Socket used by this connection. Negative if not open. */
int fd;
/** Quality of Service TOS values currently sent on this connection */
tos_t tos;
/** Netfilter MARK values currently sent on this connection */
nfmark_t nfmark;
/** COMM flags set on this connection */
int flags;
char rfc931[USER_IDENT_SZ];
#if USE_SQUID_EUI
Eui::Eui48 remoteEui48;
Eui::Eui64 remoteEui64;
#endif
private:
/** cache_peer data object (if any) */
CachePeer *peer_;
+
+ /** The time the connection object was created */
+ time_t startTime_;
};
}; // namespace Comm
MEMPROXY_CLASS_INLINE(Comm::Connection);
// NP: Order and namespace here is very important.
// * The second define inlines the first.
// * Stream inheritance overloading is searched in the global scope first.
inline std::ostream &
operator << (std::ostream &os, const Comm::Connection &conn)
{
os << "local=" << conn.local << " remote=" << conn.remote;
if (conn.fd >= 0)
os << " FD " << conn.fd;
if (conn.flags != COMM_UNSET)
os << " flags=" << conn.flags;
#if USE_IDENT
if (*conn.rfc931)
=== modified file 'src/comm/TcpAcceptor.cc'
--- src/comm/TcpAcceptor.cc 2014-06-09 13:18:48 +0000
+++ src/comm/TcpAcceptor.cc 2014-06-27 14:33:21 +0000
@@ -76,40 +76,42 @@
unsubscribe("subscription change");
theCallSub = aSub;
}
void
Comm::TcpAcceptor::unsubscribe(const char *reason)
{
debugs(5, 5, HERE << status() << " AsyncCall Subscription " << theCallSub << " removed: " << reason);
theCallSub = NULL;
}
void
Comm::TcpAcceptor::start()
{
debugs(5, 5, HERE << status() << " AsyncCall Subscription: " << theCallSub);
Must(IsConnOpen(conn));
setListen();
+ conn->noteStart();
+
// if no error so far start accepting connections.
if (errcode == 0)
SetSelect(conn->fd, COMM_SELECT_READ, doAccept, this, 0);
}
bool
Comm::TcpAcceptor::doneAll() const
{
// stop when FD is closed
if (!IsConnOpen(conn)) {
return AsyncJob::doneAll();
}
// stop when handlers are gone
if (theCallSub == NULL) {
return AsyncJob::doneAll();
}
// open FD with handlers...keep accepting.
return false;
=== modified file 'src/ssl/PeerConnector.cc'
--- src/ssl/PeerConnector.cc 2014-05-07 14:40:05 +0000
+++ src/ssl/PeerConnector.cc 2014-06-27 14:36:45 +0000
@@ -11,45 +11,48 @@
#include "comm/Loops.h"
#include "errorpage.h"
#include "fde.h"
#include "globals.h"
#include "HttpRequest.h"
#include "neighbors.h"
#include "SquidConfig.h"
#include "ssl/cert_validate_message.h"
#include "ssl/Config.h"
#include "ssl/ErrorDetail.h"
#include "ssl/helper.h"
#include "ssl/PeerConnector.h"
#include "ssl/ServerBump.h"
#include "ssl/support.h"
CBDATA_NAMESPACED_CLASS_INIT(Ssl, PeerConnector);
Ssl::PeerConnector::PeerConnector(
HttpRequestPointer &aRequest,
const Comm::ConnectionPointer &aServerConn,
- AsyncCall::Pointer &aCallback):
+ AsyncCall::Pointer &aCallback,
+ const time_t timeout):
AsyncJob("Ssl::PeerConnector"),
request(aRequest),
serverConn(aServerConn),
- callback(aCallback)
+ callback(aCallback),
+ negotiationTimeout(timeout),
+ startTime(squid_curtime)
{
// if this throws, the caller's cb dialer is not our CbDialer
Must(dynamic_cast<CbDialer*>(callback->getDialer()));
}
Ssl::PeerConnector::~PeerConnector()
{
debugs(83, 5, "Peer connector " << this << " gone");
}
bool Ssl::PeerConnector::doneAll() const
{
return (!callback || callback->canceled()) && AsyncJob::doneAll();
}
/// Preps connection and SSL state. Calls negotiate().
void
Ssl::PeerConnector::start()
{
AsyncJob::start();
@@ -161,40 +164,54 @@
// check->fd(fd); XXX: need client FD here
SSL_set_ex_data(ssl, ssl_ex_index_cert_error_check, check);
}
}
// store peeked cert to check SQUID_X509_V_ERR_CERT_CHANGE
X509 *peeked_cert;
if (request->clientConnectionManager.valid() &&
request->clientConnectionManager->serverBump() &&
(peeked_cert = request->clientConnectionManager->serverBump()->serverCert.get())) {
CRYPTO_add(&(peeked_cert->references),1,CRYPTO_LOCK_X509);
SSL_set_ex_data(ssl, ssl_ex_index_ssl_peeked_cert, peeked_cert);
}
fd_table[fd].ssl = ssl;
fd_table[fd].read_method = &ssl_read_method;
fd_table[fd].write_method = &ssl_write_method;
}
void
+Ssl::PeerConnector::setReadTimeout()
+{
+ int timeToRead;
+ if (negotiationTimeout) {
+ const int timeUsed = squid_curtime - startTime;
+ const int timeLeft = max(0, static_cast<int>(negotiationTimeout - timeUsed));
+ timeToRead = min(static_cast<int>(::Config.Timeout.read), timeLeft);
+ } else
+ timeToRead = ::Config.Timeout.read;
+ AsyncCall::Pointer nil;
+ commSetConnTimeout(serverConnection(), timeToRead, nil);
+}
+
+void
Ssl::PeerConnector::negotiateSsl()
{
if (!Comm::IsConnOpen(serverConnection()) || fd_table[serverConnection()->fd].closing())
return;
const int fd = serverConnection()->fd;
SSL *ssl = fd_table[fd].ssl;
const int result = SSL_connect(ssl);
if (result <= 0) {
handleNegotiateError(result);
return; // we might be gone by now
}
if (request->clientConnectionManager.valid()) {
// remember the server certificate from the ErrorDetail object
if (Ssl::ServerBump *serverBump = request->clientConnectionManager->serverBump()) {
serverBump->serverCert.reset(SSL_get_peer_certificate(ssl));
// remember validation errors, if any
if (Ssl::CertErrors *errs = static_cast<Ssl::CertErrors *>(SSL_get_ex_data(ssl, ssl_ex_index_ssl_errors)))
@@ -369,40 +386,41 @@
CallJobHere(83, 7, pc, Ssl::PeerConnector, negotiateSsl);
}
void
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;
int ssl_error = SSL_get_error(ssl, ret);
#ifdef EPROTO
int sysErrNo = EPROTO;
#else
int sysErrNo = EACCES;
#endif
switch (ssl_error) {
case SSL_ERROR_WANT_READ:
+ setReadTimeout();
Comm::SetSelect(fd, COMM_SELECT_READ, &NegotiateSsl, this, 0);
return;
case SSL_ERROR_WANT_WRITE:
Comm::SetSelect(fd, COMM_SELECT_WRITE, &NegotiateSsl, this, 0);
return;
case SSL_ERROR_SSL:
case SSL_ERROR_SYSCALL:
ssl_lib_error = ERR_get_error();
// store/report errno when ssl_error is SSL_ERROR_SYSCALL, ssl_lib_error is 0, and ret is -1
if (ssl_error == SSL_ERROR_SYSCALL && ret == -1 && ssl_lib_error == 0)
sysErrNo = errno;
debugs(83, DBG_IMPORTANT, "Error negotiating SSL on FD " << fd <<
": " << ERR_error_string(ssl_lib_error, NULL) << " (" <<
ssl_error << "/" << ret << "/" << errno << ")");
break; // proceed to the general error handling code
=== modified file 'src/ssl/PeerConnector.h'
--- src/ssl/PeerConnector.h 2014-05-07 14:40:05 +0000
+++ src/ssl/PeerConnector.h 2014-06-27 14:36:26 +0000
@@ -54,90 +54,97 @@
/// answer recepients must clear the error member in order to keep its info
/// XXX: We should refcount ErrorState instead of cbdata-protecting it.
CbcPointer<ErrorState> error; ///< problem details (nil on success)
};
/**
\par
* Connects Squid client-side to an SSL peer (cache_peer ... ssl).
* Handles peer certificate validation.
* 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
* 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.
\par
* The caller must monitor the connection for closure because this
* job will not inform the caller about such events.
\par
- * 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: Neither tunnel.cc
- * nor forward.cc have a "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.
+ * PeerConnector class curently supports a form of SSL negotiation timeout,
+ * which accounted only when sets the read timeout from SSL peer.
+ * 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
+ * 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.
\par
* This job never closes the connection, even on errors. If a 3rd-party
* closes the connection, this job simply quits without informing the caller.
*/
class PeerConnector: virtual public AsyncJob
{
public:
/// Callback dialier API to allow PeerConnector to set the answer.
class CbDialer
{
public:
virtual ~CbDialer() {}
/// gives PeerConnector access to the in-dialer answer
virtual PeerConnectorAnswer &answer() = 0;
};
typedef RefCount<HttpRequest> HttpRequestPointer;
public:
PeerConnector(HttpRequestPointer &aRequest,
const Comm::ConnectionPointer &aServerConn,
- AsyncCall::Pointer &aCallback);
+ AsyncCall::Pointer &aCallback, const time_t timeout = 0);
virtual ~PeerConnector();
protected:
// AsyncJob API
virtual void start();
virtual bool doneAll() const;
virtual void swanSong();
virtual const char *status() const;
/// The comm_close callback handler.
void commCloseHandler(const CommCloseCbParams ¶ms);
/// Inform us that the connection is closed. Does the required clean-up.
void connectionClosed(const char *reason);
/// Sets up TCP socket-related notification callbacks if things go wrong.
/// If socket already closed return false, else install the comm_close
/// handler to monitor the socket.
bool prepareSocket();
+ /// Sets the read timeout to avoid getting stuck while reading from a
+ /// silent server
+ void setReadTimeout();
+
void initializeSsl(); ///< Initializes SSL state
/// Performs a single secure connection negotiation step.
/// It is called multiple times untill the negotiation finish or aborted.
void negotiateSsl();
/// Called when the SSL negotiation step aborted because data needs to
/// be transferred to/from SSL 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);
private:
PeerConnector(const PeerConnector &); // not implemented
PeerConnector &operator =(const PeerConnector &); // not implemented
/// mimics FwdState to minimize changes to FwdState::initiate/negotiateSsl
Comm::ConnectionPointer const &serverConnection() const { return serverConn; }
void bail(ErrorState *error); ///< Return an error to the PeerConnector caller
@@ -145,29 +152,31 @@
/// Callback the caller class, and pass the ready to communicate secure
/// connection or an error if PeerConnector failed.
void callBack();
/// Process response from cert validator helper
void sslCrtvdHandleReply(Ssl::CertValidationResponse const &);
/// Check SSL errors returned from cert validator against sslproxy_cert_error access list
Ssl::CertErrors *sslCrtvdCheckForErrors(Ssl::CertValidationResponse const &, Ssl::ErrorDetail *&);
/// Callback function called when squid receive message from cert validator helper
static void sslCrtvdHandleReplyWrapper(void *data, Ssl::CertValidationResponse const &);
/// A wrapper function for negotiateSsl for use with Comm::SetSelect
static void NegotiateSsl(int fd, void *data);
HttpRequestPointer request; ///< peer connection trigger or cause
Comm::ConnectionPointer serverConn; ///< TCP connection to the peer
AsyncCall::Pointer callback; ///< we call this with the results
AsyncCall::Pointer closeHandler; ///< we call this when the connection closed
+ time_t negotiationTimeout; ///< the ssl connection timeout to use
+ time_t startTime; ///< when the peer connector negotiation started
CBDATA_CLASS2(PeerConnector);
};
std::ostream &operator <<(std::ostream &os, const Ssl::PeerConnectorAnswer &a);
} // namespace Ssl
#endif /* SQUID_PEER_CONNECTOR_H */
