From 0070c4c2f0ee4c2f652a3c6bd6c73a5e0558e48a Mon Sep 17 00:00:00 2001 From: alex-z Date: Fri, 5 May 2023 16:15:09 +0200 Subject: [PATCH] E2EE. Fix freeze on metadata checksum validation. Signed-off-by: alex-z --- src/gui/accountmanager.cpp | 7 +++++ src/libsync/account.cpp | 12 ++++++++ src/libsync/account.h | 5 ++++ src/libsync/clientsideencryption.cpp | 43 ++++++---------------------- src/libsync/clientsideencryption.h | 2 -- src/libsync/owncloudpropagator.cpp | 6 +++- src/libsync/syncengine.cpp | 5 ++++ 7 files changed, 42 insertions(+), 38 deletions(-) diff --git a/src/gui/accountmanager.cpp b/src/gui/accountmanager.cpp index 93b5d5dba2c2e..157aeb4fa8e67 100644 --- a/src/gui/accountmanager.cpp +++ b/src/gui/accountmanager.cpp @@ -41,6 +41,7 @@ constexpr auto caCertsKeyC = "CaCertificates"; constexpr auto accountsC = "Accounts"; constexpr auto versionC = "version"; constexpr auto serverVersionC = "serverVersion"; +constexpr auto skipE2eeMetadataChecksumValidationC = "skipE2eeMetadataChecksumValidation"; constexpr auto generalC = "General"; constexpr auto dummyAuthTypeC = "dummy"; @@ -286,6 +287,11 @@ void AccountManager::saveAccountHelper(Account *acc, QSettings &settings, bool s settings.setValue(QLatin1String(davUserC), acc->_davUser); settings.setValue(QLatin1String(displayNameC), acc->_displayName); settings.setValue(QLatin1String(serverVersionC), acc->_serverVersion); + if (!acc->_skipE2eeMetadataChecksumValidation) { + settings.remove(QLatin1String(skipE2eeMetadataChecksumValidationC)); + } else { + settings.setValue(QLatin1String(skipE2eeMetadataChecksumValidationC), acc->_skipE2eeMetadataChecksumValidation); + } if (acc->_credentials) { if (saveCredentials) { @@ -385,6 +391,7 @@ AccountPtr AccountManager::loadAccountHelper(QSettings &settings) qCInfo(lcAccountManager) << "Account for" << acc->url() << "using auth type" << authType; acc->_serverVersion = settings.value(QLatin1String(serverVersionC)).toString(); + acc->_skipE2eeMetadataChecksumValidation = settings.value(QLatin1String(skipE2eeMetadataChecksumValidationC), {}).toBool(); acc->_davUser = settings.value(QLatin1String(davUserC), "").toString(); // We want to only restore settings for that auth type and the user value diff --git a/src/libsync/account.cpp b/src/libsync/account.cpp index 9cc5e11061397..9f080fdca1975 100644 --- a/src/libsync/account.cpp +++ b/src/libsync/account.cpp @@ -61,6 +61,7 @@ namespace { constexpr int pushNotificationsReconnectInterval = 1000 * 60 * 2; constexpr int usernamePrefillServerVersionMinSupportedMajor = 24; constexpr int checksumRecalculateRequestServerVersionMinSupportedMajor = 24; +constexpr auto isSkipE2eeMetadataChecksumValidationAllowedInClientVersion = MIRALL_VERSION_MAJOR == 3 && MIRALL_VERSION_MINOR == 8; } namespace OCC { @@ -677,6 +678,17 @@ QString Account::serverVersion() const return _serverVersion; } +bool Account::shouldSkipE2eeMetadataChecksumValidation() const +{ + return isSkipE2eeMetadataChecksumValidationAllowedInClientVersion && _skipE2eeMetadataChecksumValidation; +} + +void Account::resetShouldSkipE2eeMetadataChecksumValidation() +{ + _skipE2eeMetadataChecksumValidation = false; + emit wantsAccountSaved(this); +} + int Account::serverVersionInt() const { // FIXME: Use Qt 5.5 QVersionNumber diff --git a/src/libsync/account.h b/src/libsync/account.h index 87f685dfebd2b..0546c84f170a3 100644 --- a/src/libsync/account.h +++ b/src/libsync/account.h @@ -233,6 +233,10 @@ class OWNCLOUDSYNC_EXPORT Account : public QObject */ [[nodiscard]] QString serverVersion() const; + // check if the checksum validation of E2EE metadata is allowed to be skipped via config file, this will only work before client 3.9.0 + [[nodiscard]] bool shouldSkipE2eeMetadataChecksumValidation() const; + void resetShouldSkipE2eeMetadataChecksumValidation(); + /** Server version for easy comparison. * * Example: serverVersionInt() >= makeServerVersion(11, 2, 3) @@ -402,6 +406,7 @@ protected Q_SLOTS: QSslConfiguration _sslConfiguration; Capabilities _capabilities; QString _serverVersion; + bool _skipE2eeMetadataChecksumValidation = false; QScopedPointer _sslErrorHandler; QSharedPointer _am; QScopedPointer _credentials; diff --git a/src/libsync/clientsideencryption.cpp b/src/libsync/clientsideencryption.cpp index d1716841a0482..ca957d34500fe 100644 --- a/src/libsync/clientsideencryption.cpp +++ b/src/libsync/clientsideencryption.cpp @@ -1643,9 +1643,14 @@ void FolderMetadata::setupExistingMetadata(const QByteArray& metadata) if (!migratedMetadata && !checkMetadataKeyChecksum(metadataKey, metadataKeyChecksum)) { qCInfo(lcCseMetadata) << "checksum comparison failed" << "server value" << metadataKeyChecksum << "client value" << computeMetadataKeyChecksum(metadataKey); - _metadataKey.clear(); - _files.clear(); - return; + if (_account->shouldSkipE2eeMetadataChecksumValidation()) { + qCDebug(lcCseMetadata) << "shouldSkipE2eeMetadataChecksumValidation is set. Allowing invalid checksum until next sync."; + _encryptedMetadataNeedUpdate = true; + } else { + _metadataKey.clear(); + _files.clear(); + return; + } } // decryption finished, create new metadata key to be used for encryption @@ -1721,13 +1726,6 @@ bool FolderMetadata::checkMetadataKeyChecksum(const QByteArray &metadataKey, { const auto referenceMetadataKeyValue = computeMetadataKeyChecksum(metadataKey); - if (referenceMetadataKeyValue != metadataKeyChecksum) { - if (recoverMetadataKeyChecksum(metadataKeyChecksum, metadataKey)) { - qCInfo(lcCseMetadata) << "Checksum recovery done"; - return true; - } - } - return referenceMetadataKeyValue == metadataKeyChecksum; } @@ -1748,31 +1746,6 @@ QByteArray FolderMetadata::computeMetadataKeyChecksum(const QByteArray &metadata return hashAlgorithm.result().toHex(); } -bool FolderMetadata::recoverMetadataKeyChecksum(const QByteArray &expectedChecksum, - const QByteArray &metadataKey) const -{ - const auto sortLambda = [] (const auto &first, const auto &second) { - return first.encryptedFilename < second.encryptedFilename; - }; - auto sortedFiles = _files; - - std::sort(sortedFiles.begin(), sortedFiles.end(), sortLambda); - - do { - auto hashAlgorithm = QCryptographicHash{QCryptographicHash::Sha256}; - hashAlgorithm.addData(_account->e2e()->_mnemonic.remove(' ').toUtf8()); - for (const auto &singleFile : sortedFiles) { - hashAlgorithm.addData(singleFile.encryptedFilename.toUtf8()); - } - hashAlgorithm.addData(metadataKey); - if (hashAlgorithm.result().toHex() == expectedChecksum) { - return true; - } - } while (std::next_permutation(sortedFiles.begin(), sortedFiles.end(), sortLambda)); - - return false; -} - bool FolderMetadata::isMetadataSetup() const { return _isMetadataSetup; diff --git a/src/libsync/clientsideencryption.h b/src/libsync/clientsideencryption.h index 212502a0a9a1f..077e05a43f65a 100644 --- a/src/libsync/clientsideencryption.h +++ b/src/libsync/clientsideencryption.h @@ -233,8 +233,6 @@ class OWNCLOUDSYNC_EXPORT FolderMetadata { [[nodiscard]] bool checkMetadataKeyChecksum(const QByteArray &metadataKey, const QByteArray &metadataKeyChecksum) const; [[nodiscard]] QByteArray computeMetadataKeyChecksum(const QByteArray &metadataKey) const; - [[nodiscard]] bool recoverMetadataKeyChecksum(const QByteArray &expectedChecksum, - const QByteArray &metadataKey) const; QByteArray _metadataKey; diff --git a/src/libsync/owncloudpropagator.cpp b/src/libsync/owncloudpropagator.cpp index 061fec53ee366..19d5e61913c10 100644 --- a/src/libsync/owncloudpropagator.cpp +++ b/src/libsync/owncloudpropagator.cpp @@ -652,7 +652,11 @@ void OwncloudPropagator::startDirectoryPropagation(const SyncFileItemPtr &item, _anotherSyncNeeded = true; } else if (item->_isEncryptedMetadataNeedUpdate) { SyncJournalFileRecord record; - if (_journal->getFileRecord(item->_file, &record) && record._e2eEncryptionStatus == SyncJournalFileRecord::EncryptionStatus::EncryptedMigratedV1_2) { + const auto isUnexpectedMetadataFormat = _journal->getFileRecord(item->_file, &record) + && record._e2eEncryptionStatus == SyncJournalFileRecord::EncryptionStatus::EncryptedMigratedV1_2; + if (isUnexpectedMetadataFormat && _account->shouldSkipE2eeMetadataChecksumValidation()) { + qCDebug(lcPropagator) << "Getting unexpected metadata format, but allowing to continue as shouldSkipE2eeMetadataChecksumValidation is set."; + } else if (isUnexpectedMetadataFormat && !_account->shouldSkipE2eeMetadataChecksumValidation()) { qCDebug(lcPropagator) << "could have upgraded metadata"; item->_instruction = CSyncEnums::CSYNC_INSTRUCTION_ERROR; item->_errorString = tr("Error with the metadata. Getting unexpected metadata format."); diff --git a/src/libsync/syncengine.cpp b/src/libsync/syncengine.cpp index 23c41be9e991b..f9a834d9ea42a 100644 --- a/src/libsync/syncengine.cpp +++ b/src/libsync/syncengine.cpp @@ -956,6 +956,11 @@ void SyncEngine::finalize(bool success) _syncRunning = false; emit finished(success); + if (_account->shouldSkipE2eeMetadataChecksumValidation()) { + qCDebug(lcEngine) << "shouldSkipE2eeMetadataChecksumValidation was set. Sync is finished, so resetting it..."; + _account->resetShouldSkipE2eeMetadataChecksumValidation(); + } + // Delete the propagator only after emitting the signal. _propagator.clear(); _seenConflictFiles.clear();