diff options
author | Mårten Nordheim <marten.nordheim@qt.io> | 2021-11-29 16:31:21 +0100 |
---|---|---|
committer | Mårten Nordheim <marten.nordheim@qt.io> | 2021-12-02 19:29:58 +0000 |
commit | a6744bc9f9d0a1bcdaa6769ddb39a18dfad5f5c3 (patch) | |
tree | 9dad47f98353d9e8602f69cc9bb727edd27d629e /src/plugins/tls | |
parent | 3c6582a082bdaa4940efdf93ea294e8f03f39435 (diff) |
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>
Diffstat (limited to 'src/plugins/tls')
-rw-r--r-- | src/plugins/tls/openssl/qtls_openssl.cpp | 48 | ||||
-rw-r--r-- | 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 |