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 +
+++ src/FwdState.cc 2014-06-27 13:56:41 +
@@ -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(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());
-
-
