diff --git a/src/core/CustomData.cpp b/src/core/CustomData.cpp index a9c5bcd3b..e00576632 100644 --- a/src/core/CustomData.cpp +++ b/src/core/CustomData.cpp @@ -25,6 +25,8 @@ const QString CustomData::Created = QStringLiteral("_CREATED"); const QString CustomData::BrowserKeyPrefix = QStringLiteral("KPXC_BROWSER_"); const QString CustomData::BrowserLegacyKeyPrefix = QStringLiteral("Public Key: "); const QString CustomData::ExcludeFromReportsLegacy = QStringLiteral("KnownBad"); +const QString CustomData::FdoSecretsExposedGroup = QStringLiteral("FDO_SECRETS_EXPOSED_GROUP"); +const QString CustomData::RandomSlug = QStringLiteral("KPXC_RANDOM_SLUG"); // Fallback item for return by reference static const CustomData::CustomDataItem NULL_ITEM{}; @@ -191,6 +193,11 @@ bool CustomData::isProtected(const QString& key) const return key.startsWith(CustomData::BrowserKeyPrefix) || key.startsWith(CustomData::Created); } +bool CustomData::isAutoGenerated(const QString& key) const +{ + return key == LastModified || key == RandomSlug; +} + bool CustomData::operator==(const CustomData& other) const { return (m_data == other.m_data); diff --git a/src/core/CustomData.h b/src/core/CustomData.h index f67fc61db..a8ad04487 100644 --- a/src/core/CustomData.h +++ b/src/core/CustomData.h @@ -51,6 +51,7 @@ public: QDateTime lastModified() const; QDateTime lastModified(const QString& key) const; bool isProtected(const QString& key) const; + bool isAutoGenerated(const QString& key) const; void set(const QString& key, CustomDataItem item); void set(const QString& key, const QString& value, const QDateTime& lastModified = {}); void remove(const QString& key); @@ -68,6 +69,8 @@ public: static const QString Created; static const QString BrowserKeyPrefix; static const QString BrowserLegacyKeyPrefix; + static const QString FdoSecretsExposedGroup; + static const QString RandomSlug; // Pre-KDBX 4.1 static const QString ExcludeFromReportsLegacy; diff --git a/src/core/Database.cpp b/src/core/Database.cpp index a3d569c91..14ddd87d9 100644 --- a/src/core/Database.cpp +++ b/src/core/Database.cpp @@ -276,7 +276,7 @@ bool Database::saveAs(const QString& filePath, SaveAction action, const QString& // Add random data to prevent side-channel data deduplication attacks int length = Random::instance()->randomUIntRange(64, 512); - m_metadata->customData()->set("KPXC_RANDOM_SLUG", Random::instance()->randomArray(length).toHex()); + m_metadata->customData()->set(CustomData::RandomSlug, Random::instance()->randomArray(length).toHex()); // Prevent destructive operations while saving QMutexLocker locker(&m_saveMutex); diff --git a/src/core/Merger.cpp b/src/core/Merger.cpp index 5e14e8452..ef9042242 100644 --- a/src/core/Merger.cpp +++ b/src/core/Merger.cpp @@ -57,6 +57,11 @@ void Merger::resetForcedMergeMode() m_mode = Group::Default; } +void Merger::setSkipDatabaseCustomData(bool state) +{ + m_skipCustomData = state; +} + QStringList Merger::merge() { // Order of merge steps is important - it is possible that we @@ -514,6 +519,11 @@ Merger::ChangeList Merger::mergeMetadata(const MergeContext& context) } } + // Some merges shouldn't modify the database custom data + if (m_skipCustomData) { + return changes; + } + // Merge Custom Data if source is newer const auto targetCustomDataModificationTime = targetMetadata->customData()->lastModified(); const auto sourceCustomDataModificationTime = sourceMetadata->customData()->lastModified(); @@ -535,8 +545,8 @@ Merger::ChangeList Merger::mergeMetadata(const MergeContext& context) // Transfer new/existing keys for (const auto& key : sourceCustomDataKeys) { - // Don't merge this meta field, it is updated automatically. - if (key == CustomData::LastModified) { + // Don't merge auto-generated keys + if (sourceMetadata->customData()->isAutoGenerated(key)) { continue; } diff --git a/src/core/Merger.h b/src/core/Merger.h index e98f7a062..75c8da990 100644 --- a/src/core/Merger.h +++ b/src/core/Merger.h @@ -31,6 +31,7 @@ public: Merger(const Group* sourceGroup, Group* targetGroup); void setForcedMergeMode(Group::MergeMode mode); void resetForcedMergeMode(); + void setSkipDatabaseCustomData(bool state); QStringList merge(); private: @@ -66,6 +67,7 @@ private: private: MergeContext m_context; Group::MergeMode m_mode; + bool m_skipCustomData = false; }; #endif // KEEPASSXC_MERGER_H diff --git a/src/gui/DatabaseTabWidget.cpp b/src/gui/DatabaseTabWidget.cpp index 0cff4a6b8..1333b5fef 100644 --- a/src/gui/DatabaseTabWidget.cpp +++ b/src/gui/DatabaseTabWidget.cpp @@ -272,6 +272,7 @@ DatabaseWidget* DatabaseTabWidget::importFile() if (newDb) { // Merge the imported db into the new one Merger merger(db.data(), newDb.data()); + merger.setSkipDatabaseCustomData(true); merger.merge(); // Show the new database auto dbWidget = new DatabaseWidget(newDb, this); diff --git a/src/keeshare/ShareImport.cpp b/src/keeshare/ShareImport.cpp index eb93912e7..da978423f 100644 --- a/src/keeshare/ShareImport.cpp +++ b/src/keeshare/ShareImport.cpp @@ -80,10 +80,12 @@ ShareObserver::Result ShareImport::containerInto(const QString& resolvedPath, auto key = QSharedPointer::create(); key->addKey(QSharedPointer::create(reference.password)); auto sourceDb = QSharedPointer::create(); + sourceDb->setEmitModified(false); if (!reader.readDatabase(&buffer, key, sourceDb.data())) { qCritical("Error while parsing the database: %s", qPrintable(reader.errorString())); return {reference.path, ShareObserver::Result::Error, reader.errorString()}; } + sourceDb->setEmitModified(true); qDebug("Synchronize %s %s with %s", qPrintable(reference.path), @@ -92,6 +94,7 @@ ShareObserver::Result ShareImport::containerInto(const QString& resolvedPath, Merger merger(sourceDb->rootGroup(), targetGroup); merger.setForcedMergeMode(Group::Synchronize); + merger.setSkipDatabaseCustomData(true); auto changelist = merger.merge(); if (!changelist.isEmpty()) { return {reference.path, ShareObserver::Result::Success, ShareImport::tr("Successful import")}; diff --git a/tests/TestMerge.cpp b/tests/TestMerge.cpp index d2fd5a000..3dde3a063 100644 --- a/tests/TestMerge.cpp +++ b/tests/TestMerge.cpp @@ -87,18 +87,54 @@ void TestMerge::testMergeNoChanges() m_clock->advanceSecond(1); Merger merger1(dbSource.data(), dbDestination.data()); - merger1.merge(); + auto changes = merger1.merge(); + QVERIFY(changes.isEmpty()); QCOMPARE(dbDestination->rootGroup()->entriesRecursive().size(), 2); QCOMPARE(dbSource->rootGroup()->entriesRecursive().size(), 2); m_clock->advanceSecond(1); Merger merger2(dbSource.data(), dbDestination.data()); - merger2.merge(); + changes = merger2.merge(); + + QVERIFY(changes.isEmpty()); + QCOMPARE(dbDestination->rootGroup()->entriesRecursive().size(), 2); + QCOMPARE(dbSource->rootGroup()->entriesRecursive().size(), 2); +} + +/** + * Merging without database custom data (used by imports and KeeShare) + */ +void TestMerge::testMergeCustomData() +{ + QScopedPointer dbDestination(createTestDatabase()); + QScopedPointer dbSource( + createTestDatabaseStructureClone(dbDestination.data(), Entry::CloneNoFlags, Group::CloneIncludeEntries)); QCOMPARE(dbDestination->rootGroup()->entriesRecursive().size(), 2); QCOMPARE(dbSource->rootGroup()->entriesRecursive().size(), 2); + + dbDestination->metadata()->customData()->set("TEST_CUSTOM_DATA", "OLD TESTING"); + + m_clock->advanceSecond(1); + + dbSource->metadata()->customData()->set("TEST_CUSTOM_DATA", "TESTING"); + + // First check that the custom data is not merged when skipped + Merger merger1(dbSource.data(), dbDestination.data()); + merger1.setSkipDatabaseCustomData(true); + auto changes = merger1.merge(); + + QVERIFY(changes.isEmpty()); + QCOMPARE(dbDestination->metadata()->customData()->value("TEST_CUSTOM_DATA"), QString("OLD TESTING")); + + // Second check that the custom data is merged otherwise + Merger merger2(dbSource.data(), dbDestination.data()); + changes = merger2.merge(); + + QCOMPARE(changes.size(), 1); + QCOMPARE(dbDestination->metadata()->customData()->value("TEST_CUSTOM_DATA"), QString("TESTING")); } /** diff --git a/tests/TestMerge.h b/tests/TestMerge.h index 2f4e4a224..b9ea54f72 100644 --- a/tests/TestMerge.h +++ b/tests/TestMerge.h @@ -30,6 +30,7 @@ private slots: void cleanup(); void testMergeIntoNew(); void testMergeNoChanges(); + void testMergeCustomData(); void testResolveConflictNewer(); void testResolveConflictExisting(); void testResolveGroupConflictOlder();