E2EE Fix incorrect name of a nested encrypted item in the Settings dialog when the root folder is non-encrypted and it is renamed.

Signed-off-by: allexzander <blackslayer4@gmail.com>

src/libsync/discovery.cpp

@@ -937,9 +937,9 @@ void ProcessDirectoryJob::processFileAnalyzeLocalInfo(
 
     // If it's not a move it's just a local-NEW
     if (!moveCheck()) {
-        const bool isRootEncryptedFolderRename = base._isE2eEncrypted;
-        if (isRootEncryptedFolderRename) {
-            // if we're renaming the root encrypted folder, we'd need to mark it's NEW item with _isEncrypted true so the further processing will work correctly
+        if (base._isE2eEncrypted) {
+            // renaming the encrypted folder is done via remove + re-upload hence we need to mark the newly created folder as encrypted
+            // base is a record in the SyncJournal database that contains the data about the being-renamed folder with it's old name and encryption information
             item->_isEncrypted = true;
         }
         postProcessLocalNew();

src/libsync/owncloudpropagator.h

@@ -186,6 +186,10 @@ public:
         , _item(item)
         , _parallelism(FullParallelism)
     {
+        // we should always execute jobs that process the E2EE API calls as sequential jobs
+        // TODO: In fact, we must make sure Lock/Unlock are not colliding and always wait for each other to complete. So, we could refactor this "_parallelism" later
+        // so every "PropagateItemJob" that will potentially execute Lock job on E2EE folder will get executed sequentially.
+        // As an alternative, we could optimize Lock/Unlock calls, so we do a batch-write on one folder and only lock and unlock a folder once per batch.
         _parallelism = (_item->_isEncrypted || hasEncryptedAncestor()) ? WaitForFinished : FullParallelism;
     }
     ~PropagateItemJob();

src/libsync/propagateremotemkdir.cpp

@@ -93,7 +93,7 @@ void PropagateRemoteMkdir::slotStartEncryptedMkcolJob(const QString &path, const
 
     auto job = new MkColJob(propagator()->account(),
                             propagator()->fullRemotePath(filename),
-                            {{"e2e-token", _uploadEncryptedHelper->_folderToken }},
+                            {{"e2e-token", _uploadEncryptedHelper->folderToken() }},
                             this);
     connect(job, qOverload<QNetworkReply::NetworkError>(&MkColJob::finished),
             this, &PropagateRemoteMkdir::slotMkcolJobFinished);
@@ -120,6 +120,7 @@ void PropagateRemoteMkdir::finalizeMkColJob(QNetworkReply::NetworkError err, con
 {
     if (_item->_httpErrorCode == 405) {
         // This happens when the directory already exists. Nothing to do.
+        qDebug(lcPropagateRemoteMkdir) << "Folder" << jobPath << "already exists.";
     } else if (err != QNetworkReply::NoError) {
         SyncFileItem::Status status = classifyError(err, _item->_httpErrorCode,
             &propagator()->_anotherSyncNeeded);
@@ -155,7 +156,8 @@ void PropagateRemoteMkdir::finalizeMkColJob(QNetworkReply::NetworkError err, con
     if (!_uploadEncryptedHelper && !_item->_isEncrypted) {
         success();
     } else {
-        // We still need to mark that folder encrypted
+        // We still need to mark that folder encrypted in case we were uploading it as encrypted one
+        // Another scenario, is we are creating a new folder because of move operation on an encrypted folder that works via remove + re-upload
         propagator()->_activeJobList.append(this);
 
         // We're expecting directory path in /Foo/Bar convention...
@@ -214,7 +216,8 @@ void PropagateRemoteMkdir::slotMkcolJobFinished()
 
     const auto jobPath = _job->path();
 
-    if (_uploadEncryptedHelper && !_uploadEncryptedHelper->_folderId.isEmpty()) {
+    if (_uploadEncryptedHelper && _uploadEncryptedHelper->isFolderLocked() && !_uploadEncryptedHelper->isUnlockRunning()) {
+        // since we are done, we need to unlock a folder in case it was locked
         connect(_uploadEncryptedHelper, &PropagateUploadEncrypted::folderUnlocked, this, [this, err, jobHttpReasonPhraseString, jobPath]() {
             finalizeMkColJob(err, jobHttpReasonPhraseString, jobPath);
         });

src/libsync/propagateremotemkdir.h

@@ -60,8 +60,5 @@ private slots:
 
 private:
     void finalizeMkColJob(QNetworkReply::NetworkError err, const QString &jobHttpReasonPhraseString, const QString &jobPath);
-
-private:
-    QPair<SyncFileItem::Status, QString> _status = { SyncFileItem::NoStatus, QString() };
 };
 }

src/libsync/propagateremotemove.cpp

@@ -85,6 +85,34 @@ void PropagateRemoteMove::start()
 
     if (origin == _item->_renameTarget) {
         // The parent has been renamed already so there is nothing more to do.
+
+        if (!_item->_encryptedFileName.isEmpty()) {
+            // when renaming non-encrypted folder that contains encrypted folder, nested files of its encrypted folder are incorrectly displayed in the Settings dialog
+            // encrypted name is displayed instead of a local folder name, unless the sync folder is removed, then added again and re-synced
+            // we are fixing it by modifying the "_encryptedFileName" in such a way so it will have a renamed root path at the beginning of it as expected
+            // corrected "_encryptedFileName" is later used in propagator()->updateMetadata() call that will update the record in the Sync journal DB
+
+            const auto path = _item->_file;
+            const auto slashPosition = path.lastIndexOf('/');
+            const auto parentPath = slashPosition >= 0 ? path.left(slashPosition) : QString();
+
+            SyncJournalFileRecord parentRec;
+            bool ok = propagator()->_journal->getFileRecord(parentPath, &parentRec);
+            if (!ok) {
+                done(SyncFileItem::NormalError);
+                return;
+            }
+
+            const auto remoteParentPath = parentRec._e2eMangledName.isEmpty() ? parentPath : parentRec._e2eMangledName;
+
+            const auto lastSlashPosition = _item->_encryptedFileName.lastIndexOf('/');
+            const auto encryptedName = lastSlashPosition >= 0 ? _item->_encryptedFileName.mid(lastSlashPosition + 1) : QString();
+
+            if (!encryptedName.isEmpty()) {
+                _item->_encryptedFileName = remoteParentPath + "/" + encryptedName;
+            }
+        }
+
         finalize();
         return;
     }

src/libsync/propagateupload.cpp

@@ -237,7 +237,7 @@ void PropagateUploadFileCommon::start()
     _uploadEncryptedHelper = new PropagateUploadEncrypted(propagator(), remoteParentPath, _item, this);
     connect(_uploadEncryptedHelper, &PropagateUploadEncrypted::finalized,
             this, &PropagateUploadFileCommon::setupEncryptedFile);
-    connect(_uploadEncryptedHelper, &PropagateUploadEncrypted::error, [this]{
+    connect(_uploadEncryptedHelper, &PropagateUploadEncrypted::error, [this] {
         qCDebug(lcPropagateUpload) << "Error setting up encryption.";
         done(SyncFileItem::FatalError, tr("Failed to upload encrypted file."));
     });
@@ -416,17 +416,17 @@ void PropagateUploadFileCommon::slotStartUpload(const QByteArray &transmissionCh
 void PropagateUploadFileCommon::slotFolderUnlocked(const QByteArray &folderId, int httpReturnCode)
 {
     qDebug() << "Failed to unlock encrypted folder" << folderId;
-    if (_status.first == SyncFileItem::NoStatus && httpReturnCode != 200) {
+    if (_uploadStatus.status == SyncFileItem::NoStatus && httpReturnCode != 200) {
         done(SyncFileItem::FatalError, tr("Failed to unlock encrypted folder."));
     } else {
-        done(_status.first, _status.second);
+        done(_uploadStatus.status, _uploadStatus.message);
     }
 }
 
 void PropagateUploadFileCommon::slotOnErrorStartFolderUnlock(SyncFileItem::Status status, const QString &errorString)
 {
     if (_uploadingEncrypted) {
-        _status = { status, errorString };
+        _uploadStatus = { status, errorString };
         connect(_uploadEncryptedHelper, &PropagateUploadEncrypted::folderUnlocked, this, &PropagateUploadFileCommon::slotFolderUnlocked);
         _uploadEncryptedHelper->unlockFolder();
     } else {
@@ -750,8 +750,8 @@ QMap<QByteArray, QByteArray> PropagateUploadFileCommon::headers()
             headers[QByteArrayLiteral("OC-ConflictBaseEtag")] = conflictRecord.baseEtag;
     }
 
-    if (_uploadEncryptedHelper && !_uploadEncryptedHelper->_folderToken.isEmpty()) {
-        headers.insert("e2e-token", _uploadEncryptedHelper->_folderToken);
+    if (_uploadEncryptedHelper && !_uploadEncryptedHelper->folderToken().isEmpty()) {
+        headers.insert("e2e-token", _uploadEncryptedHelper->folderToken());
     }
 
     return headers;
@@ -786,7 +786,7 @@ void PropagateUploadFileCommon::finalize()
     propagator()->_journal->commit("upload file start");
 
     if (_uploadingEncrypted) {
-        _status = { SyncFileItem::Success, QString() };
+        _uploadStatus = { SyncFileItem::Success, QString() };
         connect(_uploadEncryptedHelper, &PropagateUploadEncrypted::folderUnlocked, this, &PropagateUploadFileCommon::slotFolderUnlocked);
         _uploadEncryptedHelper->unlockFolder();
     } else {

src/libsync/propagateupload.h

@@ -205,6 +205,11 @@ class PropagateUploadFileCommon : public PropagateItemJob
 {
     Q_OBJECT
 
+    struct UploadStatus {
+        SyncFileItem::Status status = SyncFileItem::NoStatus;
+        QString message;
+    };
+
 protected:
     QVector<AbstractNetworkJob *> _jobs; /// network jobs that are currently in transit
     bool _finished BITFIELD(1); /// Tells that all the jobs have been finished
@@ -314,7 +319,7 @@ protected:
 private:
   PropagateUploadEncrypted *_uploadEncryptedHelper;
   bool _uploadingEncrypted;
-  QPair<SyncFileItem::Status, QString> _status = { SyncFileItem::NoStatus, QString() };
+  UploadStatus _uploadStatus;
 };
 
 /**

src/libsync/propagateuploadencrypted.cpp

@@ -95,6 +95,7 @@ void PropagateUploadEncrypted::slotFolderLockedSuccessfully(const QByteArray& fi
   _currentLockingInProgress = true;
   _folderToken = token;
   _folderId = fileId;
+  _isFolderLocked = true;
 
   auto job = new GetMetadataApiJob(_propagator->account(), _folderId);
   connect(job, &GetMetadataApiJob::jsonReceived,
@@ -178,7 +179,8 @@ void PropagateUploadEncrypted::slotFolderEncryptedMetadataReceived(const QJsonDo
 
       if (!encryptionResult) {
         qCDebug(lcPropagateUploadEncrypted()) << "There was an error encrypting the file, aborting upload.";
-        unlockFolder(true);
+        connect(this, &PropagateUploadEncrypted::folderUnlocked, this, &PropagateUploadEncrypted::error);
+        unlockFolder();
         return;
       }
 
@@ -229,7 +231,8 @@ void PropagateUploadEncrypted::slotUpdateMetadataError(const QByteArray& fileId,
 {
   qCDebug(lcPropagateUploadEncrypted) << "Update metadata error for folder" << fileId << "with error" << httpErrorResponse;
   qCDebug(lcPropagateUploadEncrypted()) << "Unlocking the folder.";
-  unlockFolder(true);
+  connect(this, &PropagateUploadEncrypted::folderUnlocked, this, &PropagateUploadEncrypted::error);
+  unlockFolder();
 }
 
 void PropagateUploadEncrypted::slotFolderLockedError(const QByteArray& fileId, int httpErrorCode)
@@ -262,49 +265,34 @@ void PropagateUploadEncrypted::slotFolderEncryptedIdError(QNetworkReply *r)
     qCDebug(lcPropagateUploadEncrypted) << "Error retrieving the Id of the encrypted folder.";
 }
 
-void PropagateUploadEncrypted::unlockFolder(bool uploadFailed)
+void PropagateUploadEncrypted::unlockFolder()
 {
-    {
-        QMutexLocker locker(&_isUnlockRunningMutex);
+    ASSERT(!_isUnlockRunning);
 
-        ASSERT(!_isUnlockRunning);
-
-        if (_isUnlockRunning) {
-            qWarning() << "Double-call to unlockFolder.";
-            return;
-        }
-
-        _isUnlockRunning = true;
+    if (_isUnlockRunning) {
+        qWarning() << "Double-call to unlockFolder.";
+        return;
     }
 
+    _isUnlockRunning = true;
+
     qDebug() << "Calling Unlock";
     auto *unlockJob = new UnlockEncryptFolderApiJob(_propagator->account(),
         _folderId, _folderToken, this);
 
-    connect(unlockJob, &UnlockEncryptFolderApiJob::success, [this, uploadFailed](const QByteArray &folderId) {
+    connect(unlockJob, &UnlockEncryptFolderApiJob::success, [this](const QByteArray &folderId) {
         qDebug() << "Successfully Unlocked";
         _folderToken = "";
         _folderId = "";
+        _isFolderLocked = false;
 
         emit folderUnlocked(folderId, 200);
-
-        if (uploadFailed) {
-            emit error();
-        }
-
-        QMutexLocker locker(&_isUnlockRunningMutex);
         _isUnlockRunning = false;
     });
-    connect(unlockJob, &UnlockEncryptFolderApiJob::error, [this, uploadFailed](const QByteArray &folderId, int httpStatus) {
+    connect(unlockJob, &UnlockEncryptFolderApiJob::error, [this](const QByteArray &folderId, int httpStatus) {
         qDebug() << "Unlock Error";
 
         emit folderUnlocked(folderId, httpStatus);
-
-        if (uploadFailed) {
-            emit error();
-        }
-
-        QMutexLocker locker(&_isUnlockRunningMutex);
         _isUnlockRunning = false;
     });
     unlockJob->start();

src/libsync/propagateuploadencrypted.h

@@ -33,13 +33,15 @@ class PropagateUploadEncrypted : public QObject
   Q_OBJECT
 public:
     PropagateUploadEncrypted(OwncloudPropagator *propagator, const QString &remoteParentPath, SyncFileItemPtr item, QObject *parent = nullptr);
+    ~PropagateUploadEncrypted() = default;
+
     void start();
 
-    /* unlocks the current folder that holds this file */
-    void unlockFolder(bool uploadFailed = false);
-  // Used by propagateupload
-  QByteArray _folderToken;
-  QByteArray _folderId;
+    void unlockFolder();
+
+    bool isUnlockRunning() const { return _isUnlockRunning; }
+    bool isFolderLocked() const { return _isFolderLocked; }
+    const QByteArray folderToken() const { return _folderToken; }
 
 private slots:
     void slotFolderEncryptedIdReceived(const QStringList &list);
@@ -63,17 +65,20 @@ private:
   QString _remoteParentPath;
   SyncFileItemPtr _item;
 
+  QByteArray _folderToken;
+  QByteArray _folderId;
+
   QElapsedTimer _folderLockFirstTry;
   bool _currentLockingInProgress = false;
 
+  bool _isUnlockRunning = false;
+  bool _isFolderLocked = false;
+
   QByteArray _generatedKey;
   QByteArray _generatedIv;
   FolderMetadata *_metadata;
   EncryptedFile _encryptedFile;
   QString _completeFileName;
-
-  bool _isUnlockRunning = false; // protection against multiple calls to unlock the same folder
-  QMutex _isUnlockRunningMutex; // corresponding mutex for _isUnlockRunning
 };