Skip to content

Commit

Permalink
Correctly set KDBX envelope version
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
phoerious committed Nov 20, 2021
1 parent f914b6b commit c2d4dba
Show file tree
Hide file tree
Showing 30 changed files with 209 additions and 162 deletions.
Binary file modified share/demo.kdbx
Binary file not shown.
58 changes: 42 additions & 16 deletions share/translations/keepassxc_en.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1535,6 +1535,28 @@ If you do not have a key file, please leave the field empty.</source>
<source>Please present or touch your YubiKey to continue…</source>
<translation type="unfinished"></translation>
</message>
<message>
<source>Database Version Mismatch</source>
<translation type="unfinished"></translation>
</message>
<message>
<source>The database you are trying to open was most likely
created by a newer version of KeePassXC.

You can try to open it anyway, but it may be incomplete
and saving any changes may incur data loss.

We recommend you update your KeePassXC installation.</source>
<translation type="unfinished"></translation>
</message>
<message>
<source>Open database anyway</source>
<translation type="unfinished"></translation>
</message>
<message>
<source>Database unlock canceled.</source>
<translation type="unfinished"></translation>
</message>
</context>
<context>
<name>DatabaseSettingWidgetMetaData</name>
Expand Down Expand Up @@ -1808,18 +1830,6 @@ Are you sure you want to continue without a password?</translation>
<source>Database format:</source>
<translation>Database format:</translation>
</message>
<message>
<source>This is only important if you need to use your database with other programs.</source>
<translation>This is only important if you need to use your database with other programs.</translation>
</message>
<message>
<source>KDBX 4.0 (recommended)</source>
<translation>KDBX 4.0 (recommended)</translation>
</message>
<message>
<source>KDBX 3.1</source>
<translation>KDBX 3.1</translation>
</message>
<message>
<source>unchanged</source>
<comment>Database decryption time is unchanged</comment>
Expand Down Expand Up @@ -1919,6 +1929,22 @@ If you keep this number, your database may take hours, days, or even longer to o
If you keep this number, your database will not be protected from brute force attacks.</source>
<translation type="unfinished"></translation>
</message>
<message>
<source>Format cannot be changed: Your database uses KDBX 4 features</source>
<translation type="unfinished"></translation>
</message>
<message>
<source>Unless you need to open your database with other programs, always use the latest format.</source>
<translation type="unfinished"></translation>
</message>
<message>
<source>KDBX 4 (recommended)</source>
<translation type="unfinished">KDBX 4.0 (recommended) {4 ?}</translation>
</message>
<message>
<source>KDBX 3</source>
<translation type="unfinished">KDBX 3</translation>
</message>
</context>
<context>
<name>DatabaseSettingsWidgetFdoSecrets</name>
Expand Down Expand Up @@ -6523,10 +6549,6 @@ Available commands:
<source>AES-KDF (KDBX 4)</source>
<translation>AES-KDF (KDBX 4)</translation>
</message>
<message>
<source>AES-KDF (KDBX 3.1)</source>
<translation>AES-KDF (KDBX 3.1)</translation>
</message>
<message>
<source>Invalid Settings</source>
<comment>TOTP</comment>
Expand Down Expand Up @@ -7498,6 +7520,10 @@ Please consider generating a new key file.</source>
<source>Attachments:</source>
<translation type="unfinished"></translation>
</message>
<message>
<source>AES-KDF (KDBX 3)</source>
<translation type="unfinished">AES-KDF (KDBX 3.1) {3)?}</translation>
</message>
</context>
<context>
<name>QtIOCompressor</name>
Expand Down
22 changes: 22 additions & 0 deletions src/core/Database.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,27 @@ bool Database::open(const QString& filePath, QSharedPointer<const CompositeKey>
return true;
}

/**
* KDBX format version.
*/
quint32 Database::formatVersion() const
{
return m_data.formatVersion;
}

void Database::setFormatVersion(quint32 version)
{
m_data.formatVersion = version;
}

/**
* Whether the KDBX minor version is greater than the newest supported.
*/
bool Database::hasMinorVersionMismatch() const
{
return m_data.formatVersion > KeePass2::FILE_VERSION_MAX;
}

