From 7491b513419d2aad4d4282eb2e8af2ab1597dd93 Mon Sep 17 00:00:00 2001 From: Mårten Nordheim Date: Fri, 18 Sep 2026 12:33:19 +0200 Subject: [PATCH] QWebSocket: fix auth state not being cleared When reusing a QWebSocket object we could end up, depending on the state, retaining some data that would then be used in the next connection. Fixes: QTBUG-150542 Pick-to: 6.12.0 6.11 6.11.3 6.8 Change-Id: I1fd557044a85ca2bae6db37800c68198dbb5b82e Reviewed-by: Edward Welbourne (cherry picked from commit 98249844fa4297b206620a98d369147d79910756) Reviewed-by: Qt Cherry-pick Bot --- diff --git a/src/websockets/qwebsocket_p.cpp b/src/websockets/qwebsocket_p.cpp index fea8243..cd2876b 100644 --- a/src/websockets/qwebsocket_p.cpp +++ b/src/websockets/qwebsocket_p.cpp @@ -424,6 +424,11 @@ m_isClosingHandshakeReceived = false; m_isClosingHandshakeSent = false; + m_authenticator = QAuthenticator(); + m_bytesToSkipBeforeNewResponse = 0; + m_needsResendWithCredentials = false; + m_needsReconnect = false; + setRequest(request, options); if (url.path().isEmpty()) url.setPath(QStringLiteral("/")); diff --git a/tests/auto/websockets/qwebsocket/tst_qwebsocket.cpp b/tests/auto/websockets/qwebsocket/tst_qwebsocket.cpp index da52d24..b7e0f2d 100644 --- a/tests/auto/websockets/qwebsocket/tst_qwebsocket.cpp +++ b/tests/auto/websockets/qwebsocket/tst_qwebsocket.cpp @@ -175,6 +175,8 @@ void handshakeAbortedThenReopened_data(); void handshakeAbortedThenReopened(); void handshakeFollowedByFrame(); + void reopenDoesNotReuseAuthenticator(); + void reopenDoesNotSkipResponseBytes(); }; tst_QWebSocket::tst_QWebSocket() @@ -1578,8 +1580,8 @@ QCOMPARE(messageReceivedSpy.at(0).at(0).toString(), payload); } -// Reads the client's request off the socket and returns the matching 101 response. -static QByteArray upgradeResponseFor(QTcpSocket *serverSocket) +// Reads one complete request header off the socket and returns it. +static QByteArray readRequestHeader(QTcpSocket *serverSocket) { // Spin the event loop rather than blocking on the socket: the client lives in this // same thread, and would never get to send its request. @@ -1590,6 +1592,15 @@ return {}; data.append(serverSocket->readAll()); } + return data; +} + +// Reads the client's request off the socket and returns the matching 101 response. +static QByteArray upgradeResponseFor(QTcpSocket *serverSocket) +{ + const QByteArray data = readRequestHeader(serverSocket); + if (data.isEmpty()) + return {}; const auto view = QLatin1String(data); const auto keyHeader = QLatin1String("Sec-WebSocket-Key:"); @@ -1797,6 +1808,150 @@ QCOMPARE(errorSpy.size(), 0); } +// Answers a handshake request with a 401 Basic challenge under the given realm, and +// returns the request that was answered. +static QByteArray answerWithAuthChallenge(QTcpSocket *serverSocket, QByteArrayView realm, + QByteArrayView extraHeaders = {}) +{ + const QByteArray request = readRequestHeader(serverSocket); + if (request.isEmpty()) + return {}; + + QByteArray response = "HTTP/1.1 401 Unauthorized\r\nWWW-Authenticate: Basic realm="; + response.append(realm); + response.append("\r\n"); + response.append(extraHeaders); + response.append("\r\n"); + if (serverSocket->write(response) != response.size()) + return {}; + return request; +} + +// The decoded "user:password" a request offers, so a failure names the credential that +// was actually sent rather than its base64. +static QByteArray basicCredentialIn(QByteArrayView request) +{ + const QLatin1StringView value = AuthServer::getHeaderValue("Authorization"_L1, request); + if (!value.startsWith("Basic "_L1)) + return {}; + return QByteArray::fromBase64(QByteArrayView(value.sliced(6)).toByteArray()); +} + +// A QWebSocket reused for a second connection must not carry the first connection's +// QAuthenticator with it: the credential the application produced for one host is not +// the credential it would produce for another, and it must be asked again. +void tst_QWebSocket::reopenDoesNotReuseAuthenticator() +{ + QTcpServer hostA; + QVERIFY(hostA.listen(QHostAddress::LocalHost)); + QTcpServer hostB; + QVERIFY(hostB.listen(QHostAddress::LocalHost)); + + const QUrl urlA("ws://127.0.0.1:%1"_L1.arg(QString::number(hostA.serverPort()))); + const QUrl urlB("ws://127.0.0.1:%1"_L1.arg(QString::number(hostB.serverPort()))); + + // A per-host secret, as any real credential store would return. + const QStringList secrets = { u"secret-for-host-a"_s, u"secret-for-host-b"_s }; + int authRequests = 0; + + QWebSocket socket; + connect(&socket, &QWebSocket::authenticationRequired, &socket, + [&](QAuthenticator *authenticator) { + authenticator->setUser(u"user"_s); + authenticator->setPassword(secrets.value(authRequests, u"unexpected"_s)); + ++authRequests; + }); + + // First connection: host A challenges, the application supplies host A's secret, + // and the client resends authenticated. All of this is correct. + socket.open(urlA); + QTRY_VERIFY(hostA.hasPendingConnections()); + { + const std::unique_ptr serverSocket(hostA.nextPendingConnection()); + QVERIFY(serverSocket); + QVERIFY(!answerWithAuthChallenge(serverSocket.get(), "host-a").isEmpty()); + + const QByteArray authenticated = readRequestHeader(serverSocket.get()); + QVERIFY(!authenticated.isEmpty()); + QCOMPARE(authRequests, 1); + QCOMPARE(basicCredentialIn(authenticated), "user:secret-for-host-a"); + + // Host A goes away before the exchange completes, leaving the authenticator + // holding host A's credentials. + serverSocket->disconnectFromHost(); + } + QTRY_COMPARE(socket.state(), QAbstractSocket::UnconnectedState); + + // Second connection, same QWebSocket, different host and different realm. + socket.open(urlB); + QTRY_VERIFY(hostB.hasPendingConnections()); + const std::unique_ptr serverSocket(hostB.nextPendingConnection()); + QVERIFY(serverSocket); + + const QByteArray opening = answerWithAuthChallenge(serverSocket.get(), "host-b"); + QVERIFY(!opening.isEmpty()); + // Nothing has challenged us yet, so the opening request carries no credential. + QCOMPARE(basicCredentialIn(opening), QByteArray()); + + // Host B's challenge must reach the application before any credential reaches + // host B. Without that, the carried-over authenticator answers on its own and + // host B is handed the secret that was only ever meant for host A. + const QByteArray answered = readRequestHeader(serverSocket.get()); + QVERIFY(!answered.isEmpty()); + QCOMPARE(basicCredentialIn(answered), "user:secret-for-host-b"); + QCOMPARE(authRequests, 2); +} + +// A QWebSocket reused for a second connection must not carry the first connection's +// pending body-skip count with it, or it eats the front of the next server's response. +void tst_QWebSocket::reopenDoesNotSkipResponseBytes() +{ + QTcpServer hostA; + QVERIFY(hostA.listen(QHostAddress::LocalHost)); + QTcpServer hostB; + QVERIFY(hostB.listen(QHostAddress::LocalHost)); + + const QUrl urlA("ws://127.0.0.1:%1"_L1.arg(QString::number(hostA.serverPort()))); + const QUrl urlB("ws://127.0.0.1:%1"_L1.arg(QString::number(hostB.serverPort()))); + + QWebSocket socket; + connect(&socket, &QWebSocket::authenticationRequired, &socket, + [](QAuthenticator *authenticator) { + authenticator->setUser(u"user"_s); + authenticator->setPassword(u"password"_s); + }); + + // First connection: host A announces a 4 KiB error body with its challenge but + // never sends it, so the client is still waiting to skip those bytes. + socket.open(urlA); + QTRY_VERIFY(hostA.hasPendingConnections()); + { + const std::unique_ptr serverSocket(hostA.nextPendingConnection()); + QVERIFY(serverSocket); + QVERIFY(!answerWithAuthChallenge(serverSocket.get(), "host-a", + "Content-Length: 4096\r\n").isEmpty()); + QVERIFY(!readRequestHeader(serverSocket.get()).isEmpty()); // the resend + serverSocket->disconnectFromHost(); + } + QTRY_COMPARE(socket.state(), QAbstractSocket::UnconnectedState); + + // Second connection, same QWebSocket: a well-behaved server completes a clean + // handshake, which has to be parsed from its very first byte. + QSignalSpy connectedSpy(&socket, &QWebSocket::connected); + QSignalSpy errorSpy(&socket, &QWebSocket::errorOccurred); + socket.open(urlB); + QTRY_VERIFY(hostB.hasPendingConnections()); + const std::unique_ptr serverSocket(hostB.nextPendingConnection()); + QVERIFY(serverSocket); + + const QByteArray response = upgradeResponseFor(serverSocket.get()); + QVERIFY(!response.isEmpty()); + QCOMPARE(serverSocket->write(response), response.size()); + + QTRY_COMPARE(connectedSpy.size(), 1); + QCOMPARE(errorSpy.size(), 0); +} + QTEST_MAIN(tst_QWebSocket) #include "tst_qwebsocket.moc"