From e8a69dd60c28e5e3ab01662aaad7137d97d205f7 Mon Sep 17 00:00:00 2001 From: Chocobo1 Date: Mon, 21 May 2018 23:55:29 +0800 Subject: [PATCH 1/3] Update Options dialog layout in WebUI --- .../www/private/preferences_content.html | 40 +++++++++---------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/src/webui/www/private/preferences_content.html b/src/webui/www/private/preferences_content.html index 85af4571b..5020fd938 100644 --- a/src/webui/www/private/preferences_content.html +++ b/src/webui/www/private/preferences_content.html @@ -437,27 +437,27 @@
QBT_TR(Information about certificates)QBT_TR[CONTEXT=HttpServer]
- -
- QBT_TR(Authentication)QBT_TR[CONTEXT=OptionsDialog] -
- -
-
- -
-
- - -
-
- - -
-
- -
+
+ QBT_TR(Authentication)QBT_TR[CONTEXT=OptionsDialog] +
+ +
+
+ +
+
+ + +
+
+ + +
+
+ +
+
From bad4d94f778a1ceba8a0e16d1836e649d767cca8 Mon Sep 17 00:00:00 2001 From: Chocobo1 Date: Mon, 21 May 2018 23:33:44 +0800 Subject: [PATCH 2/3] Add option to control WebUI clickjacking protection Some users actually want embedding WebUI into their custom build iframe. Closes #7370. --- src/base/preferences.cpp | 10 ++++++++++ src/base/preferences.h | 4 ++++ src/gui/optionsdlg.cpp | 6 ++++++ src/gui/optionsdlg.ui | 7 +++++++ src/webui/api/appcontroller.cpp | 5 +++++ src/webui/webapplication.cpp | 10 +++++++--- src/webui/webapplication.h | 3 +++ src/webui/www/private/preferences_content.html | 10 ++++++++++ 8 files changed, 52 insertions(+), 3 deletions(-) diff --git a/src/base/preferences.cpp b/src/base/preferences.cpp index 076a73773..0f6a458de 100644 --- a/src/base/preferences.cpp +++ b/src/base/preferences.cpp @@ -576,6 +576,16 @@ void Preferences::setWebUiPassword(const QString &new_password) setValue("Preferences/WebUI/Password_ha1", md5.result().toHex()); } +bool Preferences::isWebUiClickjackingProtectionEnabled() const +{ + return value("Preferences/WebUI/ClickjackingProtection", true).toBool(); +} + +void Preferences::setWebUiClickjackingProtectionEnabled(bool enabled) +{ + setValue("Preferences/WebUI/ClickjackingProtection", enabled); +} + bool Preferences::isWebUiHttpsEnabled() const { return value("Preferences/WebUI/HTTPS/Enabled", false).toBool(); diff --git a/src/base/preferences.h b/src/base/preferences.h index c9a0188d2..46616591d 100644 --- a/src/base/preferences.h +++ b/src/base/preferences.h @@ -194,6 +194,10 @@ public: QString getWebUiPassword() const; void setWebUiPassword(const QString &new_password); + // WebUI security + bool isWebUiClickjackingProtectionEnabled() const; + void setWebUiClickjackingProtectionEnabled(bool enabled); + // HTTPS bool isWebUiHttpsEnabled() const; void setWebUiHttpsEnabled(bool enabled); diff --git a/src/gui/optionsdlg.cpp b/src/gui/optionsdlg.cpp index dac450aaf..23af0a055 100644 --- a/src/gui/optionsdlg.cpp +++ b/src/gui/optionsdlg.cpp @@ -378,6 +378,7 @@ OptionsDialog::OptionsDialog(QWidget *parent) connect(m_ui->checkBypassLocalAuth, &QAbstractButton::toggled, this, &ThisType::enableApplyButton); connect(m_ui->checkBypassAuthSubnetWhitelist, &QAbstractButton::toggled, this, &ThisType::enableApplyButton); connect(m_ui->checkBypassAuthSubnetWhitelist, &QAbstractButton::toggled, m_ui->IPSubnetWhitelistButton, &QPushButton::setEnabled); + connect(m_ui->checkClickjacking, &QCheckBox::toggled, this, &ThisType::enableApplyButton); connect(m_ui->checkDynDNS, &QGroupBox::toggled, this, &ThisType::enableApplyButton); connect(m_ui->comboDNSService, qComboBoxCurrentIndexChanged, this, &ThisType::enableApplyButton); connect(m_ui->domainNameTxt, &QLineEdit::textChanged, this, &ThisType::enableApplyButton); @@ -694,6 +695,8 @@ void OptionsDialog::saveOptions() pref->setWebUiPassword(webUiPassword()); pref->setWebUiLocalAuthEnabled(!m_ui->checkBypassLocalAuth->isChecked()); pref->setWebUiAuthSubnetWhitelistEnabled(m_ui->checkBypassAuthSubnetWhitelist->isChecked()); + // Security + pref->setWebUiClickjackingProtectionEnabled(m_ui->checkClickjacking->isChecked()); // DynDNS pref->setDynDNSEnabled(m_ui->checkDynDNS->isChecked()); pref->setDynDNSService(m_ui->comboDNSService->currentIndex()); @@ -1096,6 +1099,9 @@ void OptionsDialog::loadOptions() m_ui->checkBypassAuthSubnetWhitelist->setChecked(pref->isWebUiAuthSubnetWhitelistEnabled()); m_ui->IPSubnetWhitelistButton->setEnabled(m_ui->checkBypassAuthSubnetWhitelist->isChecked()); + // Security + m_ui->checkClickjacking->setChecked(pref->isWebUiClickjackingProtectionEnabled()); + m_ui->checkDynDNS->setChecked(pref->isDynDNSEnabled()); m_ui->comboDNSService->setCurrentIndex(static_cast(pref->getDynDNSService())); m_ui->domainNameTxt->setText(pref->getDynDomainName()); diff --git a/src/gui/optionsdlg.ui b/src/gui/optionsdlg.ui index 0a9afb932..f298fbb62 100644 --- a/src/gui/optionsdlg.ui +++ b/src/gui/optionsdlg.ui @@ -3168,6 +3168,13 @@ Use ';' to split multiple entries. Can use wildcard '*'. + + + + Enable clickjacking protection + + + diff --git a/src/webui/api/appcontroller.cpp b/src/webui/api/appcontroller.cpp index 26775b203..a970cd06d 100644 --- a/src/webui/api/appcontroller.cpp +++ b/src/webui/api/appcontroller.cpp @@ -205,6 +205,8 @@ void AppController::preferencesAction() for (const Utils::Net::Subnet &subnet : copyAsConst(pref->getWebUiAuthSubnetWhitelist())) authSubnetWhitelistStringList << Utils::Net::subnetToString(subnet); data["bypass_auth_subnet_whitelist"] = authSubnetWhitelistStringList.join("\n"); + // Security + data["web_ui_clickjacking_protection_enabled"] = pref->isWebUiClickjackingProtectionEnabled(); // Update my dynamic domain name data["dyndns_enabled"] = pref->isDynDNSEnabled(); data["dyndns_service"] = pref->getDynDNSService(); @@ -479,6 +481,9 @@ void AppController::setPreferencesAction() // recognize new lines and commas as delimiters pref->setWebUiAuthSubnetWhitelist(m["bypass_auth_subnet_whitelist"].toString().split(QRegularExpression("\n|,"), QString::SkipEmptyParts)); } + // Security + if (m.contains("web_ui_clickjacking_protection_enabled")) + pref->setWebUiClickjackingProtectionEnabled(m["web_ui_clickjacking_protection_enabled"].toBool()); // Update my dynamic domain name if (m.contains("dyndns_enabled")) pref->setDynDNSEnabled(m["dyndns_enabled"].toBool()); diff --git a/src/webui/webapplication.cpp b/src/webui/webapplication.cpp index 35e6d2fa9..3ce44da84 100644 --- a/src/webui/webapplication.cpp +++ b/src/webui/webapplication.cpp @@ -428,6 +428,8 @@ void WebApplication::configure() m_currentLocale = newLocale; m_translatedFiles.clear(); } + + m_isClickjackingProtectionEnabled = pref->isWebUiClickjackingProtectionEnabled(); } void WebApplication::registerAPIController(const QString &scope, APIController *controller) @@ -525,11 +527,13 @@ Http::Response WebApplication::processRequest(const Http::Request &request, cons print(error.message(), Http::CONTENT_TYPE_TXT); } - // avoid clickjacking attacks - header(Http::HEADER_X_FRAME_OPTIONS, "SAMEORIGIN"); header(Http::HEADER_X_XSS_PROTECTION, "1; mode=block"); header(Http::HEADER_X_CONTENT_TYPE_OPTIONS, "nosniff"); - header(Http::HEADER_CONTENT_SECURITY_POLICY, "default-src 'self'; style-src 'self' 'unsafe-inline'; img-src 'self' data:; script-src 'self' 'unsafe-inline'; object-src 'none';"); + + if (m_isClickjackingProtectionEnabled) { + header(Http::HEADER_X_FRAME_OPTIONS, "SAMEORIGIN"); + header(Http::HEADER_CONTENT_SECURITY_POLICY, "default-src 'self'; style-src 'self' 'unsafe-inline'; img-src 'self' data:; script-src 'self' 'unsafe-inline'; object-src 'none';"); + } return response(); } diff --git a/src/webui/webapplication.h b/src/webui/webapplication.h index 509927993..e96b25300 100644 --- a/src/webui/webapplication.h +++ b/src/webui/webapplication.h @@ -142,4 +142,7 @@ private: }; QMap m_translatedFiles; QString m_currentLocale; + + // security related + bool m_isClickjackingProtectionEnabled; }; diff --git a/src/webui/www/private/preferences_content.html b/src/webui/www/private/preferences_content.html index 5020fd938..d828275cb 100644 --- a/src/webui/www/private/preferences_content.html +++ b/src/webui/www/private/preferences_content.html @@ -458,6 +458,11 @@
+ +
+ + +
@@ -1022,6 +1027,9 @@ $('bypass_auth_subnet_whitelist_textarea').setProperty('value', pref.bypass_auth_subnet_whitelist); updateBypasssAuthSettings(); + // Security + $('clickjacking_protection_checkbox').setProperty('checked', pref.web_ui_clickjacking_protection_enabled); + // Update my dynamic domain name $('use_dyndns_checkbox').setProperty('checked', pref.dyndns_enabled); $('dyndns_select').setProperty('value', pref.dyndns_service); @@ -1313,6 +1321,8 @@ settings.set('bypass_auth_subnet_whitelist_enabled', $('bypass_auth_subnet_whitelist_checkbox').getProperty('checked')); settings.set('bypass_auth_subnet_whitelist', $('bypass_auth_subnet_whitelist_textarea').getProperty('value')); + settings.set('web_ui_clickjacking_protection_enabled', $('clickjacking_protection_checkbox').getProperty('checked')); + // Update my dynamic domain name settings.set('dyndns_enabled', $('use_dyndns_checkbox').getProperty('checked')); settings.set('dyndns_service', $('dyndns_select').getProperty('value')); From 9eeef0be97424ecf83d1f7df6ca792cfa8e286c2 Mon Sep 17 00:00:00 2001 From: Chocobo1 Date: Tue, 22 May 2018 00:43:33 +0800 Subject: [PATCH 3/3] Add option to control CSRF protection Some users are using WebUI with simple port-forwarding from their router, providing an option to control the protection will save them from setting up an non-trival web proxy. Closes #7274. --- src/base/preferences.cpp | 10 ++++++++++ src/base/preferences.h | 2 ++ src/gui/optionsdlg.cpp | 3 +++ src/gui/optionsdlg.ui | 7 +++++++ src/webui/api/appcontroller.cpp | 3 +++ src/webui/webapplication.cpp | 7 +++++-- src/webui/webapplication.h | 1 + src/webui/www/private/preferences_content.html | 6 ++++++ 8 files changed, 37 insertions(+), 2 deletions(-) diff --git a/src/base/preferences.cpp b/src/base/preferences.cpp index 0f6a458de..ae044fdf2 100644 --- a/src/base/preferences.cpp +++ b/src/base/preferences.cpp @@ -586,6 +586,16 @@ void Preferences::setWebUiClickjackingProtectionEnabled(bool enabled) setValue("Preferences/WebUI/ClickjackingProtection", enabled); } +bool Preferences::isWebUiCSRFProtectionEnabled() const +{ + return value("Preferences/WebUI/CSRFProtection", true).toBool(); +} + +void Preferences::setWebUiCSRFProtectionEnabled(bool enabled) +{ + setValue("Preferences/WebUI/CSRFProtection", enabled); +} + bool Preferences::isWebUiHttpsEnabled() const { return value("Preferences/WebUI/HTTPS/Enabled", false).toBool(); diff --git a/src/base/preferences.h b/src/base/preferences.h index 46616591d..a3b5a2ae7 100644 --- a/src/base/preferences.h +++ b/src/base/preferences.h @@ -197,6 +197,8 @@ public: // WebUI security bool isWebUiClickjackingProtectionEnabled() const; void setWebUiClickjackingProtectionEnabled(bool enabled); + bool isWebUiCSRFProtectionEnabled() const; + void setWebUiCSRFProtectionEnabled(bool enabled); // HTTPS bool isWebUiHttpsEnabled() const; diff --git a/src/gui/optionsdlg.cpp b/src/gui/optionsdlg.cpp index 23af0a055..d0ac39d87 100644 --- a/src/gui/optionsdlg.cpp +++ b/src/gui/optionsdlg.cpp @@ -379,6 +379,7 @@ OptionsDialog::OptionsDialog(QWidget *parent) connect(m_ui->checkBypassAuthSubnetWhitelist, &QAbstractButton::toggled, this, &ThisType::enableApplyButton); connect(m_ui->checkBypassAuthSubnetWhitelist, &QAbstractButton::toggled, m_ui->IPSubnetWhitelistButton, &QPushButton::setEnabled); connect(m_ui->checkClickjacking, &QCheckBox::toggled, this, &ThisType::enableApplyButton); + connect(m_ui->checkCSRFProtection, &QCheckBox::toggled, this, &ThisType::enableApplyButton); connect(m_ui->checkDynDNS, &QGroupBox::toggled, this, &ThisType::enableApplyButton); connect(m_ui->comboDNSService, qComboBoxCurrentIndexChanged, this, &ThisType::enableApplyButton); connect(m_ui->domainNameTxt, &QLineEdit::textChanged, this, &ThisType::enableApplyButton); @@ -697,6 +698,7 @@ void OptionsDialog::saveOptions() pref->setWebUiAuthSubnetWhitelistEnabled(m_ui->checkBypassAuthSubnetWhitelist->isChecked()); // Security pref->setWebUiClickjackingProtectionEnabled(m_ui->checkClickjacking->isChecked()); + pref->setWebUiCSRFProtectionEnabled(m_ui->checkCSRFProtection->isChecked()); // DynDNS pref->setDynDNSEnabled(m_ui->checkDynDNS->isChecked()); pref->setDynDNSService(m_ui->comboDNSService->currentIndex()); @@ -1101,6 +1103,7 @@ void OptionsDialog::loadOptions() // Security m_ui->checkClickjacking->setChecked(pref->isWebUiClickjackingProtectionEnabled()); + m_ui->checkCSRFProtection->setChecked(pref->isWebUiCSRFProtectionEnabled()); m_ui->checkDynDNS->setChecked(pref->isDynDNSEnabled()); m_ui->comboDNSService->setCurrentIndex(static_cast(pref->getDynDNSService())); diff --git a/src/gui/optionsdlg.ui b/src/gui/optionsdlg.ui index f298fbb62..0393990f8 100644 --- a/src/gui/optionsdlg.ui +++ b/src/gui/optionsdlg.ui @@ -3175,6 +3175,13 @@ Use ';' to split multiple entries. Can use wildcard '*'. + + + + Enable Cross-Site Request Forgery (CSRF) protection + + + diff --git a/src/webui/api/appcontroller.cpp b/src/webui/api/appcontroller.cpp index a970cd06d..fad7bc23e 100644 --- a/src/webui/api/appcontroller.cpp +++ b/src/webui/api/appcontroller.cpp @@ -207,6 +207,7 @@ void AppController::preferencesAction() data["bypass_auth_subnet_whitelist"] = authSubnetWhitelistStringList.join("\n"); // Security data["web_ui_clickjacking_protection_enabled"] = pref->isWebUiClickjackingProtectionEnabled(); + data["web_ui_csrf_protection_enabled"] = pref->isWebUiCSRFProtectionEnabled(); // Update my dynamic domain name data["dyndns_enabled"] = pref->isDynDNSEnabled(); data["dyndns_service"] = pref->getDynDNSService(); @@ -484,6 +485,8 @@ void AppController::setPreferencesAction() // Security if (m.contains("web_ui_clickjacking_protection_enabled")) pref->setWebUiClickjackingProtectionEnabled(m["web_ui_clickjacking_protection_enabled"].toBool()); + if (m.contains("web_ui_csrf_protection_enabled")) + pref->setWebUiCSRFProtectionEnabled(m["web_ui_csrf_protection_enabled"].toBool()); // Update my dynamic domain name if (m.contains("dyndns_enabled")) pref->setDynDNSEnabled(m["dyndns_enabled"].toBool()); diff --git a/src/webui/webapplication.cpp b/src/webui/webapplication.cpp index 3ce44da84..0b98986ba 100644 --- a/src/webui/webapplication.cpp +++ b/src/webui/webapplication.cpp @@ -430,6 +430,7 @@ void WebApplication::configure() } m_isClickjackingProtectionEnabled = pref->isWebUiClickjackingProtectionEnabled(); + m_isCSRFProtectionEnabled = pref->isWebUiCSRFProtectionEnabled(); } void WebApplication::registerAPIController(const QString &scope, APIController *controller) @@ -514,9 +515,11 @@ Http::Response WebApplication::processRequest(const Http::Request &request, cons clear(); try { - // block cross-site requests - if (isCrossSiteRequest(m_request) || !validateHostHeader(m_domainList)) + // block suspicious requests + if ((m_isCSRFProtectionEnabled && isCrossSiteRequest(m_request)) + || !validateHostHeader(m_domainList)) { throw UnauthorizedHTTPError(); + } sessionInitialize(); doProcessRequest(); diff --git a/src/webui/webapplication.h b/src/webui/webapplication.h index e96b25300..f1a38bd3f 100644 --- a/src/webui/webapplication.h +++ b/src/webui/webapplication.h @@ -145,4 +145,5 @@ private: // security related bool m_isClickjackingProtectionEnabled; + bool m_isCSRFProtectionEnabled; }; diff --git a/src/webui/www/private/preferences_content.html b/src/webui/www/private/preferences_content.html index d828275cb..7cf03a480 100644 --- a/src/webui/www/private/preferences_content.html +++ b/src/webui/www/private/preferences_content.html @@ -463,6 +463,10 @@ +
+ + +
@@ -1029,6 +1033,7 @@ // Security $('clickjacking_protection_checkbox').setProperty('checked', pref.web_ui_clickjacking_protection_enabled); + $('csrf_protection_checkbox').setProperty('checked', pref.web_ui_csrf_protection_enabled); // Update my dynamic domain name $('use_dyndns_checkbox').setProperty('checked', pref.dyndns_enabled); @@ -1322,6 +1327,7 @@ settings.set('bypass_auth_subnet_whitelist', $('bypass_auth_subnet_whitelist_textarea').getProperty('value')); settings.set('web_ui_clickjacking_protection_enabled', $('clickjacking_protection_checkbox').getProperty('checked')); + settings.set('web_ui_csrf_protection_enabled', $('csrf_protection_checkbox').getProperty('checked')); // Update my dynamic domain name settings.set('dyndns_enabled', $('use_dyndns_checkbox').getProperty('checked'));