CA fetcher (Windows) - relax the logic a bit

In case a certificate chain is missing an intermediate,
for a certificate having "Authority Information Access"
extension it's possible to fetch this intermediate and
build the chain up to the trusted root. Unfortunately,
it's not always possible to install the root certificate
in the system "ROOT" store and then an application
wants to set it in the socket's configuration,
using setCaCertificates(). But this call also
disables CA fetcher ('no on demand root loading').
It makes sense to relax this logic for such
certificates and try to fetch the intermediate CA
and then have the complete chain verified.

Pick-to: 5.15
Fixes: QTBUG-84173
Change-Id: I5b9b4271767eba6f5fd2b5cf05e942360c6aa245
Reviewed-by: Timur Pocheptsov <timur.pocheptsov@qt.io>
Reviewed-by: Mårten Nordheim <marten.nordheim@qt.io>

8 files changed, 252 insertions, 58 deletions

diff --git a/src/network/ssl/qsslcertificate_openssl.cpp b/src/network/ssl/qsslcertificate_openssl.cpp
index 2bb7c930f2..cd178313b7 100644
--- a/src/network/ssl/qsslcertificate_openssl.cpp
+++ b/src/network/ssl/qsslcertificate_openssl.cpp
@@ -528,6 +528,9 @@ QList<QSslCertificateExtension> QSslCertificate::extensions() const
         result << QSslCertificatePrivate::convertExtension(ext);
     }
 
+    // Converting an extension may result in an error(s), clean them up.
+    Q_UNUSED(QSslSocketBackendPrivate::getErrorsFromOpenSsl());
+
     return result;
 }
 
diff --git a/src/network/ssl/qsslsocket.cpp b/src/network/ssl/qsslsocket.cpp
index 667a2e7267..4b6d8c21d1 100644
--- a/src/network/ssl/qsslsocket.cpp
+++ b/src/network/ssl/qsslsocket.cpp
@@ -2326,6 +2326,7 @@ void QSslSocketPrivate::init()
     writeBuffer.clear();
     configuration.peerCertificate.clear();
     configuration.peerCertificateChain.clear();
