Merged master into SQUID-568-premature-serverconn-use

Conflicts:
    src/FwdState.h
    src/adaptation/icap/Xaction.cc
    src/security/PeerConnector.cc
    src/security/PeerConnector.h
    src/tests/stub_libsecurity.cc

diff --cc src/BodyPipe.cc
Simple merge

diff --cc src/CommCalls.cc
Simple merge

diff --cc src/Downloader.cc
Simple merge

diff --cc src/FwdState.cc
Simple merge

diff --cc src/FwdState.h
index 4c9a66d9de505f48c70dcc555da593fb331fc9aa,eda01fb674638078a67420179c9bf9aa16e85d00..b73ce239e3dfa6db61b8be1e18e224063d241faa
--- 1/src/FwdState.h
--- 2/src/FwdState.h
+++ b/src/FwdState.h
@@@ -15,7 -15,8 +15,7 @@@
  #include "clients/forward.h"
  #include "comm.h"
  #include "comm/Connection.h"
- #include "err_type.h"
 -#include "comm/ConnOpener.h"
+ #include "error/forward.h"
  #include "fde.h"
  #include "http/StatusCode.h"
  #include "ip/Address.h"
@@@ -37,16 -38,9 +37,8 @@@ class ResolvedPeers
  typedef RefCount<ResolvedPeers> ResolvedPeersPointer;
  
  class HappyConnOpener;
 -typedef CbcPointer<HappyConnOpener> HappyConnOpenerPointer;
  class HappyConnOpenerAnswer;
  
- #if USE_OPENSSL
- namespace Ssl
- {
- class ErrorDetail;
- class CertValidationResponse;
- };
- #endif
- 
  /// Sets initial TOS value and Netfilter for the future outgoing connection.
  /// Updates the given Connection object, not the future transport connection.
  void GetMarkingsToServer(HttpRequest * request, Comm::Connection &conn);

diff --cc src/HappyConnOpener.cc
Simple merge

diff --cc src/HappyConnOpener.h
Simple merge

diff --cc src/PeerPoolMgr.cc
Simple merge

diff --cc src/PeerPoolMgr.h
Simple merge

diff --cc src/ResolvedPeers.cc
Simple merge

diff --cc src/adaptation/icap/ModXact.cc
Simple merge

diff --cc src/adaptation/icap/ModXact.h
Simple merge

diff --cc src/adaptation/icap/OptXact.cc
Simple merge

diff --cc src/adaptation/icap/OptXact.h
Simple merge

diff --cc src/adaptation/icap/ServiceRep.cc
Simple merge

diff --cc src/adaptation/icap/ServiceRep.h
Simple merge

