From 7fe92456360ea02d07e6f1d8e38f2f673ad20323 Mon Sep 17 00:00:00 2001 From: Sam Tuke Date: Sat, 5 Jan 2013 17:12:23 +0000 Subject: [PATCH] Development snapshot Crypt{} & Util{} unit tests now passing locally Added keymanager unit tests --- apps/files_encryption/hooks/hooks.php | 2 +- apps/files_encryption/lib/crypt.php | 2 +- apps/files_encryption/lib/keymanager.php | 65 ++++++++-------------- apps/files_encryption/lib/proxy.php | 8 ++- apps/files_encryption/lib/stream.php | 12 +++- apps/files_encryption/tests/crypt.php | 29 +++------- apps/files_encryption/tests/keymanager.php | 54 ++++++++++++++---- 7 files changed, 95 insertions(+), 77 deletions(-) diff --git a/apps/files_encryption/hooks/hooks.php b/apps/files_encryption/hooks/hooks.php index 7545520fa78..59bf4921913 100644 --- a/apps/files_encryption/hooks/hooks.php +++ b/apps/files_encryption/hooks/hooks.php @@ -54,7 +54,7 @@ class Hooks { \OC_FileProxy::$enabled = false; - $encryptedKey = Keymanager::getPrivateKey( $params['uid'], $view ); + $encryptedKey = Keymanager::getPrivateKey( $view, $params['uid'] ); \OC_FileProxy::$enabled = true; diff --git a/apps/files_encryption/lib/crypt.php b/apps/files_encryption/lib/crypt.php index 7895a5dd7b0..4e2128e89f4 100755 --- a/apps/files_encryption/lib/crypt.php +++ b/apps/files_encryption/lib/crypt.php @@ -628,7 +628,7 @@ class Crypt { public static function changekeypasscode($oldPassword, $newPassword) { if(\OCP\User::isLoggedIn()){ - $key = Keymanager::getPrivateKey(); + $key = Keymanager::getPrivateKey( $user, $view ); if ( ($key = Crypt::symmetricDecryptFileContent($key,$oldpasswd)) ) { if ( ($key = Crypt::symmetricEncryptFileContent($key, $newpasswd)) ) { Keymanager::setPrivateKey($key); diff --git a/apps/files_encryption/lib/keymanager.php b/apps/files_encryption/lib/keymanager.php index d3be166add2..818cd1a154d 100755 --- a/apps/files_encryption/lib/keymanager.php +++ b/apps/files_encryption/lib/keymanager.php @@ -23,7 +23,8 @@ namespace OCA\Encryption; /** - * This class provides basic operations to read/write encryption keys from/to the filesystem + * @brief Class to manage storage and retrieval of encryption keys + * @note Where a method requires a view object, it's root must be '/' */ class Keymanager { @@ -35,60 +36,46 @@ class Keymanager { * @return string private key or false * @note the key returned by this method must be decrypted before use */ - public static function getPrivateKey( $user, $view ) { + public static function getPrivateKey( $view, $user ) { - $view->chroot( '/' . $user . '/' . 'files_encryption' ); - return $view->file_get_contents( '/' . $user.'.private.key' ); - + return $view->file_get_contents( '/' . $user . '/' . 'files_encryption' . '/' . $user.'.private.key' ); } /** * @brief retrieve public key for a specified user * @return string public key or false */ - public static function getPublicKey( $userId = NULL ) { - - // If the username wasn't specified, fetch it - if ( ! $userId ) { - - $userId = \OCP\User::getUser(); - - } + public static function getPublicKey( $view, $userId ) { - // Create new view with the right - $view = new \OC_FilesystemView( '/public-keys/' ); - - return $view->file_get_contents( '/' . $userId . '.public.key' ); + return $view->file_get_contents( '/public-keys/' . '/' . $userId . '.public.key' ); } /** * @brief retrieve both keys from a user (private and public) - * - * @return string private key or false + * @return array keys: privateKey, publicKey */ - public static function getUserKeys() { + public static function getUserKeys( $view, $userId ) { - return array( - 'privatekey' => self::getPrivateKey(), - 'publickey' => self::getPublicKey(), + return array( + 'publicKey' => self::getPublicKey( $view, $userId ) + , 'privateKey' => self::getPrivateKey( $view, $userId ) ); } /** - * @brief retrieve a list of the public key from all users with access to the file - * - * @param string path to file + * @brief Retrieve public keys of all users with access to a file + * @param string $path Path to file * @return array of public keys for the given file + * @note Checks that the sharing app is enabled should be performed + * by client code, that isn't checked here */ - public static function getPublicKeys( $path ) { - - $userId = \OCP\User::getUser(); + public static function getPublicKeys( $view, $userId, $filePath ) { $path = ltrim( $path, '/' ); - $filepath = '/'.$userId.'/files/'.$path; + $filepath = '/' . $userId . '/files/' . $filePath; // Check if sharing is enabled if ( OC_App::isEnabled( 'files_sharing' ) ) { @@ -157,34 +144,30 @@ class Keymanager { /** * @brief retrieve keyfile for an encrypted file - * * @param string file name * @return string file key or false * @note The keyfile returned is asymmetrically encrypted. Decryption * of the keyfile must be performed by client code */ - public static function getFileKey( $path, $staticUserClass = 'OCP\User' ) { + public static function getFileKey( $view, $userId, $filePath ) { - $keypath = ltrim( $path, '/' ); - $user = $staticUserClass::getUser(); + $filePath_f = ltrim( $filePath, '/' ); -// // update $keypath and $user if path point to a file shared by someone else +// // update $keypath and $userId if path point to a file shared by someone else // $query = \OC_DB::prepare( "SELECT uid_owner, source, target FROM `*PREFIX*sharing` WHERE target = ? AND uid_shared_with = ?" ); // -// $result = $query->execute( array ('/'.$user.'/files/'.$keypath, $user)); +// $result = $query->execute( array ('/'.$userId.'/files/'.$keypath, $userId)); // // if ($row = $result->fetchRow()) { // // $keypath = $row['source']; // $keypath_parts = explode( '/', $keypath ); -// $user = $keypath_parts[1]; -// $keypath = str_replace( '/' . $user . '/files/', '', $keypath ); +// $userId = $keypath_parts[1]; +// $keypath = str_replace( '/' . $userId . '/files/', '', $keypath ); // // } - $view = new \OC_FilesystemView('/'.$user.'/files_encryption/keyfiles/'); - - return $view->file_get_contents( $keypath . '.key' ); + return $this->view->file_get_contents( '/' . $userId . '/files_encryption/keyfiles/' . $filePath_f ); } diff --git a/apps/files_encryption/lib/proxy.php b/apps/files_encryption/lib/proxy.php index 9c7b5f01afc..272d0a5509f 100644 --- a/apps/files_encryption/lib/proxy.php +++ b/apps/files_encryption/lib/proxy.php @@ -91,6 +91,10 @@ class Proxy extends \OC_FileProxy { if ( !is_resource( $data ) ) { //stream put contents should have been converted to fopen + $userId = \OCP\USER::getUser(); + + $rootView = new \OC_FilesystemView( '/' ); + // Set the filesize for userland, before encrypting $size = strlen( $data ); @@ -98,7 +102,7 @@ class Proxy extends \OC_FileProxy { \OC_FileProxy::$enabled = false; // Encrypt plain data and fetch key - $encrypted = Crypt::keyEncryptKeyfile( $data, Keymanager::getPublicKey() ); + $encrypted = Crypt::keyEncryptKeyfile( $data, Keymanager::getPublicKey( $rootView, $userId ) ); // Replace plain content with encrypted content by reference $data = $encrypted['data']; @@ -110,7 +114,7 @@ class Proxy extends \OC_FileProxy { $filePath = '/' . implode( '/', $filePath ); # TODO: make keyfile dir dynamic from app config - $view = new \OC_FilesystemView( '/' . \OCP\USER::getUser() . '/files_encryption/keyfiles' ); + $view = new \OC_FilesystemView( '/' . $userId . '/files_encryption/keyfiles' ); // Save keyfile for newly encrypted file in parallel directory tree Keymanager::setFileKey( $filePath, $encrypted['key'], $view, '\OC_DB' ); diff --git a/apps/files_encryption/lib/stream.php b/apps/files_encryption/lib/stream.php index dcdad7ee561..fc1b9808cc5 100644 --- a/apps/files_encryption/lib/stream.php +++ b/apps/files_encryption/lib/stream.php @@ -63,7 +63,8 @@ class Stream { private $publicKey; private $keyfile; private $encKeyfile; - private static $view; + private static $view; // a fsview object set to user dir + private $rootView; // a fsview object set to '/' public function stream_open( $path, $mode, $options, &$opened_path ) { @@ -76,6 +77,13 @@ class Stream { } + // Set rootview object if necessary + if ( ! $this->rootView ) { + + $this->rootView = new \OC_FilesystemView( $this->userId . '/' ); + + } + $this->userId = \OCP\User::getUser(); // Get the bare file path @@ -332,7 +340,7 @@ class Stream { $this->keyfile = Crypt::generateKey(); - $this->publicKey = Keymanager::getPublicKey( $this->userId ); + $this->publicKey = Keymanager::getPublicKey( $this->rootView, $this->userId ); $this->encKeyfile = Crypt::keyEncrypt( $this->keyfile, $this->publicKey ); diff --git a/apps/files_encryption/tests/crypt.php b/apps/files_encryption/tests/crypt.php index e64ec15c82f..446af7cfa09 100755 --- a/apps/files_encryption/tests/crypt.php +++ b/apps/files_encryption/tests/crypt.php @@ -7,21 +7,6 @@ * See the COPYING-README file. */ -// Load mockery files -require_once 'Mockery/Loader.php'; -require_once 'Hamcrest/Hamcrest.php'; -$loader = new \Mockery\Loader; -$loader->register(); - -use \Mockery as m; - -// Overload Session{} with a mock object before it is included -$adminEncPriKey = realpath( dirname(__FILE__).'/../../../data/admin/files_encryption/admin.private.key' ); -$adminDePriKey = OCA\Encryption\Crypt::symmetricDecryptFileContent( $adminEncPriKey, 'admin' ); - -$mockSession = m::mock('overload:OCA\Encryption\Session'); -$mockSession->shouldReceive( 'getPrivateKey' )->andReturn( file_get_contents( $adminDePriKey ) ); - //require_once "PHPUnit/Framework/TestCase.php"; require_once realpath( dirname(__FILE__).'/../../../3rdparty/Crypt_Blowfish/Blowfish.php' ); require_once realpath( dirname(__FILE__).'/../../../lib/base.php' ); @@ -38,7 +23,13 @@ use OCA\Encryption; // encryption key needs to be saved in the session \OC_User::login( 'admin', 'admin' ); -//trigger_error("session = ".var_export($_SESSION, 1)); +/** + * @note It would be better to use Mockery here for mocking out the session + * handling process, and isolate calls to session class and data from the unit + * tests relating to them (stream etc.). However getting mockery to work and + * overload classes whilst also using the OC autoloader is difficult due to + * load order Pear errors. + */ class Test_Crypt extends \PHPUnit_Framework_TestCase { @@ -66,8 +57,6 @@ class Test_Crypt extends \PHPUnit_Framework_TestCase { function tearDown() { - m::close(); - } function testGenerateKey() { @@ -247,7 +236,7 @@ class Test_Crypt extends \PHPUnit_Framework_TestCase { $this->assertNotEquals( $this->dataShort, $retreivedCryptedFile ); // Get private key - $encryptedPrivateKey = Encryption\Keymanager::getPrivateKey( $this->userId, $this->view ); + $encryptedPrivateKey = Encryption\Keymanager::getPrivateKey( $this->view, $this->userId ); $decryptedPrivateKey = Encryption\Crypt::symmetricDecryptFileContent( $encryptedPrivateKey, $this->pass ); @@ -303,7 +292,7 @@ class Test_Crypt extends \PHPUnit_Framework_TestCase { // Get private key - $encryptedPrivateKey = Encryption\Keymanager::getPrivateKey( $this->userId, $this->view ); + $encryptedPrivateKey = Encryption\Keymanager::getPrivateKey( $this->view, $this->userId ); $decryptedPrivateKey = Encryption\Crypt::symmetricDecryptFileContent( $encryptedPrivateKey, $this->pass ); diff --git a/apps/files_encryption/tests/keymanager.php b/apps/files_encryption/tests/keymanager.php index c762310dcc5..e31bbe2ab27 100644 --- a/apps/files_encryption/tests/keymanager.php +++ b/apps/files_encryption/tests/keymanager.php @@ -25,10 +25,14 @@ class Test_Keymanager extends \PHPUnit_Framework_TestCase { $this->data = realpath( dirname(__FILE__).'/../lib/crypt.php' ); $this->user = 'admin'; $this->passphrase = 'admin'; + $this->filePath = '/testing'; $this->view = new \OC_FilesystemView( '' ); // Disable encryption proxy to prevent recursive calls \OC_FileProxy::$enabled = false; + + // Notify system which iser is logged in etc. + \OC_User::setUserId( 'admin' ); } @@ -38,17 +42,29 @@ class Test_Keymanager extends \PHPUnit_Framework_TestCase { } - function testGetEncryptedPrivateKey() { + function testGetPrivateKey() { - $key = Encryption\Keymanager::getPrivateKey( $this->user, $this->view ); - - $this->assertEquals( 2302, strlen( $key ) ); + $key = Encryption\Keymanager::getPrivateKey( $this->view, $this->user ); + + + // Will this length vary? Perhaps we should use a range instead + $this->assertEquals( 2296, strlen( $key ) ); } + function testGetPublicKey() { + + $key = Encryption\Keymanager::getPublicKey( $this->view, $this->user ); + + $this->assertEquals( 451, strlen( $key ) ); + + $this->assertEquals( '-----BEGIN PUBLIC KEY-----', substr( $key, 0, 26 ) ); + } + function testSetFileKey() { - # NOTE: This cannot be tested until we are able to break out of the FileSystemView data directory root + # NOTE: This cannot be tested until we are able to break out + # of the FileSystemView data directory root // $key = Crypt::symmetricEncryptFileContentKeyfile( $this->data, 'hat' ); // @@ -62,21 +78,39 @@ class Test_Keymanager extends \PHPUnit_Framework_TestCase { } - function testGetDecryptedPrivateKey() { + function testGetPrivateKey_decrypt() { - $key = Encryption\Keymanager::getPrivateKey( $this->user, $this->view ); + $key = Encryption\Keymanager::getPrivateKey( $this->view, $this->user ); # TODO: replace call to Crypt with a mock object? $decrypted = Encryption\Crypt::symmetricDecryptFileContent( $key, $this->passphrase ); - var_dump($decrypted); - - $this->assertEquals( 1708, strlen( $decrypted ) ); + $this->assertEquals( 1704, strlen( $decrypted ) ); $this->assertEquals( '-----BEGIN PRIVATE KEY-----', substr( $decrypted, 0, 27 ) ); } + function testGetUserKeys() { + + $keys = Encryption\Keymanager::getUserKeys( $this->view, $this->user ); + + $this->assertEquals( 451, strlen( $keys['publicKey'] ) ); + $this->assertEquals( '-----BEGIN PUBLIC KEY-----', substr( $keys['publicKey'], 0, 26 ) ); + $this->assertEquals( 2296, strlen( $keys['privateKey'] ) ); + } + + function testGetPublicKeys() { + + # TODO: write me + + } + + function testGetFileKey() { + +// Encryption\Keymanager::getFileKey( $this->view, $this->user, $this->filePath ); + + } } -- 2.39.5