Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[stable-3.8] E2EE. Fix freeze on metadata checksum validation. #5657

Merged
merged 1 commit into from
May 8, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions src/gui/accountmanager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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
Expand Down
12 changes: 12 additions & 0 deletions src/libsync/account.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand Down
5 changes: 5 additions & 0 deletions src/libsync/account.h
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -402,6 +406,7 @@ protected Q_SLOTS:
QSslConfiguration _sslConfiguration;
Capabilities _capabilities;
QString _serverVersion;
bool _skipE2eeMetadataChecksumValidation = false;
QScopedPointer<AbstractSslErrorHandler> _sslErrorHandler;
QSharedPointer<QNetworkAccessManager> _am;
QScopedPointer<AbstractCredentials> _credentials;
Expand Down
43 changes: 8 additions & 35 deletions src/libsync/clientsideencryption.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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;
}

Expand All @@ -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;
Expand Down
2 changes: 0 additions & 2 deletions src/libsync/clientsideencryption.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
6 changes: 5 additions & 1 deletion src/libsync/owncloudpropagator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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.");
Expand Down
5 changes: 5 additions & 0 deletions src/libsync/syncengine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down