From a6744bc9f9d0a1bcdaa6769ddb39a18dfad5f5c3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?M=C3=A5rten=20Nordheim?= <marten.nordheim@qt.io>
Date: Mon, 29 Nov 2021 16:31:21 +0100
Subject: OpenSSL: handle renegotiate errors by comparing certs

If the certificate didn't change then our trust in it didn't either.
Sadly, cannot have an autotest because we don't have any way
to facilitate a renegotiation at the moment and with TLS 1.3
not having them at all it's unlikely we ever will.

Pick-to: 6.2 5.15
Task-number: QTBUG-92231
Change-Id: Ibaa9b2f627daca05021c574e69526710aacdadae
Reviewed-by: Edward Welbourne <edward.welbourne@qt.io>
---
 src/plugins/tls/openssl/qtls_openssl.cpp | 48 ++++++++++++++++++++++++++++++--
 src/plugins/tls/openssl/qtls_openssl_p.h |  6 ++++
 2 files changed, 52 insertions(+), 2 deletions(-)

diff --git a/src/plugins/tls/openssl/qtls_openssl.cpp b/src/plugins/tls/openssl/qtls_openssl.cpp
index 2b78059744..189730a594 100644
--- a/src/plugins/tls/openssl/qtls_openssl.cpp
+++ b/src/plugins/tls/openssl/qtls_openssl.cpp
@@ -169,9 +169,25 @@ int q_X509Callback(int ok, X509_STORE_CTX *ctx)
             // To retrieve this pointer the X509_STORE_CTX_get_ex_data() function can be
             // used with the correct index."
             const auto offset = QTlsBackendOpenSSL::s_indexForSSLExtraData
-                                + TlsCryptographOpenSSL::errorOffsetInExData;
-            if (SSL *ssl = static_cast<SSL *>(q_X509_STORE_CTX_get_ex_data(ctx, q_SSL_get_ex_data_X509_STORE_CTX_idx())))
+                    + TlsCryptographOpenSSL::errorOffsetInExData;
+            if (SSL *ssl = static_cast<SSL *>(q_X509_STORE_CTX_get_ex_data(
+                        ctx, q_SSL_get_ex_data_X509_STORE_CTX_idx()))) {
+
+                // We may be in a renegotiation, check if we are inside a call to SSL_read:
+                const auto tlsOffset = QTlsBackendOpenSSL::s_indexForSSLExtraData
+                        + TlsCryptographOpenSSL::socketOffsetInExData;
+                auto tls = static_cast<TlsCryptographOpenSSL *>(q_SSL_get_ex_data(ssl, tlsOffset));
+                Q_ASSERT(tls);
+                if (tls->isInSslRead()) {
+                    // We are in a renegotiation, make a note of this for later.
+                    // We'll check that the certificate is the same as the one we got during
+                    // the initial handshake
+                    tls->setRenegotiated(true);
+                    return 1;
+                }
+
                 errors = ErrorListPtr(q_SSL_get_ex_data(ssl, offset));
+            }
         }
 
         if (!errors) {
@@ -1047,7 +1063,25 @@ void TlsCryptographOpenSSL::transmit()
                 break;
             }
             // Don't use SSL_pending(). It's very unreliable.
+            inSslRead = true;
             readBytes = q_SSL_read(ssl, buffer.reserve(bytesToRead), bytesToRead);
+            inSslRead = false;
+            if (renegotiated) {
+                renegotiated = false;
+                X509 *x509 = q_SSL_get_peer_certificate(ssl);
+                const auto peerCertificate =
+                        QTlsPrivate::X509CertificateOpenSSL::certificateFromX509(x509);
+                // Fail the renegotiate if the certificate has changed, else: continue.
+                if (peerCertificate != q->peerCertificate()) {
+                    const ScopedBool bg(inSetAndEmitError, true);
+                    setErrorAndEmit(
+                            d, QAbstractSocket::RemoteHostClosedError,
+                            QSslSocket::tr(
+                                    "TLS certificate unexpectedly changed during renegotiation!"));
+                    q->abort();
+                    return;
+                }
+            }
             if (readBytes > 0) {
 #ifdef QSSLSOCKET_DEBUG
                 qCDebug(lcTlsBackend) << "TlsCryptographOpenSSL::transmit: decrypted" << readBytes << "bytes";
@@ -1757,6 +1791,16 @@ unsigned TlsCryptographOpenSSL::pskServerTlsCallback(const char *identity, unsig
     return pskLength;
 }
 
+bool TlsCryptographOpenSSL::isInSslRead() const
+{
+    return inSslRead;
+}
+
+void TlsCryptographOpenSSL::setRenegotiated(bool renegotiated)
+{
+    this->renegotiated = renegotiated;
+}
+
 #ifdef Q_OS_WIN
 
 void TlsCryptographOpenSSL::fetchCaRootForCert(const QSslCertificate &cert)
diff --git a/src/plugins/tls/openssl/qtls_openssl_p.h b/src/plugins/tls/openssl/qtls_openssl_p.h
index 60dae884fc..2fcefb222c 100644
--- a/src/plugins/tls/openssl/qtls_openssl_p.h
+++ b/src/plugins/tls/openssl/qtls_openssl_p.h
@@ -121,6 +121,9 @@ public:
     unsigned pskServerTlsCallback(const char *identity, unsigned char *psk,
                                   unsigned max_psk_len);
 
+    bool isInSslRead() const;
+    void setRenegotiated(bool renegotiated);
+
 #ifdef Q_OS_WIN
     void fetchCaRootForCert(const QSslCertificate &cert);
     void caRootLoaded(QSslCertificate certificate, QSslCertificate trustedRoot);
@@ -160,6 +163,9 @@ private:
     bool errorsReportedFromCallback = false;
 
     bool shutdown = false;
+
+    bool inSslRead = false;
+    bool renegotiated = false;
 };
 
 } // namespace QTlsPrivate
-- 
cgit v1.2.3