From 57235219f26d7112cf1f4010f37856577d7e6b4d Mon Sep 17 00:00:00 2001 From: Jonathan White Date: Sun, 2 Feb 2025 09:43:02 -0500 Subject: [PATCH] Fix issues with reloading and handling of externally modified db file (#10612) Fixes #5290 Fixes #9062 Fixes #8545 * Fix data loss on failed reload - External modifications to the db file can no longer be missed. - Fixed dialogFinished signal of DatabaseOpenDialog was not emitted when dialog was closed via the 'X' (close) button - For reloading with a modified db, an additional choice has been added to allow the user to ignore the changes in the file on disk. - User is now presented with an unlock database dialog if reload fails to open the db automatically. For example when the user removed the YubiKey, failed to touch the YubiKey within the timeout period, or db pw has been changed. - Mark db as modified when db file is gone or invalid. - Prevent saving when db is being reloaded - If merge is triggered by a save action, continue on with the save action after the user makes their choice --------- Co-authored-by: vuurvlieg Co-authored-by: Jonathan White --- share/translations/keepassxc_en.ts | 66 ++++++--- src/CMakeLists.txt | 1 + src/core/Database.cpp | 105 +++++++++++++-- src/core/Database.h | 11 +- src/core/FileWatcher.cpp | 9 +- src/core/FileWatcher.h | 1 + src/gui/DatabaseOpenDialog.cpp | 26 +++- src/gui/DatabaseOpenDialog.h | 3 + src/gui/DatabaseOpenWidget.cpp | 5 + src/gui/DatabaseOpenWidget.h | 2 + src/gui/DatabaseWidget.cpp | 210 ++++++++++++++++++++++++----- src/gui/DatabaseWidget.h | 6 +- src/gui/MainWindow.cpp | 1 + src/streams/HashingStream.cpp | 101 ++++++++++++++ src/streams/HashingStream.h | 52 +++++++ tests/TestDatabase.cpp | 40 +++++- tests/TestDatabase.h | 1 + 17 files changed, 566 insertions(+), 74 deletions(-) create mode 100644 src/streams/HashingStream.cpp create mode 100644 src/streams/HashingStream.h diff --git a/share/translations/keepassxc_en.ts b/share/translations/keepassxc_en.ts index 93fccec13..c221275ed 100644 --- a/share/translations/keepassxc_en.ts +++ b/share/translations/keepassxc_en.ts @@ -1547,6 +1547,10 @@ Backup database located at %2 Recycle Bin + + Database file read error. + + DatabaseOpenDialog @@ -2571,24 +2575,6 @@ Save changes? File has changed - - The database file has changed. Do you want to load the changes? - - - - Merge Request - - - - The database file has changed and you have unsaved changes. -Do you want to merge your changes? - - - - Could not open the new database file while attempting to autoreload. -Error: %1 - - Disable safe saves? @@ -2670,6 +2656,50 @@ Disable safe saves and try again? Save Search + + The database file "%1" was modified externally + + + + Do you want to load the changes? + + + + Reload database + + + + Reloading database… + + + + Reload canceled + + + + Reload successful + + + + Reload pending user action… + + + + The database file "%1" was modified externally.<br>How would you like to proceed?<br><br>Merge all changes<br>Ignore the changes on disk until save<br>Discard unsaved changes + + + + The database file "%1" was modified externally.<br>How would you like to proceed?<br><br>Merge all changes then save<br>Overwrite the changes on disk<br>Discard unsaved changes + + + + Database file overwritten. + + + + Database file on disk cannot be unlocked with current credentials.<br>Enter new credentials and/or present hardware key to continue. + + EditEntryWidget diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index cfcb3f9de..c183180bc 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -181,6 +181,7 @@ set(keepassx_SOURCES keys/PasswordKey.cpp keys/ChallengeResponseKey.cpp streams/HashedBlockStream.cpp + streams/HashingStream.cpp streams/HmacBlockStream.cpp streams/LayeredStream.cpp streams/qtiocompressor.cpp diff --git a/src/core/Database.cpp b/src/core/Database.cpp index 0d1e2eb49..ab3d09ba2 100644 --- a/src/core/Database.cpp +++ b/src/core/Database.cpp @@ -25,6 +25,7 @@ #include "format/KdbxXmlReader.h" #include "format/KeePass2Reader.h" #include "format/KeePass2Writer.h" +#include "streams/HashingStream.h" #include #include @@ -62,8 +63,8 @@ Database::Database() updateTagList(); }); connect(this, &Database::modified, this, [this] { updateTagList(); }); - connect(this, &Database::databaseSaved, this, [this]() { updateCommonUsernames(); }); - connect(m_fileWatcher, &FileWatcher::fileChanged, this, &Database::databaseFileChanged); + connect(this, &Database::databaseSaved, this, [this] { updateCommonUsernames(); }); + connect(m_fileWatcher, &FileWatcher::fileChanged, this, [this] { emit databaseFileChanged(false); }); // static uuid map s_uuidMap.insert(m_uuid, this); @@ -149,6 +150,20 @@ bool Database::open(const QString& filePath, QSharedPointer setEmitModified(false); + // update the hash of the first block + m_fileBlockHash.clear(); + auto fileBlockData = dbFile.peek(kFileBlockToHashSizeBytes); + if (fileBlockData.size() != kFileBlockToHashSizeBytes) { + if (dbFile.size() >= kFileBlockToHashSizeBytes) { + if (error) { + *error = tr("Database file read error."); + } + return false; + } + } else { + m_fileBlockHash = QCryptographicHash::hash(fileBlockData, QCryptographicHash::Md5); + } + KeePass2Reader reader; if (!reader.readDatabase(&dbFile, std::move(key), this)) { if (error) { @@ -258,14 +273,33 @@ bool Database::saveAs(const QString& filePath, SaveAction action, const QString& return false; } - if (filePath == m_data.filePath) { - // Fail-safe check to make sure we don't overwrite underlying file changes - // that have not yet triggered a file reload/merge operation. - if (!m_fileWatcher->hasSameFileChecksum()) { - if (error) { - *error = tr("Database file has unmerged changes."); + // Make sure we don't overwrite external modifications unless explicitly allowed + if (!m_ignoreFileChangesUntilSaved && !m_fileBlockHash.isEmpty() && filePath == m_data.filePath) { + QFile dbFile(filePath); + if (dbFile.exists()) { + if (!dbFile.open(QIODevice::ReadOnly)) { + if (error) { + *error = tr("Unable to open file %1.").arg(filePath); + } + return false; + } + auto fileBlockData = dbFile.read(kFileBlockToHashSizeBytes); + if (fileBlockData.size() == kFileBlockToHashSizeBytes) { + auto hash = QCryptographicHash::hash(fileBlockData, QCryptographicHash::Md5); + if (m_fileBlockHash != hash) { + if (error) { + *error = tr("Database file has unmerged changes."); + } + // emit the databaseFileChanged(true) signal async + QMetaObject::invokeMethod(this, "databaseFileChanged", Qt::QueuedConnection, Q_ARG(bool, true)); + return false; + } + } else if (dbFile.size() >= kFileBlockToHashSizeBytes) { + if (error) { + *error = tr("Database file read error."); + } + return false; } - return false; } } @@ -300,7 +334,7 @@ bool Database::saveAs(const QString& filePath, SaveAction action, const QString& SetFileAttributes(realFilePath.toStdString().c_str(), FILE_ATTRIBUTE_HIDDEN); } #endif - + m_ignoreFileChangesUntilSaved = false; m_fileWatcher->start(realFilePath, 30, 1); } else { // Saving failed, don't rewatch file since it does not represent our database @@ -325,8 +359,12 @@ bool Database::performSave(const QString& filePath, SaveAction action, const QSt case Atomic: { QSaveFile saveFile(filePath); if (saveFile.open(QIODevice::WriteOnly)) { + HashingStream hashingStream(&saveFile, QCryptographicHash::Md5, kFileBlockToHashSizeBytes); + if (!hashingStream.open(QIODevice::WriteOnly)) { + return false; + } // write the database to the file - if (!writeDatabase(&saveFile, error)) { + if (!writeDatabase(&hashingStream, error)) { return false; } @@ -336,6 +374,9 @@ bool Database::performSave(const QString& filePath, SaveAction action, const QSt #endif if (saveFile.commit()) { + // store the new hash + m_fileBlockHash = hashingStream.hashingResult(); + // successfully saved database file return true; } @@ -349,8 +390,12 @@ bool Database::performSave(const QString& filePath, SaveAction action, const QSt case TempFile: { QTemporaryFile tempFile; if (tempFile.open()) { + HashingStream hashingStream(&tempFile, QCryptographicHash::Md5, kFileBlockToHashSizeBytes); + if (!hashingStream.open(QIODevice::WriteOnly)) { + return false; + } // write the database to the file - if (!writeDatabase(&tempFile, error)) { + if (!writeDatabase(&hashingStream, error)) { return false; } tempFile.close(); // flush to disk @@ -370,6 +415,8 @@ bool Database::performSave(const QString& filePath, SaveAction action, const QSt // Retain orginal creation time tempFile.setFileTime(createTime, QFile::FileBirthTime); #endif + // store the new hash + m_fileBlockHash = hashingStream.hashingResult(); return true; } else if (backupFilePath.isEmpty() || !restoreDatabase(filePath, backupFilePath)) { // Failed to copy new database in place, and @@ -391,10 +438,16 @@ bool Database::performSave(const QString& filePath, SaveAction action, const QSt // Open the original database file for direct-write QFile dbFile(filePath); if (dbFile.open(QIODevice::WriteOnly | QIODevice::Truncate)) { - if (!writeDatabase(&dbFile, error)) { + HashingStream hashingStream(&dbFile, QCryptographicHash::Md5, kFileBlockToHashSizeBytes); + if (!hashingStream.open(QIODevice::WriteOnly)) { + return false; + } + if (!writeDatabase(&hashingStream, error)) { return false; } dbFile.close(); + // store the new hash + m_fileBlockHash = hashingStream.hashingResult(); return true; } if (error) { @@ -512,6 +565,9 @@ void Database::releaseData() m_deletedObjects.clear(); m_commonUsernames.clear(); m_tagList.clear(); + + m_fileBlockHash.clear(); + m_ignoreFileChangesUntilSaved = false; } /** @@ -648,10 +704,33 @@ void Database::setFilePath(const QString& filePath) m_data.filePath = filePath; // Don't watch for changes until the next open or save operation m_fileWatcher->stop(); + m_ignoreFileChangesUntilSaved = false; emit filePathChanged(oldPath, filePath); } } +const QByteArray& Database::fileBlockHash() const +{ + return m_fileBlockHash; +} + +void Database::setIgnoreFileChangesUntilSaved(bool ignore) +{ + if (m_ignoreFileChangesUntilSaved != ignore) { + m_ignoreFileChangesUntilSaved = ignore; + if (ignore) { + m_fileWatcher->pause(); + } else { + m_fileWatcher->resume(); + } + } +} + +bool Database::ignoreFileChangesUntilSaved() const +{ + return m_ignoreFileChangesUntilSaved; +} + QList Database::deletedObjects() { return m_deletedObjects; diff --git a/src/core/Database.h b/src/core/Database.h index 27e187b7a..fb9f7ed79 100644 --- a/src/core/Database.h +++ b/src/core/Database.h @@ -75,6 +75,9 @@ public: ~Database() override; private: + // size of the block of file data to hash for detecting external changes + static const quint32 kFileBlockToHashSizeBytes = 1024; // 1 KiB + bool writeDatabase(QIODevice* device, QString* error = nullptr); bool backupDatabase(const QString& filePath, const QString& destinationFilePath); bool restoreDatabase(const QString& filePath, const QString& fromBackupFilePath); @@ -107,6 +110,10 @@ public: QString canonicalFilePath() const; void setFilePath(const QString& filePath); + const QByteArray& fileBlockHash() const; + void setIgnoreFileChangesUntilSaved(bool ignore); + bool ignoreFileChangesUntilSaved() const; + QString publicName(); void setPublicName(const QString& name); QString publicColor(); @@ -177,7 +184,7 @@ signals: void databaseOpened(); void databaseSaved(); void databaseDiscarded(); - void databaseFileChanged(); + void databaseFileChanged(bool triggeredBySave); void databaseNonDataChanged(); void tagListUpdated(); @@ -229,6 +236,8 @@ private: void startModifiedTimer(); void stopModifiedTimer(); + QByteArray m_fileBlockHash; + bool m_ignoreFileChangesUntilSaved; QPointer const m_metadata; DatabaseData m_data; QPointer m_rootGroup; diff --git a/src/core/FileWatcher.cpp b/src/core/FileWatcher.cpp index 1cf3be8e6..3a3c6d83b 100644 --- a/src/core/FileWatcher.cpp +++ b/src/core/FileWatcher.cpp @@ -79,17 +79,18 @@ void FileWatcher::stop() m_fileChecksum.clear(); m_fileChecksumTimer.stop(); m_fileChangeDelayTimer.stop(); + m_paused = false; } void FileWatcher::pause() { - m_ignoreFileChange = true; + m_paused = true; m_fileChangeDelayTimer.stop(); } void FileWatcher::resume() { - m_ignoreFileChange = false; + m_paused = false; // Add a short delay to start in the next event loop if (!m_fileIgnoreDelayTimer.isActive()) { m_fileIgnoreDelayTimer.start(0); @@ -98,7 +99,7 @@ void FileWatcher::resume() bool FileWatcher::shouldIgnoreChanges() { - return m_filePath.isEmpty() || m_ignoreFileChange || m_fileIgnoreDelayTimer.isActive() + return m_filePath.isEmpty() || m_ignoreFileChange || m_paused || m_fileIgnoreDelayTimer.isActive() || m_fileChangeDelayTimer.isActive(); } @@ -118,7 +119,7 @@ void FileWatcher::checkFileChanged() AsyncTask::runThenCallback([this] { return calculateChecksum(); }, this, - [this](QByteArray checksum) { + [this](const QByteArray& checksum) { if (checksum != m_fileChecksum) { m_fileChecksum = checksum; m_fileChangeDelayTimer.start(0); diff --git a/src/core/FileWatcher.h b/src/core/FileWatcher.h index 27159d17a..7a5649764 100644 --- a/src/core/FileWatcher.h +++ b/src/core/FileWatcher.h @@ -56,6 +56,7 @@ private: QTimer m_fileChecksumTimer; int m_fileChecksumSizeBytes = -1; bool m_ignoreFileChange = false; + bool m_paused = false; }; #endif // KEEPASSXC_FILEWATCHER_H diff --git a/src/gui/DatabaseOpenDialog.cpp b/src/gui/DatabaseOpenDialog.cpp index 5f3b1f33f..9fc00b92a 100644 --- a/src/gui/DatabaseOpenDialog.cpp +++ b/src/gui/DatabaseOpenDialog.cpp @@ -196,11 +196,34 @@ void DatabaseOpenDialog::clearForms() m_tabBar->blockSignals(false); } +void DatabaseOpenDialog::showMessage(const QString& text, MessageWidget::MessageType type, int autoHideTimeout) +{ + m_view->showMessage(text, type, autoHideTimeout); +} + QSharedPointer DatabaseOpenDialog::database() const { return m_db; } +void DatabaseOpenDialog::done(int result) +{ + hide(); + + emit dialogFinished(result == QDialog::Accepted, m_currentDbWidget); + clearForms(); + + QDialog::done(result); + +#if QT_VERSION < QT_VERSION_CHECK(6, 3, 0) + // CDialogs are not really closed, just hidden, pre Qt 6.3? + if (testAttribute(Qt::WA_DeleteOnClose)) { + setAttribute(Qt::WA_DeleteOnClose, false); + deleteLater(); + } +#endif +} + void DatabaseOpenDialog::complete(bool accepted) { // save DB, since DatabaseOpenWidget will reset its data after accept() is called @@ -211,7 +234,4 @@ void DatabaseOpenDialog::complete(bool accepted) } else { reject(); } - - emit dialogFinished(accepted, m_currentDbWidget); - clearForms(); } diff --git a/src/gui/DatabaseOpenDialog.h b/src/gui/DatabaseOpenDialog.h index b1a59b59a..1aa24e4d9 100644 --- a/src/gui/DatabaseOpenDialog.h +++ b/src/gui/DatabaseOpenDialog.h @@ -19,6 +19,7 @@ #define KEEPASSX_UNLOCKDATABASEDIALOG_H #include "core/Global.h" +#include "gui/MessageWidget.h" #include #include @@ -50,11 +51,13 @@ public: Intent intent() const; QSharedPointer database() const; void clearForms(); + void showMessage(const QString& text, MessageWidget::MessageType type, int autoHideTimeout); signals: void dialogFinished(bool accepted, DatabaseWidget* dbWidget); public slots: + void done(int result) override; void complete(bool accepted); void tabChanged(int index); diff --git a/src/gui/DatabaseOpenWidget.cpp b/src/gui/DatabaseOpenWidget.cpp index f1d59cc63..ccfcf9846 100644 --- a/src/gui/DatabaseOpenWidget.cpp +++ b/src/gui/DatabaseOpenWidget.cpp @@ -232,6 +232,11 @@ bool DatabaseOpenWidget::unlockingDatabase() return m_unlockingDatabase; } +void DatabaseOpenWidget::showMessage(const QString& text, MessageWidget::MessageType type, int autoHideTimeout) +{ + m_ui->messageWidget->showMessage(text, type, autoHideTimeout); +} + void DatabaseOpenWidget::load(const QString& filename) { clearForms(); diff --git a/src/gui/DatabaseOpenWidget.h b/src/gui/DatabaseOpenWidget.h index 334180eb3..8b02d8c90 100644 --- a/src/gui/DatabaseOpenWidget.h +++ b/src/gui/DatabaseOpenWidget.h @@ -25,6 +25,7 @@ #include "config-keepassx.h" #include "gui/DialogyWidget.h" +#include "gui/MessageWidget.h" #ifdef WITH_XC_YUBIKEY #include "osutils/DeviceListener.h" #endif @@ -51,6 +52,7 @@ public: void enterKey(const QString& pw, const QString& keyFile); QSharedPointer database(); bool unlockingDatabase(); + void showMessage(const QString& text, MessageWidget::MessageType type, int autoHideTimeout); // Quick Unlock helper functions bool canPerformQuickUnlock() const; diff --git a/src/gui/DatabaseWidget.cpp b/src/gui/DatabaseWidget.cpp index eabe4b771..ebe7c6db1 100644 --- a/src/gui/DatabaseWidget.cpp +++ b/src/gui/DatabaseWidget.cpp @@ -221,6 +221,7 @@ DatabaseWidget::DatabaseWidget(QSharedPointer db, QWidget* parent) connectDatabaseSignals(); m_blockAutoSave = false; + m_reloading = false; m_autosaveTimer = new QTimer(this); m_autosaveTimer->setSingleShot(true); @@ -1818,6 +1819,11 @@ bool DatabaseWidget::lock() return isLocked(); } + // ignore when reloading + if (m_reloading) { + return false; + } + // Don't try to lock the database while saving, this will cause a deadlock if (m_db->isSaving()) { QTimer::singleShot(200, this, SLOT(lock())); @@ -1861,6 +1867,18 @@ bool DatabaseWidget::lock() if (config()->get(Config::AutoSaveOnExit).toBool() || config()->get(Config::AutoSaveAfterEveryChange).toBool()) { saved = save(); + + if (!saved) { + // detect if a reload was triggered + bool reloadTriggered = false; + auto connection = + connect(this, &DatabaseWidget::reloadBegin, [&reloadTriggered] { reloadTriggered = true; }); + QApplication::processEvents(); + disconnect(connection); + if (reloadTriggered) { + return false; + } + } } if (!saved) { @@ -1919,52 +1937,76 @@ bool DatabaseWidget::lock() return true; } -void DatabaseWidget::reloadDatabaseFile() +void DatabaseWidget::reloadDatabaseFile(bool triggeredBySave) { - // Ignore reload if we are locked, saving, or currently editing an entry or group - if (!m_db || isLocked() || isEntryEditActive() || isGroupEditActive() || isSaving()) { + if (triggeredBySave) { + // not a failed save attempt due to file locking + m_saveAttempts = 0; + } + // Ignore reload if we are locked, saving, reloading, or currently editing an entry or group + if (!m_db || isLocked() || isEntryEditActive() || isGroupEditActive() || isSaving() || m_reloading) { return; } m_blockAutoSave = true; + m_reloading = true; - if (!config()->get(Config::AutoReloadOnChange).toBool()) { + emit reloadBegin(); + + if (!triggeredBySave && !config()->get(Config::AutoReloadOnChange).toBool()) { // Ask if we want to reload the db - auto result = MessageBox::question(this, - tr("File has changed"), - tr("The database file has changed. Do you want to load the changes?"), - MessageBox::Yes | MessageBox::No); + auto result = MessageBox::question( + this, + tr("File has changed"), + QString("%1.\n%2").arg(tr("The database file \"%1\" was modified externally").arg(displayFileName()), + tr("Do you want to load the changes?")), + MessageBox::Yes | MessageBox::No); if (result == MessageBox::No) { // Notify everyone the database does not match the file m_db->markAsModified(); + m_reloading = false; + + emit reloadEnd(); return; } } + // Remove any latent error messages and switch to progress updates + hideMessage(); + emit updateSyncProgress(0, tr("Reloading database…")); + // Lock out interactions m_entryView->setDisabled(true); m_groupView->setDisabled(true); m_tagView->setDisabled(true); QApplication::processEvents(); - QString error; - auto db = QSharedPointer::create(m_db->filePath()); - if (db->open(database()->key(), &error)) { - if (m_db->isModified() || db->hasNonDataChanges()) { - // Ask if we want to merge changes into new database - auto result = MessageBox::question( - this, - tr("Merge Request"), - tr("The database file has changed and you have unsaved changes.\nDo you want to merge your changes?"), - MessageBox::Merge | MessageBox::Discard, - MessageBox::Merge); + auto reloadFinish = [this](bool hideMsg = true) { + // Return control + m_entryView->setDisabled(false); + m_groupView->setDisabled(false); + m_tagView->setDisabled(false); - if (result == MessageBox::Merge) { - // Merge the old database into the new one - Merger merger(m_db.data(), db.data()); - merger.merge(); - } + m_reloading = false; + + // Keep the previous message visible for 2 seconds if not hiding + QTimer::singleShot(hideMsg ? 0 : 2000, this, [this] { emit updateSyncProgress(-1, ""); }); + + emit reloadEnd(); + }; + auto reloadCanceled = [this, reloadFinish] { + // Mark db as modified since existing data may differ from file or file was deleted + m_db->markAsModified(); + + emit updateSyncProgress(100, tr("Reload canceled")); + reloadFinish(false); + }; + auto reloadContinue = [this, triggeredBySave, reloadFinish](QSharedPointer db, bool merge) { + if (merge) { + // Merge the old database into the new one + Merger merger(m_db.data(), db.data()); + merger.merge(); } QUuid groupBeforeReload = m_db->rootGroup()->uuid(); @@ -1981,17 +2023,108 @@ void DatabaseWidget::reloadDatabaseFile() processAutoOpen(); restoreGroupEntryFocus(groupBeforeReload, entryBeforeReload); m_blockAutoSave = false; - } else { - showMessage(tr("Could not open the new database file while attempting to autoreload.\nError: %1").arg(error), - MessageWidget::Error); - // Mark db as modified since existing data may differ from file or file was deleted - m_db->markAsModified(); + + emit updateSyncProgress(100, tr("Reload successful")); + reloadFinish(false); + + // If triggered by save, attempt another save + if (triggeredBySave) { + save(); + } + }; + + auto db = QSharedPointer::create(m_db->filePath()); + bool openResult = db->open(database()->key()); + + // skip if the db is unchanged, or the db file is gone or for sure not a kp-db + if (bool sameHash = db->fileBlockHash() == m_db->fileBlockHash() || db->fileBlockHash().isEmpty()) { + if (!sameHash) { + // db file gone or invalid so mark modified + m_db->markAsModified(); + } + m_blockAutoSave = false; + reloadFinish(); + return; } - // Return control - m_entryView->setDisabled(false); - m_groupView->setDisabled(false); - m_tagView->setDisabled(false); + bool merge = false; + QString changesActionStr; + if (triggeredBySave || m_db->isModified() || m_db->hasNonDataChanges()) { + emit updateSyncProgress(50, tr("Reload pending user action…")); + + // Ask how to proceed + auto message = tr("The database file \"%1\" was modified externally.
" + "How would you like to proceed?

" + "Merge all changes
" + "Ignore the changes on disk until save
" + "Discard unsaved changes") + .arg(displayFileName()); + auto buttons = MessageBox::Merge | MessageBox::Discard | MessageBox::Ignore | MessageBox::Cancel; + // Different message if we are attempting to save + if (triggeredBySave) { + message = tr("The database file \"%1\" was modified externally.
" + "How would you like to proceed?

" + "Merge all changes then save
" + "Overwrite the changes on disk
" + "Discard unsaved changes") + .arg(displayFileName()); + buttons = MessageBox::Merge | MessageBox::Discard | MessageBox::Overwrite | MessageBox::Cancel; + } + + auto result = MessageBox::question(this, tr("Reload database"), message, buttons, MessageBox::Merge); + switch (result) { + case MessageBox::Cancel: + reloadCanceled(); + return; + case MessageBox::Overwrite: + case MessageBox::Ignore: + m_db->setIgnoreFileChangesUntilSaved(true); + m_blockAutoSave = false; + reloadFinish(!triggeredBySave); + // If triggered by save, attempt another save + if (triggeredBySave) { + save(); + emit updateSyncProgress(100, tr("Database file overwritten.")); + } + return; + case MessageBox::Merge: + merge = true; + default: + break; + } + } + + // Database file on disk previously opened successfully + if (openResult) { + reloadContinue(std::move(db), merge); + return; + } + + // The user needs to provide credentials + auto dbWidget = new DatabaseWidget(std::move(db)); + auto openDialog = new DatabaseOpenDialog(this); + connect(openDialog, &QObject::destroyed, [=](QObject*) { dbWidget->deleteLater(); }); + connect(openDialog, &DatabaseOpenDialog::dialogFinished, this, [=](bool accepted, DatabaseWidget*) { + if (accepted) { + reloadContinue(openDialog->database(), merge); + } else { + reloadCanceled(); + } + }); + openDialog->setAttribute(Qt::WA_DeleteOnClose); + openDialog->addDatabaseTab(dbWidget); + openDialog->setActiveDatabaseTab(dbWidget); + openDialog->showMessage(tr("Database file on disk cannot be unlocked with current credentials.
" + "Enter new credentials and/or present hardware key to continue."), + MessageWidget::Error, + MessageWidget::DisableAutoHide); + + // ensure the main window is visible for this + getMainWindow()->bringToFront(); + // show the unlock dialog + openDialog->show(); + openDialog->raise(); + openDialog->activateWindow(); } int DatabaseWidget::numberOfSelectedEntries() const @@ -2154,6 +2287,11 @@ bool DatabaseWidget::save() return true; } + // Do no try to save if the database is being reloaded + if (m_reloading) { + return false; + } + // Read-only and new databases ask for filename if (m_db->filePath().isEmpty()) { return saveAs(); @@ -2168,6 +2306,7 @@ bool DatabaseWidget::save() m_saveAttempts = 0; m_blockAutoSave = false; m_autosaveTimer->stop(); // stop autosave delay to avoid triggering another save + hideMessage(); return true; } @@ -2209,6 +2348,11 @@ bool DatabaseWidget::saveAs() return true; } + // Do no try to save if the database is being reloaded + if (m_reloading) { + return false; + } + QString oldFilePath = m_db->filePath(); if (!QFileInfo::exists(oldFilePath)) { QString defaultFileName = config()->get(Config::DefaultDatabaseFileName).toString(); diff --git a/src/gui/DatabaseWidget.h b/src/gui/DatabaseWidget.h index 879e804b1..7f74e9e4e 100644 --- a/src/gui/DatabaseWidget.h +++ b/src/gui/DatabaseWidget.h @@ -146,6 +146,7 @@ signals: void requestOpenDatabase(const QString& filePath, bool inBackground, const QString& password, const QString& keyFile); void databaseMerged(QSharedPointer mergedDb); + void updateSyncProgress(int progress, const QString& message); void groupContextMenuRequested(const QPoint& globalPos); void entryContextMenuRequested(const QPoint& globalPos); void listModeAboutToActivate(); @@ -157,6 +158,8 @@ signals: void clearSearch(); void requestGlobalAutoType(const QString& search); void requestSearch(const QString& search); + void reloadBegin(); + void reloadEnd(); public slots: bool lock(); @@ -269,7 +272,7 @@ private slots: void mergeDatabase(bool accepted); void emitCurrentModeChanged(); // Database autoreload slots - void reloadDatabaseFile(); + void reloadDatabaseFile(bool triggeredBySave); void restoreGroupEntryFocus(const QUuid& groupUuid, const QUuid& EntryUuid); void onConfigChanged(Config::ConfigKey key); @@ -318,6 +321,7 @@ private: // Autoreload bool m_blockAutoSave; + bool m_reloading; // Autosave delay QPointer m_autosaveTimer; diff --git a/src/gui/MainWindow.cpp b/src/gui/MainWindow.cpp index 71f3a204e..cf974d806 100644 --- a/src/gui/MainWindow.cpp +++ b/src/gui/MainWindow.cpp @@ -717,6 +717,7 @@ MainWindow::MainWindow() m_progressBar->setMaximum(100); statusBar()->addPermanentWidget(m_progressBar); connect(clipboard(), SIGNAL(updateCountdown(int, QString)), this, SLOT(updateProgressBar(int, QString))); + m_actionMultiplexer.connect(SIGNAL(updateSyncProgress(int, QString)), this, SLOT(updateProgressBar(int, QString))); m_statusBarLabel = new QLabel(statusBar()); m_statusBarLabel->setObjectName("statusBarLabel"); statusBar()->addPermanentWidget(m_statusBarLabel); diff --git a/src/streams/HashingStream.cpp b/src/streams/HashingStream.cpp new file mode 100644 index 000000000..5139dae87 --- /dev/null +++ b/src/streams/HashingStream.cpp @@ -0,0 +1,101 @@ +/* + * Copyright (C) 2024 KeePassXC Team + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 2 or (at your option) + * version 3 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +#include "HashingStream.h" + +HashingStream::HashingStream(QIODevice* baseDevice) + : LayeredStream(baseDevice) + , m_hash(QCryptographicHash::Md5) + , m_sizeToHash(0) +{ + init(); +} + +HashingStream::HashingStream(QIODevice* baseDevice, QCryptographicHash::Algorithm hashAlgo, qint64 sizeToHash) + : LayeredStream(baseDevice) + , m_hash(hashAlgo) + , m_sizeToHash(sizeToHash) +{ + init(); +} + +HashingStream::~HashingStream() +{ + close(); +} + +void HashingStream::init() +{ + m_sizeHashed = 0; + m_sizeStreamed = 0; + m_hashFinalized = false; +} + +bool HashingStream::reset() +{ + init(); + m_hash.reset(); + return LayeredStream::reset(); +} + +QByteArray HashingStream::hashingResult() +{ + if (m_sizeHashed <= 0 || (m_sizeToHash > 0 && m_sizeHashed != m_sizeToHash)) { + setErrorString("Not enough data to compute hash"); + return {}; + } + m_hashFinalized = true; + return m_hash.result(); +} + +qint64 HashingStream::readData(char* data, qint64 maxSize) +{ + auto sizeRead = LayeredStream::readData(data, maxSize); + if (sizeRead > 0) { + if (!m_hashFinalized) { + qint64 sizeToHash = sizeRead; + if (m_sizeToHash > 0) { + sizeToHash = qMin(m_sizeToHash - m_sizeStreamed, sizeRead); + } + if (sizeToHash > 0) { + m_hash.addData(data, sizeToHash); + m_sizeHashed += sizeToHash; + } + } + m_sizeStreamed += sizeRead; + } + return sizeRead; +} + +qint64 HashingStream::writeData(const char* data, qint64 maxSize) +{ + auto sizeWritten = LayeredStream::writeData(data, maxSize); + if (sizeWritten > 0) { + if (!m_hashFinalized) { + qint64 sizeToHash = sizeWritten; + if (m_sizeToHash > 0) { + sizeToHash = qMin(m_sizeToHash - m_sizeStreamed, sizeWritten); + } + if (sizeToHash > 0) { + m_hash.addData(data, sizeToHash); + m_sizeHashed += sizeToHash; + } + } + m_sizeStreamed += sizeWritten; + } + return sizeWritten; +} diff --git a/src/streams/HashingStream.h b/src/streams/HashingStream.h new file mode 100644 index 000000000..5f8c2ac3e --- /dev/null +++ b/src/streams/HashingStream.h @@ -0,0 +1,52 @@ +/* + * Copyright (C) 2024 KeePassXC Team + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 2 or (at your option) + * version 3 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +#ifndef KEEPASSX_HASHINGSTREAM_H +#define KEEPASSX_HASHINGSTREAM_H + +#include + +#include "streams/LayeredStream.h" + +class HashingStream : public LayeredStream +{ + Q_OBJECT + +public: + explicit HashingStream(QIODevice* baseDevice); + HashingStream(QIODevice* baseDevice, QCryptographicHash::Algorithm hashAlgo, qint64 sizeToHash); + ~HashingStream() override; + + bool reset() override; + + QByteArray hashingResult(); + +protected: + void init(); + + qint64 readData(char* data, qint64 maxSize) override; + qint64 writeData(const char* data, qint64 maxSize) override; + +protected: + QCryptographicHash m_hash; + bool m_hashFinalized; + qint64 m_sizeToHash; + qint64 m_sizeHashed; + qint64 m_sizeStreamed; +}; + +#endif // KEEPASSX_HASHINGSTREAM_H \ No newline at end of file diff --git a/tests/TestDatabase.cpp b/tests/TestDatabase.cpp index 3b20d6fc5..78b6dac1d 100644 --- a/tests/TestDatabase.cpp +++ b/tests/TestDatabase.cpp @@ -164,7 +164,7 @@ void TestDatabase::testSignals() // Short delay to allow file system settling to reduce test failures Tools::wait(100); - QSignalSpy spyFileChanged(db.data(), SIGNAL(databaseFileChanged())); + QSignalSpy spyFileChanged(db.data(), &Database::databaseFileChanged); QVERIFY(tempFile.copyFromFile(dbFileName)); QTRY_COMPARE(spyFileChanged.count(), 1); QTRY_VERIFY(!db->isModified()); @@ -268,3 +268,41 @@ void TestDatabase::testCustomIcons() QCOMPARE(iconData.name, QString("Test")); QCOMPARE(iconData.lastModified, date); } + +void TestDatabase::testExternallyModified() +{ + TemporaryFile tempFile; + QVERIFY(tempFile.copyFromFile(dbFileName)); + + auto db = QSharedPointer::create(); + auto key = QSharedPointer::create(); + key->addKey(QSharedPointer::create("a")); + + QString error; + QVERIFY(db->open(tempFile.fileName(), key, &error) == true); + db->metadata()->setName("test2"); + QVERIFY(db->save(Database::Atomic, {}, &error)); + + QSignalSpy spyFileChanged(db.data(), &Database::databaseFileChanged); + QVERIFY(tempFile.copyFromFile(dbFileName)); + QTRY_COMPARE(spyFileChanged.count(), 1); + // the first argument of the databaseFileChanged signal (triggeredBySave) should be false + QVERIFY(spyFileChanged.at(0).length() == 1); + QVERIFY(spyFileChanged.at(0).at(0).type() == QVariant::Bool); + QVERIFY(spyFileChanged.at(0).at(0).toBool() == false); + spyFileChanged.clear(); + // shouldn't be able to save due to external changes + QVERIFY(db->save(Database::Atomic, {}, &error) == false); + QApplication::processEvents(); + // save should have triggered another databaseFileChanged signal + QVERIFY(spyFileChanged.count() >= 1); + // the first argument of the databaseFileChanged signal (triggeredBySave) should be true + QVERIFY(spyFileChanged.at(0).at(0).type() == QVariant::Bool); + QVERIFY(spyFileChanged.at(0).at(0).toBool() == true); + + // should be able to overwrite externally modified changes when explicitly requested + db->setIgnoreFileChangesUntilSaved(true); + QVERIFY(db->save(Database::Atomic, {}, &error)); + // ignoreFileChangesUntilSaved should reset after save + QVERIFY(db->ignoreFileChangesUntilSaved() == false); +} diff --git a/tests/TestDatabase.h b/tests/TestDatabase.h index 9f4bfab56..e23b23cf8 100644 --- a/tests/TestDatabase.h +++ b/tests/TestDatabase.h @@ -36,6 +36,7 @@ private slots: void testEmptyRecycleBinOnEmpty(); void testEmptyRecycleBinWithHierarchicalData(); void testCustomIcons(); + void testExternallyModified(); }; #endif // KEEPASSX_TESTDATABASE_H