From e4f92ad1a1052db1e462e6992867a956372091c9 Mon Sep 17 00:00:00 2001 From: Felix Weilbach Date: Tue, 13 Jul 2021 14:19:47 +0200 Subject: [PATCH 1/4] Enforce https in flow1 and flow2 for https connections Signed-off-by: Felix Weilbach --- src/gui/creds/flow2auth.cpp | 14 ++++++++++++++ src/gui/creds/flow2auth.h | 1 + src/gui/wizard/webview.cpp | 23 ++++++++++++++++++++++- 3 files changed, 37 insertions(+), 1 deletion(-) diff --git a/src/gui/creds/flow2auth.cpp b/src/gui/creds/flow2auth.cpp index f8281ea4dd9cc..70566c0b43be2 100644 --- a/src/gui/creds/flow2auth.cpp +++ b/src/gui/creds/flow2auth.cpp @@ -79,6 +79,9 @@ void Flow2Auth::fetchNewToken(const TokenAction action) // Step 1: Initiate a login, do an anonymous POST request QUrl url = Utility::concatUrlPath(_account->url().toString(), QLatin1String("/index.php/login/v2")); + if (url.scheme() == "https") { + _enforceHttps = true; + } // add 'Content-Length: 0' header (see https://github.com/nextcloud/desktop/issues/1473) QNetworkRequest req; @@ -187,6 +190,12 @@ void Flow2Auth::slotPollTimerTimeout() QUrlQuery arguments(QString("token=%1").arg(_pollToken)); requestBody->setData(arguments.query(QUrl::FullyEncoded).toLatin1()); + if (_enforceHttps && QUrl(_pollEndpoint).scheme() != "https") { + qCWarning(lcFlow2auth) << "Can not poll endpoint because the returned url" << _pollEndpoint << "does not start with https"; + emit result(Error, tr("The polling URL does not start with https despite the login URL started with https. Login will not be possible because this might be a security issue. Please contact your administrator.")); + return; + } + auto job = _account->sendRequest("POST", _pollEndpoint, req, requestBody); job->setTimeout(qMin(30 * 1000ll, job->timeoutMsec())); @@ -200,6 +209,11 @@ void Flow2Auth::slotPollTimerTimeout() if (reply->error() == QNetworkReply::NoError && jsonParseError.error == QJsonParseError::NoError && !json.isEmpty()) { serverUrl = json["server"].toString(); + if (_enforceHttps && serverUrl.scheme() != "https") { + qCWarning(lcFlow2auth) << "Returned server url" << serverUrl << "does not start with https"; + emit result(Error, tr("The returned server URL does not start with https despite the login URL started with https. Login will not be possible because this might be a security issue. Please contact your administrator.")); + return; + } loginName = json["loginName"].toString(); appPassword = json["appPassword"].toString(); } diff --git a/src/gui/creds/flow2auth.h b/src/gui/creds/flow2auth.h index 907ead5881bc2..6cb05c1d39680 100644 --- a/src/gui/creds/flow2auth.h +++ b/src/gui/creds/flow2auth.h @@ -81,6 +81,7 @@ private slots: qint64 _secondsInterval; bool _isBusy; bool _hasToken; + bool _enforceHttps = false; }; } // namespace OCC diff --git a/src/gui/wizard/webview.cpp b/src/gui/wizard/webview.cpp index 6c2207f486805..125969bde2ebc 100644 --- a/src/gui/wizard/webview.cpp +++ b/src/gui/wizard/webview.cpp @@ -52,6 +52,11 @@ class WebEnginePage : public QWebEnginePage { protected: bool certificateError(const QWebEngineCertificateError &certificateError) override; + + bool acceptNavigationRequest(const QUrl &url, QWebEnginePage::NavigationType type, bool isMainFrame) override; + +private: + bool _enforceHttps = false; }; // We need a separate class here, since we cannot simply return the same WebEnginePage object @@ -186,8 +191,15 @@ QWebEnginePage * WebEnginePage::createWindow(QWebEnginePage::WebWindowType type) return view; } -void WebEnginePage::setUrl(const QUrl &url) { +void WebEnginePage::setUrl(const QUrl &url) +{ QWebEnginePage::setUrl(url); + + if (url.scheme() == "https") { + _enforceHttps = true; + } else { + _enforceHttps = false; + } } bool WebEnginePage::certificateError(const QWebEngineCertificateError &certificateError) @@ -211,6 +223,15 @@ bool WebEnginePage::certificateError(const QWebEngineCertificateError &certifica return ret == QMessageBox::Yes; } +bool WebEnginePage::acceptNavigationRequest(const QUrl &url, QWebEnginePage::NavigationType /*type*/, bool /*isMainFrame*/) +{ + if (_enforceHttps && url.scheme() != "https") { + QMessageBox::warning(nullptr, "Security warning", "Can not follow non https link on a https website. This might be a security issue. Please contact your administrator"); + return false; + } + return true; +} + ExternalWebEnginePage::ExternalWebEnginePage(QWebEngineProfile *profile, QObject* parent) : QWebEnginePage(profile, parent) { } From ee49a7ed52861592ba5cec95f391a92060170077 Mon Sep 17 00:00:00 2001 From: Felix Weilbach Date: Fri, 16 Jul 2021 11:32:56 +0200 Subject: [PATCH 2/4] Fix review comments Signed-off-by: Felix Weilbach --- src/gui/creds/flow2auth.cpp | 15 ++++++--------- src/gui/wizard/webview.cpp | 7 +------ 2 files changed, 7 insertions(+), 15 deletions(-) diff --git a/src/gui/creds/flow2auth.cpp b/src/gui/creds/flow2auth.cpp index 70566c0b43be2..4c4245e5f1c00 100644 --- a/src/gui/creds/flow2auth.cpp +++ b/src/gui/creds/flow2auth.cpp @@ -79,9 +79,7 @@ void Flow2Auth::fetchNewToken(const TokenAction action) // Step 1: Initiate a login, do an anonymous POST request QUrl url = Utility::concatUrlPath(_account->url().toString(), QLatin1String("/index.php/login/v2")); - if (url.scheme() == "https") { - _enforceHttps = true; - } + _enforceHttps = url.scheme() == "https"; // add 'Content-Length: 0' header (see https://github.com/nextcloud/desktop/issues/1473) QNetworkRequest req; @@ -101,6 +99,11 @@ void Flow2Auth::fetchNewToken(const TokenAction action) && !json.isEmpty()) { pollToken = json.value("poll").toObject().value("token").toString(); pollEndpoint = json.value("poll").toObject().value("endpoint").toString(); + if (_enforceHttps && QUrl(pollEndpoint).scheme() != "https") { + qCWarning(lcFlow2auth) << "Can not poll endpoint because the returned url" << _pollEndpoint << "does not start with https"; + emit result(Error, tr("The polling URL does not start with https despite the login URL started with https. Login will not be possible because this might be a security issue. Please contact your administrator.")); + return; + } loginUrl = json["login"].toString(); } @@ -190,12 +193,6 @@ void Flow2Auth::slotPollTimerTimeout() QUrlQuery arguments(QString("token=%1").arg(_pollToken)); requestBody->setData(arguments.query(QUrl::FullyEncoded).toLatin1()); - if (_enforceHttps && QUrl(_pollEndpoint).scheme() != "https") { - qCWarning(lcFlow2auth) << "Can not poll endpoint because the returned url" << _pollEndpoint << "does not start with https"; - emit result(Error, tr("The polling URL does not start with https despite the login URL started with https. Login will not be possible because this might be a security issue. Please contact your administrator.")); - return; - } - auto job = _account->sendRequest("POST", _pollEndpoint, req, requestBody); job->setTimeout(qMin(30 * 1000ll, job->timeoutMsec())); diff --git a/src/gui/wizard/webview.cpp b/src/gui/wizard/webview.cpp index 125969bde2ebc..f6560ddc1af38 100644 --- a/src/gui/wizard/webview.cpp +++ b/src/gui/wizard/webview.cpp @@ -194,12 +194,7 @@ QWebEnginePage * WebEnginePage::createWindow(QWebEnginePage::WebWindowType type) void WebEnginePage::setUrl(const QUrl &url) { QWebEnginePage::setUrl(url); - - if (url.scheme() == "https") { - _enforceHttps = true; - } else { - _enforceHttps = false; - } + _enforceHttps = url.scheme() == "https"; } bool WebEnginePage::certificateError(const QWebEngineCertificateError &certificateError) From 3f6bb4a929c5b909522612498de35c8e03c9563d Mon Sep 17 00:00:00 2001 From: Felix Weilbach Date: Mon, 19 Jul 2021 11:11:56 +0200 Subject: [PATCH 3/4] Use QStringLiteral instead of plain strings Signed-off-by: Felix Weilbach --- src/gui/creds/flow2auth.cpp | 6 +++--- src/gui/wizard/webview.cpp | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/gui/creds/flow2auth.cpp b/src/gui/creds/flow2auth.cpp index 4c4245e5f1c00..468322138975a 100644 --- a/src/gui/creds/flow2auth.cpp +++ b/src/gui/creds/flow2auth.cpp @@ -79,7 +79,7 @@ void Flow2Auth::fetchNewToken(const TokenAction action) // Step 1: Initiate a login, do an anonymous POST request QUrl url = Utility::concatUrlPath(_account->url().toString(), QLatin1String("/index.php/login/v2")); - _enforceHttps = url.scheme() == "https"; + _enforceHttps = url.scheme() == QStringLiteral("https"); // add 'Content-Length: 0' header (see https://github.com/nextcloud/desktop/issues/1473) QNetworkRequest req; @@ -99,7 +99,7 @@ void Flow2Auth::fetchNewToken(const TokenAction action) && !json.isEmpty()) { pollToken = json.value("poll").toObject().value("token").toString(); pollEndpoint = json.value("poll").toObject().value("endpoint").toString(); - if (_enforceHttps && QUrl(pollEndpoint).scheme() != "https") { + if (_enforceHttps && QUrl(pollEndpoint).scheme() != QStringLiteral("https")) { qCWarning(lcFlow2auth) << "Can not poll endpoint because the returned url" << _pollEndpoint << "does not start with https"; emit result(Error, tr("The polling URL does not start with https despite the login URL started with https. Login will not be possible because this might be a security issue. Please contact your administrator.")); return; @@ -206,7 +206,7 @@ void Flow2Auth::slotPollTimerTimeout() if (reply->error() == QNetworkReply::NoError && jsonParseError.error == QJsonParseError::NoError && !json.isEmpty()) { serverUrl = json["server"].toString(); - if (_enforceHttps && serverUrl.scheme() != "https") { + if (_enforceHttps && serverUrl.scheme() != QStringLiteral("https")) { qCWarning(lcFlow2auth) << "Returned server url" << serverUrl << "does not start with https"; emit result(Error, tr("The returned server URL does not start with https despite the login URL started with https. Login will not be possible because this might be a security issue. Please contact your administrator.")); return; diff --git a/src/gui/wizard/webview.cpp b/src/gui/wizard/webview.cpp index f6560ddc1af38..26c9e2107c689 100644 --- a/src/gui/wizard/webview.cpp +++ b/src/gui/wizard/webview.cpp @@ -194,7 +194,7 @@ QWebEnginePage * WebEnginePage::createWindow(QWebEnginePage::WebWindowType type) void WebEnginePage::setUrl(const QUrl &url) { QWebEnginePage::setUrl(url); - _enforceHttps = url.scheme() == "https"; + _enforceHttps = url.scheme() == QStringLiteral("https"); } bool WebEnginePage::certificateError(const QWebEngineCertificateError &certificateError) @@ -220,7 +220,7 @@ bool WebEnginePage::certificateError(const QWebEngineCertificateError &certifica bool WebEnginePage::acceptNavigationRequest(const QUrl &url, QWebEnginePage::NavigationType /*type*/, bool /*isMainFrame*/) { - if (_enforceHttps && url.scheme() != "https") { + if (_enforceHttps && url.scheme() != QStringLiteral("https")) { QMessageBox::warning(nullptr, "Security warning", "Can not follow non https link on a https website. This might be a security issue. Please contact your administrator"); return false; } From 302651620b3b7612ef00fd91f27eacbfaf0f491b Mon Sep 17 00:00:00 2001 From: Felix Weilbach Date: Mon, 19 Jul 2021 11:13:22 +0200 Subject: [PATCH 4/4] Use Q_UNUSED instead of commenting out arg names Signed-off-by: Felix Weilbach --- src/gui/wizard/webview.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/gui/wizard/webview.cpp b/src/gui/wizard/webview.cpp index 26c9e2107c689..d9cd20922935b 100644 --- a/src/gui/wizard/webview.cpp +++ b/src/gui/wizard/webview.cpp @@ -218,8 +218,11 @@ bool WebEnginePage::certificateError(const QWebEngineCertificateError &certifica return ret == QMessageBox::Yes; } -bool WebEnginePage::acceptNavigationRequest(const QUrl &url, QWebEnginePage::NavigationType /*type*/, bool /*isMainFrame*/) +bool WebEnginePage::acceptNavigationRequest(const QUrl &url, QWebEnginePage::NavigationType type, bool isMainFrame) { + Q_UNUSED(type); + Q_UNUSED(isMainFrame); + if (_enforceHttps && url.scheme() != QStringLiteral("https")) { QMessageBox::warning(nullptr, "Security warning", "Can not follow non https link on a https website. This might be a security issue. Please contact your administrator"); return false;