Skip to content

Commit

Permalink
Limit zxcvbn entropy estimation length
Browse files Browse the repository at this point in the history 
Limit the use of zxcvbn based password entropy estimation to 256 bytes. After this threshold, the average per-byte entropy from the zxcvbn calculation is added for each additional byte. In practice, this produces a slightly higher entropy calculation for purely randomized passwords than zxcvbn would normally calculate. However, the time to calculate is capped leading to a much better user experience and removing unnecessary calculations.

Fixes #7712
  • Loading branch information
@libklein @droidmonkey
libklein authored and droidmonkey committed May 28, 2022
1 parent a4d4adb commit 9720030
Show file tree
Hide file tree
Showing 6 changed files with 158 additions and 106 deletions.
32 changes: 25 additions & 7 deletions src/core/PasswordHealth.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,33 @@
#include "PasswordHealth.h"
#include "zxcvbn.h"

namespace
{
const static int ZXCVBN_ESTIMATE_THRESHOLD = 256;
} // namespace

PasswordHealth::PasswordHealth(double entropy)
: m_score(entropy)
, m_entropy(entropy)
{
init(entropy);
}

PasswordHealth::PasswordHealth(const QString& pwd)
{
auto entropy = 0.0;
auto pwdBytes = pwd.toUtf8();
entropy += ZxcvbnMatch(pwdBytes.left(ZXCVBN_ESTIMATE_THRESHOLD), nullptr, nullptr);
if (pwdBytes.size() > ZXCVBN_ESTIMATE_THRESHOLD) {
// Add the average entropy per byte for any additional bytes above the estimate threshold
auto average = entropy / ZXCVBN_ESTIMATE_THRESHOLD;
entropy += average * (pwdBytes.size() - ZXCVBN_ESTIMATE_THRESHOLD);
}
init(entropy);
}

void PasswordHealth::init(double entropy)
{
m_score = m_entropy = entropy;

switch (quality()) {
case Quality::Bad:
case Quality::Poor:
Expand All @@ -43,11 +66,6 @@ PasswordHealth::PasswordHealth(double entropy)
}
}

PasswordHealth::PasswordHealth(const QString& pwd)
: PasswordHealth(ZxcvbnMatch(pwd.toUtf8(), nullptr, nullptr))
{
}

