Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Files moved into an encrypted groupfolder are no longer decrypted when needed #2909

Closed
danxuliu opened this issue Apr 4, 2024 · 3 comments · Fixed by #2942
Closed

Files moved into an encrypted groupfolder are no longer decrypted when needed #2909

danxuliu opened this issue Apr 4, 2024 · 3 comments · Fixed by #2942
Assignees
Labels
2. developing Items that are currently under development bug feature: encryption Items related to encryption features of NC

Comments

@danxuliu
Copy link
Member

danxuliu commented Apr 4, 2024

How to use GitHub

  • Please use the 👍 reaction to show that you are affected by the same issue.
  • Please don't comment if you have no relevant information to add. It's just extra noise for everyone subscribed to this issue.
  • Subscribe to receive notifications on status change and new comments.

When encryption is enabled in both the home storage and in groupfolders and an encrypted file is moved from the home storage into a groupfolder the file is no longer decrypted when needed. That is, trying to download or open the file with a viewer in Nextcloud will show the encrypted content.

During the move the file is properly decrypted and encrypted again and the keys moved. However, the problem is that the file is marked as not encrypted in the file cache. This can be verified by manually modifying the database to set encrypted = 1 in oc_filecache for the file once moved into the groupfolder and then trying to download or view the file.

The file is marked as not encrypted because the storage in the Cache object does not have an encryption wrapper; $this->storage is a OCA\Files_Trashbin\Storage that wraps a OC\Files\Storage\LocalRootStorage (so $this->hasEncryptionWrapper() returns false).

Interestingly, in the View object that triggers the move in the cache (through the cache updater) $targetStorage is a OCA\Files_Trashbin\Storage that wraps a OC\Files\Storage\Wrapper\Encryption.

I am afraid that I do not know why or how the storage used by the cache ends being a different one, but hopefully the information above is somehow useful :-)

Steps to reproduce

  • Enable the default encryption module (occ app:enable encryption)
  • Enable encryption (occ config:app:set --value=yes --type string core encryption_enabled)
    • By default the home storage will be encrypted
  • Enable encryption in groupfolders (occ config:app:set groupfolders enable_encryption --value="true")
  • Upload a file to the root directory
  • Move that file into a groupfolder
  • Download the file

Expected beaviour

The downloaded file is not encrypted

Actual behaviour

The downloaded file is encrypted

@danxuliu danxuliu added bug 0. Needs triage Issues that need to be triaged labels Apr 4, 2024
@come-nc come-nc self-assigned this Apr 11, 2024
@come-nc
Copy link
Contributor

come-nc commented Apr 11, 2024

Findings so far:

  • Copying works, only moving cause the problem
  • Keys are correctly moved
  • encryption column in filecache is the problem, it’s 1 intsead of 0. Calling encryption:fix-encrypted-version fixes it correctly.
  • In the Encryption wrapper, updateEncryptedVersion does get called. The content of $cacheInformation is {"encrypted":true,"encryptedVersion":1} and $isRename is true so the put is done on source storage. I tried to switch to target storage but it does not help.

@come-nc
Copy link
Contributor

come-nc commented Apr 11, 2024

@come-nc
Copy link
Contributor

come-nc commented Apr 15, 2024

Here is the full wrapping tree for the groupfolder storage when encryption is enabled:

storage wrapping: 
	OCA\Files_Trashbin\Storage (cache:OC\Files\Cache\Wrapper\CachePermissionsMask)
	> OC\Files\Storage\Wrapper\Encryption (cache:OC\Files\Cache\Wrapper\CachePermissionsMask)
	> OC\Files\Storage\Wrapper\PermissionsMask (cache:OC\Files\Cache\Wrapper\CachePermissionsMask)
	> OCA\GroupFolders\Mount\GroupFolderStorage (cache:OCA\GroupFolders\Mount\RootEntryCache)
	> OC\Files\Storage\Wrapper\Jail (cache:OC\Files\Cache\Wrapper\CacheJail)
	> OCA\Files_Trashbin\Storage (cache:OC\Files\Cache\Cache)
	> OC\Files\Storage\LocalRootStorage (cache:OC\Files\Cache\Cache)

https://github.com/nextcloud/server/pull/35961/files#diff-f36621cc749f37af880e8d2d5ea4b70870a0a13de93ce1461cc99be75e1abf47R663 this does not work because the cache is wrapped as well, so it does not see the original storage here, and fails to understand that encryption wrapper is there.

Maybe we need to put the encryption wrapper inside the jail, but I’m not sure that easy to do, and not sure it will fix the problem.

@joshtrichards joshtrichards added 2. developing Items that are currently under development feature: encryption Items related to encryption features of NC and removed 0. Needs triage Issues that need to be triaged labels Jul 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2. developing Items that are currently under development bug feature: encryption Items related to encryption features of NC
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants