Merge branch 'release/2.3.1' into develop

CMakeLists.txt

@@ -80,7 +80,9 @@ set_property(CACHE KEEPASSXC_BUILD_TYPE PROPERTY STRINGS Snapshot Release PreRel
 execute_process(COMMAND git tag --points-at HEAD
         WORKING_DIRECTORY ${CMAKE_SOURCE_DIR}
         OUTPUT_VARIABLE GIT_TAG)
-if(NOT GIT_TAG AND EXISTS ${CMAKE_SOURCE_DIR}/.version)
+if(GIT_TAG)
+  set(OVERRIDE_VERSION ${GIT_TAG})
+elseif(EXISTS ${CMAKE_SOURCE_DIR}/.version)
   file(READ ${CMAKE_SOURCE_DIR}/.version OVERRIDE_VERSION)
 endif()
 

src/format/KdbxXmlWriter.cpp

@@ -129,9 +129,7 @@ void KdbxXmlWriter::writeMetadata()
     if (m_kdbxVersion < KeePass2::FILE_VERSION_4) {
         writeBinaries();
     }
-    if (m_kdbxVersion >= KeePass2::FILE_VERSION_4) {
     writeCustomData(m_meta->customData());
-    }
 
     m_xml.writeEndElement();
 }

src/format/KeePass2Writer.cpp

@@ -43,6 +43,36 @@ bool KeePass2Writer::writeDatabase(const QString& filename, Database* db)
     return writeDatabase(&file, db);
 }
 
+/**
+ * @return true if the database should upgrade to KDBX4.
+ */
+bool KeePass2Writer::implicitUpgradeNeeded(Database const* db) const
+{
+    if (!db->publicCustomData().isEmpty()) {
+        return true;
+    }
+
+    for (const auto& group: db->rootGroup()->groupsRecursive(true)) {
+        if (group->customData() && !group->customData()->isEmpty()) {
+            return true;
+        }
+
+        for (const auto& entry: group->entries()) {
+            if (entry->customData() && !entry->customData()->isEmpty()) {
+                return true;
+            }
+
+            for (const auto& historyItem: entry->historyItems()) {
+                if (historyItem->customData() && !historyItem->customData()->isEmpty()) {
+                    return true;
+                }
+            }
+        }
+    }
+
+    return false;
+}
+
 /**
  * Write a database to a device in KDBX format.
  *
@@ -55,19 +85,15 @@ bool KeePass2Writer::writeDatabase(QIODevice* device, Database* db) {
     m_error = false;
     m_errorStr.clear();
 
-    // determine KDBX3 vs KDBX4
-    bool hasCustomData = !db->publicCustomData().isEmpty() || (db->metadata()->customData() && !db->metadata()->customData()->isEmpty());
-    if (!hasCustomData) {
-        for (const auto& entry: db->rootGroup()->entriesRecursive(true)) {
-            if ((entry->customData() && !entry->customData()->isEmpty()) ||
-                (entry->group() && entry->group()->customData() && !entry->group()->customData()->isEmpty())) {
-                hasCustomData = true;
-                break;
-            }
-        }
+    bool upgradeNeeded = implicitUpgradeNeeded(db);
+    if (upgradeNeeded) {
+        // We MUST re-transform the key, because challenge-response hashing has changed in KDBX 4.
+        // If we forget to re-transform, the database will be saved WITHOUT a challenge-response key component!
+        db->changeKdf(KeePass2::uuidToKdf(KeePass2::KDF_AES_KDBX4));
     }
 
-    if (db->kdf()->uuid() == KeePass2::KDF_AES_KDBX3 && !hasCustomData) {
+    if (db->kdf()->uuid() == KeePass2::KDF_AES_KDBX3) {
+        Q_ASSERT(!upgradeNeeded);
         m_version = KeePass2::FILE_VERSION_3_1;
         m_writer.reset(new Kdbx3Writer());
     } else {

src/format/KeePass2Writer.h

@@ -42,6 +42,7 @@ public:
 
 private:
     void raiseError(const QString& errorMessage);
+    bool implicitUpgradeNeeded(Database const* db) const;
 
     bool m_error = false;
     QString m_errorStr = "";

tests/TestKdbx4.cpp

@@ -149,8 +149,10 @@ void TestKdbx4::testFormat400Upgrade()
     sourceDb->changeKdf(KeePass2::uuidToKdf(kdfUuid));
     sourceDb->setCipher(cipherUuid);
 
-    if (addCustomData) {
+    // CustomData in meta should not cause any version change
     sourceDb->metadata()->customData()->set("CustomPublicData", "Hey look, I turned myself into a pickle!");
+    if (addCustomData) {
+        // this, however, should
         sourceDb->rootGroup()->customData()->set("CustomGroupData", "I just killed my family! I don't care who they were!");
     }