HTTP/2 authentication required

With Qt 6 we made HTTP/2 default, which exposed missing handling of
401 Unauthorized (and 407 Proxy Authentication Required).

In HTTP/1.* we would handle this after the response had finished, while
handling the status code. For h2 this path isn't used since it is
heavily reliant on the structure we have for HTTP/1.* (one request per
channel). So we must handle the status code and header directly.

Having that part fixed exposed another issue - when resetting/rewinding
uploaded data we were not resetting the 'totallyUploadedData' counter in
the reply (this, in turn, exposed another small issue). Because of that
we did not actually send any data on the retry, only sending the
content-length followed by no data.

Finally, the small issue mentioned in the previous paragraph was how we
check if we have uploaded all our data. It was only checking if the
byte-device was atEnd(), which it was. But only because it had not yet
prepared any data for us.

Fixes: QTBUG-91284
Change-Id: I798d105b02688b18a02897cc476f19f57a47f98f
Reviewed-by: Timur Pocheptsov <timur.pocheptsov@qt.io>
Reviewed-by: Edward Welbourne <edward.welbourne@qt.io>
(cherry picked from commit 52a0eb4791727157a7b385f7e022faad28da4821)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>

8 files changed, 215 insertions, 29 deletions

diff --git a/src/network/access/qhttp2protocolhandler.cpp b/src/network/access/qhttp2protocolhandler.cpp
index 8f77911341..8f2ad8391b 100644
--- a/src/network/access/qhttp2protocolhandler.cpp
+++ b/src/network/access/qhttp2protocolhandler.cpp
@@ -495,6 +495,10 @@ bool QHttp2ProtocolHandler::sendHEADERS(Stream &stream)
 #ifndef QT_NO_NETWORKPROXY
     useProxy = m_connection->d_func()->networkProxy.type() != QNetworkProxy::NoProxy;
 #endif
+    if (stream.request().withCredentials()) {
+        m_connection->d_func()->createAuthorization(m_socket, stream.request());
+        stream.request().d->needResendWithCredentials = false;
+    }
     const auto headers = build_headers(stream.request(), maxHeaderListSize, useProxy);
     if (!headers.size()) // nothing fits into maxHeaderListSize
         return false;
@@ -520,7 +524,7 @@ bool QHttp2ProtocolHandler::sendDATA(Stream &stream)
     Q_ASSERT(replyPrivate);
 
     auto slot = std::min<qint32>(sessionSendWindowSize, stream.sendWindow);
