diff options
author | Roeland Jago Douma <rullzer@users.noreply.github.com> | 2020-10-27 20:23:07 +0100 |
---|---|---|
committer | GitHub <noreply@github.com> | 2020-10-27 20:23:07 +0100 |
commit | cf1af5b83bbea6fa3e238fef66f8f9fa94400c89 (patch) | |
tree | 4b01df29a4ead3ecfb51e9d5aa838cde2110fba3 | |
parent | 28d82394f1b66696532456f091a693f682349570 (diff) | |
parent | a3bdb0c4cb1b4dfd1dda567f72817f920db4b410 (diff) | |
download | nextcloud-server-cf1af5b83bbea6fa3e238fef66f8f9fa94400c89.tar.gz nextcloud-server-cf1af5b83bbea6fa3e238fef66f8f9fa94400c89.zip |
Merge pull request #23496 from lynn-stephenson/fix/related-key-issue
Fixes related key issue.
-rw-r--r-- | lib/private/Security/Crypto.php | 22 | ||||
-rw-r--r-- | tests/lib/Security/CryptoTest.php | 20 |
2 files changed, 35 insertions, 7 deletions
diff --git a/lib/private/Security/Crypto.php b/lib/private/Security/Crypto.php index 154448281b9..35add2dc649 100644 --- a/lib/private/Security/Crypto.php +++ b/lib/private/Security/Crypto.php @@ -10,6 +10,7 @@ declare(strict_types=1); * @author Lukas Reschke <lukas@statuscode.ch> * @author Morris Jobke <hey@morrisjobke.de> * @author Roeland Jago Douma <roeland@famdouma.nl> + * @author Lynn Stephenson <lynn.stephenson@protonmail.com> * * @license AGPL-3.0 * @@ -90,16 +91,17 @@ class Crypto implements ICrypto { if ($password === '') { $password = $this->config->getSystemValue('secret'); } - $this->cipher->setPassword($password); + $keyMaterial = hash_hkdf('sha512', $password); + $this->cipher->setPassword(substr($keyMaterial, 0, 32)); $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)); + $hmac = bin2hex($this->calculateHMAC($ciphertext.$iv, substr($keyMaterial, 32))); - return $ciphertext.'|'.$iv.'|'.$hmac.'|2'; + return $ciphertext.'|'.$iv.'|'.$hmac.'|3'; } /** @@ -114,7 +116,7 @@ class Crypto implements ICrypto { if ($password === '') { $password = $this->config->getSystemValue('secret'); } - $this->cipher->setPassword($password); + $hmacKey = $encryptionKey = $password; $parts = explode('|', $authenticatedCiphertext); $partCount = \count($parts); @@ -128,14 +130,20 @@ class Crypto implements ICrypto { if ($partCount === 4) { $version = $parts[3]; - if ($version === '2') { + if ($version >= '2') { $iv = hex2bin($iv); } - } + if ($version === '3') { + $keyMaterial = hash_hkdf('sha512', $password); + $encryptionKey = substr($keyMaterial, 0, 32); + $hmacKey = substr($keyMaterial, 32); + } + } + $this->cipher->setPassword($encryptionKey); $this->cipher->setIV($iv); - if (!hash_equals($this->calculateHMAC($parts[0] . $parts[1], $password), $hmac)) { + if (!hash_equals($this->calculateHMAC($parts[0] . $parts[1], $hmacKey), $hmac)) { throw new \Exception('HMAC does not match.'); } diff --git a/tests/lib/Security/CryptoTest.php b/tests/lib/Security/CryptoTest.php index dbdf6f6a195..0c7c1aa1ac7 100644 --- a/tests/lib/Security/CryptoTest.php +++ b/tests/lib/Security/CryptoTest.php @@ -75,4 +75,24 @@ class CryptoTest extends \Test\TestCase { $this->assertSame('legacy test', $this->crypto->decrypt($cipherText, $password)); } + + public function testVersion2CiphertextDecryptsToCorrectPlaintext() { + $this->assertSame( + 'This is a plaintext value that will be encrypted with version 2. Which addresses the reduced permutations on the IV.', + $this->crypto->decrypt( + 'be006387f753e8728717e43cfc5526c37adf7b2c9b4a113ceec03b7b0bccfebee74e0acfa0015c5712b4376dacbd7bce26a8fbca916fdccee46203d8289f6b2e4c19318044d375edfc67c72e6c3ae329d4c276b8d866ac1b281844e81f7681fe83d90bc4b6fffa4f3cbc157d64257a493b67fd2af3c8976cb76df520f5739305|02e78ea7c73a32f3b407c54227a9d2ce|3e7a09628f818b7b1cd7724467f5b1b33135de6d2ec62d8c0361be4f2c5203385f10babdcae017d7b30abe5be2117803e3195fb6d9ef20949fe35dad5e9241ea|2', + 'insecure-static-password' + ) + ); + } + + public function testVersion3CiphertextDecryptsToCorrectPlaintext() { + $this->assertSame( + 'Another plaintext value that will be encrypted with version 3. It addresses the related key issue. Old ciphertexts should be decrypted properly, but only use the better version for encryption.', + $this->crypto->decrypt( + 'c99823461db746aa74f819c8640e9e3c367fa3bb9c21dff905b5dd14072c1d1b0da8b7e6b7307bf1561b6ba7aaa932a16c23b1fd5217dc019d55233ef0813c65fccaeabd6ea3a971ce1bbbdfda790ae00fb4442693cbb50072e02875b9f50591df74d00e96fd5b9bd13cb02a5f57b062ec98a4c64fc518ed325d097454883adbfc1687c2af995a392407c5e040a54afee4b2997ab158fe48ef67ccf721a6a7031fcb44d51170892ce7971021a7f3a00d19002eb9b007efe7aecf397ec0dc22064fb5d4a15ad83949f0804feca3c69cdd|8476f53c8d49a7e119798a70086d8911|ae3f7e23d469fbc791714ceb07d854624b1bbd39ac6a4edc05d552e10659adfdcada3a059fae737ffd7d842dd3fcc84bcc364cd298e814dd4967de4ad4a658eb|3', + 'insecure-static-password' + ) + ); + } } |