From d44486ce9435f4e6f69a4398334e248e56e64ac4 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 33dfbd1ad..6d77ba8a5 100644
--- a/share/translations/keepassxc_en.ts
+++ b/share/translations/keepassxc_en.ts
@@ -1843,6 +1843,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
@@ -6511,6 +6523,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 a5ab7e2b6..41943ae80 100644
--- a/src/core/Config.cpp
+++ b/src/core/Config.cpp
@@ -144,6 +144,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 59014f04d..31bd261c9 100644
--- a/src/core/Config.h
+++ b/src/core/Config.h
@@ -124,6 +124,7 @@ public:
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 04e6ccb29..e99ec4cfd 100644
--- a/src/gui/MessageBox.cpp
+++ b/src/gui/MessageBox.cpp
@@ -66,6 +66,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 46939a535..1dbcc2076 100644
--- a/src/gui/MessageBox.h
+++ b/src/gui/MessageBox.h
@@ -58,10 +58,11 @@ public:
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 b6d2b3ec7..04d63c43d 100644
--- a/src/gui/databasekey/PasswordEditWidget.cpp
+++ b/src/gui/databasekey/PasswordEditWidget.cpp
@@ -62,6 +62,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 6fc9370ad..5e7bb28d6 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 @@ public:
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 bac749979..cad13f1db 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"
@@ -153,6 +155,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);
@@ -171,6 +174,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 4840092ea..f52e30629 100644
--- a/tests/gui/TestGui.cpp
+++ b/tests/gui/TestGui.cpp
@@ -286,7 +286,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 be5f44e4e..c690dedf7 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();