Uploaded image for project: 'Qt'
  1. Qt
  2. QTBUG-92231

SSL handshake failure after ignoreSslErrors

    XMLWordPrintable

    Details

    • Platform/s:
      Linux/X11, Windows
    • Story Points:
      8
    • Sprint:
      Team 2 Foundation_Sprint 47

      Description

      Hi all,

      we have found a bug in the qt network module. I have attached a sample Qt application to reproduce the issue. The problem that we face is that the QNetworkReply's ignoreSslErrors() does not work in some cases.
      We have an application that downloads some data from a web server. If the X509 certificate can't be verified, then we show the certificate details for the user and the user can ignore the error. In this code path, we will call the ignoreSslErrors().

       

      It works fine with Qt 5.13.0, I used git bisect to find where the regression was introduced, the details can be found below. Newer versions, e.g.: Qt 5.15.2 and Qt 6.0.0 are affected too.

      Here is the error message:

      qt.network.ssl: Neither X509_STORE, nor SSL contains error list, handshake failure
      client:errorOccurred QNetworkReply::UnknownNetworkError

       

      I could not reproduce the issue with Nginx, the server that produces the issue uses the HTTP.sys web server implementation in ASP.NET Core. https://docs.microsoft.com/en-us/aspnet/core/fundamentals/servers/httpsys?view=aspnetcore-5.0

      The self-signed CA certificate and it's key is in the bundle too, the nginx web server can be started with docker-compose. The url should be changed to localhost in the cpp code. Unfortunately this won't reproduce the issue. I will try to create an example server with .net and attach it later If I succeed.

       

      I used git bisect, and it looks like this is the first bad commit:

      commit fe6e54fb1f5cda652b9489f740763f8d735621dd
      Author: Timur Pocheptsov <timur.pocheptsov@qt.io>
      Date: Fri Jun 14 11:34:08 2019 +0200
      TLS socket: make verification callback lock-free (OpenSSL)
       
       When our QSslSocketBackendPrivate (OpenSSL backend) was developed,
       the ancient versions of OpenSSL did not have an API needed to pass
       an application-specific data into verification callback. Thus the
       developers resorted to the use of global variables (a list with errors)
       and locks. Some of our auto-tests use QNAM and in-process server.
       Whenever the client (essentially qhttpthreadeddelegate) and the server
       live in different threads, any use of 'https' is dead-lock prone,
       which recent events demonstrated and which were previously observed
       but not understood properly (rare occasions, not always easy to
       reproduce). Now we fix this for good by removing locking.
       
       There are two places (in 5.12) where these locks are needed:
       
       1. Before calling SSL_connect/SSL_accept (handshake) - here
       we reuse the same trick we do in PSK callback ('SSL' has
       an external data set, and it's 'this', meaning an object
       of type QSslSocketBackendPrivate).
       
       2. The static member function 'verify', here we do not have
       'SSL', but we have our temporary 'X509_STORE', to which
       we can directly attach an external data - a pointer to
       a vector to collect verification errors.
       
       Note, this change assumes that OpenSSL Qt is build/linked
       against is at least of version 1.0.1 - we set external data
       on SSL unconditionally (no version checks).
       
       Fixes: QTBUG-76157
       Change-Id: I05c98e77dfd5fb0c2c260fb6c463732facf53ffc
       Reviewed-by: Mårten Nordheim <marten.nordheim@qt.io>
      src/network/ssl/qsslsocket_openssl.cpp | 106 +++++++++++----------
       src/network/ssl/qsslsocket_openssl11_symbols_p.h | 2 +
       src/network/ssl/qsslsocket_openssl_symbols.cpp | 10 ++
       src/network/ssl/qsslsocket_openssl_symbols_p.h | 1 +
       .../ssl/qsslsocket_opensslpre11_symbols_p.h | 2 +
       5 files changed, 70 insertions(+), 51 deletions(-)

       

      If I add the self-signed CA certificate to the trusted anchors, e.g.:

      cp cert.pem /etc/ca-certificates/trust-source/anchors/bradcert.pem
      update-ca-trust

      then everything works as expected. No sslErrors are triggered.

       

      I can also disable the peer verification, setPeerVerifyMode(QSslSocket::VerifyNone) works, I can download the data. This is not a good workaround for us. The strange thing is that if I set it to VerifyPeer then no matter what I do, I can't change it to VerifyNone, I will get the same error. Or if I set VerifyNone for the first try, and the second try won't trigger sslErrors, even with VerifyPeer mode. It looks like something is cached.

       

      I also attached Qt debug logs plus some additional hexdumps of the sent/received network traffic. Well, maybe the errors are not cleared correctly or something like that.

       

      If you need any additional information let me know. I try to make a server example that can be used for reproduction.

       

      Here is the example code:

        QNetworkRequest request(QUrl{"https://10.5.34.62:443/service/"});
      
        QSslConfiguration conf = request.sslConfiguration();
        conf.setPeerVerifyMode(QSslSocket::VerifyPeer);
        conf.setProtocol(QSsl::TlsV1_2OrLater);
        request.setSslConfiguration(conf);
      
        QNetworkReply *reply = manager.get(request);
        QObject::connect(reply, &QIODevice::readyRead, [reply]() {
          qDebug() << qPrintable(
              reply->sslConfiguration().peerCertificateChain().at(0).toPem());
          qDebug() << reply->sslConfiguration()
                          .peerCertificateChain()
                          .at(0)
                          .toPem()
                          .toBase64();
          qDebug() << "client:readyRead" << reply->readAll();
        });
        QObject::connect(reply, &QNetworkReply::sslErrors,
                         [reply](QList<QSslError> errors) {
                           qDebug() << "client:sslErrors" << errors << "\n"
                                    << qPrintable(errors.at(0).certificate().toPem());
                           qDebug() << "BEFORE IGNORE";
                           reply->ignoreSslErrors();
                           qDebug() << "AFTER IGNORE";
                         });
      
        QObject::connect(reply, &QNetworkReply::errorOccurred,
                         [](QNetworkReply::NetworkError error) {
                           qDebug() << "client:errorOccurred" << error << "\n";
                         }); 

        Attachments

        1. 0001-extra-debugs.patch
          11 kB
        2. extra_debuglogs_5.txt
          35 kB
        3. extra_debuglogs.txt
          34 kB
        4. memory_usage.png
          memory_usage.png
          165 kB
        5. qt_5_13_0_debug_logs_full_url.txt
          41 kB
        6. Qt_5_15_2_debug_logs_longest_url_verify_none.txt
          40 kB
        7. Qt_5_15_2_debug_logs_longest_url_verify_peer_in_trusted_anchors.txt
          40 kB
        8. Qt_5_15_2_debug_logs_longest_url_verify_peer.txt
          32 kB
        9. segfault
          16 kB
        10. sslcert.tar.gz
          92 kB
        11. TestWebSslFw2.zip
          9.59 MB
        For Gerrit Dashboard: QTBUG-92231
        # Subject Branch Project Status CR V

          Activity

            People

            Assignee:
            manordheim Mårten Nordheim
            Reporter:
            tomicooler Tamás Dömők
            PM Owner:
            Vladimir Minenko Vladimir Minenko
            Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

              Dates

              Created:
              Updated:

                Gerrit Reviews

                There is 1 open Gerrit change