Update the unencrypted file size when closing streams

[juliusknorr]

Summary

• Fixes Encryption header visible in newly created files richdocuments#2102

This PR fixes two issues related to primary object storage and master key encryption, both related to apps using the PHP Nodes api for file access.

77edd36 makes sure that the proper unencrypted size is written into the filecache when writing to a file through the putContent method. This for example is done by Collabora to save a file. Without updating the unencrypted size any follow up download would cause a BadSignature error.

f6d6898 Addresses a similar issue when a newFile was created with empty content. In this case the encryption stream writes just the header to the file but never updates the isEncrypted flag on it, so when creating a new empty file in the text app the encrypted content is returned to the user instead of the decrypted one.

Checklist

• Code is properly formatted
• Sign-off message is added to all commits
• Tests (unit, integration, api and/or acceptance) are included
• Screenshots before/after for front-end changes
• Documentation (manuals or wiki) has been updated or is not required
• Backports requested where applicable (ex: critical bugfixes)

[juliusknorr]

/backport to stabel25

[juliusknorr]

/backport to stable24

[juliusknorr]

Backporting only to 24/25 as in general this requires #31966 to work

[PVince81]

👍 wow, nice catch!

[juliusknorr]

@icewind1991 @PVince81 Any hint on where the best place for a test case of this would be?

To reproduce this I used the following plain code snippets:

<?php
require_once __DIR__ . '/../lib/versioncheck.php';
try {
	require_once __DIR__ . '/../lib/base.php';
	$root = \OC::$server->getRootFolder()->getUserFolder('admin');
	try {
		$root->delete('existingfile');
	} catch (\OCP\Files\InvalidPathException $e) {
	} catch (\OCP\Files\NotFoundException $e) {
	} catch (\OCP\Files\NotPermittedException $e) {
	}

	$file = $root->newFile('existingfile', '');
	$data = [
		'size' => $file->getSize(),
		'unencrypted_size' => $file->getFileInfo()['unencrypted_size'],
		'encrypted' => $file->isEncrypted()
	];
	var_dump($data);

	$file = $root->get('existingfile');
	$file->putContent(str_repeat('A', 64*1024));
	$data = [
		'size' => $file->getSize(),
		'unencrypted_size' => $file->getFileInfo()['unencrypted_size'],
		'encrypted' => $file->isEncrypted()
	];
	var_dump($data);
} catch (Error $ex) {
	echo $ex->getMessage();
}

Before

array(3) { ["size"]=> int(8192) ["unencrypted_size"]=> int(0) ["encrypted"]=> bool(true) } array(3) { ["size"]=> int(74592) ["unencrypted_size"]=> int(0) ["encrypted"]=> bool(true) }

After

array(3) { ["size"]=> int(8192) ["unencrypted_size"]=> int(0) ["encrypted"]=> bool(true) } array(3) { ["size"]=> int(65536) ["unencrypted_size"]=> int(65536) ["encrypted"]=> bool(true) }

[PVince81]

@juliushaertl it seems there's a matching test class that might be useable for a unit test: tests/lib/Files/Stream/EncryptionTest.php

for integration tests, it seems we don't have encryption there.
otherwise we could have added a test that uploads and then propfinds the size

another idea would be to simply add code like your snippet above either the storage tests or stream tests, and rely on https://github.com/nextcloud/server/blob/master/.github/workflows/s3-primary.yml#L59 to have them run on S3.

maybe we should copy that workflow to also have one with encryption enabled ?

[juliusknorr]

Apparently the webdav API didn't trigger those issues, but maybe because they interact a bit differently with the file system apis through the view directly.

I adjusted the unit tests accordingly to have some basic coverage so we can get this in, but at some point dedicated integration tests with encryption might be nice to have.

[backportbot-nextcloud]

The backport to stabel25 failed. Please do this backport manually.

[PVince81]

/backport to stable25