Server: Moving files/folders from/to shared folders causes Encryption Errors

[arcanatigris]

Steps to reproduce

1. Enable Encryption
2. Let user1 share 2 folders, place some dummy content in folder1 with multiple layers of directories. (folder1/test/test2/file.txt)
3. Let user2 move the folder inside folder1 to folder2 (Using web GUI)
4. Process get stuck and Encryption errors in Logging

Expected behaviour

Folder should move without errors

Actual behaviour

Encryption errors

Server configuration

Operating system: Debian 9

Web server: NGINX

Database: MariaDB, Redis

PHP version: 7.3

Nextcloud version: 16.0.3

Updated from an older Nextcloud/ownCloud or fresh install: fresh

Where did you install Nextcloud from: latest archive

Signing status:

Signing status

No errors have been found.

List of activated apps:

App list

Enabled:
  - accessibility: 1.2.0
  - activity: 2.9.1
  - bruteforcesettings: 1.4.0
  - cloud_federation_api: 0.2.0
  - comments: 1.6.0
  - dav: 1.9.2
  - encryption: 2.4.0
  - federatedfilesharing: 1.6.0
  - federation: 1.6.0
  - files: 1.11.0
  - files_pdfviewer: 1.5.0
  - files_rightclick: 0.13.0
  - files_sharing: 1.8.0
  - files_texteditor: 2.8.0
  - files_trashbin: 1.6.0
  - files_versions: 1.9.0
  - files_videoplayer: 1.5.0
  - firstrunwizard: 2.5.0
  - gallery: 18.3.0
  - logreader: 2.1.0
  - lookup_server_connector: 1.4.0
  - nextcloud_announcements: 1.5.0
  - notifications: 2.4.1
  - oauth2: 1.4.2
  - password_policy: 1.6.0
  - privacy: 1.0.0
  - provisioning_api: 1.6.0
  - recommendations: 0.4.0
  - serverinfo: 1.6.0
  - sharebymail: 1.6.0
  - support: 1.0.0
  - survey_client: 1.4.0
  - systemtags: 1.6.0
  - theming: 1.7.0
  - twofactor_backupcodes: 1.5.0
  - twofactor_u2f: 3.0.0
  - updatenotification: 1.6.0
  - viewer: 1.0.0
  - workflowengine: 1.6.0
Disabled:
  - admin_audit
  - files_external
  - user_ldap

Nextcloud configuration:

Config report

{
    "system": {
        "instanceid": "***REMOVED SENSITIVE VALUE***",
        "passwordsalt": "***REMOVED SENSITIVE VALUE***",
        "secret": "***REMOVED SENSITIVE VALUE***",
        "trusted_domains": [
            "***REMOVED SENSITIVE VALUE***"
        ],
        "datadirectory": "***REMOVED SENSITIVE VALUE***",
        "dbtype": "mysql",
        "version": "16.0.3.0",
        "overwrite.cli.url": "https:\/\/nc.yisp.nl",
        "dbname": "***REMOVED SENSITIVE VALUE***",
        "dbhost": "***REMOVED SENSITIVE VALUE***",
        "dbport": "",
        "dbtableprefix": "oc_",
        "mysql.utf8mb4": true,
        "dbuser": "***REMOVED SENSITIVE VALUE***",
        "dbpassword": "***REMOVED SENSITIVE VALUE***",
        "installed": true,
        "maintenance": false,
        "memcache.locking": "\\OC\\Memcache\\Redis",
        "memcache.distributed": "\\OC\\Memcache\\Redis",
        "redis": {
            "host": "***REMOVED SENSITIVE VALUE***",
            "port": 0
        }
    }
}

Are you using external storage, if yes which one: no

Are you using encryption: yes

Are you using an external user-backend, if yes which one: no

Client configuration

Browser: Chrome

Operating system: Ubuntu 19.04

Logs

Nextcloud log GUI

Nextcloud log

OCP\Encryption\Exceptions\GenericEncryptionException: Bad Signature
/var/www/nextcloud/apps/encryption/lib/Crypto/Crypt.php - line 467:

OCA\Encryption\Crypto\Crypt->checkSignature("d+nOgAED6pO ... E", null, "65ac461c517 ... 5")

/var/www/nextcloud/apps/encryption/lib/Crypto/Encryption.php - line 379:

OCA\Encryption\Crypto\Crypt->symmetricDecryptFileContent("*** sensiti ... *", null, "AES-256-CTR", "*** sensiti ... *", "*** sensiti ... *")

/var/www/nextcloud/lib/private/Files/Stream/Encryption.php - line 479:

OCA\Encryption\Crypto\Encryption->decrypt("*** sensiti ... *")

/var/www/nextcloud/lib/private/Files/Stream/Encryption.php - line 299:

OC\Files\Stream\Encryption->readCache()

<<closure>>

OC\Files\Stream\Encryption->stream_read(8192)

/var/www/nextcloud/3rdparty/icewind/streams/src/Wrapper.php - line 91:

fread(null, 8192)

/var/www/nextcloud/3rdparty/icewind/streams/src/CallbackWrapper.php - line 98:

Icewind\Streams\Wrapper->stream_read(8192)

<<closure>>

Icewind\Streams\CallbackWrapper->stream_read(8192)

/var/www/nextcloud/3rdparty/sabre/http/lib/Sapi.php - line 80:

stream_copy_to_stream(null, null, "12624")

/var/www/nextcloud/3rdparty/sabre/dav/lib/DAV/Server.php - line 498:

Sabre\HTTP\Sapi::sendResponse(Sabre\HTTP\Response {})

/var/www/nextcloud/3rdparty/sabre/dav/lib/DAV/Server.php - line 254:

