Dynamically determine database validity

* Check that the database composite key exists, has sub-keys associated with it, and the root group exists.
This commit is contained in:
Jonathan White 2020-03-04 09:37:13 -05:00
parent 7ac292e09b
commit 91c6e436b3
8 changed files with 14 additions and 49 deletions

View file

@ -1122,9 +1122,7 @@ QSharedPointer<Database> BrowserService::selectedDatabase()
for (int i = 0;; ++i) { for (int i = 0;; ++i) {
auto* dbWidget = m_dbTabWidget->databaseWidgetFromIndex(i); auto* dbWidget = m_dbTabWidget->databaseWidgetFromIndex(i);
// Add only open databases // Add only open databases
if (dbWidget && dbWidget->database()->hasKey() if (dbWidget && !dbWidget->isLocked()) {
&& (dbWidget->currentMode() == DatabaseWidget::Mode::ViewMode
|| dbWidget->currentMode() == DatabaseWidget::Mode::EditMode)) {
databaseWidgets.push_back(dbWidget); databaseWidgets.push_back(dbWidget);
continue; continue;
} }

View file

@ -121,7 +121,6 @@ int Create::execute(const QStringList& arguments)
QSharedPointer<Database> db(new Database); QSharedPointer<Database> db(new Database);
db->setKey(key); db->setKey(key);
db->setInitialized(true);
if (decryptionTime != 0) { if (decryptionTime != 0) {
auto kdf = db->kdf(); auto kdf = db->kdf();

View file

@ -84,7 +84,6 @@ int Import::execute(const QStringList& arguments)
Database db; Database db;
db.setKdf(KeePass2::uuidToKdf(KeePass2::KDF_ARGON2)); db.setKdf(KeePass2::uuidToKdf(KeePass2::KDF_ARGON2));
db.setKey(key); db.setKey(key);
db.setInitialized(true);
if (!db.import(xmlExportPath, &errorMessage)) { if (!db.import(xmlExportPath, &errorMessage)) {
errorTextStream << QObject::tr("Unable to import XML database: %1").arg(errorMessage) << endl; errorTextStream << QObject::tr("Unable to import XML database: %1").arg(errorMessage) << endl;

View file

@ -112,13 +112,6 @@ bool Database::open(QSharedPointer<const CompositeKey> key, QString* error, bool
*/ */
bool Database::open(const QString& filePath, QSharedPointer<const CompositeKey> key, QString* error, bool readOnly) bool Database::open(const QString& filePath, QSharedPointer<const CompositeKey> key, QString* error, bool readOnly)
{ {
if (isInitialized() && m_modified) {
emit databaseDiscarded();
}
m_initialized = false;
setEmitModified(false);
QFile dbFile(filePath); QFile dbFile(filePath);
if (!dbFile.exists()) { if (!dbFile.exists()) {
if (error) { if (error) {
@ -138,6 +131,8 @@ bool Database::open(const QString& filePath, QSharedPointer<const CompositeKey>
return false; return false;
} }
setEmitModified(false);
KeePass2Reader reader; KeePass2Reader reader;
if (!reader.readDatabase(&dbFile, std::move(key), this)) { if (!reader.readDatabase(&dbFile, std::move(key), this)) {
if (error) { if (error) {
@ -152,7 +147,6 @@ bool Database::open(const QString& filePath, QSharedPointer<const CompositeKey>
markAsClean(); markAsClean();
m_initialized = true;
emit databaseOpened(); emit databaseOpened();
m_fileWatcher->start(canonicalFilePath(), 30, 1); m_fileWatcher->start(canonicalFilePath(), 30, 1);
setEmitModified(true); setEmitModified(true);
@ -220,7 +214,7 @@ bool Database::saveAs(const QString& filePath, QString* error, bool atomic, bool
} }
// Never save an uninitialized database // Never save an uninitialized database
if (!m_initialized) { if (!isInitialized()) {
if (error) { if (error) {
*error = tr("Could not save, database has not been initialized!"); *error = tr("Could not save, database has not been initialized!");
} }
@ -346,7 +340,7 @@ bool Database::writeDatabase(QIODevice* device, QString* error)
} }
PasswordKey oldTransformedKey; PasswordKey oldTransformedKey;
if (m_data.hasKey) { if (m_data.key->isEmpty()) {
oldTransformedKey.setHash(m_data.transformedMasterKey->rawKey()); oldTransformedKey.setHash(m_data.transformedMasterKey->rawKey());
} }
@ -440,7 +434,6 @@ void Database::releaseData()
m_deletedObjects.clear(); m_deletedObjects.clear();
m_commonUsernames.clear(); m_commonUsernames.clear();
m_initialized = false;
m_modified = false; m_modified = false;
m_modifiedTimer.stop(); m_modifiedTimer.stop();
} }
@ -496,22 +489,14 @@ void Database::setReadOnly(bool readOnly)
} }
/** /**
* Returns true if database has been fully decrypted and populated, i.e. if * Returns true if the database key exists, has subkeys, and the
* it's not just an empty default instance. * root group exists
* *
* @return true if database has been fully initialized * @return true if database has been fully initialized
*/ */
bool Database::isInitialized() const bool Database::isInitialized() const
{ {
return m_initialized; return m_data.key && !m_data.key->isEmpty() && m_rootGroup;
}
/**
* @param initialized true to mark database as initialized
*/
void Database::setInitialized(bool initialized)
{
m_initialized = initialized;
} }
Group* Database::rootGroup() Group* Database::rootGroup()
@ -535,7 +520,7 @@ void Database::setRootGroup(Group* group)
{ {
Q_ASSERT(group); Q_ASSERT(group);
if (isInitialized() && m_modified) { if (isInitialized() && isModified()) {
emit databaseDiscarded(); emit databaseDiscarded();
} }
@ -723,7 +708,6 @@ bool Database::setKey(const QSharedPointer<const CompositeKey>& key,
if (!key) { if (!key) {
m_data.key.reset(); m_data.key.reset();
m_data.transformedMasterKey.reset(new PasswordKey()); m_data.transformedMasterKey.reset(new PasswordKey());
m_data.hasKey = false;
return true; return true;
} }
@ -733,7 +717,7 @@ bool Database::setKey(const QSharedPointer<const CompositeKey>& key,
} }
PasswordKey oldTransformedMasterKey; PasswordKey oldTransformedMasterKey;
if (m_data.hasKey) { if (m_data.key && !m_data.key->isEmpty()) {
oldTransformedMasterKey.setHash(m_data.transformedMasterKey->rawKey()); oldTransformedMasterKey.setHash(m_data.transformedMasterKey->rawKey());
} }
@ -749,7 +733,6 @@ bool Database::setKey(const QSharedPointer<const CompositeKey>& key,
if (!transformedMasterKey.isEmpty()) { if (!transformedMasterKey.isEmpty()) {
m_data.transformedMasterKey->setHash(transformedMasterKey); m_data.transformedMasterKey->setHash(transformedMasterKey);
} }
m_data.hasKey = true;
if (updateChangedTime) { if (updateChangedTime) {
m_metadata->setMasterKeyChanged(Clock::currentDateTimeUtc()); m_metadata->setMasterKeyChanged(Clock::currentDateTimeUtc());
} }
@ -761,14 +744,9 @@ bool Database::setKey(const QSharedPointer<const CompositeKey>& key,
return true; return true;
} }
bool Database::hasKey() const
{
return m_data.hasKey;
}
bool Database::verifyKey(const QSharedPointer<CompositeKey>& key) const bool Database::verifyKey(const QSharedPointer<CompositeKey>& key) const
{ {
Q_ASSERT(hasKey()); Q_ASSERT(!m_data.key->isEmpty());
if (!m_data.challengeResponseKey->rawKey().isEmpty()) { if (!m_data.challengeResponseKey->rawKey().isEmpty()) {
QByteArray result; QByteArray result;

View file

@ -81,7 +81,6 @@ public:
void releaseData(); void releaseData();
bool isInitialized() const; bool isInitialized() const;
void setInitialized(bool initialized);
bool isModified() const; bool isModified() const;
void setEmitModified(bool value); void setEmitModified(bool value);
bool isReadOnly() const; bool isReadOnly() const;
@ -115,7 +114,6 @@ public:
QList<QString> commonUsernames(); QList<QString> commonUsernames();
bool hasKey() const;
QSharedPointer<const CompositeKey> key() const; QSharedPointer<const CompositeKey> key() const;
bool setKey(const QSharedPointer<const CompositeKey>& key, bool setKey(const QSharedPointer<const CompositeKey>& key,
bool updateChangedTime = true, bool updateChangedTime = true,
@ -168,7 +166,6 @@ private:
QScopedPointer<PasswordKey> transformedMasterKey; QScopedPointer<PasswordKey> transformedMasterKey;
QScopedPointer<PasswordKey> challengeResponseKey; QScopedPointer<PasswordKey> challengeResponseKey;
bool hasKey = false;
QSharedPointer<const CompositeKey> key; QSharedPointer<const CompositeKey> key;
QSharedPointer<Kdf> kdf = QSharedPointer<AesKdf>::create(true); QSharedPointer<Kdf> kdf = QSharedPointer<AesKdf>::create(true);
@ -190,7 +187,6 @@ private:
transformedMasterKey.reset(); transformedMasterKey.reset();
challengeResponseKey.reset(); challengeResponseKey.reset();
hasKey = false;
key.reset(); key.reset();
kdf.reset(); kdf.reset();
@ -212,7 +208,6 @@ private:
QTimer m_modifiedTimer; QTimer m_modifiedTimer;
QMutex m_saveMutex; QMutex m_saveMutex;
QPointer<FileWatcher> m_fileWatcher; QPointer<FileWatcher> m_fileWatcher;
bool m_initialized = false;
bool m_modified = false; bool m_modified = false;
bool m_emitModified; bool m_emitModified;

View file

@ -671,7 +671,7 @@ void DatabaseTabWidget::relockPendingDatabase()
return; return;
} }
if (m_dbWidgetPendingLock->isLocked() || !m_dbWidgetPendingLock->database()->hasKey()) { if (m_dbWidgetPendingLock->isLocked() || !m_dbWidgetPendingLock->database()->isInitialized()) {
m_dbWidgetPendingLock = nullptr; m_dbWidgetPendingLock = nullptr;
return; return;
} }

View file

@ -57,11 +57,7 @@ NewDatabaseWizard::~NewDatabaseWizard()
bool NewDatabaseWizard::validateCurrentPage() bool NewDatabaseWizard::validateCurrentPage()
{ {
bool ok = m_pages[currentId()]->validatePage(); return m_pages[currentId()]->validatePage();
if (ok && currentId() == m_pages.size() - 1) {
m_db->setInitialized(true);
}
return ok;
} }
/** /**

View file

@ -106,7 +106,7 @@ void TestKeePass1Reader::testBasic()
void TestKeePass1Reader::testMasterKey() void TestKeePass1Reader::testMasterKey()
{ {
QVERIFY(m_db->hasKey()); QVERIFY(m_db->isInitialized());
QCOMPARE(m_db->kdf()->rounds(), 713); QCOMPARE(m_db->kdf()->rounds(), 713);
} }