Prevent duplicate entries in passphrase wordlists

Replace a QVector for the wordlist with a QSet. This removes all duplicate entries in a given wordlist.
Thus, it hinders a malicious wordlist that has the proper length (>4000 entries) but with repetitions (effectively << 4000 entries) to be used and potentially create weaker passphrases than estimated.

Example:
List with 4000 items but only 64 unique words would lead to only 48 bit of Entropy instead of ~95 bit!
This commit is contained in:
christianwengert 2024-06-28 11:24:44 +02:00 committed by Jonathan White
parent 9b4e6b4e11
commit c7d318236f
8 changed files with 47 additions and 26 deletions

View File

@ -7653,10 +7653,6 @@ Do you want to overwrite it?</source>
<source>Invalid word count %1</source>
<translation type="unfinished"></translation>
</message>
<message>
<source>The word list is too small (&lt; 1000 items)</source>
<translation type="unfinished"></translation>
</message>
<message>
<source>Title for the entry.</source>
<translation type="unfinished"></translation>
@ -8902,6 +8898,10 @@ This option is deprecated, use --set-key-file instead.</source>
<source>Only PBKDF and Argon2 are supported, cannot decrypt json file</source>
<translation type="unfinished"></translation>
</message>
<message>
<source>Cannot generate valid passphrases because the wordlist is too short</source>
<translation type="unfinished"></translation>
</message>
</context>
<context>
<name>QtIOCompressor</name>

View File

@ -71,7 +71,7 @@ int Diceware::execute(const QStringList& arguments)
if (!dicewareGenerator.isValid()) {
// We already validated the word count input so if the generator is invalid, it
// must be because the word list is too small.
err << QObject::tr("The word list is too small (< 1000 items)") << Qt::endl;
err << QObject::tr("Cannot generate valid passphrases because the wordlist is too short") << Qt::endl;
return EXIT_FAILURE;
}

View File

@ -18,12 +18,14 @@
#include "PassphraseGenerator.h"
#include <QFile>
#include <QSet>
#include <QTextStream>
#include <cmath>
#include "core/Resources.h"
#include "crypto/Random.h"
const int PassphraseGenerator::DefaultWordCount = 7;
const char* PassphraseGenerator::DefaultSeparator = " ";
const char* PassphraseGenerator::DefaultWordList = "eff_large.wordlist";
@ -60,10 +62,12 @@ void PassphraseGenerator::setWordCase(PassphraseWordCase wordCase)
void PassphraseGenerator::setWordList(const QString& path)
{
m_wordlist.clear();
// Initially load wordlist into a set to avoid duplicates
QSet<QString> wordset;
QFile file(path);
if (!file.open(QIODevice::ReadOnly | QIODevice::Text)) {
qWarning("Couldn't load passphrase wordlist.");
qWarning("Couldn't load passphrase wordlist: %s", qPrintable(path));
return;
}
@ -87,14 +91,15 @@ void PassphraseGenerator::setWordList(const QString& path)
line = line.trimmed();
line.replace(rx, "\\2");
if (!line.isEmpty()) {
m_wordlist.append(line);
wordset.insert(line);
}
line = in.readLine();
}
if (m_wordlist.size() < 4000) {
qWarning("Wordlist too short!");
return;
m_wordlist = wordset.toList();
if (m_wordlist.size() < m_minimum_wordlist_length) {
qWarning("Wordlist is less than minimum acceptable size: %s", qPrintable(path));
}
}
@ -111,18 +116,15 @@ void PassphraseGenerator::setWordSeparator(const QString& separator)
QString PassphraseGenerator::generatePassphrase() const
{
QString tmpWord;
Q_ASSERT(isValid());
// In case there was an error loading the wordlist
if (m_wordlist.length() == 0) {
if (!isValid() || m_wordlist.empty()) {
return {};
}
QStringList words;
for (int i = 0; i < m_wordCount; ++i) {
int wordIndex = randomGen()->randomUInt(static_cast<quint32>(m_wordlist.length()));
tmpWord = m_wordlist.at(wordIndex);
int wordIndex = randomGen()->randomUInt(static_cast<quint32>(m_wordlist.size()));
auto tmpWord = m_wordlist.at(wordIndex);
// convert case
switch (m_wordCase) {
@ -133,7 +135,6 @@ QString PassphraseGenerator::generatePassphrase() const
tmpWord = tmpWord.replace(0, 1, tmpWord.left(1).toUpper());
break;
case LOWERCASE:
default:
tmpWord = tmpWord.toLower();
break;
}
@ -145,9 +146,5 @@ QString PassphraseGenerator::generatePassphrase() const
bool PassphraseGenerator::isValid() const
{
if (m_wordCount == 0) {
return false;
}
return m_wordlist.size() >= 1000;
return m_wordCount > 0 && m_wordlist.size() >= m_minimum_wordlist_length;
}

View File

@ -18,7 +18,7 @@
#ifndef KEEPASSX_PASSPHRASEGENERATOR_H
#define KEEPASSX_PASSPHRASEGENERATOR_H
#include <QVector>
#include <QList>
class PassphraseGenerator
{
@ -43,15 +43,18 @@ public:
QString generatePassphrase() const;
static constexpr int DefaultWordCount = 7;
static const int DefaultWordCount;
static const char* DefaultSeparator;
static const char* DefaultWordList;
private:
int m_wordCount;
int m_minimum_wordlist_length = 4000;
PassphraseWordCase m_wordCase;
QString m_separator;
QVector<QString> m_wordlist;
QList<QString> m_wordlist;
friend class TestPassphraseGenerator;
};
#endif // KEEPASSX_PASSPHRASEGENERATOR_H

View File

@ -1088,7 +1088,7 @@ void TestCli::testDiceware()
smallWordFile.close();
execCmd(dicewareCmd, {"diceware", "-W", "11", "-w", smallWordFile.fileName()});
QCOMPARE(m_stderr->readLine(), QByteArray("The word list is too small (< 1000 items)\n"));
QCOMPARE(m_stderr->readLine(), QByteArray("Cannot generate valid passphrases because the wordlist is too short\n"));
}
void TestCli::testEdit()

View File

@ -16,6 +16,7 @@
*/
#include "TestPassphraseGenerator.h"
#include "config-keepassx-tests.h"
#include "core/PassphraseGenerator.h"
#include "crypto/Crypto.h"
@ -52,3 +53,18 @@ void TestPassphraseGenerator::testWordCase()
QRegularExpression regex("^(?:[A-Z][a-z-]* )*[A-Z][a-z-]*$");
QVERIFY2(regex.match(passphrase).hasMatch(), qPrintable(passphrase));
}
void TestPassphraseGenerator::testUniqueEntriesInWordlist()
{
PassphraseGenerator generator;
// set the limit down, so we don;t have to do a very large file
generator.m_minimum_wordlist_length = 4;
// link to bad wordlist
QString path = QString(KEEPASSX_TEST_DATA_DIR).append("/wordlists/bad_wordlist_with_duplicate_entries.wordlist");
// setting will work, it creates the warning however, and isValid will fail
generator.setWordList(path);
// so this fails
QVERIFY(!generator.isValid());
}

View File

@ -27,6 +27,7 @@ class TestPassphraseGenerator : public QObject
private slots:
void initTestCase();
void testWordCase();
void testUniqueEntriesInWordlist();
};
#endif // KEEPASSXC_TESTPASSPHRASEGENERATOR_H

View File

@ -0,0 +1,4 @@
abacus
abdomen
abdominal
abdominal