Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add configurable password strength check on database password #9782

Merged
merged 13 commits into from
Jan 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions share/translations/keepassxc_en.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1839,6 +1839,18 @@ Are you sure you want to continue without a password?</source>
<source>Failed to change database credentials</source>
<translation type="unfinished"></translation>
</message>
<message>
<source>Weak password</source>
<translation type="unfinished"></translation>
</message>
<message>
<source>You must enter a stronger password to protect your database.</source>
<translation type="unfinished"></translation>
</message>
<message>
<source>This is a weak password! For better protection of your secrets, you should choose a stronger password.</source>
<translation type="unfinished"></translation>
</message>
</context>
<context>
<name>DatabaseSettingsWidgetEncryption</name>
Expand Down Expand Up @@ -6507,6 +6519,10 @@ Do you want to overwrite it?</source>
<source>Continue</source>
<translation type="unfinished"></translation>
</message>
<message>
<source>Continue with weak password</source>
<translation type="unfinished"></translation>
</message>
</context>
<context>
<name>QObject</name>
Expand Down
1 change: 1 addition & 0 deletions src/core/Config.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,7 @@ static const QHash<Config::ConfigKey, ConfigDirective> 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}},
Expand Down
1 change: 1 addition & 0 deletions src/core/Config.h
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,7 @@ class Config : public QObject
Security_NoConfirmMoveEntryToRecycleBin,
Security_EnableCopyOnDoubleClick,
Security_QuickUnlock,
Security_DatabasePasswordMinimumQuality,

Browser_Enabled,
Browser_ShowNotification,
Expand Down
1 change: 1 addition & 0 deletions src/gui/MessageBox.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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}},
};
}

Expand Down
3 changes: 2 additions & 1 deletion src/gui/MessageBox.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 8 additions & 0 deletions src/gui/databasekey/PasswordEditWidget.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
3 changes: 3 additions & 0 deletions src/gui/databasekey/PasswordEditWidget.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@

#include "KeyComponentWidget.h"

#include "core/PasswordHealth.h"

namespace Ui
{
class PasswordEditWidget;
Expand All @@ -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:
Expand Down
30 changes: 30 additions & 0 deletions src/gui/dbsettings/DatabaseSettingsWidgetDatabaseKey.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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<QMessageBox> msgBox(new QMessageBox(this));
msgBox->setIcon(QMessageBox::Warning);
Expand All @@ -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<PasswordHealth::Quality>(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;
}
Expand Down
3 changes: 3 additions & 0 deletions tests/gui/TestGui.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down
2 changes: 2 additions & 0 deletions tests/gui/TestGuiFdoSecrets.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down