From 1258caeab7dec7510781ba7b575390459e2e61e5 Mon Sep 17 00:00:00 2001 From: plumbeo Date: Sat, 16 Apr 2022 17:14:13 +0200 Subject: 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 --- apps/encryption/lib/Crypto/Crypt.php | 47 +++++++++++++++++++++------- apps/encryption/lib/Crypto/Encryption.php | 50 +++++++++++++++++++++--------- apps/encryption/tests/Crypto/CryptTest.php | 23 ++++++++------ 3 files changed, 85 insertions(+), 35 deletions(-) (limited to 'apps') diff --git a/apps/encryption/lib/Crypto/Crypt.php b/apps/encryption/lib/Crypto/Crypt.php index 93120068c6a..f8ba3d69b80 100644 --- 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 58cefcbe087..b44472fd04a 100644 --- 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 60548a0f1dc..ef4e46085a4 100644 --- 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) -- cgit v1.2.3