Sabre\DAV\Server->invokeMethod(Sabre\HTTP\R ... "}, Sabre\HTTP\Response {})

/var/www/nextcloud/apps/dav/lib/Server.php - line 316:

Sabre\DAV\Server->exec()

/var/www/nextcloud/apps/dav/appinfo/v2/remote.php - line 35:

OCA\DAV\Server->exec()

/var/www/nextcloud/remote.php - line 163:

require_once("/var/www/ne ... p")

[yahesh]

It seems that we have the same issue (see https://github.com/nextcloud/desktop/issues/1168). We're currently looking into this issue as the corresponding restores take a long time and we'd like to have this issue resolved.

We're trying to reproduce this problem by creating two shared folders and moving huge subfolders between those two shared folders.

One thing we've seen is that at some point the web UI issues as warning message that moving the subfolder failed. However, when watching the target folder we see that files are still moved after this message has been issued. Even after all files seem to have been moved on disk, the corresponding PHP process is still hard at work and the database is active as well. (As long as this task isn't finished, the moved files don't show up in the web UI.)

As we've learned, when moving files around the oc_filecache table has to be rewritten for every single file, which is important for signature checks as the database contains the encrypted value (which is actually a version number) that is required to derive the passphase that is used to calculate the HMAC "signatures" in the moved files.

Furthermore, it seems as if files get completely re-encrypted when copying or moving them between shared folder. In contrast, they "only" get re-encrypted when being copied within the same shared folder. ~Unnecessary re-encryptions increase the risk of data loss during these actions.~ (We found out that Nextcloud doesn't move the file but actually creates a copy and with that re-encrypts the file.)

[yahesh]

We now found a reliable way that breaks the signature in 100% of our test cases.

Howto:

1. Create a new text file in a shared folder.
2. Enter some text and wait until it is saved.
3. Enter some more text and wait until it is saved.
4. Try to move the file into another shared folder. You'll get an Could not move "filename.txt" error. At this stage the file is already broken. The working copy of the file will have been moved to the trashbin of the owner of the source folder. A broken copy will be stored on disk in the target folder but the oc_filecache table will not yet have been updated - so the broken copy of the file will not be visible through the web UI.
5. If you have the web UI still open in which you tried to move the file then you have the possibility to "move" the file again. You'll get an Could not move "filename.txt", target exists error because the file already exists on disk but this second try updates the oc_filecache table to make the file visible. The other alternative is to do a ./occ files:scan.

There are some more things we found out in the meantime: We were wondering why the files got re-encrypted when moving them from one shared folder to another shared folder. The reason for the re-encryption is that the file is not really moved but it is rather copied over and then deleted. This leads to several things happening:

• In some cases this re-encryption breaks, making the file unreadable for all users.
• The original copy of the file is moved to the trashbin of the owner of the source folder. This unaltered copy still works.
• Whenever a file is removed from a shared folder, a re-encrypted copy of the file is stored in the trashbin of every user that is allowed to read that shared folder. Depending on the number of users and on the number and size of the files this can lead to a massive storage consumption. This also seems to be the reason why there is so much processing going on in the background. Each moved file has to be re-encrypted for the trashbin of each user with access rights. And all those newly created copies get a separate entry in the oc_filecache table as well.

[yahesh]

Final revelation: Encrypted files must have an encrypted value higher or equal to 1 in oc_filecache or otherwise Nextcloud will assume a bad signature in an encrypted file even if the signature itself is correct.

We found out that Nextcloud stores the value 0 as the encrypted value in oc_filecache for the example file shown above, even though the file is signed with the value 1. Updating the oc_filecache table accordingly fixes the Bad Signature error for this single issue.

The problem persists that moved files may be signed with other encrypted values than 1, we're not sure yet whether rewriting this value in oc_filecache fixes all Bad Signature errors.

[yahesh]

To be able to debug all this we have written two helpful tools by reimplementing the signature checks (and updates) of Nextcloud and by reimplementing the decryption process of Nextcloud. These are standalone scripts that need some configuration information of Nextcloud, but not the Nextcloud codebase itself:

• check-signature.php is able to check the correctness of master-key-encrypted master private keys, pub share private keys, files, version files, trashed files and trashed version files. It is also able to rewrite the signatures within the mentioned file types (by setting the experimental FIXSIGNATURES constant). As we didn't want to implement a database abstraction layer we rely on partial CSV dumps of the two tables oc_filecache and oc_storages. The comment in the script gives an example of how to create these dumps from PostgreSQL.
• decrypt-file.php is able to decrypt a single master-key-encrypted file, version file, trashed file or trashed version file. It ignores the signatures contained in the file and simply writes the decrypted content to STDOUT.

[jospoortvliet]

Thanks for the extensive debugging, that will sure help finding and fixing the issue!

[nickvergessen]

cc @icewind1991

[yahesh]

I just wanted to document some more thoughts on the design of the encryption and signature scheme:

• We were wondering why the key used for the signature has the form $filekey.$version.$position instead of just $filekey. Adding the $position info prevents a properly signed block from being moved within a file (because there would be a $position mismatch). As different file versions reuse the same $filekey, adding the $version info prevents a properly signed block from being moved from one version of the file into another version of the file.
• We were also wondering why the last block uses a slightly different signature key in the form of $filekey.$version.$position."end". By having a different key for the last block you prevent the file from being shortened by one or more blocks as then the signature of the last block wouldn't match anymore.
• Finally: We were wondering why the key for the signature didn't use some identifier of the actual file being signed. But some kind of identifier is in fact included indirectly through $filekey as each new file has its individual file encryption key (as long as it's not a versioned file which is distinguished through $version instead).
• This final revelation in our opinion also is the reason why files have to be re-encrypted when copies of them are created. Otherwise you would have separate files with the same file encryption key and thus would open up a possibility for blocks to be swapped between independent files (as long as their $version and $position within the file also match).

[yahesh]

To get a complete understanding of the inner workings of the default encryption module we now also implemented the support for public sharing keys, recovery keys and user keys in our nextcloud-tools. Our goal is to create a document containing our newly-gained knowledge about how the encryption works.

[yahesh]

We're currently in the process of testing our Nextcloud dataset with the written tools. We stumbled upon lots of files that seemingly have an encrypted value of 0 assigned to them. When we looked into the database, we saw that the files actually have two entries in the oc_filecache table:

• One of them with path in the form of <username>/files/<filename> and storage equal to 1 (which denotes the "data directory" according to the oc_storages table). These entries have the encrypted field set to 1 for regular files.
• The other one with path in the form of files/<filename> and storage with the ID of the home directory of <username> according to the oc_storages table. These entries have the encrypted field set to 0 for regular files.

We're not sure yet where these duplicate entries come from. Even in our test installation some files have duplicate entries in the mentioned formats.

[yahesh]

We also found that you should not use ./occ files:scan when operating an encrypted Nextcloud instance (for example to correct the database after moving files on the file level). Files are added to the oc_filecache table with encrypted equal to 0 even if the files are encrypted.

[nickvergessen]

Uff, yeah files:scan should be really used with care. I would argue to even disable it when encryption is on.

cc @MorrisJobke @rullzer

[MorrisJobke]

We also found that you should not use ./occ files:scan when operating an encrypted Nextcloud instance (for example to correct the database after moving files on the file level). Files are added to the oc_filecache table with encrypted equal to 0 even if the files are encrypted.

It's hard to know when a file is encrypted ... it could also be just a normal file with similar headers. Thus the file:scan can't do much here. Also moving files on the hard disk is just not supported. Doing so causes always (also non-encrypted) issues, because it is from our side just guessing and files are simply just removed and newly added and thus shares are also lost. Long story short: don't move files on filesystem level if you do not want to loose metadata (like shares, encryption state, tags, activities, versions, ...).

[yahesh]

@MorrisJobke Don't misunderstand. This is just one thing we noticed while testing how the encryption module reacts. The problem with bad signature checks also occurs when moving files on the application level (see the mentioned text file example above).

As there doesn't seem to be an extensive description of how the encryption works we have to find this out ourselves.

[yahesh]

@MorrisJobke @nextcloud/encryption @nickvergessen @rullzer Due to this issue why dug pretty deep into the default encryption module and gathered at lot of knowledge about the inner workings of the encryption and signature processes. We created a document called server-side-encryption.md that contains the general knowledge and tiny details that we learned.

Is there the possibility to add this to the official documentation of the encryption module so that this knowledge is easier to find for others?

[nickvergessen]

I guess it could either go into https://github.com/nextcloud/documentation/blob/master/admin_manual/configuration_files/encryption_configuration.rst or in a new page next to it.

[yahesh]

Now that we know how to calculate the MACs and decrypt files we started looking into the actual problems of the encryption module more closely:

We started by creating a reproducible failure again. To do this we created three files called bild.png. We upload the first version of the picture (A) into the shared folder shared. Then we upload the second version of the picture (B) into the same shared folder and finally we upload the third version of the picture (C) into the same shared folder again. The resulting database entries for select fileid, storage, path, encrypted from oc_filecache where path like '%bild%' order by fileid; look like this:

 fileid | storage |                                      path                                      | encrypted 
--------+---------+--------------------------------------------------------------------------------+-----------
    957 |       1 | files_encryption/keys/files/shared/bild.png                                    |         0
    958 |       1 | files_encryption/keys/files/shared/bild.png/OC_DEFAULT_MODULE                  |         0
    959 |       1 | files/shared/bild.png                                                          |         3
    962 |       1 | files_versions/shared/bild.png.v1564491200                                     |         1
    963 |       1 | files_encryption/keys/files/shared/bild.png/OC_DEFAULT_MODULE/fileKey          |         0
    964 |       1 | files_encryption/keys/files/shared/bild.png/OC_DEFAULT_MODULE/ncadmin.shareKey |         0
    965 |       1 | files_encryption/keys/files/shared/bild.png/OC_DEFAULT_MODULE/kenny.shareKey   |         0
    968 |       1 | files_versions/shared/bild.png.v1564491211                                     |         2

Fileid 959 is the third version of the picture (C), fileid 968 is the second version of the file (B) and fileid 962 is the first version of the picture (A). We see that C has the field encrypted set to 3, B has it set to 2 while A has it set to 1.

Now we try to move bild.png to the shared folder shared2. This will break expectedly with an Could not move "bild.png" error. The database now looks like this:

 fileid | storage |                                                path                                                | encrypted 
--------+---------+----------------------------------------------------------------------------------------------------+-----------
    957 |       1 | files_encryption/keys/files_trashbin/files/bild.png.d1564492024                                    |         0
    958 |       1 | files_encryption/keys/files_trashbin/files/bild.png.d1564492024/OC_DEFAULT_MODULE                  |         0
    959 |       1 | files_trashbin/files/bild.png.d1564492024                                                          |         1
    963 |       1 | files_encryption/keys/files_trashbin/files/bild.png.d1564492024/OC_DEFAULT_MODULE/fileKey          |         0
    964 |       1 | files_encryption/keys/files_trashbin/files/bild.png.d1564492024/OC_DEFAULT_MODULE/ncadmin.shareKey |         0
    965 |       1 | files_encryption/keys/files_trashbin/files/bild.png.d1564492024/OC_DEFAULT_MODULE/kenny.shareKey   |         0
    968 |       3 | files_trashbin/versions/bild.png.v1564491211.d1564492024                                           |         2
    971 |       1 | files_encryption/keys/files/shared2/bild.png                                                       |         0
    972 |       1 | files_encryption/keys/files/shared2/bild.png/OC_DEFAULT_MODULE                                     |         0
    973 |       1 | files_encryption/keys/files/shared2/bild.png/OC_DEFAULT_MODULE/fileKey                             |         0
    974 |       1 | files_encryption/keys/files/shared2/bild.png/OC_DEFAULT_MODULE/ncadmin.shareKey                    |         0
    975 |       1 | files_encryption/keys/files/shared2/bild.png/OC_DEFAULT_MODULE/kenny.shareKey                      |         0
    982 |       1 | files_trashbin/versions/bild.png.v1564491211.d1564492024                                           |         2
    985 |       3 | files_encryption/keys/files_trashbin/files/bild.png.d1564492024                                    |         0
    986 |       3 | files_encryption/keys/files_trashbin/files/bild.png.d1564492024/OC_DEFAULT_MODULE                  |         0
    987 |       3 | files_trashbin/files/bild.png.d1564492024                                                          |         0
    988 |       3 | files_encryption/keys/files_trashbin/files/bild.png.d1564492024/OC_DEFAULT_MODULE/fileKey          |         0
    989 |       3 | files_encryption/keys/files_trashbin/files/bild.png.d1564492024/OC_DEFAULT_MODULE/kenny.shareKey   |         0

Fileid 959 (which is C) has been rewritten to be located in the trashbin now but its encrypted field has been changed to 1 even though its signed with the version 3. This breaks the signature of the trashed file and it will also stay broken after a restore.

Fileid 987 is a copy of C which has been put into the trashbin of the user the file has been shared with but its encrypted field has been changed to 0. This would mean that the file is not encrypted but it actually is encrypted, also breaking the signature of this trashed file.

Fileids 968 and 982 are the trashed version files containing B and the version information has stayed intact. The version file containing A has been lost during the moving process.

The actual file containing C that was meant to be moved isn't written to the database at all but it exists on disk. If you still have the website opened where you initially tried to move the file you have the possibility to try and move the file again. This will result in an Could not move "bild.png", target exists error but this will at least fix the database a bit:

 fileid | storage |                                                path                                                | encrypted 
--------+---------+----------------------------------------------------------------------------------------------------+-----------
    957 |       1 | files_encryption/keys/files_trashbin/files/bild.png.d1564492024                                    |         0
    958 |       1 | files_encryption/keys/files_trashbin/files/bild.png.d1564492024/OC_DEFAULT_MODULE                  |         0
    959 |       1 | files_trashbin/files/bild.png.d1564492024                                                          |         1
    963 |       1 | files_encryption/keys/files_trashbin/files/bild.png.d1564492024/OC_DEFAULT_MODULE/fileKey          |         0
    964 |       1 | files_encryption/keys/files_trashbin/files/bild.png.d1564492024/OC_DEFAULT_MODULE/ncadmin.shareKey |         0
    965 |       1 | files_encryption/keys/files_trashbin/files/bild.png.d1564492024/OC_DEFAULT_MODULE/kenny.shareKey   |         0
    968 |       3 | files_trashbin/versions/bild.png.v1564491211.d1564492024                                           |         2
    971 |       1 | files_encryption/keys/files/shared2/bild.png                                                       |         0
    972 |       1 | files_encryption/keys/files/shared2/bild.png/OC_DEFAULT_MODULE                                     |         0
    973 |       1 | files_encryption/keys/files/shared2/bild.png/OC_DEFAULT_MODULE/fileKey                             |         0
    974 |       1 | files_encryption/keys/files/shared2/bild.png/OC_DEFAULT_MODULE/ncadmin.shareKey                    |         0
    975 |       1 | files_encryption/keys/files/shared2/bild.png/OC_DEFAULT_MODULE/kenny.shareKey                      |         0
    982 |       1 | files_trashbin/versions/bild.png.v1564491211.d1564492024                                           |         2
    985 |       3 | files_encryption/keys/files_trashbin/files/bild.png.d1564492024                                    |         0
    986 |       3 | files_encryption/keys/files_trashbin/files/bild.png.d1564492024/OC_DEFAULT_MODULE                  |         0
    987 |       3 | files_trashbin/files/bild.png.d1564492024                                                          |         0
    988 |       3 | files_encryption/keys/files_trashbin/files/bild.png.d1564492024/OC_DEFAULT_MODULE/fileKey          |         0
    989 |       3 | files_encryption/keys/files_trashbin/files/bild.png.d1564492024/OC_DEFAULT_MODULE/kenny.shareKey   |         0
    990 |       3 | files_encryption/keys/files_trashbin/files/bild.png.d1564492024/OC_DEFAULT_MODULE/ncadmin.shareKey |         0
    991 |       1 | files/shared2/bild.png                                                                             |         0

As you can see the actual file containing C has now been added as fileid 991 but the encrypted field has been set to 0. This would mean that the file is not encrypted but it actually is encrypted, also breaking the signature of this trashed file. Additionally, a previously missing shareKey file has been added to the database.

[yahesh]

Some more things we learned about copying and moving files around:

• We learned that files with multiple versions can be moved from one shared folder to another shared folder without a problem if both folders are owned by the user executing the file move. No copies of the moved file are put in the trashbin of the recipients.
• We learned that files with multiple versions can be moved from one shared folder to another shared folder if only the source folder is owned by the user executing the file move but a copy of that file will be put in the trashbin of that user and the oc_filecache table will have the wrong encrypted value for that trashed file.
• We learned that files with multiple versions cannot be moved from one shared folder to another shared folder if only the target folder is owned by the user executing the file move.
• We learned that files with multiple versions cannot be moved from one shared folder to another shared folder if no folder is owned by the user executing the file move.
• When the owner of the shared folder deletes a file then it is just moved to their trashbin.
• When a recipient of the shared folder moves a file then it is copied over and each recipient (including the owner) gets another copy of the file added to the trashbin.
• When a recipient of the shared folder deletes a file then each recipient (including the owner) gets a copy of the file added to the trashbin.
• Moving files with multiple versions between shared folders breaks. Copying files with multiple versions between shared folders doesn't break.
• Copying a file somewhere resets the encrypted value of the created file copy back to 1. Copies of files also lose the versions of the source file.

This leads us to the conclusion that files seem to be handled differently depending on whether they are handled by the owner of the shared folder or by the recipient of a shared folder. Putting broken files in the trashbin and failing to properly move a file to a shared folder seem to be two different problems. One seems to be related with the ownership of the source folder while the other seems to be related to the ownership of the target folder.

There are also some database inconsistencies that we saw during our tests:

• When a file is first uploaded then only the following entries are written to the database:

 fileid | storage |                             path                              | encrypted 
--------+---------+---------------------------------------------------------------+-----------
   1095 |       1 | files_encryption/keys/files/shared/bild.png                   |         0
   1096 |       1 | files_encryption/keys/files/shared/bild.png/OC_DEFAULT_MODULE |         0
   1097 |       1 | files/shared/bild.png                                         |         1

• The contents of the OC_DEFAULT_MODULE folder is only added to the database when a new version of that file is added or if it is moved. However, these entries don't seem to be necessary for Nextcloud to properly decrypt the file:

 fileid | storage |                                      path                                      | encrypted 
--------+---------+--------------------------------------------------------------------------------+-----------
   1095 |       1 | files_encryption/keys/files/shared/bild.png                                    |         0
   1096 |       1 | files_encryption/keys/files/shared/bild.png/OC_DEFAULT_MODULE                  |         0
   1097 |       1 | files/shared/bild.png                                                          |         2
   1100 |       1 | files_versions/shared/bild.png.v1564503210                                     |         1
   1101 |       1 | files_encryption/keys/files/shared/bild.png/OC_DEFAULT_MODULE/fileKey          |         0
   1102 |       1 | files_encryption/keys/files/shared/bild.png/OC_DEFAULT_MODULE/ncadmin.shareKey |         0
   1103 |       1 | files_encryption/keys/files/shared/bild.png/OC_DEFAULT_MODULE/kenny.shareKey   |         0

• If the contents of the OC_DEFAULT_MODULE folder has been added to the database then it is properly rewritten when moving the file.
• The contents of the OC_DEFAULT_MODULE folder will not be added to the database if the file is copied somewhere (even if the contents of the OC_DEFAULT_MODULE folder had beed added to the database for the source file).

[yahesh]

We started to debug Nextcloud and have seen that $fullPath values like $datadirectory/$username//$username/files_encryption/keys/files_trashbin/files/$filename.d$timestamp/OC_DEFAULT_MODULE/fileKey pop up in OC\\Files\\Storage\\Local:getSourcePath().

It seems as if there is some problem with the generation of the path string during the deletion of the moved file which might probably make the whole move operation fail at some point.

This is a typical call stack where this happens:

array (
  0 =>
  array (
    'file' => '/var/www/nextcloud/lib/private/Files/Storage/Local.php',
    'function' => 'getSourcePath',
    'class' => 'OC\\Files\\Storage\\Local',
  ),
  1 =>
  array (
    'file' => '/var/www/nextcloud/lib/private/Files/Storage/Wrapper/Encryption.php',
    'function' => 'file_exists',
    'class' => 'OC\\Files\\Storage\\Local',
  ),
  2 =>
  array (
    'file' => '/var/www/nextcloud/lib/private/Files/Storage/Wrapper/Encryption.php',
    'function' => 'getHeader',
    'class' => 'OC\\Files\\Storage\\Wrapper\\Encryption',
  ),
  3 =>
  array (
    'file' => '/var/www/nextcloud/lib/private/Files/Storage/Wrapper/Encryption.php',
    'function' => 'getEncryptionModule',
    'class' => 'OC\\Files\\Storage\\Wrapper\\Encryption',
  ),
  4 =>
  array (
    'file' => '/var/www/nextcloud/lib/private/Files/Storage/Wrapper/Encryption.php',
    'function' => 'shouldEncrypt',
    'class' => 'OC\\Files\\Storage\\Wrapper\\Encryption',
  ),
  5 =>
  array (
    'file' => '/var/www/nextcloud/lib/private/Files/Storage/Wrapper/Encryption.php',
    'function' => 'updateEncryptedVersion',
    'class' => 'OC\\Files\\Storage\\Wrapper\\Encryption',
  ),
  6 =>
  array (
    'file' => '/var/www/nextcloud/lib/private/Files/Storage/Wrapper/Encryption.php',
    'function' => 'copyBetweenStorage',
    'class' => 'OC\\Files\\Storage\\Wrapper\\Encryption',
  ),
  7 =>
  array (
    'file' => '/var/www/nextcloud/lib/private/Files/Storage/Wrapper/Encryption.php',
    'function' => 'copyFromStorage',
    'class' => 'OC\\Files\\Storage\\Wrapper\\Encryption',
  ),
  8 =>
  array (
    'file' => '/var/www/nextcloud/lib/private/Files/Storage/Wrapper/Encryption.php',
    'function' => 'copyBetweenStorage',
    'class' => 'OC\\Files\\Storage\\Wrapper\\Encryption',
  ),
  9 =>
  array (
    'file' => '/var/www/nextcloud/lib/private/Files/Storage/Wrapper/Encryption.php',
    'function' => 'copyFromStorage',
    'class' => 'OC\\Files\\Storage\\Wrapper\\Encryption',
  ),
  10 =>
  array (
    'file' => '/var/www/nextcloud/lib/private/Files/Storage/Wrapper/Encryption.php',
    'function' => 'copyBetweenStorage',
    'class' => 'OC\\Files\\Storage\\Wrapper\\Encryption',
  ),
  11 =>
  array (
    'file' => '/var/www/nextcloud/lib/private/Files/Storage/Wrapper/Wrapper.php',
    'function' => 'copyFromStorage',
    'class' => 'OC\\Files\\Storage\\Wrapper\\Encryption',
  ),
  12 =>
  array (
    'file' => '/var/www/nextcloud/lib/private/Files/View.php',
    'function' => 'copyFromStorage',
    'class' => 'OC\\Files\\Storage\\Wrapper\\Wrapper',
  ),
  13 =>
  array (
    'file' => '/var/www/nextcloud/lib/private/Encryption/Keys/Storage.php',
    'function' => 'copy',
    'class' => 'OC\\Files\\View',
  ),
  14 =>
  array (
    'file' => '/var/www/nextcloud/lib/private/Files/Storage/Wrapper/Encryption.php',
    'function' => 'copyKeys',
    'class' => 'OC\\Encryption\\Keys\\Storage',
  ),
  15 =>
  array (
    'file' => '/var/www/nextcloud/lib/private/Files/Storage/Wrapper/Encryption.php',
    'function' => 'copyKeys',
    'class' => 'OC\\Files\\Storage\\Wrapper\\Encryption',
  ),
  16 =>
  array (
    'file' => '/var/www/nextcloud/lib/private/Files/Storage/Wrapper/Encryption.php',
    'function' => 'copyBetweenStorage',
    'class' => 'OC\\Files\\Storage\\Wrapper\\Encryption',
  ),
  17 =>
  array (
    'file' => '/var/www/nextcloud/lib/private/Files/Storage/Wrapper/Wrapper.php',
    'function' => 'copyFromStorage',
    'class' => 'OC\\Files\\Storage\\Wrapper\\Encryption',
  ),
  18 =>
  array (
    'file' => '/var/www/nextcloud/lib/private/Files/View.php',
    'function' => 'copyFromStorage',
    'class' => 'OC\\Files\\Storage\\Wrapper\\Wrapper',
  ),
  19 =>
  array (
    'file' => '/var/www/nextcloud/apps/files_trashbin/lib/Trashbin.php',
    'function' => 'copy',
    'class' => 'OC\\Files\\View',
  ),
  20 =>
  array (
    'file' => '/var/www/nextcloud/apps/files_trashbin/lib/Trashbin.php',
    'function' => 'copy_recursive',
    'class' => 'OCA\\Files_Trashbin\\Trashbin',
  ),
  21 =>
  array (
    'file' => '/var/www/nextcloud/apps/files_trashbin/lib/Trashbin.php',
    'function' => 'copyFilesToUser',
    'class' => 'OCA\\Files_Trashbin\\Trashbin',
  ),
  22 =>
  array (
    'file' => '/var/www/nextcloud/apps/files_trashbin/lib/Trash/LegacyTrashBackend.php',
    'function' => 'move2trash',
    'class' => 'OCA\\Files_Trashbin\\Trashbin',
  ),
  23 =>
  array (
    'file' => '/var/www/nextcloud/apps/files_trashbin/lib/Trash/TrashManager.php',
    'function' => 'moveToTrash',
    'class' => 'OCA\\Files_Trashbin\\Trash\\LegacyTrashBackend',
  ),
  24 =>
  array (
    'file' => '/var/www/nextcloud/apps/files_trashbin/lib/Storage.php',
    'function' => 'moveToTrash',
    'class' => 'OCA\\Files_Trashbin\\Trash\\TrashManager',
  ),
  25 =>
  array (
    'file' => '/var/www/nextcloud/apps/files_trashbin/lib/Storage.php',
    'function' => 'doDelete',
    'class' => 'OCA\\Files_Trashbin\\Storage',
  ),
  26 =>
  array (
    'file' => '/var/www/nextcloud/lib/private/Files/View.php',
    'function' => 'unlink',
    'class' => 'OCA\\Files_Trashbin\\Storage',
  ),
  27 =>
  array (
    'file' => '/var/www/nextcloud/lib/private/Files/View.php',
    'function' => 'basicOperation',
    'class' => 'OC\\Files\\View',
  ),
  28 =>
  array (
    'file' => '/var/www/nextcloud/apps/dav/lib/Connector/Sabre/File.php',
    'function' => 'unlink',
    'class' => 'OC\\Files\\View',
  ),
  29 =>
  array (
    'file' => '/var/www/nextcloud/3rdparty/sabre/dav/lib/DAV/Tree.php',
    'function' => 'delete',
    'class' => 'OCA\\DAV\\Connector\\Sabre\\File',
  ),
  30 =>
  array (
    'file' => '/var/www/nextcloud/3rdparty/sabre/dav/lib/DAV/CorePlugin.php',
    'function' => 'delete',
    'class' => 'Sabre\\DAV\\Tree',
  ),
  31 =>
  array (
    'function' => 'httpDelete',
    'class' => 'Sabre\\DAV\\CorePlugin',
  ),
  32 =>
  array (
    'file' => '/var/www/nextcloud/3rdparty/sabre/event/lib/EventEmitterTrait.php',
    'function' => 'call_user_func_array',
  ),
  33 =>
  array (
    'file' => '/var/www/nextcloud/3rdparty/sabre/dav/lib/DAV/Server.php',
    'function' => 'emit',
    'class' => 'Sabre\\Event\\EventEmitter',
  ),
  34 =>
  array (
    'file' => '/var/www/nextcloud/3rdparty/sabre/dav/lib/DAV/Server.php',
    'function' => 'invokeMethod',
    'class' => 'Sabre\\DAV\\Server',
  ),
  35 =>
  array (
    'file' => '/var/www/nextcloud/apps/dav/lib/Server.php',
    'function' => 'exec',
    'class' => 'Sabre\\DAV\\Server',
  ),
  36 =>
  array (
    'file' => '/var/www/nextcloud/apps/dav/appinfo/v2/remote.php',
    'function' => 'exec',
    'class' => 'OCA\\DAV\\Server',
  ),
  37 =>
  array (
    'file' => '/var/www/nextcloud/remote.php',
    'function' => 'require_once',
  ),
)

[yahesh]

New learnings: We do know now how to prevent the generation of these malformed paths but this seems to break the encryption at some other point. This is the place we found:

In lib/private/Files/Storage/Wrapper/Encryption.php#L1011 there is the function shouldEncrypt() that checks whether a certain file should be encrypted. In line #L1012 it assigns $fullPath = $this->getFullPath($path);. It seems that getEncryptionModule($fullPath) in #L1019 cannot handle this value correctly.

In addition: Debugging this code is unnecessarily difficult as it uses 5 different representations of the filename in different spots (and even uses different representations within the same class) and it is unclear which method uses which representation at what point and why. These are the representations we found:

• <datadirectory>/<username>/<jail>/<shared folder>/<filename>
• <username>/<jail>/<shared folder>/<filename>
• <jail>/<shared folder>/<filename>
• <shared folder>/<filename>
• <filename>

P.S.: We're also still not sure if this is the actual root cause or just an unrelated finding.

[yahesh]

I think we solved it. We now have an instance in which we can move versioned files (and folders containing versioned files) between shared folders. I'll prepare a pull request tomorrow and will then describe what lead to the broken files.

[the-sane]

@yahesh I said it elsewhere, but I'll say it again: You're my hero of the month for tackling these issues with encryption. Thank you.

[yahesh]

@the-sane Well, we need the provided solutions ourselves. We've stored over a million files in our internal system occupying over a terabyte of storage and the system has become inconsistent because of this problem. It wouldn't be of any help to restore the consistency when it can break so easily again. That's why we worked on a solution to recover broken files first (and to reassure us that we haven't suffered a data loss due to this broken encryption) and now worked on a permanent fix. :)

