Skip to content

Commit

Permalink
Prevent unnecessary key transformations, resolves #2704
Browse files Browse the repository at this point in the history
The database master key settings widget does not actually
need to (re-)transform the master key, it only needs to update
the Key object on the database. Transformation can be deferred
until the Database is persisted to disk. This avoids delays
and unnecessary user interaction with challenge-response
dongles by eliminating redundant key transformations.
  • Loading branch information
phoerious authored Feb 18, 2019
1 parent 0c58799 commit 9bc20f0
Show file tree
Hide file tree
Showing 4 changed files with 24 additions and 5 deletions.
18 changes: 16 additions & 2 deletions src/core/Database.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,7 @@ bool Database::writeDatabase(QIODevice* device, QString* error)
return false;
}

QByteArray oldTransformedKey = m_data.transformedMasterKey;
KeePass2Writer writer;
setEmitModified(false);
writer.writeDatabase(device, this);
Expand All @@ -288,6 +289,15 @@ bool Database::writeDatabase(QIODevice* device, QString* error)
return false;
}

Q_ASSERT(!m_data.transformedMasterKey.isEmpty());
Q_ASSERT(m_data.transformedMasterKey != oldTransformedKey);
if (m_data.transformedMasterKey.isEmpty() || m_data.transformedMasterKey == oldTransformedKey) {
if (error) {
*error = tr("Key not transformed. This is a bug, please report it to the developers!");
}
return false;
}

markAsClean();
return true;
}
Expand Down Expand Up @@ -499,9 +509,11 @@ void Database::setCompressionAlgorithm(Database::CompressionAlgorithm algo)
* @param key key to set and transform or nullptr to reset the key
* @param updateChangedTime true to update database change time
* @param updateTransformSalt true to update the transform salt
* @param transformKey trigger the KDF after setting the key
* @return true on success
*/
bool Database::setKey(const QSharedPointer<const CompositeKey>& key, bool updateChangedTime, bool updateTransformSalt)
bool Database::setKey(const QSharedPointer<const CompositeKey>& key, bool updateChangedTime,
bool updateTransformSalt, bool transformKey)
{
Q_ASSERT(!m_data.isReadOnly);

Expand All @@ -519,7 +531,9 @@ bool Database::setKey(const QSharedPointer<const CompositeKey>& key, bool update

QByteArray oldTransformedMasterKey = m_data.transformedMasterKey;
QByteArray transformedMasterKey;
if (!key->transform(*m_data.kdf, transformedMasterKey)) {
if (!transformKey) {
transformedMasterKey = oldTransformedMasterKey;
} else if (!key->transform(*m_data.kdf, transformedMasterKey)) {
return false;
}

Expand Down
3 changes: 2 additions & 1 deletion src/core/Database.h
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,8 @@ class Database : public QObject
QSharedPointer<const CompositeKey> key() const;
bool setKey(const QSharedPointer<const CompositeKey>& key,
bool updateChangedTime = true,
bool updateTransformSalt = false);
bool updateTransformSalt = false,
bool transformKey = true);
QByteArray challengeResponseKey() const;
bool challengeMasterSeed(const QByteArray& masterSeed);
bool verifyKey(const QSharedPointer<CompositeKey>& key) const;
Expand Down
2 changes: 1 addition & 1 deletion src/gui/dbsettings/DatabaseSettingsWidgetEncryption.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ void DatabaseSettingsWidgetEncryption::initialize()
isDirty = true;
}
if (!m_db->key()) {
m_db->setKey(QSharedPointer<CompositeKey>::create());
m_db->setKey(QSharedPointer<CompositeKey>::create(), true, false, false);
m_db->setCipher(KeePass2::CIPHER_AES256);
isDirty = true;
}
Expand Down
6 changes: 5 additions & 1 deletion src/gui/dbsettings/DatabaseSettingsWidgetMasterKey.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -190,9 +190,13 @@ bool DatabaseSettingsWidgetMasterKey::save()
}
}

m_db->setKey(newKey);
m_db->setKey(newKey, true, false, false);

emit editFinished(true);
if (m_isDirty) {
m_db->markAsModified();
}

return true;
}

Expand Down

0 comments on commit 9bc20f0

Please sign in to comment.