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
Pick-to: 6.1 6.0 5.15
Change-Id: I798d105b02688b18a02897cc476f19f57a47f98f
Reviewed-by: Timur Pocheptsov <timur.pocheptsov@qt.io>
Reviewed-by: Edward Welbourne <edward.welbourne@qt.io>

5 files changed, 101 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 eb4ad6f618..13c86c6a9b 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 bf4df3626b..1081266bb1 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;