Merge pull request #8967 from Chocobo1/protect

Add options to control WebUI security measures
2025-01-10 14:57:52 +00:00 · 2018-05-28 14:21:53 +08:00 · 2018-05-28 14:21:53 +08:00 · 4a51f14328
commit 4a51f14328
parent 2ef7adec89 9eeef0be97
8 changed files with 105 additions and 21 deletions
--- a/src/base/preferences.cpp
+++ b/src/base/preferences.cpp
@ -576,6 +576,26 @@ 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::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();
--- a/src/base/preferences.h
+++ b/src/base/preferences.h
@ -194,6 +194,12 @@ public:
    QString getWebUiPassword() const;
    void setWebUiPassword(const QString &new_password);

+    // WebUI security
+    bool isWebUiClickjackingProtectionEnabled() const;
+    void setWebUiClickjackingProtectionEnabled(bool enabled);
+    bool isWebUiCSRFProtectionEnabled() const;
+    void setWebUiCSRFProtectionEnabled(bool enabled);
+
    // HTTPS
    bool isWebUiHttpsEnabled() const;
    void setWebUiHttpsEnabled(bool enabled);
--- a/src/gui/optionsdlg.cpp
+++ b/src/gui/optionsdlg.cpp
@ -378,6 +378,8 @@ 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->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);
@ -694,6 +696,9 @@ 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());
+        pref->setWebUiCSRFProtectionEnabled(m_ui->checkCSRFProtection->isChecked());
        // DynDNS
        pref->setDynDNSEnabled(m_ui->checkDynDNS->isChecked());
        pref->setDynDNSService(m_ui->comboDNSService->currentIndex());
@ -1096,6 +1101,10 @@ 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->checkCSRFProtection->setChecked(pref->isWebUiCSRFProtectionEnabled());
+
    m_ui->checkDynDNS->setChecked(pref->isDynDNSEnabled());
    m_ui->comboDNSService->setCurrentIndex(static_cast<int>(pref->getDynDNSService()));
    m_ui->domainNameTxt->setText(pref->getDynDomainName());
--- a/src/gui/optionsdlg.ui
+++ b/src/gui/optionsdlg.ui
@ -3168,6 +3168,20 @@ Use ';' to split multiple entries. Can use wildcard '*'.</string>
                 </layout>
                </widget>
               </item>
+               <item>
+                <widget class="QCheckBox" name="checkClickjacking">
+                 <property name="text">
+                  <string>Enable clickjacking protection</string>
+                 </property>
+                </widget>
+               </item>
+               <item>
+                <widget class="QCheckBox" name="checkCSRFProtection">
+                 <property name="text">
+                  <string>Enable Cross-Site Request Forgery (CSRF) protection</string>
+                 </property>
+                </widget>
+               </item>
               <item>
                <widget class="QGroupBox" name="checkDynDNS">
                 <property name="title">
--- a/src/webui/api/appcontroller.cpp
+++ b/src/webui/api/appcontroller.cpp
@ -205,6 +205,9 @@ 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();
+    data["web_ui_csrf_protection_enabled"] = pref->isWebUiCSRFProtectionEnabled();
    // Update my dynamic domain name
    data["dyndns_enabled"] = pref->isDynDNSEnabled();
    data["dyndns_service"] = pref->getDynDNSService();
@ -479,6 +482,11 @@ 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());
+    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());
--- a/src/webui/webapplication.cpp
+++ b/src/webui/webapplication.cpp
@ -428,6 +428,9 @@ void WebApplication::configure()
        m_currentLocale = newLocale;
        m_translatedFiles.clear();
    }
+
+    m_isClickjackingProtectionEnabled = pref->isWebUiClickjackingProtectionEnabled();
+    m_isCSRFProtectionEnabled = pref->isWebUiCSRFProtectionEnabled();
 }

 void WebApplication::registerAPIController(const QString &scope, APIController *controller)
@ -512,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();
@ -525,11 +530,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();
 }
--- a/src/webui/webapplication.h
+++ b/src/webui/webapplication.h
@ -142,4 +142,8 @@ private:
    };
    QMap<QString, TranslatedFile> m_translatedFiles;
    QString m_currentLocale;
+
+    // security related
+    bool m_isClickjackingProtectionEnabled;
+    bool m_isCSRFProtectionEnabled;
 };
