Fix truncation of files upon read when using object store and encryption.

When using and object store as primary storage and using the default
encryption module at the same time,  any encrypted file would be truncated
when read, and a text error message added to the end.

This was caused by a combination of the reliance of the read functions on
on knowing the unencrypted file size,  and a bug in the function which
calculated the unencrypted file size for a given file.

In order to calculate the unencrypted file size,  the function would first
skip the header block, then use fseek to skip to the last encrypted block
in the file.  Because there was a corresponence between the encrypted and
unencrypted blocks, this would also be the last encrypted block.  It would
then read the final block and decrypt it to get the unencrypted length of
the last block.  With that, the number of blocks, and the unencrypted block
size, it could calculate the unencrypted file size.

The trouble was that when using an object store, an fread call doesn't
always get you the number of bytes you asked for, even if they are
available.  To resolve this I adapted the stream_read_block function from
lib/private/Files/Streams/Encryption.php to work here.  This function
wraps the fread call in a loop and repeats until it has the entire set of
bytes that were requested,  or there are no more to get.

This fixes the imediate bug, and should (with luck) allow people to get
their encrypted files out of Nextcloud now.  (The problem was purely on
the decryption side).  In the future it would be nice to do some
refactoring here.

I have tested this with image files ranging from 1kb to 10mb using
Nextcloud version 22.1.0 (the nextcloud:22.1-apache docker image), with
sqlite and a Linode object store as the primary storage.

Signed-off-by: Alan Meeson <alan@carefullycalculated.co.uk>

diff --git a/lib/private/Files/Storage/Wrapper/Encryption.php b/lib/private/Files/Storage/Wrapper/Encryption.php
index a52c87ec5a1edc1798e3266b93196880d601abbe..745568142bbc487b6350208e386e202befe369f0 100644 (file)
--- a/lib/private/Files/Storage/Wrapper/Encryption.php
+++ b/lib/private/Files/Storage/Wrapper/Encryption.php
@@ -471,6 +471,7 @@ class Encryption extends Wrapper {
                                $handle = \OC\Files\Stream\Encryption::wrap($source, $path, $fullPath, $header,
                                        $this->uid, $encryptionModule, $this->storage, $this, $this->util, $this->fileHelper, $mode,
                                        $size, $unencryptedSize, $headerSize, $signed);
+
                                return $handle;
                        }
                }
@@ -540,7 +541,7 @@ class Encryption extends Wrapper {
 
                // if a header exists we skip it
                if ($headerSize > 0) {
-                       fread($stream, $headerSize);
+                       $this->fread_block($stream, $headerSize);
                }
 
                // fast path, else the calculation for $lastChunkNr is bogus
@@ -567,7 +568,7 @@ class Encryption extends Wrapper {
                $count = $blockSize;
 
                while ($count > 0) {
-                       $data = fread($stream, $blockSize);
+                       $data = $this->fread_block($stream, $blockSize);
                        $count = strlen($data);
                        $lastChunkContentEncrypted .= $data;
                        if (strlen($lastChunkContentEncrypted) > $blockSize) {
@@ -598,6 +599,33 @@ class Encryption extends Wrapper {
                return $newUnencryptedSize;
        }
 
+       /**
+        * fread_block
+        *
+        * This function is a wrapper around the fread function.  It is based on the
+        * stream_read_block function from lib/private/Files/Streams/Encryption.php
+        * It calls stream read until the requested $blockSize was received or no remaining data is present.
+        * This is required as stream_read only returns smaller chunks of data when the stream fetches from a
+        * remote storage over the internet and it does not care about the given $blockSize.
+        *
+        * @param handle the stream to read from
+        * @param int $blockSize Length of requested data block in bytes
+        * @return string Data fetched from stream.
+        */
+       private function fread_block($handle, $blockSize): string {
+               $remaining = $blockSize;
+               $data = '';
+
+               do {
+                       $chunk = fread($handle, $remaining);
+                       $chunk_len = strlen($chunk);
+                       $data .= $chunk;
+                       $remaining -= $chunk_len;
+               } while (($remaining > 0) && ($chunk_len > 0));
+
+         return $data;
+       }
+
        /**
         * @param Storage\IStorage $sourceStorage
         * @param string $sourceInternalPath