From: Christos Tsantilas Date: Mon, 22 Feb 2021 21:15:32 +0000 (+0000) Subject: Handling missing issuer certificates for TLSv1.3 (#766) X-Git-Tag: 4.15-20210522-snapshot~30 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=800967afde7921eb22a2711c5a75b5cd9c9d7178;p=thirdparty%2Fsquid.git Handling missing issuer certificates for TLSv1.3 (#766) Prior to TLS v1.3 Squid could detect and fetch missing intermediate server certificates by parsing TLS ServerHello. TLS v1.3 encrypts the relevant part of the handshake, making such "prefetch" impossible. Instead of looking for certificates in TLS ServerHello, Squid now waits for the OpenSSL built-in certificate validation to fail with an X509_V_ERR_UNABLE_TO_GET_ISSUER_CERT_LOCALLY error. Squid-supplied verify_callback function tells OpenSSL to ignore that error. Squid SSL_connect()-calling code detects that the error was ignored and, if possible, fetches the missing certificates and orchestrates certificate chain validation outside the SSL_connect() sequence. If that validation is successful, Squid continues with SSL_connect(). See comments inside Security::PeerConnector::negotiate() for low-level details. In some cases, OpenSSL is able to complete SSL_connect() with an ignored X509_V_ERR_UNABLE_TO_GET_ISSUER_CERT_LOCALLY error. If Squid validation fails afterwards, the TLS connection is closed (before any payload is exchanged). Ideally, the negotiation with an untrusted server should not complete, but complexity BIO changes required to prevent such premature completion is probably not worth reaching that ideal, especially since all of this is just a workaround. The long-term solution is adding SSL_ERROR_WANT_RETRY_VERIFY to OpenSSL, giving an application a chance to download the missing certificates during SSL_connect() negotiations. We assist OpenSSL team with that change, but it will not be available at least until OpenSSL v3.0. This description and changes are not specific to SslBump code paths. This is a Measurement Factory project. --- diff --git a/acinclude/lib-checks.m4 b/acinclude/lib-checks.m4 index 3058613499..c746cbd85e 100644 --- a/acinclude/lib-checks.m4 +++ b/acinclude/lib-checks.m4 @@ -71,10 +71,13 @@ AC_DEFUN([SQUID_CHECK_LIBCRYPTO_API],[ AH_TEMPLATE(HAVE_LIBCRYPTO_X509_STORE_CTX_GET0_CERT, "Define to 1 if the X509_STORE_CTX_get0_cert() OpenSSL API function exists") AH_TEMPLATE(HAVE_LIBCRYPTO_X509_VERIFY_PARAM_GET_DEPTH, "Define to 1 if the X509_VERIFY_PARAM_get_depth() OpenSSL API function exists") AH_TEMPLATE(HAVE_LIBCRYPTO_X509_STORE_CTX_GET0_UNTRUSTED, "Define to 1 if the X509_STORE_CTX_get0_untrusted() OpenSSL API function exists") + AH_TEMPLATE(HAVE_X509_VERIFY_PARAM_SET_AUTH_LEVEL, "Define to 1 if the X509_VERIFY_PARAM_set_auth_level() OpenSSL API function exists") AH_TEMPLATE(HAVE_LIBCRYPTO_X509_UP_REF, "Define to 1 if the X509_up_ref() OpenSSL API function exists") + AH_TEMPLATE(HAVE_LIBCRYPTO_X509_CHAIN_UP_REF, "Define to 1 if the X509_chain_up_ref() OpenSSL API function exists") AH_TEMPLATE(HAVE_LIBCRYPTO_X509_CRL_UP_REF, "Define to 1 if the X509_CRL_up_ref() OpenSSL API function exists") AH_TEMPLATE(HAVE_LIBCRYPTO_DH_UP_REF, "Define to 1 if the DH_up_ref() OpenSSL API function exists") AH_TEMPLATE(HAVE_LIBCRYPTO_X509_GET0_SIGNATURE, "Define to 1 if the X509_get0_signature() OpenSSL API function exists") + AH_TEMPLATE(HAVE_SSL_GET0_PARAM, "Define to 1 of the SSL_get0_param() OpenSSL API function exists") SQUID_STATE_SAVE(check_openssl_libcrypto_api) LIBS="$LIBS $SSLLIB" AC_CHECK_LIB(crypto, OPENSSL_LH_strhash, AC_DEFINE(HAVE_LIBCRYPTO_OPENSSL_LH_STRHASH, 1)) @@ -87,10 +90,13 @@ AC_DEFUN([SQUID_CHECK_LIBCRYPTO_API],[ AC_CHECK_LIB(crypto, X509_STORE_CTX_get0_cert, AC_DEFINE(HAVE_LIBCRYPTO_X509_STORE_CTX_GET0_CERT, 1)) AC_CHECK_LIB(crypto, X509_VERIFY_PARAM_get_depth, AC_DEFINE(HAVE_LIBCRYPTO_X509_VERIFY_PARAM_GET_DEPTH, 1)) AC_CHECK_LIB(crypto, X509_STORE_CTX_get0_untrusted, AC_DEFINE(HAVE_LIBCRYPTO_X509_STORE_CTX_GET0_UNTRUSTED, 1)) + AC_CHECK_LIB(crypto, X509_VERIFY_PARAM_set_auth_level, AC_DEFINE(HAVE_X509_VERIFY_PARAM_SET_AUTH_LEVEL)) AC_CHECK_LIB(crypto, X509_up_ref, AC_DEFINE(HAVE_LIBCRYPTO_X509_UP_REF, 1)) + AC_CHECK_LIB(crypto, X509_chain_up_ref, AC_DEFINE(HAVE_LIBCRYPTO_X509_CHAIN_UP_REF, 1)) AC_CHECK_LIB(crypto, X509_CRL_up_ref, AC_DEFINE(HAVE_LIBCRYPTO_X509_CRL_UP_REF, 1)) AC_CHECK_LIB(crypto, DH_up_ref, AC_DEFINE(HAVE_LIBCRYPTO_DH_UP_REF, 1)) AC_CHECK_LIB(crypto, X509_get0_signature, AC_DEFINE(HAVE_LIBCRYPTO_X509_GET0_SIGNATURE, 1), AC_DEFINE(SQUID_CONST_X509_GET0_SIGNATURE_ARGS,)) + AC_CHECK_LIB(crypto, SSL_get0_param, AC_DEFINE(HAVE_SSL_GET0_PARAM, 1)) SQUID_STATE_ROLLBACK(check_openssl_libcrypto_api) ]) diff --git a/compat/openssl.h b/compat/openssl.h index ad85cbe899..6bd98cd6e3 100644 --- a/compat/openssl.h +++ b/compat/openssl.h @@ -222,6 +222,29 @@ extern "C" { #endif /* CRYPTO_LOCK_X509 */ #endif /* X509_up_ref */ +#if !HAVE_LIBCRYPTO_X509_CHAIN_UP_REF + inline STACK_OF(X509) * + X509_chain_up_ref(STACK_OF(X509) *chain) + { + if (STACK_OF(X509) *newChain = sk_X509_dup(chain)) { + bool error = false; + int i; + for (i = 0; !error && i < sk_X509_num(newChain); i++) { + X509 *cert = sk_X509_value(newChain, i); + if (!X509_up_ref(cert)) + error = true; + } + if (!error) + return newChain; + + for (int k = 0; k < i; k++) + X509_free(sk_X509_value(newChain, k)); + sk_X509_free(newChain); + } + return nullptr; + } +#endif /* X509_chain_up_ref */ + #if !HAVE_LIBCRYPTO_X509_VERIFY_PARAM_GET_DEPTH inline int X509_VERIFY_PARAM_get_depth(const X509_VERIFY_PARAM *param) @@ -230,6 +253,13 @@ extern "C" { } #endif +#if !HAVE_SSL_GET0_PARAM + inline X509_VERIFY_PARAM * + SSL_get0_param(SSL *ssl) + { + return ssl->param; + } +#endif } /* extern "C" */ inline void diff --git a/src/security/Handshake.cc b/src/security/Handshake.cc index f045b82c32..e41f8846ed 100644 --- a/src/security/Handshake.cc +++ b/src/security/Handshake.cc @@ -366,11 +366,6 @@ Security::HandshakeParser::parseHandshakeMessage() if (Tls1p3orLater(details->tlsSupportedVersion)) done = "ServerHello in v1.3+"; return; - case HandshakeType::hskCertificate: - Must(state < atCertificatesReceived); - parseServerCertificates(message.msg_body); - state = atCertificatesReceived; - return; case HandshakeType::hskServerHelloDone: Must(state < atHelloDoneReceived); // zero-length @@ -664,47 +659,6 @@ Security::HandshakeParser::parseHello(const SBuf &data) return false; // unreached } -/// Creates and returns a certificate by parsing a DER-encoded X509 structure. -/// Throws on failures. -Security::CertPointer -Security::HandshakeParser::ParseCertificate(const SBuf &raw) -{ - Security::CertPointer pCert; -#if USE_OPENSSL - auto x509Start = reinterpret_cast(raw.rawContent()); - auto x509Pos = x509Start; - X509 *x509 = d2i_X509(nullptr, &x509Pos, raw.length()); - pCert.resetWithoutLocking(x509); - Must(x509); // successfully parsed - Must(x509Pos == x509Start + raw.length()); // no leftovers -#else - assert(false); // this code should never be reached - pCert = Security::CertPointer(nullptr); // avoid warnings about uninitialized pCert; XXX: Fix CertPoint declaration. - (void)raw; // avoid warnings about unused method parameter; TODO: Add a SimulateUse() macro. -#endif - assert(pCert); - return pCert; -} - -void -Security::HandshakeParser::parseServerCertificates(const SBuf &raw) -{ -#if USE_OPENSSL - Parser::BinaryTokenizer tkList(raw); - const SBuf clist = tkList.pstring24("CertificateList"); - Must(tkList.atEnd()); // no leftovers after all certificates - - Parser::BinaryTokenizer tkItems(clist); - while (!tkItems.atEnd()) { - if (Security::CertPointer cert = ParseCertificate(tkItems.pstring24("Certificate"))) - serverCertificates.push_back(cert); - debugs(83, 7, "parsed " << serverCertificates.size() << " certificates so far"); - } -#else - debugs(83, 7, "no support for CertificateList parsing; ignoring " << raw.length() << " bytes"); -#endif -} - /// A helper function to create a set of all supported TLS extensions static Security::Extensions diff --git a/src/security/Handshake.h b/src/security/Handshake.h index 3e5d7a09b4..8a84344fd3 100644 --- a/src/security/Handshake.h +++ b/src/security/Handshake.h @@ -61,7 +61,7 @@ class HandshakeParser { public: /// The parsing states - typedef enum {atHelloNone = 0, atHelloStarted, atHelloReceived, atCertificatesReceived, atHelloDoneReceived, atNstReceived, atCcsReceived, atFinishReceived} ParserState; + typedef enum { atHelloNone = 0, atHelloStarted, atHelloReceived, atHelloDoneReceived, atNstReceived, atCcsReceived, atFinishReceived } ParserState; /// the originator of the TLS handshake being parsed typedef enum { fromClient = 0, fromServer } MessageSource; @@ -76,8 +76,6 @@ public: TlsDetails::Pointer details; ///< TLS handshake meta info. Never nil. - Security::CertList serverCertificates; ///< parsed certificates chain - ParserState state; ///< current parsing state. bool resumingSession; ///< True if this is a resuming session @@ -112,7 +110,6 @@ private: void parseV23Ciphers(const SBuf &raw); void parseServerCertificates(const SBuf &raw); - static CertPointer ParseCertificate(const SBuf &raw); unsigned int currentContentType; ///< The current TLS/SSL record content type diff --git a/src/security/Io.cc b/src/security/Io.cc index d6714049b3..14d21b9179 100644 --- a/src/security/Io.cc +++ b/src/security/Io.cc @@ -22,6 +22,33 @@ typedef SessionPointer::element_type *ConnectionPointer; } // namespace Security +void +Security::IoResult::print(std::ostream &os) const +{ + const char *strCat = "unknown"; + switch (category) { + case ioSuccess: + strCat = "success"; + break; + case ioWantRead: + strCat = "want-read"; + break; + case ioWantWrite: + strCat = "want-write"; + break; + case ioError: + strCat = "error"; + break; + } + os << strCat; + + if (errorDescription) + os << ", " << errorDescription; + + if (important) + os << ", important"; +} + // TODO: Replace high-level ERR_get_error() calls with a new std::ostream // ReportErrors manipulator inside debugs(), followed by a ForgetErrors() call. void diff --git a/src/security/Io.h b/src/security/Io.h index 3a14b6fa1a..2ae15260ac 100644 --- a/src/security/Io.h +++ b/src/security/Io.h @@ -16,13 +16,16 @@ namespace Security { /// a summary a TLS I/O operation outcome -class IoResult { +class IoResult: public RefCountable { public: + typedef RefCount Pointer; + /// all possible outcome cases typedef enum { ioSuccess, ioWantRead, ioWantWrite, ioError } Category; explicit IoResult(const Category aCategory): category(aCategory) {} explicit IoResult(const ErrorDetailPointer &anErrorDetail): errorDetail(anErrorDetail) {} + IoResult(const IoResult &aRes) = default; /// convenience wrapper to detect successful I/O outcome; implies !wantsIo() bool successful() const { return category == ioSuccess; } @@ -30,6 +33,8 @@ public: /// convenience wrapper to detect whether more I/O is needed bool wantsIo() const { return category == ioWantRead || category == ioWantWrite; } + void print(std::ostream &os) const; + ErrorDetailPointer errorDetail; ///< ioError case details (or nil) Category category = ioError; ///< primary outcome classification @@ -39,6 +44,13 @@ public: bool important = false; ///< whether the error was serious/unusual }; +inline std::ostream & +operator <<(std::ostream &os, const IoResult &result) +{ + result.print(os); + return os; +} + /// accept a TLS connection over the specified to-Squid transport connection IoResult Accept(Comm::Connection &transport); diff --git a/src/security/PeerConnector.cc b/src/security/PeerConnector.cc index 8a2a69a138..956ef0aeac 100644 --- a/src/security/PeerConnector.cc +++ b/src/security/PeerConnector.cc @@ -57,6 +57,8 @@ Security::PeerConnector::PeerConnector(const Comm::ConnectionPointer &aServerCon comm_add_close_handler(serverConn->fd, closeHandler); } +Security::PeerConnector::~PeerConnector() = default; + bool Security::PeerConnector::doneAll() const { return (!callback || callback->canceled()) && AsyncJob::doneAll(); @@ -142,6 +144,16 @@ Security::PeerConnector::initialize(Security::SessionPointer &serverSession) SSL_set_ex_data(serverSession.get(), ssl_ex_index_cert_error_check, check); } } + + // Protect from cycles in the certificate dependency graph: TLS site S1 is + // missing certificate C1 located at TLS site S2. TLS site S2 is missing + // certificate C2 located at [...] TLS site S1. + const auto cycle = certDownloadNestingLevel() >= MaxNestedDownloads; + if (cycle) + debugs(83, 3, "will not fetch any missing certificates; suspecting cycle: " << certDownloadNestingLevel() << '/' << MaxNestedDownloads); + const auto sessData = Ssl::VerifyCallbackParameters::New(*serverSession); + // when suspecting a cycle, break it by not fetching any missing certs + sessData->callerHandlesMissingCertificates = !cycle; #endif return true; @@ -173,6 +185,42 @@ Security::PeerConnector::negotiate() return; const auto result = Security::Connect(*serverConnection()); + +#if USE_OPENSSL + auto &sconn = *fd_table[fd].ssl; + + // 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 + } +#endif + + handleNegotiationResult(result); +} + +void +Security::PeerConnector::handleNegotiationResult(const Security::IoResult &result) +{ switch (result.category) { case Security::IoResult::ioSuccess: recordNegotiationDetails(); @@ -194,7 +242,7 @@ Security::PeerConnector::negotiate() // TODO: Honor result.important when working in a reverse proxy role? debugs(83, 2, "ERROR: " << result.errorDescription << - " while establishing TLS connection on FD: " << fd << result.errorDetail); + " while establishing TLS connection on FD: " << serverConnection()->fd << result.errorDetail); recordNegotiationDetails(); noteNegotiationError(result.errorDetail); } @@ -372,28 +420,6 @@ 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(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 // read timeout to avoid getting stuck while reading from a silent server typedef CommCbMemFunT TimeoutDialer; @@ -533,6 +559,20 @@ public: CbcPointer peerConnector_; ///< The Security::PeerConnector object }; +/// the number of concurrent PeerConnector jobs waiting for us +unsigned int +Security::PeerConnector::certDownloadNestingLevel() const +{ + if (request) { + // Nesting level increases when a PeerConnector (at level L) creates a + // Downloader (which is assigned level L+1). If we were initiated by + // such a Downloader, then their nesting level is our nesting level. + if (const auto previousDownloader = request->downloader.get()) + return previousDownloader->nestedLevel(); + } + return 0; // no other PeerConnector job waits for us +} + void Security::PeerConnector::startCertDownloading(SBuf &url) { @@ -540,8 +580,7 @@ Security::PeerConnector::startCertDownloading(SBuf &url) "Security::PeerConnector::certDownloadingDone", PeerConnectorCertDownloaderDialer(&Security::PeerConnector::certDownloadingDone, this)); - const Downloader *csd = (request ? dynamic_cast(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); } @@ -551,11 +590,7 @@ Security::PeerConnector::certDownloadingDone(SBuf &obj, int downloadStatus) ++certsDownloads; debugs(81, 5, "Certificate downloading status: " << downloadStatus << " certificate size: " << obj.length()); - // get ServerBio from SSL object - const int fd = serverConnection()->fd; - Security::SessionPointer session(fd_table[fd].ssl); - BIO *b = SSL_get_rbio(session.get()); - Ssl::ServerBio *srvBio = static_cast(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: @@ -568,12 +603,29 @@ Security::PeerConnector::certDownloadingDone(SBuf &obj, int downloadStatus) if (X509 *cert = d2i_X509(NULL, &raw, obj.length())) { char buffer[1024]; debugs(81, 5, "Retrieved certificate: " << X509_NAME_oneline(X509_get_subject_name(cert), buffer, 1024)); + + if (!downloadedCerts) + downloadedCerts.reset(sk_X509_new_null()); + sk_X509_push(downloadedCerts.get(), cert); + ContextPointer ctx(getTlsContext()); - const Security::CertList &certsList = srvBio->serverCertificatesIfAny(); - if (const char *issuerUri = Ssl::uriOfIssuerIfMissing(cert, certsList, ctx)) { - urlsOfMissingCerts.push(SBuf(issuerUri)); + const auto certsList = SSL_get_peer_cert_chain(&sconn); + if (!Ssl::findIssuerCertificate(cert, certsList, ctx)) { + if (const auto issuerUri = Ssl::findIssuerUri(cert)) { + debugs(81, 5, "certificate " << + X509_NAME_oneline(X509_get_subject_name(cert), buffer, sizeof(buffer)) << + " points to its missing issuer certificate at " << issuerUri); + urlsOfMissingCerts.push(SBuf(issuerUri)); + } else { + debugs(81, 3, "found a certificate with no IAI, " << + "signed by a missing issuer certificate: " << + X509_NAME_oneline(X509_get_subject_name(cert), buffer, sizeof(buffer))); + // We could short-circuit here, proceeding to chain validation + // that is likely to fail. Instead, we keep going because we + // hope that if we find at least one certificate to fetch, it + // will complete the chain (that contained extra certificates). + } } - Ssl::SSL_add_untrusted_cert(session.get(), cert); } // Check if there are URIs to download from and if yes start downloading @@ -584,39 +636,82 @@ Security::PeerConnector::certDownloadingDone(SBuf &obj, int downloadStatus) 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 ...). + 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); + + assert(!urlsOfMissingCerts.empty()); + startCertDownloading(urlsOfMissingCerts.front()); + urlsOfMissingCerts.pop(); +} - const Downloader *csd = (request ? request->downloader.get() : nullptr); - if (csd && csd->nestedLevel() >= MaxNestedDownloads) +/// 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)); - const int fd = serverConnection()->fd; - Security::SessionPointer session(fd_table[fd].ssl); - BIO *b = SSL_get_rbio(session.get()); - Ssl::ServerBio *srvBio = static_cast(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 --git a/src/security/PeerConnector.h b/src/security/PeerConnector.h index f0c7be97d8..96f2ff668d 100644 --- a/src/security/PeerConnector.h +++ b/src/security/PeerConnector.h @@ -30,6 +30,9 @@ typedef RefCount AccessLogEntryPointer; namespace Security { +class IoResult; +typedef RefCount IoResultPointer; + /** * Initiates encryption of a given open TCP connection to a peer or server. * Despite its name does not perform any connect(2) operations. Owns the @@ -59,7 +62,7 @@ public: AsyncCall::Pointer &aCallback, const AccessLogEntryPointer &alp, const time_t timeout = 0); - virtual ~PeerConnector() = default; + virtual ~PeerConnector(); /// hack: whether the connection requires fwdPconnPool->noteUses() bool noteFwdPconnUse; @@ -89,23 +92,27 @@ protected: /// Otherwise, returns true, regardless of negotiation success/failure. bool sslFinalized(); - /// Called when the negotiation step aborted because data needs to - /// be transferred to/from server or on error. In the first case - /// setups the appropriate Comm::SetSelect handler. In second case - /// fill an error and report to the PeerConnector caller. - void handleNegotiateError(const int result); + /// Called after each negotiation step to handle the result + void handleNegotiationResult(const Security::IoResult &); /// Called when the openSSL SSL_connect fnction request more data from /// the remote SSL server. Sets the read timeout and sets the /// Squid COMM_SELECT_READ handler. void noteWantRead(); + /// Whether TLS negotiation has been paused and not yet resumed + bool isSuspended() const { return static_cast(suspendedError_); } + #if USE_OPENSSL - /// Run the certificates list sent by the SSL server and check if there - /// are missing certificates. Adds to the urlOfMissingCerts list the - /// URLS of missing certificates if this information provided by the - /// issued certificates with Authority Info Access extension. - bool checkForMissingCertificates(); + /// Suspends TLS negotiation to download the missing certificates + /// \param lastError an error to handle when resuming negotiations + void suspendNegotiation(const Security::IoResult &lastError); + + /// Resumes TLS negotiation paused by suspendNegotiation() + void resumeNegotiation(); + + /// Either initiates fetching of missing certificates or bails with an error + void handleMissingCertificates(const Security::IoResult &lastError); /// Start downloading procedure for the given URL. void startCertDownloading(SBuf &url); @@ -161,19 +168,24 @@ private: PeerConnector &operator =(const PeerConnector &); // not implemented #if USE_OPENSSL + unsigned int certDownloadNestingLevel() const; + /// Process response from cert validator helper void sslCrtvdHandleReply(Ssl::CertValidationResponsePointer); /// Check SSL errors returned from cert validator against sslproxy_cert_error access list Security::CertErrors *sslCrtvdCheckForErrors(Ssl::CertValidationResponse const &, ErrorDetailPointer &); + + bool computeMissingCertificateUrls(const Connection &); #endif static void NegotiateSsl(int fd, void *data); void negotiateSsl(); - /// The maximum allowed missing certificates downloads. + /// The maximum number of missing certificates a single PeerConnector may download static const unsigned int MaxCertsDownloads = 10; - /// The maximum allowed nested certificates downloads. + + /// The maximum number of inter-dependent Downloader jobs a worker may initiate static const unsigned int MaxNestedDownloads = 3; AsyncCall::Pointer closeHandler; ///< we call this when the connection closed @@ -183,6 +195,14 @@ private: /// The list of URLs where missing certificates should be downloaded. std::queue 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_; }; } // namespace Security diff --git a/src/security/Session.h b/src/security/Session.h index d32bd44696..52b3681691 100644 --- a/src/security/Session.h +++ b/src/security/Session.h @@ -41,6 +41,8 @@ class PeerOptions; bool CreateServerSession(const Security::ContextPointer &, const Comm::ConnectionPointer &, Security::PeerOptions &, const char *squidCtx); #if USE_OPENSSL +typedef SSL Connection; + typedef std::shared_ptr SessionPointer; typedef std::unique_ptr> SessionStatePointer; @@ -53,6 +55,8 @@ inline void squid_gnutls_free(void *d) {gnutls_free(d);} typedef std::unique_ptr> SessionStatePointer; #else +typedef std::nullptr_t Connection; + typedef std::shared_ptr SessionPointer; typedef std::unique_ptr SessionStatePointer; diff --git a/src/ssl/bio.cc b/src/ssl/bio.cc index 10b8e838b0..564417d9b3 100644 --- a/src/ssl/bio.cc +++ b/src/ssl/bio.cc @@ -245,7 +245,6 @@ Ssl::ServerBio::ServerBio(const int anFd): allowSplice(false), allowBump(false), holdWrite_(false), - holdRead_(true), record_(false), parsedHandshake(false), parseError(false), @@ -319,12 +318,6 @@ Ssl::ServerBio::readAndParse(char *buf, const int size, BIO *table) parseError = true; } - if (holdRead_) { - debugs(83, 7, "Hold flag is set, retry latter. (Hold " << size << "bytes)"); - BIO_set_retry_read(table); - return -1; - } - return giveBuffered(buf, size); } diff --git a/src/ssl/bio.h b/src/ssl/bio.h index c27801e752..35ba2e5866 100644 --- a/src/ssl/bio.h +++ b/src/ssl/bio.h @@ -151,10 +151,6 @@ public: bool holdWrite() const {return holdWrite_;} /// Enables or disables the write hold state void holdWrite(bool h) {holdWrite_ = h;} - /// The read hold state - bool holdRead() const {return holdRead_;} - /// Enables or disables the read hold state - void holdRead(bool h) {holdRead_ = h;} /// Enables or disables the input data recording, for internal analysis. void recordInput(bool r) {record_ = r;} /// Whether we can splice or not the SSL stream @@ -171,9 +167,6 @@ public: /// Return true if the Server Hello parsing failed bool gotHelloFailed() const { return (parsedHandshake && parseError); } - /// \return the server certificates list if received and parsed correctly - const Security::CertList &serverCertificatesIfAny() { return parser_.serverCertificates; } - /// \return the TLS Details advertised by TLS server. const Security::TlsDetails::Pointer &receivedHelloDetails() const {return parser_.details;} @@ -193,7 +186,6 @@ private: bool allowSplice; ///< True if the SSL stream can be spliced bool allowBump; ///< True if the SSL stream can be bumped bool holdWrite_; ///< The write hold state of the bio. - bool holdRead_; ///< The read hold state of the bio. bool record_; ///< If true the input data recorded to rbuf for internal use bool parsedHandshake; ///< whether we are done parsing TLS Hello bool parseError; ///< error while parsing server hello message diff --git a/src/ssl/gadgets.h b/src/ssl/gadgets.h index c9817f14c3..42dc7376e9 100644 --- a/src/ssl/gadgets.h +++ b/src/ssl/gadgets.h @@ -70,6 +70,7 @@ typedef std::unique_ptr> X509_EXTENSION_Pointer; +typedef std::unique_ptr> X509_STORE_CTX_Pointer; /** \ingroup SslCrtdSslAPI * Create 1024 bits rsa key. diff --git a/src/ssl/support.cc b/src/ssl/support.cc index 413cb366f5..424fa6324a 100644 --- a/src/ssl/support.cc +++ b/src/ssl/support.cc @@ -37,7 +37,7 @@ #include // TODO: Move ssl_ex_index_* global variables from global.cc here. -int ssl_ex_index_ssl_untrusted_chain = -1; +static int ssl_ex_index_verify_callback_parameters = -1; static Ssl::CertsIndexedList SquidUntrustedCerts; @@ -254,7 +254,8 @@ bool Ssl::checkX509ServerValidity(X509 *cert, const char *server) return matchX509CommonNames(cert, (void *)server, check_domain); } -/// \ingroup ServerProtocolSSLInternal +/// adjusts OpenSSL validation results for each verified certificate in ctx +/// OpenSSL "verify_callback function" (\ref OpenSSL_vcb_disambiguation) static int ssl_verify_cb(int ok, X509_STORE_CTX * ctx) { @@ -312,6 +313,16 @@ ssl_verify_cb(int ok, X509_STORE_CTX * ctx) } } + if (!ok && error_no == X509_V_ERR_UNABLE_TO_GET_ISSUER_CERT_LOCALLY) { + if (const auto params = Ssl::VerifyCallbackParameters::Find(*ssl)) { + if (params->callerHandlesMissingCertificates) { + debugs(83, 3, "hiding X509_V_ERR_UNABLE_TO_GET_ISSUER_CERT_LOCALLY"); + params->hidMissingIssuer = true; + ok = 1; + } + } + } + if (!ok) { Security::CertPointer broken_cert; broken_cert.resetAndLock(X509_STORE_CTX_get_current_cert(ctx)); @@ -425,6 +436,126 @@ Ssl::DisablePeerVerification(Security::ContextPointer &ctx) SSL_CTX_set_verify(ctx.get(),SSL_VERIFY_NONE,nullptr); } +static int VerifyCtxCertificates(X509_STORE_CTX *ctx, STACK_OF(X509) *extraCerts); + +bool +Ssl::VerifyConnCertificates(Security::Connection &sconn, const Ssl::X509_STACK_Pointer &extraCerts) +{ + const auto peerCertificatesChain = SSL_get_peer_cert_chain(&sconn); + + // TODO: Replace debugs/return false with returning ErrorDetail::Pointer. + // Using Security::ErrorDetail terminology, errors in _this_ function are + // "non-validation errors", but VerifyCtxCertificates() errors may be + // "certificate validation errors". Callers detail SQUID_TLS_ERR_CONNECT. + // Some details should be created right here. Others extracted from OpenSSL. + // Why not throw? Most of the reasons detailed in the following commit apply + // here as well: https://github.com/measurement-factory/squid/commit/e862d33 + + if (!peerCertificatesChain || sk_X509_num(peerCertificatesChain) == 0) { + debugs(83, 2, "no server certificates"); + return false; + } + + const auto verificationStore = SSL_CTX_get_cert_store(SSL_get_SSL_CTX(&sconn)); + if (!verificationStore) { + debugs(83, 2, "no certificate store"); + return false; + } + + const X509_STORE_CTX_Pointer storeCtx(X509_STORE_CTX_new()); + if (!storeCtx) { + debugs(83, 2, "cannot create X509_STORE_CTX; likely OOM"); + return false; + } + + const auto peerCert = sk_X509_value(peerCertificatesChain, 0); + if (!X509_STORE_CTX_init(storeCtx.get(), verificationStore, peerCert, peerCertificatesChain)) { + debugs(83, 2, "cannot initialize X509_STORE_CTX"); + return false; + } + +#if defined(SSL_CERT_FLAG_SUITEB_128_LOS) + // overwrite context Suite B (RFC 5759) flags with connection non-defaults + // SSL_set_cert_flags() return type is long, but its implementation actually + // returns an unsigned long flags value expected by X509_STORE_CTX_set_flags + const unsigned long certFlags = SSL_set_cert_flags(&sconn, 0); + if (const auto suiteBflags = certFlags & SSL_CERT_FLAG_SUITEB_128_LOS) + X509_STORE_CTX_set_flags(storeCtx.get(), suiteBflags); +#endif + + if (!X509_STORE_CTX_set_ex_data(storeCtx.get(), SSL_get_ex_data_X509_STORE_CTX_idx(), &sconn)) { + debugs(83, 2, "cannot attach SSL object to X509_STORE_CTX"); + return false; + } + + // If we ever add DANE support to Squid, we will supply DANE details here: + // X509_STORE_CTX_set0_dane(storeCtx.get(), SSL_get0_dane(&sconn)); + + // tell OpenSSL we are verifying a server certificate + if (!X509_STORE_CTX_set_default(storeCtx.get(), "ssl_server")) { + debugs(83, 2, "cannot set default verification method to ssl_server"); + return false; + } + + // overwrite context "verification parameters" with connection non-defaults + const auto param = X509_STORE_CTX_get0_param(storeCtx.get()); + if (!param) { + debugs(83, 2, "no context verification parameters"); + return false; + } +#if defined(HAVE_X509_VERIFY_PARAM_SET_AUTH_LEVEL) + X509_VERIFY_PARAM_set_auth_level(param, SSL_get_security_level(&sconn)); +#endif + if (!X509_VERIFY_PARAM_set1(param, SSL_get0_param(&sconn))) { + debugs(83, 2, "cannot overwrite context verification parameters"); + return false; + } + + // copy any connection "verify_callback function" to the validation context + // (\ref OpenSSL_vcb_disambiguation) + if (const auto cb = SSL_get_verify_callback(&sconn)) + X509_STORE_CTX_set_verify_cb(storeCtx.get(), cb); + + // verify the server certificate chain in the prepared validation context + if (VerifyCtxCertificates(storeCtx.get(), extraCerts.get()) <= 0) { + // see also: ssl_verify_cb() details errors via ssl_ex_index_ssl_errors + const auto verifyResult = X509_STORE_CTX_get_error(storeCtx.get()); + debugs(83, 3, "verification failure: " << verifyResult << ' ' << X509_verify_cert_error_string(verifyResult)); + return false; + } + + debugs(83, 7, "success"); + return true; +} + +/* Ssl::VerifyCallbackParameters */ + +Ssl::VerifyCallbackParameters * +Ssl::VerifyCallbackParameters::Find(Security::Connection &sconn) +{ + return static_cast(SSL_get_ex_data(&sconn, ssl_ex_index_verify_callback_parameters)); +} + +Ssl::VerifyCallbackParameters * +Ssl::VerifyCallbackParameters::New(Security::Connection &sconn) +{ + Must(!Find(sconn)); + const auto parameters = new VerifyCallbackParameters(); + if (!SSL_set_ex_data(&sconn, ssl_ex_index_verify_callback_parameters, parameters)) { + delete parameters; + throw TextException("SSL_set_ex_data() failed; likely OOM", Here()); + } + return parameters; +} + +Ssl::VerifyCallbackParameters & +Ssl::VerifyCallbackParameters::At(Security::Connection &sconn) +{ + const auto parameters = Find(sconn); + Must(parameters); + return *parameters; +} + // "dup" function for SSL_get_ex_new_index("cert_err_check") #if SQUID_USE_CONST_CRYPTO_EX_DATA_DUP static int @@ -505,6 +636,14 @@ ssl_free_SBuf(void *, void *ptr, CRYPTO_EX_DATA *, delete buf; } +/// "free" function for the ssl_ex_index_verify_callback_parameters entry +static void +ssl_free_VerifyCallbackParameters(void *, void *ptr, CRYPTO_EX_DATA *, + int, long, void *) +{ + delete static_cast(ptr); +} + void Ssl::Initialize(void) { @@ -545,7 +684,7 @@ Ssl::Initialize(void) ssl_ex_index_ssl_errors = SSL_get_ex_new_index(0, (void *) "ssl_errors", NULL, NULL, &ssl_free_SslErrors); ssl_ex_index_ssl_cert_chain = SSL_get_ex_new_index(0, (void *) "ssl_cert_chain", NULL, NULL, &ssl_free_CertChain); ssl_ex_index_ssl_validation_counter = SSL_get_ex_new_index(0, (void *) "ssl_validation_counter", NULL, NULL, &ssl_free_int); - ssl_ex_index_ssl_untrusted_chain = SSL_get_ex_new_index(0, (void *) "ssl_untrusted_chain", NULL, NULL, &ssl_free_CertChain); + ssl_ex_index_verify_callback_parameters = SSL_get_ex_new_index(0, (void *) "verify_callback_parameters", nullptr, nullptr, &ssl_free_VerifyCallbackParameters); } bool @@ -937,8 +1076,8 @@ Ssl::setClientSNI(SSL *ssl, const char *fqdn) #endif } -static const char * -hasAuthorityInfoAccessCaIssuers(X509 *cert) +const char * +Ssl::findIssuerUri(X509 *cert) { AUTHORITY_INFO_ACCESS *info; if (!cert) @@ -1010,170 +1149,196 @@ findCertIssuerFast(Ssl::CertsIndexedList &list, X509 *cert) } /// slowly find the issuer certificate of a given cert using linear search -static bool -findCertIssuer(Security::CertList const &list, X509 *cert) +static X509 * +sk_x509_findIssuer(const STACK_OF(X509) *sk, X509 *cert) { - for (Security::CertList::const_iterator it = list.begin(); it != list.end(); ++it) { - if (X509_check_issued(it->get(), cert) == X509_V_OK) - return true; + if (!sk) + return nullptr; + + const auto certCount = sk_X509_num(sk); + for (int i = 0; i < certCount; ++i) { + const auto issuer = sk_X509_value(sk, i); + if (X509_check_issued(issuer, cert) == X509_V_OK) + return issuer; } - return false; + return nullptr; } -/// \return true if the cert issuer exist in the certificates stored in connContext -static bool -issuerExistInCaDb(X509 *cert, const Security::ContextPointer &connContext) +/// finds issuer of a given certificate in CA store of the given connContext +/// \returns the cert issuer (after increasing its reference count) or nil +static X509 * +findIssuerInCaDb(X509 *cert, const Security::ContextPointer &connContext) { if (!connContext) - return false; + return nullptr; X509_STORE_CTX *storeCtx = X509_STORE_CTX_new(); if (!storeCtx) { debugs(83, DBG_IMPORTANT, "Failed to allocate STORE_CTX object"); - return false; + return nullptr; } - bool gotIssuer = false; + X509 *issuer = nullptr; X509_STORE *store = SSL_CTX_get_cert_store(connContext.get()); if (X509_STORE_CTX_init(storeCtx, store, nullptr, nullptr)) { - X509 *issuer = nullptr; - gotIssuer = (X509_STORE_CTX_get1_issuer(&issuer, storeCtx, cert) > 0); - if (issuer) - X509_free(issuer); + const auto ret = X509_STORE_CTX_get1_issuer(&issuer, storeCtx, cert); + if (ret > 0) { + assert(issuer); + char buffer[256]; + debugs(83, 5, "found " << X509_NAME_oneline(X509_get_subject_name(issuer), buffer, sizeof(buffer))); + } else { + debugs(83, ret < 0 ? 2 : 3, "not found or failure: " << ret); + assert(!issuer); + } } else { const auto ssl_error = ERR_get_error(); debugs(83, DBG_IMPORTANT, "Failed to initialize STORE_CTX object: " << Security::ErrorString(ssl_error)); } + X509_STORE_CTX_free(storeCtx); - return gotIssuer; + return issuer; } -const char * -Ssl::uriOfIssuerIfMissing(X509 *cert, Security::CertList const &serverCertificates, const Security::ContextPointer &context) +Security::CertPointer +Ssl::findIssuerCertificate(X509 *cert, const STACK_OF(X509) *serverCertificates, const Security::ContextPointer &context) { - if (!cert || !serverCertificates.size()) - return nullptr; + Must(cert); - if (!findCertIssuer(serverCertificates, cert)) { - //if issuer is missing - if (const char *issuerUri = hasAuthorityInfoAccessCaIssuers(cert)) { - // There is a URI where we can download a certificate. - if (!findCertIssuerFast(SquidUntrustedCerts, cert) && - !issuerExistInCaDb(cert, context)) { - // and issuer not found in local databases containing - // untrusted certificates and trusted CA certificates - return issuerUri; - } - } + // check certificate chain, if any + if (const auto issuer = serverCertificates ? sk_x509_findIssuer(serverCertificates, cert) : nullptr) { + X509_up_ref(issuer); + return Security::CertPointer(issuer); } - return nullptr; -} -void -Ssl::missingChainCertificatesUrls(std::queue &URIs, Security::CertList const &serverCertificates, const Security::ContextPointer &context) -{ - if (!serverCertificates.size()) - return; - - for (const auto &i : serverCertificates) { - if (const char *issuerUri = uriOfIssuerIfMissing(i.get(), serverCertificates, context)) - URIs.push(SBuf(issuerUri)); + // check untrusted certificates + if (const auto issuer = findCertIssuerFast(SquidUntrustedCerts, cert)) { + X509_up_ref(issuer); + return Security::CertPointer(issuer); } -} -void -Ssl::SSL_add_untrusted_cert(SSL *ssl, X509 *cert) -{ - STACK_OF(X509) *untrustedStack = static_cast (SSL_get_ex_data(ssl, ssl_ex_index_ssl_untrusted_chain)); - if (!untrustedStack) { - untrustedStack = sk_X509_new_null(); - if (!SSL_set_ex_data(ssl, ssl_ex_index_ssl_untrusted_chain, untrustedStack)) { - sk_X509_pop_free(untrustedStack, X509_free); - throw TextException("Failed to attach untrusted certificates chain", Here()); - } + // check trusted CA certificates + if (const auto issuer = findIssuerInCaDb(cert, context)) { + // no X509_up_ref(issuer) because findIssuerInCaDb() ups reference count + return Security::CertPointer(issuer); } - sk_X509_push(untrustedStack, cert); + + return Security::CertPointer(nullptr); } -/// Search for the issuer certificate of cert in sk list. -static X509 * -sk_x509_findIssuer(STACK_OF(X509) *sk, X509 *cert) +bool +Ssl::missingChainCertificatesUrls(std::queue &URIs, const STACK_OF(X509) &serverCertificates, const Security::ContextPointer &context) { - if (!sk) - return NULL; + for (int i = 0; i < sk_X509_num(&serverCertificates); ++i) { + const auto cert = sk_X509_value(&serverCertificates, i); - const int skItemsNum = sk_X509_num(sk); - for (int i = 0; i < skItemsNum; ++i) { - X509 *issuer = sk_X509_value(sk, i); - if (X509_check_issued(issuer, cert) == X509_V_OK) - return issuer; + if (findIssuerCertificate(cert, &serverCertificates, context)) + continue; + + if (const auto issuerUri = findIssuerUri(cert)) { + URIs.push(SBuf(issuerUri)); + } else { + static char name[2048]; + debugs(83, 3, "Issuer certificate for " << + X509_NAME_oneline(X509_get_subject_name(cert), name, sizeof(name)) << + " is missing and its URI is not provided"); + } } - return NULL; + + debugs(83, (URIs.empty() ? 3 : 5), "found: " << URIs.size()); + return !URIs.empty(); } /// add missing issuer certificates to untrustedCerts static void -completeIssuers(X509_STORE_CTX *ctx, STACK_OF(X509) *untrustedCerts) +completeIssuers(X509_STORE_CTX *ctx, STACK_OF(X509) &untrustedCerts) { - debugs(83, 2, "completing " << sk_X509_num(untrustedCerts) << " OpenSSL untrusted certs using " << SquidUntrustedCerts.size() << " configured untrusted certificates"); + debugs(83, 2, "completing " << sk_X509_num(&untrustedCerts) << + " OpenSSL untrusted certs using " << SquidUntrustedCerts.size() << + " configured untrusted certificates"); const X509_VERIFY_PARAM *param = X509_STORE_CTX_get0_param(ctx); int depth = X509_VERIFY_PARAM_get_depth(param); - X509 *current = X509_STORE_CTX_get0_cert(ctx); + Security::CertPointer current; + current.resetAndLock(X509_STORE_CTX_get0_cert(ctx)); int i = 0; for (i = 0; current && (i < depth); ++i) { - if (X509_check_issued(current, current) == X509_V_OK) { + if (X509_check_issued(current.get(), current.get()) == X509_V_OK) { // either ctx->cert is itself self-signed or untrustedCerts // already contain the self-signed current certificate break; } // untrustedCerts is short, not worth indexing - X509 *issuer = sk_x509_findIssuer(untrustedCerts, current); - if (!issuer) { - if ((issuer = findCertIssuerFast(SquidUntrustedCerts, current))) - sk_X509_push(untrustedCerts, issuer); - } + const Security::ContextPointer nullCtx; + auto issuer = Ssl::findIssuerCertificate(current.get(), &untrustedCerts, nullCtx); current = issuer; + if (issuer) + sk_X509_push(&untrustedCerts, issuer.release()); } if (i >= depth) debugs(83, 2, "exceeded the maximum certificate chain length: " << depth); } -/// OpenSSL certificate validation callback. +/// Validates certificates while consulting sslproxy_foreign_intermediate_certs +/// and, optionally, the given extra certificates. +/// \returns whatever OpenSSL X509_verify_cert() returns static int -untrustedToStoreCtx_cb(X509_STORE_CTX *ctx,void *data) +VerifyCtxCertificates(X509_STORE_CTX *ctx, STACK_OF(X509) *extraCerts) { - debugs(83, 4, "Try to use pre-downloaded intermediate certificates"); - - SSL *ssl = static_cast(X509_STORE_CTX_get_ex_data(ctx, SSL_get_ex_data_X509_STORE_CTX_idx())); - STACK_OF(X509) *sslUntrustedStack = static_cast (SSL_get_ex_data(ssl, ssl_ex_index_ssl_untrusted_chain)); - // OpenSSL already maintains ctx->untrusted but we cannot modify // internal OpenSSL list directly. We have to give OpenSSL our own // list, but it must include certificates on the OpenSSL ctx->untrusted STACK_OF(X509) *oldUntrusted = X509_STORE_CTX_get0_untrusted(ctx); - STACK_OF(X509) *sk = sk_X509_dup(oldUntrusted); // oldUntrusted is always not NULL - - for (int i = 0; i < sk_X509_num(sslUntrustedStack); ++i) { - X509 *cert = sk_X509_value(sslUntrustedStack, i); - sk_X509_push(sk, cert); + // X509_chain_up_ref() increments cert references _and_ dupes the stack + Ssl::X509_STACK_Pointer untrustedCerts(oldUntrusted ? X509_chain_up_ref(oldUntrusted) : sk_X509_new_null()); + + if (extraCerts) { + for (int i = 0; i < sk_X509_num(extraCerts); ++i) { + const auto cert = sk_X509_value(extraCerts, i); + X509_up_ref(cert); + sk_X509_push(untrustedCerts.get(), cert); + } } // If the local untrusted certificates internal database is used // run completeIssuers to add missing certificates if possible. if (SquidUntrustedCerts.size() > 0) - completeIssuers(ctx, sk); + completeIssuers(ctx, *untrustedCerts); - X509_STORE_CTX_set0_untrusted(ctx, sk); // No locking/unlocking, just sets ctx->untrusted + X509_STORE_CTX_set0_untrusted(ctx, untrustedCerts.get()); // No locking/unlocking, just sets ctx->untrusted int ret = X509_verify_cert(ctx); X509_STORE_CTX_set0_untrusted(ctx, oldUntrusted); // Set back the old untrusted list - sk_X509_free(sk); // Release sk list return ret; } +/// \interface OpenSSL_vcb_disambiguation +/// +/// OpenSSL has two very different concepts with nearly identical names: +/// +/// a) A (replaceable) certificate verification function -- X509_verify_cert(): +/// This function drives the entire certificate verification algorithm. +/// It can be called directly, but is usually called during SSL_connect(). +/// OpenSSL calls this function a "verification callback function". +/// SSL_CTX_set_cert_verify_callback(3) replaces X509_verify_cert() default. +/// +/// b) An (optional) certificate verification adjustment callback: +/// This function, if set, is called at the end of (a) to adjust (a) results. +/// It is never called directly, only from (a). +/// OpenSSL calls this function a "verify_callback function". +/// The SSL_CTX_set_verify(3) family of functions sets this function. + +/// Validates certificates while consulting sslproxy_foreign_intermediate_certs +/// but without using any dynamically downloaded intermediate certificates. +/// OpenSSL "verification callback function" (\ref OpenSSL_vcb_disambiguation) +static int +untrustedToStoreCtx_cb(X509_STORE_CTX *ctx, void *) +{ + debugs(83, 4, "Try to use pre-downloaded intermediate certificates"); + return VerifyCtxCertificates(ctx, nullptr); +} + void Ssl::useSquidUntrusted(SSL_CTX *sslContext) { diff --git a/src/ssl/support.h b/src/ssl/support.h index 30833874a3..7bfb998ad5 100644 --- a/src/ssl/support.h +++ b/src/ssl/support.h @@ -168,17 +168,20 @@ void unloadSquidUntrusted(); */ void SSL_add_untrusted_cert(SSL *ssl, X509 *cert); -/** - * Searches in serverCertificates list for the cert issuer and if not found - * and Authority Info Access of cert provides a URI return it. - */ -const char *uriOfIssuerIfMissing(X509 *cert, Security::CertList const &serverCertificates, const Security::ContextPointer &context); +/// finds certificate issuer URI in the Authority Info Access extension +const char *findIssuerUri(X509 *cert); + +/// Searches serverCertificates and local databases for the cert issuer. +/// \param context where to retrieve the configured CA's db; may be nil +/// \returns the found issuer certificate or nil +Security::CertPointer findIssuerCertificate(X509 *cert, const STACK_OF(X509) *serverCertificates, const Security::ContextPointer &context); /** * Fill URIs queue with the uris of missing certificates from serverCertificate chain * if this information provided by Authority Info Access. + \return whether at least one URI is known, including previously known ones */ -void missingChainCertificatesUrls(std::queue &URIs, Security::CertList const &serverCertificates, const Security::ContextPointer &context); +bool missingChainCertificatesUrls(std::queue &URIs, const STACK_OF(X509) &serverCertificates, const Security::ContextPointer &context); /** \ingroup ServerProtocolSSLAPI @@ -319,6 +322,48 @@ void InRamCertificateDbKey(const Ssl::CertificateProperties &certProperties, SBu TODO: Add support for reading from `buf`. */ BIO *BIO_new_SBuf(SBuf *buf); + +/// Validates the given TLS connection server certificate chain in conjunction +/// with a (possibly empty) set of "extra" intermediate certs. Also consults +/// sslproxy_foreign_intermediate_certs. This is a C++/Squid-friendly wrapper of +/// OpenSSL "verification callback function" (\ref OpenSSL_vcb_disambiguation). +/// OpenSSL has a similar wrapper, ssl_verify_cert_chain(), but that wrapper is +/// not a part of the public OpenSSL API. +bool VerifyConnCertificates(Security::Connection &, const Ssl::X509_STACK_Pointer &extraCerts); + +// TODO: Move other ssl_ex_index_* validation-related information here. +/// OpenSSL "verify_callback function" input/output parameters. This information +/// cannot be passed through the verification API directly, so it is aggregated +/// in this class and exchanged via ssl_ex_index_verify_callback_parameters. For +/// OpenSSL validation callback details, see \ref OpenSSL_vcb_disambiguation. +class VerifyCallbackParameters { +public: + /// creates a VerifyCallbackParameters object and adds it to the given TLS connection + /// \returns the successfully created and added object + static VerifyCallbackParameters *New(Security::Connection &); + + /// \returns the VerifyCallbackParameters object previously attached via New() + static VerifyCallbackParameters &At(Security::Connection &); + + /// \returns the VerifyCallbackParameters object previously attached via New() or nil + static VerifyCallbackParameters *Find(Security::Connection &); + + /* input parameters */ + + /// whether X509_V_ERR_UNABLE_TO_GET_ISSUER_CERT_LOCALLY should be cleared + /// (after setting hidMissingIssuer) because the validation initiator wants + /// to get the missing certificates and redo the validation with them + bool callerHandlesMissingCertificates = false; + + /* output parameters */ + + /// whether certificate validation has failed due to missing certificate(s) + /// (i.e. X509_V_ERR_UNABLE_TO_GET_ISSUER_CERT_LOCALLY), but the failure was + /// cleared/hidden due to true callerHandlesMissingCertificates setting; the + /// certificate chain has to be deemed untrusted until revalidation (if any) + bool hidMissingIssuer = false; +}; + } //namespace Ssl #if _SQUID_WINDOWS_ diff --git a/src/tests/stub_libsecurity.cc b/src/tests/stub_libsecurity.cc index 6a2e2091a1..80eeb6506b 100644 --- a/src/tests/stub_libsecurity.cc +++ b/src/tests/stub_libsecurity.cc @@ -63,11 +63,13 @@ const char *Security::NegotiationHistory::cipherName() const STUB const char *Security::NegotiationHistory::printTlsVersion(AnyP::ProtocolVersion const &v) const STUB #include "security/PeerConnector.h" +class TlsNegotiationDetails: public RefCountable {}; CBDATA_NAMESPACED_CLASS_INIT(Security, PeerConnector); namespace Security { PeerConnector::PeerConnector(const Comm::ConnectionPointer &, AsyncCall::Pointer &, const AccessLogEntryPointer &, const time_t) : AsyncJob("Security::PeerConnector") {STUB} +PeerConnector::~PeerConnector() STUB void PeerConnector::start() STUB bool PeerConnector::doneAll() const STUB_RETVAL(true) void PeerConnector::swanSong() STUB @@ -77,7 +79,7 @@ void PeerConnector::commTimeoutHandler(const CommTimeoutCbParams &) STUB bool PeerConnector::initialize(Security::SessionPointer &) STUB_RETVAL(false) void PeerConnector::negotiate() STUB bool PeerConnector::sslFinalized() STUB_RETVAL(false) -void PeerConnector::handleNegotiateError(const int) STUB +void PeerConnector::handleNegotiationResult(const Security::IoResult &) STUB; void PeerConnector::noteWantRead() STUB void PeerConnector::noteWantWrite() STUB void PeerConnector::noteNegotiationError(const Security::ErrorDetailPointer &) STUB