summaryrefslogtreecommitdiffstats
path: root/src
diff options
context:
space:
mode:
authorMårten Nordheim <marten.nordheim@qt.io>2021-11-29 16:31:21 +0100
committerMårten Nordheim <marten.nordheim@qt.io>2021-12-02 19:29:58 +0000
commita6744bc9f9d0a1bcdaa6769ddb39a18dfad5f5c3 (patch)
tree9dad47f98353d9e8602f69cc9bb727edd27d629e /src
parent3c6582a082bdaa4940efdf93ea294e8f03f39435 (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')
-rw-r--r--src/plugins/tls/openssl/qtls_openssl.cpp48
-rw-r--r--src/plugins/tls/openssl/qtls_openssl_p.h6
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