From 130c0d848751ca0ebf236ddc09310b7d3a32a80e Mon Sep 17 00:00:00 2001 From: Chocobo1 Date: Sat, 7 Sep 2024 21:38:27 +0800 Subject: [PATCH] Revise cookie 'secure flag' enable condition The localhost is 'potentially trustworthy' and RFC 6265 allows setting secure flag in this case. Also check `X-Forwarded-Proto` header value to support reverse proxy usage. Note: for reverse proxy users, now the `X-Forwarded-Proto` header is expected to be sent to qbt otherwise the `secure` flag might be set erroneously. https://datatracker.ietf.org/doc/html/rfc6265#section-4.1.2.5 https://w3c.github.io/webappsec-secure-contexts/#is-origin-trustworthy Closes #21250. PR #21260. --- src/base/http/types.h | 1 + src/gui/optionsdialog.cpp | 2 -- src/gui/optionsdialog.ui | 2 +- src/webui/webapplication.cpp | 23 +++++++++++++++++++- src/webui/webapplication.h | 2 ++ src/webui/www/private/views/preferences.html | 3 +-- 6 files changed, 27 insertions(+), 6 deletions(-) diff --git a/src/base/http/types.h b/src/base/http/types.h index 4f79c0d20..8641003e8 100644 --- a/src/base/http/types.h +++ b/src/base/http/types.h @@ -57,6 +57,7 @@ namespace Http inline const QString HEADER_X_CONTENT_TYPE_OPTIONS = u"x-content-type-options"_s; inline const QString HEADER_X_FORWARDED_FOR = u"x-forwarded-for"_s; inline const QString HEADER_X_FORWARDED_HOST = u"x-forwarded-host"_s; + inline const QString HEADER_X_FORWARDED_PROTO = u"X-forwarded-proto"_s; inline const QString HEADER_X_FRAME_OPTIONS = u"x-frame-options"_s; inline const QString HEADER_X_XSS_PROTECTION = u"x-xss-protection"_s; diff --git a/src/gui/optionsdialog.cpp b/src/gui/optionsdialog.cpp index d3bad9f98..264018117 100644 --- a/src/gui/optionsdialog.cpp +++ b/src/gui/optionsdialog.cpp @@ -1273,7 +1273,6 @@ void OptionsDialog::loadWebUITabOptions() // Security m_ui->checkClickjacking->setChecked(pref->isWebUIClickjackingProtectionEnabled()); m_ui->checkCSRFProtection->setChecked(pref->isWebUICSRFProtectionEnabled()); - m_ui->checkSecureCookie->setEnabled(pref->isWebUIHttpsEnabled()); m_ui->checkSecureCookie->setChecked(pref->isWebUISecureCookieEnabled()); m_ui->groupHostHeaderValidation->setChecked(pref->isWebUIHostHeaderValidationEnabled()); m_ui->textServerDomains->setText(pref->getServerDomains()); @@ -1315,7 +1314,6 @@ void OptionsDialog::loadWebUITabOptions() connect(m_ui->checkClickjacking, &QCheckBox::toggled, this, &ThisType::enableApplyButton); connect(m_ui->checkCSRFProtection, &QCheckBox::toggled, this, &ThisType::enableApplyButton); - connect(m_ui->checkWebUIHttps, &QGroupBox::toggled, m_ui->checkSecureCookie, &QWidget::setEnabled); connect(m_ui->checkSecureCookie, &QCheckBox::toggled, this, &ThisType::enableApplyButton); connect(m_ui->groupHostHeaderValidation, &QGroupBox::toggled, this, &ThisType::enableApplyButton); connect(m_ui->textServerDomains, &QLineEdit::textChanged, this, &ThisType::enableApplyButton); diff --git a/src/gui/optionsdialog.ui b/src/gui/optionsdialog.ui index 11299615f..3534fdc1e 100644 --- a/src/gui/optionsdialog.ui +++ b/src/gui/optionsdialog.ui @@ -3675,7 +3675,7 @@ Specify an IPv4 or IPv6 address. You can specify "0.0.0.0" for any IPv - Enable cookie Secure flag (requires HTTPS) + Enable cookie Secure flag (requires HTTPS or localhost connection) diff --git a/src/webui/webapplication.cpp b/src/webui/webapplication.cpp index f5c771454..ddcf39176 100644 --- a/src/webui/webapplication.cpp +++ b/src/webui/webapplication.cpp @@ -744,7 +744,7 @@ void WebApplication::sessionStart() QNetworkCookie cookie {m_sessionCookieName.toLatin1(), m_currentSession->id().toLatin1()}; cookie.setHttpOnly(true); - cookie.setSecure(m_isSecureCookieEnabled && m_isHttpsEnabled); + cookie.setSecure(m_isSecureCookieEnabled && isOriginTrustworthy()); // [rfc6265] 4.1.2.5. The Secure Attribute cookie.setPath(u"/"_s); if (m_isCSRFProtectionEnabled) cookie.setSameSitePolicy(QNetworkCookie::SameSite::Strict); @@ -767,6 +767,27 @@ void WebApplication::sessionEnd() setHeader({Http::HEADER_SET_COOKIE, QString::fromLatin1(cookie.toRawForm())}); } +bool WebApplication::isOriginTrustworthy() const +{ + // https://w3c.github.io/webappsec-secure-contexts/#is-origin-trustworthy + + if (m_isReverseProxySupportEnabled) + { + const QString forwardedProto = request().headers.value(Http::HEADER_X_FORWARDED_PROTO); + if (forwardedProto.compare(u"https", Qt::CaseInsensitive) == 0) + return true; + } + + if (m_isHttpsEnabled) + return true; + + // client is on localhost + if (env().clientAddress.isLoopback()) + return true; + + return false; +} + bool WebApplication::isCrossSiteRequest(const Http::Request &request) const { // https://www.owasp.org/index.php/Cross-Site_Request_Forgery_(CSRF)_Prevention_Cheat_Sheet#Verifying_Same_Origin_with_Standard_Headers diff --git a/src/webui/webapplication.h b/src/webui/webapplication.h index 1aa266a3a..80530b15d 100644 --- a/src/webui/webapplication.h +++ b/src/webui/webapplication.h @@ -128,9 +128,11 @@ private: bool isAuthNeeded(); bool isPublicAPI(const QString &scope, const QString &action) const; + bool isOriginTrustworthy() const; bool isCrossSiteRequest(const Http::Request &request) const; bool validateHostHeader(const QStringList &domains) const; + // reverse proxy QHostAddress resolveClientAddress() const; // Persistent data diff --git a/src/webui/www/private/views/preferences.html b/src/webui/www/private/views/preferences.html index e93bdcf54..f878118f6 100644 --- a/src/webui/www/private/views/preferences.html +++ b/src/webui/www/private/views/preferences.html @@ -988,7 +988,7 @@
- +
@@ -1965,7 +1965,6 @@ Use ';' to split multiple entries. Can use wildcard '*'.)QBT_TR[CONTEXT=OptionsD const isUseHttpsEnabled = $("use_https_checkbox").checked; $("ssl_cert_text").disabled = !isUseHttpsEnabled; $("ssl_key_text").disabled = !isUseHttpsEnabled; - $("secureCookieCheckbox").disabled = !isUseHttpsEnabled; }; const updateBypasssAuthSettings = function() {