From 40a5ba72fc868207356c9143c99a947f1a6e6500 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bj=C3=B6rn=20Schie=C3=9Fle?= <bjoern@schiessle.org>
Date: Tue, 5 Jan 2016 12:51:05 +0100
Subject: [PATCH] sign all encrypted blocks and check signature on decrypt
---
apps/encryption/appinfo/application.php | 3 +-
apps/encryption/appinfo/register_command.php | 3 +-
apps/encryption/lib/crypto/crypt.php | 131 +++++++++++++++---
apps/encryption/lib/crypto/encryption.php | 13 +-
apps/encryption/settings/settings-admin.php | 3 +-
.../encryption/settings/settings-personal.php | 3 +-
.../encryption/tests/lib/crypto/cryptTest.php | 7 +-
settings/changepassword/controller.php | 3 +-
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;
@@ -287,6 +303,15 @@ class Crypt {
return $encryptedContent . '00iv00' . $iv;
}
+ /**
+ * @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
@@ -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,41 +453,101 @@ 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
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(),
--
GitLab