Save encrypted files in binary format

Default to the more space-efficient binary encoding for newly encrypted files
instead of the traditional base64 encoding, eliminating the 33% overhead.

The new option 'encryption.use_legacy_encoding' allows to force the legacy
encoding format if needed. Files encoded in the old format remain readable.

Based on https://github.com/owncloud/encryption/pull/224 and
https://github.com/owncloud/core/pull/38249 by karakayasemi.

Signed-off-by: plumbeo <plumbeo@users.noreply.github.com>

diff --git a/apps/encryption/lib/Crypto/Crypt.php b/apps/encryption/lib/Crypto/Crypt.php
index 93120068c6ab60ac04abcc9d22ba5277e936f1a8..f8ba3d69b80f84e8a2a745b0f2a21d3a8e5ec30d 100644 (file)
--- a/apps/encryption/lib/Crypto/Crypt.php
+++ b/apps/encryption/lib/Crypto/Crypt.php
@@ -77,6 +77,9 @@ class Crypt {
        public const HEADER_START = 'HBEGIN';
        public const HEADER_END = 'HEND';
 
+       // default encoding format, old Nextcloud versions used base64
+       public const BINARY_ENCODING_FORMAT = 'binary';
+
        /** @var ILogger */
        private $logger;
 
@@ -95,6 +98,11 @@ class Crypt {
        /** @var bool */
        private $supportLegacy;
 
+       /**
+        * Use the legacy base64 encoding instead of the more space-efficient binary encoding.
+        */
+       private bool $useLegacyBase64Encoding;
+
        /**
         * @param ILogger $logger
         * @param IUserSession $userSession
@@ -107,6 +115,7 @@ class Crypt {
                $this->config = $config;
                $this->l = $l;
                $this->supportLegacy = $this->config->getSystemValueBool('encryption.legacy_format_support', false);
+               $this->useLegacyBase64Encoding = $this->config->getSystemValueBool('encryption.use_legacy_base64_encoding', false);
        }
 
        /**
@@ -215,12 +224,15 @@ class Crypt {
                        throw new \InvalidArgumentException('key format "' . $keyFormat . '" is not supported');
                }
 
-               $cipher = $this->getCipher();
-
                $header = self::HEADER_START
-                       . ':cipher:' . $cipher
-                       . ':keyFormat:' . $keyFormat
-                       . ':' . self::HEADER_END;
+                       . ':cipher:' . $this->getCipher()
+                       . ':keyFormat:' . $keyFormat;
+
+               if ($this->useLegacyBase64Encoding !== true) {
+                       $header .= ':encoding:' . self::BINARY_ENCODING_FORMAT;
+               }
+
+               $header .= ':' . self::HEADER_END;
 
                return $header;
        }
@@ -234,10 +246,11 @@ class Crypt {
         * @throws EncryptionFailedException
         */
        private function encrypt($plainContent, $iv, $passPhrase = '', $cipher = self::DEFAULT_CIPHER) {
+               $options = $this->useLegacyBase64Encoding ? 0 : OPENSSL_RAW_DATA;
                $encryptedContent = openssl_encrypt($plainContent,
                        $cipher,
                        $passPhrase,
-                       0,
+                       $options,
                        $iv);
 
                if (!$encryptedContent) {
@@ -424,6 +437,8 @@ class Crypt {
                        $password = $this->generatePasswordHash($password, $cipher, $uid);
                }
 
+               $binaryEncoding = isset($header['encoding']) && $header['encoding'] === self::BINARY_ENCODING_FORMAT;
+
                // If we found a header we need to remove it from the key we want to decrypt
                if (!empty($header)) {
                        $privateKey = substr($privateKey,
@@ -435,7 +450,9 @@ class Crypt {
                        $privateKey,
                        $password,
                        $cipher,
-                       0
+                       0,
+                       0,
+                       $binaryEncoding
                );
 
                if ($this->isValidPrivateKey($plainKey) === false) {
@@ -470,10 +487,11 @@ class Crypt {
         * @param string $cipher
         * @param int $version
         * @param int|string $position
+        * @param boolean $binaryEncoding
         * @return string
         * @throws DecryptionFailedException
         */
-       public function symmetricDecryptFileContent($keyFileContents, $passPhrase, $cipher = self::DEFAULT_CIPHER, $version = 0, $position = 0) {
+       public function symmetricDecryptFileContent($keyFileContents, $passPhrase, $cipher = self::DEFAULT_CIPHER, $version = 0, $position = 0, bool $binaryEncoding = false) {
                if ($keyFileContents == '') {
                        return '';
                }
@@ -493,7 +511,8 @@ class Crypt {
                return $this->decrypt($catFile['encrypted'],
                        $catFile['iv'],
                        $passPhrase,
-                       $cipher);
+                       $cipher,
+                       $binaryEncoding);
        }
 
        /**
@@ -610,14 +629,16 @@ class Crypt {
         * @param string $iv
         * @param string $passPhrase
         * @param string $cipher
+        * @param boolean $binaryEncoding
         * @return string
         * @throws DecryptionFailedException
         */
-       private function decrypt($encryptedContent, $iv, $passPhrase = '', $cipher = self::DEFAULT_CIPHER) {
+       private function decrypt(string $encryptedContent, string $iv, string $passPhrase = '', string $cipher = self::DEFAULT_CIPHER, bool $binaryEncoding = false): string {
+               $options = $binaryEncoding === true ? OPENSSL_RAW_DATA : 0;
                $plainContent = openssl_decrypt($encryptedContent,
                        $cipher,
                        $passPhrase,
-                       0,
+                       $options,
                        $iv);
 
                if ($plainContent) {
@@ -728,4 +749,8 @@ class Crypt {
                        throw new MultiKeyEncryptException('multikeyencryption failed ' . openssl_error_string());
                }
        }
+
+       public function useLegacyBase64Encoding(): bool {
+               return $this->useLegacyBase64Encoding;
+       }
 }

diff --git a/apps/encryption/lib/Crypto/Encryption.php b/apps/encryption/lib/Crypto/Encryption.php
index 58cefcbe087916f1058e54009b6d7769a28b33d7..b44472fd04a528a213aa6b493e199c9cb30ec8be 100644 (file)
--- a/apps/encryption/lib/Crypto/Encryption.php
+++ b/apps/encryption/lib/Crypto/Encryption.php
@@ -103,11 +103,7 @@ class Encryption implements IEncryptionModule {
        /** @var DecryptAll  */
        private $decryptAll;
 
-       /** @var int unencrypted block size if block contains signature */
-       private $unencryptedBlockSizeSigned = 6072;
-
-       /** @var int unencrypted block size */
-       private $unencryptedBlockSize = 6126;
+       private bool $useLegacyBase64Encoding = false;
 
        /** @var int Current version of the file */
        private $version = 0;
@@ -184,6 +180,11 @@ class Encryption implements IEncryptionModule {
                $this->user = $user;
                $this->isWriteOperation = false;
                $this->writeCache = '';
+               $this->useLegacyBase64Encoding = true;
+
+               if (isset($header['encoding'])) {
+                       $this->useLegacyBase64Encoding = $header['encoding'] !== Crypt::BINARY_ENCODING_FORMAT;
+               }
 
                if ($this->session->isReady() === false) {
                        // if the master key is enabled we can initialize encryption
@@ -229,6 +230,7 @@ class Encryption implements IEncryptionModule {
 
                if ($this->isWriteOperation) {
                        $this->cipher = $this->crypt->getCipher();
+                       $this->useLegacyBase64Encoding = $this->crypt->useLegacyBase64Encoding();
                } elseif (isset($header['cipher'])) {
                        $this->cipher = $header['cipher'];
                } else {
@@ -237,7 +239,13 @@ class Encryption implements IEncryptionModule {
                        $this->cipher = $this->crypt->getLegacyCipher();
                }
 
-               return ['cipher' => $this->cipher, 'signed' => 'true'];
+               $result = ['cipher' => $this->cipher, 'signed' => 'true'];
+
+               if ($this->useLegacyBase64Encoding !== true) {
+                       $result['encoding'] = Crypt::BINARY_ENCODING_FORMAT;
+               }
+
+               return $result;
        }
 
        /**
@@ -325,8 +333,8 @@ class Encryption implements IEncryptionModule {
                        $remainingLength = strlen($data);
 
                        // If data remaining to be written is less than the
-                       // size of 1 6126 byte block
-                       if ($remainingLength < $this->unencryptedBlockSizeSigned) {
+                       // size of 1 unencrypted block
+                       if ($remainingLength < $this->getUnencryptedBlockSize(true)) {
 
                                // Set writeCache to contents of $data
                                // The writeCache will be carried over to the
@@ -343,14 +351,14 @@ class Encryption implements IEncryptionModule {
                        } else {
 
                                // Read the chunk from the start of $data
-                               $chunk = substr($data, 0, $this->unencryptedBlockSizeSigned);
+                               $chunk = substr($data, 0, $this->getUnencryptedBlockSize(true));
 
                                $encrypted .= $this->crypt->symmetricEncryptFileContent($chunk, $this->fileKey, $this->version + 1, $position);
 
                                // Remove the chunk we just processed from
                                // $data, leaving only unprocessed data in $data
                                // var, for handling on the next round
-                               $data = substr($data, $this->unencryptedBlockSizeSigned);
+                               $data = substr($data, $this->getUnencryptedBlockSize(true));
                        }
                }
 
@@ -374,7 +382,7 @@ class Encryption implements IEncryptionModule {
                        throw new DecryptionFailedException($msg, $hint);
                }
 
-               return $this->crypt->symmetricDecryptFileContent($data, $this->fileKey, $this->cipher, $this->version, $position);
+               return $this->crypt->symmetricDecryptFileContent($data, $this->fileKey, $this->cipher, $this->version, $position, !$this->useLegacyBase64Encoding);
        }
 
        /**
@@ -462,15 +470,27 @@ class Encryption implements IEncryptionModule {
         * get size of the unencrypted payload per block.
         * Nextcloud read/write files with a block size of 8192 byte
         *
+        * Encrypted blocks have a 22-byte IV and 2 bytes of padding, encrypted and
+        * signed blocks have also a 71-byte signature and 1 more byte of padding,
+        * resulting respectively in:
+        *
+        *  8192 - 22 - 2 = 8168 bytes in each unsigned unencrypted block
+        *  8192 - 22 - 2 - 71 - 1 = 8096 bytes in each signed unencrypted block
+        *
+        * Legacy base64 encoding then reduces the available size by a 3/4 factor:
+        *
+        *  8168 * (3/4) = 6126 bytes in each base64-encoded unsigned unencrypted block
+        *  8096 * (3/4) = 6072 bytes in each base64-encoded signed unencrypted block
+        *
         * @param bool $signed
         * @return int
         */
        public function getUnencryptedBlockSize($signed = false) {
-               if ($signed === false) {
-                       return $this->unencryptedBlockSize;
+               if ($this->useLegacyBase64Encoding) {
+                       return $signed ? 6072 : 6126;
+               } else {
+                       return $signed ? 8096 : 8168;
                }
-
-               return $this->unencryptedBlockSizeSigned;
        }
 
        /**

diff --git a/apps/encryption/tests/Crypto/CryptTest.php b/apps/encryption/tests/Crypto/CryptTest.php
index 60548a0f1dcd17a7da216a6a7a25bef1c798e945..ef4e46085a4c1dae103eb4b6ed5b806ab1fc8a01 100644 (file)
--- a/apps/encryption/tests/Crypto/CryptTest.php
+++ b/apps/encryption/tests/Crypto/CryptTest.php
@@ -139,9 +139,9 @@ class CryptTest extends TestCase {
         */
        public function dataTestGenerateHeader() {
                return [
-                       [null, 'HBEGIN:cipher:AES-128-CFB:keyFormat:hash:HEND'],
-                       ['password', 'HBEGIN:cipher:AES-128-CFB:keyFormat:password:HEND'],
-                       ['hash', 'HBEGIN:cipher:AES-128-CFB:keyFormat:hash:HEND']
+                       [null, 'HBEGIN:cipher:AES-128-CFB:keyFormat:hash:encoding:binary:HEND'],
+                       ['password', 'HBEGIN:cipher:AES-128-CFB:keyFormat:password:encoding:binary:HEND'],
+                       ['hash', 'HBEGIN:cipher:AES-128-CFB:keyFormat:hash:encoding:binary:HEND']
                ];
        }
 
@@ -305,15 +305,17 @@ class CryptTest extends TestCase {
         * test parseHeader()
         */
        public function testParseHeader() {
-               $header = 'HBEGIN:foo:bar:cipher:AES-256-CFB:HEND';
+               $header = 'HBEGIN:foo:bar:cipher:AES-256-CFB:encoding:binary:HEND';
                $result = self::invokePrivate($this->crypt, 'parseHeader', [$header]);
 
                $this->assertTrue(is_array($result));
-               $this->assertSame(2, count($result));
+               $this->assertSame(3, count($result));
                $this->assertArrayHasKey('foo', $result);
                $this->assertArrayHasKey('cipher', $result);
+               $this->assertArrayHasKey('encoding', $result);
                $this->assertSame('bar', $result['foo']);
                $this->assertSame('AES-256-CFB', $result['cipher']);
+               $this->assertSame('binary', $result['encoding']);
        }
 
        /**
@@ -324,18 +326,20 @@ class CryptTest extends TestCase {
        public function testEncrypt() {
                $decrypted = 'content';
                $password = 'password';
+               $cipher = 'AES-256-CTR';
                $iv = self::invokePrivate($this->crypt, 'generateIv');
 
                $this->assertTrue(is_string($iv));
                $this->assertSame(16, strlen($iv));
 
-               $result = self::invokePrivate($this->crypt, 'encrypt', [$decrypted, $iv, $password]);
+               $result = self::invokePrivate($this->crypt, 'encrypt', [$decrypted, $iv, $password, $cipher]);
 
                $this->assertTrue(is_string($result));
 
                return [
                        'password' => $password,
                        'iv' => $iv,
+                       'cipher' => $cipher,
                        'encrypted' => $result,
                        'decrypted' => $decrypted];
        }
@@ -349,7 +353,7 @@ class CryptTest extends TestCase {
                $result = self::invokePrivate(
                        $this->crypt,
                        'decrypt',
-                       [$data['encrypted'], $data['iv'], $data['password']]);
+                       [$data['encrypted'], $data['iv'], $data['password'], $data['cipher'], true]);
 
                $this->assertSame($data['decrypted'], $result);
        }
@@ -391,8 +395,9 @@ class CryptTest extends TestCase {
         */
        public function testDecryptPrivateKey($header, $privateKey, $expectedCipher, $isValidKey, $expected) {
                $this->config->method('getSystemValueBool')
-                       ->with('encryption.legacy_format_support', false)
-                       ->willReturn(true);
+                       ->withConsecutive(['encryption.legacy_format_support', false],
+                                         ['encryption.use_legacy_base64_encoding', false])
+                       ->willReturnOnConsecutiveCalls(true, false);
 
                /** @var \OCA\Encryption\Crypto\Crypt | \PHPUnit\Framework\MockObject\MockObject $crypt */
                $crypt = $this->getMockBuilder(Crypt::class)

diff --git a/config/config.sample.php b/config/config.sample.php
index 54d3d46070aa49a96cc50665255d2095e9a28233..77e587acf83bc2bc289b0d33347f89c9c81eb0ba 100644 (file)
--- a/config/config.sample.php
+++ b/config/config.sample.php
@@ -1798,6 +1798,15 @@ $CONFIG = [
  */
 'cipher' => 'AES-256-CTR',
 
+/**
+ * Use the legacy base64 format for encrypted files instead of the more space-efficient
+ * binary format. The option affects only newly written files, existing encrypted files
+ * will not be touched and will remain readable whether they use the new format or not.
+ *
+ * Defaults to ``false``
+ */
+'encryption.use_legacy_base64_encoding' => false,
+
 /**
  * The minimum Nextcloud desktop client version that will be allowed to sync with
  * this server instance. All connections made from earlier clients will be denied

diff --git a/lib/private/Files/Stream/Encryption.php b/lib/private/Files/Stream/Encryption.php
index e1233e76b934966a6197d40375d969b94b94d994..0f1838c97c82e74e57ad4e40675c27f7958a44f9 100644 (file)
--- a/lib/private/Files/Stream/Encryption.php
+++ b/lib/private/Files/Stream/Encryption.php
@@ -259,7 +259,6 @@ class Encryption extends Wrapper {
                $this->cache = '';
                $this->writeFlag = false;
                $this->fileUpdated = false;
-               $this->unencryptedBlockSize = $this->encryptionModule->getUnencryptedBlockSize($this->signed);
 
                if (
                        $mode === 'w'
@@ -284,6 +283,7 @@ class Encryption extends Wrapper {
                        $accessList = $this->file->getAccessList($sharePath);
                }
                $this->newHeader = $this->encryptionModule->begin($this->fullPath, $this->uid, $mode, $this->header, $accessList);
+               $this->unencryptedBlockSize = $this->encryptionModule->getUnencryptedBlockSize($this->signed);
 
                if (
                        $mode === 'w'