[yahesh]

I've now created pull request #16696 which solves this problem for master key encrypted setups.

It seems that moving user key encrypted folders leads into some weird locking issues in the $this->file->getAccessList() call of lib/private/Files/Stream/Encryption.php:stream_open() (which we can reproduce reliably but haven't fully understood yet). At least, this locking issue doesn't break files. Copying user key encrypted folders works.

This is a write-down of what the general problem was:
Let's shortly re-iterate on what happens when a file is moved from one shared folder to another shared folder:

• a copy of the file is put into the target folder
• a copy of the file is put into the trashbin of the user moving the file
• the original copy of the file is put into the trashbin of the file owner

The general logic can be found in lib/private/Files/Storage/Wrapper/Encryption.php: moveFromStorage() which first copies the file/folder with the help of lib/private/Files/Storage/Wrapper/Encryption.php:copyBetweenStorage() and then deletes the file/folder.

One task that has to be done is to update the oc_filecache table. Most of this is done by underlying classes but the encrypted value in the database is handled here in lib/private/Files/Storage/Wrapper/Encryption.php:updateEncryptedVersion() and this is also where the problem is buried.

When copyBetweenStorage is first called in moveFromStorage it provides true as the $isRename value. This later prompts updateEncryptedVersion to overwrite the encrypted value of the source file with 1. So back to the process:

• When the source file is copied to the target file, the encrypted value of the source file is changed into a wrong value.
• When the source file is copied to the trashbin of the user moving the file, the encrypted value of the source file doesn't match and the file copy fails because the signature checks fails while reading the content.

What we did for now to remedy this problem:

• At the start of copyBetweenStorage we backup the original encrypted value.
• We let the rest of the program logic untouched because some underlying code seems to rely on this broken behaviour.
• At the end of copyBetweenStorage we restore the original encrypted value so that it is the correct value for the next copy task.

[kesselb]

Thank you @yahesh :+1:

Mind to add your conclusion to the pull request? That makes it easier for the reviewer.

[yahesh]

We found a work around that can be used until this issue is completely fixed: As this problem comes from the broken encrypted value during the copying of the file, it is sufficient to add 'encryption_skip_signature_check' => true, to the configuration to circumvent this problem.

There is one problem with this temporary solution: The original file that is moved into the trashbin of the share owner will have a wrong encrypted value and thus will be broken when the signature check is activated again later on. But the files in the trashbin probably aren't as critical as the live files and we're currently working on a script that will be able to find the correct encrypted values for given files.

[yahesh]

We now implemented a FIX_DATABASE feature in check-signature.php that tries to find the correct encrypted value and also calculates the correct size for an encrypted file. This can be helpful for encrypted files that have been added to oc_filecache via ./occ files:scan or for files that broke during a file move.

[jknockaert]

It's hard to know when a file is encrypted ... it could also be just a normal file with similar headers.

I remember once hitting an issue when I was daisy-chaining next-/owncloud instances through webdav. The (untrusted) external server was guessing encryption status and wouldn't properly serve the encrypted file to the (trusted) server where the encryption key was hosted. So the guessing happens somewhere anyway.

[jknockaert]

@yahesh Thanks for debugging and properly fixing the issue. It was in a similar situation that one day long ago I decided to simply rewrite the encryption wrapper because the original one was terribly inefficient. While doing so I also fixed some issues with sabredav. So thanks again; much appreciated to complement the work done before on the encryption mechanism in nextcloud.

[yahesh]

@jknockaert IMHO the whole file handling is really inefficient. To be able to debug this problem we used xdebug to create the necessary call stacks. The xdebug trace of a single MOVE statement of a 6kb sized file is about 500mb in size. This is, quite frankly, a lot.

[jknockaert]

@yahesh I never really looked into the nextcloud file management code, so I cannot comment. The inefficiency of the encryption wrapper was in its algorithms rather than in the depth of the call stacks.

[yahesh]

As mentioned in #issuecomment-514605972 we found file entry duplicates in oc_filecache where we think that these have been introduced by some former version of ./occ files:scan. To remove these duplicates we now wrote the fix-duplicate.php script that generates delete statements for those duplicates.

Depending on the table size, deleting all duplicates can take a lot of time. To drastically increase the speed of this cleanup, it has proven to be helpful to add a new index to the table through CREATE INDEX fs_storage_path ON oc_filecache(storage, path); and to drop this index afterwards again through ALTER TABLE oc_filecache DROP INDEX fs_storage_path;.

[the-sane]

@yahesh, you previously mentioned that you were seeing those duplicate entries even on your test installation. Does this mean it's an issue that could affect new installs? Does these duplicates cause any serious problems?

Please forgive me if these questions have already been answered. I've been doing my best to follow your research but some of it goes over my head!

[yahesh]

Our current assumption is that these entries aren't harmful and only clutter the oc_filecache table.

We're in the process of testing a procedure to fix the integrity of our own Nextcloud setup. When we're done with this test we'll gain a better understanding of whether these duplicates are relevant or not.

[ghost]

I am equally affected by this bug.
@yahesh any news on your PR ? Would very much love to see this fixed properly and permamently.

[yahesh]

@henry-nicolas Unfortunately, there're no news regarding the PR. Internally, we're currently using the work-around (which deactivates the integrity-protection) which isn't a huge problem for us as we're not relying on external storage.

As far as I've understood, the Nextcloud developers will have a deeper look into the server-side encryption when Nextcloud 18 has been released which is due on January 16th, 2020 (I guess in part also thanks to some other problems I identified within the server-side encryption).

[ghost]

@yahesh could you please detail that workaround a bit please? Neither am I using external storage so I understand I could give it a try as well :)

