sign all encrypted blocks and check signature on decrypt

2 files changed, 117 insertions, 27 deletions

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;
 	}
 
 	/**