diff --cc src/adaptation/icap/Xaction.cc
index 0b84b7208ef03e718f94629a16551d6359380f88,e4e539ee4657394b737aa3457351a6a0bb677002..ce7a4dce069b015df6fe5968b11dd9404f173052
--- 1/src/adaptation/icap/Xaction.cc
--- 2/src/adaptation/icap/Xaction.cc
+++ b/src/adaptation/icap/Xaction.cc
@@@ -86,8 -89,15 +89,9 @@@ Adaptation::Icap::Xaction::Xaction(cons
      isRetriable(true),
      isRepeatable(true),
      ignoreLastWrite(false),
 -    stopReason(NULL),
 -    connector(NULL),
 -    reader(NULL),
 -    writer(NULL),
 -    closer(NULL),
+     waitingForDns(false),
      alep(new AccessLogEntry),
 -    al(*alep),
 -    cs(NULL)
 +    al(*alep)
  {
      debugs(93,3, typeName << " constructed, this=" << this <<
             " [icapx" << id << ']'); // we should not call virtual status() here
@@@ -196,26 -230,12 +205,12 @@@ Adaptation::Icap::Xaction::dnsLookupDon
  
      // TODO: service bypass status may differ from that of a transaction
      typedef CommCbMemFunT<Adaptation::Icap::Xaction, CommConnectCbParams> ConnectDialer;
 -    connector = JobCallback(93,3, ConnectDialer, this, Adaptation::Icap::Xaction::noteCommConnected);
 -    cs = new Comm::ConnOpener(connection, connector, TheConfig.connect_timeout(service().cfg().bypass));
 +    AsyncCall::Pointer callback = JobCallback(93, 3, ConnectDialer, this, Adaptation::Icap::Xaction::noteCommConnected);
 +    const auto cs = new Comm::ConnOpener(conn, callback, TheConfig.connect_timeout(service().cfg().bypass));
      cs->setHost(s.cfg().host.termedBuf());
 -    AsyncJob::Start(cs.get());
 +    transportWait.start(cs, callback);
  }
  
- /*
-  * This event handler is necessary to work around the no-rentry policy
-  * of Adaptation::Icap::Xaction::callStart()
-  */
- #if 0
- void
- Adaptation::Icap::Xaction::reusedConnection(void *data)
- {
-     debugs(93, 5, HERE << "reused connection");
-     Adaptation::Icap::Xaction *x = (Adaptation::Icap::Xaction*)data;
-     x->noteCommConnected(Comm::OK);
- }
- #endif
- 
  void Adaptation::Icap::Xaction::closeConnection()
  {
      if (haveConnection()) {
@@@ -367,12 -380,18 +363,14 @@@ void Adaptation::Icap::Xaction::noteCom
  // unexpected connection close while talking to the ICAP service
  void Adaptation::Icap::Xaction::noteCommClosed(const CommCloseCbParams &)
  {
 -    if (securer != NULL) {
 -        securer->cancel("Connection closed before SSL negotiation finished");
 -        securer = NULL;
 +    if (connection) {
 +        connection->noteClosure();
 +        connection = nullptr;
      }
      closer = NULL;
-     detailError(ERR_DETAIL_ICAP_XACT_CLOSE);
 -    handleCommClosed();
 -}
+ 
 -void Adaptation::Icap::Xaction::handleCommClosed()
 -{
+     static const auto d = MakeNamedErrorDetail("ICAP_XACT_CLOSE");
+     detailError(d);
      mustStop("ICAP service connection externally closed");
  }
  
@@@ -394,7 -413,8 +392,9 @@@ void Adaptation::Icap::Xaction::callEnd
  
  bool Adaptation::Icap::Xaction::doneAll() const
  {
-     return !transportWait && !encryptionWait && !reader && !writer && Adaptation::Initiate::doneAll();
 -    return !waitingForDns && !connector && !securer && !reader && !writer &&
++    return !waitingForDns && !transportWait && !encryptionWait &&
++           !reader && !writer &&
+            Adaptation::Initiate::doneAll();
  }
  
  void Adaptation::Icap::Xaction::updateTimeout()
@@@ -727,17 -767,8 +736,18 @@@ Adaptation::Icap::Xaction::handleSecure
  
      debugs(93, 5, "TLS negotiation to " << service().cfg().uri << " complete");
  
 -    service().noteConnectionUse(answer.conn);
 +    assert(answer.conn);
 +
 +    // The socket could get closed while our callback was queued. Sync
 +    // Connection. XXX: Connection::fd may already be stale/invalid here.
 +    if (answer.conn->isOpen() && fd_table[answer.conn->fd].closing()) {
 +        answer.conn->noteClosure();
 +        service().noteConnectionFailed("external TLS connection closure");
-         detailError(ERR_DETAIL_ICAP_XACT_SSL_START);
++        static const auto d = MakeNamedErrorDetail("ICAP_XACT_SSL_CLOSE");
++        detailError(d);
 +        throw TexcHere("external closure of the TLS ICAP service connection");
 +    }
  
 -    handleCommConnected();
 +    useIcapConnection(answer.conn);
  }
  

diff --cc src/adaptation/icap/Xaction.h
index 5b1c47cfa33d38e97f289893144a80389c25735f,8fcceaa0ac05a7f2e3d738635b6bc1274ce78d68..9c2742149a20942d1ae8e87dacd971202d0d3818
--- 1/src/adaptation/icap/Xaction.h
--- 2/src/adaptation/icap/Xaction.h
+++ b/src/adaptation/icap/Xaction.h
@@@ -12,8 -12,8 +12,9 @@@
  #include "AccessLogEntry.h"
  #include "adaptation/icap/ServiceRep.h"
  #include "adaptation/Initiate.h"
 +#include "base/JobWait.h"
  #include "comm/ConnOpener.h"
+ #include "error/forward.h"
  #include "HttpReply.h"
  #include "ipcache.h"
  #include "sbuf/SBuf.h"
@@@ -142,9 -137,15 +143,10 @@@ protected
      bool isRetriable;  ///< can retry on persistent connection failures
      bool isRepeatable; ///< can repeat if no or unsatisfactory response
      bool ignoreLastWrite;
+     bool waitingForDns; ///< expecting a ipcache_nbgethostbyname() callback
  
 -    const char *stopReason;
 -
 -    // active (pending) comm callbacks for the ICAP server connection
 -    AsyncCall::Pointer connector;
      AsyncCall::Pointer reader;
      AsyncCall::Pointer writer;
 -    AsyncCall::Pointer closer;
  
      AccessLogEntry::Pointer alep; ///< icap.log entry
      AccessLogEntry &al; ///< short for *alep

diff --cc src/base/AsyncJob.cc
Simple merge

diff --cc src/base/AsyncJob.h
Simple merge

diff --cc src/base/Makefile.am
Simple merge

diff --cc src/base/forward.h
Simple merge

diff --cc src/client_side.cc
Simple merge

diff --cc src/clients/FtpClient.cc
Simple merge

diff --cc src/clients/FtpClient.h
Simple merge

diff --cc src/clients/FtpGateway.cc
Simple merge

diff --cc src/clients/FtpRelay.cc
Simple merge

diff --cc src/clients/HttpTunneler.cc
Simple merge

diff --cc src/clients/HttpTunneler.h
Simple merge

diff --cc src/clients/forward.h
Simple merge

diff --cc src/comm.cc
Simple merge

diff --cc src/comm/ConnOpener.cc
Simple merge

diff --cc src/comm/ConnOpener.h
Simple merge

diff --cc src/comm/Connection.cc
Simple merge

diff --cc src/comm/Connection.h
Simple merge

diff --cc src/comm/TcpAcceptor.cc
Simple merge

diff --cc src/dns_internal.cc
Simple merge

diff --cc src/fs/rock/RockRebuild.cc
Simple merge

diff --cc src/gopher.cc
Simple merge

diff --cc src/ident/Ident.cc
Simple merge

diff --cc src/log/TcpLogger.cc
Simple merge

diff --cc src/log/TcpLogger.h
Simple merge

diff --cc src/mgr/Forwarder.cc
Simple merge

diff --cc src/mgr/Inquirer.cc
Simple merge

diff --cc src/mgr/StoreToCommWriter.cc
Simple merge

diff --cc src/security/BlindPeerConnector.h
Simple merge

diff --cc src/security/PeerConnector.cc
index d06e281ee2ba8344be8e3fc48916504978286ca2,6d51b7fec61ed35d6eee5fd01c8f154361895496..114b33124fd832c15d4121b473290dcaa71e3042
--- 1/src/security/PeerConnector.cc
--- 2/src/security/PeerConnector.cc
+++ b/src/security/PeerConnector.cc
@@@ -88,19 -107,12 +107,18 @@@ Security::PeerConnector::fillChecklist(
  void
  Security::PeerConnector::commCloseHandler(const CommCloseCbParams &params)
  {
 +    debugs(83, 5, "FD " << params.fd << ", Security::PeerConnector=" << params.data);
 +
      closeHandler = nullptr;
 +    if (serverConn) {
 +        countFailingConnection();
 +        serverConn->noteClosure();
 +        serverConn = nullptr;
 +    }
  
 -    debugs(83, 5, "FD " << params.fd << ", Security::PeerConnector=" << params.data);
      const auto err = new ErrorState(ERR_SECURE_CONNECT_FAIL, Http::scServiceUnavailable, request.getRaw(), al);
- #if USE_OPENSSL
-     err->detail = new Ssl::ErrorDetail(SQUID_ERR_SSL_HANDSHAKE, nullptr, nullptr);
- #endif
+     static const auto d = MakeNamedErrorDetail("TLS_CONNECT_CLOSE");
+     err->detailError(d);
      bail(err);
  }
  
@@@ -187,42 -200,70 +212,70 @@@ Security::PeerConnector::negotiate(
      if (fd_table[fd].closing())
          return;
  
+     const auto result = Security::Connect(*serverConnection());
+ 
  #if USE_OPENSSL
-     auto session = fd_table[fd].ssl.get();
-     debugs(83, 5, "SSL_connect session=" << (void*)session);
-     const int result = SSL_connect(session);
-     if (result <= 0) {
- #elif USE_GNUTLS
-     auto session = fd_table[fd].ssl.get();
-     const int result = gnutls_handshake(session);
-     debugs(83, 5, "gnutls_handshake session=" << (void*)session << ", result=" << result);
- 
-     if (result == GNUTLS_E_SUCCESS) {
-         char *desc = gnutls_session_get_desc(session);
-         debugs(83, 2, serverConnection() << " TLS Session info: " << desc);
-         gnutls_free(desc);
+     auto &sconn = *fd_table[fd].ssl;
+ 
+     // log ASAP, even if the handshake has not completed (or failed)
+     keyLogger.checkpoint(sconn, *this);
+ 
+     // OpenSSL v1 APIs do not allow unthreaded applications like Squid to fetch
+     // missing certificates _during_ OpenSSL certificate validation. Our
+     // handling of X509_V_ERR_UNABLE_TO_GET_ISSUER_CERT_LOCALLY (abbreviated
+     // here as EUNABLE) approximates what would happen if we did (attempt to)
+     // fetch any missing certificates during OpenSSL certificate validation.
+     // * We did not hide EUNABLE; SSL_connect() was successful: Handle success.
+     // * We did not hide EUNABLE; SSL_connect() reported some error E: Honor E.
+     // * We hid EUNABLE; SSL_connect() was successful: Remember success and try
+     //   to fetch the missing certificates. If all goes well, honor success.
+     // * We hid EUNABLE; SSL_connect() reported EUNABLE: Warn but honor EUNABLE.
+     // * We hid EUNABLE; SSL_connect() reported some EOTHER: Remember EOTHER and
+     //   try to fetch the missing certificates. If all goes well, honor EOTHER.
+     //   If fetching or post-fetching validation fails, then honor that failure
+     //   because EOTHER would not have happened if we fetched during validation.
+     if (auto &hidMissingIssuer = Ssl::VerifyCallbackParameters::At(sconn).hidMissingIssuer) {
+         hidMissingIssuer = false; // prep for the next SSL_connect()
+ 
+         if (result.category == IoResult::ioSuccess ||
+                 !(result.errorDetail && result.errorDetail->errorNo() == X509_V_ERR_UNABLE_TO_GET_ISSUER_CERT_LOCALLY))
+             return handleMissingCertificates(result);
+ 
+         debugs(83, DBG_IMPORTANT, "BUG: Honoring unexpected SSL_connect() error: X509_V_ERR_UNABLE_TO_GET_ISSUER_CERT_LOCALLY");
+         // fall through to regular error handling
      }
- 
-     if (result != GNUTLS_E_SUCCESS) {
-         // debug the TLS session state so far
-         auto descIn = gnutls_handshake_get_last_in(session);
-         debugs(83, 2, "handshake IN: " << gnutls_handshake_description_get_name(descIn));
-         auto descOut = gnutls_handshake_get_last_out(session);
-         debugs(83, 2, "handshake OUT: " << gnutls_handshake_description_get_name(descOut));
- #else
-     if (const int result = -1) {
  #endif
-         handleNegotiateError(result);
-         return; // we might be gone by now
-     }
  
-     recordNegotiationDetails();
+     handleNegotiationResult(result);
+ }
+ 
+ void
+ Security::PeerConnector::handleNegotiationResult(const Security::IoResult &result)
+ {
+     switch (result.category) {
+     case Security::IoResult::ioSuccess:
+         recordNegotiationDetails();
 -        if (sslFinalized())
++        if (sslFinalized() && callback)
+             sendSuccess();
+         return; // we may be gone by now
  
-     if (!sslFinalized())
+     case Security::IoResult::ioWantRead:
+         noteWantRead();
+         return;
+ 
+     case Security::IoResult::ioWantWrite:
+         noteWantWrite();
          return;
  
-     if (callback) // true sslFinalized() may bail(), calling the callback
-         sendSuccess();
+     case Security::IoResult::ioError:
+         break; // fall through to error handling
+     }
+ 
+     // TODO: Honor result.important when working in a reverse proxy role?
+     debugs(83, 2, "ERROR: " << result.errorDescription <<
+            " while establishing TLS connection on FD: " << serverConnection()->fd << result.errorDetail);
+     recordNegotiationDetails();
+     noteNegotiationError(result.errorDetail);
  }
  
  bool
@@@ -274,9 -314,8 +327,9 @@@ voi
  Security::PeerConnector::sslCrtvdHandleReply(Ssl::CertValidationResponse::Pointer validationResponse)
  {
      Must(validationResponse != NULL);
 +    Must(Comm::IsConnOpen(serverConnection()));
  
-     Ssl::ErrorDetail *errDetails = NULL;
+     ErrorDetail::Pointer errDetails;
      bool validatorFailed = false;
  
      if (Debug::Enabled(83, 5)) {
@@@ -322,10 -360,8 +375,10 @@@
  /// The first honored error, if any, is returned via errDetails parameter.
  /// The method returns all seen errors except SSL_ERROR_NONE as Security::CertErrors.
  Security::CertErrors *
- Security::PeerConnector::sslCrtvdCheckForErrors(Ssl::CertValidationResponse const &resp, Ssl::ErrorDetail *& errDetails)
+ Security::PeerConnector::sslCrtvdCheckForErrors(Ssl::CertValidationResponse const &resp, ErrorDetail::Pointer &errDetails)
  {
 +    Must(Comm::IsConnOpen(serverConnection()));
 +
      ACLFilledChecklist *check = NULL;
      Security::SessionPointer session(fd_table[serverConnection()->fd].ssl);
  
@@@ -401,99 -435,9 +452,11 @@@ Security::PeerConnector::negotiateSsl(
  void
  Security::PeerConnector::noteWantRead()
  {
 -    const int fd = serverConnection()->fd;
      debugs(83, 5, serverConnection());
  
- #if USE_OPENSSL
-     Security::SessionPointer session(fd_table[fd].ssl);
-     BIO *b = SSL_get_rbio(session.get());
-     Ssl::ServerBio *srvBio = static_cast<Ssl::ServerBio *>(BIO_get_data(b));
-     if (srvBio->holdRead()) {
-         if (srvBio->gotHello()) {
-             if (checkForMissingCertificates())
-                 return; // Wait to download certificates before proceed.
- 
-             srvBio->holdRead(false);
-             // schedule a negotiateSSl to allow openSSL parse received data
-             negotiateSsl();
-             return;
-         } else if (srvBio->gotHelloFailed()) {
-             srvBio->holdRead(false);
-             debugs(83, DBG_IMPORTANT, "Error parsing SSL Server Hello Message on FD " << fd);
-             // schedule a negotiateSSl to allow openSSL parse received data
-             negotiateSsl();
-             return;
-         }
-     }
- #endif
 +    Must(Comm::IsConnOpen(serverConnection()));
 +    const int fd = serverConnection()->fd;
 +
      // read timeout to avoid getting stuck while reading from a silent server
      typedef CommCbMemFunT<Security::PeerConnector, CommTimeoutCbParams> TimeoutDialer;
      AsyncCall::Pointer timeoutCall = JobCallback(83, 5,
@@@ -697,9 -597,8 +638,8 @@@ Security::PeerConnector::startCertDownl
                                        "Security::PeerConnector::certDownloadingDone",
                                        PeerConnectorCertDownloaderDialer(&Security::PeerConnector::certDownloadingDone, this));
  
-     const Downloader *csd = (request ? dynamic_cast<const Downloader*>(request->downloader.valid()) : nullptr);
-     Downloader *dl = new Downloader(url, certCallback, XactionInitiator::initCertFetcher, csd ? csd->nestedLevel() + 1 : 1);
+     const auto dl = new Downloader(url, certCallback, XactionInitiator::initCertFetcher, certDownloadNestingLevel() + 1);
 -    AsyncJob::Start(dl);
 +    certDownloadWait.start(dl, certCallback);
  }
  
  void
@@@ -710,12 -607,7 +650,8 @@@ Security::PeerConnector::certDownloadin
      ++certsDownloads;
      debugs(81, 5, "Certificate downloading status: " << downloadStatus << " certificate size: " << obj.length());
  
-     // get ServerBio from SSL object
 +    Must(Comm::IsConnOpen(serverConnection()));
-     const int fd = serverConnection()->fd;
-     Security::SessionPointer session(fd_table[fd].ssl);
-     BIO *b = SSL_get_rbio(session.get());
-     Ssl::ServerBio *srvBio = static_cast<Ssl::ServerBio *>(BIO_get_data(b));
+     const auto &sconn = *fd_table[serverConnection()->fd].ssl;
  
      // Parse Certificate. Assume that it is in DER format.
      // According to RFC 4325:
@@@ -744,40 -653,82 +697,83 @@@
          return;
      }
  
-     srvBio->holdRead(false);
-     negotiateSsl();
+     resumeNegotiation();
  }
  
- bool
- Security::PeerConnector::checkForMissingCertificates()
+ void
+ Security::PeerConnector::handleMissingCertificates(const Security::IoResult &ioResult)
  {
-     // Check for nested SSL certificates downloads. For example when the
-     // certificate located in an SSL site which requires to download a
-     // a missing certificate (... from an SSL site which requires to ...).
++    Must(Comm::IsConnOpen(serverConnection()));
+     auto &sconn = *fd_table[serverConnection()->fd].ssl;
+ 
+     // We download the missing certificate(s) once. We would prefer to clear
+     // this right after the first validation, but that ideal place is _inside_
+     // OpenSSL if validation is triggered by SSL_connect(). That function and
+     // our OpenSSL verify_callback function (\ref OpenSSL_vcb_disambiguation)
+     // may be called multiple times, so we cannot reset there.
+     auto &callerHandlesMissingCertificates = Ssl::VerifyCallbackParameters::At(sconn).callerHandlesMissingCertificates;
+     Must(callerHandlesMissingCertificates);
+     callerHandlesMissingCertificates = false;
+ 
+     if (!computeMissingCertificateUrls(sconn))
+         return handleNegotiationResult(ioResult);
+ 
+     suspendNegotiation(ioResult);
  
-     const Downloader *csd = (request ? request->downloader.get() : nullptr);
-     if (csd && csd->nestedLevel() >= MaxNestedDownloads)
+     assert(!urlsOfMissingCerts.empty());
+     startCertDownloading(urlsOfMissingCerts.front());
+     urlsOfMissingCerts.pop();
+ }
+ 
+ /// finds URLs of (some) missing intermediate certificates or returns false
+ bool
+ Security::PeerConnector::computeMissingCertificateUrls(const Connection &sconn)
+ {
+     const auto certs = SSL_get_peer_cert_chain(&sconn);
+     if (!certs) {
+         debugs(83, 3, "nothing to bootstrap the fetch with");
          return false;
+     }
+     debugs(83, 5, "server certificates: " << sk_X509_num(certs));
  
-     Must(Comm::IsConnOpen(serverConnection()));
-     const int fd = serverConnection()->fd;
-     Security::SessionPointer session(fd_table[fd].ssl);
-     BIO *b = SSL_get_rbio(session.get());
-     Ssl::ServerBio *srvBio = static_cast<Ssl::ServerBio *>(BIO_get_data(b));
-     const Security::CertList &certs = srvBio->serverCertificatesIfAny();
+     const auto ctx = getTlsContext();
+     if (!Ssl::missingChainCertificatesUrls(urlsOfMissingCerts, *certs, ctx))
+         return false; // missingChainCertificatesUrls() reports the exact reason
  
-     if (certs.size()) {
-         debugs(83, 5, "SSL server sent " << certs.size() << " certificates");
-         ContextPointer ctx(getTlsContext());
-         Ssl::missingChainCertificatesUrls(urlsOfMissingCerts, certs, ctx);
-         if (urlsOfMissingCerts.size()) {
-             startCertDownloading(urlsOfMissingCerts.front());
-             urlsOfMissingCerts.pop();
-             return true;
-         }
+     debugs(83, 5, "URLs: " << urlsOfMissingCerts.size());
+     assert(!urlsOfMissingCerts.empty());
+     return true;
+ }
+ 
+ void
+ Security::PeerConnector::suspendNegotiation(const Security::IoResult &ioResult)
+ {
+     debugs(83, 5, "after " << ioResult);
+     Must(!isSuspended());
+     suspendedError_ = new Security::IoResult(ioResult);
+     Must(isSuspended());
+     // negotiations resume with a resumeNegotiation() call
+ }
+ 
+ void
+ Security::PeerConnector::resumeNegotiation()
+ {
+     Must(isSuspended());
+ 
+     auto lastError = suspendedError_; // may be reset below
+     suspendedError_ = nullptr;
+ 
+     auto &sconn = *fd_table[serverConnection()->fd].ssl;
+     if (!Ssl::VerifyConnCertificates(sconn, downloadedCerts)) {
+         // simulate an earlier SSL_connect() failure with a new error
+         // TODO: When we can use Security::ErrorDetail, we should resume with a
+         // detailed _validation_ error, not just a generic SSL_ERROR_SSL!
+         const ErrorDetail::Pointer errorDetail = new ErrorDetail(SQUID_TLS_ERR_CONNECT, SSL_ERROR_SSL, 0);
+         lastError = new Security::IoResult(errorDetail);
      }
  
-     return false;
+     handleNegotiationResult(*lastError);
  }
+ 
  #endif //USE_OPENSSL
  

diff --cc src/security/PeerConnector.h
index f886b6ee761cde3bc9cd54c644b89cecbbe3522e,819dd1fa9b082705eaf9eb8710228c5f6f46b592..ce1dcc6a6718499bf400165b94c99d56d7e3fba5
--- 1/src/security/PeerConnector.h
--- 2/src/security/PeerConnector.h
+++ b/src/security/PeerConnector.h
@@@ -10,9 -10,9 +10,10 @@@
  #define SQUID_SRC_SECURITY_PEERCONNECTOR_H
  
  #include "acl/Acl.h"
+ #include "acl/ChecklistFiller.h"
  #include "base/AsyncCbdataCalls.h"
  #include "base/AsyncJob.h"
 +#include "base/JobWait.h"
  #include "CommCalls.h"
  #include "http/forward.h"
  #include "security/EncryptorAnswer.h"
@@@ -194,7 -203,14 +211,16 @@@ private
      /// The list of URLs where missing certificates should be downloaded.
      std::queue<SBuf> urlsOfMissingCerts;
      unsigned int certsDownloads; ///< the number of downloaded missing certificates
+ 
+ #if USE_OPENSSL
+     /// successfully downloaded intermediate certificates (omitted by the peer)
+     Ssl::X509_STACK_Pointer downloadedCerts;
+ #endif
+ 
+     /// outcome of the last (failed and) suspended negotiation attempt (or nil)
+     Security::IoResultPointer suspendedError_;
++
 +    JobWait<Downloader> certDownloadWait; ///< waits for the missing certificate to be downloaded
  };
  
  } // namespace Security

diff --cc src/security/forward.h
Simple merge

diff --cc src/servers/FtpServer.cc
Simple merge

diff --cc src/servers/FtpServer.h
Simple merge

diff --cc src/snmp/Forwarder.cc
Simple merge

diff --cc src/snmp/Inquirer.cc
Simple merge

diff --cc src/ssl/PeekingPeerConnector.cc
Simple merge

diff --cc src/tests/stub_libcomm.cc
Simple merge

diff --cc src/tests/stub_libsecurity.cc
Simple merge

diff --cc src/tunnel.cc
Simple merge

diff --cc src/whois.cc
Simple merge