From 9bc20f0b425b1d4f64aab735ee9693a489adb274 Mon Sep 17 00:00:00 2001
From: Janek Bevendorff <janek@jbev.net>
Date: Mon, 18 Feb 2019 17:21:02 +0100
Subject: Prevent unnecessary key transformations, resolves #2704

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.
---
 src/core/Database.cpp                                  | 18 ++++++++++++++++--
 src/core/Database.h                                    |  3 ++-
 .../dbsettings/DatabaseSettingsWidgetEncryption.cpp    |  2 +-
 src/gui/dbsettings/DatabaseSettingsWidgetMasterKey.cpp |  6 +++++-
 4 files changed, 24 insertions(+), 5 deletions(-)

diff --git a/src/core/Database.cpp b/src/core/Database.cpp
index 13dea7d1c..94113546d 100644
--- a/src/core/Database.cpp
+++ b/src/core/Database.cpp
@@ -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);
@@ -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;
 }
@@ -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);
 
@@ -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;
     }
 
diff --git a/src/core/Database.h b/src/core/Database.h
index 84e2f90ef..8df2b9317 100644
--- a/src/core/Database.h
+++ b/src/core/Database.h
@@ -108,7 +108,8 @@ public:
     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;
diff --git a/src/gui/dbsettings/DatabaseSettingsWidgetEncryption.cpp b/src/gui/dbsettings/DatabaseSettingsWidgetEncryption.cpp
index b536dfc71..80860673e 100644
--- a/src/gui/dbsettings/DatabaseSettingsWidgetEncryption.cpp
+++ b/src/gui/dbsettings/DatabaseSettingsWidgetEncryption.cpp
@@ -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;
     }
diff --git a/src/gui/dbsettings/DatabaseSettingsWidgetMasterKey.cpp b/src/gui/dbsettings/DatabaseSettingsWidgetMasterKey.cpp
index a95f5b322..a8cff2c6d 100644
--- a/src/gui/dbsettings/DatabaseSettingsWidgetMasterKey.cpp
+++ b/src/gui/dbsettings/DatabaseSettingsWidgetMasterKey.cpp
@@ -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;
 }
 
-- 
cgit v1.2.3