Merge pull request #20925 from nextcloud/backport/20915/stable17

[stable17] Use random_bytes
nextcloud · May 11, 2020 · 3c7cd00 · 3c7cd00
2 parents c76487e + 8fabe71
commit 3c7cd00
Show file tree

Hide file tree

Showing 2 changed files with 21 additions and 8 deletions.
diff --git a/lib/private/Security/Crypto.php b/lib/private/Security/Crypto.php
@@ -50,17 +50,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;
 	}
 
 	/**
@@ -93,13 +90,14 @@ public function encrypt(string $plaintext, string $password = ''): string {
 		}
 		$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';
 	}
 
 	/**
@@ -117,14 +115,22 @@ public function decrypt(string $authenticatedCiphertext, string $password = ''):
 		$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.');
 		}
 
 		$ciphertext = hex2bin($parts[0]);
 		$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
@@ -26,7 +26,7 @@ public function defaultEncryptionProvider()
 
 	protected function setUp() {
 		parent::setUp();
-		$this->crypto = new Crypto(\OC::$server->getConfig(), \OC::$server->getSecureRandom());
+		$this->crypto = new Crypto(\OC::$server->getConfig());
 	}
 
 	/**
@@ -70,4 +70,11 @@ function testWrongParameters() {
 		$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));
+	}
 }