Correctly set KDBX envelope version

Shows a warning when trying to open with a newer minor version than what is currently supported.

We always try to save with the lowest KDBX version possible for maximum compatibility.

src/core/Database.cpp

@@ -116,14 +116,15 @@ bool Database::open(QSharedPointer<const CompositeKey> key, QString* error, bool
  * @param error error message in case of failure
  * @return true on success
  */
-bool Database::open(const QString& filePath, QSharedPointer<const CompositeKey> key, QString* error, bool readOnly)
+KeePass2Reader::Status
+Database::open(const QString& filePath, QSharedPointer<const CompositeKey> key, QString* error, bool readOnly)
 {
     QFile dbFile(filePath);
     if (!dbFile.exists()) {
         if (error) {
             *error = tr("File %1 does not exist.").arg(filePath);
         }
-        return false;
+        return KeePass2Reader::Status::Error;
     }
 
     // Don't autodetect read-only mode, as it triggers an upstream bug.
@@ -137,17 +138,18 @@ bool Database::open(const QString& filePath, QSharedPointer<const CompositeKey>
         if (error) {
             *error = tr("Unable to open file %1.").arg(filePath);
         }
-        return false;
+        return KeePass2Reader::Status::Error;
     }
 
     setEmitModified(false);
 
     KeePass2Reader reader;
-    if (!reader.readDatabase(&dbFile, std::move(key), this)) {
+    KeePass2Reader::Status status = reader.readDatabase(&dbFile, std::move(key), this);
+    if (status == KeePass2Reader::Status::Error) {
         if (error) {
             *error = tr("Error while reading the database: %1").arg(reader.errorString());
         }
-        return false;
+        return status;
     }
 
     setReadOnly(readOnly);
@@ -160,7 +162,17 @@ bool Database::open(const QString& filePath, QSharedPointer<const CompositeKey>
     m_fileWatcher->start(canonicalFilePath(), 30, 1);
     setEmitModified(true);
 
-    return true;
+    return status;
+}
+
+quint32 Database::formatVersion() const
+{
+    return m_data.formatVersion;
+}
+
+void Database::setFormatVersion(quint32 version)
+{
+    m_data.formatVersion = version;
 }
 
 bool Database::isSaving()
@@ -935,6 +947,7 @@ void Database::setKdf(QSharedPointer<Kdf> kdf)
 {
     Q_ASSERT(!m_data.isReadOnly);
     m_data.kdf = std::move(kdf);
+    setFormatVersion(KeePass2Writer::needsKdbxVersion(this, true, false));
 }
 
 bool Database::changeKdf(const QSharedPointer<Kdf>& kdf)

src/core/Database.h

@@ -29,6 +29,7 @@
 #include "core/ModifiableObject.h"
 #include "crypto/kdf/AesKdf.h"
 #include "format/KeePass2.h"
+#include "format/KeePass2Reader.h"
 #include "keys/CompositeKey.h"
 #include "keys/PasswordKey.h"
 
@@ -75,10 +76,10 @@ class Database : public ModifiableObject
     ~Database() override;
 
     bool open(QSharedPointer<const CompositeKey> key, QString* error = nullptr, bool readOnly = false);
-    bool open(const QString& filePath,
-              QSharedPointer<const CompositeKey> key,
-              QString* error = nullptr,
-              bool readOnly = false);
+    KeePass2Reader::Status open(const QString& filePath,
+                                QSharedPointer<const CompositeKey> key,
+                                QString* error = nullptr,
+                                bool readOnly = false);
     bool save(SaveAction action = Atomic, const QString& backupFilePath = QString(), QString* error = nullptr);
     bool saveAs(const QString& filePath,
                 SaveAction action = Atomic,
@@ -87,6 +88,9 @@ class Database : public ModifiableObject
     bool extract(QByteArray&, QString* error = nullptr);
     bool import(const QString& xmlExportPath, QString* error = nullptr);
 
+    quint32 formatVersion() const;
+    void setFormatVersion(quint32 version);
+
     void releaseData();
 
     bool isInitialized() const;
@@ -166,6 +170,7 @@ public slots:
 private:
     struct DatabaseData
     {
+        quint32 formatVersion = 0;
         QString filePath;
         bool isReadOnly = false;
         QUuid cipher = KeePass2::CIPHER_AES256;

src/format/Kdbx3Reader.cpp

@@ -34,7 +34,7 @@ bool Kdbx3Reader::readDatabaseImpl(QIODevice* device,
                                    QSharedPointer<const CompositeKey> key,
                                    Database* db)
 {
-    Q_ASSERT(m_kdbxVersion <= KeePass2::FILE_VERSION_3_1);
+    Q_ASSERT((db->formatVersion() & KeePass2::FILE_VERSION_CRITICAL_MASK) <= KeePass2::FILE_VERSION_3);
 
     if (hasError()) {
         return false;
@@ -120,7 +120,7 @@ bool Kdbx3Reader::readDatabaseImpl(QIODevice* device,
         return false;
     }
 
-    Q_ASSERT(!xmlReader.headerHash().isEmpty() || m_kdbxVersion < KeePass2::FILE_VERSION_3_1);
+    Q_ASSERT(!xmlReader.headerHash().isEmpty() || db->formatVersion() < KeePass2::FILE_VERSION_3_1);
 
     if (!xmlReader.headerHash().isEmpty()) {
         QByteArray headerHash = CryptoHash::hash(headerData, CryptoHash::Sha256);

src/format/Kdbx3Writer.cpp

@@ -68,7 +68,7 @@ bool Kdbx3Writer::writeDatabase(QIODevice* device, Database* db)
     QBuffer header;
     header.open(QIODevice::WriteOnly);
 
-    writeMagicNumbers(&header, KeePass2::SIGNATURE_1, KeePass2::SIGNATURE_2, formatVersion());
+    writeMagicNumbers(&header, KeePass2::SIGNATURE_1, KeePass2::SIGNATURE_2, db->formatVersion());
 
     CHECK_RETURN_FALSE(writeHeaderField<quint16>(&header, KeePass2::HeaderFieldID::CipherID, db->cipher().toRfc4122()));
     CHECK_RETURN_FALSE(
@@ -137,7 +137,7 @@ bool Kdbx3Writer::writeDatabase(QIODevice* device, Database* db)
         return false;
     }
 
-    KdbxXmlWriter xmlWriter(formatVersion());
+    KdbxXmlWriter xmlWriter(db->formatVersion());
     xmlWriter.writeDatabase(outputDevice, db, &randomStream, headerHash);
 
     // Explicitly close/reset streams so they are flushed and we can detect
@@ -161,8 +161,3 @@ bool Kdbx3Writer::writeDatabase(QIODevice* device, Database* db)
 
     return true;
 }
-
-quint32 Kdbx3Writer::formatVersion()
-{
-    return KeePass2::FILE_VERSION_3_1;
-}

src/format/Kdbx3Writer.h

@@ -29,7 +29,6 @@ class Kdbx3Writer : public KdbxWriter
 
 public:
     bool writeDatabase(QIODevice* device, Database* db) override;
-    quint32 formatVersion() override;
 };
 
 #endif // KEEPASSX_KDBX3WRITER_H

src/format/Kdbx4Reader.cpp

@@ -36,7 +36,7 @@ bool Kdbx4Reader::readDatabaseImpl(QIODevice* device,
                                    QSharedPointer<const CompositeKey> key,
                                    Database* db)
 {
-    Q_ASSERT(m_kdbxVersion == KeePass2::FILE_VERSION_4);
+    Q_ASSERT((db->formatVersion() & KeePass2::FILE_VERSION_CRITICAL_MASK) == KeePass2::FILE_VERSION_4);
 
     m_binaryPool.clear();
 

src/format/Kdbx4Writer.cpp

@@ -66,7 +66,7 @@ bool Kdbx4Writer::writeDatabase(QIODevice* device, Database* db)
         QBuffer header;
         header.open(QIODevice::WriteOnly);
 
-        writeMagicNumbers(&header, KeePass2::SIGNATURE_1, KeePass2::SIGNATURE_2, formatVersion());
+        writeMagicNumbers(&header, KeePass2::SIGNATURE_1, KeePass2::SIGNATURE_2, db->formatVersion());
 
         CHECK_RETURN_FALSE(
             writeHeaderField<quint32>(&header, KeePass2::HeaderFieldID::CipherID, db->cipher().toRfc4122()));
@@ -166,7 +166,7 @@ bool Kdbx4Writer::writeDatabase(QIODevice* device, Database* db)
         return false;
     }
 
-    KdbxXmlWriter xmlWriter(formatVersion());
+    KdbxXmlWriter xmlWriter(db->formatVersion());
     xmlWriter.writeDatabase(outputDevice, db, &randomStream, headerHash);
 
     // Explicitly close/reset streams so they are flushed and we can detect
@@ -306,8 +306,3 @@ bool Kdbx4Writer::serializeVariantMap(const QVariantMap& map, QByteArray& output
     CHECK_RETURN_FALSE(buf.write(endBytes) == 1);
     return true;
 }
-
-quint32 Kdbx4Writer::formatVersion()
-{
-    return KeePass2::FILE_VERSION_4;
-}

src/format/Kdbx4Writer.h

@@ -29,7 +29,6 @@ class Kdbx4Writer : public KdbxWriter
 
 public:
     bool writeDatabase(QIODevice* device, Database* db) override;
-    quint32 formatVersion() override;
 
 private:
     bool writeInnerHeaderField(QIODevice* device, KeePass2::InnerHeaderFieldID fieldId, const QByteArray& data);

src/format/KdbxReader.cpp

@@ -74,14 +74,12 @@ bool KdbxReader::readDatabase(QIODevice* device, QSharedPointer<const CompositeK
     headerStream.open(QIODevice::ReadOnly);
 
     // read KDBX magic numbers
-    quint32 sig1, sig2;
-    if (!readMagicNumbers(&headerStream, sig1, sig2, m_kdbxVersion)) {
+    quint32 sig1, sig2, version;
+    if (!readMagicNumbers(&headerStream, sig1, sig2, version)) {
         return false;
     }
     m_kdbxSignature = qMakePair(sig1, sig2);
-
-    // mask out minor version
-    m_kdbxVersion &= KeePass2::FILE_VERSION_CRITICAL_MASK;
+    m_db->setFormatVersion(version);
 
     // read header fields
     while (readHeaderField(headerStream, m_db) && !hasError()) {

src/format/KdbxReader.h

@@ -83,8 +83,6 @@ class KdbxReader
 
     void raiseError(const QString& errorMessage);
 
-    quint32 m_kdbxVersion = 0;
-
     QByteArray m_masterSeed;
     QByteArray m_encryptionIV;
     QByteArray m_streamStartBytes;

src/format/KdbxWriter.cpp

@@ -71,7 +71,7 @@ void KdbxWriter::extractDatabase(QByteArray& xmlOutput, Database* db)
     QBuffer buffer;
     buffer.setBuffer(&xmlOutput);
     buffer.open(QIODevice::WriteOnly);
-    KdbxXmlWriter writer(formatVersion());
+    KdbxXmlWriter writer(db->formatVersion());
     writer.disableInnerStreamProtection(true);
     writer.writeDatabase(&buffer, db);
 }

src/format/KdbxWriter.h

@@ -52,11 +52,6 @@ class KdbxWriter
      */
     virtual bool writeDatabase(QIODevice* device, Database* db) = 0;
 
-    /**
-     * Get the database format version for the writer.
-     */
-    virtual quint32 formatVersion() = 0;
-
     void extractDatabase(QByteArray& xmlOutput, Database* db);
 
     bool hasError() const;

src/format/KdbxXmlWriter.cpp

@@ -166,7 +166,7 @@ void KdbxXmlWriter::writeIcon(const QUuid& uuid, const Metadata::CustomIconData&
     m_xml.writeStartElement("Icon");
 
     writeUuid("UUID", uuid);
-    if (m_kdbxVersion >= KeePass2::FILE_VERSION_4) {
+    if (m_kdbxVersion >= KeePass2::FILE_VERSION_4_1) {
         if (!iconData.name.isEmpty()) {
             writeString("Name", iconData.name);
         }
@@ -243,7 +243,7 @@ void KdbxXmlWriter::writeCustomDataItem(const QString& key,
 
     writeString("Key", key);
     writeString("Value", item.value);
-    if (writeLastModified && m_kdbxVersion >= KeePass2::FILE_VERSION_4 && item.lastModified.isValid()) {
+    if (writeLastModified && m_kdbxVersion >= KeePass2::FILE_VERSION_4_1 && item.lastModified.isValid()) {
         writeDateTime("LastModificationTime", item.lastModified);
     }
 
@@ -291,9 +291,9 @@ void KdbxXmlWriter::writeGroup(const Group* group)
 
     if (m_kdbxVersion >= KeePass2::FILE_VERSION_4) {
         writeCustomData(group->customData());
-        if (!group->previousParentGroupUuid().isNull()) {
-            writeUuid("PreviousParentGroup", group->previousParentGroupUuid());
-        }
+    }
+    if (m_kdbxVersion >= KeePass2::FILE_VERSION_4_1 && !group->previousParentGroupUuid().isNull()) {
+        writeUuid("PreviousParentGroup", group->previousParentGroupUuid());
     }
 
     const QList<Entry*>& entryList = group->entries();
@@ -363,7 +363,7 @@ void KdbxXmlWriter::writeEntry(const Entry* entry)
     writeString("Tags", entry->tags());
     writeTimes(entry->timeInfo());
 
-    if (m_kdbxVersion >= KeePass2::FILE_VERSION_4) {
+    if (m_kdbxVersion >= KeePass2::FILE_VERSION_4_1) {
         if (entry->excludeFromReports()) {
             writeBool("QualityCheck", false);
         }

src/format/KeePass2.h

@@ -31,16 +31,18 @@ namespace KeePass2
     constexpr quint32 SIGNATURE_2 = 0xB54BFB67;
 
     constexpr quint32 FILE_VERSION_CRITICAL_MASK = 0xFFFF0000;
+    constexpr quint32 FILE_VERSION_4_1 = 0x00040001;
     constexpr quint32 FILE_VERSION_4 = 0x00040000;
     constexpr quint32 FILE_VERSION_3_1 = 0x00030001;
     constexpr quint32 FILE_VERSION_3 = 0x00030000;
     constexpr quint32 FILE_VERSION_2 = 0x00020000;
     constexpr quint32 FILE_VERSION_MIN = FILE_VERSION_2;
+    constexpr quint32 FILE_VERSION_MAX = FILE_VERSION_4_1;
 
     constexpr quint16 VARIANTMAP_VERSION = 0x0100;
     constexpr quint16 VARIANTMAP_CRITICAL_MASK = 0xFF00;
 
-    const QSysInfo::Endian BYTEORDER = QSysInfo::LittleEndian;
+    constexpr QSysInfo::Endian BYTEORDER = QSysInfo::LittleEndian;
 
     extern const QUuid CIPHER_AES128;
     extern const QUuid CIPHER_AES256;