Moving files/folders from/to shared folders causes Encryption Errors #16419

arcanatigris · 2019-07-16T10:03:07Z

Steps to reproduce

Enable Encryption
Let user1 share 2 folders, place some dummy content in folder1 with multiple layers of directories. (folder1/test/test2/file.txt)
Let user2 move the folder inside folder1 to folder2 (Using web GUI)
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 · 2019-07-19T11:28:18Z

It seems that we have the same issue (see nextcloud/desktop#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 · 2019-07-19T13:23:30Z

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

Howto:

Create a new text file in a shared folder.
Enter some text and wait until it is saved.
Enter some more text and wait until it is saved.
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.
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 · 2019-07-19T15:11:10Z

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 · 2019-07-19T15:24:27Z

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 · 2019-07-19T18:27:34Z

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

nickvergessen · 2019-07-20T05:14:35Z

cc @icewind1991

yahesh · 2019-07-21T06:49:50Z

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 · 2019-07-22T17:24:48Z

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 · 2019-07-24T12:12:40Z

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 · 2019-07-24T12:14:06Z

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 · 2019-07-24T12:49:49Z

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 · 2019-07-24T13:42:17Z

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 · 2019-07-24T14:06:59Z

@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 · 2019-07-26T16:04:14Z

@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 · 2019-07-29T10:21:21Z

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 · 2019-07-30T13:21:40Z

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 · 2019-07-30T16:57:43Z

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 · 2019-08-02T14:35:09Z

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 · 2019-08-07T05:00:39Z

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 · 2019-08-07T22:34:15Z

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 · 2019-08-07T22:54:41Z

@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 · 2019-08-07T23:05:08Z

@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 · 2019-08-08T11:30:17Z

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 · 2019-08-08T13:26:30Z

Thank you @yahesh 👍

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

yahesh · 2019-08-09T09:56:48Z

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 · 2019-08-09T19:08:45Z

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 · 2019-08-12T08:56:04Z

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 · 2019-08-12T08:59:57Z

@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 · 2019-08-12T12:25:27Z

@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 · 2019-08-12T12:43:32Z

@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 · 2019-08-13T15:55:50Z

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 · 2019-08-14T16:16:04Z

@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 · 2019-08-14T16:54:31Z

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.

henry-nicolas · 2019-12-08T16:48:55Z

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 · 2019-12-08T17:21:39Z

@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).

henry-nicolas · 2019-12-08T21:23:16Z

@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 :)

arcanatigris added 0. Needs triage bug labels Jul 16, 2019

yahesh mentioned this issue Jul 16, 2019

Moving a lot of encrypted files makes them unaccessible #1168

Closed

yahesh added feature: encryption (server-side) feature: sharing and removed feature: encryption (server-side) feature: sharing labels Jul 19, 2019

nickvergessen added 1. to develop feature: encryption (server-side) feature: sharing feature: trashbin and removed 0. Needs triage labels Jul 20, 2019

yahesh mentioned this issue Jul 29, 2019

added the encryption details page which decribes the server-side encr… #1571

Merged

yahesh mentioned this issue Aug 8, 2019

fixed moving of master key encrypted and versioned files between shared folders #16696

Open

yahesh added 2. developing security and removed 1. to develop labels Aug 8, 2019

the-sane mentioned this issue Aug 15, 2019

Question: How does snap revert handle server-side encryption? #1087

Closed

Sponsor nextcloud/server

nextcloud/server

Join GitHub today

Moving files/folders from/to shared folders causes Encryption Errors #16419

Comments

arcanatigris commented Jul 16, 2019 • edited

Steps to reproduce

Expected behaviour

Actual behaviour

Server configuration

Client configuration

Logs

Nextcloud log GUI

This comment has been minimized.

yahesh commented Jul 19, 2019 • edited

This comment has been minimized.

yahesh commented Jul 19, 2019 • edited

This comment has been minimized.

yahesh commented Jul 19, 2019 • edited

This comment has been minimized.

yahesh commented Jul 19, 2019 • edited

This comment has been minimized.

jospoortvliet commented Jul 19, 2019

This comment has been minimized.

nickvergessen commented Jul 20, 2019

This comment has been minimized.

yahesh commented Jul 21, 2019 • edited

This comment has been minimized.

yahesh commented Jul 22, 2019

This comment has been minimized.

yahesh commented Jul 24, 2019 • edited

This comment has been minimized.

yahesh commented Jul 24, 2019 • edited

This comment has been minimized.

nickvergessen commented Jul 24, 2019

This comment has been minimized.

MorrisJobke commented Jul 24, 2019

This comment has been minimized.

yahesh commented Jul 24, 2019 • edited

This comment has been minimized.

yahesh commented Jul 26, 2019

This comment has been minimized.

nickvergessen commented Jul 29, 2019

This comment has been minimized.

yahesh commented Jul 30, 2019 • edited

This comment has been minimized.

yahesh commented Jul 30, 2019 • edited

This comment has been minimized.

yahesh commented Aug 2, 2019

This comment has been minimized.

yahesh commented Aug 7, 2019 • edited

This comment has been minimized.

yahesh commented Aug 7, 2019

This comment has been minimized.

the-sane commented Aug 7, 2019

This comment has been minimized.

yahesh commented Aug 7, 2019

This comment has been minimized.

yahesh commented Aug 8, 2019 • edited

This comment has been minimized.

kesselb commented Aug 8, 2019

This comment has been minimized.

yahesh commented Aug 9, 2019 • edited

This comment has been minimized.

yahesh commented Aug 9, 2019

This comment has been minimized.

jknockaert commented Aug 12, 2019

This comment has been minimized.

jknockaert commented Aug 12, 2019

This comment has been minimized.

yahesh commented Aug 12, 2019

This comment has been minimized.

jknockaert commented Aug 12, 2019

This comment has been minimized.

yahesh commented Aug 13, 2019 • edited

This comment has been minimized.

the-sane commented Aug 14, 2019

This comment has been minimized.

yahesh commented Aug 14, 2019 • edited

This comment has been minimized.

arcanatigris commented Jul 16, 2019 •

edited

yahesh commented Jul 19, 2019 •

edited

yahesh commented Jul 19, 2019 •

edited

yahesh commented Jul 19, 2019 •

edited

yahesh commented Jul 19, 2019 •

edited

yahesh commented Jul 21, 2019 •

edited

yahesh commented Jul 24, 2019 •

edited

yahesh commented Jul 24, 2019 •

edited

yahesh commented Jul 24, 2019 •

edited

yahesh commented Jul 30, 2019 •

edited

yahesh commented Jul 30, 2019 •

edited

yahesh commented Aug 7, 2019 •

edited

yahesh commented Aug 8, 2019 •

edited

yahesh commented Aug 9, 2019 •

edited

yahesh commented Aug 13, 2019 •

edited

yahesh commented Aug 14, 2019 •

edited