+    fetchAuthorityInformation = false;
 }
 
 /*!
diff --git a/src/network/ssl/qsslsocket_openssl.cpp b/src/network/ssl/qsslsocket_openssl.cpp
index 4c96c02ade..9d95a58118 100644
--- a/src/network/ssl/qsslsocket_openssl.cpp
+++ b/src/network/ssl/qsslsocket_openssl.cpp
@@ -134,6 +134,49 @@ QAlertType tlsAlertType(int value)
     return QAlertType(value & 0xff);
 }
 
+#ifdef Q_OS_WIN
+
+QSslCertificate findCertificateToFetch(const QVector<QSslError> &tlsErrors, bool checkAIA)
+{
+    QSslCertificate certToFetch;
+
+    for (const auto &tlsError : tlsErrors) {
+        switch (tlsError.error()) {
+        case QSslError::UnableToGetLocalIssuerCertificate: // site presented intermediate cert, but root is unknown
+        case QSslError::SelfSignedCertificateInChain: // site presented a complete chain, but root is unknown
+            certToFetch = tlsError.certificate();
+            break;
+        case QSslError::SelfSignedCertificate:
+        case QSslError::CertificateBlacklisted:
+            //With these errors, we know it will be untrusted so save time by not asking windows
+            return QSslCertificate{};
+        default:
+#ifdef QSSLSOCKET_DEBUG
+            qCDebug(lcSsl) << tlsError.errorString();
+#endif
+            //TODO - this part is strange.
+            break;
+        }
+    }
+
+    if (checkAIA) {
+        const auto extensions = certToFetch.extensions();
+        for (const auto &ext : extensions) {
+            if (ext.oid() == QStringLiteral("1.3.6.1.5.5.7.1.1")) // See RFC 4325
+                return certToFetch;
+        }
+        //The only reason we check this extensions is because an application set trusted
+        //CA certificates explicitly, thus technically disabling CA fetch. So, if it's
+        //the case and an intermediate certificate is missing, and no extensions is
+        //present on the leaf certificate - we fail the handshake immediately.
+        return QSslCertificate{};
+    }
+
+    return certToFetch;
+}
+
+#endif // Q_OS_WIN
+
 } // Unnamed namespace
 
 extern "C"
@@ -1489,40 +1532,27 @@ bool QSslSocketBackendPrivate::startHandshake()
         sslErrors = errors;
 
 #ifdef Q_OS_WIN
+        const bool fetchEnabled = s_loadRootCertsOnDemand
+                                  && allowRootCertOnDemandLoading;
+        // !fetchEnabled is a special case scenario, when we potentially have a missing
+        // intermediate certificate and a recoverable chain, but on demand cert loading
+        // was disabled by setCaCertificates call. For this scenario we check if "Authority
+        // Information Access" is present - wincrypt can deal with such certificates.
+        QSslCertificate certToFetch;
+        if (doVerifyPeer && !verifyErrorsHaveBeenIgnored())
+            certToFetch = findCertificateToFetch(sslErrors, !fetchEnabled);
+
         //Skip this if not using system CAs, or if the SSL errors are configured in advance to be ignorable
-        if (doVerifyPeer
-            && s_loadRootCertsOnDemand
-            && allowRootCertOnDemandLoading
-            && !verifyErrorsHaveBeenIgnored()) {
-            //Windows desktop versions starting from vista ship with minimal set of roots
-            //and download on demand from the windows update server CA roots that are
-            //trusted by MS.
+        if (!certToFetch.isNull()) {
+            fetchAuthorityInformation = !fetchEnabled;
+            //Windows desktop versions starting from vista ship with minimal set of roots and download on demand
+            //from the windows update server CA roots that are trusted by MS. It also can fetch a missing intermediate
+            //in case "Authority Information Access" extension is present.
+            //
             //However, this is only transparent if using WinINET - we have to trigger it
             //ourselves.
-            QSslCertificate certToFetch;
-            bool fetchCertificate = true;
-            for (int i=0; i< sslErrors.count(); i++) {
-                switch (sslErrors.at(i).error()) {
-                case QSslError::UnableToGetLocalIssuerCertificate: // site presented intermediate cert, but root is unknown
-                case QSslError::SelfSignedCertificateInChain: // site presented a complete chain, but root is unknown
-                    certToFetch = sslErrors.at(i).certificate();
-                    break;
-                case QSslError::SelfSignedCertificate:
-                case QSslError::CertificateBlacklisted:
-                    //With these errors, we know it will be untrusted so save time by not asking windows
-                    fetchCertificate = false;
-                    break;
-                default:
-#ifdef QSSLSOCKET_DEBUG
-                    qCDebug(lcSsl) << sslErrors.at(i).errorString();
-#endif
-                    break;
-                }
-            }
-            if (fetchCertificate && !certToFetch.isNull()) {
-                fetchCaRootForCert(certToFetch);
-                return false;
-            }
+            fetchCaRootForCert(certToFetch);
+            return false;
         }
 #endif
         if (!checkSslErrors())
@@ -1699,7 +1729,11 @@ void QSslSocketBackendPrivate::fetchCaRootForCert(const QSslCertificate &cert)
     Q_Q(QSslSocket);
     //The root certificate is downloaded from windows update, which blocks for 15 seconds in the worst case
     //so the request is done in a worker thread.
-    QWindowsCaRootFetcher *fetcher = new QWindowsCaRootFetcher(cert, mode);
+    QList<QSslCertificate> customRoots;
+    if (fetchAuthorityInformation)
+        customRoots = configuration.caCertificates;
+
+    QWindowsCaRootFetcher *fetcher = new QWindowsCaRootFetcher(cert, mode, customRoots, q->peerVerifyName());
     QObject::connect(fetcher, SIGNAL(finished(QSslCertificate,QSslCertificate)), q, SLOT(_q_caRootLoaded(QSslCertificate,QSslCertificate)), Qt::QueuedConnection);
     QMetaObject::invokeMethod(fetcher, "start", Qt::QueuedConnection);
     pauseSocketNotifiers(q);
@@ -1709,6 +1743,12 @@ void QSslSocketBackendPrivate::fetchCaRootForCert(const QSslCertificate &cert)
 //This is the callback from QWindowsCaRootFetcher, trustedRoot will be invalid (default constructed) if it failed.
 void QSslSocketBackendPrivate::_q_caRootLoaded(QSslCertificate cert, QSslCertificate trustedRoot)
 {
+    if (fetchAuthorityInformation) {
+        if (!configuration.caCertificates.contains(trustedRoot))
+            trustedRoot = QSslCertificate{};
+        fetchAuthorityInformation = false;
+    }
+
     if (!trustedRoot.isNull() && !trustedRoot.isBlacklisted()) {
         if (s_loadRootCertsOnDemand) {
             //Add the new root cert to default cert list for use by future sockets
@@ -1735,6 +1775,7 @@ void QSslSocketBackendPrivate::_q_caRootLoaded(QSslCertificate cert, QSslCertifi
             }
         }
     }
+
     // Continue with remaining errors
     if (plainSocket)
         plainSocket->resume();
@@ -2249,14 +2290,23 @@ QList<QSslCertificate> QSslSocketBackendPrivate::STACKOFX509_to_QSslCertificates
     return certificates;
 }
 
-QList<QSslError> QSslSocketBackendPrivate::verify(const QList<QSslCertificate> &certificateChain, const QString &hostName)
+QList<QSslError> QSslSocketBackendPrivate::verify(const QList<QSslCertificate> &certificateChain,
+                                                  const QString &hostName)
 {
-    QList<QSslError> errors;
-    if (certificateChain.count() <= 0) {
-        errors << QSslError(QSslError::UnspecifiedError);
-        return errors;
-    }
+    if (s_loadRootCertsOnDemand)
+        setDefaultCaCertificates(defaultCaCertificates() + systemCaCertificates());
+
+    return verify(QSslConfiguration::defaultConfiguration().caCertificates(), certificateChain, hostName);
+}
 
+QList<QSslError> QSslSocketBackendPrivate::verify(const QList<QSslCertificate> &caCertificates,
+                                                  const QList<QSslCertificate> &certificateChain,
+                                                  const QString &hostName)
+{
+    if (certificateChain.count() <= 0)
+        return {QSslError(QSslError::UnspecifiedError)};
+
+    QList<QSslError> errors;
     // Setup the store with the default CA certificates
     X509_STORE *certStore = q_X509_STORE_new();
     if (!certStore) {
@@ -2266,12 +2316,7 @@ QList<QSslError> QSslSocketBackendPrivate::verify(const QList<QSslCertificate> &
     }
     const std::unique_ptr<X509_STORE, decltype(&q_X509_STORE_free)> storeGuard(certStore, q_X509_STORE_free);
 
-    if (s_loadRootCertsOnDemand) {
-        setDefaultCaCertificates(defaultCaCertificates() + systemCaCertificates());
-    }
-
     const QDateTime now = QDateTime::currentDateTimeUtc();
-    const auto caCertificates = QSslConfiguration::defaultConfiguration().caCertificates();
     for (const QSslCertificate &caCertificate : caCertificates) {
         // From https://www.openssl.org/docs/ssl/SSL_CTX_load_verify_locations.html:
         //
diff --git a/src/network/ssl/qsslsocket_openssl_p.h b/src/network/ssl/qsslsocket_openssl_p.h
index 04feec9f45..de1dcfc91a 100644
--- a/src/network/ssl/qsslsocket_openssl_p.h
+++ b/src/network/ssl/qsslsocket_openssl_p.h
@@ -181,6 +181,8 @@ public:
     static QSslCipher QSslCipher_from_SSL_CIPHER(const SSL_CIPHER *cipher);
     static QList<QSslCertificate> STACKOFX509_to_QSslCertificates(STACK_OF(X509) *x509);
     static QList<QSslError> verify(const QList<QSslCertificate> &certificateChain, const QString &hostName);
+    static QList<QSslError> verify(const QList<QSslCertificate> &cas, const QList<QSslCertificate> &certificateChain,
+                                   const QString &hostName);
     static QString getErrorsFromOpenSsl();
     static void logAndClearErrorQueue();
     static bool importPkcs12(QIODevice *device,
diff --git a/src/network/ssl/qsslsocket_p.h b/src/network/ssl/qsslsocket_p.h
index 25625a1313..971650a71c 100644
--- a/src/network/ssl/qsslsocket_p.h
+++ b/src/network/ssl/qsslsocket_p.h
@@ -74,6 +74,7 @@ class QSslContext;
 #include <CoreFoundation/CFArray.h>
 #elif defined(Q_OS_WIN)
 #include <QtCore/qt_windows.h>
+#include <memory>
 #ifndef Q_OS_WINRT
 #include <wincrypt.h>
 #endif // !Q_OS_WINRT
@@ -90,6 +91,20 @@ QT_BEGIN_NAMESPACE
     typedef OSStatus (*PtrSecTrustCopyAnchorCertificates)(CFArrayRef*);
 #endif
 
+#if defined(Q_OS_WIN)
+
+// Those are needed by both OpenSSL and SChannel back-ends on Windows:
+struct QHCertStoreDeleter {
+    void operator()(HCERTSTORE store)
+    {
+        CertCloseStore(store, 0);
+    }
+};
+
+using QHCertStorePointer = std::unique_ptr<void, QHCertStoreDeleter>;
+
+#endif // Q_OS_WIN
+
 class QSslSocketPrivate : public QTcpSocketPrivate
 {
     Q_DECLARE_PUBLIC(QSslSocket)
@@ -208,6 +223,7 @@ protected:
     bool systemOrSslErrorDetected = false;
     QVector<QOcspResponse> ocspResponses;
     bool handshakeInterrupted = false;
+    bool fetchAuthorityInformation = false;
 };
 
 #if QT_CONFIG(securetransport) || QT_CONFIG(schannel)
diff --git a/src/network/ssl/qsslsocket_schannel_p.h b/src/network/ssl/qsslsocket_schannel_p.h
index a184deef49..fe29dadec0 100644
--- a/src/network/ssl/qsslsocket_schannel_p.h
+++ b/src/network/ssl/qsslsocket_schannel_p.h
@@ -62,18 +62,8 @@ QT_REQUIRE_CONFIG(schannel);
 #include <schnlsp.h>
 #undef SECURITY_WIN32
 
-#include <memory>
-
 QT_BEGIN_NAMESPACE
 
-struct QHCertStoreDeleter {
-    void operator()(HCERTSTORE store)
-    {
-        CertCloseStore(store, 0);
-    }
-};
-typedef std::unique_ptr<void, QHCertStoreDeleter> QHCertStorePointer;
-
 class QSslSocketBackendPrivate final : public QSslSocketPrivate
 {
     Q_DISABLE_COPY_MOVE(QSslSocketBackendPrivate)
diff --git a/src/network/ssl/qwindowscarootfetcher.cpp b/src/network/ssl/qwindowscarootfetcher.cpp
index f83f96886c..c414ca580b 100644
--- a/src/network/ssl/qwindowscarootfetcher.cpp
+++ b/src/network/ssl/qwindowscarootfetcher.cpp
@@ -42,6 +42,8 @@
 #include <QtCore/QThread>
 #include <QtGlobal>
 
+#include <QtCore/qscopeguard.h>
+
 #ifdef QSSLSOCKET_DEBUG
 #include "qssl_p.h" // for debug categories
 #include <QtCore/QElapsedTimer>
@@ -49,6 +51,10 @@
 
 #include "qsslsocket_p.h" // Transitively includes Wincrypt.h
 
+#if QT_CONFIG(openssl)
+#include "qsslsocket_openssl_p.h"
+#endif
+
 QT_BEGIN_NAMESPACE
 
 class QWindowsCaRootFetcherThread : public QThread
@@ -69,8 +75,67 @@ public:
 
 Q_GLOBAL_STATIC(QWindowsCaRootFetcherThread, windowsCaRootFetcherThread);
 
-QWindowsCaRootFetcher::QWindowsCaRootFetcher(const QSslCertificate &certificate, QSslSocket::SslMode sslMode)
-    : cert(certificate), mode(sslMode)
+#if QT_CONFIG(openssl)
+namespace {
+
+const QList<QSslCertificate> buildVerifiedChain(const QList<QSslCertificate> &caCertificates,
+                                                PCCERT_CHAIN_CONTEXT chainContext,
+                                                const QString &peerVerifyName)
+{
+    // We ended up here because OpenSSL verification failed to
+    // build a chain, with intermediate certificate missing
+    // but "Authority Information Access" extension present.
+    // Also, apparently the normal CA fetching path was disabled
+    // by setting custom CA certificates. We convert wincrypt's
+    // structures in QSslCertificate and give OpenSSL the second
+    // chance to verify the now (apparently) complete chain.
+    // In addition, wincrypt gives us a benifit of some checks
+    // we don't have in OpenSSL back-end.
+    Q_ASSERT(chainContext);
+
+    if (!chainContext->cChain)
+        return {};
+
+    QList<QSslCertificate> verifiedChain;
+
+    CERT_SIMPLE_CHAIN *chain = chainContext->rgpChain[chainContext->cChain - 1];
+    if (!chain)
+        return {};
+
+    if (chain->TrustStatus.dwErrorStatus & CERT_TRUST_IS_PARTIAL_CHAIN)
+        return {}; // No need to mess with OpenSSL (the chain is still incomplete).
+
+    for (DWORD i = 0; i < chain->cElement; ++i) {
+        CERT_CHAIN_ELEMENT *element = chain->rgpElement[i];
+        QSslCertificate cert(QByteArray(reinterpret_cast<const char*>(element->pCertContext->pbCertEncoded),
+                             int(element->pCertContext->cbCertEncoded)), QSsl::Der);
+
+        if (cert.isBlacklisted())
+            return {};
+
+        if (element->TrustStatus.dwErrorStatus & CERT_TRUST_IS_REVOKED) // Good to know!
+            return {};
+
+        if (element->TrustStatus.dwErrorStatus & CERT_TRUST_IS_NOT_SIGNATURE_VALID)
+            return {};
+
+        verifiedChain.append(cert);
+    }
+
+    // We rely on OpenSSL's ability to find other problems.
+    const auto tlsErrors = QSslSocketBackendPrivate::verify(caCertificates, verifiedChain, peerVerifyName);
+    if (tlsErrors.size())
+        verifiedChain.clear();
+
+    return verifiedChain;
+}
+
+} // unnamed namespace
+#endif // QT_CONFIG(openssl)
+
+QWindowsCaRootFetcher::QWindowsCaRootFetcher(const QSslCertificate &certificate, QSslSocket::SslMode sslMode,
+                                             const QList<QSslCertificate> &caCertificates, const QString &hostName)
+    : cert(certificate), mode(sslMode), explicitlyTrustedCAs(caCertificates), peerVerifyName(hostName)
 {
     moveToThread(windowsCaRootFetcherThread());
 }
@@ -106,11 +171,12 @@ void QWindowsCaRootFetcher::start()
     stopwatch.start();
 #endif
     PCCERT_CHAIN_CONTEXT chain;
+    auto additionalStore = createAdditionalStore();
     BOOL result = CertGetCertificateChain(
         nullptr, //default engine
         wincert,
         nullptr, //current date/time
-        nullptr, //default store
+        additionalStore.get(), //default store (nullptr) or CAs an application trusts
         &parameters,
         0, //default dwFlags
         nullptr, //reserved
@@ -156,6 +222,16 @@ void QWindowsCaRootFetcher::start()
                     trustedRoot = QSslCertificate(QByteArray((const char *)finalChain->rgpElement[finalChain->cElement - 1]->pCertContext->pbCertEncoded
                         , finalChain->rgpElement[finalChain->cElement - 1]->pCertContext->cbCertEncoded), QSsl::Der);
             }
+        } else if (explicitlyTrustedCAs.size()) {
+            // Setting custom CA in configuration, and those CAs are not trusted by MS.
+#if QT_CONFIG(openssl)
+            const auto verifiedChain = buildVerifiedChain(explicitlyTrustedCAs, chain, peerVerifyName);
+            if (verifiedChain.size())
+                trustedRoot = verifiedChain.last();
+#else
+            //It's only OpenSSL code-path that can trigger such a fetch.
+            Q_UNREACHABLE();
+#endif
         }
         CertFreeCertificateChain(chain);
     }
@@ -165,4 +241,49 @@ void QWindowsCaRootFetcher::start()
     deleteLater();
 }
 
+QHCertStorePointer QWindowsCaRootFetcher::createAdditionalStore() const
+{
+    QHCertStorePointer customStore;
+    if (explicitlyTrustedCAs.isEmpty())
+        return customStore;
+
+    if (HCERTSTORE rawPtr = CertOpenStore(CERT_STORE_PROV_MEMORY, 0, 0, 0, nullptr)) {
+        customStore.reset(rawPtr);
+
+        unsigned rootsAdded = 0;
+        for (const QSslCertificate &caCert : explicitlyTrustedCAs) {
+            const auto der = caCert.toDer();
+            PCCERT_CONTEXT winCert = CertCreateCertificateContext(X509_ASN_ENCODING,
+                                                                  reinterpret_cast<const BYTE *>(der.data()),
+                                                                  DWORD(der.length()));
+            if (!winCert) {
+#if defined(QSSLSOCKET_DEBUG)
+                qCWarning(lcSsl) << "CA fetcher, failed to convert QSslCertificate"
+                                 << "to the native representation";
+#endif // QSSLSOCKET_DEBUG
+                continue;
+            }
+            const auto deleter = qScopeGuard([winCert](){
+                CertFreeCertificateContext(winCert);
+            });
+            if (CertAddCertificateContextToStore(customStore.get(), winCert, CERT_STORE_ADD_ALWAYS, nullptr))
+                ++rootsAdded;
+#if defined(QSSLSOCKET_DEBUG)
+            else //Why assert? With flags we're using and winCert check - should not happen!
+                Q_ASSERT("CertAddCertificateContextToStore() failed");
+#endif // QSSLSOCKET_DEBUG
+        }
+        if (!rootsAdded) //Useless store, no cert was added.
+            customStore.reset();
+#if defined(QSSLSOCKET_DEBUG)
+    } else {
+
+        qCWarning(lcSsl) << "CA fetcher, failed to create a custom"
+                         << "store for explicitly trusted CA certificate";
+#endif // QSSLSOCKET_DEBUG
+    }
+
+    return customStore;
+}
+
 QT_END_NAMESPACE
diff --git a/src/network/ssl/qwindowscarootfetcher_p.h b/src/network/ssl/qwindowscarootfetcher_p.h
index 181c309388..e98e59f0cf 100644
--- a/src/network/ssl/qwindowscarootfetcher_p.h
+++ b/src/network/ssl/qwindowscarootfetcher_p.h
@@ -43,9 +43,13 @@
 #include <QtCore/QtGlobal>
 #include <QtCore/QObject>
 
+#include "qsslsocket_p.h"
+
 #include "qsslsocket.h"
 #include "qsslcertificate.h"
 
+#include <memory>
+
 //
 //  W A R N I N G
 //  -------------
@@ -61,17 +65,29 @@ QT_BEGIN_NAMESPACE
 
 class QWindowsCaRootFetcher : public QObject
 {
-    Q_OBJECT;
+    Q_OBJECT
 public:
-    QWindowsCaRootFetcher(const QSslCertificate &certificate, QSslSocket::SslMode sslMode);
+    QWindowsCaRootFetcher(const QSslCertificate &certificate, QSslSocket::SslMode sslMode,
+                          const QList<QSslCertificate> &caCertificates = {},
+                          const QString &hostName = {});
     ~QWindowsCaRootFetcher();
 public slots:
     void start();
 signals:
     void finished(QSslCertificate brokenChain, QSslCertificate caroot);
 private:
+    QHCertStorePointer createAdditionalStore() const;
+
     QSslCertificate cert;
     QSslSocket::SslMode mode;
+    // In case the application set CA certificates in the configuration,
+    // in the past we did not load missing certs. But this disables
+    // recoverable case when a certificate has Authority Information Access
+    // extension. So we try to fetch in this scenario also, but in case
+    // explicitly trusted root was not in a system store, we'll do
+    // additional checks, thus we need 'peerVerifyName':
+    QList<QSslCertificate> explicitlyTrustedCAs;
+    QString peerVerifyName;
 };
 
 QT_END_NAMESPACE