"Weak password" warning incorrectly shown after adding (or removing) a key file from database

[zxsldfjsalkdf]

"Weak password" warning incorrectly shown after adding (or removing) a key file from database

[0xdeadbeer]

A few of my additional findings:

• it does not matter what encryption algorithm you used
• nor what kind of password you have, whether you generated it or not

I suspect that it comes down to this:

keepassxc/src/gui/dbsettings/DatabaseSettingsWidgetDatabaseKey.cpp

Lines 181 to 205 in da90319

	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;
	}

To my surprise, m_passwordEditWidget->isEmpty() does not return true if the user hasn't clicked on "Change Password" button in the Database Security Settings.. an additional check with m_passwordEditWidget->visiblePage() == KeyComponentWidget::Page::Edit is I hope enough to safe-guard it..? Like so:

diff --git a/src/gui/dbsettings/DatabaseSettingsWidgetDatabaseKey.cpp b/src/gui/dbsettings/DatabaseSettingsWidgetDatabaseKey.cpp
index 1de8e6a9..0c26bccb 100644
--- a/src/gui/dbsettings/DatabaseSettingsWidgetDatabaseKey.cpp
+++ b/src/gui/dbsettings/DatabaseSettingsWidgetDatabaseKey.cpp
@@ -178,7 +178,8 @@ bool DatabaseSettingsWidgetDatabaseKey::save()
     }
 
     // Show warning if database password is weak
-    if (!m_passwordEditWidget->isEmpty()
+    bool isNewPasswordDirty = !m_passwordEditWidget->isEmpty() && m_passwordEditWidget->visiblePage() == KeyComponentWidget::Page::Edit;
+    if (isNewPasswordDirty
         && m_passwordEditWidget->getPasswordQuality() < PasswordHealth::Quality::Good) {
         auto dialogResult = MessageBox::warning(this,
                                                 tr("Weak password"),
@@ -195,7 +196,7 @@ bool DatabaseSettingsWidgetDatabaseKey::save()
     // 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) {
+    if (isNewPasswordDirty && m_passwordEditWidget->getPasswordQuality() < minQuality) {
         MessageBox::critical(this,
                              tr("Weak password"),
                              tr("You must enter a stronger password to protect your database."),

[droidmonkey]

That code won't compile. simply call isVisible() on the password widget.

[0xdeadbeer]

Strange, it compiles on my machine..? Anyhow, after some checking to me it seems that calling m_passwordEditWidget->isVisible() at that point in time returns true even if the user does not click on "Change Password"..? Even weirder is the fact that the visible property seems to be false at that point when inspected through Gamma Ray.. ->isVisible() still returns true though.. will look further into the source code the coming days