--- /dev/null
+------------------------------------------------------------
+revno: 13225
+revision-id: squid3@treenet.co.nz-20150709032133-qg1patn5zngt4o4h
+parent: squid3@treenet.co.nz-20150501100500-3utkhrao1yrd8ig6
+author: Alex Rousskov <rousskov@measurement-factory.com>
+committer: Amos Jeffries <squid3@treenet.co.nz>
+branch nick: 3.4
+timestamp: Wed 2015-07-08 20:21:33 -0700
+message:
+ Do not blindly forward cache peer CONNECT responses.
+
+ Squid blindly forwards cache peer CONNECT responses to clients. This
+ may break things if the peer responds with something like HTTP 403
+ (Forbidden) and keeps the connection with Squid open:
+ - The client application issues a CONNECT request.
+ - Squid forwards this request to a cache peer.
+ - Cache peer correctly responds back with a "403 Forbidden".
+ - Squid does not parse cache peer response and
+ just forwards it as if it was a Squid response to the client.
+ - The TCP connections are not closed.
+
+ At this stage, Squid is unaware that the CONNECT request has failed. All
+ subsequent requests on the user agent TCP connection are treated as
+ tunnelled traffic. Squid is forwarding these requests to the peer on the
+ TCP connection previously used for the 403-ed CONNECT request, without
+ proper processing. The additional headers which should have been applied
+ by Squid to these requests are not applied, and the requests are being
+ forwarded to the cache peer even though the Squid configuration may
+ state that these requests must go directly to the origin server.
+
+ This fixes Squid to parse cache peer responses, and if an error response
+ found, respond with "502 Bad Gateway" to the client and close the
+ connections.
+------------------------------------------------------------
+# Bazaar merge directive format 2 (Bazaar 0.90)
+# revision_id: squid3@treenet.co.nz-20150709032133-qg1patn5zngt4o4h
+# target_branch: http://bzr.squid-cache.org/bzr/squid3/3.4
+# testament_sha1: 6cbce093f30c8a09173eb610eaa423c7c305ff23
+# timestamp: 2015-07-09 03:40:35 +0000
+# source_branch: http://bzr.squid-cache.org/bzr/squid3/3.4
+# base_revision_id: squid3@treenet.co.nz-20150501100500-\
+# 3utkhrao1yrd8ig6
+#
+# Begin patch
+=== modified file 'src/tunnel.cc'
+--- src/tunnel.cc 2014-04-26 10:58:22 +0000
++++ src/tunnel.cc 2015-07-09 03:21:33 +0000
+@@ -122,6 +122,10 @@
+ (request->flags.interceptTproxy || request->flags.intercepted));
+ }
+
++ /// Sends "502 Bad Gateway" error response to the client,
++ /// if it is waiting for Squid CONNECT response, closing connections.
++ void informUserOfPeerError(const char *errMsg);
++
+ class Connection
+ {
+
+@@ -139,13 +143,14 @@
+
+ void error(int const xerrno);
+ int debugLevelForError(int const xerrno) const;
+- /// handles a non-I/O error associated with this Connection
+- void logicError(const char *errMsg);
+ void closeIfOpen();
+ void dataSent (size_t amount);
++ /// writes 'b' buffer, setting the 'writer' member to 'callback'.
++ void write(const char *b, int size, AsyncCall::Pointer &callback, FREE * free_func);
+ int len;
+ char *buf;
+ int64_t *size_ptr; /* pointer to size in an ConnStateData for logging */
++ AsyncCall::Pointer writer; ///< pending Comm::Write callback
+
+ Comm::ConnectionPointer conn; ///< The currently connected connection.
+
+@@ -195,13 +200,14 @@
+ TunnelStateData *tunnelState = (TunnelStateData *)params.data;
+ debugs(26, 3, HERE << tunnelState->server.conn);
+ tunnelState->server.conn = NULL;
++ tunnelState->server.writer = NULL;
+
+ if (tunnelState->noConnections()) {
+ delete tunnelState;
+ return;
+ }
+
+- if (!tunnelState->server.len) {
++ if (!tunnelState->client.writer) {
+ tunnelState->client.conn->close();
+ return;
+ }
+@@ -213,13 +219,14 @@
+ TunnelStateData *tunnelState = (TunnelStateData *)params.data;
+ debugs(26, 3, HERE << tunnelState->client.conn);
+ tunnelState->client.conn = NULL;
++ tunnelState->client.writer = NULL;
+
+ if (tunnelState->noConnections()) {
+ delete tunnelState;
+ return;
+ }
+
+- if (!tunnelState->client.len) {
++ if (!tunnelState->server.writer) {
+ tunnelState->server.conn->close();
+ return;
+ }
+@@ -343,6 +350,23 @@
+ handleConnectResponse(len);
+ }
+
++void
++TunnelStateData::informUserOfPeerError(const char *errMsg)
++{
++ server.len = 0;
++ if (!clientExpectsConnectResponse()) {
++ // closing the connection is the best we can do here
++ debugs(50, 3, server.conn << " closing on error: " << errMsg);
++ server.conn->close();
++ return;
++ }
++ ErrorState *err = new ErrorState(ERR_CONNECT_FAIL, Http::scBadGateway, request.getRaw());
++ err->callback = tunnelErrorComplete;
++ err->callback_data = this;
++ *status_ptr = Http::scBadGateway;
++ errorSend(http->getConn()->clientConnection, err);
++}
++
+ /* Read from client side and queue it for writing to the server */
+ void
+ TunnelStateData::ReadConnectResponseDone(const Comm::ConnectionPointer &, char *buf, size_t len, comm_err_t errcode, int xerrno, void *data)
+@@ -374,7 +398,7 @@
+ const bool parsed = rep.parse(connectRespBuf, eof, &parseErr);
+ if (!parsed) {
+ if (parseErr > 0) { // unrecoverable parsing error
+- server.logicError("malformed CONNECT response from peer");
++ informUserOfPeerError("malformed CONNECT response from peer");
+ return;
+ }
+
+@@ -383,7 +407,7 @@
+ assert(!parseErr);
+
+ if (!connectRespBuf->hasSpace()) {
+- server.logicError("huge CONNECT response from peer");
++ informUserOfPeerError("huge CONNECT response from peer");
+ return;
+ }
+
+@@ -397,7 +421,8 @@
+
+ // bail if we did not get an HTTP 200 (Connection Established) response
+ if (rep.sline.status() != Http::scOkay) {
+- server.logicError("unsupported CONNECT response status code");
++ // if we ever decide to reuse the peer connection, we must extract the error response first
++ informUserOfPeerError("unsupported CONNECT response status code");
+ return;
+ }
+
+@@ -416,13 +441,6 @@
+ }
+
+ void
+-TunnelStateData::Connection::logicError(const char *errMsg)
+-{
+- debugs(50, 3, conn << " closing on error: " << errMsg);
+- conn->close();
+-}
+-
+-void
+ TunnelStateData::Connection::error(int const xerrno)
+ {
+ /* XXX fixme xstrerror and xerrno... */
+@@ -517,7 +535,7 @@
+ debugs(26, 3, HERE << "Schedule Write");
+ AsyncCall::Pointer call = commCbCall(5,5, "TunnelBlindCopyWriteHandler",
+ CommIoCbPtrFun(completion, this));
+- Comm::Write(to.conn, from.buf, len, call, NULL);
++ to.write(from.buf, len, call, NULL);
+ }
+
+ /* Writes data from the client buffer to the server side */
+@@ -526,6 +544,7 @@
+ {
+ TunnelStateData *tunnelState = (TunnelStateData *)data;
+ assert (cbdataReferenceValid (tunnelState));
++ tunnelState->server.writer = NULL;
+
+ tunnelState->writeServerDone(buf, len, flag, xerrno);
+ }
+@@ -575,6 +594,7 @@
+ {
+ TunnelStateData *tunnelState = (TunnelStateData *)data;
+ assert (cbdataReferenceValid (tunnelState));
++ tunnelState->client.writer = NULL;
+
+ tunnelState->writeClientDone(buf, len, flag, xerrno);
+ }
+@@ -592,7 +612,14 @@
+ }
+
+ void
+-TunnelStateData::writeClientDone(char *buf, size_t len, comm_err_t flag, int xerrno)
++TunnelStateData::Connection::write(const char *b, int size, AsyncCall::Pointer &callback, FREE * free_func)
++{
++ writer = callback;
++ Comm::Write(conn, b, size, callback, free_func);
++}
++
++void
++TunnelStateData::writeClientDone(char *, size_t len, comm_err_t flag, int xerrno)
+ {
+ debugs(26, 3, HERE << client.conn << ", " << len << " bytes written, flag=" << flag);
+
+@@ -712,6 +739,7 @@
+ {
+ TunnelStateData *tunnelState = (TunnelStateData *)data;
+ debugs(26, 3, HERE << conn << ", flag=" << flag);
++ tunnelState->client.writer = NULL;
+
+ if (flag != COMM_OK) {
+ *tunnelState->status_ptr = Http::scInternalServerError;
+@@ -728,6 +756,7 @@
+ {
+ TunnelStateData *tunnelState = (TunnelStateData *)data;
+ debugs(26, 3, conn << ", flag=" << flag);
++ tunnelState->server.writer = NULL;
+ assert(tunnelState->waitingForConnectRequest());
+
+ if (flag != COMM_OK) {
+@@ -768,7 +797,7 @@
+ else {
+ AsyncCall::Pointer call = commCbCall(5,5, "tunnelConnectedWriteDone",
+ CommIoCbPtrFun(tunnelConnectedWriteDone, tunnelState));
+- Comm::Write(tunnelState->client.conn, conn_established, strlen(conn_established), call, NULL);
++ tunnelState->client.write(conn_established, strlen(conn_established), call, NULL);
+ }
+ }
+
+@@ -955,29 +984,20 @@
+ debugs(11, 2, "Tunnel Server REQUEST: " << tunnelState->server.conn << ":\n----------\n" <<
+ Raw("tunnelRelayConnectRequest", mb.content(), mb.contentSize()) << "\n----------");
+
+- if (tunnelState->clientExpectsConnectResponse()) {
+- // hack: blindly tunnel peer response (to our CONNECT request) to the client as ours.
+- AsyncCall::Pointer writeCall = commCbCall(5,5, "tunnelConnectedWriteDone",
+- CommIoCbPtrFun(tunnelConnectedWriteDone, tunnelState));
+- Comm::Write(srv, &mb, writeCall);
+- } else {
+- // we have to eat the connect response from the peer (so that the client
+- // does not see it) and only then start shoveling data to the client
+- AsyncCall::Pointer writeCall = commCbCall(5,5, "tunnelConnectReqWriteDone",
+- CommIoCbPtrFun(tunnelConnectReqWriteDone,
+- tunnelState));
+- Comm::Write(srv, &mb, writeCall);
+- tunnelState->connectReqWriting = true;
+-
+- tunnelState->connectRespBuf = new MemBuf;
+- // SQUID_TCP_SO_RCVBUF: we should not accumulate more than regular I/O buffer
+- // can hold since any CONNECT response leftovers have to fit into server.buf.
+- // 2*SQUID_TCP_SO_RCVBUF: HttpMsg::parse() zero-terminates, which uses space.
+- tunnelState->connectRespBuf->init(SQUID_TCP_SO_RCVBUF, 2*SQUID_TCP_SO_RCVBUF);
+- tunnelState->readConnectResponse();
+-
+- assert(tunnelState->waitingForConnectExchange());
+- }
++ AsyncCall::Pointer writeCall = commCbCall(5,5, "tunnelConnectReqWriteDone",
++ CommIoCbPtrFun(tunnelConnectReqWriteDone, tunnelState));
++
++ tunnelState->server.write(mb.buf, mb.size, writeCall, mb.freeFunc());
++ tunnelState->connectReqWriting = true;
++
++ tunnelState->connectRespBuf = new MemBuf;
++ // SQUID_TCP_SO_RCVBUF: we should not accumulate more than regular I/O buffer
++ // can hold since any CONNECT response leftovers have to fit into server.buf.
++ // 2*SQUID_TCP_SO_RCVBUF: HttpMsg::parse() zero-terminates, which uses space.
++ tunnelState->connectRespBuf->init(SQUID_TCP_SO_RCVBUF, 2*SQUID_TCP_SO_RCVBUF);
++ tunnelState->readConnectResponse();
++
++ assert(tunnelState->waitingForConnectExchange());
+
+ AsyncCall::Pointer timeoutCall = commCbCall(5, 4, "tunnelTimeout",
+ CommTimeoutCbPtrFun(tunnelTimeout, tunnelState));
+
projects / people / pmueller / ipfire-2.x.git / commitdiff
