Compare commits

...

No commits in common. "c8" and "c8-beta" have entirely different histories.
c8 ... c8-beta

3 changed files with 1 additions and 239 deletions

View File

@ -1,168 +0,0 @@
From b1e75376cc3adfc7da5502a277dfe9711f3e0536 Mon Sep 17 00:00:00 2001
From: Mårten Nordheim <marten.nordheim@qt.io>
Date: Tue, 25 Jun 2024 17:09:35 +0200
Subject: [PATCH] HTTP2: Delay any communication until encrypted() can be responded to
We have the encrypted() signal that lets users do extra checks on the
established connection. It is emitted as BlockingQueued, so the HTTP
thread stalls until it is done emitting. Users can potentially call
abort() on the QNetworkReply at that point, which is passed as a Queued
call back to the HTTP thread. That means that any currently queued
signal emission will be processed before the abort() call is processed.
In the case of HTTP2 it is a little special since it is multiplexed and
the code is built to start requests as they are available. This means
that, while the code worked fine for HTTP1, since one connection only
has one request, it is not working for HTTP2, since we try to send more
requests in-between the encrypted() signal and the abort() call.
This patch changes the code to delay any communication until the
encrypted() signal has been emitted and processed, for HTTP2 only.
It's done by adding a few booleans, both to know that we have to return
early and so we can keep track of what events arose and what we need to
resume once enough time has passed that any abort() call must have been
processed.
Fixes: QTBUG-126610
Pick-to: 6.8 6.7 6.5 6.2 5.15 5.12
Change-Id: Ic25a600c278203256e35f541026f34a8783235ae
Reviewed-by: Marc Mutz <marc.mutz@qt.io>
Reviewed-by: Volker Hilsheimer <volker.hilsheimer@qt.io>
---
diff --git a/src/network/access/qhttp2protocolhandler.cpp b/src/network/access/qhttp2protocolhandler.cpp
index 91c41d82..562dfd66 100644
--- a/src/network/access/qhttp2protocolhandler.cpp
+++ b/src/network/access/qhttp2protocolhandler.cpp
@@ -371,12 +371,12 @@ bool QHttp2ProtocolHandler::sendRequest()
}
}
- if (!prefaceSent && !sendClientPreface())
- return false;
-
if (!requests.size())
return true;
+ if (!prefaceSent && !sendClientPreface())
+ return false;
+
m_channel->state = QHttpNetworkConnectionChannel::WritingState;
// Check what was promised/pushed, maybe we do not have to send a request
// and have a response already?
diff --git a/src/network/access/qhttpnetworkconnectionchannel.cpp b/src/network/access/qhttpnetworkconnectionchannel.cpp
index 647ef431..608e7914 100644
--- a/src/network/access/qhttpnetworkconnectionchannel.cpp
+++ b/src/network/access/qhttpnetworkconnectionchannel.cpp
@@ -255,6 +255,10 @@ void QHttpNetworkConnectionChannel::abort()
bool QHttpNetworkConnectionChannel::sendRequest()
{
Q_ASSERT(!protocolHandler.isNull());
+ if (waitingForPotentialAbort) {
+ needInvokeSendRequest = true;
+ return false; // this return value is unused
+ }
return protocolHandler->sendRequest();
}
@@ -267,21 +271,28 @@ bool QHttpNetworkConnectionChannel::sendRequest()
void QHttpNetworkConnectionChannel::sendRequestDelayed()
{
QMetaObject::invokeMethod(this, [this] {
- Q_ASSERT(!protocolHandler.isNull());
if (reply)
- protocolHandler->sendRequest();
+ sendRequest();
}, Qt::ConnectionType::QueuedConnection);
}
void QHttpNetworkConnectionChannel::_q_receiveReply()
{
Q_ASSERT(!protocolHandler.isNull());
+ if (waitingForPotentialAbort) {
+ needInvokeReceiveReply = true;
+ return;
+ }
protocolHandler->_q_receiveReply();
}
void QHttpNetworkConnectionChannel::_q_readyRead()
{
Q_ASSERT(!protocolHandler.isNull());
+ if (waitingForPotentialAbort) {
+ needInvokeReadyRead = true;
+ return;
+ }
protocolHandler->_q_readyRead();
}
@@ -1287,7 +1298,18 @@ void QHttpNetworkConnectionChannel::_q_encrypted()
// Similar to HTTP/1.1 counterpart below:
const auto &pairs = spdyRequestsToSend.values(); // (request, reply)
const auto &pair = pairs.first();
+ waitingForPotentialAbort = true;
emit pair.second->encrypted();
+
+ // We don't send or handle any received data until any effects from
+ // emitting encrypted() have been processed. This is necessary
+ // because the user may have called abort(). We may also abort the
+ // whole connection if the request has been aborted and there is
+ // no more requests to send.
+ QMetaObject::invokeMethod(this,
+ &QHttpNetworkConnectionChannel::checkAndResumeCommunication,
+ Qt::QueuedConnection);
+
// In case our peer has sent us its settings (window size, max concurrent streams etc.)
// let's give _q_receiveReply a chance to read them first ('invokeMethod', QueuedConnection).
QMetaObject::invokeMethod(connection, "_q_startNextRequest", Qt::QueuedConnection);
@@ -1305,6 +1327,26 @@ void QHttpNetworkConnectionChannel::_q_encrypted()
}
}
+void QHttpNetworkConnectionChannel::checkAndResumeCommunication()
+{
+ Q_ASSERT(connection->connectionType() > QHttpNetworkConnection::ConnectionTypeHTTP);
+
+ // Because HTTP/2 requires that we send a SETTINGS frame as the first thing we do, and respond
+ // to a SETTINGS frame with an ACK, we need to delay any handling until we can ensure that any
+ // effects from emitting encrypted() have been processed.
+ // This function is called after encrypted() was emitted, so check for changes.
+
+ if (!reply && spdyRequestsToSend.isEmpty())
+ abort();
+ waitingForPotentialAbort = false;
+ if (needInvokeReadyRead)
+ _q_readyRead();
+ if (needInvokeReceiveReply)
+ _q_receiveReply();
+ if (needInvokeSendRequest)
+ sendRequest();
+}
+
void QHttpNetworkConnectionChannel::requeueSpdyRequests()
{
QList<HttpMessagePair> spdyPairs = spdyRequestsToSend.values();
diff --git a/src/network/access/qhttpnetworkconnectionchannel_p.h b/src/network/access/qhttpnetworkconnectionchannel_p.h
index d8ac3979..eac44464 100644
--- a/src/network/access/qhttpnetworkconnectionchannel_p.h
+++ b/src/network/access/qhttpnetworkconnectionchannel_p.h
@@ -107,6 +107,10 @@ public:
QAbstractSocket *socket;
bool ssl;
bool isInitialized;
+ bool waitingForPotentialAbort = false;
+ bool needInvokeReceiveReply = false;
+ bool needInvokeReadyRead = false;
+ bool needInvokeSendRequest = false;
ChannelState state;
QHttpNetworkRequest request; // current request, only used for HTTP
QHttpNetworkReply *reply; // current reply for this request, only used for HTTP
@@ -187,6 +191,8 @@ public:
void closeAndResendCurrentRequest();
void resendCurrentRequest();
+ void checkAndResumeCommunication();
+
bool isSocketBusy() const;
bool isSocketWriting() const;
bool isSocketWaiting() const;

View File

@ -1,60 +0,0 @@
From 95064c35826793c5d6a4edff9fa08ad308b047bb Mon Sep 17 00:00:00 2001
From: Timur Pocheptsov <timur.pocheptsov@qt.io>
Date: Tue, 20 Jul 2021 08:16:28 +0200
Subject: [PATCH] H2: emit encrypted for at least the first reply, similar to
H1
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Fixes: QTBUG-95277
Change-Id: I1fe01503376c0d6278e366d7bd31b412b7cc3a69
Reviewed-by: Mårten Nordheim <marten.nordheim@qt.io>
(cherry picked from commit c23b7886348dc313ccec1a131850a7cce1b429de)
---
src/network/access/qhttpnetworkconnectionchannel.cpp | 4 ++++
tests/auto/network/access/http2/tst_http2.cpp | 9 +++++++++
2 files changed, 13 insertions(+)
diff --git a/src/network/access/qhttpnetworkconnectionchannel.cpp b/src/network/access/qhttpnetworkconnectionchannel.cpp
index f1db2744..647ef431 100644
--- a/src/network/access/qhttpnetworkconnectionchannel.cpp
+++ b/src/network/access/qhttpnetworkconnectionchannel.cpp
@@ -1284,6 +1284,10 @@ void QHttpNetworkConnectionChannel::_q_encrypted()
connection->connectionType() == QHttpNetworkConnection::ConnectionTypeHTTP2Direct) {
// we call setSpdyWasUsed(true) on the replies in the SPDY handler when the request is sent
if (spdyRequestsToSend.count() > 0) {
+ // Similar to HTTP/1.1 counterpart below:
+ const auto &pairs = spdyRequestsToSend.values(); // (request, reply)
+ const auto &pair = pairs.first();
+ emit pair.second->encrypted();
// In case our peer has sent us its settings (window size, max concurrent streams etc.)
// let's give _q_receiveReply a chance to read them first ('invokeMethod', QueuedConnection).
QMetaObject::invokeMethod(connection, "_q_startNextRequest", Qt::QueuedConnection);
diff --git a/tests/auto/network/access/http2/tst_http2.cpp b/tests/auto/network/access/http2/tst_http2.cpp
index 6702f25b..95a97902 100644
--- a/tests/auto/network/access/http2/tst_http2.cpp
+++ b/tests/auto/network/access/http2/tst_http2.cpp
@@ -267,6 +267,10 @@ void tst_Http2::singleRequest()
request.setAttribute(h2Attribute, QVariant(true));
auto reply = manager->get(request);
+#if QT_CONFIG(ssl)
+ QSignalSpy encSpy(reply, &QNetworkReply::encrypted);
+#endif // QT_CONFIG(ssl)
+
connect(reply, &QNetworkReply::finished, this, &tst_Http2::replyFinished);
// Since we're using self-signed certificates,
// ignore SSL errors:
@@ -281,6 +285,11 @@ void tst_Http2::singleRequest()
QCOMPARE(reply->error(), QNetworkReply::NoError);
QVERIFY(reply->isFinished());
+
+#if QT_CONFIG(ssl)
+ if (connectionType == H2Type::h2Alpn || connectionType == H2Type::h2Direct)
+ QCOMPARE(encSpy.count(), 1);
+#endif // QT_CONFIG(ssl)
}
void tst_Http2::multipleRequests()

View File

@ -41,7 +41,7 @@ BuildRequires: pkgconfig(libsystemd)
Name: qt5-qtbase
Summary: Qt5 - QtBase components
Version: 5.15.3
Release: 8%{?dist}
Release: 7%{?dist}
# See LGPL_EXCEPTIONS.txt, for exception details
License: LGPLv2 with exceptions or GPLv3 with exceptions
@ -124,7 +124,6 @@ Patch90: %{name}-gcc11.patch
Patch100: kde-5.15-rollup-20220324.patch.gz
# HACK to make 'fedpkg sources' consider it 'used"
Source100: kde-5.15-rollup-20220324.patch.gz
Patch110: CVE-2023-32762-qtbase-5.15.patch
Patch111: CVE-2023-32763-qtbase-5.15.patch
Patch112: CVE-2023-33285-qtbase-5.15.patch
@ -134,9 +133,6 @@ Patch115: CVE-2023-38197-qtbase-5.15.patch
Patch116: 0001-CVE-2023-51714-qtbase-5.15.patch
Patch117: 0002-CVE-2023-51714-qtbase-5.15.patch
Patch118: CVE-2024-25580-qtbase-5.15.patch
# Fix related to CVE-2024-39936, which I think is needed to be backported
Patch119: qtbase-h2-emit-encrypted-for-first-reply-similar-to-h1.patch
Patch120: CVE-2024-39936.patch
# Do not check any files in %%{_qt5_plugindir}/platformthemes/ for requires.
# Those themes are there for platform integration. If the required libraries are
@ -404,8 +400,6 @@ Qt5 libraries used for drawing widgets and OpenGL items.
%patch116 -p1
%patch117 -p1
%patch118 -p1
%patch119 -p1
%patch120 -p1
# move some bundled libs to ensure they're not accidentally used
pushd src/3rdparty
@ -1080,10 +1074,6 @@ fi
%changelog
* Tue Jul 16 2024 Jan Grulich <jgrulich@redhat.com> - 5.15.3-8
- HTTP2: Delay any communication until encrypted() can be responded to
Resolves: RHEL-46340
* Fri Feb 16 2024 Jan Grulich <jgrulich@redhat.com> - 5.15.3-7
- Fix CVE-2024-25580: potential buffer overflow when reading KTX images
Resolves: RHEL-25725