VFS + E2EE. Improved data alignment and unit tests for StreamingDecryptor. Refactoring and renaming for readability.

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

src/common/constants.h

@@ -14,8 +14,10 @@
 
 #pragma once
 
+#include <QtGlobal>
+
 namespace OCC {
-namespace CommonConstants {
-    const qint64 e2EeTagSize = 16;
+namespace Constants {
+    constexpr qint32 e2EeTagSize = 16;
 };
 };

src/libsync/clientsideencryption.cpp

@@ -67,7 +67,7 @@ namespace {
     const char e2e_private[] = "_e2e-private";
     const char e2e_mnemonic[] = "_e2e-mnemonic";
 
-    const int blockSize = 1024;
+    constexpr qint64 blockSize = 1024;
 
     QList<QByteArray> oldCipherFormatSplit(const QByteArray &cipher)
     {
@@ -298,7 +298,7 @@ namespace {
     };
 
     QByteArray BIO2ByteArray(Bio &b) {
-        int pending = static_cast<int>(BIO_ctrl_pending(b));
+        int pending = BIO_ctrl_pending(b);
         QByteArray res(pending, '\0');
         BIO_read(b, unsignedData(res), pending);
         return res;
@@ -426,17 +426,17 @@ QByteArray encryptPrivateKey(
     }
     clen += len;
 
-    /* Get the tag */
-    QByteArray tag(OCC::CommonConstants::e2EeTagSize, '\0');
-    if(1 != EVP_CIPHER_CTX_ctrl(ctx, EVP_CTRL_GCM_GET_TAG, OCC::CommonConstants::e2EeTagSize, unsignedData(tag))) {
-        qCInfo(lcCse()) << "Error getting the tag";
+    /* Get the e2EeTag */
+    QByteArray e2EeTag(OCC::Constants::e2EeTagSize, '\0');
+    if(1 != EVP_CIPHER_CTX_ctrl(ctx, EVP_CTRL_GCM_GET_TAG, OCC::Constants::e2EeTagSize, unsignedData(e2EeTag))) {
+        qCInfo(lcCse()) << "Error getting the e2EeTag";
         handleErrors();
     }
 
     QByteArray cipherTXT;
-    cipherTXT.reserve(clen + 16);
+    cipherTXT.reserve(clen + OCC::Constants::e2EeTagSize);
     cipherTXT.append(ctext, clen);
-    cipherTXT.append(tag);
+    cipherTXT.append(e2EeTag);
 
     QByteArray result = cipherTXT.toBase64();
     result += '|';
@@ -466,8 +466,8 @@ QByteArray decryptPrivateKey(const QByteArray& key, const QByteArray& data) {
     QByteArray cipherTXT = QByteArray::fromBase64(cipherTXT64);
     QByteArray iv = QByteArray::fromBase64(ivB64);
 
-    QByteArray tag = cipherTXT.right(OCC::CommonConstants::e2EeTagSize);
-    cipherTXT.chop(OCC::CommonConstants::e2EeTagSize);
+    const QByteArray e2EeTag = cipherTXT.right(OCC::Constants::e2EeTagSize);
+    cipherTXT.chop(OCC::Constants::e2EeTagSize);
 
     // Init
     CipherCtx ctx;
@@ -496,7 +496,7 @@ QByteArray decryptPrivateKey(const QByteArray& key, const QByteArray& data) {
         return QByteArray();
     }
 
-    QByteArray ptext(cipherTXT.size() + OCC::CommonConstants::e2EeTagSize, '\0');
+    QByteArray ptext(cipherTXT.size() + OCC::Constants::e2EeTagSize, '\0');
     int plen = 0;
 
     /* Provide the message to be decrypted, and obtain the plaintext output.
@@ -507,9 +507,9 @@ QByteArray decryptPrivateKey(const QByteArray& key, const QByteArray& data) {
         return QByteArray();
     }
 
-    /* Set expected tag value. Works in OpenSSL 1.0.1d and later */
-    if(!EVP_CIPHER_CTX_ctrl(ctx, EVP_CTRL_GCM_SET_TAG, tag.size(), (unsigned char *)tag.constData())) {
-        qCInfo(lcCse()) << "Could not set tag";
+    /* Set expected e2EeTag value. Works in OpenSSL 1.0.1d and later */
+    if(!EVP_CIPHER_CTX_ctrl(ctx, EVP_CTRL_GCM_SET_TAG, e2EeTag.size(), (unsigned char *)e2EeTag.constData())) {
+        qCInfo(lcCse()) << "Could not set e2EeTag";
         return QByteArray();
     }
 
@@ -556,8 +556,8 @@ QByteArray decryptStringSymmetric(const QByteArray& key, const QByteArray& data)
     QByteArray cipherTXT = QByteArray::fromBase64(cipherTXT64);
     QByteArray iv = QByteArray::fromBase64(ivB64);
 
-    QByteArray tag = cipherTXT.right(OCC::CommonConstants::e2EeTagSize);
-    cipherTXT.chop(OCC::CommonConstants::e2EeTagSize);
+    const QByteArray e2EeTag = cipherTXT.right(OCC::Constants::e2EeTagSize);
+    cipherTXT.chop(OCC::Constants::e2EeTagSize);
 
     // Init
     CipherCtx ctx;
@@ -586,7 +586,7 @@ QByteArray decryptStringSymmetric(const QByteArray& key, const QByteArray& data)
         return QByteArray();
     }
 
-    QByteArray ptext(cipherTXT.size() + OCC::CommonConstants::e2EeTagSize, '\0');
+    QByteArray ptext(cipherTXT.size() + OCC::Constants::e2EeTagSize, '\0');
     int plen = 0;
 
     /* Provide the message to be decrypted, and obtain the plaintext output.
@@ -597,9 +597,9 @@ QByteArray decryptStringSymmetric(const QByteArray& key, const QByteArray& data)
         return QByteArray();
     }
 
-    /* Set expected tag value. Works in OpenSSL 1.0.1d and later */
-    if(!EVP_CIPHER_CTX_ctrl(ctx, EVP_CTRL_GCM_SET_TAG, tag.size(), (unsigned char *)tag.constData())) {
-        qCInfo(lcCse()) << "Could not set tag";
+    /* Set expected e2EeTag value. Works in OpenSSL 1.0.1d and later */
+    if(!EVP_CIPHER_CTX_ctrl(ctx, EVP_CTRL_GCM_SET_TAG, e2EeTag.size(), (unsigned char *)e2EeTag.constData())) {
+        qCInfo(lcCse()) << "Could not set e2EeTag";
         return QByteArray();
     }
 
@@ -689,18 +689,18 @@ QByteArray encryptStringSymmetric(const QByteArray& key, const QByteArray& data)
     }
     clen += len;
 
-    /* Get the tag */
-    QByteArray tag(OCC::CommonConstants::e2EeTagSize, '\0');
-    if(1 != EVP_CIPHER_CTX_ctrl(ctx, EVP_CTRL_GCM_GET_TAG, OCC::CommonConstants::e2EeTagSize, unsignedData(tag))) {
-        qCInfo(lcCse()) << "Error getting the tag";
+    /* Get the e2EeTag */
+    QByteArray e2EeTag(OCC::Constants::e2EeTagSize, '\0');
+    if(1 != EVP_CIPHER_CTX_ctrl(ctx, EVP_CTRL_GCM_GET_TAG, OCC::Constants::e2EeTagSize, unsignedData(e2EeTag))) {
+        qCInfo(lcCse()) << "Error getting the e2EeTag";
         handleErrors();
         return {};
     }
 
     QByteArray cipherTXT;
-    cipherTXT.reserve(clen + 16);
+    cipherTXT.reserve(clen + OCC::Constants::e2EeTagSize);
     cipherTXT.append(ctext, clen);
-    cipherTXT.append(tag);
+    cipherTXT.append(e2EeTag);
 
     QByteArray result = cipherTXT.toBase64();
     result += '|';
@@ -1653,13 +1653,13 @@ bool EncryptionHelper::fileEncryption(const QByteArray &key, const QByteArray &i
         return false;
     }
 
-    QByteArray out(blockSize + OCC::CommonConstants::e2EeTagSize - 1, '\0');
+    QByteArray out(blockSize + OCC::Constants::e2EeTagSize - 1, '\0');
     int len = 0;
     int total_len = 0;
 
     qCDebug(lcCse) << "Starting to encrypt the file" << input->fileName() << input->atEnd();
     while(!input->atEnd()) {
-        QByteArray data = input->read(blockSize);
+        const auto data = input->read(blockSize);
 
         if (data.size() == 0) {
             qCInfo(lcCse()) << "Could not read data from file";
@@ -1683,15 +1683,15 @@ bool EncryptionHelper::fileEncryption(const QByteArray &key, const QByteArray &i
     output->write(out, len);
     total_len += len;
 
-    /* Get the tag */
-    QByteArray tag(OCC::CommonConstants::e2EeTagSize, '\0');
-    if(1 != EVP_CIPHER_CTX_ctrl(ctx, EVP_CTRL_GCM_GET_TAG, OCC::CommonConstants::e2EeTagSize, unsignedData(tag))) {
-        qCInfo(lcCse()) << "Could not get tag";
+    /* Get the e2EeTag */
+    QByteArray e2EeTag(OCC::Constants::e2EeTagSize, '\0');
+    if(1 != EVP_CIPHER_CTX_ctrl(ctx, EVP_CTRL_GCM_GET_TAG, OCC::Constants::e2EeTagSize, unsignedData(e2EeTag))) {
+        qCInfo(lcCse()) << "Could not get e2EeTag";
         return false;
     }
 
-    returnTag = tag;
-    output->write(tag, OCC::CommonConstants::e2EeTagSize);
+    returnTag = e2EeTag;
+    output->write(e2EeTag, OCC::Constants::e2EeTagSize);
 
     input->close();
     output->close();
@@ -1734,9 +1734,9 @@ bool EncryptionHelper::fileDecryption(const QByteArray &key, const QByteArray& i
         return false;
     }
 
-    qint64 size = input->size() - OCC::CommonConstants::e2EeTagSize;
+    qint64 size = input->size() - OCC::Constants::e2EeTagSize;
 
-    QByteArray out(blockSize + OCC::CommonConstants::e2EeTagSize - 1, '\0');
+    QByteArray out(blockSize + OCC::Constants::e2EeTagSize - 1, '\0');
     int len = 0;
 
     while(input->pos() < size) {
@@ -1761,11 +1761,11 @@ bool EncryptionHelper::fileDecryption(const QByteArray &key, const QByteArray& i
         output->write(out, len);
     }
 
-    QByteArray tag = input->read(OCC::CommonConstants::e2EeTagSize);
+    const QByteArray e2EeTag = input->read(OCC::Constants::e2EeTagSize);
 
-    /* Set expected tag value. Works in OpenSSL 1.0.1d and later */
-    if(!EVP_CIPHER_CTX_ctrl(ctx, EVP_CTRL_GCM_SET_TAG, tag.size(), (unsigned char *)tag.constData())) {
-        qCInfo(lcCse()) << "Could not set expected tag";
+    /* Set expected e2EeTag value. Works in OpenSSL 1.0.1d and later */
+    if(!EVP_CIPHER_CTX_ctrl(ctx, EVP_CTRL_GCM_SET_TAG, e2EeTag.size(), (unsigned char *)e2EeTag.constData())) {
+        qCInfo(lcCse()) << "Could not set expected e2EeTag";
         return false;
     }
 
@@ -1807,30 +1807,34 @@ EncryptionHelper::StreamingDecryptor::StreamingDecryptor(const QByteArray &key,
     }
 }
 
-qint32 EncryptionHelper::StreamingDecryptor::chunkDecryption(const char *input, QIODevice *output, quint32 chunkSize)
+QByteArray EncryptionHelper::StreamingDecryptor::chunkDecryption(const char *input, quint64 chunkSize)
 {
+    QByteArray byteArray;
+    QBuffer buffer(&byteArray);
+    buffer.open(QIODevice::WriteOnly);
+
     Q_ASSERT(isInitialized());
     if (!isInitialized()) {
         qCritical(lcCse()) << "Decryption failed. Decryptor is not initialized!";
-        return -1;
+        return QByteArray();
     }
 
-    Q_ASSERT(output && output->isOpen() && output->isWritable());
-    if (!output || !output->isOpen() || !output->isWritable()) {
+    Q_ASSERT(buffer.isOpen() && buffer.isWritable());
+    if (!buffer.isOpen() || !buffer.isWritable()) {
         qCritical(lcCse()) << "Decryption failed. Incorrect output device!";
-        return -1;
+        return QByteArray();
     }
 
     Q_ASSERT(input);
     if (!input) {
         qCritical(lcCse()) << "Decryption failed. Incorrect input!";
-        return -1;
+        return QByteArray();
     }
 
     Q_ASSERT(chunkSize > 0);
     if (chunkSize <= 0) {
         qCritical(lcCse()) << "Decryption failed. Incorrect chunkSize!";
-        return -1;
+        return QByteArray();
     }
 
     if (_decryptedSoFar == 0) {
@@ -1840,47 +1844,54 @@ qint32 EncryptionHelper::StreamingDecryptor::chunkDecryption(const char *input,
     Q_ASSERT(_decryptedSoFar + chunkSize <= _totalSize);
     if (_decryptedSoFar + chunkSize > _totalSize) {
         qCritical(lcCse()) << "Decryption failed. Chunk is out of range!";
-        return -1;
+        return QByteArray();
+    }
+
+    Q_ASSERT(_decryptedSoFar + chunkSize < OCC::Constants::e2EeTagSize || _totalSize - OCC::Constants::e2EeTagSize >= _decryptedSoFar + chunkSize - OCC::Constants::e2EeTagSize);
+    if (_decryptedSoFar + chunkSize > OCC::Constants::e2EeTagSize && _totalSize - OCC::Constants::e2EeTagSize < _decryptedSoFar + chunkSize - OCC::Constants::e2EeTagSize) {
+        qCritical(lcCse()) << "Decryption failed. Incorrect chunk!";
+        return QByteArray();
     }
 
     const bool isLastChunk = _decryptedSoFar + chunkSize == _totalSize;
 
-    // last OCC::CommonConstants::e2EeTagSize bytes is ALWAYS a tag!!!
-    const qint32 size = isLastChunk ? static_cast<qint32>(chunkSize) - OCC::CommonConstants::e2EeTagSize : static_cast<qint32>(chunkSize);
+    // last OCC::Constants::e2EeTagSize bytes is ALWAYS a e2EeTag!!!
+    const qint64 size = isLastChunk ? chunkSize - OCC::Constants::e2EeTagSize : chunkSize;
 
-    Q_ASSERT(size > 0);
-    if (size <= 0) {
+    // either the size is more than 0 and an e2EeTag is at the end of chunk, or, chunk is the e2EeTag itself
+    Q_ASSERT(size > 0 || chunkSize == OCC::Constants::e2EeTagSize);
+    if (size <= 0 && chunkSize != OCC::Constants::e2EeTagSize) {
         qCritical(lcCse()) << "Decryption failed. Invalid input size: " << size << " !";
-        return -1;
+        return QByteArray();
     }
 
-    int bytesWritten = 0;
+    qint64 bytesWritten = 0;
+    qint64 inputPos = 0;
 
-    QByteArray decryptedBlock(blockSize + OCC::CommonConstants::e2EeTagSize - 1, '\0');
-    int inputPos = 0;
+    QByteArray decryptedBlock(blockSize + OCC::Constants::e2EeTagSize - 1, '\0');
 
     while(inputPos < size) {
         // read blockSize or less bytes
-        const QByteArray encryptedBlock = QByteArray(input + inputPos, qMin(size - inputPos, blockSize));
+        const QByteArray encryptedBlock(input + inputPos, qMin(size - inputPos, blockSize));
 
         if (encryptedBlock.size() == 0) {
             qCritical(lcCse()) << "Could not read data from the input buffer.";
-            return -1;
+            return QByteArray();
         }
 
         int outLen = 0;
 
         if(!EVP_DecryptUpdate(_ctx, unsignedData(decryptedBlock), &outLen, reinterpret_cast<const unsigned char*>(encryptedBlock.data()), encryptedBlock.size())) {
             qCritical(lcCse()) << "Could not decrypt";
-            return -1;
+            return QByteArray();
         }
 
-        const auto writtenToOutput = output->write(decryptedBlock, outLen);
+        const auto writtenToOutput = buffer.write(decryptedBlock, outLen);
 
         Q_ASSERT(writtenToOutput == outLen);
         if (writtenToOutput != outLen) {
             qCritical(lcCse()) << "Failed to write decrypted data to device.";
-            return -1;
+            return QByteArray();
         }
 
         bytesWritten += writtenToOutput;
@@ -1892,51 +1903,49 @@ qint32 EncryptionHelper::StreamingDecryptor::chunkDecryption(const char *input,
     }
 
     if (isLastChunk) {
-        // if it's a last chunk, we'd need to read a tag at the end and finalize the decryption
+        // if it's a last chunk, we'd need to read a e2EeTag at the end and finalize the decryption
 
-        Q_ASSERT(chunkSize - inputPos == OCC::CommonConstants::e2EeTagSize);
-        if (chunkSize - inputPos != OCC::CommonConstants::e2EeTagSize) {
-            qCritical(lcCse()) << "Decryption failed. E2EE tag is missing!";
-            return -1;
+        Q_ASSERT(chunkSize - inputPos == OCC::Constants::e2EeTagSize);
+        if (chunkSize - inputPos != OCC::Constants::e2EeTagSize) {
+            qCritical(lcCse()) << "Decryption failed. e2EeTag is missing!";
+            return QByteArray();
         }
 
         int outLen = 0;
 
-        QByteArray tag = QByteArray(input + inputPos, OCC::CommonConstants::e2EeTagSize);
+        QByteArray e2EeTag = QByteArray(input + inputPos, OCC::Constants::e2EeTagSize);
 
-        /* Set expected tag value. Works in OpenSSL 1.0.1d and later */
-        if(!EVP_CIPHER_CTX_ctrl(_ctx, EVP_CTRL_GCM_SET_TAG, tag.size(), reinterpret_cast<unsigned char*>(tag.data()))) {
-            qCritical(lcCse()) << "Could not set expected tag";
-            return -1;
+        /* Set expected e2EeTag value. Works in OpenSSL 1.0.1d and later */
+        if(!EVP_CIPHER_CTX_ctrl(_ctx, EVP_CTRL_GCM_SET_TAG, e2EeTag.size(), reinterpret_cast<unsigned char*>(e2EeTag.data()))) {
+            qCritical(lcCse()) << "Could not set expected e2EeTag";
+            return QByteArray();
         }
 
         if(1 != EVP_DecryptFinal_ex(_ctx, unsignedData(decryptedBlock), &outLen)) {
             qCritical(lcCse()) << "Could finalize decryption";
-            return -1;
+            return QByteArray();
         }
 
-        const auto writtenToOutput = output->write(decryptedBlock, outLen);
+        const auto writtenToOutput = buffer.write(decryptedBlock, outLen);
 
         Q_ASSERT(writtenToOutput == outLen);
         if (writtenToOutput != outLen) {
             qCritical(lcCse()) << "Failed to write decrypted data to device.";
-            return -1;
+            return QByteArray();
         }
 
         bytesWritten += writtenToOutput;
 
-        _decryptedSoFar += OCC::CommonConstants::e2EeTagSize;
+        _decryptedSoFar += OCC::Constants::e2EeTagSize;
 
         _isFinished = true;
     }
 
-    // qCDebug(lcCse()) <<"Decrypting:" << _decryptedSoFar << "/" << _totalSize;
-
     if (isFinished()) {
         qCDebug(lcCse()) << "Decryption complete";
     }
 
-    return bytesWritten;
+    return byteArray;
 }
 
 bool EncryptionHelper::StreamingDecryptor::isInitialized() const

src/libsync/clientsideencryption.h

@@ -66,11 +66,10 @@ namespace EncryptionHelper {
     OWNCLOUDSYNC_EXPORT bool fileDecryption(const QByteArray &key, const QByteArray &iv,
                                QFile *input, QFile *output);
 
-    //
-    // Simple classes for safe (RAII) handling of OpenSSL
-    // data structures
-    //
-
+//
+// Simple classes for safe (RAII) handling of OpenSSL
+// data structures
+//
 class CipherCtx {
 public:
     CipherCtx() : _ctx(EVP_CIPHER_CTX_new())
@@ -98,7 +97,7 @@ public:
     StreamingDecryptor(const QByteArray &key, const QByteArray &iv, quint64 totalSize);
     ~StreamingDecryptor() = default;
 
-    qint32 chunkDecryption(const char *input, QIODevice *output, quint32 chunkSize);
+    QByteArray chunkDecryption(const char *input, quint64 chunkSize);
 
     bool isInitialized() const;
     bool isFinished() const;
@@ -112,7 +111,7 @@ private:
     quint64 _decryptedSoFar = 0;
     quint64 _totalSize = 0;
 };
-};
+}
 
 class OWNCLOUDSYNC_EXPORT ClientSideEncryption : public QObject {
     Q_OBJECT

src/libsync/discovery.cpp

@@ -460,8 +460,10 @@ void ProcessDirectoryJob::processFileAnalyzeRemoteInfo(
 
     // The file is known in the db already
     if (dbEntry.isValid()) {
-        const bool isVirtualE2EePlaceholder = dbEntry.isVirtualFile() && !item->_encryptedFileName.isEmpty() && serverEntry.size > 0;
-        const qint64 sizeOnServer = isVirtualE2EePlaceholder ? serverEntry.size - CommonConstants::e2EeTagSize : serverEntry.size;
+        const bool isDbEntryAnE2EePlaceholder = dbEntry.isVirtualFile() && !dbEntry.e2eMangledName().isEmpty();
+        Q_ASSERT(!isDbEntryAnE2EePlaceholder || serverEntry.size >= Constants::e2EeTagSize);
+        const bool isVirtualE2EePlaceholder = isDbEntryAnE2EePlaceholder && serverEntry.size >= Constants::e2EeTagSize;
+        const qint64 sizeOnServer = isVirtualE2EePlaceholder ? serverEntry.size - Constants::e2EeTagSize : serverEntry.size;
         const bool metaDataSizeNeedsUpdateForE2EeFilePlaceholder = isVirtualE2EePlaceholder && dbEntry._fileSize == serverEntry.size;
 
         if (serverEntry.isDirectory != dbEntry.isDirectory()) {
@@ -494,6 +496,7 @@ void ProcessDirectoryJob::processFileAnalyzeRemoteInfo(
         } else if (dbEntry._remotePerm != serverEntry.remotePerm || dbEntry._fileId != serverEntry.fileId || metaDataSizeNeedsUpdateForE2EeFilePlaceholder) {
             if (metaDataSizeNeedsUpdateForE2EeFilePlaceholder) {
                 // we are updating placeholder sizes after migrating from older versions with VFS + E2EE implicit hydration not supported
+                qCDebug(lcDisco) << "Migrating the E2EE VFS placeholder " << dbEntry.path() << " from older version. The old size is " << item->_size << ". The new size is " << sizeOnServer;
                 item->_size = sizeOnServer;
             }
             item->_instruction = CSYNC_INSTRUCTION_UPDATE_METADATA;
@@ -503,19 +506,23 @@ void ProcessDirectoryJob::processFileAnalyzeRemoteInfo(
             // to update a placeholder with corrected size (-16 Bytes)
             // or, maybe, add a flag to the database - vfsE2eeSizeCorrected? if it is not set - subtract it from the placeholder's size and re-create/update a placeholder?
             const QueryMode serverQueryMode = [this, &dbEntry, &serverEntry]() {
-                if (dbEntry.isDirectory() && dbEntry._isE2eEncrypted) {
+                const bool isVfsModeOn = _discoveryData && _discoveryData->_syncOptions._vfs && _discoveryData->_syncOptions._vfs->mode() != Vfs::Off;
+                if (isVfsModeOn && dbEntry.isDirectory() && dbEntry._isE2eEncrypted) {
                     qint64 localFolderSize = 0;
                     const auto listFilesCallback = [this, &localFolderSize](const OCC::SyncJournalFileRecord &record) {
                         if (record.isFile()) {
-                            // add CommonConstants::e2EeTagSize so we will know the size of E2EE file on the server
-                            localFolderSize += record._fileSize + CommonConstants::e2EeTagSize;
+                            // add Constants::e2EeTagSize so we will know the size of E2EE file on the server
+                            localFolderSize += record._fileSize + Constants::e2EeTagSize;
                         } else if (record.isVirtualFile()) {
-                            // just a virtual file, so, the size must contain CommonConstants::e2EeTagSize if it was not corrected already
+                            // just a virtual file, so, the size must contain Constants::e2EeTagSize if it was not corrected already
                             localFolderSize += record._fileSize;
                         }
                     };
-                    if (_discoveryData->_statedb->listFilesInPath(dbEntry.path().toUtf8(), listFilesCallback)
-                        && localFolderSize != 0 && localFolderSize == serverEntry.sizeOfFolder) {
+
+                    const bool listFilesSucceeded = _discoveryData->_statedb->listFilesInPath(dbEntry.path().toUtf8(), listFilesCallback);
+
+                    if (listFilesSucceeded && localFolderSize != 0 && localFolderSize == serverEntry.sizeOfFolder) {
+                        qCInfo(lcDisco) << "Migration of E2EE folder " << dbEntry.path() << " from older version to the one, supporting the implicit VFS hydration.";
                         return NormalQuery;
                     }
                 }
@@ -563,20 +570,14 @@ void ProcessDirectoryJob::processFileAnalyzeRemoteInfo(
             if (isVfsWithSuffix())
                 addVirtualFileSuffix(tmp_path._original);
         }
-        // a virtual file is on disk but is not in the database
-        const bool fileOnDiskIsVirtual = localEntry.isValid() && localEntry.isVirtualFile;
-        // a new virtual file
-        const bool isNewVirtualFileNotOnDiskYet = !localEntry.isValid() && item->_type == ItemTypeVirtualFile;
 
-        if ((fileOnDiskIsVirtual || isNewVirtualFileNotOnDiskYet)
-            && opts._vfs->mode() != Vfs::Off
-            && !item->_encryptedFileName.isEmpty()) {
+        if (opts._vfs->mode() != Vfs::Off && !item->_encryptedFileName.isEmpty()) {
             // We are syncing a file for the first time (local entry is invalid) and it is encrypted file that will be virtual once synced
-            // to avoid having error of "file has changed during sync" when trying to hydrate it excplicitly - we must remove CommonConstants::e2EeTagSize bytes from the end
+            // to avoid having error of "file has changed during sync" when trying to hydrate it excplicitly - we must remove Constants::e2EeTagSize bytes from the end
             // as explicit hydration does not care if these bytes are present in the placeholder or not, but, the size must not change in the middle of the sync
-            // this way it works for both implicit and explicit hydration by making a placeholder size that does not includes encryption tag CommonConstants::e2EeTagSize bytes
+            // this way it works for both implicit and explicit hydration by making a placeholder size that does not includes encryption tag Constants::e2EeTagSize bytes
             // another scenario - we are syncing a file which is on disk but not in the database (database was removed or file was not written there yet)
-            item->_size = serverEntry.size - CommonConstants::e2EeTagSize;
+            item->_size = serverEntry.size - Constants::e2EeTagSize;
         }
         processFileAnalyzeLocalInfo(item, tmp_path, localEntry, serverEntry, dbEntry, _queryServer);
     };

src/libsync/propagatedownload.cpp

@@ -22,8 +22,9 @@
 #include "common/utility.h"
 #include "filesystem.h"
 #include "propagatorjobs.h"
-#include "common/checksums.h"
-#include "common/asserts.h"
+#include <common/checksums.h>
+#include <common/asserts.h>
+#include <common/constants.h>
 #include "clientsideencryptionjobs.h"
 #include "propagatedownloadencrypted.h"
 #include "common/vfs.h"
@@ -73,6 +74,7 @@ GETFileJob::GETFileJob(AccountPtr account, const QString &path, QIODevice *devic
     const QMap<QByteArray, QByteArray> &headers, const QByteArray &expectedEtagForResume,
     qint64 resumeStart, QObject *parent)
     : AbstractNetworkJob(account, path, parent)
+    , _device(device)
     , _headers(headers)
     , _expectedEtagForResume(expectedEtagForResume)
     , _expectedContentLength(-1)
@@ -85,7 +87,6 @@ GETFileJob::GETFileJob(AccountPtr account, const QString &path, QIODevice *devic
     , _bandwidthManager(nullptr)
     , _hasEmittedFinishedSignal(false)
     , _lastModified()
-    , _device(device)
 {
 }
 
@@ -250,8 +251,6 @@ void GETFileJob::slotMetaDataChanged()
     }
 
     _saveBodyToFile = true;
-
-    processMetaData();
 }
 
 void GETFileJob::setBandwidthManager(BandwidthManager *bwm)
@@ -286,9 +285,9 @@ qint64 GETFileJob::currentDownloadPosition()
     return _resumeStart;
 }
 
-qint64 GETFileJob::writeToDevice(const char *data, qint64 len)
+qint64 GETFileJob::writeToDevice(const QByteArray &data)
 {
-    return _device->write(data, len);
+    return _device->write(data);
 }
 
 void GETFileJob::slotReadyRead()
@@ -313,8 +312,8 @@ void GETFileJob::slotReadyRead()
             _bandwidthQuota -= toRead;
         }
 
-        const qint64 r = reply()->read(buffer.data(), toRead);
-        if (r < 0) {
+        const qint64 readBytes = reply()->read(buffer.data(), toRead);
+        if (readBytes < 0) {
             _errorString = networkReplyErrorString(*reply());
             _errorStatus = SyncFileItem::NormalError;
             qCWarning(lcGetJob) << "Error while reading from device: " << _errorString;
@@ -322,11 +321,11 @@ void GETFileJob::slotReadyRead()
             return;
         }
 
-        const qint64 w = writeToDevice(buffer.constData(), r);
-        if (w != r) {
+        const qint64 writtenBytes = writeToDevice(QByteArray::fromRawData(buffer.constData(), readBytes));
+        if (writtenBytes != readBytes) {
             _errorString = _device->errorString();
             _errorStatus = SyncFileItem::NormalError;
-            qCWarning(lcGetJob) << "Error while writing to file" << w << r << _errorString;
+            qCWarning(lcGetJob) << "Error while writing to file" << writtenBytes << readBytes << _errorString;
             reply()->abort();
             return;
         }
@@ -394,29 +393,57 @@ GETEncryptedFileJob::GETEncryptedFileJob(AccountPtr account, const QUrl &url, QI
 {
 }
 
-qint64 GETEncryptedFileJob::writeToDevice(const char *data, qint64 len)
+qint64 GETEncryptedFileJob::writeToDevice(const QByteArray &data)
 {
+    if (!_decryptor) {
+        // only initialize the decryptor once, because, according to Qt documentation, metadata might get changed during the processing of the data sometimes
+        // https://doc.qt.io/qt-5/qnetworkreply.html#metaDataChanged
+        _decryptor.reset(new EncryptionHelper::StreamingDecryptor(_encryptedFileInfo.encryptionKey, _encryptedFileInfo.initializationVector, _contentLength));
+    }
+
     if (!_decryptor->isInitialized()) {
         return -1;
     }
 
-    const auto bytesDecrypted = _decryptor->chunkDecryption(data, _device, len);
+    const auto bytesRemaining = _contentLength - _processedSoFar - data.length();
 
-    if (bytesDecrypted == -1) {
+    if (bytesRemaining != 0 && bytesRemaining < OCC::Constants::e2EeTagSize) {
+        // decryption is going to fail if last chunk does not include or does not equal to OCC::Constants::e2EeTagSize bytes tag
+        // we may end up receiving packets beyond OCC::Constants::e2EeTagSize bytes tag at the end
+        // in that case, we don't want to try and decrypt less than OCC::Constants::e2EeTagSize ending bytes of tag, we will accumulate all the incoming data till the end
+        // and then, we are going to decrypt the entire chunk containing OCC::Constants::e2EeTagSize bytes at the end
+        _pendingBytes += QByteArray(data.constData(), data.length());
+        _processedSoFar += data.length();
+        if (_processedSoFar != _contentLength) {
+            return data.length();
+        }
+    }
+
+    if (!_pendingBytes.isEmpty()) {
+        const auto decryptedChunk = _decryptor->chunkDecryption(_pendingBytes.constData(), _pendingBytes.size());
+
+        if (decryptedChunk.isEmpty()) {
+            qCCritical(lcPropagateDownload) << "Decryption failed!";
+            return -1;
+        }
+
+        GETFileJob::writeToDevice(decryptedChunk);
+
+        return data.length();
+    }
+
+    const auto decryptedChunk = _decryptor->chunkDecryption(data.constData(), data.length());
+
+    if (decryptedChunk.isEmpty()) {
         qCCritical(lcPropagateDownload) << "Decryption failed!";
         return -1;
     }
 
-    return len;
-}
+    GETFileJob::writeToDevice(decryptedChunk);
 
-void GETEncryptedFileJob::processMetaData()
-{
-    if (!_decryptor) {
-        // only initialize the decryptor once, because, according to Qt documentation, metadata might get changed during the processing of the data sometimes
-        // https://doc.qt.io/qt-5/qnetworkreply.html#metaDataChanged
-        _decryptor.reset(new EncryptionHelper::StreamingDecryptor(_encryptedFileInfo.encryptionKey, _encryptedFileInfo.initializationVector, _contentLength));
-    }
+    _processedSoFar += data.length();
+
+    return data.length();
 }
 
 void PropagateDownloadFile::start()

src/libsync/propagatedownload.h

@@ -31,6 +31,7 @@ class PropagateDownloadEncrypted;
 class OWNCLOUDSYNC_EXPORT GETFileJob : public AbstractNetworkJob
 {
     Q_OBJECT
+    QIODevice *_device;
     QMap<QByteArray, QByteArray> _headers;
     QString _errorString;
     QByteArray _expectedEtagForResume;
@@ -50,7 +51,6 @@ class OWNCLOUDSYNC_EXPORT GETFileJob : public AbstractNetworkJob
     bool _saveBodyToFile = false;
 
 protected:
-    QIODevice *_device;
     qint64 _contentLength;
 
 public:
@@ -113,8 +113,7 @@ public:
     void setExpectedContentLength(qint64 size) { _expectedContentLength = size; }
 
 protected:
-    virtual qint64 writeToDevice(const char *data, qint64 len);
-    virtual void processMetaData() {}
+    virtual qint64 writeToDevice(const QByteArray &data);
 
 signals:
     void finishedSignal();
@@ -143,12 +142,13 @@ public:
     virtual ~GETEncryptedFileJob() = default;
 
 protected:
-    virtual qint64 writeToDevice(const char *data, qint64 len) override;
-    virtual void processMetaData() override;
+    virtual qint64 writeToDevice(const QByteArray &data) override;
 
 private:
     QSharedPointer<EncryptionHelper::StreamingDecryptor> _decryptor;
     EncryptedFile _encryptedFileInfo = {};
+    QByteArray _pendingBytes;
+    qint64 _processedSoFar = 0;
 };
 
 /**

src/libsync/vfs/cfapi/hydrationjob.cpp

@@ -101,14 +101,14 @@ void OCC::HydrationJob::setIsEncryptedFile(bool isEncrypted)
     _isEncryptedFile = isEncrypted;
 }
 
-QString OCC::HydrationJob::encryptedFileName() const
+QString OCC::HydrationJob::e2eMangledName() const
 {
-    return _encryptedFileName;
+    return _e2eMangledName;
 }
 
-void OCC::HydrationJob::setEncryptedFileName(const QString &encryptedName)
+void OCC::HydrationJob::setE2eMangledName(const QString &e2eMangledName)
 {
-    _encryptedFileName = encryptedName;
+    _e2eMangledName = e2eMangledName;
 }
 
 OCC::HydrationJob::Status OCC::HydrationJob::status() const
@@ -189,7 +189,7 @@ void OCC::HydrationJob::slotCheckFolderId(const QStringList &list)
 void OCC::HydrationJob::slotFolderEncryptedMetadataError(const QByteArray & /*fileId*/, int /*httpReturnCode*/)
 {
     // TODO: the following code is borrowed from PropagateDownloadEncrypted (see HydrationJob::onNewConnection() for explanation of next steps)
-    qCCritical(lcHydration) << "Failed to find encrypted metadata information of remote file" << encryptedFileName();
+    qCCritical(lcHydration) << "Failed to find encrypted metadata information of remote file" << e2eMangledName();
     emitFinished(Error);
     return;
 }
@@ -197,14 +197,14 @@ void OCC::HydrationJob::slotFolderEncryptedMetadataError(const QByteArray & /*fi
 void OCC::HydrationJob::slotCheckFolderEncryptedMetadata(const QJsonDocument &json)
 {
     // TODO: the following code is borrowed from PropagateDownloadEncrypted (see HydrationJob::onNewConnection() for explanation of next steps)
-    qCDebug(lcHydration) << "Metadata Received reading" << encryptedFileName();
-    const QString filename = encryptedFileName();
+    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();
 
     EncryptedFile encryptedInfo = {};
 
-    const QString encryptedFileExactName = encryptedFileName().section(QLatin1Char('/'), -1);
+    const QString encryptedFileExactName = e2eMangledName().section(QLatin1Char('/'), -1);
     for (const EncryptedFile &file : files) {
         if (encryptedFileExactName == file.encryptedFilename) {
             EncryptedFile encryptedInfo = file;
@@ -212,7 +212,7 @@ void OCC::HydrationJob::slotCheckFolderEncryptedMetadata(const QJsonDocument &js
 
             qCDebug(lcHydration) << "Found matching encrypted metadata for file, starting download" << _requestId << _folderPath;
             _transferDataSocket = _transferDataServer->nextPendingConnection();
-            _job = new GETEncryptedFileJob(_account, _remotePath + encryptedFileName(), _transferDataSocket, {}, {}, 0, encryptedInfo, this);
+            _job = new GETEncryptedFileJob(_account, _remotePath + e2eMangledName(), _transferDataSocket, {}, {}, 0, encryptedInfo, this);
 
             connect(qobject_cast<GETEncryptedFileJob *>(_job), &GETEncryptedFileJob::finishedSignal, this, &HydrationJob::onGetFinished);
             _job->start();
@@ -272,34 +272,9 @@ void OCC::HydrationJob::onNewConnection()
     Q_ASSERT(!_job);
 
     if (isEncryptedFile()) {
-        // TODO: the following code is borrowed from PropagateDownloadEncrypted (should we factor it out and reuse? YES! Should we do it now? Probably not, as, this would imply modifying PropagateDownloadEncrypted, so we need a separate PR)
-        qCInfo(lcHydration) << "Got new connection for encrypted file. Getting required info for decryption...";
-        const auto rootPath = [=]() {
-            const auto result = _remotePath;
-            if (result.startsWith('/')) {
-                return result.mid(1);
-            } else {
-                return result;
-            }
-        }();
-
-        const auto remoteFilename = encryptedFileName();
-        const auto remotePath = QString(rootPath + remoteFilename);
-        const auto remoteParentPath = remotePath.left(remotePath.lastIndexOf('/'));
-
-        auto job = new LsColJob(_account, remoteParentPath, this);
-        job->setProperties({ "resourcetype", "http://owncloud.org/ns:fileid" });
-        connect(job, &LsColJob::directoryListingSubfolders,
-            this, &HydrationJob::slotCheckFolderId);
-        connect(job, &LsColJob::finishedWithError,
-            this, &HydrationJob::slotFolderIdError);
-        job->start();
+        handleNewConnectionForEncryptedFile();
     } else {
-        qCInfo(lcHydration) << "Got new connection starting GETFileJob" << _requestId << _folderPath;
-        _transferDataSocket = _transferDataServer->nextPendingConnection();
-        _job = new GETFileJob(_account, _remotePath + _folderPath, _transferDataSocket, {}, {}, 0, this);
-        connect(_job, &GETFileJob::finishedSignal, this, &HydrationJob::onGetFinished);
-        _job->start();
+        handleNewConnection();
     }
 }
 
@@ -349,3 +324,38 @@ void OCC::HydrationJob::onGetFinished()
     }
     emitFinished(Success);
 }
+
+void OCC::HydrationJob::handleNewConnection()
+{
+    qCInfo(lcHydration) << "Got new connection starting GETFileJob" << _requestId << _folderPath;
+    _transferDataSocket = _transferDataServer->nextPendingConnection();
+    _job = new GETFileJob(_account, _remotePath + _folderPath, _transferDataSocket, {}, {}, 0, this);
+    connect(_job, &GETFileJob::finishedSignal, this, &HydrationJob::onGetFinished);
+    _job->start();
+}
+
+void OCC::HydrationJob::handleNewConnectionForEncryptedFile()
+{
+    // TODO: the following code is borrowed from PropagateDownloadEncrypted (should we factor it out and reuse? YES! Should we do it now? Probably not, as, this would imply modifying PropagateDownloadEncrypted, so we need a separate PR)
+    qCInfo(lcHydration) << "Got new connection for encrypted file. Getting required info for decryption...";
+    const auto rootPath = [=]() {
+        const auto result = _remotePath;
+        if (result.startsWith('/')) {
+            return result.mid(1);
+        } else {
+            return result;
+        }
+    }();
+
+    const auto remoteFilename = e2eMangledName();
+    const auto remotePath = QString(rootPath + remoteFilename);
+    const auto remoteParentPath = remotePath.left(remotePath.lastIndexOf('/'));
+
+    auto job = new LsColJob(_account, remoteParentPath, this);
+    job->setProperties({ "resourcetype", "http://owncloud.org/ns:fileid" });
+    connect(job, &LsColJob::directoryListingSubfolders,
+        this, &HydrationJob::slotCheckFolderId);
+    connect(job, &LsColJob::finishedWithError,
+        this, &HydrationJob::slotFolderIdError);
+    job->start();
+}

src/libsync/vfs/cfapi/hydrationjob.h

@@ -63,8 +63,8 @@ public:
     bool isEncryptedFile() const;
     void setIsEncryptedFile(bool isEncrypted);
 
-    QString encryptedFileName() const;
-    void setEncryptedFileName(const QString &encryptedName);
+    QString e2eMangledName() const;
+    void setE2eMangledName(const QString &e2eMangledName);
 
     qint64 fileTotalSize() const;
     void setFileTotalSize(qint64 totalSize);
@@ -91,6 +91,9 @@ private:
     void onCancellationServerNewConnection();
     void onGetFinished();
 
+    void handleNewConnection();
+    void handleNewConnectionForEncryptedFile();
+
     void startServerAndWaitForConnections();
 
     AccountPtr _account;
@@ -103,7 +106,7 @@ private:
     QString _folderPath;
 
     bool _isEncryptedFile = false;
-    QString _encryptedFileName;
+    QString _e2eMangledName;
 
     QLocalServer *_transferDataServer = nullptr;
     QLocalServer *_signalServer = nullptr;

src/libsync/vfs/cfapi/vfs_cfapi.cpp

@@ -355,8 +355,8 @@ void VfsCfApi::scheduleHydrationJob(const QString &requestId, const QString &fol
     job->setJournal(params().journal);
     job->setRequestId(requestId);
     job->setFolderPath(folderPath);
-    job->setIsEncryptedFile(!record._e2eMangledName.isEmpty());
-    job->setEncryptedFileName(record._e2eMangledName);
+    job->setIsEncryptedFile(record._isE2eEncrypted);
+    job->setE2eMangledName(record._e2eMangledName);
     connect(job, &HydrationJob::finished, this, &VfsCfApi::onHydrationJobFinished);
     d->hydrationJobs << job;
     job->start();

test/testclientsideencryption.cpp

@@ -9,6 +9,8 @@
 #include <QTemporaryFile>
 #include <QRandomGenerator>
 
+#include <common/constants.h>
+
 #include "clientsideencryption.h"
 
 using namespace OCC;
@@ -136,13 +138,26 @@ private slots:
         QCOMPARE(data, originalData);
     }
 
+    void testStreamingDecryptor_data()
+    {
+        QTest::addColumn<int>("totalBytes");
+        QTest::addColumn<int>("bytesToRead");
+
+        QTest::newRow("data1") << 64  << 2;
+        QTest::newRow("data2") << 32  << 8;
+        QTest::newRow("data3") << 76  << 64;
+        QTest::newRow("data4") << 272 << 256;
+    }
+
     void testStreamingDecryptor()
     {
+        QFETCH(int, totalBytes);
+
         QTemporaryFile dummyInputFile;
 
         QVERIFY(dummyInputFile.open());
 
-        const auto dummyFileRandomContents = EncryptionHelper::generateRandom(272);
+        const auto dummyFileRandomContents = EncryptionHelper::generateRandom(totalBytes);
 
         QCOMPARE(dummyInputFile.write(dummyFileRandomContents), dummyFileRandomContents.size());
 
@@ -190,18 +205,42 @@ private slots:
 
         QVERIFY(dummyEncryptionOutputFile.open());
 
-        const auto readBytesAvailable = dummyEncryptionOutputFile.bytesAvailable();
-        while (dummyEncryptionOutputFile.pos() < readBytesAvailable) {
-            // auto toRead = QRandomGenerator::global()->bounded(8, 128);
-            // decryption is going to fail if last chunk does not include or does not equal to 16-bytes tag (accumulation required? - but where? in the caller's code?)
-            auto toRead = 64;
-            if (dummyEncryptionOutputFile.pos() + toRead > readBytesAvailable) {
-                toRead = readBytesAvailable - dummyEncryptionOutputFile.pos();
+        QByteArray pendingBytes;
+
+        QFETCH(int, bytesToRead);
+
+        while (dummyEncryptionOutputFile.pos() < dummyEncryptionOutputFile.size()) {
+            const auto bytesRemaining = dummyEncryptionOutputFile.size() - dummyEncryptionOutputFile.pos();
+            auto toRead = bytesRemaining > bytesToRead ? bytesToRead : bytesRemaining;
+
+            if (dummyEncryptionOutputFile.pos() + toRead > dummyEncryptionOutputFile.size()) {
+                toRead = dummyEncryptionOutputFile.size() - dummyEncryptionOutputFile.pos();
             }
-            const auto decryptedBytes = streamingDecryptor.chunkDecryption(dummyEncryptionOutputFile.read(toRead).constData(), &chunkedOutputDecrypted, toRead);
-            QVERIFY(decryptedBytes != -1);
-            QVERIFY(decryptedBytes == toRead || streamingDecryptor.isFinished());
+
+            if (bytesRemaining - toRead != 0 && bytesRemaining - toRead < OCC::Constants::e2EeTagSize) {
+                // decryption is going to fail if last chunk does not include or does not equal to OCC::Constants::e2EeTagSize bytes tag
+                // since we are emulating random size of network packets, we may end up reading beyond OCC::Constants::e2EeTagSize bytes tag at the end
+                // in that case, we don't want to try and decrypt less than OCC::Constants::e2EeTagSize ending bytes of tag, we will accumulate all the incoming data till the end
+                // and then, we are going to decrypt the entire chunk containing OCC::Constants::e2EeTagSize bytes at the end
+                pendingBytes += dummyEncryptionOutputFile.read(bytesRemaining);
+                continue;
+            }
+
+            const auto decryptedChunk = streamingDecryptor.chunkDecryption(dummyEncryptionOutputFile.read(toRead).constData(), toRead);
+
+            QVERIFY(decryptedChunk.size() == toRead || streamingDecryptor.isFinished() || !pendingBytes.isEmpty());
+
+            chunkedOutputDecrypted.write(decryptedChunk);
         }
+
+        if (!pendingBytes.isEmpty()) {
+            const auto decryptedChunk = streamingDecryptor.chunkDecryption(pendingBytes.constData(), pendingBytes.size());
+
+            QVERIFY(decryptedChunk.size() == pendingBytes.size() || streamingDecryptor.isFinished());
+
+            chunkedOutputDecrypted.write(decryptedChunk);
+        }
+
         chunkedOutputDecrypted.close();
 
         QVERIFY(chunkedOutputDecrypted.open(QBuffer::ReadOnly));