sign all encrypted blocks and check signature on decrypt

8 files changed, 133 insertions, 33 deletions

diff --git a/apps/encryption/appinfo/application.php b/apps/encryption/appinfo/application.php
index 433e9e86284..6d01d3e8353 100644
--- a/apps/encryption/appinfo/application.php
+++ b/apps/encryption/appinfo/application.php
@@ -131,7 +131,8 @@ class Application extends \OCP\AppFramework\App {
 				$server = $c->getServer();
 				return new Crypt($server->getLogger(),
 					$server->getUserSession(),
-					$server->getConfig());
+					$server->getConfig(),
+					$server->getL10N($c->getAppName()));
 			});
 
 		$container->registerService('Session',
diff --git a/apps/encryption/appinfo/register_command.php b/apps/encryption/appinfo/register_command.php
index 2bb49d55c2e..5f32718cdf0 100644
--- a/apps/encryption/appinfo/register_command.php
+++ b/apps/encryption/appinfo/register_command.php
@@ -25,11 +25,12 @@ use Symfony\Component\Console\Helper\QuestionHelper;
 $userManager = OC::$server->getUserManager();
 $view = new \OC\Files\View();
 $config = \OC::$server->getConfig();
+$l = \OC::$server->getL10N('encryption');
 $userSession = \OC::$server->getUserSession();
 $connection = \OC::$server->getDatabaseConnection();
 $logger = \OC::$server->getLogger();
 $questionHelper = new QuestionHelper();
-$crypt = new \OCA\Encryption\Crypto\Crypt($logger, $userSession, $config);
+$crypt = new \OCA\Encryption\Crypto\Crypt($logger, $userSession, $config, $l);
 $util = new \OCA\Encryption\Util($view, $crypt, $logger, $userSession, $config, $userManager);
 
 $application->add(new MigrateKeys($userManager, $view, $connection, $config, $logger));
diff --git a/apps/encryption/lib/crypto/crypt.php b/apps/encryption/lib/crypto/crypt.php
index 1cbf39a82bf..f4c47d33f00 100644
--- a/apps/encryption/lib/crypto/crypt.php
+++ b/apps/encryption/lib/crypto/crypt.php
@@ -29,10 +29,12 @@ namespace OCA\Encryption\Crypto;
 
 use OC\Encryption\Exceptions\DecryptionFailedException;
 use OC\Encryption\Exceptions\EncryptionFailedException;
+use OC\HintException;
 use OCA\Encryption\Exceptions\MultiKeyDecryptException;
 use OCA\Encryption\Exceptions\MultiKeyEncryptException;
 use OCP\Encryption\Exceptions\GenericEncryptionException;
 use OCP\IConfig;
+use OCP\IL10N;
 use OCP\ILogger;
 use OCP\IUserSession;
 
@@ -60,14 +62,22 @@ class Crypt {
 
 	const HEADER_START = 'HBEGIN';
 	const HEADER_END = 'HEND';
+
 	/** @var ILogger */
 	private $logger;
+
 	/** @var string */
 	private $user;
+
 	/** @var IConfig */
 	private $config;
+
 	/** @var array */
 	private $supportedKeyFormats;
+
+	/** @var IL10N */
+	private $l;
+
 	/** @var array */
 	private $supportedCiphersAndKeySize = [
 		'AES-256-CTR' => 32,
@@ -80,11 +90,13 @@ class Crypt {
 	 * @param ILogger $logger
 	 * @param IUserSession $userSession
 	 * @param IConfig $config
+	 * @param IL10N $l
 	 */
-	public function __construct(ILogger $logger, IUserSession $userSession, IConfig $config) {
+	public function __construct(ILogger $logger, IUserSession $userSession, IConfig $config, IL10N $l) {
 		$this->logger = $logger;
 		$this->user = $userSession && $userSession->isLoggedIn() ? $userSession->getUser()->getUID() : '"no user given"';
 		$this->config = $config;
+		$this->l = $l;
 		$this->supportedKeyFormats = ['hash', 'password'];
 	}
 
@@ -172,8 +184,12 @@ class Crypt {
 			$iv,
 			$passPhrase,
 			$this->getCipher());
+
+		$sig = $this->createSignature($encryptedContent, $passPhrase);
+
 		// combine content to encrypt the IV identifier and actual IV
 		$catFile = $this->concatIV($encryptedContent, $iv);
+		$catFile = $this->concatSig($catFile, $sig);
 		$padded = $this->addPadding($catFile);
 
 		return $padded;
@@ -288,6 +304,15 @@ class Crypt {
 	}
 
 	/**
+	 * @param string $encryptedContent
+	 * @param string $signature
+	 * @return string
+	 */
+	private function concatSig($encryptedContent, $signature) {
+		return $encryptedContent . '00sig00' . $signature;
+	}
+
+	/**
 	 * Note: This is _NOT_ a padding used for encryption purposes. It is solely
 	 * used to achieve the PHP stream size. It has _NOTHING_ to do with the
 	 * encrypted content and is not used in any crypto primitive.
@@ -296,7 +321,7 @@ class Crypt {
 	 * @return string
 	 */
 	private function addPadding($data) {
-		return $data . 'xx';
+		return $data . 'xxx';
 	}
 
 	/**
@@ -414,10 +439,12 @@ class Crypt {
 	 * @throws DecryptionFailedException
 	 */
 	public function symmetricDecryptFileContent($keyFileContents, $passPhrase, $cipher = self::DEFAULT_CIPHER) {
-		// Remove Padding
-		$noPadding = $this->removePadding($keyFileContents);
 
-		$catFile = $this->splitIv($noPadding);
+		$catFile = $this->splitMetaData($keyFileContents, $cipher);
+
+		if ($catFile['signature']) {
+			$this->checkSignature($catFile['encrypted'], $passPhrase, $catFile['signature']);
+		}
 
 		return $this->decrypt($catFile['encrypted'],
 			$catFile['iv'],
@@ -426,42 +453,102 @@ class Crypt {
 	}
 
 	/**
-	 * remove padding
+	 * check for valid signature
 	 *
-	 * @param $padded
-	 * @return string|false
+	 * @param string $data
+	 * @param string $passPhrase
+	 * @param string $expectedSignature
+	 * @throws HintException
 	 */
-	private function removePadding($padded) {
-		if (substr($padded, -2) === 'xx') {
-			return substr($padded, 0, -2);
+	private function checkSignature($data, $passPhrase, $expectedSignature) {
+		$signature = $this->createSignature($data, $passPhrase);
+		if (hash_equals($expectedSignature, $signature)) {
+			throw new HintException('Bad Signature', $this->l->t('Bad Signature'));
 		}
-		return false;
 	}
 
 	/**
-	 * split iv from encrypted content
+	 * create signature
 	 *
-	 * @param string|false $catFile
+	 * @param string $data
+	 * @param string $passPhrase
 	 * @return string
 	 */
-	private function splitIv($catFile) {
-		// Fetch encryption metadata from end of file
-		$meta = substr($catFile, -22);
+	private function createSignature($data, $passPhrase) {
+		$signature = hash_hmac('sha256', $data, $passPhrase);
+		return $signature;
+	}
 
-		// Fetch IV from end of file
-		$iv = substr($meta, -16);
 
-		// Remove IV and IV Identifier text to expose encrypted content
+	/**
+	 * remove padding
+	 *
+	 * @param string $padded
+	 * @param bool $hasSignature did the block contain a signature, in this case we use a different padding
+	 * @return string|false
+	 */
+	private function removePadding($padded, $hasSignature = false) {
+		if ($hasSignature === false && substr($padded, -2) === 'xx') {
+			return substr($padded, 0, -2);
+		} elseif ($hasSignature === true && substr($padded, -3) === 'xxx') {
+			return substr($padded, 0, -3);
+		}
+		return false;
+	}
 
-		$encrypted = substr($catFile, 0, -22);
+	/**
+	 * split meta data from encrypted file
+	 * Note: for now, we assume that the meta data always start with the iv
+	 *       followed by the signature, if available
+	 *
+	 * @param string $catFile
+	 * @param string $cipher
+	 * @return array
+	 */
+	private function splitMetaData($catFile, $cipher) {
+		if ($this->hasSignature($catFile, $cipher)) {
+			$catFile = $this->removePadding($catFile, true);
+			$meta = substr($catFile, -93);
+			$iv = substr($meta, strlen('00iv00'), 16);
+			$sig = substr($meta, 22 + strlen('00sig00'));
+			$encrypted = substr($catFile, 0, -93);
+		} else {
+			$catFile = $this->removePadding($catFile);
+			$meta = substr($catFile, -22);
+			$iv = substr($meta, -16);
+			$sig = false;
+			$encrypted = substr($catFile, 0, -93);
+		}
 
 		return [
 			'encrypted' => $encrypted,
-			'iv' => $iv
+			'iv' => $iv,
+			'signature' => $sig
 		];
 	}
 
 	/**
+	 * check if encrypted block is signed
+	 *
+	 * @param string $catFile
+	 * @param string $cipher
+	 * @return bool
+	 * @throws HintException
+	 */
+	private function hasSignature($catFile, $cipher) {
+		$meta = substr($catFile, 93);
+		$signaturePosition = strpos($meta, '00sig00');
+
+		// enforce signature for the new 'CTR' ciphers
+		if ($signaturePosition === false && strpos(strtolower($cipher), 'ctr') !== false) {
+			throw new HintException('Missing Signature', $this->l->t('Missing Signature'));
+		}
+
+		return ($signaturePosition !== false);
+	}
+
+
+	/**
 	 * @param string $encryptedContent
 	 * @param string $iv
 	 * @param string $passPhrase
diff --git a/apps/encryption/lib/crypto/encryption.php b/apps/encryption/lib/crypto/encryption.php
index 3b66684a7f4..7099f53e2ab 100644
--- a/apps/encryption/lib/crypto/encryption.php
+++ b/apps/encryption/lib/crypto/encryption.php
@@ -94,6 +94,9 @@ class Encryption implements IEncryptionModule {
 	/** @var DecryptAll  */
 	private $decryptAll;
 
+	/** @var int unencrypted block size */
+	private $unencryptedBlockSize = 6072;
+
 	/**
 	 *
 	 * @param Crypt $crypt
@@ -253,7 +256,7 @@ class Encryption implements IEncryptionModule {
 	public function encrypt($data) {
 
 		// If extra data is left over from the last round, make sure it
-		// is integrated into the next 6126 / 8192 block
+		// is integrated into the next block
 		if ($this->writeCache) {
 
 			// Concat writeCache to start of $data
@@ -275,7 +278,7 @@ class Encryption implements IEncryptionModule {
 
 			// If data remaining to be written is less than the
 			// size of 1 6126 byte block
-			if ($remainingLength < 6126) {
+			if ($remainingLength < $this->unencryptedBlockSize) {
 
 				// Set writeCache to contents of $data
 				// The writeCache will be carried over to the
@@ -293,14 +296,14 @@ class Encryption implements IEncryptionModule {
 			} else {
 
 				// Read the chunk from the start of $data
-				$chunk = substr($data, 0, 6126);
+				$chunk = substr($data, 0, $this->unencryptedBlockSize);
 
 				$encrypted .= $this->crypt->symmetricEncryptFileContent($chunk, $this->fileKey);
 
 				// Remove the chunk we just processed from
 				// $data, leaving only unprocessed data in $data
 				// var, for handling on the next round
-				$data = substr($data, 6126);
+				$data = substr($data, $this->unencryptedBlockSize);
 
 			}
 
@@ -410,7 +413,7 @@ class Encryption implements IEncryptionModule {
 	 * @return integer
 	 */
 	public function getUnencryptedBlockSize() {
-		return 6126;
+		return $this->unencryptedBlockSize;
 	}
 
 	/**
diff --git a/apps/encryption/settings/settings-admin.php b/apps/encryption/settings/settings-admin.php
index c3d523f27da..6c7c0987fd7 100644
--- a/apps/encryption/settings/settings-admin.php
+++ b/apps/encryption/settings/settings-admin.php
@@ -29,7 +29,8 @@ $tmpl = new OCP\Template('encryption', 'settings-admin');
 $crypt = new \OCA\Encryption\Crypto\Crypt(
 	\OC::$server->getLogger(),
 	\OC::$server->getUserSession(),
-	\OC::$server->getConfig());
+	\OC::$server->getConfig(),
+	\OC::$server->getL10N('encryption'));
 
 $util = new \OCA\Encryption\Util(
 	new \OC\Files\View(),
diff --git a/apps/encryption/settings/settings-personal.php b/apps/encryption/settings/settings-personal.php
index 2dff5904850..0f6e9353707 100644
--- a/apps/encryption/settings/settings-personal.php
+++ b/apps/encryption/settings/settings-personal.php
@@ -28,7 +28,8 @@ $template = new OCP\Template('encryption', 'settings-personal');
 $crypt = new \OCA\Encryption\Crypto\Crypt(
 	\OC::$server->getLogger(),
 	$userSession,
-	\OC::$server->getConfig());
+	\OC::$server->getConfig(),
+	\OC::$server->getL10N('encryption'));
 
 $util = new \OCA\Encryption\Util(
 	new \OC\Files\View(),
diff --git a/apps/encryption/tests/lib/crypto/cryptTest.php b/apps/encryption/tests/lib/crypto/cryptTest.php
index ce4dfb68cf1..c774da1836c 100644
--- a/apps/encryption/tests/lib/crypto/cryptTest.php
+++ b/apps/encryption/tests/lib/crypto/cryptTest.php
@@ -39,6 +39,10 @@ class cryptTest extends TestCase {
 	/** @var \PHPUnit_Framework_MockObject_MockObject */
 	private $config;
 
+
+	/** @var \PHPUnit_Framework_MockObject_MockObject */
+	private $l;
+
 	/** @var  Crypt */
 	private $crypt;
 
@@ -57,8 +61,9 @@ class cryptTest extends TestCase {
 		$this->config = $this->getMockBuilder('OCP\IConfig')
 			->disableOriginalConstructor()
 			->getMock();
+		$this->l = $this->getMock('OCP\IL10N');
 
-		$this->crypt = new Crypt($this->logger, $this->userSession, $this->config);
+		$this->crypt = new Crypt($this->logger, $this->userSession, $this->config, $this->l);
 	}
 
 	/**
diff --git a/settings/changepassword/controller.php b/settings/changepassword/controller.php
index bfa599c1d04..8469ec1423a 100644
--- a/settings/changepassword/controller.php
+++ b/settings/changepassword/controller.php
@@ -89,7 +89,8 @@ class Controller {
 			$crypt = new \OCA\Encryption\Crypto\Crypt(
 				\OC::$server->getLogger(),
 				\OC::$server->getUserSession(),
-				\OC::$server->getConfig());
+				\OC::$server->getConfig(),
+				\OC::$server->getL10N('encryption'));
 			$keyStorage = \OC::$server->getEncryptionKeyStorage();
 			$util = new \OCA\Encryption\Util(
 				new \OC\Files\View(),