From 8c91836038b7ec77df2a34e9f0397e5db170c250 Mon Sep 17 00:00:00 2001 From: egglessness <14147983+egglessness@users.noreply.github.com> Date: Sat, 6 Jan 2024 19:53:18 +0100 Subject: [PATCH] Add configurable password strength check on database password (#9782) * Set default value of DatabasePasswordMinimumQuality to 3 (do not accept a master password that is less than Good) * Add custom message box button "Continue with weak password" --- share/translations/keepassxc_en.ts | 16 ++++++++++ src/core/Config.cpp | 1 + src/core/Config.h | 1 + src/gui/MessageBox.cpp | 1 + src/gui/MessageBox.h | 3 +- src/gui/databasekey/PasswordEditWidget.cpp | 8 +++++ src/gui/databasekey/PasswordEditWidget.h | 3 ++ .../DatabaseSettingsWidgetDatabaseKey.cpp | 30 +++++++++++++++++++ tests/gui/TestGui.cpp | 3 ++ tests/gui/TestGuiFdoSecrets.cpp | 2 ++ 10 files changed, 67 insertions(+), 1 deletion(-) diff --git a/share/translations/keepassxc_en.ts b/share/translations/keepassxc_en.ts index aa23543ba8..cd46884e67 100644 --- a/share/translations/keepassxc_en.ts +++ b/share/translations/keepassxc_en.ts @@ -1873,6 +1873,18 @@ Are you sure you want to continue without a password? Failed to change database credentials + + Weak password + + + + You must enter a stronger password to protect your database. + + + + This is a weak password! For better protection of your secrets, you should choose a stronger password. + + DatabaseSettingsWidgetEncryption @@ -6656,6 +6668,10 @@ Do you want to overwrite it? Continue + + Continue with weak password + + QObject diff --git a/src/core/Config.cpp b/src/core/Config.cpp index 63fc20ef69..ed570cdd91 100644 --- a/src/core/Config.cpp +++ b/src/core/Config.cpp @@ -147,6 +147,7 @@ static const QHash configStrings = { {Config::Security_NoConfirmMoveEntryToRecycleBin,{QS("Security/NoConfirmMoveEntryToRecycleBin"), Roaming, true}}, {Config::Security_EnableCopyOnDoubleClick,{QS("Security/EnableCopyOnDoubleClick"), Roaming, false}}, {Config::Security_QuickUnlock, {QS("Security/QuickUnlock"), Local, true}}, + {Config::Security_DatabasePasswordMinimumQuality, {QS("Security/DatabasePasswordMinimumQuality"), Local, 0}}, // Browser {Config::Browser_Enabled, {QS("Browser/Enabled"), Roaming, false}}, diff --git a/src/core/Config.h b/src/core/Config.h index 25220b0cf2..9624a1a847 100644 --- a/src/core/Config.h +++ b/src/core/Config.h @@ -127,6 +127,7 @@ class Config : public QObject Security_NoConfirmMoveEntryToRecycleBin, Security_EnableCopyOnDoubleClick, Security_QuickUnlock, + Security_DatabasePasswordMinimumQuality, Browser_Enabled, Browser_ShowNotification, diff --git a/src/gui/MessageBox.cpp b/src/gui/MessageBox.cpp index 7cf8eb0ffb..a2c1498cdd 100644 --- a/src/gui/MessageBox.cpp +++ b/src/gui/MessageBox.cpp @@ -67,6 +67,7 @@ void MessageBox::initializeButtonDefs() {Disable, {QMessageBox::tr("Disable"), QMessageBox::ButtonRole::AcceptRole}}, {Merge, {QMessageBox::tr("Merge"), QMessageBox::ButtonRole::AcceptRole}}, {Continue, {QMessageBox::tr("Continue"), QMessageBox::ButtonRole::AcceptRole}}, + {ContinueWithWeakPass, {QMessageBox::tr("Continue with weak password"), QMessageBox::ButtonRole::AcceptRole}}, }; } diff --git a/src/gui/MessageBox.h b/src/gui/MessageBox.h index 46939a535a..1dbcc2076c 100644 --- a/src/gui/MessageBox.h +++ b/src/gui/MessageBox.h @@ -58,10 +58,11 @@ class MessageBox Disable = 1 << 25, Merge = 1 << 26, Continue = 1 << 27, + ContinueWithWeakPass = 1 << 28, // Internal loop markers. Update Last when new KeePassXC button is added First = Ok, - Last = Continue, + Last = ContinueWithWeakPass, }; enum Action diff --git a/src/gui/databasekey/PasswordEditWidget.cpp b/src/gui/databasekey/PasswordEditWidget.cpp index 5ed6b63284..647bdc2d2b 100644 --- a/src/gui/databasekey/PasswordEditWidget.cpp +++ b/src/gui/databasekey/PasswordEditWidget.cpp @@ -64,6 +64,14 @@ bool PasswordEditWidget::isEmpty() const return (visiblePage() == Page::Edit) && m_compUi->enterPasswordEdit->text().isEmpty(); } +PasswordHealth::Quality PasswordEditWidget::getPasswordQuality() const +{ + QString pwd = m_compUi->enterPasswordEdit->text(); + PasswordHealth passwordHealth(pwd); + + return passwordHealth.quality(); +} + QWidget* PasswordEditWidget::componentEditWidget() { m_compEditWidget = new QWidget(); diff --git a/src/gui/databasekey/PasswordEditWidget.h b/src/gui/databasekey/PasswordEditWidget.h index 6fc9370ad7..5e7bb28d61 100644 --- a/src/gui/databasekey/PasswordEditWidget.h +++ b/src/gui/databasekey/PasswordEditWidget.h @@ -20,6 +20,8 @@ #include "KeyComponentWidget.h" +#include "core/PasswordHealth.h" + namespace Ui { class PasswordEditWidget; @@ -38,6 +40,7 @@ class PasswordEditWidget : public KeyComponentWidget void setPasswordVisible(bool visible); bool isPasswordVisible() const; bool isEmpty() const; + PasswordHealth::Quality getPasswordQuality() const; bool validate(QString& errorMessage) const override; protected: diff --git a/src/gui/dbsettings/DatabaseSettingsWidgetDatabaseKey.cpp b/src/gui/dbsettings/DatabaseSettingsWidgetDatabaseKey.cpp index 266770ffc1..552098227c 100644 --- a/src/gui/dbsettings/DatabaseSettingsWidgetDatabaseKey.cpp +++ b/src/gui/dbsettings/DatabaseSettingsWidgetDatabaseKey.cpp @@ -17,7 +17,9 @@ #include "DatabaseSettingsWidgetDatabaseKey.h" +#include "core/Config.h" #include "core/Database.h" +#include "core/PasswordHealth.h" #include "gui/MessageBox.h" #include "gui/databasekey/KeyFileEditWidget.h" #include "gui/databasekey/PasswordEditWidget.h" @@ -163,6 +165,7 @@ bool DatabaseSettingsWidgetDatabaseKey::save() } } + // Show warning if database password has not been set if (m_passwordEditWidget->visiblePage() == KeyComponentWidget::Page::AddNew || m_passwordEditWidget->isEmpty()) { QScopedPointer msgBox(new QMessageBox(this)); msgBox->setIcon(QMessageBox::Warning); @@ -182,6 +185,33 @@ bool DatabaseSettingsWidgetDatabaseKey::save() return false; } + // Show warning if database password is weak + if (!m_passwordEditWidget->isEmpty() + && m_passwordEditWidget->getPasswordQuality() < PasswordHealth::Quality::Good) { + auto dialogResult = MessageBox::warning(this, + tr("Weak password"), + tr("This is a weak password! For better protection of your secrets, " + "you should choose a stronger password."), + MessageBox::ContinueWithWeakPass | MessageBox::Cancel, + MessageBox::Cancel); + + if (dialogResult == MessageBox::Cancel) { + return false; + } + } + + // If enforced in the config file, deny users from continuing with a weak password + auto minQuality = + static_cast(config()->get(Config::Security_DatabasePasswordMinimumQuality).toInt()); + if (!m_passwordEditWidget->isEmpty() && m_passwordEditWidget->getPasswordQuality() < minQuality) { + MessageBox::critical(this, + tr("Weak password"), + tr("You must enter a stronger password to protect your database."), + MessageBox::Ok, + MessageBox::Ok); + return false; + } + if (!addToCompositeKey(m_keyFileEditWidget, newKey, oldFileKey)) { return false; } diff --git a/tests/gui/TestGui.cpp b/tests/gui/TestGui.cpp index bdc364ca66..f65c42940b 100644 --- a/tests/gui/TestGui.cpp +++ b/tests/gui/TestGui.cpp @@ -289,7 +289,10 @@ void TestGui::testCreateDatabase() tmpFile.close(); fileDialog()->setNextFileName(tmpFile.fileName()); + // click Continue on the warning due to weak password + MessageBox::setNextAnswer(MessageBox::ContinueWithWeakPass); QTest::keyClick(fileEdit, Qt::Key::Key_Enter); + tmpFile.remove();); triggerAction("actionDatabaseNew"); diff --git a/tests/gui/TestGuiFdoSecrets.cpp b/tests/gui/TestGuiFdoSecrets.cpp index 7a83a05f90..8de34e71a3 100644 --- a/tests/gui/TestGuiFdoSecrets.cpp +++ b/tests/gui/TestGuiFdoSecrets.cpp @@ -1879,6 +1879,8 @@ bool TestGuiFdoSecrets::driveNewDatabaseWizard() tmpFile.close(); fileDialog()->setNextFileName(tmpFile.fileName()); + // click Continue on the warning due to weak password + MessageBox::setNextAnswer(MessageBox::ContinueWithWeakPass); wizard->accept(); tmpFile.remove();