Merge pull request #4352 from owncloud/encryption_clean_up

crypt.php clean up
This commit is contained in:
Björn Schießle 2013-08-17 04:10:15 -07:00
commit d3e2f31ada
5 changed files with 50 additions and 445 deletions

View File

@ -30,9 +30,6 @@ use OC\Files\Filesystem;
*/ */
class Hooks { class Hooks {
// TODO: use passphrase for encrypting private key that is separate to
// the login password
/** /**
* @brief Startup encryption backend upon user login * @brief Startup encryption backend upon user login
* @note This method should never be called for users using client side encryption * @note This method should never be called for users using client side encryption

View File

@ -25,7 +25,6 @@
namespace OCA\Encryption; namespace OCA\Encryption;
//require_once '../3rdparty/Crypt_Blowfish/Blowfish.php';
require_once realpath(dirname(__FILE__) . '/../3rdparty/Crypt_Blowfish/Blowfish.php'); require_once realpath(dirname(__FILE__) . '/../3rdparty/Crypt_Blowfish/Blowfish.php');
/** /**
@ -86,7 +85,7 @@ class Crypt {
* blocks with encryption alone, hence padding is added to achieve the * blocks with encryption alone, hence padding is added to achieve the
* required length. * required length.
*/ */
public static function addPadding($data) { private static function addPadding($data) {
$padded = $data . 'xx'; $padded = $data . 'xx';
@ -99,7 +98,7 @@ class Crypt {
* @param string $padded padded data to remove padding from * @param string $padded padded data to remove padding from
* @return string unpadded data on success, false on error * @return string unpadded data on success, false on error
*/ */
public static function removePadding($padded) { private static function removePadding($padded) {
if (substr($padded, -2) === 'xx') { if (substr($padded, -2) === 'xx') {
@ -207,7 +206,7 @@ class Crypt {
* @param string $passphrase * @param string $passphrase
* @return string encrypted file content * @return string encrypted file content
*/ */
public static function encrypt($plainContent, $iv, $passphrase = '') { private static function encrypt($plainContent, $iv, $passphrase = '') {
if ($encryptedContent = openssl_encrypt($plainContent, 'AES-128-CFB', $passphrase, false, $iv)) { if ($encryptedContent = openssl_encrypt($plainContent, 'AES-128-CFB', $passphrase, false, $iv)) {
return $encryptedContent; return $encryptedContent;
@ -228,7 +227,7 @@ class Crypt {
* @throws \Exception * @throws \Exception
* @return string decrypted file content * @return string decrypted file content
*/ */
public static function decrypt($encryptedContent, $iv, $passphrase) { private static function decrypt($encryptedContent, $iv, $passphrase) {
if ($plainContent = openssl_decrypt($encryptedContent, 'AES-128-CFB', $passphrase, false, $iv)) { if ($plainContent = openssl_decrypt($encryptedContent, 'AES-128-CFB', $passphrase, false, $iv)) {
@ -248,7 +247,7 @@ class Crypt {
* @param string $iv IV to be concatenated * @param string $iv IV to be concatenated
* @returns string concatenated content * @returns string concatenated content
*/ */
public static function concatIv($content, $iv) { private static function concatIv($content, $iv) {
$combined = $content . '00iv00' . $iv; $combined = $content . '00iv00' . $iv;
@ -261,7 +260,7 @@ class Crypt {
* @param string $catFile concatenated data to be split * @param string $catFile concatenated data to be split
* @returns array keys: encrypted, iv * @returns array keys: encrypted, iv
*/ */
public static function splitIv($catFile) { private static function splitIv($catFile) {
// Fetch encryption metadata from end of file // Fetch encryption metadata from end of file
$meta = substr($catFile, -22); $meta = substr($catFile, -22);
@ -378,34 +377,6 @@ class Crypt {
} }
/**
* @brief Creates symmetric keyfile content using a generated key
* @param string $plainContent content to be encrypted
* @returns array keys: key, encrypted
* @note symmetricDecryptFileContent() can be used to decrypt files created using this method
*
* This function decrypts a file
*/
public static function symmetricEncryptFileContentKeyfile($plainContent) {
$key = self::generateKey();
if ($encryptedContent = self::symmetricEncryptFileContent($plainContent, $key)) {
return array(
'key' => $key,
'encrypted' => $encryptedContent
);
} else {
return false;
}
}
/** /**
* @brief Create asymmetrically encrypted keyfile content using a generated key * @brief Create asymmetrically encrypted keyfile content using a generated key
* @param string $plainContent content to be encrypted * @param string $plainContent content to be encrypted
@ -488,43 +459,11 @@ class Crypt {
} }
/**
* @brief Asymetrically encrypt a string using a public key
* @param $plainContent
* @param $publicKey
* @return string encrypted file
*/
public static function keyEncrypt($plainContent, $publicKey) {
openssl_public_encrypt($plainContent, $encryptedContent, $publicKey);
return $encryptedContent;
}
/**
* @brief Asymetrically decrypt a file using a private key
* @param $encryptedContent
* @param $privatekey
* @return string decrypted file
*/
public static function keyDecrypt($encryptedContent, $privatekey) {
$result = @openssl_private_decrypt($encryptedContent, $plainContent, $privatekey);
if ($result) {
return $plainContent;
}
return $result;
}
/** /**
* @brief Generates a pseudo random initialisation vector * @brief Generates a pseudo random initialisation vector
* @return String $iv generated IV * @return String $iv generated IV
*/ */
public static function generateIv() { private static function generateIv() {
if ($random = openssl_random_pseudo_bytes(12, $strong)) { if ($random = openssl_random_pseudo_bytes(12, $strong)) {
@ -550,7 +489,7 @@ class Crypt {
} }
/** /**
* @brief Generate a pseudo random 1024kb ASCII key * @brief Generate a pseudo random 1024kb ASCII key, used as file key
* @returns $key Generated key * @returns $key Generated key
*/ */
public static function generateKey() { public static function generateKey() {
@ -576,13 +515,13 @@ class Crypt {
} }
/** /**
* @brief Get the blowfish encryption handeler for a key * @brief Get the blowfish encryption handler for a key
* @param $key string (optional) * @param $key string (optional)
* @return \Crypt_Blowfish blowfish object * @return \Crypt_Blowfish blowfish object
* *
* if the key is left out, the default handeler will be used * if the key is left out, the default handler will be used
*/ */
public static function getBlowfish($key = '') { private static function getBlowfish($key = '') {
if ($key) { if ($key) {
@ -596,38 +535,6 @@ class Crypt {
} }
/**
* @param $passphrase
* @return mixed
*/
public static function legacyCreateKey($passphrase) {
// Generate a random integer
$key = mt_rand(10000, 99999) . mt_rand(10000, 99999) . mt_rand(10000, 99999) . mt_rand(10000, 99999);
// Encrypt the key with the passphrase
$legacyEncKey = self::legacyEncrypt($key, $passphrase);
return $legacyEncKey;
}
/**
* @brief encrypts content using legacy blowfish system
* @param string $content the cleartext message you want to encrypt
* @param string $passphrase
* @returns string encrypted content
*
* This function encrypts an content
*/
public static function legacyEncrypt($content, $passphrase = '') {
$bf = self::getBlowfish($passphrase);
return $bf->encrypt($content);
}
/** /**
* @brief decrypts content using legacy blowfish system * @brief decrypts content using legacy blowfish system
* @param string $content the cleartext message you want to decrypt * @param string $content the cleartext message you want to decrypt

View File

@ -21,30 +21,6 @@
* *
*/ */
# Bugs
# ----
# Sharing a file to a user without encryption set up will not provide them with access but won't notify the sharer
# Sharing all files to admin for recovery purposes still in progress
# Possibly public links are broken (not tested since last merge of master)
# Missing features
# ----------------
# Make sure user knows if large files weren't encrypted
# Test
# ----
# Test that writing files works when recovery is enabled, and sharing API is disabled
# Test trashbin support
// Old Todo:
// - Crypt/decrypt button in the userinterface
// - Setting if crypto should be on by default
// - Add a setting "Don´t encrypt files larger than xx because of performance
// reasons"
namespace OCA\Encryption; namespace OCA\Encryption;
/** /**
@ -57,45 +33,6 @@ namespace OCA\Encryption;
class Util { class Util {
// Web UI:
//// DONE: files created via web ui are encrypted
//// DONE: file created & encrypted via web ui are readable in web ui
//// DONE: file created & encrypted via web ui are readable via webdav
// WebDAV:
//// DONE: new data filled files added via webdav get encrypted
//// DONE: new data filled files added via webdav are readable via webdav
//// DONE: reading unencrypted files when encryption is enabled works via
//// webdav
//// DONE: files created & encrypted via web ui are readable via webdav
// Legacy support:
//// DONE: add method to check if file is encrypted using new system
//// DONE: add method to check if file is encrypted using old system
//// DONE: add method to fetch legacy key
//// DONE: add method to decrypt legacy encrypted data
// Admin UI:
//// DONE: changing user password also changes encryption passphrase
//// TODO: add support for optional recovery in case of lost passphrase / keys
//// TODO: add admin optional required long passphrase for users
//// TODO: implement flag system to allow user to specify encryption by folder, subfolder, etc.
// Integration testing:
//// TODO: test new encryption with versioning
//// DONE: test new encryption with sharing
//// TODO: test new encryption with proxies
const MIGRATION_COMPLETED = 1; // migration to new encryption completed const MIGRATION_COMPLETED = 1; // migration to new encryption completed
const MIGRATION_IN_PROGRESS = -1; // migration is running const MIGRATION_IN_PROGRESS = -1; // migration is running
const MIGRATION_OPEN = 0; // user still needs to be migrated const MIGRATION_OPEN = 0; // user still needs to be migrated
@ -878,46 +815,22 @@ class Util {
} }
/** /**
* @brief Decrypt a keyfile without knowing how it was encrypted * @brief Decrypt a keyfile
* @param string $filePath * @param string $filePath
* @param string $fileOwner
* @param string $privateKey * @param string $privateKey
* @return bool|string * @return bool|string
* @note Checks whether file was encrypted with openssl_seal or
* openssl_encrypt, and decrypts accrdingly
* @note This was used when 2 types of encryption for keyfiles was used,
* but now we've switched to exclusively using openssl_seal()
*/ */
public function decryptUnknownKeyfile($filePath, $fileOwner, $privateKey) { private function decryptKeyfile($filePath, $privateKey) {
// Get the encrypted keyfile // Get the encrypted keyfile
// NOTE: the keyfile format depends on how it was encrypted! At
// this stage we don't know how it was encrypted
$encKeyfile = Keymanager::getFileKey($this->view, $this->userId, $filePath); $encKeyfile = Keymanager::getFileKey($this->view, $this->userId, $filePath);
// We need to decrypt the keyfile
// Has the file been shared yet?
if (
$this->userId === $fileOwner
&& !Keymanager::getShareKey($this->view, $this->userId, $filePath) // NOTE: we can't use isShared() here because it's a post share hook so it always returns true
) {
// The file has no shareKey, and its keyfile must be
// decrypted conventionally
$plainKeyfile = Crypt::keyDecrypt($encKeyfile, $privateKey);
} else {
// The file has a shareKey and must use it for decryption // The file has a shareKey and must use it for decryption
$shareKey = Keymanager::getShareKey($this->view, $this->userId, $filePath); $shareKey = Keymanager::getShareKey($this->view, $this->userId, $filePath);
$plainKeyfile = Crypt::multiKeyDecrypt($encKeyfile, $shareKey, $privateKey); $plainKeyfile = Crypt::multiKeyDecrypt($encKeyfile, $shareKey, $privateKey);
}
return $plainKeyfile; return $plainKeyfile;
} }
/** /**
@ -956,7 +869,7 @@ class Util {
$fileOwner = \OC\Files\Filesystem::getOwner($filePath); $fileOwner = \OC\Files\Filesystem::getOwner($filePath);
// Decrypt keyfile // Decrypt keyfile
$plainKeyfile = $this->decryptUnknownKeyfile($filePath, $fileOwner, $privateKey); $plainKeyfile = $this->decryptKeyfile($filePath, $privateKey);
// Re-enc keyfile to (additional) sharekeys // Re-enc keyfile to (additional) sharekeys
$multiEncKey = Crypt::multiKeyEncrypt($plainKeyfile, $userPubKeys); $multiEncKey = Crypt::multiKeyEncrypt($plainKeyfile, $userPubKeys);

View File

@ -115,130 +115,6 @@ class Test_Encryption_Crypt extends \PHPUnit_Framework_TestCase {
} }
/**
* @large
* @return String
*/
function testGenerateIv() {
$iv = Encryption\Crypt::generateIv();
$this->assertEquals(16, strlen($iv));
return $iv;
}
/**
* @large
* @depends testGenerateIv
*/
function testConcatIv($iv) {
$catFile = Encryption\Crypt::concatIv($this->dataLong, $iv);
// Fetch encryption metadata from end of file
$meta = substr($catFile, -22);
$identifier = substr($meta, 0, 6);
// Fetch IV from end of file
$foundIv = substr($meta, 6);
$this->assertEquals('00iv00', $identifier);
$this->assertEquals($iv, $foundIv);
// Remove IV and IV identifier text to expose encrypted content
$data = substr($catFile, 0, -22);
$this->assertEquals($this->dataLong, $data);
return array(
'iv' => $iv
,
'catfile' => $catFile
);
}
/**
* @medium
* @depends testConcatIv
*/
function testSplitIv($testConcatIv) {
// Split catfile into components
$splitCatfile = Encryption\Crypt::splitIv($testConcatIv['catfile']);
// Check that original IV and split IV match
$this->assertEquals($testConcatIv['iv'], $splitCatfile['iv']);
// Check that original data and split data match
$this->assertEquals($this->dataLong, $splitCatfile['encrypted']);
}
/**
* @medium
* @return string padded
*/
function testAddPadding() {
$padded = Encryption\Crypt::addPadding($this->dataLong);
$padding = substr($padded, -2);
$this->assertEquals('xx', $padding);
return $padded;
}
/**
* @medium
* @depends testAddPadding
*/
function testRemovePadding($padded) {
$noPadding = Encryption\Crypt::RemovePadding($padded);
$this->assertEquals($this->dataLong, $noPadding);
}
/**
* @medium
*/
function testEncrypt() {
$random = openssl_random_pseudo_bytes(13);
$iv = substr(base64_encode($random), 0, -4); // i.e. E5IG033j+mRNKrht
$crypted = Encryption\Crypt::encrypt($this->dataUrl, $iv, 'hat');
$this->assertNotEquals($this->dataUrl, $crypted);
}
/**
* @medium
*/
function testDecrypt() {
$random = openssl_random_pseudo_bytes(13);
$iv = substr(base64_encode($random), 0, -4); // i.e. E5IG033j+mRNKrht
$crypted = Encryption\Crypt::encrypt($this->dataUrl, $iv, 'hat');
$decrypt = Encryption\Crypt::decrypt($crypted, $iv, 'hat');
$this->assertEquals($this->dataUrl, $decrypt);
}
function testDecryptPrivateKey() { function testDecryptPrivateKey() {
// test successful decrypt // test successful decrypt
@ -364,14 +240,12 @@ class Test_Encryption_Crypt extends \PHPUnit_Framework_TestCase {
//print_r($r); //print_r($r);
// Join IVs and their respective data chunks // Join IVs and their respective data chunks
$e = array( $e = array();
$r[0] . $r[1], $i = 0;
$r[2] . $r[3], while ($i < count($r)-1) {
$r[4] . $r[5], $e[] = $r[$i] . $r[$i+1];
$r[6] . $r[7], $i = $i + 2;
$r[8] . $r[9], }
$r[10] . $r[11]
); //.$r[11], $r[12].$r[13], $r[14] );
//print_r($e); //print_r($e);
@ -466,24 +340,6 @@ class Test_Encryption_Crypt extends \PHPUnit_Framework_TestCase {
$this->view->unlink($this->userId . '/files/' . $filename); $this->view->unlink($this->userId . '/files/' . $filename);
} }
/**
* @medium
*/
function testSymmetricEncryptFileContentKeyfile() {
# TODO: search in keyfile for actual content as IV will ensure this test always passes
$crypted = Encryption\Crypt::symmetricEncryptFileContentKeyfile($this->dataUrl);
$this->assertNotEquals($this->dataUrl, $crypted['encrypted']);
$decrypt = Encryption\Crypt::symmetricDecryptFileContent($crypted['encrypted'], $crypted['key']);
$this->assertEquals($this->dataUrl, $decrypt);
}
/** /**
* @medium * @medium
*/ */
@ -526,49 +382,13 @@ class Test_Encryption_Crypt extends \PHPUnit_Framework_TestCase {
} }
/**
* @medium
*/
function testKeyEncrypt() {
// Generate keypair
$pair1 = Encryption\Crypt::createKeypair();
// Encrypt data
$crypted = Encryption\Crypt::keyEncrypt($this->dataUrl, $pair1['publicKey']);
$this->assertNotEquals($this->dataUrl, $crypted);
// Decrypt data
$decrypt = Encryption\Crypt::keyDecrypt($crypted, $pair1['privateKey']);
$this->assertEquals($this->dataUrl, $decrypt);
}
/**
* @medium
* @brief test encryption using legacy blowfish method
*/
function testLegacyEncryptShort() {
$crypted = Encryption\Crypt::legacyEncrypt($this->dataShort, $this->pass);
$this->assertNotEquals($this->dataShort, $crypted);
# TODO: search inencrypted text for actual content to ensure it
# genuine transformation
return $crypted;
}
/** /**
* @medium * @medium
* @brief test decryption using legacy blowfish method * @brief test decryption using legacy blowfish method
* @depends testLegacyEncryptShort
*/ */
function testLegacyDecryptShort($crypted) { function testLegacyDecryptShort() {
$crypted = $this->legacyEncrypt($this->dataShort, $this->pass);
$decrypted = Encryption\Crypt::legacyBlockDecrypt($crypted, $this->pass); $decrypted = Encryption\Crypt::legacyBlockDecrypt($crypted, $this->pass);
@ -576,55 +396,17 @@ class Test_Encryption_Crypt extends \PHPUnit_Framework_TestCase {
} }
/**
* @medium
* @brief test encryption using legacy blowfish method
*/
function testLegacyEncryptLong() {
$crypted = Encryption\Crypt::legacyEncrypt($this->dataLong, $this->pass);
$this->assertNotEquals($this->dataLong, $crypted);
# TODO: search inencrypted text for actual content to ensure it
# genuine transformation
return $crypted;
}
/** /**
* @medium * @medium
* @brief test decryption using legacy blowfish method * @brief test decryption using legacy blowfish method
* @depends testLegacyEncryptLong
*/ */
function testLegacyDecryptLong($crypted) { function testLegacyDecryptLong() {
$crypted = $this->legacyEncrypt($this->dataLong, $this->pass);
$decrypted = Encryption\Crypt::legacyBlockDecrypt($crypted, $this->pass); $decrypted = Encryption\Crypt::legacyBlockDecrypt($crypted, $this->pass);
$this->assertEquals($this->dataLong, $decrypted); $this->assertEquals($this->dataLong, $decrypted);
$this->assertFalse(Encryption\Crypt::getBlowfish(''));
}
/**
* @medium
* @brief test generation of legacy encryption key
* @depends testLegacyDecryptShort
*/
function testLegacyCreateKey() {
// Create encrypted key
$encKey = Encryption\Crypt::legacyCreateKey($this->pass);
// Decrypt key
$key = Encryption\Crypt::legacyBlockDecrypt($encKey, $this->pass);
$this->assertTrue(is_numeric($key));
// Check that key is correct length
$this->assertEquals(20, strlen($key));
} }
/** /**
@ -871,4 +653,20 @@ class Test_Encryption_Crypt extends \PHPUnit_Framework_TestCase {
// tear down // tear down
$view->unlink($filename); $view->unlink($filename);
} }
/**
* @brief encryption using legacy blowfish method
* @param $data string data to encrypt
* @param $passwd string password
* @return string
*/
function legacyEncrypt($data, $passwd) {
$bf = new \Crypt_Blowfish($passwd);
$crypted = $bf->encrypt($data);
return $crypted;
}
} }

View File

@ -141,10 +141,7 @@ class Test_Encryption_Keymanager extends \PHPUnit_Framework_TestCase {
*/ */
function testSetFileKey() { function testSetFileKey() {
# NOTE: This cannot be tested until we are able to break out $key = $this->randomKey;
# of the FileSystemView data directory root
$key = Encryption\Crypt::symmetricEncryptFileContentKeyfile($this->randomKey, 'hat');
$file = 'unittest-' . time() . '.txt'; $file = 'unittest-' . time() . '.txt';
@ -152,24 +149,17 @@ class Test_Encryption_Keymanager extends \PHPUnit_Framework_TestCase {
$proxyStatus = \OC_FileProxy::$enabled; $proxyStatus = \OC_FileProxy::$enabled;
\OC_FileProxy::$enabled = false; \OC_FileProxy::$enabled = false;
$this->view->file_put_contents($this->userId . '/files/' . $file, $key['encrypted']); $this->view->file_put_contents($this->userId . '/files/' . $file, $this->dataShort);
// Re-enable proxy - our work is done Encryption\Keymanager::setFileKey($this->view, $file, $this->userId, $key);
\OC_FileProxy::$enabled = $proxyStatus;
//$view = new \OC_FilesystemView( '/' . $this->userId . '/files_encryption/keyfiles' ); $this->assertTrue($this->view->file_exists('/' . $this->userId . '/files_encryption/keyfiles/' . $file . '.key'));
Encryption\Keymanager::setFileKey($this->view, $file, $this->userId, $key['key']);
// enable encryption proxy
$proxyStatus = \OC_FileProxy::$enabled;
\OC_FileProxy::$enabled = true;
// cleanup // cleanup
$this->view->unlink('/' . $this->userId . '/files/' . $file); $this->view->unlink('/' . $this->userId . '/files/' . $file);
// change encryption proxy to previous state // change encryption proxy to previous state
\OC_FileProxy::$enabled = $proxyStatus; \OC_FileProxy::$enabled = $proxyStatus;
} }
/** /**