Extend encryption:fix-encrypted-version to also detect encrypted files with 0

[PVince81]

How to use GitHub

• Please use the 👍 reaction to show that you are interested into the same feature.
• 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.

Is your feature request related to a problem? Please describe.
In some setups, it can happen that encrypted files exist or have been recovered from backup.
When running occ files:scan on those files, the "encrypted" column will still say "0" which means unencrypted as the scan command is not able to detect if files are encrypted.
So those files are not accessible nor downloadable.

Describe the solution you'd like
Extend the occ encryption:fix-encrypted-version to also attempt to detect encrypted files that have the column set to 0.
The detection can attempt to set the value to "1" or higher as per its current algorithm and detect if decryption worked.
If decryption worked, it must adjust the value in the database.

There is already logic in place that does this for already encrypted files, so the extension should be a matter of including "0" in the files to be searched.

To avoid potential unwanted side effects, the option to include "0" in the search should be optional. So the user will need to add an option like --detect-encrypted (find better wording)

Additional context
This is with master key encryption.

[patrickuhlmann]

I like this feature. I had the problem that I executed the decrypt-all command but for some reason not all files were decrypted. Such a feature would help to repair in such a case

[PVince81]

Steps to recreate:

1. occ app:enable encryption && occ encryption:enable
2. Login as admin
3. Upload a picture "test.jpg"
4. Try to download it: works
5. select * from oc_filecache where name='test.jpg' see that encrypted=1
6. Upload the picture "test.jpg" again and overwrite it
7. Try to download it: works
8. select * from oc_filecache where name='test.jpg' see that encrypted=2 (for every write this value is increased)
9. In the database, reset the encrypted flag to 0: update oc_filecache set encrypted=0 where name='test.jpg' (the value "0" means unencrypted)
10. Try to download it: error message
11. Try to repair it with occ encryption:fix-encrypted-version: this does nothing because the command only looks for files with encrypted>0

After the fix running occ encryption:fix-encrypted-version --detect-encrypted should set "encrypted=2" again in the database for this specific scenario. The command already contains the logic to "brute-force" the encrypted value by incrementing the value of "encrypted" and checking if the file can be decrypted with it.

[come-nc]

I am not sure if I got this correctly:

I am unable to get the file after setting encrypted to 0, but when looking at apps/encryption/lib/Command/FixEncryptedVersion.php it does not seem to specifically check that encrypted is not 0, it tries an fread:

			$handle = $this->view->fopen($path, 'rb');

			if (\fread($handle, 9001) !== false) {
				$output->writeln("<info>The file \"$path\" is: OK</info>");
			}

			\fclose($handle);

And this fread returns true in my case so it says the file is OK. (even if failing to display in Files UI)

How can I test occ encryption:fix-encrypted-version in a case where it works?

[PVince81]

a lot of magic is happening in the Encryption storage and stream wrappers.

I don't remember exactly whether it's checking the header on fopen or on the first fread.
Maybe set a breakpoint here https://github.com/nextcloud/server/blob/master/lib/private/Files/Storage/Wrapper/Encryption.php#L978 to find out if it reaches that place when downloading in the web UI and in the occ command.

[PVince81]

Also note: in the PHP $cacheEntry it is likely that the attribute key is "encryptedVersion" for the number and "encrypted" for the bool. See https://github.com/nextcloud/server/blob/master/lib/private/Files/Cache/Cache.php#L457

[PVince81]

Now thinking, I think the encryption check for the web UI is done at a higher level to prevent people to download the raw encrypted file, which would be bad especially when done automatically through the desktop client.

The PHP level's fopen/fread will simply ignore the encrypted header if the flag is not set.
So probably we need to add an additional check in the command, like trying to see if a header exists and if yes, attempt setting encryptedVersion from 1-N until the file can be decrypted.

[come-nc]

There is actually nothing preventing downloading the raw encrypted file from what I can see, if I set encrypted to 0 in the DB I can download it, it just does not show as an image because it is encrypted.

If I alter the fix command to test encryption first it works. So I can add a flag that does this.
But this means that the logic in https://github.com/nextcloud/server/blob/master/apps/encryption/lib/Command/FixEncryptedVersion.php#L194-L201 which triggers https://github.com/nextcloud/server/blob/master/apps/encryption/lib/Command/FixEncryptedVersion.php#L247-L256 is bogus as trying with encrypted=0 will always work.

I think I found a solution by using stat and checking for hasHeader: https://github.com/nextcloud/server/blob/master/lib/private/Files/Storage/Wrapper/Encryption.php#L871
When encrypted version is 0 but hasHeader is true I consider that a failure.