[yahesh]

@henry-nicolas The work-around is to set the 'encryption_skip_signature_check' => true in your Nextcloud configuration as can be seen in https://github.com/nextcloud/server/issues/16419#issuecomment-519859698.

[inthreedee]

@yahesh I believe I have run into this same issue but without the shared folders aspect. Could you please take a look at #18304 to see if you think it's being caused by the same problem?

[yahesh]

@inthreedee At first sight the two problems don't seem to be related. According to your error logs you get an error when adding the file to the oc_filecache table:

An exception occurred while executing 'UPDATE oc_filecache SET storage = ?, path = ?, path_hash = ?, name = ?, parent = ? WHERE fileid = ?' with params [1, "files_encryption\/keys\/files\/test\/ten\/Screenshot from 2019-12-09 11-13-09 - 1.png", "0059bfdd4df23c5e8e19287fc633c4ea", "Screenshot from 2019-12-09 11-13-09 - 1.png", 102622, 102616]

[inthreedee]

@yahesh Yes, although I get that different error along with the bad signature error, I'm seeing very similar behavior to what you describe here https://github.com/nextcloud/server/issues/16419#issuecomment-513226914, leading me to suspect some common broken move mechanism. In any case, I appreciate you checking, thanks.

[yahesh]

@inthreedee You should have a look at the mentioned oc_filecache table and see whether there are leftover entries in there. There is an index defined with a unique constraint on the two fields storage and path. Nextcloud seems to fail when it tries to add the folder for the encryption keys to said database table as it probably already exists within the table. What you might want to try is delete filecache entries for files/folders that do not exist using ./occ files:cleanup.

[jknockaert]

As far as I've understood, the Nextcloud developers will have a deeper look into the server-side encryption when Nextcloud 18 has been released which is due on January 16th, 2020 (I guess in part also thanks to some other problems I identified within the server-side encryption).

That would be excellent. Server-side encryption is plagued by a number of bugs that result in (meta)data loss (mostly at the file management level). And its documentation is well behind and should be reviewed.

[hostingnuggets]

@MorrisJobke @jospoortvliet is there any status update when we can expect this PR to be merged and back-ported?

@yahesh has done here tremendous work and we are all looking forward to see many-years-old server-side encryption bugs finally fixed...

[frlan]

Any progress here? It's really annoying.

[raceface2nd]

Would be great if a solution would be found. Especially Mac our Mac users are dissatisfied as files within shares on Mac are only moved by the OS which causes loads of encrypted files.

[papimla]

This is a serious issue and should be escalated. Great work on the diagnostic and the proposed fix. Once a problem can be reproduced, it can be fixed :-)

[redtux]

I can confirm this bug still exists (NC 18.0.4.2) and also would love to see the fix being merged into upstream soon. Hetzner support today recommended me to disable encryption which I think would be really sad (and risky). I have been successfully moving the folders via Android app, but ran into troubles while syncing on Ubuntu via PC app. Also from my side lot of thanks to @yahesh and your team for all your efforts! 🤩