void PasswordHealth::setScore(int score)
{
m_score = score;
Expand Down
2 changes: 2 additions & 0 deletions src/core/PasswordHealth.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ class PasswordHealth
explicit PasswordHealth(double entropy);
explicit PasswordHealth(const QString& pwd);

void init(double entropy);

/*
* The password score is defined to be the greater the better
* (more secure) the password is. It doesn't have a dimension,
Expand Down
98 changes: 44 additions & 54 deletions src/gui/PasswordGeneratorWidget.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ PasswordGeneratorWidget::PasswordGeneratorWidget(QWidget* parent)
connect(shortcut, &QShortcut::activated, this, [this] { applyPassword(); });

connect(m_ui->editNewPassword, SIGNAL(textChanged(QString)), SLOT(updateButtonsEnabled(QString)));
connect(m_ui->editNewPassword, SIGNAL(textChanged(QString)), SLOT(updatePasswordStrength(QString)));
connect(m_ui->editNewPassword, SIGNAL(textChanged(QString)), SLOT(updatePasswordStrength()));
connect(m_ui->buttonAdvancedMode, SIGNAL(toggled(bool)), SLOT(setAdvancedMode(bool)));
connect(m_ui->buttonAddHex, SIGNAL(clicked()), SLOT(excludeHexChars()));
connect(m_ui->editAdditionalChars, SIGNAL(textChanged(QString)), SLOT(updateGenerator()));
Expand Down Expand Up @@ -115,9 +115,7 @@ PasswordGeneratorWidget::PasswordGeneratorWidget(QWidget* parent)
loadSettings();
}

PasswordGeneratorWidget::~PasswordGeneratorWidget()
{
}
PasswordGeneratorWidget::~PasswordGeneratorWidget() = default;

void PasswordGeneratorWidget::closeEvent(QCloseEvent* event)
{
Expand Down Expand Up @@ -253,15 +251,11 @@ void PasswordGeneratorWidget::regeneratePassword()
{
if (m_ui->tabWidget->currentIndex() == Password) {
if (m_passwordGenerator->isValid()) {
QString password = m_passwordGenerator->generatePassword();
m_ui->editNewPassword->setText(password);
updatePasswordStrength(password);
m_ui->editNewPassword->setText(m_passwordGenerator->generatePassword());
}
} else {
if (m_dicewareGenerator->isValid()) {
QString password = m_dicewareGenerator->generatePassphrase();
m_ui->editNewPassword->setText(password);
updatePasswordStrength(password);
m_ui->editNewPassword->setText(m_dicewareGenerator->generatePassphrase());
}
}
}
Expand All @@ -274,21 +268,52 @@ void PasswordGeneratorWidget::updateButtonsEnabled(const QString& password)
m_ui->buttonCopy->setEnabled(!password.isEmpty());
}

void PasswordGeneratorWidget::updatePasswordStrength(const QString& password)
void PasswordGeneratorWidget::updatePasswordStrength()
{
PasswordHealth health(password);
// Calculate the password / passphrase health
PasswordHealth passwordHealth(0);
if (m_ui->tabWidget->currentIndex() == Diceware) {
// Diceware estimates entropy differently
health = PasswordHealth(m_dicewareGenerator->estimateEntropy());

m_ui->charactersInPassphraseLabel->setText(QString::number(password.length()));
passwordHealth.init(m_dicewareGenerator->estimateEntropy());
m_ui->charactersInPassphraseLabel->setText(QString::number(m_ui->editNewPassword->text().length()));
} else {
passwordHealth = PasswordHealth(m_ui->editNewPassword->text());
}

m_ui->entropyLabel->setText(tr("Entropy: %1 bit").arg(QString::number(health.entropy(), 'f', 2)));
// Update the entropy text labels
m_ui->entropyLabel->setText(tr("Entropy: %1 bit").arg(QString::number(passwordHealth.entropy(), 'f', 2)));
m_ui->entropyProgressBar->setValue(std::min(int(passwordHealth.entropy()), m_ui->entropyProgressBar->maximum()));

// Update the visual strength meter
QString style = m_ui->entropyProgressBar->styleSheet();
QRegularExpression re("(QProgressBar::chunk\\s*\\{.*?background-color:)[^;]+;",
QRegularExpression::CaseInsensitiveOption | QRegularExpression::DotMatchesEverythingOption);
style.replace(re, "\\1 %1;");

StateColorPalette statePalette;
switch (passwordHealth.quality()) {
case PasswordHealth::Quality::Bad:
case PasswordHealth::Quality::Poor:
m_ui->entropyProgressBar->setStyleSheet(
style.arg(statePalette.color(StateColorPalette::HealthCritical).name()));
m_ui->strengthLabel->setText(tr("Password Quality: %1").arg(tr("Poor", "Password quality")));
break;

case PasswordHealth::Quality::Weak:
m_ui->entropyProgressBar->setStyleSheet(style.arg(statePalette.color(StateColorPalette::HealthBad).name()));
m_ui->strengthLabel->setText(tr("Password Quality: %1").arg(tr("Weak", "Password quality")));
break;

m_ui->entropyProgressBar->setValue(std::min(int(health.entropy()), m_ui->entropyProgressBar->maximum()));
case PasswordHealth::Quality::Good:
m_ui->entropyProgressBar->setStyleSheet(style.arg(statePalette.color(StateColorPalette::HealthOk).name()));
m_ui->strengthLabel->setText(tr("Password Quality: %1").arg(tr("Good", "Password quality")));
break;

colorStrengthIndicator(health);
case PasswordHealth::Quality::Excellent:
m_ui->entropyProgressBar->setStyleSheet(
style.arg(statePalette.color(StateColorPalette::HealthExcellent).name()));
m_ui->strengthLabel->setText(tr("Password Quality: %1").arg(tr("Excellent", "Password quality")));
break;
}
}

void PasswordGeneratorWidget::applyPassword()
Expand Down Expand Up @@ -471,41 +496,6 @@ void PasswordGeneratorWidget::excludeHexChars()
updateGenerator();
}

void PasswordGeneratorWidget::colorStrengthIndicator(const PasswordHealth& health)
{
// Take the existing stylesheet and convert the text and background color to arguments
QString style = m_ui->entropyProgressBar->styleSheet();
QRegularExpression re("(QProgressBar::chunk\\s*\\{.*?background-color:)[^;]+;",
QRegularExpression::CaseInsensitiveOption | QRegularExpression::DotMatchesEverythingOption);
style.replace(re, "\\1 %1;");

StateColorPalette statePalette;
switch (health.quality()) {
case PasswordHealth::Quality::Bad:
case PasswordHealth::Quality::Poor:
m_ui->entropyProgressBar->setStyleSheet(
style.arg(statePalette.color(StateColorPalette::HealthCritical).name()));
m_ui->strengthLabel->setText(tr("Password Quality: %1").arg(tr("Poor", "Password quality")));
break;

case PasswordHealth::Quality::Weak:
m_ui->entropyProgressBar->setStyleSheet(style.arg(statePalette.color(StateColorPalette::HealthBad).name()));
m_ui->strengthLabel->setText(tr("Password Quality: %1").arg(tr("Weak", "Password quality")));
break;

case PasswordHealth::Quality::Good:
m_ui->entropyProgressBar->setStyleSheet(style.arg(statePalette.color(StateColorPalette::HealthOk).name()));
m_ui->strengthLabel->setText(tr("Password Quality: %1").arg(tr("Good", "Password quality")));
break;

case PasswordHealth::Quality::Excellent:
m_ui->entropyProgressBar->setStyleSheet(
style.arg(statePalette.color(StateColorPalette::HealthExcellent).name()));
m_ui->strengthLabel->setText(tr("Password Quality: %1").arg(tr("Excellent", "Password quality")));
break;
}
}

PasswordGenerator::CharClasses PasswordGeneratorWidget::charClasses()
{
PasswordGenerator::CharClasses classes;
Expand Down
12 changes: 6 additions & 6 deletions src/gui/PasswordGeneratorWidget.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#define KEEPASSX_PASSWORDGENERATORWIDGET_H

#include <QComboBox>
#include <QTimer>

#include "core/PassphraseGenerator.h"
#include "core/PasswordGenerator.h"
Expand Down Expand Up @@ -57,6 +58,10 @@ class PasswordGeneratorWidget : public QWidget

static PasswordGeneratorWidget* popupGenerator(QWidget* parent = nullptr);

signals:
void appliedPassword(const QString& password);
void closed();

public slots:
void regeneratePassword();
void applyPassword();
Expand All @@ -65,19 +70,14 @@ public slots:
void deleteWordList();
void addWordList();

signals:
void appliedPassword(const QString& password);
void closed();

private slots:
void updateButtonsEnabled(const QString& password);
void updatePasswordStrength(const QString& password);
void updatePasswordStrength();
void setAdvancedMode(bool advanced);
void excludeHexChars();

void passwordLengthChanged(int length);
void passphraseLengthChanged(int length);
void colorStrengthIndicator(const PasswordHealth& health);

void updateGenerator();

Expand Down
119 changes: 80 additions & 39 deletions tests/gui/TestGui.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -648,8 +648,79 @@ void TestGui::testAddEntry()
QTRY_COMPARE(entryView->model()->rowCount(), 3);
}

void TestGui::testPasswordEntryEntropy_data()
{
QTest::addColumn<QString>("password");
QTest::addColumn<QString>("expectedEntropyLabel");
QTest::addColumn<QString>("expectedStrengthLabel");

QTest::newRow("Empty password") << ""
<< "Entropy: 0.00 bit"
<< "Password Quality: Poor";

QTest::newRow("Well-known password") << "hello"
<< "Entropy: 6.38 bit"
<< "Password Quality: Poor";

QTest::newRow("Password composed of well-known words.") << "helloworld"
<< "Entropy: 13.10 bit"
<< "Password Quality: Poor";

QTest::newRow("Password composed of well-known words with number.") << "password1"
<< "Entropy: 4.00 bit"
<< "Password Quality: Poor";

QTest::newRow("Password out of small character space.") << "D0g.................."
<< "Entropy: 19.02 bit"
<< "Password Quality: Poor";

QTest::newRow("XKCD, easy substitutions.") << "Tr0ub4dour&3"
<< "Entropy: 30.87 bit"
<< "Password Quality: Poor";

QTest::newRow("XKCD, word generator.") << "correcthorsebatterystaple"
<< "Entropy: 47.98 bit"
<< "Password Quality: Weak";

QTest::newRow("Random characters, medium length.") << "YQC3kbXbjC652dTDH"
<< "Entropy: 95.83 bit"
<< "Password Quality: Good";

QTest::newRow("Random characters, long.") << "Bs5ZFfthWzR8DGFEjaCM6bGqhmCT4km"
<< "Entropy: 174.59 bit"
<< "Password Quality: Excellent";

QTest::newRow("Long password using Zxcvbn chunk estimation")
<< "quintet-tamper-kinswoman-humility-vengeful-haven-tastiness-aspire-widget-ipad-cussed-reaffirm-ladylike-"
"ashamed-anatomy-daybed-jam-swear-strudel-neatness-stalemate-unbundle-flavored-relation-emergency-underrate-"
"registry-getting-award-unveiled-unshaken-stagnate-cartridge-magnitude-ointment-hardener-enforced-scrubbed-"
"radial-fiddling-envelope-unpaved-moisture-unused-crawlers-quartered-crushed-kangaroo-tiptop-doily"
<< "Entropy: 1205.85 bit"
<< "Password Quality: Excellent";

QTest::newRow("Longer password above Zxcvbn threshold")
<< "quintet-tamper-kinswoman-humility-vengeful-haven-tastiness-aspire-widget-ipad-cussed-reaffirm-ladylike-"
"ashamed-anatomy-daybed-jam-swear-strudel-neatness-stalemate-unbundle-flavored-relation-emergency-underrate-"
"registry-getting-award-unveiled-unshaken-stagnate-cartridge-magnitude-ointment-hardener-enforced-scrubbed-"
"radial-fiddling-envelope-unpaved-moisture-unused-crawlers-quartered-crushed-kangaroo-tiptop-doily-hefty-"
"untie-fidgeting-radiance-twilight-freebase-sulphuric-parrot-decree-monotype-nautical-pout-sip-geometric-"
"crunching-deviancy-festival-hacking-rage-unify-coronary-zigzagged-dwindle-possum-lilly-exhume-daringly-"
"barbell-rage-animate-lapel-emporium-renounce-justifier-relieving-gauze-arrive-alive-collected-immobile-"
"unleash-snowman-gift-expansion-marbles-requisite-excusable-flatness-displace-caloric-sensuous-moustache-"
"sensuous-capillary-aversion-contents-cadet-giggly-amenity-peddling-spotting-drier-mooned-rudder-peroxide-"
"posting-oppressor-scrabble-scorer-whomever-paprika-slapstick-said-spectacle-capture-debate-attire-emcee-"
"unfocused-sympathy-doily-election-ambulance-polish-subtype-grumbling-neon-stooge-reanalyze-rockfish-"
"disparate-decorated-washroom-threefold-muzzle-buckwheat-kerosene-swell-why-reprocess-correct-shady-"
"impatient-slit-banshee-scrubbed-dreadful-unlocking-urologist-hurried-citable-fragment-septic-lapped-"
"prankish-phantom-unpaved-moisture-unused-crawlers-quartered-crushed-kangaroo-lapel-emporium-renounce"
<< "Entropy: 4210.27 bit"
<< "Password Quality: Excellent";
}

void TestGui::testPasswordEntryEntropy()
{
const qint64 MAX_TIME_TO_FIRST_RESPONSE = 75;

auto* toolBar = m_mainWindow->findChild<QToolBar*>("toolBar");

// Find the new entry action
Expand Down Expand Up @@ -689,45 +760,13 @@ void TestGui::testPasswordEntryEntropy()
auto* entropyLabel = pwGeneratorWidget->findChild<QLabel*>("entropyLabel");
auto* strengthLabel = pwGeneratorWidget->findChild<QLabel*>("strengthLabel");

generatedPassword->setText("");
QTest::keyClicks(generatedPassword, "hello");
QCOMPARE(entropyLabel->text(), QString("Entropy: 6.38 bit"));
QCOMPARE(strengthLabel->text(), QString("Password Quality: Poor"));

generatedPassword->setText("");
QTest::keyClicks(generatedPassword, "helloworld");
QCOMPARE(entropyLabel->text(), QString("Entropy: 13.10 bit"));
QCOMPARE(strengthLabel->text(), QString("Password Quality: Poor"));

generatedPassword->setText("");
QTest::keyClicks(generatedPassword, "password1");
QCOMPARE(entropyLabel->text(), QString("Entropy: 4.00 bit"));
QCOMPARE(strengthLabel->text(), QString("Password Quality: Poor"));

generatedPassword->setText("");
QTest::keyClicks(generatedPassword, "D0g..................");
QCOMPARE(entropyLabel->text(), QString("Entropy: 19.02 bit"));
QCOMPARE(strengthLabel->text(), QString("Password Quality: Poor"));

generatedPassword->setText("");
QTest::keyClicks(generatedPassword, "Tr0ub4dour&3");
QCOMPARE(entropyLabel->text(), QString("Entropy: 30.87 bit"));
QCOMPARE(strengthLabel->text(), QString("Password Quality: Poor"));

generatedPassword->setText("");
QTest::keyClicks(generatedPassword, "correcthorsebatterystaple");
QCOMPARE(entropyLabel->text(), QString("Entropy: 47.98 bit"));
QCOMPARE(strengthLabel->text(), QString("Password Quality: Weak"));

generatedPassword->setText("");
QTest::keyClicks(generatedPassword, "YQC3kbXbjC652dTDH");
QCOMPARE(entropyLabel->text(), QString("Entropy: 95.83 bit"));
QCOMPARE(strengthLabel->text(), QString("Password Quality: Good"));
QFETCH(QString, password);
QFETCH(QString, expectedEntropyLabel);
QFETCH(QString, expectedStrengthLabel);

generatedPassword->setText("");
QTest::keyClicks(generatedPassword, "Bs5ZFfthWzR8DGFEjaCM6bGqhmCT4km");
QCOMPARE(entropyLabel->text(), QString("Entropy: 174.59 bit"));
QCOMPARE(strengthLabel->text(), QString("Password Quality: Excellent"));
generatedPassword->setText(password);
QCOMPARE(entropyLabel->text(), expectedEntropyLabel);
QCOMPARE(strengthLabel->text(), expectedStrengthLabel);

QTest::mouseClick(generatedPassword, Qt::LeftButton);
QTest::keyClick(generatedPassword, Qt::Key_Escape););
Expand Down Expand Up @@ -786,9 +825,11 @@ void TestGui::testDicewareEntryEntropy()
// Verify entropy and strength
auto* entropyLabel = pwGeneratorWidget->findChild<QLabel*>("entropyLabel");
auto* strengthLabel = pwGeneratorWidget->findChild<QLabel*>("strengthLabel");
auto* wordLengthLabel = pwGeneratorWidget->findChild<QLabel*>("charactersInPassphraseLabel");

QCOMPARE(entropyLabel->text(), QString("Entropy: 77.55 bit"));
QTRY_COMPARE_WITH_TIMEOUT(entropyLabel->text(), QString("Entropy: 77.55 bit"), 200);
QCOMPARE(strengthLabel->text(), QString("Password Quality: Good"));
QCOMPARE(wordLengthLabel->text().toInt(), pwGeneratorWidget->getGeneratedPassword().size());

QTest::mouseClick(generatedPassword, Qt::LeftButton);
QTest::keyClick(generatedPassword, Qt::Key_Escape););
Expand Down
1 change: 1 addition & 0 deletions tests/gui/TestGui.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ private slots:
void testSearchEditEntry();
void testAddEntry();
void testPasswordEntryEntropy();
void testPasswordEntryEntropy_data();
void testDicewareEntryEntropy();
void testTotp();
void testSearch();
Expand Down

0 comments on commit 9720030

Please sign in to comment.