From 387af7eb600fcab56a04b4184d53eb0fe509e30b Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Mon, 11 May 2020 10:31:46 +0200 Subject: [PATCH] Use random_bytes Since we don't care if it is human readbale. The code is backwards compatible with the old format. Signed-off-by: Roeland Jago Douma --- lib/private/Security/Crypto.php | 20 +++++++++++++------- tests/lib/Security/CryptoTest.php | 15 +++++++++++---- 2 files changed, 24 insertions(+), 11 deletions(-) diff --git a/lib/private/Security/Crypto.php b/lib/private/Security/Crypto.php index 10a52d9fc8..154448281b 100644 --- a/lib/private/Security/Crypto.php +++ b/lib/private/Security/Crypto.php @@ -52,17 +52,14 @@ class Crypto implements ICrypto { private $ivLength = 16; /** @var IConfig */ private $config; - /** @var ISecureRandom */ - private $random; /** * @param IConfig $config * @param ISecureRandom $random */ - public function __construct(IConfig $config, ISecureRandom $random) { + public function __construct(IConfig $config) { $this->cipher = new AES(); $this->config = $config; - $this->random = $random; } /** @@ -95,13 +92,14 @@ class Crypto implements ICrypto { } $this->cipher->setPassword($password); - $iv = $this->random->generate($this->ivLength); + $iv = \random_bytes($this->ivLength); $this->cipher->setIV($iv); $ciphertext = bin2hex($this->cipher->encrypt($plaintext)); + $iv = bin2hex($iv); $hmac = bin2hex($this->calculateHMAC($ciphertext.$iv, $password)); - return $ciphertext.'|'.$iv.'|'.$hmac; + return $ciphertext.'|'.$iv.'|'.$hmac.'|2'; } /** @@ -119,7 +117,8 @@ class Crypto implements ICrypto { $this->cipher->setPassword($password); $parts = explode('|', $authenticatedCiphertext); - if (\count($parts) !== 3) { + $partCount = \count($parts); + if ($partCount < 3 || $partCount > 4) { throw new \Exception('Authenticated ciphertext could not be decoded.'); } @@ -127,6 +126,13 @@ class Crypto implements ICrypto { $iv = $parts[1]; $hmac = hex2bin($parts[2]); + if ($partCount === 4) { + $version = $parts[3]; + if ($version === '2') { + $iv = hex2bin($iv); + } + } + $this->cipher->setIV($iv); if (!hash_equals($this->calculateHMAC($parts[0] . $parts[1], $password), $hmac)) { diff --git a/tests/lib/Security/CryptoTest.php b/tests/lib/Security/CryptoTest.php index a6f4902594..dbdf6f6a19 100644 --- a/tests/lib/Security/CryptoTest.php +++ b/tests/lib/Security/CryptoTest.php @@ -24,7 +24,7 @@ class CryptoTest extends \Test\TestCase { protected function setUp(): void { parent::setUp(); - $this->crypto = new Crypto(\OC::$server->getConfig(), \OC::$server->getSecureRandom()); + $this->crypto = new Crypto(\OC::$server->getConfig()); } /** @@ -35,7 +35,7 @@ class CryptoTest extends \Test\TestCase { $this->assertEquals($stringToEncrypt, $this->crypto->decrypt($ciphertext)); } - + public function testWrongPassword() { $this->expectException(\Exception::class); $this->expectExceptionMessage('HMAC does not match.'); @@ -51,7 +51,7 @@ class CryptoTest extends \Test\TestCase { $this->assertEquals($stringToEncrypt, $this->crypto->decrypt($encryptedString, 'ThisIsAVeryS3cur3P4ssw0rd')); } - + public function testWrongIV() { $this->expectException(\Exception::class); $this->expectExceptionMessage('HMAC does not match.'); @@ -60,7 +60,7 @@ class CryptoTest extends \Test\TestCase { $this->crypto->decrypt($encryptedString, 'ThisIsAVeryS3cur3P4ssw0rd'); } - + public function testWrongParameters() { $this->expectException(\Exception::class); $this->expectExceptionMessage('Authenticated ciphertext could not be decoded.'); @@ -68,4 +68,11 @@ class CryptoTest extends \Test\TestCase { $encryptedString = '1|2'; $this->crypto->decrypt($encryptedString, 'ThisIsAVeryS3cur3P4ssw0rd'); } + + public function testLegacy() { + $cipherText = 'e16599188e3d212f5c7f17fdc2abca46|M1WfLAxbcAmITeD6|509457885d6ca5e6c3bfd3741852687a7f2bffce197f8d5ae97b65818b15a1b7f616b68326ff312371540f4ca8ac55f8e2de4aa13aab3474bd3431e51214e3ee'; + $password = 'mypass'; + + $this->assertSame('legacy test', $this->crypto->decrypt($cipherText, $password)); + } }