[hostingnuggets]

@redtux it is really stupid to have get to the point to have to disable encryption because of this bug not getting fixed, that makes nextcloud insecure :cry: I wonder what are the priorities of nextcloud but it looks like encryption/security is not their priority since 2-3 years.

[redtux]

Let's hope the best! 😃

[papimla]

@redtux it is really stupid to have get to the point ...

Could you please avoid such strong language ? It makes me feel unsafe.

[Rhobil]

I can also confirm that this bug still exists (Version 18.0.6).
Moving shared files into a shared folder of another user with encryption enabled destroys the files.
The files are getting about 40% bigger and can not be read anymore (no valid file header).

[RedKage]

I think this bug still exists as of Nextcloud 19.0.4
I'm glad I found this issue, as I have been plagued with these issues since Owncloud 8 or 9
Then I switched to Nextcloud 11

So this means that there is also an invisible side effect of this, the database contains lots of duplicates in the cache table?

Also to the Nextcloud maintainers: because of that very issue, I tried to get away from nextcloud all together.
Tried many other solutions, like Syncthing and stuff, but no product is as good as Nextcloud.
But I wanted to leave, so bad these bugs are.

They create dataloss. That's not acceptable for a cloud solution.

[redtux]

Yes, me and my company also wanted to leave because of that issue. Let's hope one day it will get solved… 👍❤️

