From: Alex Rousskov <rousskov@measurement-factory.com>
Date: Fri, 17 Nov 2017 16:30:09 +0000 (-0700)
Subject: Relay peer CONNECT error status line and headers to users (#80)
X-Git-Tag: SQUID_4_0_22~3
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=4d607337f620eef0f687b9b76c40b8a2a80a5596;p=thirdparty%2Fsquid.git

Relay peer CONNECT error status line and headers to users (#80)

Automated agents and human users (or their support staff!) often benefit
from knowing what went wrong. Dropping such details is a bad default.
For example, automation may rely on receiving the original status code.

Our CVE-2015-5400 fix (74f35ca) was too aggressive -- it hid all peer
errors behind a generic 502 (Bad Gateway) response. Pass-through peer
authentication errors were later (971003b) exposed again, but our CVE
fix intent was _not_ to hide _any_ peer errors in the first place! The
intent was to close the connection after delivering the error response.
Hiding peer errors was an (unfortunate) implementation choice.

It could be argued that some peer errors should not be relayed, but
since Squid successfully relayed all peer errors prior to 74f35ca and
continues to relay all non-CONNECT peer errors today, discriminating
peer errors is a separate (and possibly unnecessary) feature.

Ideally, Squid should mangle and relay the whole error message (instead
of sending small original headers). Squid should also relay 1xx control
messages while waiting for the final response. Unfortunately, doing so
properly, without reopening CVE-2015-5400 or duplicating a lot of
complex code, is a huge project. This small change fixes the most acute
manifestation of the "hiding errors from users" problem. The rest is a
long-term TODO.
---

diff --git a/src/tunnel.cc b/src/tunnel.cc
index ec023ac731..42ddfd29dc 100644
--- a/src/tunnel.cc
+++ b/src/tunnel.cc
@@ -517,17 +517,11 @@ TunnelStateData::handleConnectResponse(const size_t chunkSize)
     // CONNECT response was successfully parsed
     *status_ptr = rep.sline.status();
 
-    // we need to relay the 401/407 responses when login=PASS(THRU)
-    const CachePeer *peer = server.conn->getPeer();
-    const char *pwd = (peer ? peer->login : nullptr);
-    const bool relay = pwd && (strcmp(pwd, "PASS") == 0 || strcmp(pwd, "PASSTHRU") == 0) &&
-                       (*status_ptr == Http::scProxyAuthenticationRequired ||
-                        *status_ptr == Http::scUnauthorized);
-
     // bail if we did not get an HTTP 200 (Connection Established) response
     if (rep.sline.status() != Http::scOkay) {
         // if we ever decide to reuse the peer connection, we must extract the error response first
-        informUserOfPeerError("unsupported CONNECT response status code", (relay ? rep.hdr_sz : 0));
+        *status_ptr = rep.sline.status(); // we are relaying peer response
+        informUserOfPeerError("unsupported CONNECT response status code", rep.hdr_sz);
         return;
     }