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/openssl/qtls_openssl.cpp | |
| 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/openssl/qtls_openssl.cpp')
| -rw-r--r-- | src/plugins/tls/openssl/qtls_openssl.cpp | 48 |
1 files changed, 46 insertions, 2 deletions
diff --git a/src/plugins/tls/openssl/qtls_openssl.cpp b/src/plugins/tls/openssl/qtls_openssl.cpp index 2b780597446..189730a594c 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) |