From: Alex Rousskov <rousskov@measurement-factory.com>
Date: Wed, 17 Aug 2016 22:46:38 +0000 (-0600)
Subject: Do not access-log SslBump-faked CONNECTs with _ABORTED suffixes.
X-Git-Tag: SQUID_4_0_14~24
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=08a8a70bdd97e75c41acfce6d852d4934533b704;p=thirdparty%2Fsquid.git

Do not access-log SslBump-faked CONNECTs with _ABORTED suffixes.

All successful transactions, including fake CONNECTs, must be removed
from the pipeline (via a finished() call) to avoid _ABORTED suffix
added by Pipeline::terminateAll() which always calls noteIoError().

Also added a check that the remaining request in the pipeline is the
expected CONNECT. Old comments said as much but the code did not throw
if a different request was pipelined (and asserted if more than one
requests were). We now throw in all these problematic cases. I cannot
track all ConnStateData::getSslContextStart() callers deep enough to
be sure that only a single CONNECT can get through, but the change
reflects the expected behavior (and it may be useful to discover any
reality deviations from our expectations).
---

diff --git a/src/client_side.cc b/src/client_side.cc
index a84b0589dd..de14fe96ab 100644
--- a/src/client_side.cc
+++ b/src/client_side.cc
@@ -2981,10 +2981,14 @@ void ConnStateData::buildSslCertGenerationParams(Ssl::CertificateProperties &cer
 void
 ConnStateData::getSslContextStart()
 {
-    // XXX starting SSL with a pipeline of requests still waiting for non-SSL replies?
-    assert(pipeline.count() < 2); // the CONNECT is okay for now. Anything else is a bug.
-    pipeline.terminateAll(0);
-    /* careful: terminateAll(0) above frees request, host, etc. */
+    // If we are called, then CONNECT has succeeded. Finalize it.
+    if (auto xact = pipeline.front()) {
+        if (xact->http && xact->http->request && xact->http->request->method == Http::METHOD_CONNECT)
+            xact->finished();
+        // cannot proceed with encryption if requests wait for plain responses
+        Must(pipeline.empty());
+    }
+    /* careful: finished() above frees request, host, etc. */
 
     if (port->generateHostCertificates) {
         Ssl::CertificateProperties certProperties;