From f88d6b2e55c8e94623ac96f977fe7887268ea60a Mon Sep 17 00:00:00 2001 From: Chocobo1 Date: Wed, 12 Feb 2020 18:51:38 +0800 Subject: [PATCH 1/4] Expose WebUI ban counter to users --- src/base/preferences.cpp | 10 ++++++ src/base/preferences.h | 2 ++ src/gui/optionsdialog.cpp | 3 ++ src/gui/optionsdialog.ui | 34 ++++++++++++++++++++ src/webui/api/appcontroller.cpp | 3 ++ src/webui/api/authcontroller.cpp | 10 +++--- src/webui/api/authcontroller.h | 2 +- src/webui/www/private/views/preferences.html | 8 +++++ 8 files changed, 67 insertions(+), 5 deletions(-) diff --git a/src/base/preferences.cpp b/src/base/preferences.cpp index 1ab55e24d..3e8424353 100644 --- a/src/base/preferences.cpp +++ b/src/base/preferences.cpp @@ -621,6 +621,16 @@ void Preferences::setWebUIPassword(const QByteArray &password) setValue("Preferences/WebUI/Password_PBKDF2", password); } +int Preferences::getWebUIMaxAuthFailCount() const +{ + return value("Preferences/WebUI/MaxAuthenticationFailCount", 5).toInt(); +} + +void Preferences::setWebUIMaxAuthFailCount(const int count) +{ + setValue("Preferences/WebUI/MaxAuthenticationFailCount", count); +} + int Preferences::getWebUISessionTimeout() const { return value("Preferences/WebUI/SessionTimeout", 3600).toInt(); diff --git a/src/base/preferences.h b/src/base/preferences.h index 596cf49b3..b4d25590a 100644 --- a/src/base/preferences.h +++ b/src/base/preferences.h @@ -194,6 +194,8 @@ public: void setWebUiUsername(const QString &username); QByteArray getWebUIPassword() const; void setWebUIPassword(const QByteArray &password); + int getWebUIMaxAuthFailCount() const; + void setWebUIMaxAuthFailCount(int count); int getWebUISessionTimeout() const; void setWebUISessionTimeout(int timeout); diff --git a/src/gui/optionsdialog.cpp b/src/gui/optionsdialog.cpp index bb307c1ad..8df4dea70 100644 --- a/src/gui/optionsdialog.cpp +++ b/src/gui/optionsdialog.cpp @@ -421,6 +421,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->spinBanCounter, qSpinBoxValueChanged, this, &ThisType::enableApplyButton); connect(m_ui->spinSessionTimeout, qSpinBoxValueChanged, this, &ThisType::enableApplyButton); connect(m_ui->checkClickjacking, &QCheckBox::toggled, this, &ThisType::enableApplyButton); connect(m_ui->checkCSRFProtection, &QCheckBox::toggled, this, &ThisType::enableApplyButton); @@ -770,6 +771,7 @@ void OptionsDialog::saveOptions() pref->setWebUiHttpsEnabled(m_ui->checkWebUiHttps->isChecked()); pref->setWebUIHttpsCertificatePath(m_ui->textWebUIHttpsCert->selectedPath()); pref->setWebUIHttpsKeyPath(m_ui->textWebUIHttpsKey->selectedPath()); + pref->setWebUIMaxAuthFailCount(m_ui->spinBanCounter->value()); pref->setWebUISessionTimeout(m_ui->spinSessionTimeout->value()); // Authentication pref->setWebUiUsername(webUiUsername()); @@ -1153,6 +1155,7 @@ void OptionsDialog::loadOptions() m_ui->checkBypassLocalAuth->setChecked(!pref->isWebUiLocalAuthEnabled()); m_ui->checkBypassAuthSubnetWhitelist->setChecked(pref->isWebUiAuthSubnetWhitelistEnabled()); m_ui->IPSubnetWhitelistButton->setEnabled(m_ui->checkBypassAuthSubnetWhitelist->isChecked()); + m_ui->spinBanCounter->setValue(pref->getWebUIMaxAuthFailCount()); m_ui->spinSessionTimeout->setValue(pref->getWebUISessionTimeout()); // Security diff --git a/src/gui/optionsdialog.ui b/src/gui/optionsdialog.ui index 5b341cb21..f46bded2d 100644 --- a/src/gui/optionsdialog.ui +++ b/src/gui/optionsdialog.ui @@ -2986,6 +2986,40 @@ Specify an IPv4 or IPv6 address. You can specify "0.0.0.0" for any IPv + + + + + + Ban client after consecutive failures: + + + + + + + Never + + + 2147483647 + + + + + + + Qt::Horizontal + + + + 40 + 20 + + + + + + diff --git a/src/webui/api/appcontroller.cpp b/src/webui/api/appcontroller.cpp index 7f367210a..c28800445 100644 --- a/src/webui/api/appcontroller.cpp +++ b/src/webui/api/appcontroller.cpp @@ -232,6 +232,7 @@ void AppController::preferencesAction() for (const Utils::Net::Subnet &subnet : asConst(pref->getWebUiAuthSubnetWhitelist())) authSubnetWhitelistStringList << Utils::Net::subnetToString(subnet); data["bypass_auth_subnet_whitelist"] = authSubnetWhitelistStringList.join("\n"); + data["web_ui_max_auth_fail_count"] = pref->getWebUIMaxAuthFailCount(); data["web_ui_session_timeout"] = pref->getWebUISessionTimeout(); // Use alternative Web UI data["alternative_webui_enabled"] = pref->isAltWebUiEnabled(); @@ -601,6 +602,8 @@ void AppController::setPreferencesAction() // recognize new lines and commas as delimiters pref->setWebUiAuthSubnetWhitelist(it.value().toString().split(QRegularExpression("\n|,"), QString::SkipEmptyParts)); } + if (hasKey("web_ui_max_auth_fail_count")) + pref->setWebUIMaxAuthFailCount(it.value().toInt()); if (hasKey("web_ui_session_timeout")) pref->setWebUISessionTimeout(it.value().toInt()); // Use alternative Web UI diff --git a/src/webui/api/authcontroller.cpp b/src/webui/api/authcontroller.cpp index 1a34f163c..6b759ba47 100644 --- a/src/webui/api/authcontroller.cpp +++ b/src/webui/api/authcontroller.cpp @@ -38,7 +38,6 @@ #include "isessionmanager.h" constexpr int BAN_TIME = 3600000; // 1 hour -constexpr int MAX_AUTH_FAILED_ATTEMPTS = 5; void AuthController::loginAction() { @@ -74,7 +73,8 @@ void AuthController::loginAction() LogMsg(tr("WebAPI login success. IP: %1").arg(clientAddr)); } else { - increaseFailedAttempts(); + if (Preferences::instance()->getWebUIMaxAuthFailCount() > 0) + increaseFailedAttempts(); setResult(QLatin1String("Fails.")); LogMsg(tr("WebAPI login failure. Reason: invalid credentials, attempt count: %1, IP: %2, username: %3") .arg(QString::number(failedAttemptsCount()), clientAddr, usernameFromWeb) @@ -82,7 +82,7 @@ void AuthController::loginAction() } } -void AuthController::logoutAction() +void AuthController::logoutAction() const { sessionManager()->sessionEnd(); } @@ -108,10 +108,12 @@ int AuthController::failedAttemptsCount() const void AuthController::increaseFailedAttempts() { + Q_ASSERT(Preferences::instance()->getWebUIMaxAuthFailCount() > 0); + FailedLogin &failedLogin = m_clientFailedLogins[sessionManager()->clientId()]; ++failedLogin.failedAttemptsCount; - if (failedLogin.failedAttemptsCount == MAX_AUTH_FAILED_ATTEMPTS) { + if (failedLogin.failedAttemptsCount >= Preferences::instance()->getWebUIMaxAuthFailCount()) { // Max number of failed attempts reached // Start ban period failedLogin.bannedAt = QDateTime::currentMSecsSinceEpoch() / 1000; diff --git a/src/webui/api/authcontroller.h b/src/webui/api/authcontroller.h index 0dd896473..5ad1ddc12 100644 --- a/src/webui/api/authcontroller.h +++ b/src/webui/api/authcontroller.h @@ -44,7 +44,7 @@ public: private slots: void loginAction(); - void logoutAction(); + void logoutAction() const; private: bool isBanned() const; diff --git a/src/webui/www/private/views/preferences.html b/src/webui/www/private/views/preferences.html index 88dbb5fcd..32f2b2fe6 100644 --- a/src/webui/www/private/views/preferences.html +++ b/src/webui/www/private/views/preferences.html @@ -729,6 +729,12 @@
+ + + + + +
@@ -1719,6 +1725,7 @@ $('bypass_auth_subnet_whitelist_checkbox').setProperty('checked', pref.bypass_auth_subnet_whitelist_enabled); $('bypass_auth_subnet_whitelist_textarea').setProperty('value', pref.bypass_auth_subnet_whitelist); updateBypasssAuthSettings(); + $('webUIMaxAuthFailCountInput').setProperty('value', pref.web_ui_max_auth_fail_count.toInt()); $('webUISessionTimeoutInput').setProperty('value', pref.web_ui_session_timeout.toInt()); // Use alternative Web UI @@ -2082,6 +2089,7 @@ settings.set('bypass_local_auth', $('bypass_local_auth_checkbox').getProperty('checked')); 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_max_auth_fail_count', $('webUIMaxAuthFailCountInput').getProperty('value')); settings.set('web_ui_session_timeout', $('webUISessionTimeoutInput').getProperty('value')); // Use alternative Web UI From 4f7b799732d1ed2f73f31243a3af33acb34113ab Mon Sep 17 00:00:00 2001 From: Chocobo1 Date: Wed, 12 Feb 2020 21:37:25 +0800 Subject: [PATCH 2/4] Use QDeadlineTimer for tracking WebUI banned duration It simplifies our code and the new timer is monotonic. --- src/webui/api/authcontroller.cpp | 14 +++++++------- src/webui/api/authcontroller.h | 3 ++- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/src/webui/api/authcontroller.cpp b/src/webui/api/authcontroller.cpp index 6b759ba47..132bb0abe 100644 --- a/src/webui/api/authcontroller.cpp +++ b/src/webui/api/authcontroller.cpp @@ -28,7 +28,6 @@ #include "authcontroller.h" -#include #include #include "base/logger.h" @@ -89,12 +88,13 @@ void AuthController::logoutAction() const bool AuthController::isBanned() const { - const qint64 now = QDateTime::currentMSecsSinceEpoch() / 1000; - const FailedLogin failedLogin = m_clientFailedLogins.value(sessionManager()->clientId()); + const auto failedLoginIter = m_clientFailedLogins.find(sessionManager()->clientId()); + if (failedLoginIter == m_clientFailedLogins.end()) + return false; - bool isBanned = (failedLogin.bannedAt > 0); - if (isBanned && ((now - failedLogin.bannedAt) > BAN_TIME)) { - m_clientFailedLogins.remove(sessionManager()->clientId()); + bool isBanned = (failedLoginIter->banTimer.remainingTime() >= 0); + if (isBanned && failedLoginIter->banTimer.hasExpired()) { + m_clientFailedLogins.erase(failedLoginIter); isBanned = false; } @@ -116,6 +116,6 @@ void AuthController::increaseFailedAttempts() if (failedLogin.failedAttemptsCount >= Preferences::instance()->getWebUIMaxAuthFailCount()) { // Max number of failed attempts reached // Start ban period - failedLogin.bannedAt = QDateTime::currentMSecsSinceEpoch() / 1000; + failedLogin.banTimer.setRemainingTime(BAN_TIME); } } diff --git a/src/webui/api/authcontroller.h b/src/webui/api/authcontroller.h index 5ad1ddc12..ae4c84fda 100644 --- a/src/webui/api/authcontroller.h +++ b/src/webui/api/authcontroller.h @@ -28,6 +28,7 @@ #pragma once +#include #include #include "apicontroller.h" @@ -54,7 +55,7 @@ private: struct FailedLogin { int failedAttemptsCount = 0; - qint64 bannedAt = 0; + QDeadlineTimer banTimer {-1}; }; mutable QHash m_clientFailedLogins; }; From ec61ef5145cd56a64d8d1452a0df4908beed11fd Mon Sep 17 00:00:00 2001 From: Chocobo1 Date: Wed, 12 Feb 2020 23:15:25 +0800 Subject: [PATCH 3/4] Fix coding inconsistencies in preferences.html 1. Format table tags properly 2. Add a proper label tag 3. Don't use abbreviation for seconds, we use full name everywhere else --- src/webui/www/private/views/preferences.html | 45 ++++++++++---------- 1 file changed, 22 insertions(+), 23 deletions(-) diff --git a/src/webui/www/private/views/preferences.html b/src/webui/www/private/views/preferences.html index 32f2b2fe6..55c9e411a 100644 --- a/src/webui/www/private/views/preferences.html +++ b/src/webui/www/private/views/preferences.html @@ -562,28 +562,27 @@ - - - - - - - - + + + + + + + + +
- - - - QBT_TR(minutes)QBT_TR[CONTEXT=OptionsDialog] -
- QBT_TR(then)QBT_TR[CONTEXT=OptionsDialog] - - -
+ + + + QBT_TR(minutes)QBT_TR[CONTEXT=OptionsDialog] +
+ +
@@ -738,7 +737,7 @@ - +
  QBT_TR(sec)QBT_TR[CONTEXT=OptionsDialog]  QBT_TR(seconds)QBT_TR[CONTEXT=OptionsDialog]
