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

Bugfix/e2ee vulnerability empty metadatakeys #5323

Merged
merged 4 commits into from
Jan 18, 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
51 changes: 31 additions & 20 deletions src/libsync/clientsideencryption.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1419,6 +1419,12 @@ void FolderMetadata::setupExistingMetadata(const QByteArray& metadata)
QJsonDocument metaDataDoc = QJsonDocument::fromJson(metaDataStr.toLocal8Bit());
QJsonObject metadataObj = metaDataDoc.object()["metadata"].toObject();
QJsonObject metadataKeys = metadataObj["metadataKeys"].toObject();

if (metadataKeys.isEmpty()) {
qCDebug(lcCse()) << "Could not setup existing metadata with missing metadataKeys!";
return;
}

QByteArray sharing = metadataObj["sharing"].toString().toLocal8Bit();
QJsonObject files = metaDataDoc.object()["files"].toObject();

Expand Down Expand Up @@ -1447,14 +1453,19 @@ void FolderMetadata::setupExistingMetadata(const QByteArray& metadata)
// Cool, We actually have the key, we can decrypt the rest of the metadata.
qCDebug(lcCse) << "Sharing: " << sharing;
if (sharing.size()) {
auto sharingDecrypted = decryptJsonObject(sharing, _metadataKeys.last());
qCDebug(lcCse) << "Sharing Decrypted" << sharingDecrypted;

//Sharing is also a JSON object, so extract it and populate.
auto sharingDoc = QJsonDocument::fromJson(sharingDecrypted);
auto sharingObj = sharingDoc.object();
for (auto it = sharingObj.constBegin(), end = sharingObj.constEnd(); it != end; it++) {
_sharing.push_back({it.key(), it.value().toString()});
const auto metaDataKey = !_metadataKeys.isEmpty() ? _metadataKeys.last() : QByteArray{};
if (metaDataKey.isEmpty()) {
qCDebug(lcCse) << "Failed to decrypt sharing! Empty metadata key!";
} else {
auto sharingDecrypted = decryptJsonObject(sharing, metaDataKey);
qCDebug(lcCse) << "Sharing Decrypted" << sharingDecrypted;

// Sharing is also a JSON object, so extract it and populate.
auto sharingDoc = QJsonDocument::fromJson(sharingDecrypted);
auto sharingObj = sharingDoc.object();
for (auto it = sharingObj.constBegin(), end = sharingObj.constEnd(); it != end; it++) {
_sharing.push_back({it.key(), it.value().toString()});
}
}
} else {
qCDebug(lcCse) << "Skipping sharing section since it is empty";
Expand All @@ -1470,9 +1481,9 @@ void FolderMetadata::setupExistingMetadata(const QByteArray& metadata)
file.initializationVector = QByteArray::fromBase64(fileObj["initializationVector"].toString().toLocal8Bit());

//Decrypt encrypted part
QByteArray key = _metadataKeys[file.metadataKey];
const auto key = _metadataKeys.value(file.metadataKey, {});
auto encryptedFile = fileObj["encrypted"].toString().toLocal8Bit();
auto decryptedFile = decryptJsonObject(encryptedFile, key);
auto decryptedFile = !key.isEmpty() ? decryptJsonObject(encryptedFile, key) : QByteArray{};
auto decryptedFileDoc = QJsonDocument::fromJson(decryptedFile);
auto decryptedFileObj = decryptedFileDoc.object();

Expand Down Expand Up @@ -1532,6 +1543,11 @@ QByteArray FolderMetadata::decryptJsonObject(const QByteArray& encryptedMetadata
return EncryptionHelper::decryptStringSymmetric(pass, encryptedMetadata);
}

bool FolderMetadata::isMetadataSetup() const
{
return !_metadataKeys.isEmpty();
}

void FolderMetadata::setupEmptyMetadata() {
qCDebug(lcCse) << "Settint up empty metadata";
QByteArray newMetadataPass = EncryptionHelper::generateRandom(16);
Expand All @@ -1546,6 +1562,11 @@ void FolderMetadata::setupEmptyMetadata() {
QByteArray FolderMetadata::encryptedMetadata() {
qCDebug(lcCse) << "Generating metadata";

if (_metadataKeys.isEmpty()) {
qCDebug(lcCse) << "Metadata generation failed! Empty metadata key!";
return {};
}

QJsonObject metadataKeys;
for (auto it = _metadataKeys.constBegin(), end = _metadataKeys.constEnd(); it != end; it++) {
/*
Expand All @@ -1556,16 +1577,6 @@ QByteArray FolderMetadata::encryptedMetadata() {
metadataKeys.insert(QString::number(it.key()), QString(encryptedKey));
}

/* NO SHARING IN V1
QJsonObject recepients;
for (auto it = _sharing.constBegin(), end = _sharing.constEnd(); it != end; it++) {
recepients.insert(it->first, it->second);
}
QJsonDocument recepientDoc;
recepientDoc.setObject(recepients);
QString sharingEncrypted = encryptJsonObject(recepientDoc.toJson(QJsonDocument::Compact), _metadataKeys.last());
*/

QJsonObject metadata = {
{"metadataKeys", metadataKeys},
// {"sharing", sharingEncrypted},
Expand Down
1 change: 1 addition & 0 deletions src/libsync/clientsideencryption.h
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,7 @@ class OWNCLOUDSYNC_EXPORT FolderMetadata {
void removeEncryptedFile(const EncryptedFile& f);
void removeAllEncryptedFiles();
[[nodiscard]] QVector<EncryptedFile> files() const;
[[nodiscard]] bool isMetadataSetup() const;


private:
Expand Down
26 changes: 14 additions & 12 deletions src/libsync/propagatedownloadencrypted.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -73,18 +73,20 @@ void PropagateDownloadEncrypted::checkFolderEncryptedMetadata(const QJsonDocumen
qCDebug(lcPropagateDownloadEncrypted) << "Metadata Received reading"
<< _item->_instruction << _item->_file << _item->_encryptedFileName;
const QString filename = _info.fileName();
auto meta = new FolderMetadata(_propagator->account(), json.toJson(QJsonDocument::Compact));
const QVector<EncryptedFile> files = meta->files();

const QString encryptedFilename = _item->_encryptedFileName.section(QLatin1Char('/'), -1);
for (const EncryptedFile &file : files) {
if (encryptedFilename == file.encryptedFilename) {
_encryptedInfo = file;

qCDebug(lcPropagateDownloadEncrypted) << "Found matching encrypted metadata for file, starting download";
emit fileMetadataFound();
return;
}
const FolderMetadata metadata(_propagator->account(), json.toJson(QJsonDocument::Compact));
if (metadata.isMetadataSetup()) {
const QVector<EncryptedFile> files = metadata.files();

const QString encryptedFilename = _item->_encryptedFileName.section(QLatin1Char('/'), -1);
for (const EncryptedFile &file : files) {
if (encryptedFilename == file.encryptedFilename) {
_encryptedInfo = file;

qCDebug(lcPropagateDownloadEncrypted) << "Found matching encrypted metadata for file, starting download";
emit fileMetadataFound();
return;
}
}
}

emit failed();
Expand Down
5 changes: 5 additions & 0 deletions src/libsync/propagateremotedeleteencrypted.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,11 @@ void PropagateRemoteDeleteEncrypted::slotFolderEncryptedMetadataReceived(const Q

FolderMetadata metadata(_propagator->account(), json.toJson(QJsonDocument::Compact), statusCode);

if (!metadata.isMetadataSetup()) {
taskFailed();
return;
}

qCDebug(PROPAGATE_REMOVE_ENCRYPTED) << "Metadata Received, preparing it for removal of the file";

const QFileInfo info(_propagator->fullLocalPath(_item->_file));
Expand Down
5 changes: 5 additions & 0 deletions src/libsync/propagateremotedeleteencryptedrootfolder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,11 @@ void PropagateRemoteDeleteEncryptedRootFolder::slotFolderEncryptedMetadataReceiv

FolderMetadata metadata(_propagator->account(), json.toJson(QJsonDocument::Compact), statusCode);

if (!metadata.isMetadataSetup()) {
taskFailed();
return;
}

qCDebug(PROPAGATE_REMOVE_ENCRYPTED_ROOTFOLDER) << "It's a root encrypted folder. Let's remove nested items first.";

metadata.removeAllEncryptedFiles();
Expand Down
12 changes: 11 additions & 1 deletion src/libsync/propagateuploadencrypted.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,17 @@ void PropagateUploadEncrypted::slotFolderEncryptedMetadataReceived(const QJsonDo
qCDebug(lcPropagateUploadEncrypted) << "Metadata Received, Preparing it for the new file." << json.toVariant();

// Encrypt File!
_metadata = new FolderMetadata(_propagator->account(), json.toJson(QJsonDocument::Compact), statusCode);
_metadata.reset(new FolderMetadata(_propagator->account(), json.toJson(QJsonDocument::Compact), statusCode));

if (!_metadata->isMetadataSetup()) {
if (_isFolderLocked) {
connect(this, &PropagateUploadEncrypted::folderUnlocked, this, &PropagateUploadEncrypted::error);
unlockFolder();
} else {
emit error();
}
return;
}

QFileInfo info(_propagator->fullLocalPath(_item->_file));
const QString fileName = info.fileName();
Expand Down
3 changes: 2 additions & 1 deletion src/libsync/propagateuploadencrypted.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#include <QByteArray>
#include <QJsonDocument>
#include <QNetworkReply>
#include <QScopedPointer>
#include <QFile>
#include <QTemporaryFile>

Expand Down Expand Up @@ -76,7 +77,7 @@ private slots:

QByteArray _generatedKey;
QByteArray _generatedIv;
FolderMetadata *_metadata;
QScopedPointer<FolderMetadata> _metadata;
EncryptedFile _encryptedFile;
QString _completeFileName;
};
Expand Down
31 changes: 17 additions & 14 deletions src/libsync/vfs/cfapi/hydrationjob.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -199,24 +199,27 @@ void OCC::HydrationJob::slotCheckFolderEncryptedMetadata(const QJsonDocument &js
// TODO: the following code is borrowed from PropagateDownloadEncrypted (see HydrationJob::onNewConnection() for explanation of next steps)
qCDebug(lcHydration) << "Metadata Received reading" << e2eMangledName();
const QString filename = e2eMangledName();
auto meta = new FolderMetadata(_account, json.toJson(QJsonDocument::Compact));
const QVector<EncryptedFile> files = meta->files();
const FolderMetadata metadata(_account, json.toJson(QJsonDocument::Compact));

EncryptedFile encryptedInfo = {};
if (metadata.isMetadataSetup()) {
const QVector<EncryptedFile> files = metadata.files();

const QString encryptedFileExactName = e2eMangledName().section(QLatin1Char('/'), -1);
for (const EncryptedFile &file : files) {
if (encryptedFileExactName == file.encryptedFilename) {
EncryptedFile encryptedInfo = file;
encryptedInfo = file;
EncryptedFile encryptedInfo = {};

qCDebug(lcHydration) << "Found matching encrypted metadata for file, starting download" << _requestId << _folderPath;
_transferDataSocket = _transferDataServer->nextPendingConnection();
_job = new GETEncryptedFileJob(_account, _remotePath + e2eMangledName(), _transferDataSocket, {}, {}, 0, encryptedInfo, this);
const QString encryptedFileExactName = e2eMangledName().section(QLatin1Char('/'), -1);
for (const EncryptedFile &file : files) {
if (encryptedFileExactName == file.encryptedFilename) {
EncryptedFile encryptedInfo = file;
encryptedInfo = file;

connect(qobject_cast<GETEncryptedFileJob *>(_job), &GETEncryptedFileJob::finishedSignal, this, &HydrationJob::onGetFinished);
_job->start();
return;
qCDebug(lcHydration) << "Found matching encrypted metadata for file, starting download" << _requestId << _folderPath;
_transferDataSocket = _transferDataServer->nextPendingConnection();
_job = new GETEncryptedFileJob(_account, _remotePath + e2eMangledName(), _transferDataSocket, {}, {}, 0, encryptedInfo, this);

connect(qobject_cast<GETEncryptedFileJob *>(_job), &GETEncryptedFileJob::finishedSignal, this, &HydrationJob::onGetFinished);
_job->start();
return;
}
}
}

Expand Down