Add challenge response to key before transformation, resolves #1060

* Re-implement KDBX4 challenge-response key assembly with transform
seed instead of master seed

src/core/Database.cpp

@@ -255,10 +255,19 @@ void Database::setCompressionAlgo(Database::CompressionAlgorithm algo)
     m_data.compressionAlgo = algo;
 }
 
+/**
+ * Set and transform a new encryption key.
+ *
+ * @param key key to set and transform
+ * @param updateChangedTime true to update database change time
+ * @param updateTransformSalt true to update the transform salt
+ * @return true on success
+ */
 bool Database::setKey(const CompositeKey& key, bool updateChangedTime, bool updateTransformSalt)
 {
     if (updateTransformSalt) {
         m_data.kdf->randomizeSeed();
+        Q_ASSERT(!m_data.kdf->seed().isEmpty());
     }
 
     QByteArray oldTransformedMasterKey = m_data.transformedMasterKey;

src/format/Kdbx4Reader.cpp

@@ -20,7 +20,6 @@
 #include <QBuffer>
 
 #include "core/Group.h"
-#include "core/Database.h"
 #include "core/Endian.h"
 #include "crypto/CryptoHash.h"
 #include "format/KeePass2RandomStream.h"
@@ -48,19 +47,13 @@ Database* Kdbx4Reader::readDatabaseImpl(QIODevice* device, const QByteArray& hea
         return nullptr;
     }
 
-    if (!m_db->setKey(key, false)) {
+    if (!m_db->setKey(key, false, false)) {
         raiseError(tr("Unable to calculate master key"));
         return nullptr;
     }
 
-    if (!m_db->challengeMasterSeed(m_masterSeed)) {
-        raiseError(tr("Unable to issue challenge-response."));
-        return nullptr;
-    }
-
     CryptoHash hash(CryptoHash::Sha256);
     hash.addData(m_masterSeed);
-    hash.addData(m_db->challengeResponseKey());
     hash.addData(m_db->transformedMasterKey());
     QByteArray finalKey = hash.result();
 

src/format/Kdbx4Writer.cpp

@@ -51,11 +51,6 @@ bool Kdbx4Writer::writeDatabase(QIODevice* device, Database* db)
     QByteArray startBytes;
     QByteArray endOfHeader = "\r\n\r\n";
 
-    if (!db->challengeMasterSeed(masterSeed)) {
-        raiseError(tr("Unable to issue challenge-response."));
-        return false;
-    }
-
     if (!db->setKey(db->key(), false, true)) {
         raiseError(tr("Unable to calculate master key"));
         return false;
@@ -64,7 +59,6 @@ bool Kdbx4Writer::writeDatabase(QIODevice* device, Database* db)
     // generate transformed master key
     CryptoHash hash(CryptoHash::Sha256);
     hash.addData(masterSeed);
-    hash.addData(db->challengeResponseKey());
     Q_ASSERT(!db->transformedMasterKey().isEmpty());
     hash.addData(db->transformedMasterKey());
     QByteArray finalKey = hash.result();

src/keys/CompositeKey.cpp

@@ -19,6 +19,7 @@
 #include "CompositeKey.h"
 #include <QFile>
 #include <QtConcurrent>
+#include <format/KeePass2.h>
 
 #include "core/Global.h"
 #include "crypto/CryptoHash.h"
@@ -73,7 +74,31 @@ CompositeKey& CompositeKey::operator=(const CompositeKey& key)
     return *this;
 }
 
+/**
+ * Get raw key hash as bytes.
+ *
+ * The key hash does not contain contributions by challenge-response components for
+ * backwards compatibility with KeePassXC's pre-KDBX4 challenge-response
+ * implementation. To include challenge-response in the raw key,
+ * use \link CompositeKey::rawKey(const QByteArray*) instead.
+ *
+ * @return key hash
+ */
 QByteArray CompositeKey::rawKey() const
+{
+    return rawKey(nullptr);
+}
+
+/**
+ * Get raw key hash as bytes.
+ *
+ * Challenge-response key components will use the provided <tt>transformSeed</tt>
+ * as a challenge to acquire their key contribution.
+ *
+ * @param transformSeed transform seed to challenge or nullptr to exclude challenge-response components
+ * @return key hash
+ */
+QByteArray CompositeKey::rawKey(const QByteArray* transformSeed) const
 {
     CryptoHash cryptoHash(CryptoHash::Sha256);
 
@@ -81,12 +106,38 @@ QByteArray CompositeKey::rawKey() const
         cryptoHash.addData(key->rawKey());
     }
 
+    if (transformSeed) {
+        QByteArray challengeResult;
+        challenge(*transformSeed, challengeResult);
+        cryptoHash.addData(challengeResult);
+    }
+
     return cryptoHash.result();
 }
 
+/**
+ * Transform this composite key.
+ *
+ * If using AES-KDF as transform function, the transformed key will not include
+ * any challenge-response components. Only static key components will be hashed
+ * for backwards-compatibility with KeePassXC's KDBX3 implementation, which added
+ * challenge response key components after key transformation.
+ * KDBX4+ KDFs transform the whole key including challenge-response components.
+ *
+ * @param kdf key derivation function
+ * @param result transformed key hash
+ * @return true on success
+ */
 bool CompositeKey::transform(const Kdf& kdf, QByteArray& result) const
 {
-    return kdf.transform(rawKey(), result);
+    if (kdf.uuid() == KeePass2::KDF_AES) {
+        // legacy KDBX3 AES-KDF, challenge response is added later to the hash
+        return kdf.transform(rawKey(), result);
+    }
+
+    QByteArray seed = kdf.seed();
+    Q_ASSERT(!seed.isEmpty());
+    return kdf.transform(rawKey(&seed), result);
 }
 
 bool CompositeKey::challenge(const QByteArray& seed, QByteArray& result) const
@@ -103,6 +154,7 @@ bool CompositeKey::challenge(const QByteArray& seed, QByteArray& result) const
     for (const auto key : m_challengeResponseKeys) {
         // if the device isn't present or fails, return an error
         if (!key->challenge(seed)) {
+            qWarning("Failed to issue challenge");
             return false;
         }
         cryptoHash.addData(key->rawKey());

src/keys/CompositeKey.h

@@ -32,13 +32,14 @@ class CompositeKey : public Key
 public:
     CompositeKey();
     CompositeKey(const CompositeKey& key);
-    ~CompositeKey();
+    ~CompositeKey() override;
     void clear();
     bool isEmpty() const;
-    CompositeKey* clone() const;
+    CompositeKey* clone() const override;
     CompositeKey& operator=(const CompositeKey& key);
 
-    QByteArray rawKey() const;
+    QByteArray rawKey() const override;
+    QByteArray rawKey(const QByteArray* transformSeed) const;
     bool transform(const Kdf& kdf, QByteArray& result) const Q_REQUIRED_RESULT;
     bool challenge(const QByteArray& seed, QByteArray &result) const;