Merge pull request #2853 from nextcloud/e2ee-fix-rename-root-folder-issue

E2ee fix rename root folder issue & Fix upload stuck issue due to incorrect Lock/Unlock sequence
This commit is contained in:
allexzander 2021-01-25 10:44:25 +02:00 committed by GitHub
commit bc753d5fba
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
11 changed files with 199 additions and 111 deletions

View file

@ -937,6 +937,11 @@ void ProcessDirectoryJob::processFileAnalyzeLocalInfo(
// If it's not a move it's just a local-NEW
if (!moveCheck()) {
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();
finalize();
return;

View file

@ -178,12 +178,19 @@ protected slots:
private:
QScopedPointer<PropagateItemJob> _restoreJob;
JobParallelism _parallelism;
public:
PropagateItemJob(OwncloudPropagator *propagator, const SyncFileItemPtr &item)
: PropagatorJob(propagator)
, _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();
@ -199,6 +206,8 @@ public:
return true;
}
virtual JobParallelism parallelism() override { return _parallelism; }
SyncFileItemPtr _item;
public slots:

View file

@ -26,11 +26,6 @@ namespace OCC {
Q_LOGGING_CATEGORY(lcPropagateRemoteDelete, "nextcloud.sync.propagator.remotedelete", QtInfoMsg)
PropagatorJob::JobParallelism PropagateRemoteDelete::parallelism()
{
return _item->_encryptedFileName.isEmpty() ? FullParallelism : WaitForFinished;
}
void PropagateRemoteDelete::start()
{
if (propagator()->_abortRequested)

View file

@ -37,7 +37,6 @@ public:
: PropagateItemJob(propagator, item)
{
}
JobParallelism parallelism() override;
void start() override;
void createDeleteJob(const QString &filename);
void abort(PropagatorJob::AbortType abortType) override;

View file

@ -32,7 +32,6 @@ PropagateRemoteMkdir::PropagateRemoteMkdir(OwncloudPropagator *propagator, const
: PropagateItemJob(propagator, item)
, _deleteExisting(false)
, _uploadEncryptedHelper(nullptr)
, _parallelism(FullParallelism)
{
const auto path = _item->_file;
const auto slashPosition = path.lastIndexOf('/');
@ -43,15 +42,6 @@ PropagateRemoteMkdir::PropagateRemoteMkdir(OwncloudPropagator *propagator, const
if (!ok) {
return;
}
if (hasEncryptedAncestor()) {
_parallelism = WaitForFinished;
}
}
PropagatorJob::JobParallelism PropagateRemoteMkdir::parallelism()
{
return _parallelism;
}
void PropagateRemoteMkdir::start()
@ -103,10 +93,8 @@ 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),
_uploadEncryptedHelper, &PropagateUploadEncrypted::unlockFolder);
connect(job, qOverload<QNetworkReply::NetworkError>(&MkColJob::finished),
this, &PropagateRemoteMkdir::slotMkcolJobFinished);
_job = job;
@ -128,6 +116,59 @@ void PropagateRemoteMkdir::setDeleteExisting(bool enabled)
_deleteExisting = enabled;
}
void PropagateRemoteMkdir::finalizeMkColJob(QNetworkReply::NetworkError err, const QString &jobHttpReasonPhraseString, const QString &jobPath)
{
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);
done(status, _item->_errorString);
return;
} else if (_item->_httpErrorCode != 201) {
// Normally we expect "201 Created"
// If it is not the case, it might be because of a proxy or gateway intercepting the request, so we must
// throw an error.
done(SyncFileItem::NormalError,
tr("Wrong HTTP code returned by server. Expected 201, but received \"%1 %2\".")
.arg(_item->_httpErrorCode)
.arg(jobHttpReasonPhraseString));
return;
}
if (_item->_fileId.isEmpty()) {
// Owncloud 7.0.0 and before did not have a header with the file id.
// (https://github.com/owncloud/core/issues/9000)
// So we must get the file id using a PROPFIND
// This is required so that we can detect moves even if the folder is renamed on the server
// while files are still uploading
propagator()->_activeJobList.append(this);
auto propfindJob = new PropfindJob(propagator()->account(), jobPath, this);
propfindJob->setProperties(QList<QByteArray>() << "http://owncloud.org/ns:id");
QObject::connect(propfindJob, &PropfindJob::result, this, &PropagateRemoteMkdir::propfindResult);
QObject::connect(propfindJob, &PropfindJob::finishedWithError, this, &PropagateRemoteMkdir::propfindError);
propfindJob->start();
_job = propfindJob;
return;
}
if (!_uploadEncryptedHelper && !_item->_isEncrypted) {
success();
} else {
// 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...
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, this);
connect(job, &OCC::EncryptFolderJob::finished, this, &PropagateRemoteMkdir::slotEncryptFolderFinished);
job->start();
}
}
void PropagateRemoteMkdir::slotMkdir()
{
const auto path = _item->_file;
@ -167,56 +208,22 @@ void PropagateRemoteMkdir::slotMkcolJobFinished()
_item->_responseTimeStamp = _job->responseTimestamp();
_item->_requestId = _job->requestId();
if (_item->_httpErrorCode == 405) {
// This happens when the directory already exists. Nothing to do.
} else if (err != QNetworkReply::NoError) {
SyncFileItem::Status status = classifyError(err, _item->_httpErrorCode,
&propagator()->_anotherSyncNeeded);
done(status, _job->errorString());
return;
} else if (_item->_httpErrorCode != 201) {
// Normally we expect "201 Created"
// If it is not the case, it might be because of a proxy or gateway intercepting the request, so we must
// throw an error.
done(SyncFileItem::NormalError,
tr("Wrong HTTP code returned by server. Expected 201, but received \"%1 %2\".")
.arg(_item->_httpErrorCode)
.arg(_job->reply()->attribute(QNetworkRequest::HttpReasonPhraseAttribute).toString()));
return;
}
_item->_fileId = _job->reply()->rawHeader("OC-FileId");
if (_item->_fileId.isEmpty()) {
// Owncloud 7.0.0 and before did not have a header with the file id.
// (https://github.com/owncloud/core/issues/9000)
// So we must get the file id using a PROPFIND
// This is required so that we can detect moves even if the folder is renamed on the server
// while files are still uploading
propagator()->_activeJobList.append(this);
auto propfindJob = new PropfindJob(_job->account(), _job->path(), this);
propfindJob->setProperties(QList<QByteArray>() << "http://owncloud.org/ns:id");
QObject::connect(propfindJob, &PropfindJob::result, this, &PropagateRemoteMkdir::propfindResult);
QObject::connect(propfindJob, &PropfindJob::finishedWithError, this, &PropagateRemoteMkdir::propfindError);
propfindJob->start();
_job = propfindJob;
return;
}
_item->_errorString = _job->errorString();
if (!_uploadEncryptedHelper) {
success();
const auto jobHttpReasonPhraseString = _job->reply()->attribute(QNetworkRequest::HttpReasonPhraseAttribute).toString();
const auto jobPath = _job->path();
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);
});
_uploadEncryptedHelper->unlockFolder();
} else {
// We still need to mark that folder encrypted
propagator()->_activeJobList.append(this);
// We're expecting directory path in /Foo/Bar convention...
Q_ASSERT(_job->path().startsWith('/') && !_job->path().endsWith('/'));
// But encryption job expect it in Foo/Bar/ convention
const auto path = _job->path().mid(1);
auto job = new OCC::EncryptFolderJob(propagator()->account(), propagator()->_journal, path, _item->_fileId, this);
connect(job, &OCC::EncryptFolderJob::finished, this, &PropagateRemoteMkdir::slotEncryptFolderFinished);
job->start();
finalizeMkColJob(err, jobHttpReasonPhraseString, jobPath);
}
}

