From 73158a9cb0942c2cdb3c6a98bcfd5763eed65c85 Mon Sep 17 00:00:00 2001 From: Timur Pocheptsov Date: Thu, 14 May 2020 16:40:08 +0200 Subject: CA fetcher (Windows) - relax the logic a bit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Reviewed-by: Mårten Nordheim --- src/network/ssl/qsslsocket_openssl.cpp | 131 ++++++++++++++++++++++----------- 1 file changed, 88 insertions(+), 43 deletions(-) (limited to 'src/network/ssl/qsslsocket_openssl.cpp') 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 &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 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 QSslSocketBackendPrivate::STACKOFX509_to_QSslCertificates return certificates; } -QList QSslSocketBackendPrivate::verify(const QList &certificateChain, const QString &hostName) +QList QSslSocketBackendPrivate::verify(const QList &certificateChain, + const QString &hostName) { - QList 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 QSslSocketBackendPrivate::verify(const QList &caCertificates, + const QList &certificateChain, + const QString &hostName) +{ + if (certificateChain.count() <= 0) + return {QSslError(QSslError::UnspecifiedError)}; + + QList errors; // Setup the store with the default CA certificates X509_STORE *certStore = q_X509_STORE_new(); if (!certStore) { @@ -2266,12 +2316,7 @@ QList QSslSocketBackendPrivate::verify(const QList & } const std::unique_ptr 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: // -- cgit v1.2.3