--- a/src/webui/www/private/preferences_content.html
+++ b/src/webui/www/private/preferences_content.html
@ -437,26 +437,35 @@
            </div>
            <div style="padding-left: 10px;"><a target="_blank" href="https://httpd.apache.org/docs/current/ssl/ssl_faq.html#aboutcerts">QBT_TR(Information about certificates)QBT_TR[CONTEXT=HttpServer]</a></div>
        </fieldset>
-    </fieldset>

-    <fieldset class="settings">
-        <legend>QBT_TR(Authentication)QBT_TR[CONTEXT=OptionsDialog]</legend>
+        <fieldset class="settings">
+            <legend>QBT_TR(Authentication)QBT_TR[CONTEXT=OptionsDialog]</legend>
+            <div class="formRow">
+                <label for="webui_username_text" class="leftLabelSmall">QBT_TR(Username:)QBT_TR[CONTEXT=OptionsDialog]</label><input type="text" id="webui_username_text" />
+            </div>
+            <div class="formRow">
+                <label for="webui_password_text" class="leftLabelSmall">QBT_TR(Password:)QBT_TR[CONTEXT=OptionsDialog]</label><input type="password" id="webui_password_text" />
+            </div>
+            <div class="formRow">
+                <input type="checkbox" id="bypass_local_auth_checkbox" />
+                <label for="bypass_local_auth_checkbox">QBT_TR(Bypass authentication for clients on localhost)QBT_TR[CONTEXT=OptionsDialog]</label>
+            </div>
+            <div class="formRow">
+                <input type="checkbox" id="bypass_auth_subnet_whitelist_checkbox" onclick="updateBypasssAuthSettings();" />
+                <label for="bypass_auth_subnet_whitelist_checkbox">QBT_TR(Bypass authentication for clients in whitelisted IP subnets)QBT_TR[CONTEXT=OptionsDialog]</label>
+            </div>
+            <div class="formRow" style="padding-left: 30px; padding-top: 5px;">
+                <textarea id="bypass_auth_subnet_whitelist_textarea" rows="5" cols="48" placeholder="Example: 172.17.32.0/24, fdff:ffff:c8::/40"></textarea>
+            </div>
+        </fieldset>
+
        <div class="formRow">
-            <label for="webui_username_text" class="leftLabelSmall">QBT_TR(Username:)QBT_TR[CONTEXT=OptionsDialog]</label><input type="text" id="webui_username_text" />
+            <input type="checkbox" id="clickjacking_protection_checkbox" />
+            <label for="clickjacking_protection_checkbox">QBT_TR(Enable clickjacking protection)QBT_TR[CONTEXT=OptionsDialog]</label>
        </div>
        <div class="formRow">
-            <label for="webui_password_text" class="leftLabelSmall">QBT_TR(Password:)QBT_TR[CONTEXT=OptionsDialog]</label><input type="password" id="webui_password_text" />
-        </div>
-        <div class="formRow">
-            <input type="checkbox" id="bypass_local_auth_checkbox" />
-            <label for="bypass_local_auth_checkbox">QBT_TR(Bypass authentication for clients on localhost)QBT_TR[CONTEXT=OptionsDialog]</label>
-        </div>
-        <div class="formRow">
-            <input type="checkbox" id="bypass_auth_subnet_whitelist_checkbox" onclick="updateBypasssAuthSettings();" />
-            <label for="bypass_auth_subnet_whitelist_checkbox">QBT_TR(Bypass authentication for clients in whitelisted IP subnets)QBT_TR[CONTEXT=OptionsDialog]</label>
-        </div>
-        <div class="formRow" style="padding-left: 30px; padding-top: 5px;">
-            <textarea id="bypass_auth_subnet_whitelist_textarea" rows="5" cols="48" placeholder="Example: 172.17.32.0/24, fdff:ffff:c8::/40"></textarea>
+            <input type="checkbox" id="csrf_protection_checkbox" />
+            <label for="csrf_protection_checkbox">QBT_TR(Enable Cross-Site Request Forgery (CSRF) protection)QBT_TR[CONTEXT=OptionsDialog]</label>
        </div>
    </fieldset>

@ -1022,6 +1031,10 @@
                    $('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);
+                    $('csrf_protection_checkbox').setProperty('checked', pref.web_ui_csrf_protection_enabled);
+
                    // Update my dynamic domain name
                    $('use_dyndns_checkbox').setProperty('checked', pref.dyndns_enabled);
                    $('dyndns_select').setProperty('value', pref.dyndns_service);
@ -1313,6 +1326,9 @@
        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'));
+        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'));
        settings.set('dyndns_service', $('dyndns_select').getProperty('value'));