View file

@ -30,13 +30,10 @@ class PropagateRemoteMkdir : public PropagateItemJob
QPointer<AbstractNetworkJob> _job;
bool _deleteExisting;
PropagateUploadEncrypted *_uploadEncryptedHelper;
JobParallelism _parallelism;
friend class PropagateDirectory; // So it can access the _item;
public:
PropagateRemoteMkdir(OwncloudPropagator *propagator, const SyncFileItemPtr &item);
JobParallelism parallelism() override;
void start() override;
void abort(PropagatorJob::AbortType abortType) override;
@ -60,5 +57,8 @@ private slots:
void propfindResult(const QVariantMap &);
void propfindError();
void success();
private:
void finalizeMkColJob(QNetworkReply::NetworkError err, const QString &jobHttpReasonPhraseString, const QString &jobPath);
};
}

View file

@ -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;
}

View file

@ -192,7 +192,6 @@ PropagateUploadFileCommon::PropagateUploadFileCommon(OwncloudPropagator *propaga
, _finished(false)
, _deleteExisting(false)
, _aborting(false)
, _parallelism(FullParallelism)
, _uploadEncryptedHelper(nullptr)
, _uploadingEncrypted(false)
{
@ -205,15 +204,6 @@ PropagateUploadFileCommon::PropagateUploadFileCommon(OwncloudPropagator *propaga
if (!ok) {
return;
}
if (hasEncryptedAncestor()) {
_parallelism = WaitForFinished;
}
}
PropagatorJob::JobParallelism PropagateUploadFileCommon::parallelism()
{
return _parallelism;
}
void PropagateUploadFileCommon::setDeleteExisting(bool enabled)
@ -247,8 +237,10 @@ void PropagateUploadFileCommon::start()
_uploadEncryptedHelper = new PropagateUploadEncrypted(propagator(), remoteParentPath, _item, this);
connect(_uploadEncryptedHelper, &PropagateUploadEncrypted::finalized,
this, &PropagateUploadFileCommon::setupEncryptedFile);
connect(_uploadEncryptedHelper, &PropagateUploadEncrypted::error,
[]{ qCDebug(lcPropagateUpload) << "Error setting up encryption."; });
connect(_uploadEncryptedHelper, &PropagateUploadEncrypted::error, [this] {
qCDebug(lcPropagateUpload) << "Error setting up encryption.";
done(SyncFileItem::FatalError, tr("Failed to upload encrypted file."));
});
_uploadEncryptedHelper->start();
}
@ -394,11 +386,7 @@ void PropagateUploadFileCommon::slotStartUpload(const QByteArray &transmissionCh
const QString originalFilePath = propagator()->fullLocalPath(_item->_file);
if (!FileSystem::fileExists(fullFilePath)) {
if (_uploadingEncrypted) {
_uploadEncryptedHelper->unlockFolder();
}
done(SyncFileItem::SoftError, tr("File Removed (start upload) %1").arg(fullFilePath));
return;
return slotOnErrorStartFolderUnlock(SyncFileItem::SoftError, tr("File Removed (start upload) %1").arg(fullFilePath));
}
time_t prevModtime = _item->_modtime; // the _item value was set in PropagateUploadFile::start()
// but a potential checksum calculation could have taken some time during which the file could
@ -407,12 +395,8 @@ void PropagateUploadFileCommon::slotStartUpload(const QByteArray &transmissionCh
_item->_modtime = FileSystem::getModTime(originalFilePath);
if (prevModtime != _item->_modtime) {
propagator()->_anotherSyncNeeded = true;
if (_uploadingEncrypted) {
_uploadEncryptedHelper->unlockFolder();
}
qDebug() << "prevModtime" << prevModtime << "Curr" << _item->_modtime;
done(SyncFileItem::SoftError, tr("Local file changed during syncing. It will be resumed."));
return;
return slotOnErrorStartFolderUnlock(SyncFileItem::SoftError, tr("Local file changed during syncing. It will be resumed."));
}
_fileToUpload._size = FileSystem::getSize(fullFilePath);
@ -423,16 +407,33 @@ void PropagateUploadFileCommon::slotStartUpload(const QByteArray &transmissionCh
// or not yet fully copied to the destination.
if (fileIsStillChanging(*_item)) {
propagator()->_anotherSyncNeeded = true;
if (_uploadingEncrypted) {
_uploadEncryptedHelper->unlockFolder();
}
done(SyncFileItem::SoftError, tr("Local file changed during sync."));
return;
return slotOnErrorStartFolderUnlock(SyncFileItem::SoftError, tr("Local file changed during sync."));
}
doStartUpload();
}
void PropagateUploadFileCommon::slotFolderUnlocked(const QByteArray &folderId, int httpReturnCode)
{
qDebug() << "Failed to unlock encrypted folder" << folderId;
if (_uploadStatus.status == SyncFileItem::NoStatus && httpReturnCode != 200) {
done(SyncFileItem::FatalError, tr("Failed to unlock encrypted folder."));
} else {
done(_uploadStatus.status, _uploadStatus.message);
}
}
void PropagateUploadFileCommon::slotOnErrorStartFolderUnlock(SyncFileItem::Status status, const QString &errorString)
{
if (_uploadingEncrypted) {
_uploadStatus = { status, errorString };
connect(_uploadEncryptedHelper, &PropagateUploadEncrypted::folderUnlocked, this, &PropagateUploadFileCommon::slotFolderUnlocked);
_uploadEncryptedHelper->unlockFolder();
} else {
done(status, errorString);
}
}
UploadDevice::UploadDevice(const QString &fileName, qint64 start, qint64 size, BandwidthManager *bwm)
: _file(fileName)
, _start(start)
@ -749,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;
@ -785,10 +786,13 @@ void PropagateUploadFileCommon::finalize()
propagator()->_journal->commit("upload file start");
if (_uploadingEncrypted) {
_uploadStatus = { SyncFileItem::Success, QString() };
connect(_uploadEncryptedHelper, &PropagateUploadEncrypted::folderUnlocked, this, &PropagateUploadFileCommon::slotFolderUnlocked);
_uploadEncryptedHelper->unlockFolder();
}
} else {
done(SyncFileItem::Success);
}
}
void PropagateUploadFileCommon::abortNetworkJobs(
PropagatorJob::AbortType abortType,

View file

@ -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
@ -231,13 +236,10 @@ protected:
};
UploadFileInfo _fileToUpload;
QByteArray _transmissionChecksumHeader;
JobParallelism _parallelism;
public:
PropagateUploadFileCommon(OwncloudPropagator *propagator, const SyncFileItemPtr &item);
JobParallelism parallelism() override;
/**
* Whether an existing entity with the same name may be deleted before
* the upload.
@ -260,6 +262,10 @@ private slots:
void slotComputeTransmissionChecksum(const QByteArray &contentChecksumType, const QByteArray &contentChecksum);
// transmission checksum computed, prepare the upload
void slotStartUpload(const QByteArray &transmissionChecksumType, const QByteArray &transmissionChecksum);
// invoked when encrypted folder lock has been released
void slotFolderUnlocked(const QByteArray &folderId, int httpReturnCode);
// invoked on internal error to unlock a folder and faile
void slotOnErrorStartFolderUnlock(SyncFileItem::Status status, const QString &errorString);
public:
virtual void doStartUpload() = 0;
@ -313,6 +319,7 @@ protected:
private:
PropagateUploadEncrypted *_uploadEncryptedHelper;
bool _uploadingEncrypted;
UploadStatus _uploadStatus;
};
/**

View file

@ -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,6 +179,7 @@ void PropagateUploadEncrypted::slotFolderEncryptedMetadataReceived(const QJsonDo
if (!encryptionResult) {
qCDebug(lcPropagateUploadEncrypted()) << "There was an error encrypting the file, aborting upload.";
connect(this, &PropagateUploadEncrypted::folderUnlocked, this, &PropagateUploadEncrypted::error);
unlockFolder();
return;
}
@ -229,6 +231,7 @@ void PropagateUploadEncrypted::slotUpdateMetadataError(const QByteArray& fileId,
{
qCDebug(lcPropagateUploadEncrypted) << "Update metadata error for folder" << fileId << "with error" << httpErrorResponse;
qCDebug(lcPropagateUploadEncrypted()) << "Unlocking the folder.";
connect(this, &PropagateUploadEncrypted::folderUnlocked, this, &PropagateUploadEncrypted::error);
unlockFolder();
}
@ -264,12 +267,34 @@ void PropagateUploadEncrypted::slotFolderEncryptedIdError(QNetworkReply *r)
void PropagateUploadEncrypted::unlockFolder()
{
ASSERT(!_isUnlockRunning);
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, []{ qDebug() << "Successfully Unlocked"; });
connect(unlockJob, &UnlockEncryptFolderApiJob::error, []{ qDebug() << "Unlock Error"; });
connect(unlockJob, &UnlockEncryptFolderApiJob::success, [this](const QByteArray &folderId) {
qDebug() << "Successfully Unlocked";
_folderToken = "";
_folderId = "";
_isFolderLocked = false;
emit folderUnlocked(folderId, 200);
_isUnlockRunning = false;
});
connect(unlockJob, &UnlockEncryptFolderApiJob::error, [this](const QByteArray &folderId, int httpStatus) {
qDebug() << "Unlock Error";
emit folderUnlocked(folderId, httpStatus);
_isUnlockRunning = false;
});
unlockJob->start();
}

View file

@ -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();
// Used by propagateupload
QByteArray _folderToken;
QByteArray _folderId;
bool isUnlockRunning() const { return _isUnlockRunning; }
bool isFolderLocked() const { return _isFolderLocked; }
const QByteArray folderToken() const { return _folderToken; }
private slots:
void slotFolderEncryptedIdReceived(const QStringList &list);
@ -56,15 +58,22 @@ signals:
// Emmited after the file is encrypted and everythign is setup.
void finalized(const QString& path, const QString& filename, quint64 size);
void error();
void folderUnlocked(const QByteArray &folderId, int httpStatus);
private:
OwncloudPropagator *_propagator;
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;