Skip to content

Commit

Permalink
Fix keepassxc-browser entropy display
Browse files Browse the repository at this point in the history
Pass correct entropy amount to keepassxc-browser instead of amount of bits in password/passphrase.
Rename json key from "login" to "entropy" (keeping "login" key as well for backwards compatibility).

Also make some changes to entropy calculation methods:
  - Rename PassphraseGenerator::calculateEntropy to getCurrentEntropy
  - Rename PasswordGenerator::calculateEntropy to estimateEntropy
  - Implement PasswordGenerator::getCurrentEntropy
  • Loading branch information
AndrolGenhald committed May 1, 2019
1 parent 1cbd395 commit aebdbbe
Show file tree
Hide file tree
Showing 8 changed files with 81 additions and 16 deletions.
8 changes: 4 additions & 4 deletions src/browser/BrowserAction.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -279,16 +279,16 @@ QJsonObject BrowserAction::handleGetLogins(const QJsonObject& json, const QStrin
QJsonObject BrowserAction::handleGeneratePassword(const QJsonObject& json, const QString& action)
{
const QString nonce = json.value("nonce").toString();
const QString password = browserSettings()->generatePassword();
const QJsonObject password = browserSettings()->generatePassword();

if (nonce.isEmpty() || password.isEmpty()) {
return QJsonObject();
}

QJsonArray arr;
QJsonObject passwd;
passwd["login"] = QString::number(password.length() * 8); // bits;
passwd["password"] = password;
QJsonObject passwd = password;
// For backwards compatibility, "login" doesn't really make sense here
passwd["login"] = passwd["entropy"];
arr.append(passwd);

const QString newNonce = incrementNonce(nonce);
Expand Down
11 changes: 8 additions & 3 deletions src/browser/BrowserSettings.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -502,19 +502,24 @@ PasswordGenerator::GeneratorFlags BrowserSettings::passwordGeneratorFlags()
return flags;
}

QString BrowserSettings::generatePassword()
QJsonObject BrowserSettings::generatePassword()
{
QJsonObject password;
if (generatorType() == 0) {
m_passwordGenerator.setLength(passwordLength());
m_passwordGenerator.setCharClasses(passwordCharClasses());
m_passwordGenerator.setFlags(passwordGeneratorFlags());
return m_passwordGenerator.generatePassword();
const QString pw = m_passwordGenerator.generatePassword();
password["entropy"] = m_passwordGenerator.estimateEntropy(pw);
password["password"] = pw;
} else {
m_passPhraseGenerator.setDefaultWordList();
m_passPhraseGenerator.setWordCount(passPhraseWordCount());
m_passPhraseGenerator.setWordSeparator(passPhraseWordSeparator());
return m_passPhraseGenerator.generatePassphrase();
password["entropy"] = m_passPhraseGenerator.getCurrentEntropy();
password["password"] = m_passPhraseGenerator.generatePassphrase();
}
return password;
}

void BrowserSettings::updateBinaryPaths(const QString& customProxyLocation)
Expand Down
2 changes: 1 addition & 1 deletion src/browser/BrowserSettings.h
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ class BrowserSettings
void setPasswordLength(int length);
PasswordGenerator::CharClasses passwordCharClasses();
PasswordGenerator::GeneratorFlags passwordGeneratorFlags();
QString generatePassword();
QJsonObject generatePassword();
void updateBinaryPaths(const QString& customProxyLocation = QString());
bool checkIfProxyExists(QString& path);

Expand Down
4 changes: 1 addition & 3 deletions src/core/PassphraseGenerator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,8 @@ PassphraseGenerator::PassphraseGenerator()
{
}

double PassphraseGenerator::calculateEntropy(const QString& passphrase)
double PassphraseGenerator::getCurrentEntropy()
{
Q_UNUSED(passphrase);

if (m_wordlist.isEmpty()) {
return 0.0;
}
Expand Down
2 changes: 1 addition & 1 deletion src/core/PassphraseGenerator.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ class PassphraseGenerator
PassphraseGenerator();
Q_DISABLE_COPY(PassphraseGenerator)

double calculateEntropy(const QString& passphrase);
double getCurrentEntropy();
void setWordCount(int wordCount);
void setWordList(const QString& path);
void setDefaultWordList();
Expand Down
62 changes: 61 additions & 1 deletion src/core/PasswordGenerator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,10 @@

#include "PasswordGenerator.h"

#include <algorithm>
#include <cmath>
#include <numeric>

#include "crypto/Random.h"
#include <zxcvbn.h>

Expand All @@ -31,7 +35,63 @@ PasswordGenerator::PasswordGenerator()
{
}

double PasswordGenerator::calculateEntropy(const QString& password)
double PasswordGenerator::getCurrentEntropy()
{
Q_ASSERT(isValid());

const QVector<PasswordGroup> groups = passwordGroups();

int totalSize = 0;
for (const PasswordGroup& group : groups) {
// Note: Character groups must be non-overlapping
totalSize += group.size();
}
if (!(m_flags & CharFromEveryGroup)) {
return std::log2(totalSize) * m_length;
}

// CharFromEveryGroup means all groups are required
QVector<int> requiredGroups(groups.size());
std::iota(requiredGroups.begin(), requiredGroups.end(), 0);

// Get all subsets of the set of required groups
// From https://stackoverflow.com/a/16310898 by Ronald Rey (https://stackoverflow.com/users/2175684/ronald-rey)
QVector<QVector<int>> subsets;
QVector<int> empty;
subsets.push_back(empty);
for (int i = 0; i < requiredGroups.size(); ++i) {
QVector<QVector<int>> subsetTemp = subsets;
for (int j = 0; j < subsetTemp.size(); ++j) {
subsetTemp[j].push_back(requiredGroups[i]);
}
for (int j = 0; j < subsetTemp.size(); ++j) {
subsets.push_back(subsetTemp[j]);
}
}

// Sort subsets by size ascending
std::sort(subsets.begin(), subsets.end(), [](const QVector<int>& a, const QVector<int>& b) -> bool {
return a.size() < b.size();
});

// Inclusion-exclusion
// Note: This depends on character groups being non-overlapping
double passwordSpace = 0;
int includeExclude = 1;
QVector<QVector<int>>::iterator subset = subsets.begin();
for (int i = 0; i < requiredGroups.size(); ++i, includeExclude *= -1) {
do {
int totalSubsetSize = 0;
for (int groupIndex : *subset) {
totalSubsetSize += groups[groupIndex].size();
}
passwordSpace += includeExclude * pow((totalSize - totalSubsetSize), m_length);
} while ((++subset)->size() == i);
}
return std::log2(passwordSpace);
}

double PasswordGenerator::estimateEntropy(const QString& password)
{
return ZxcvbnMatch(password.toLatin1(), nullptr, nullptr);
}
Expand Down
3 changes: 2 additions & 1 deletion src/core/PasswordGenerator.h
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,8 @@ class PasswordGenerator
public:
PasswordGenerator();

double calculateEntropy(const QString& password);
double getCurrentEntropy();
double estimateEntropy(const QString& password);
void setLength(int length);
void setCharClasses(const CharClasses& classes);
void setFlags(const GeneratorFlags& flags);
Expand Down
5 changes: 3 additions & 2 deletions src/gui/PasswordGeneratorWidget.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -247,9 +247,10 @@ void PasswordGeneratorWidget::updatePasswordStrength(const QString& password)
{
double entropy = 0.0;
if (m_ui->tabWidget->currentIndex() == Password) {
entropy = m_passwordGenerator->calculateEntropy(password);
entropy = m_passwordGenerator->estimateEntropy(password);
} else {
entropy = m_dicewareGenerator->calculateEntropy(password);
// TODO see issue #867
entropy = m_dicewareGenerator->getCurrentEntropy();
}

m_ui->entropyLabel->setText(tr("Entropy: %1 bit").arg(QString::number(entropy, 'f', 2)));
Expand Down

0 comments on commit aebdbbe

Please sign in to comment.