-    while (!stream.data()->atEnd() && slot) {
+    while (replyPrivate->totallyUploadedData < request.contentLength() && slot) {
         qint64 chunkSize = 0;
         const uchar *src =
             reinterpret_cast<const uchar *>(stream.data()->readPointer(slot, chunkSize));
@@ -1020,8 +1024,10 @@ void QHttp2ProtocolHandler::handleContinuedHEADERS()
         if (activeStreams.contains(streamID)) {
             Stream &stream = activeStreams[streamID];
             updateStream(stream, decoder.decodedHeader());
-            // No DATA frames.
-            if (continuedFrames[0].flags() & FrameFlag::END_STREAM) {
+            // Needs to resend the request; we should finish and delete the current stream
+            const bool needResend = stream.request().d->needResendWithCredentials;
+            // No DATA frames. Or needs to resend.
+            if (continuedFrames[0].flags() & FrameFlag::END_STREAM || needResend) {
                 finishStream(stream);
                 deleteActiveStream(stream.streamID);
             }
@@ -1089,7 +1095,7 @@ bool QHttp2ProtocolHandler::acceptSetting(Http2::Settings identifier, quint32 ne
 
     if (identifier == Settings::MAX_FRAME_SIZE_ID) {
         if (newValue < Http2::minPayloadLimit || newValue > Http2::maxPayloadSize) {
-            connectionError(PROTOCOL_ERROR, "SETTGINGS max frame size is out of range");
+            connectionError(PROTOCOL_ERROR, "SETTINGS max frame size is out of range");
             return false;
         }
         maxFrameSize = newValue;
@@ -1109,7 +1115,7 @@ void QHttp2ProtocolHandler::updateStream(Stream &stream, const HPack::HttpHeader
                                          Qt::ConnectionType connectionType)
 {
     const auto httpReply = stream.reply();
-    const auto &httpRequest = stream.request();
+    auto &httpRequest = stream.request();
     Q_ASSERT(httpReply || stream.state == Stream::remoteReserved);
 
     if (!httpReply) {
@@ -1147,6 +1153,7 @@ void QHttp2ProtocolHandler::updateStream(Stream &stream, const HPack::HttpHeader
         if (name == ":status") {
             statusCode = value.left(3).toInt();
             httpReply->setStatusCode(statusCode);
+            m_channel->lastStatus = statusCode; // Mostly useless for http/2, needed for auth
             httpReplyPrivate->reasonPhrase = QString::fromLatin1(value.mid(4));
         } else if (name == ":version") {
             httpReplyPrivate->majorVersion = value.at(5) - '0';
@@ -1166,6 +1173,63 @@ void QHttp2ProtocolHandler::updateStream(Stream &stream, const HPack::HttpHeader
         }
     }
 
+    const auto handleAuth = [&, this](const QByteArray &authField, bool isProxy) -> bool {
+        Q_ASSERT(httpReply);
+        const auto auth = authField.trimmed();
+        if (auth.startsWith("Negotiate") || auth.startsWith("NTLM")) {
+            // @todo: We're supposed to fall back to http/1.1:
+            // https://docs.microsoft.com/en-us/iis/get-started/whats-new-in-iis-10/http2-on-iis#when-is-http2-not-supported
+            // "Windows authentication (NTLM/Kerberos/Negotiate) is not supported with HTTP/2.
+            // In this case IIS will fall back to HTTP/1.1."
+            // Though it might be OK to ignore this. The server shouldn't let us connect with
+            // HTTP/2 if it doesn't support us using it.
+        } else if (!auth.isEmpty()) {
+            // Somewhat mimics parts of QHttpNetworkConnectionChannel::handleStatus
+            bool resend = false;
+            const bool authenticateHandled = m_connection->d_func()->handleAuthenticateChallenge(
+                    m_socket, httpReply, isProxy, resend);
+            if (authenticateHandled && resend) {
+                httpReply->d_func()->eraseData();
+                // Add the request back in queue, we'll retry later now that
+                // we've gotten some username/password set on it:
+                httpRequest.d->needResendWithCredentials = true;
+                m_channel->h2RequestsToSend.insert(httpRequest.priority(), stream.httpPair);
+                httpReply->d_func()->clearHeaders();
+                // If we have data we were uploading we need to reset it:
+                if (stream.data()) {
+                    stream.data()->reset();
+                    httpReplyPrivate->totallyUploadedData = 0;
+                }
+                return true;
+            } // else: Authentication failed or was cancelled
+        }
+        return false;
+    };
+
+    if (httpReply) {
+        // See Note further down. These statuses would in HTTP/1.1 be handled
+        // by QHttpNetworkConnectionChannel::handleStatus. But because h2 has
+        // multiple streams/requests in a single channel this structure does not
+        // map properly to that function.
+        if (httpReply->statusCode() == 401) {
+            const auto wwwAuth = httpReply->headerField("www-authenticate");
+            if (handleAuth(wwwAuth, false)) {
+                sendRST_STREAM(stream.streamID, CANCEL);
+                markAsReset(stream.streamID);
+                // The stream is finalized and deleted after returning
+                return;
+            } // else: errors handled later
+        } else if (httpReply->statusCode() == 407) {
+            const auto proxyAuth = httpReply->headerField("proxy-authenticate");
+            if (handleAuth(proxyAuth, true)) {
+                sendRST_STREAM(stream.streamID, CANCEL);
+                markAsReset(stream.streamID);
+                // The stream is finalized and deleted after returning
+                return;
+            } // else: errors handled later
+        }
+    }
+
     if (QHttpNetworkReply::isHttpRedirect(statusCode) && redirectUrl.isValid())
         httpReply->setRedirectUrl(redirectUrl);
 
@@ -1177,15 +1241,16 @@ void QHttp2ProtocolHandler::updateStream(Stream &stream, const HPack::HttpHeader
             httpReplyPrivate->decompressHelper.setArchiveBombDetectionEnabled(false);
     }
 
-    if (QHttpNetworkReply::isHttpRedirect(statusCode)
-        || statusCode == 401 || statusCode == 407) {
-        // These are the status codes that can trigger uploadByteDevice->reset()
-        // in QHttpNetworkConnectionChannel::handleStatus. Alas, we have no
-        // single request/reply, we multiplex several requests and thus we never
-        // simply call 'handleStatus'. If we have byte-device - we try to reset
-        // it here, we don't (and can't) handle any error during reset operation.
-        if (stream.data())
+    if (QHttpNetworkReply::isHttpRedirect(statusCode)) {
+        // Note: This status code can trigger uploadByteDevice->reset() in
+        // QHttpNetworkConnectionChannel::handleStatus. Alas, we have no single
+        // request/reply, we multiplex several requests and thus we never simply
+        // call 'handleStatus'. If we have a byte-device - we try to reset it
+        // here, we don't (and can't) handle any error during reset operation.
+        if (stream.data()) {
             stream.data()->reset();
+            httpReplyPrivate->totallyUploadedData = 0;
+        }
     }
 
     if (connectionType == Qt::DirectConnection)
@@ -1259,10 +1324,12 @@ void QHttp2ProtocolHandler::finishStream(Stream &stream, Qt::ConnectionType conn
         if (stream.data())
             stream.data()->disconnect(this);
 
-        if (connectionType == Qt::DirectConnection)
-            emit httpReply->finished();
-        else
-            QMetaObject::invokeMethod(httpReply, "finished", connectionType);
+        if (!stream.request().d->needResendWithCredentials) {
+            if (connectionType == Qt::DirectConnection)
+                emit httpReply->finished();
+            else
+                QMetaObject::invokeMethod(httpReply, "finished", connectionType);
+        }
     }
 
     qCDebug(QT_HTTP2) << "stream" << stream.streamID << "closed";
diff --git a/src/network/access/qhttpnetworkconnection.cpp b/src/network/access/qhttpnetworkconnection.cpp
index 3508f9a629..021c1ce62d 100644
--- a/src/network/access/qhttpnetworkconnection.cpp
+++ b/src/network/access/qhttpnetworkconnection.cpp
@@ -506,8 +506,8 @@ bool QHttpNetworkConnectionPrivate::handleAuthenticateChallenge(QAbstractSocket
                 channels[i].authenticator = QAuthenticator();
 
             // authentication is cancelled, send the current contents to the user.
-            emit channels[i].reply->headerChanged();
-            emit channels[i].reply->readyRead();
+            emit reply->headerChanged();
+            emit reply->readyRead();
             QNetworkReply::NetworkError errorCode =
                 isProxy
                 ? QNetworkReply::ProxyAuthenticationRequiredError
diff --git a/src/network/access/qhttpnetworkconnectionchannel.cpp b/src/network/access/qhttpnetworkconnectionchannel.cpp
index 1ba833aef4..fddec35a3c 100644
--- a/src/network/access/qhttpnetworkconnectionchannel.cpp
+++ b/src/network/access/qhttpnetworkconnectionchannel.cpp
@@ -687,17 +687,19 @@ bool QHttpNetworkConnectionChannel::resetUploadData()
         //this happens if server closes connection while QHttpNetworkConnectionPrivate::_q_startNextRequest is pending
         return false;
     }
-    QNonContiguousByteDevice* uploadByteDevice = request.uploadByteDevice();
-    if (!uploadByteDevice)
-        return true;
-
-    if (uploadByteDevice->reset()) {
+    if (connection->connectionType() == QHttpNetworkConnection::ConnectionTypeHTTP2Direct
+        || switchedToHttp2) {
+        // The else branch doesn't make any sense for HTTP/2, since 1 channel is multiplexed into
+        // many streams. And having one stream fail to reset upload data should not completely close
+        // the channel. Handled in the http2 protocol handler.
+    } else if (QNonContiguousByteDevice *uploadByteDevice = request.uploadByteDevice()) {
+        if (!uploadByteDevice->reset()) {
+            connection->d_func()->emitReplyError(socket, reply, QNetworkReply::ContentReSendError);
+            return false;
+        }
         written = 0;
-        return true;
-    } else {
-        connection->d_func()->emitReplyError(socket, reply, QNetworkReply::ContentReSendError);
-        return false;
     }
+    return true;
 }
 
 #ifndef QT_NO_NETWORKPROXY
diff --git a/src/network/access/qhttpnetworkrequest.cpp b/src/network/access/qhttpnetworkrequest.cpp
index c0b2167d15..3518dba9ed 100644
--- a/src/network/access/qhttpnetworkrequest.cpp
+++ b/src/network/access/qhttpnetworkrequest.cpp
@@ -65,6 +65,7 @@ QHttpNetworkRequestPrivate::QHttpNetworkRequestPrivate(const QHttpNetworkRequest
       ssl(other.ssl),
       preConnect(other.preConnect),
       ignoreDecompressionRatio(other.ignoreDecompressionRatio),
+      needResendWithCredentials(other.needResendWithCredentials),
       redirectCount(other.redirectCount),
       redirectPolicy(other.redirectPolicy),
       peerVerifyName(other.peerVerifyName)
@@ -91,7 +92,8 @@ bool QHttpNetworkRequestPrivate::operator==(const QHttpNetworkRequestPrivate &ot
         && (ssl == other.ssl)
         && (preConnect == other.preConnect)
         && (redirectPolicy == other.redirectPolicy)
-        && (peerVerifyName == other.peerVerifyName);
+        && (peerVerifyName == other.peerVerifyName)
+        && (needResendWithCredentials == other.needResendWithCredentials);
 }
 
 QByteArray QHttpNetworkRequest::methodName() const
diff --git a/src/network/access/qhttpnetworkrequest_p.h b/src/network/access/qhttpnetworkrequest_p.h
index 1a38b24a8a..f18ab1c877 100644
--- a/src/network/access/qhttpnetworkrequest_p.h
+++ b/src/network/access/qhttpnetworkrequest_p.h
@@ -185,6 +185,7 @@ public:
     bool ssl;
     bool preConnect;
     bool ignoreDecompressionRatio = false;
+    bool needResendWithCredentials = false;
     int redirectCount;
     QNetworkRequest::RedirectPolicy redirectPolicy;
     QString peerVerifyName;
diff --git a/tests/auto/network/access/http2/http2srv.cpp b/tests/auto/network/access/http2/http2srv.cpp
index 9513744476..dfd0a69327 100644
--- a/tests/auto/network/access/http2/http2srv.cpp
+++ b/tests/auto/network/access/http2/http2srv.cpp
@@ -125,6 +125,11 @@ void Http2Server::setContentEncoding(const QByteArray &encoding)
     contentEncoding = encoding;
 }
 
+void Http2Server::setAuthenticationHeader(const QByteArray &authentication)
+{
+    authenticationHeader = authentication;
+}
+
 void Http2Server::emulateGOAWAY(int timeout)
 {
     Q_ASSERT(timeout >= 0);
@@ -143,6 +148,17 @@ bool Http2Server::isClearText() const
     return connectionType == H2Type::h2c || connectionType == H2Type::h2cDirect;
 }
 
+QByteArray Http2Server::requestAuthorizationHeader()
+{
+    const auto isAuthHeader = [](const HeaderField &field) {
+        return field.name == "authorization";
+    };
+    const auto requestHeaders = decoder.decodedHeader();
+    const auto authentication =
+            std::find_if(requestHeaders.cbegin(), requestHeaders.cend(), isAuthHeader);
+    return authentication == requestHeaders.cend() ? QByteArray() : authentication->value;
+}
+
 void Http2Server::startServer()
 {
     if (listen()) {
@@ -741,6 +757,9 @@ void Http2Server::handleDATA()
         streamWindows.erase(it);
         emit receivedData(streamID);
     }
+    emit receivedDATAFrame(streamID,
+                           QByteArray(reinterpret_cast<const char *>(inboundFrame.dataBegin()),
+                                      inboundFrame.dataSize()));
 }
 
 void Http2Server::handleWINDOW_UPDATE()
@@ -821,6 +840,9 @@ void Http2Server::sendResponse(quint32 streamID, bool emptyBody)
     if (emptyBody)
         writer.addFlag(FrameFlag::END_STREAM);
 
+    // We assume any auth is correct. Leaves the checking to the test itself
+    const bool hasAuth = !requestAuthorizationHeader().isEmpty();
+
     HttpHeader header;
     if (redirectWhileReading) {
         if (redirectSent) {
@@ -837,6 +859,10 @@ void Http2Server::sendResponse(quint32 streamID, bool emptyBody)
         header.push_back({"location", url.arg(isClearText() ? QStringLiteral("http") : QStringLiteral("https"),
                                               QString::number(targetPort)).toLatin1()});
 
+    } else if (!authenticationHeader.isEmpty() && !hasAuth) {
+        header.push_back({ ":status", "401" });
+        header.push_back(HPack::HeaderField("www-authenticate", authenticationHeader));
+        authenticationHeader.clear();
     } else {
         header.push_back({":status", "200"});
     }
diff --git a/tests/auto/network/access/http2/http2srv.h b/tests/auto/network/access/http2/http2srv.h
index baf0155988..671cacbd54 100644
--- a/tests/auto/network/access/http2/http2srv.h
+++ b/tests/auto/network/access/http2/http2srv.h
@@ -88,11 +88,15 @@ public:
     void setResponseBody(const QByteArray &body);
     // No content encoding is actually performed, call setResponseBody with already encoded data
     void setContentEncoding(const QByteArray &contentEncoding);
+    // No authentication data is generated for the method, the full header value must be set
+    void setAuthenticationHeader(const QByteArray &authentication);
     void emulateGOAWAY(int timeout);
     void redirectOpenStream(quint16 targetPort);
 
     bool isClearText() const;
 
+    QByteArray requestAuthorizationHeader();
+
     // Invokables, since we can call them from the main thread,
     // but server (can) work on its own thread.
     Q_INVOKABLE void startServer();
@@ -129,6 +133,8 @@ Q_SIGNALS:
     void decompressionFailed(quint32 streamID);
     void receivedRequest(quint32 streamID);
     void receivedData(quint32 streamID);
+    // Emitted for every DATA frame. Includes the content of the frame as \a body.
+    void receivedDATAFrame(quint32 streamID, const QByteArray &body);
     void windowUpdate(quint32 streamID);
     void sendingData();
 
@@ -215,6 +221,7 @@ private:
     QAtomicInt interrupted;
 
     QByteArray contentEncoding;
+    QByteArray authenticationHeader;
 protected slots:
     void ignoreErrorSlot();
 };
diff --git a/tests/auto/network/access/http2/tst_http2.cpp b/tests/auto/network/access/http2/tst_http2.cpp
index 0282942225..29121581f8 100644
--- a/tests/auto/network/access/http2/tst_http2.cpp
+++ b/tests/auto/network/access/http2/tst_http2.cpp
@@ -114,6 +114,9 @@ private slots:
     void contentEncoding_data();
     void contentEncoding();
 
+    void authenticationRequired_data();
+    void authenticationRequired();
+
 protected slots:
     // Slots to listen to our in-process server:
     void serverStarted(quint16 port);
@@ -883,6 +886,84 @@ void tst_Http2::contentEncoding()
     QTEST(reply->readAll(), "expected");
 }
 
+void tst_Http2::authenticationRequired_data()
+{
+    QTest::addColumn<bool>("success");
+
+    QTest::addRow("failed-auth") << false;
+    QTest::addRow("successful-auth") << true;
+}
+
+void tst_Http2::authenticationRequired()
+{
+    clearHTTP2State();
+
+    QFETCH(const bool, success);
+
+    ServerPtr targetServer(newServer(defaultServerSettings, defaultConnectionType()));
+    targetServer->setResponseBody("Hello");
+    targetServer->setAuthenticationHeader("Basic realm=\"Shadow\"");
+
+    QMetaObject::invokeMethod(targetServer.data(), "startServer", Qt::QueuedConnection);
+    runEventLoop();
+
+    QVERIFY(serverPort != 0);
+
+    nRequests = 1;
+
+    auto url = requestUrl(defaultConnectionType());
+    url.setPath("/index.html");
+    QNetworkRequest request(url);
+
+    QByteArray expectedBody = "Hello, World!";
+    request.setHeader(QNetworkRequest::ContentTypeHeader, "application/x-www-form-urlencoded");
+    QScopedPointer<QNetworkReply> reply;
+    reply.reset(manager->post(request, expectedBody));
+
+    bool authenticationRequested = false;
+    connect(manager.get(), &QNetworkAccessManager::authenticationRequired, reply.get(),
+            [&](QNetworkReply *, QAuthenticator *auth) {
+                authenticationRequested = true;
+                if (success) {
+                    auth->setUser("admin");
+                    auth->setPassword("admin");
+                }
+            });
+
+    QByteArray receivedBody;
+    connect(targetServer.get(), &Http2Server::receivedDATAFrame, reply.get(),
+            [&receivedBody](quint32 streamID, const QByteArray &body) {
+                if (streamID == 3) // The expected body is on the retry, so streamID == 3
+                    receivedBody += body;
+            });
+
+    if (success)
+        connect(reply.get(), &QNetworkReply::finished, this, &tst_Http2::replyFinished);
+    else
+        connect(reply.get(), &QNetworkReply::errorOccurred, this, &tst_Http2::replyFinishedWithError);
+    // Since we're using self-signed certificates,
+    // ignore SSL errors:
+    reply->ignoreSslErrors();
+
+    runEventLoop();
+    STOP_ON_FAILURE
+
+    if (!success)
+        QCOMPARE(reply->error(), QNetworkReply::AuthenticationRequiredError);
+    // else: no error (is checked in tst_Http2::replyFinished)
+
+    QVERIFY(authenticationRequested);
+
+    const auto isAuthenticated = [](QByteArray bv) {
+        return bv == "Basic YWRtaW46YWRtaW4="; // admin:admin
+    };
+    // Get the "authorization" header out from the server and make sure it's as expected:
+    auto reqAuthHeader = targetServer->requestAuthorizationHeader();
+    QCOMPARE(isAuthenticated(reqAuthHeader), success);
+    if (success)
+        QCOMPARE(receivedBody, expectedBody);
+}
+
 void tst_Http2::serverStarted(quint16 port)
 {
     serverPort = port;