bool Database::isSaving()
{
bool locked = m_saveMutex.tryLock();
Expand Down Expand Up @@ -935,6 +956,7 @@ void Database::setKdf(QSharedPointer<Kdf> kdf)
{
Q_ASSERT(!m_data.isReadOnly);
m_data.kdf = std::move(kdf);
setFormatVersion(KeePass2Writer::kdbxVersionRequired(this, true, m_data.kdf.isNull()));
}

bool Database::changeKdf(const QSharedPointer<Kdf>& kdf)
Expand Down
5 changes: 5 additions & 0 deletions src/core/Database.h
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,10 @@ 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);
bool hasMinorVersionMismatch() const;

void releaseData();

bool isInitialized() const;
Expand Down Expand Up @@ -166,6 +170,7 @@ public slots:
private:
struct DatabaseData
{
quint32 formatVersion = 0;
QString filePath;
bool isReadOnly = false;
QUuid cipher = KeePass2::CIPHER_AES256;
Expand Down
4 changes: 2 additions & 2 deletions src/format/Kdbx3Reader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down
9 changes: 2 additions & 7 deletions src/format/Kdbx3Writer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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
Expand All @@ -161,8 +161,3 @@ bool Kdbx3Writer::writeDatabase(QIODevice* device, Database* db)

return true;
}

quint32 Kdbx3Writer::formatVersion()
{
return KeePass2::FILE_VERSION_3_1;
}
1 change: 0 additions & 1 deletion src/format/Kdbx3Writer.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ class Kdbx3Writer : public KdbxWriter

public:
bool writeDatabase(QIODevice* device, Database* db) override;
quint32 formatVersion() override;
};

#endif // KEEPASSX_KDBX3WRITER_H
2 changes: 1 addition & 1 deletion src/format/Kdbx4Reader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down
9 changes: 2 additions & 7 deletions src/format/Kdbx4Writer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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()));
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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;
}
1 change: 0 additions & 1 deletion src/format/Kdbx4Writer.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
8 changes: 3 additions & 5 deletions src/format/KdbxReader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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()) {
Expand Down
2 changes: 0 additions & 2 deletions src/format/KdbxReader.h
Original file line number Diff line number Diff line change
Expand Up @@ -83,8 +83,6 @@ class KdbxReader

void raiseError(const QString& errorMessage);

quint32 m_kdbxVersion = 0;

QByteArray m_masterSeed;
QByteArray m_encryptionIV;
QByteArray m_streamStartBytes;
Expand Down
2 changes: 1 addition & 1 deletion src/format/KdbxWriter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
5 changes: 0 additions & 5 deletions src/format/KdbxWriter.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
12 changes: 6 additions & 6 deletions src/format/KdbxXmlWriter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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);
}
Expand Down
2 changes: 1 addition & 1 deletion src/format/KeePass2.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ const QList<QPair<QUuid, QString>> KeePass2::KDFS{
qMakePair(KeePass2::KDF_ARGON2D, QObject::tr("Argon2d (KDBX 4 – recommended)")),
qMakePair(KeePass2::KDF_ARGON2ID, QObject::tr("Argon2id (KDBX 4)")),
qMakePair(KeePass2::KDF_AES_KDBX4, QObject::tr("AES-KDF (KDBX 4)")),
qMakePair(KeePass2::KDF_AES_KDBX3, QObject::tr("AES-KDF (KDBX 3.1)"))};
qMakePair(KeePass2::KDF_AES_KDBX3, QObject::tr("AES-KDF (KDBX 3)"))};

QByteArray KeePass2::hmacKey(const QByteArray& masterSeed, const QByteArray& transformedMasterKey)
{
Expand Down
4 changes: 3 additions & 1 deletion src/format/KeePass2.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
7 changes: 2 additions & 5 deletions src/format/KeePass2Reader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -81,11 +81,8 @@ bool KeePass2Reader::readDatabase(QIODevice* device, QSharedPointer<const Compos
return false;
}

// mask out minor version
m_version &= KeePass2::FILE_VERSION_CRITICAL_MASK;

quint32 maxVersion = KeePass2::FILE_VERSION_4 & KeePass2::FILE_VERSION_CRITICAL_MASK;
if (m_version < KeePass2::FILE_VERSION_MIN || m_version > maxVersion) {
if (m_version < KeePass2::FILE_VERSION_MIN
|| (m_version & KeePass2::FILE_VERSION_CRITICAL_MASK) > KeePass2::FILE_VERSION_MAX) {
raiseError(tr("Unsupported KeePass 2 database version."));
return false;
}
Expand Down
Loading

0 comments on commit c2d4dba

Please sign in to comment.