[hostingnuggets]

Same here, still not working correctly and no feedback from the security chiefs at Nextcloud. I would be glad if someone can fix this after so many years.

[inthreedee]

Our friendly neighborhood superhero @yahesh has a pull request in to fix this (https://github.com/nextcloud/server/pull/16696) and progress is, thankfully, being made. Reading through that, there appear to have been some hiccups preventing the merge into v20 and it needs some more review before it can get into v21.

[hostingnuggets]

@inthreedee looking forward to these fixes and hopefully this can happen soon. Please also think about backporting these fixes to Nextcloud 20, in case it ever makes it into Nextcloud 21.

[Kuphi]

Starting some days ago, I get more and more broken files. Yesterday three files got broken, today already 8 files. I did not change anything on my nextcloud. Nextcloud can no longer sync those file to my clients and I can no longer download or open those files. When I click on download, I get ERR_INVALID_RESPONSE, the website can not be reached.
https://cloud.domain.com/remote.php/webdav/username/filename.pdf?downloadStartSecret=1x2x3x4x5x6x

I have encryption enabled, with master key, recovery key and encryption keys for every single user. The broken files seem to only occur in shared folders. But they also occur in shared folders, that are only shared by a dummy user to me alone (only me accessing those files).

I am not sure, if I really lost those files or if I have already renamed and moved those files, but the sync was partially broken, resulting in broken leftover files on the cloud. It seems to primarily affect new scanned documents starting with S30C-xxxxxxxxxxxx.pdf or S30C-xxxxxxxxxxxx.jpg.

Server configuration:
Operating system: Ubuntu server 18.04
Web server: Apache2
Database: MySQL / MariaDB
PHP version: 7.4.3
Nextcloud version: 19.0.4
Updated from an older Nextcloud/ownCloud or fresh install: from 18

[inthreedee]

@kesselb Could we get this added to the faq + known issues list? The fix has been in progress here but it seems to still need more work and might take a while: https://github.com/nextcloud/server/pull/16696

[frlan]

What is the actually reason that this one is not "simple" fixed? Adding a FAQ-entry for an security issue (data loss is security) where a nearly ready patch is available sound weird to me.