I am also attaching the patch for squid-3.5.
The patch for trunk does not apply to 3.5
On 11/09/2015 06:25 PM, Christos Tsantilas wrote:
Patch applied to trunk as r14390.
On 11/06/2015 08:39 PM, Amos Jeffries wrote:
On 7/11/2015 7:17 a.m., Christos Tsantilas wrote:
Project description:
- Squid receives SSL Hello from the client (TCP connection A).
- Squid successfully negotiates an SSL connection with the origin
server (TCP connection B).
- Squid successfully negotiates an SSL connection with the client
(TCP connection A).
- Squid marks connection B as "idle" and waits an HTTP request from
connection A.
- The origin server continues talking to Squid (TCP connection B).
Squid detects a network read on an idle connection and closes TCP
connection B (and then the associated TCP connection A as well).
This patch:
- When squid detects a network read on server idle connection do an
SSL_read to:
a) see if application data received from server and abort in
this case
b) detect possible SSL error, or SSL shutdown message from server
c) or ignore if only SSL protocol related packets received.
This is a Measurement Factory project
in src/client_side.cc:
* Please use "TLS" in debugs messages instead of "SSL".
* Please use "Tls" instead of "Ssl" in new symbol names such as the
handleIdleClientPinned***Read() method being added.
* Missing whitespace; "if(!ssl)" should be "if (!ssl)"
* Please use nullptr instead of NULL on new and altered lines.
* Please debug output the full pinning.serverConnection instead of just
the FD on debugs with levels higher than 1 / IMPORTANT.
in src/client_side.h:
* Please use "\returns" instead if "Returns" in the doxygen comment.
Since those are all cosmetic I dont think it needs another audit.
+1. Please apply once the polishing edits are done.
Amos
Handshake Problem during Renegotiation
Here is what happens:
- Squid receives TLS Hello from the client (TCP connection A).
- Squid successfully negotiates an TLS connection with the origin server
(TCP connection B).
- Squid successfully negotiates an TLS connection with the client
(TCP connection A).
- Squid marks connection B as "idle" and waits an HTTP request from
connection A.
- The origin server continues talking to Squid (TCP connection B).
Squid detects a network read on an idle connection and closes TCP
connection B (and then the associated TCP connection A as well).
This patch:
- When squid detects a network read on server idle connection do an
SSL_read to:
a) see if application data received from server and abort in this case
b) detect possible TLS error, or TLS shutdown message from server
c) or ignore if only TLS protocol related packets received.
This is a Measurement Factory project
=== modified file 'src/client_side.cc'
--- src/client_side.cc 2015-09-17 05:40:34 +0000
+++ src/client_side.cc 2015-11-09 16:36:22 +0000
@@ -4976,57 +4976,103 @@
ConnStateData::startPinnedConnectionMonitoring()
{
if (pinning.readHandler != NULL)
return; // already monitoring
typedef CommCbMemFunT<ConnStateData, CommIoCbParams> Dialer;
pinning.readHandler = JobCallback(33, 3,
Dialer, this, ConnStateData::clientPinnedConnectionRead);
Comm::Read(pinning.serverConnection, pinning.readHandler);
}
void
ConnStateData::stopPinnedConnectionMonitoring()
{
if (pinning.readHandler != NULL) {
Comm::ReadCancel(pinning.serverConnection->fd, pinning.readHandler);
pinning.readHandler = NULL;
}
}
+#if USE_OPENSSL
+bool
+ConnStateData::handleIdleClientPinnedTlsRead()
+{
+ // A ready-for-reading connection means that the TLS server either closed
+ // the connection, sent us some unexpected HTTP data, or started TLS
+ // renegotiations. We should close the connection except for the last case.
+
+ Must(pinning.serverConnection != nullptr);
+ SSL *ssl = fd_table[pinning.serverConnection->fd].ssl;
+ if (!ssl)
+ return false;
+
+ char buf[1];
+ const int readResult = SSL_read(ssl, buf, sizeof(buf));
+
+ if (readResult > 0 || SSL_pending(ssl) > 0) {
+ debugs(83, 2, pinning.serverConnection << " TLS application data read");
+ return false;
+ }
+
+ switch(const int error = SSL_get_error(ssl, readResult)) {
+ case SSL_ERROR_WANT_WRITE:
+ debugs(83, DBG_IMPORTANT, pinning.serverConnection << " TLS SSL_ERROR_WANT_WRITE request for idle pinned connection");
+ // fall through to restart monitoring, for now
+ case SSL_ERROR_NONE:
+ case SSL_ERROR_WANT_READ:
+ startPinnedConnectionMonitoring();
+ return true;
+
+ default:
+ debugs(83, 2, pinning.serverConnection << " TLS error: " << error);
+ return false;
+ }
+
+ // not reached
+ return true;
+}
+#endif
+
/// Our read handler called by Comm when the server either closes an idle pinned connection or
/// perhaps unexpectedly sends something on that idle (from Squid p.o.v.) connection.
void
ConnStateData::clientPinnedConnectionRead(const CommIoCbParams &io)
{
pinning.readHandler = NULL; // Comm unregisters handlers before calling
if (io.flag == Comm::ERR_CLOSING)
return; // close handler will clean up
+ Must(pinning.serverConnection == io.conn);
+
+#if USE_OPENSSL
+ if (handleIdleClientPinnedTlsRead())
+ return;
+#endif
+
// We could use getConcurrentRequestCount(), but this may be faster.
const bool clientIsIdle = !getCurrentContext();
debugs(33, 3, "idle pinned " << pinning.serverConnection << " read " <<
io.size << (clientIsIdle ? " with idle client" : ""));
- assert(pinning.serverConnection == io.conn);
pinning.serverConnection->close();
// If we are still sending data to the client, do not close now. When we are done sending,
// ClientSocketContext::keepaliveNextRequest() checks pinning.serverConnection and will close.
// However, if we are idle, then we must close to inform the idle client and minimize races.
if (clientIsIdle && clientConnection != NULL)
clientConnection->close();
}
const Comm::ConnectionPointer
ConnStateData::validatePinnedConnection(HttpRequest *request, const CachePeer *aPeer)
{
debugs(33, 7, HERE << pinning.serverConnection);
bool valid = true;
if (!Comm::IsConnOpen(pinning.serverConnection))
valid = false;
else if (pinning.auth && pinning.host && request && strcasecmp(pinning.host, request->GetHost()) != 0)
valid = false;
else if (request && pinning.port != request->port)
=== modified file 'src/client_side.h'
--- src/client_side.h 2015-08-08 04:04:45 +0000
+++ src/client_side.h 2015-11-09 16:36:22 +0000
@@ -399,40 +399,46 @@
/// stop parsing the request and create context for relaying error info
ClientSocketContext *abortRequestParsing(const char *const errUri);
/// generate a fake CONNECT request with the given payload
/// at the beginning of the client I/O buffer
void fakeAConnectRequest(const char *reason, const SBuf &payload);
/* Registered Runner API */
virtual void startShutdown();
virtual void endingShutdown();
protected:
void startDechunkingRequest();
void finishDechunkingRequest(bool withSuccess);
void abortChunkedRequestBody(const err_type error);
err_type handleChunkedRequestBody(size_t &putSize);
void startPinnedConnectionMonitoring();
void clientPinnedConnectionRead(const CommIoCbParams &io);
+#if USE_OPENSSL
+ /// Handles a ready-for-reading TLS squid-to-server connection that
+ /// we thought was idle.
+ /// Returns false if and only if the connection should be closed.
+ bool handleIdleClientPinnedTlsRead();
+#endif
/// parse input buffer prefix into a single transfer protocol request
/// return NULL to request more header bytes (after checking any limits)
/// use abortRequestParsing() to handle parsing errors w/o creating request
virtual ClientSocketContext *parseOneRequest(Http::ProtocolVersion &ver) = 0;
/// start processing a freshly parsed request
virtual void processParsedRequest(ClientSocketContext *context, const Http::ProtocolVersion &ver) = 0;
/// returning N allows a pipeline of 1+N requests (see pipeline_prefetch)
virtual int pipelinePrefetchMax() const;
/// timeout to use when waiting for the next request
virtual time_t idleTimeout() const = 0;
BodyPipe::Pointer bodyPipe; ///< set when we are reading request body
private:
int connFinishedWithConn(int size);
void clientAfterReadingRequests();
_______________________________________________
squid-dev mailing list
[email protected]
http://lists.squid-cache.org/listinfo/squid-dev