From 6eb190c3733a493e01221770104b66af973aed8f Mon Sep 17 00:00:00 2001 From: Chocobo1 Date: Wed, 12 Feb 2020 22:04:54 +0800 Subject: [PATCH 4/4] Expose WebUI ban duration to users --- src/base/preferences.cpp | 12 +++++ src/base/preferences.h | 2 + src/gui/optionsdialog.cpp | 3 ++ src/gui/optionsdialog.ui | 49 ++++++++++++++------ src/webui/api/appcontroller.cpp | 3 ++ src/webui/api/authcontroller.cpp | 4 +- src/webui/www/private/views/preferences.html | 6 +++ 7 files changed, 63 insertions(+), 16 deletions(-) diff --git a/src/base/preferences.cpp b/src/base/preferences.cpp index 3e8424353..80660f17d 100644 --- a/src/base/preferences.cpp +++ b/src/base/preferences.cpp @@ -29,6 +29,8 @@ #include "preferences.h" +#include + #ifdef Q_OS_MACOS #include #endif @@ -631,6 +633,16 @@ void Preferences::setWebUIMaxAuthFailCount(const int count) setValue("Preferences/WebUI/MaxAuthenticationFailCount", count); } +std::chrono::seconds Preferences::getWebUIBanDuration() const +{ + return std::chrono::seconds {value("Preferences/WebUI/BanDuration", 3600).toInt()}; +} + +void Preferences::setWebUIBanDuration(const std::chrono::seconds duration) +{ + setValue("Preferences/WebUI/BanDuration", static_cast(duration.count())); +} + int Preferences::getWebUISessionTimeout() const { return value("Preferences/WebUI/SessionTimeout", 3600).toInt(); diff --git a/src/base/preferences.h b/src/base/preferences.h index b4d25590a..aba678a3d 100644 --- a/src/base/preferences.h +++ b/src/base/preferences.h @@ -196,6 +196,8 @@ public: void setWebUIPassword(const QByteArray &password); int getWebUIMaxAuthFailCount() const; void setWebUIMaxAuthFailCount(int count); + std::chrono::seconds getWebUIBanDuration() const; + void setWebUIBanDuration(std::chrono::seconds duration); int getWebUISessionTimeout() const; void setWebUISessionTimeout(int timeout); diff --git a/src/gui/optionsdialog.cpp b/src/gui/optionsdialog.cpp index 8df4dea70..db680ae81 100644 --- a/src/gui/optionsdialog.cpp +++ b/src/gui/optionsdialog.cpp @@ -422,6 +422,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->spinBanCounter, qSpinBoxValueChanged, this, &ThisType::enableApplyButton); + connect(m_ui->spinBanDuration, qSpinBoxValueChanged, this, &ThisType::enableApplyButton); connect(m_ui->spinSessionTimeout, qSpinBoxValueChanged, this, &ThisType::enableApplyButton); connect(m_ui->checkClickjacking, &QCheckBox::toggled, this, &ThisType::enableApplyButton); connect(m_ui->checkCSRFProtection, &QCheckBox::toggled, this, &ThisType::enableApplyButton); @@ -772,6 +773,7 @@ void OptionsDialog::saveOptions() pref->setWebUIHttpsCertificatePath(m_ui->textWebUIHttpsCert->selectedPath()); pref->setWebUIHttpsKeyPath(m_ui->textWebUIHttpsKey->selectedPath()); pref->setWebUIMaxAuthFailCount(m_ui->spinBanCounter->value()); + pref->setWebUIBanDuration(std::chrono::seconds {m_ui->spinBanDuration->value()}); pref->setWebUISessionTimeout(m_ui->spinSessionTimeout->value()); // Authentication pref->setWebUiUsername(webUiUsername()); @@ -1156,6 +1158,7 @@ void OptionsDialog::loadOptions() m_ui->checkBypassAuthSubnetWhitelist->setChecked(pref->isWebUiAuthSubnetWhitelistEnabled()); m_ui->IPSubnetWhitelistButton->setEnabled(m_ui->checkBypassAuthSubnetWhitelist->isChecked()); m_ui->spinBanCounter->setValue(pref->getWebUIMaxAuthFailCount()); + m_ui->spinBanDuration->setValue(pref->getWebUIBanDuration().count()); m_ui->spinSessionTimeout->setValue(pref->getWebUISessionTimeout()); // Security diff --git a/src/gui/optionsdialog.ui b/src/gui/optionsdialog.ui index f46bded2d..dd0857d37 100644 --- a/src/gui/optionsdialog.ui +++ b/src/gui/optionsdialog.ui @@ -2987,25 +2987,15 @@ Specify an IPv4 or IPv6 address. You can specify "0.0.0.0" for any IPv - - + + Ban client after consecutive failures: - - - - Never - - - 2147483647 - - - - + Qt::Horizontal @@ -3018,6 +3008,39 @@ Specify an IPv4 or IPv6 address. You can specify "0.0.0.0" for any IPv + + + + Never + + + 2147483647 + + + + + + + ban for: + + + Qt::AlignRight|Qt::AlignTrailing|Qt::AlignVCenter + + + + + + + sec + + + 1 + + + 2147483647 + + + diff --git a/src/webui/api/appcontroller.cpp b/src/webui/api/appcontroller.cpp index c28800445..49f5c5a72 100644 --- a/src/webui/api/appcontroller.cpp +++ b/src/webui/api/appcontroller.cpp @@ -233,6 +233,7 @@ void AppController::preferencesAction() authSubnetWhitelistStringList << Utils::Net::subnetToString(subnet); data["bypass_auth_subnet_whitelist"] = authSubnetWhitelistStringList.join("\n"); data["web_ui_max_auth_fail_count"] = pref->getWebUIMaxAuthFailCount(); + data["web_ui_ban_duration"] = static_cast(pref->getWebUIBanDuration().count()); data["web_ui_session_timeout"] = pref->getWebUISessionTimeout(); // Use alternative Web UI data["alternative_webui_enabled"] = pref->isAltWebUiEnabled(); @@ -604,6 +605,8 @@ void AppController::setPreferencesAction() } if (hasKey("web_ui_max_auth_fail_count")) pref->setWebUIMaxAuthFailCount(it.value().toInt()); + if (hasKey("web_ui_ban_duration")) + pref->setWebUIBanDuration(std::chrono::seconds {it.value().toInt()}); if (hasKey("web_ui_session_timeout")) pref->setWebUISessionTimeout(it.value().toInt()); // Use alternative Web UI diff --git a/src/webui/api/authcontroller.cpp b/src/webui/api/authcontroller.cpp index 132bb0abe..f8446145d 100644 --- a/src/webui/api/authcontroller.cpp +++ b/src/webui/api/authcontroller.cpp @@ -36,8 +36,6 @@ #include "apierror.h" #include "isessionmanager.h" -constexpr int BAN_TIME = 3600000; // 1 hour - void AuthController::loginAction() { if (sessionManager()->session()) { @@ -116,6 +114,6 @@ void AuthController::increaseFailedAttempts() if (failedLogin.failedAttemptsCount >= Preferences::instance()->getWebUIMaxAuthFailCount()) { // Max number of failed attempts reached // Start ban period - failedLogin.banTimer.setRemainingTime(BAN_TIME); + failedLogin.banTimer.setRemainingTime(Preferences::instance()->getWebUIBanDuration()); } } diff --git a/src/webui/www/private/views/preferences.html b/src/webui/www/private/views/preferences.html index 55c9e411a..8f5e1cc8a 100644 --- a/src/webui/www/private/views/preferences.html +++ b/src/webui/www/private/views/preferences.html @@ -733,6 +733,10 @@ + + + QBT_TR(seconds)QBT_TR[CONTEXT=OptionsDialog] + @@ -1725,6 +1729,7 @@ $('bypass_auth_subnet_whitelist_textarea').setProperty('value', pref.bypass_auth_subnet_whitelist); updateBypasssAuthSettings(); $('webUIMaxAuthFailCountInput').setProperty('value', pref.web_ui_max_auth_fail_count.toInt()); + $('webUIBanDurationInput').setProperty('value', pref.web_ui_ban_duration.toInt()); $('webUISessionTimeoutInput').setProperty('value', pref.web_ui_session_timeout.toInt()); // Use alternative Web UI @@ -2089,6 +2094,7 @@ 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_max_auth_fail_count', $('webUIMaxAuthFailCountInput').getProperty('value')); + settings.set('web_ui_ban_duration', $('webUIBanDurationInput').getProperty('value')); settings.set('web_ui_session_timeout', $('webUISessionTimeoutInput').getProperty('value')); // Use alternative Web UI