Make legacy format opt-in

Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>

3 files changed, 40 insertions, 10 deletions

diff --git a/apps/encryption/lib/Crypto/Crypt.php b/apps/encryption/lib/Crypto/Crypt.php
index c8311f4cf73..96438dfd110 100644
--- a/apps/encryption/lib/Crypto/Crypt.php
+++ b/apps/encryption/lib/Crypto/Crypt.php
@@ -31,6 +31,7 @@ namespace OCA\Encryption\Crypto;
 
 use OC\Encryption\Exceptions\DecryptionFailedException;
 use OC\Encryption\Exceptions\EncryptionFailedException;
+use OC\ServerNotAvailableException;
 use OCA\Encryption\Exceptions\MultiKeyDecryptException;
 use OCA\Encryption\Exceptions\MultiKeyEncryptException;
 use OCP\Encryption\Exceptions\GenericEncryptionException;
@@ -89,6 +90,9 @@ class Crypt {
 		'AES-128-CFB' => 16,
 	];
 
+	/** @var bool */
+	private $supportLegacy;
+
 	/**
 	 * @param ILogger $logger
 	 * @param IUserSession $userSession
@@ -101,6 +105,8 @@ class Crypt {
 		$this->config = $config;
 		$this->l = $l;
 		$this->supportedKeyFormats = ['hash', 'password'];
+
+		$this->supportLegacy = $this->config->getSystemValueBool('encryption.legacy_format_support', true);
 	}
 
 	/**
@@ -302,6 +308,10 @@ class Crypt {
 	 * @return string
 	 */
 	public function getLegacyCipher() {
+		if (!$this->supportLegacy) {
+			throw new ServerNotAvailableException('Legacy cipher is no longer supported!');
+		}
+
 		return self::LEGACY_CIPHER;
 	}
 
@@ -395,7 +405,7 @@ class Crypt {
 		if (isset($header['cipher'])) {
 			$cipher = $header['cipher'];
 		} else {
-			$cipher = self::LEGACY_CIPHER;
+			$cipher = $this->getLegacyCipher();
 		}
 
 		if (isset($header['keyFormat'])) {
@@ -574,6 +584,11 @@ class Crypt {
 		$meta = substr($catFile, -93);
 		$signaturePosition = strpos($meta, '00sig00');
 
+		// If we no longer support the legacy format then everything needs a signature
+		if (!$skipSignatureCheck && !$this->supportLegacy && $signaturePosition === false) {
+			throw new GenericEncryptionException('Missing Signature', $this->l->t('Missing Signature'));
+		}
+
 		// enforce signature for the new 'CTR' ciphers
 		if (!$skipSignatureCheck && $signaturePosition === false && stripos($cipher, 'ctr') !== false) {
 			throw new GenericEncryptionException('Missing Signature', $this->l->t('Missing Signature'));
diff --git a/apps/encryption/tests/Crypto/CryptTest.php b/apps/encryption/tests/Crypto/CryptTest.php
index 9645dc3cce0..f606457704a 100644
--- a/apps/encryption/tests/Crypto/CryptTest.php
+++ b/apps/encryption/tests/Crypto/CryptTest.php
@@ -215,6 +215,10 @@ class CryptTest extends TestCase {
 	 * @dataProvider dataTestSplitMetaData
 	 */
 	public function testSplitMetaData($data, $expected) {
+		$this->config->method('getSystemValue')
+			->with('encryption_skip_signature_check', false)
+			->willReturn(true);
+
 		$result = self::invokePrivate($this->crypt, 'splitMetaData', array($data, 'AES-256-CFB'));
 		$this->assertTrue(is_array($result));
 		$this->assertSame(3, count($result));
@@ -239,6 +243,10 @@ class CryptTest extends TestCase {
 	 * @dataProvider dataTestHasSignature
 	 */
 	public function testHasSignature($data, $expected) {
+		$this->config->method('getSystemValue')
+			->with('encryption_skip_signature_check', false)
+			->willReturn(true);
+
 		$this->assertSame($expected,
 			$this->invokePrivate($this->crypt, 'hasSignature', array($data, 'AES-256-CFB'))
 		);
@@ -394,6 +402,10 @@ class CryptTest extends TestCase {
 	 * @dataProvider dataTestDecryptPrivateKey
 	 */
 	public function testDecryptPrivateKey($header, $privateKey, $expectedCipher, $isValidKey, $expected) {
+		$this->config->method('getSystemValueBool')
+			->with('encryption.legacy_format_support', true)
+			->willReturn(true);
+
 		/** @var \OCA\Encryption\Crypto\Crypt | \PHPUnit_Framework_MockObject_MockObject $crypt */
 		$crypt = $this->getMockBuilder(Crypt::class)
 			->setConstructorArgs(
diff --git a/apps/encryption/tests/Settings/AdminTest.php b/apps/encryption/tests/Settings/AdminTest.php
index 54b3187a97e..07f406f5303 100644
--- a/apps/encryption/tests/Settings/AdminTest.php
+++ b/apps/encryption/tests/Settings/AdminTest.php
@@ -72,20 +72,23 @@ class AdminTest extends TestCase {
 
 	public function testGetForm() {
 		$this->config
-			->expects($this->at(0))
 			->method('getAppValue')
-			->with('encryption', 'recoveryAdminEnabled', '0')
-			->willReturn(1);
-		$this->config
-			->expects($this->at(1))
-			->method('getAppValue')
-			->with('encryption', 'encryptHomeStorage', '1')
-			->willReturn(1);
+			->will($this->returnCallback(function ($app, $key, $default) {
+				if ($app === 'encryption') {
+					if ($key === 'recoveryAdminEnabled') {
+						return 1;
+					}
+					if ($key === 'encryptHomeStorage') {
+						return 1;
+					}
+				}
+				return $default;
+			}));
 		$params = [
 			'recoveryEnabled' => 1,
 			'initStatus' => '0',
 			'encryptHomeStorage' => false,
-			'masterKeyEnabled' => false
+			'masterKeyEnabled' => true
 		];
 		$expected = new TemplateResponse('encryption', 'settings-admin', $params, '');
 		$this->assertEquals($expected, $this->admin->getForm());