From 8d2cbc02e17acef31c0606013c342d6c33a35764 Mon Sep 17 00:00:00 2001 From: alex-z Date: Sat, 24 Feb 2024 17:47:03 +0100 Subject: [PATCH] Bugfix. E2EE V2. Fix incorrect root e2ee folder path search in local db. Signed-off-by: alex-z --- src/common/utility.cpp | 24 ++++++++++ src/common/utility.h | 2 + src/gui/accountsettings.cpp | 2 +- src/gui/folder.cpp | 5 ++ src/gui/folder.h | 2 + src/gui/socketapi/socketapi.cpp | 2 +- .../basepropagateremotedeleteencrypted.cpp | 2 +- src/libsync/encryptfolderjob.cpp | 13 ++--- src/libsync/encryptfolderjob.h | 6 +-- src/libsync/owncloudpropagator.cpp | 9 ++++ src/libsync/owncloudpropagator.h | 2 + src/libsync/propagatedownloadencrypted.cpp | 3 +- src/libsync/propagateremotemkdir.cpp | 10 +++- src/libsync/propagateuploadencrypted.cpp | 4 +- src/libsync/updatee2eefoldermetadatajob.cpp | 5 +- .../updatee2eefolderusersmetadatajob.cpp | 4 +- src/libsync/vfs/cfapi/hydrationjob.cpp | 2 +- test/testutility.cpp | 48 +++++++++++++++++++ 18 files changed, 122 insertions(+), 23 deletions(-) diff --git a/src/common/utility.cpp b/src/common/utility.cpp index e4eff93d3fc6..a2a315cadc5a 100644 --- a/src/common/utility.cpp +++ b/src/common/utility.cpp @@ -693,5 +693,29 @@ QString Utility::noLeadingSlashPath(const QString &path) return path.startsWith(slash) ? path.mid(1) : path; } +QString Utility::noTrailingSlashPath(const QString &path) +{ + static const auto slash = QLatin1Char('/'); + return path.endsWith(slash) ? path.chopped(1) : path; +} + +QString Utility::fullRemotePathToRemoteSyncRootRelative(const QString &fullRemotePath, const QString &remoteSyncRoot) +{ + const auto remoteSyncRootNoLeadingSlashWithTrailingSlash = Utility::trailingSlashPath(noLeadingSlashPath(remoteSyncRoot)); + const auto fullRemotePathNoLeadingSlash = noLeadingSlashPath(fullRemotePath); + + if (remoteSyncRootNoLeadingSlashWithTrailingSlash == QStringLiteral("/")) { + return noLeadingSlashPath(noTrailingSlashPath(fullRemotePath)); + } + + if (!fullRemotePathNoLeadingSlash.startsWith(remoteSyncRootNoLeadingSlashWithTrailingSlash)) { + return fullRemotePath; + } + + const auto relativePathToRemoteSyncRoot = fullRemotePathNoLeadingSlash.mid(remoteSyncRootNoLeadingSlashWithTrailingSlash.size()); + Q_ASSERT(!relativePathToRemoteSyncRoot.isEmpty()); + return noLeadingSlashPath(noTrailingSlashPath(relativePathToRemoteSyncRoot)); +} + } // namespace OCC diff --git a/src/common/utility.h b/src/common/utility.h index a76527c1caa8..c7b3668bdbcb 100644 --- a/src/common/utility.h +++ b/src/common/utility.h @@ -269,6 +269,8 @@ namespace Utility { OCSYNC_EXPORT QString trailingSlashPath(const QString &path); OCSYNC_EXPORT QString noLeadingSlashPath(const QString &path); + OCSYNC_EXPORT QString noTrailingSlashPath(const QString &path); + OCSYNC_EXPORT QString fullRemotePathToRemoteSyncRootRelative(const QString &fullRemotePath, const QString &remoteSyncRoot); #ifdef Q_OS_WIN OCSYNC_EXPORT bool registryKeyExists(HKEY hRootKey, const QString &subKey); diff --git a/src/gui/accountsettings.cpp b/src/gui/accountsettings.cpp index 8387c25c0bdc..2ac67f6adce5 100644 --- a/src/gui/accountsettings.cpp +++ b/src/gui/accountsettings.cpp @@ -425,7 +425,7 @@ void AccountSettings::slotMarkSubfolderEncrypted(FolderStatusModel::SubFolderInf Q_ASSERT(!path.startsWith('/') && path.endsWith('/')); // But EncryptFolderJob expects directory path Foo/Bar convention const auto choppedPath = path.chopped(1); - auto job = new OCC::EncryptFolderJob(accountsState()->account(), folder->journalDb(), choppedPath, fileId); + auto job = new OCC::EncryptFolderJob(accountsState()->account(), folder->journalDb(), choppedPath, choppedPath, folder->remotePath(), fileId); job->setParent(this); job->setProperty(propertyFolder, QVariant::fromValue(folder)); job->setProperty(propertyPath, QVariant::fromValue(path)); diff --git a/src/gui/folder.cpp b/src/gui/folder.cpp index a8098fd9a6a0..7b42535f22d8 100644 --- a/src/gui/folder.cpp +++ b/src/gui/folder.cpp @@ -271,6 +271,11 @@ QString Folder::remotePathTrailingSlash() const return Utility::trailingSlashPath(remotePath()); } +QString Folder::fulllRemotePathToPathInSyncJournalDb(const QString &fullRemotePath) const +{ + return Utility::fullRemotePathToRemoteSyncRootRelative(fullRemotePath, remotePathTrailingSlash()); +} + QUrl Folder::remoteUrl() const { return Utility::concatUrlPath(_accountState->account()->davUrl(), remotePath()); diff --git a/src/gui/folder.h b/src/gui/folder.h index ec50403afe55..eea99e144778 100644 --- a/src/gui/folder.h +++ b/src/gui/folder.h @@ -162,6 +162,8 @@ class Folder : public QObject */ QString remotePathTrailingSlash() const; + [[nodiscard]] QString fulllRemotePathToPathInSyncJournalDb(const QString &fullRemotePath) const; + void setNavigationPaneClsid(const QUuid &clsid) { _definition.navigationPaneClsid = clsid; } QUuid navigationPaneClsid() const { return _definition.navigationPaneClsid; } diff --git a/src/gui/socketapi/socketapi.cpp b/src/gui/socketapi/socketapi.cpp index aa909934ea76..91219f9741f5 100644 --- a/src/gui/socketapi/socketapi.cpp +++ b/src/gui/socketapi/socketapi.cpp @@ -542,7 +542,7 @@ void SocketApi::processEncryptRequest(const QString &localFile) choppedPath = choppedPath.mid(1); } - auto job = new OCC::EncryptFolderJob(account, folder->journalDb(), choppedPath, rec.numericFileId()); + auto job = new OCC::EncryptFolderJob(account, folder->journalDb(), choppedPath, choppedPath, folder->remotePath(), rec.numericFileId()); job->setParent(this); connect(job, &OCC::EncryptFolderJob::finished, this, [fileData, job](const int status) { if (status == OCC::EncryptFolderJob::Error) { diff --git a/src/libsync/basepropagateremotedeleteencrypted.cpp b/src/libsync/basepropagateremotedeleteencrypted.cpp index 9503a081091e..8828a37252a1 100644 --- a/src/libsync/basepropagateremotedeleteencrypted.cpp +++ b/src/libsync/basepropagateremotedeleteencrypted.cpp @@ -61,7 +61,7 @@ void BasePropagateRemoteDeleteEncrypted::fetchMetadataForPath(const QString &pat _fullFolderRemotePath = _propagator->fullRemotePath(path); SyncJournalFileRecord rec; - if (!_propagator->_journal->getRootE2eFolderRecord(_fullFolderRemotePath, &rec) || !rec.isValid()) { + if (!_propagator->_journal->getRootE2eFolderRecord(Utility::fullRemotePathToRemoteSyncRootRelative(_fullFolderRemotePath, _propagator->remotePath()), &rec) || !rec.isValid()) { taskFailed(); return; } diff --git a/src/libsync/encryptfolderjob.cpp b/src/libsync/encryptfolderjob.cpp index 11e383f4136b..dba7c8593f8f 100644 --- a/src/libsync/encryptfolderjob.cpp +++ b/src/libsync/encryptfolderjob.cpp @@ -23,19 +23,21 @@ namespace OCC { Q_LOGGING_CATEGORY(lcEncryptFolderJob, "nextcloud.sync.propagator.encryptfolder", QtInfoMsg) -EncryptFolderJob::EncryptFolderJob(const AccountPtr &account, SyncJournalDb *journal, const QString &path, const QByteArray &fileId, OwncloudPropagator *propagator, SyncFileItemPtr item, +EncryptFolderJob::EncryptFolderJob(const AccountPtr &account, SyncJournalDb *journal, const QString &path, const QString &pathNonEncrypted, const QString &remoteSyncRootPath, const QByteArray &fileId, OwncloudPropagator *propagator, SyncFileItemPtr item, QObject * parent) : QObject(parent) , _account(account) , _journal(journal) , _path(path) + , _pathNonEncrypted(pathNonEncrypted) + , _remoteSyncRootPath(remoteSyncRootPath) , _fileId(fileId) , _propagator(propagator) , _item(item) { SyncJournalFileRecord rec; const auto currentPath = !_pathNonEncrypted.isEmpty() ? _pathNonEncrypted : _path; - [[maybe_unused]] const auto result = _journal->getRootE2eFolderRecord(currentPath, &rec); + [[maybe_unused]] const auto result = _journal->getRootE2eFolderRecord(Utility::fullRemotePathToRemoteSyncRootRelative(currentPath, _remoteSyncRootPath), &rec); _encryptedFolderMetadataHandler.reset(new EncryptedFolderMetadataHandler(account, _path, _journal, rec.path())); } @@ -57,11 +59,6 @@ QString EncryptFolderJob::errorString() const return _errorString; } -void EncryptFolderJob::setPathNonEncrypted(const QString &pathNonEncrypted) -{ - _pathNonEncrypted = pathNonEncrypted; -} - void EncryptFolderJob::slotEncryptionFlagSuccess(const QByteArray &fileId) { SyncJournalFileRecord rec; @@ -106,7 +103,7 @@ void EncryptFolderJob::uploadMetadata() { const auto currentPath = !_pathNonEncrypted.isEmpty() ? _pathNonEncrypted : _path; SyncJournalFileRecord rec; - if (!_journal->getRootE2eFolderRecord(currentPath, &rec)) { + if (!_journal->getRootE2eFolderRecord(Utility::fullRemotePathToRemoteSyncRootRelative(currentPath, _remoteSyncRootPath), &rec)) { emit finished(Error, EncryptionStatusEnums::ItemEncryptionStatus::NotEncrypted); return; } diff --git a/src/libsync/encryptfolderjob.h b/src/libsync/encryptfolderjob.h index 7d5600d840d3..3281f580d350 100644 --- a/src/libsync/encryptfolderjob.h +++ b/src/libsync/encryptfolderjob.h @@ -36,6 +36,8 @@ class OWNCLOUDSYNC_EXPORT EncryptFolderJob : public QObject explicit EncryptFolderJob(const AccountPtr &account, SyncJournalDb *journal, const QString &path, + const QString &pathNonEncrypted, + const QString &_remoteSyncRootPath, const QByteArray &fileId, OwncloudPropagator *propagator = nullptr, SyncFileItemPtr item = {}, @@ -47,9 +49,6 @@ class OWNCLOUDSYNC_EXPORT EncryptFolderJob : public QObject signals: void finished(int status, EncryptionStatusEnums::ItemEncryptionStatus encryptionStatus); -public slots: - void setPathNonEncrypted(const QString &pathNonEncrypted); - private: void uploadMetadata(); @@ -64,6 +63,7 @@ private slots: SyncJournalDb *_journal; QString _path; QString _pathNonEncrypted; + QString _remoteSyncRootPath; QByteArray _fileId; QString _errorString; OwncloudPropagator *_propagator = nullptr; diff --git a/src/libsync/owncloudpropagator.cpp b/src/libsync/owncloudpropagator.cpp index 010771a183e5..a5d10ed1c903 100644 --- a/src/libsync/owncloudpropagator.cpp +++ b/src/libsync/owncloudpropagator.cpp @@ -1660,6 +1660,15 @@ QString OwncloudPropagator::fullRemotePath(const QString &tmp_file_name) const return _remoteFolder + tmp_file_name; } +QString OwncloudPropagator::fulllRemotePathToPathInSyncJournalDb(const QString &fullRemotePath) const +{ + auto result = _remoteFolder != QStringLiteral("/") ? fullRemotePath.mid(_remoteFolder.size()) : fullRemotePath; + if (result.startsWith("/")) { + result = result.mid(1); + } + return result; +} + QString OwncloudPropagator::remotePath() const { return _remoteFolder; diff --git a/src/libsync/owncloudpropagator.h b/src/libsync/owncloudpropagator.h index 9f5c8d058182..0e4640e22cf8 100644 --- a/src/libsync/owncloudpropagator.h +++ b/src/libsync/owncloudpropagator.h @@ -526,6 +526,8 @@ class OWNCLOUDSYNC_EXPORT OwncloudPropagator : public QObject Q_REQUIRED_RESULT QString fullRemotePath(const QString &tmp_file_name) const; [[nodiscard]] QString remotePath() const; + [[nodiscard]] QString fulllRemotePathToPathInSyncJournalDb(const QString &fullRemotePath) const; + /** Creates the job for an item. */ PropagateItemJob *createJob(const SyncFileItemPtr &item); diff --git a/src/libsync/propagatedownloadencrypted.cpp b/src/libsync/propagatedownloadencrypted.cpp index 837c4f98ffd2..bb77c8a3e6f9 100644 --- a/src/libsync/propagatedownloadencrypted.cpp +++ b/src/libsync/propagatedownloadencrypted.cpp @@ -37,7 +37,8 @@ PropagateDownloadEncrypted::PropagateDownloadEncrypted(OwncloudPropagator *propa void PropagateDownloadEncrypted::start() { SyncJournalFileRecord rec; - if (!_propagator->_journal->getRootE2eFolderRecord(_remoteParentPath, &rec) || !rec.isValid()) { + if (!_propagator->_journal->getRootE2eFolderRecord(Utility::fullRemotePathToRemoteSyncRootRelative(_remoteParentPath, _propagator->remotePath()), &rec) + || !rec.isValid()) { emit failed(); return; } diff --git a/src/libsync/propagateremotemkdir.cpp b/src/libsync/propagateremotemkdir.cpp index 8f8da5fd3062..cb7a6587888e 100644 --- a/src/libsync/propagateremotemkdir.cpp +++ b/src/libsync/propagateremotemkdir.cpp @@ -157,9 +157,15 @@ void PropagateRemoteMkdir::finalizeMkColJob(QNetworkReply::NetworkError err, con // We're expecting directory path in /Foo/Bar convention... Q_ASSERT(jobPath.startsWith('/') && !jobPath.endsWith('/')); // But encryption job expect it in Foo/Bar/ convention - auto job = new OCC::EncryptFolderJob(propagator()->account(), propagator()->_journal, jobPath.mid(1), _item->_fileId, propagator(), _item); + auto job = new OCC::EncryptFolderJob(propagator()->account(), + propagator()->_journal, + jobPath.mid(1), + _item->_file, + propagator()->remotePath(), + _item->_fileId, + propagator(), + _item); job->setParent(this); - job->setPathNonEncrypted(_item->_file); connect(job, &OCC::EncryptFolderJob::finished, this, &PropagateRemoteMkdir::slotEncryptFolderFinished); job->start(); } diff --git a/src/libsync/propagateuploadencrypted.cpp b/src/libsync/propagateuploadencrypted.cpp index 2e24e7725469..e6fd284ccbbe 100644 --- a/src/libsync/propagateuploadencrypted.cpp +++ b/src/libsync/propagateuploadencrypted.cpp @@ -55,7 +55,9 @@ void PropagateUploadEncrypted::start() */ // Encrypt File! SyncJournalFileRecord rec; - if (!_propagator->_journal->getRootE2eFolderRecord(_remoteParentAbsolutePath, &rec) || !rec.isValid()) { + if (!_propagator->_journal->getRootE2eFolderRecord(Utility::fullRemotePathToRemoteSyncRootRelative(_remoteParentAbsolutePath, _propagator->remotePath()), + &rec) + || !rec.isValid()) { emit error(); return; } diff --git a/src/libsync/updatee2eefoldermetadatajob.cpp b/src/libsync/updatee2eefoldermetadatajob.cpp index 7bca9c4381b7..6d9267944acb 100644 --- a/src/libsync/updatee2eefoldermetadatajob.cpp +++ b/src/libsync/updatee2eefoldermetadatajob.cpp @@ -42,7 +42,7 @@ void UpdateE2eeFolderMetadataJob::start() qCDebug(lcUpdateFileDropMetadataJob) << "Folder is encrypted, let's fetch metadata."; SyncJournalFileRecord rec; - if (!propagator()->_journal->getRootE2eFolderRecord(_encryptedRemotePath, &rec) || !rec.isValid()) { + if (!propagator()->_journal->getRootE2eFolderRecord(Utility::fullRemotePathToRemoteSyncRootRelative(_encryptedRemotePath, propagator()->remotePath()), &rec) || !rec.isValid()) { unlockFolder(EncryptedFolderMetadataHandler::UnlockFolderWithResult::Failure); return; } @@ -84,7 +84,8 @@ void UpdateE2eeFolderMetadataJob::slotFetchMetadataJobFinished(int httpReturnCod } SyncJournalFileRecord rec; - if (!propagator()->_journal->getRootE2eFolderRecord(_encryptedRemotePath, &rec) || !rec.isValid()) { + if (!propagator()->_journal->getRootE2eFolderRecord(Utility::fullRemotePathToRemoteSyncRootRelative(_encryptedRemotePath, propagator()->remotePath()), &rec) + || !rec.isValid()) { unlockFolder(EncryptedFolderMetadataHandler::UnlockFolderWithResult::Failure); return; } diff --git a/src/libsync/updatee2eefolderusersmetadatajob.cpp b/src/libsync/updatee2eefolderusersmetadatajob.cpp index ecbbd5b24f2d..e69bd81e351d 100644 --- a/src/libsync/updatee2eefolderusersmetadatajob.cpp +++ b/src/libsync/updatee2eefolderusersmetadatajob.cpp @@ -45,7 +45,7 @@ UpdateE2eeFolderUsersMetadataJob::UpdateE2eeFolderUsersMetadataJob(const Account const auto folderPath = _syncFolderRemotePath + pathSanitized; SyncJournalFileRecord rec; - if (!_journalDb->getRootE2eFolderRecord(_path, &rec) || !rec.isValid()) { + if (!_journalDb->getRootE2eFolderRecord(Utility::fullRemotePathToRemoteSyncRootRelative(_path, _syncFolderRemotePath), &rec) || !rec.isValid()) { qCDebug(lcUpdateE2eeFolderUsersMetadataJob) << "Could not get root E2ee folder recort for path" << _path; return; } @@ -96,7 +96,7 @@ void UpdateE2eeFolderUsersMetadataJob::slotStartE2eeMetadataJobs() const auto pathSanitized = _path.startsWith(QLatin1Char('/')) ? _path.mid(1) : _path; const auto folderPath = _syncFolderRemotePath + pathSanitized; SyncJournalFileRecord rec; - if (!_journalDb->getRootE2eFolderRecord(_path, &rec) || !rec.isValid()) { + if (!_journalDb->getRootE2eFolderRecord(Utility::fullRemotePathToRemoteSyncRootRelative(_path, _syncFolderRemotePath), &rec) || !rec.isValid()) { emit finished(404, tr("Could not find root encrypted folder for folder %1").arg(_path)); return; } diff --git a/src/libsync/vfs/cfapi/hydrationjob.cpp b/src/libsync/vfs/cfapi/hydrationjob.cpp index bed4155e7499..2e215ff5776e 100644 --- a/src/libsync/vfs/cfapi/hydrationjob.cpp +++ b/src/libsync/vfs/cfapi/hydrationjob.cpp @@ -370,7 +370,7 @@ void OCC::HydrationJob::handleNewConnectionForEncryptedFile() const auto _remoteParentPath = remotePath.left(remotePath.lastIndexOf('/')); SyncJournalFileRecord rec; - if (!_journal->getRootE2eFolderRecord(_remoteParentPath, &rec) || !rec.isValid()) { + if (!_journal->getRootE2eFolderRecord(Utility::fullRemotePathToRemoteSyncRootRelative(_remoteParentPath, rootPath), &rec) || !rec.isValid()) { emitFinished(Error); return; } diff --git a/test/testutility.cpp b/test/testutility.cpp index 89b54f5d607f..4f41e42faebd 100644 --- a/test/testutility.cpp +++ b/test/testutility.cpp @@ -291,6 +291,54 @@ private slots: QVERIFY(!isPathWindowsDrivePartitionRoot("c:\\")); #endif } + + void testFullRemotePathToRemoteSyncRootRelative() + { + QVector> remoteFullPathsForRoot = { + {"2020", {"2020"}}, + {"/2021/", {"2021"}}, + {"/2022/file.docx", {"2022/file.docx"}} + }; + // test against root remote path - result must stay unchanged, leading and trailing slashes must get removed + for (const auto &remoteFullPathForRoot : remoteFullPathsForRoot) { + const auto fullRemotePathOriginal = remoteFullPathForRoot.first; + const auto fullRemotePathExpected = remoteFullPathForRoot.second; + const auto fullRepotePathResult = OCC::Utility::fullRemotePathToRemoteSyncRootRelative(fullRemotePathOriginal, "/"); + QCOMPARE(fullRepotePathResult, fullRemotePathExpected); + } + + const auto remotePathNonRoot = QStringLiteral("/Documents/reports"); + QVector> remoteFullPathsForNonRoot = { + {remotePathNonRoot + "/" + "2020", {"2020"}}, + {remotePathNonRoot + "/" + "2021/", {"2021"}}, + {remotePathNonRoot + "/" + "2022/file.docx", {"2022/file.docx"}} + }; + + // test against non-root remote path - must always return a proper path as in local db + for (const auto &remoteFullPathForNonRoot : remoteFullPathsForNonRoot) { + const auto fullRemotePathOriginal = remoteFullPathForNonRoot.first; + const auto fullRemotePathExpected = remoteFullPathForNonRoot.second; + const auto fullRepotePathResult = OCC::Utility::fullRemotePathToRemoteSyncRootRelative(fullRemotePathOriginal, remotePathNonRoot); + QCOMPARE(fullRepotePathResult, fullRemotePathExpected); + } + + // test against non-root remote path with trailing slash - must work the same + const auto remotePathNonRootWithTrailingSlash = QStringLiteral("/Documents/reports/"); + for (const auto &remoteFullPathForNonRoot : remoteFullPathsForNonRoot) { + const auto fullRemotePathOriginal = remoteFullPathForNonRoot.first; + const auto fullRemotePathExpected = remoteFullPathForNonRoot.second; + const auto fullRepotePathResult = OCC::Utility::fullRemotePathToRemoteSyncRootRelative(fullRemotePathOriginal, remotePathNonRootWithTrailingSlash); + QCOMPARE(fullRepotePathResult, fullRemotePathExpected); + } + + // test against unrelated remote path - result must stay unchanged + const auto remotePathUnrelated = QStringLiteral("/Documents1/reports"); + for (const auto &remoteFullPathForNonRoot : remoteFullPathsForNonRoot) { + const auto fullRemotePathOriginal = remoteFullPathForNonRoot.first; + const auto fullRepotePathResult = OCC::Utility::fullRemotePathToRemoteSyncRootRelative(fullRemotePathOriginal, remotePathUnrelated); + QCOMPARE(fullRepotePathResult, fullRemotePathOriginal); + } + } }; QTEST_GUILESS_MAIN(TestUtility)