Added password obfuscation for external storage config

Added obfuscation for all "password" options from external storages.
Added unit tests for reading/writing the configuration.

diff --git a/apps/files_external/lib/config.php b/apps/files_external/lib/config.php
index 2c8828c4d5111f8e4868810f02e89ef46f92fafe..05cc88c5d579e8e92d8de136e40201272fcd931b 100755 (executable)
--- a/apps/files_external/lib/config.php
+++ b/apps/files_external/lib/config.php
@@ -4,6 +4,7 @@
 *
 * @author Michael Gapczynski
 * @copyright 2012 Michael Gapczynski mtgap@owncloud.com
+* @copyright 2014 Vincent Petry <pvince81@owncloud.com>
 *
 * This library is free software; you can redistribute it and/or
 * modify it under the terms of the GNU AFFERO GENERAL PUBLIC LICENSE
@@ -19,9 +20,16 @@
 * License along with this library.  If not, see <http://www.gnu.org/licenses/>.
 */
 
+set_include_path(
+       get_include_path() . PATH_SEPARATOR .
+       \OC_App::getAppPath('files_external') . '/3rdparty/phpseclib/phpseclib'
+);
+include('Crypt/AES.php');
+
 /**
 * Class to configure the config/mount.php and data/$user/mount.php files
-*/
+ */
+// TODO: make this class non-static
 class OC_Mount_Config {
 
        const MOUNT_TYPE_GLOBAL = 'global';
@@ -31,6 +39,9 @@ class OC_Mount_Config {
        // whether to skip backend test (for unit tests, as this static class is not mockable)
        public static $skipTest = false;
 
+       // password encryption cipher
+       private static $cipher;
+
        /**
        * Get details on each of the external storage backends, used for the mount config UI
        * If a custom UI is needed, add the key 'custom' and a javascript file with that name will be loaded
@@ -203,6 +214,7 @@ class OC_Mount_Config {
                                        if (strpos($mount['class'], 'OC_Filestorage_') !== false) {
                                                $mount['class'] = '\OC\Files\Storage\\'.substr($mount['class'], 15);
                                        }
+                                       $mount['options'] = self::decryptPasswords($mount['options']);
                                        // Remove '/$user/files/' from mount point
                                        $mountPoint = substr($mountPoint, 13);
                                        // Merge the mount point into the current mount points
@@ -228,6 +240,7 @@ class OC_Mount_Config {
                                        if (strpos($mount['class'], 'OC_Filestorage_') !== false) {
                                                $mount['class'] = '\OC\Files\Storage\\'.substr($mount['class'], 15);
                                        }
+                                       $mount['options'] = self::decryptPasswords($mount['options']);
                                        // Remove '/$user/files/' from mount point
                                        $mountPoint = substr($mountPoint, 13);
                                        // Merge the mount point into the current mount points
@@ -265,6 +278,7 @@ class OC_Mount_Config {
                                if (strpos($mount['class'], 'OC_Filestorage_') !== false) {
                                        $mount['class'] = '\OC\Files\Storage\\'.substr($mount['class'], 15);
                                }
+                               $mount['options'] = self::decryptPasswords($mount['options']);
                                // Remove '/uid/files/' from mount point
                                $personal[substr($mountPoint, strlen($uid) + 8)] = array(
                                        'class' => $mount['class'],
@@ -334,7 +348,13 @@ class OC_Mount_Config {
                } else {
                        $mountPoint = '/$user/files/'.ltrim($mountPoint, '/');
                }
-               $mount = array($applicable => array($mountPoint => array('class' => $class, 'options' => $classOptions)));
+
+               $mount = array($applicable => array(
+                       $mountPoint => array(
+                               'class' => $class,
+                               'options' => self::encryptPasswords($classOptions))
+                       )
+               );
                $mountPoints = self::readData($isPersonal);
                // Merge the new mount point into the current mount points
                if (isset($mountPoints[$mountType])) {
@@ -527,4 +547,42 @@ class OC_Mount_Config {
 
                return $txt;
        }
+
+       /**
+        * Encrypt passwords in the given config options
+        * @param array $options mount options
+        * @return array updated options
+        */
+       private static function encryptPasswords($options) {
+               if (isset($options['password'])) {
+                       $options['password_encrypted'] = base64_encode(self::getCipher()->encrypt($options['password']));
+                       unset($options['password']);
+               }
+               return $options;
+       }
+
+       /**
+        * Decrypt passwords in the given config options
+        * @param array $options mount options
+        * @return array updated options
+        */
+       private static function decryptPasswords($options) {
+               // note: legacy options might still have the unencrypted password in the "password" field
+               if (isset($options['password_encrypted'])) {
+                       $options['password'] = self::getCipher()->decrypt(base64_decode($options['password_encrypted']));
+                       unset($options['password_encrypted']);
+               }
+               return $options;
+       }
+
+       /**
+        * Returns the encryption cipher
+        */
+       private static function getCipher() {
+               if (!isset(self::$cipher)) {
+                       self::$cipher = new Crypt_AES(CRYPT_AES_MODE_CBC);
+                       self::$cipher->setKey(\OCP\Config::getSystemValue('passwordsalt'));
+               }
+               return self::$cipher;
+       }
 }

diff --git a/apps/files_external/lib/smb.php b/apps/files_external/lib/smb.php
index c5fba92ee68217b0a83e4a253dc9403d6bf95af6..f3f3b3ed7f39b55e8b9fc30d628ab1c64747f1ce 100644 (file)
--- a/apps/files_external/lib/smb.php
+++ b/apps/files_external/lib/smb.php
@@ -37,7 +37,7 @@ class SMB extends \OC\Files\Storage\StreamWrapper{
                                $this->share = substr($this->share, 0, -1);
                        }
                } else {
-                       throw new \Exception();
+                       throw new \Exception('Invalid configuration');
                }
        }
 

diff --git a/apps/files_external/tests/mountconfig.php b/apps/files_external/tests/mountconfig.php
index 090b5f8e5cf748aa4ed17fab2e0399419e4ec79b..8c255769fc1d72e2af9599232621b179cad9e797 100644 (file)
--- a/apps/files_external/tests/mountconfig.php
+++ b/apps/files_external/tests/mountconfig.php
@@ -94,6 +94,14 @@ class Test_Mount_Config extends \PHPUnit_Framework_TestCase {
                return json_decode(file_get_contents($configFile), true);
        }
 
+       /**
+        * Write the user config, to simulate existing files
+        */
+       private function writeUserConfig($config) {
+               $configFile = $this->userHome . '/mount.json';
+               file_put_contents($configFile, json_encode($config));
+       }
+
        /**
         * Test mount point validation
         */
@@ -258,4 +266,83 @@ class Test_Mount_Config extends \PHPUnit_Framework_TestCase {
                $savedMountConfig = $config['ext']['configuration'];
                $this->assertEquals($mountConfig, $savedMountConfig);
        }
+
+       /**
+        * Test password obfuscation
+        */
+       public function testPasswordObfuscation() {
+               $mountType = OC_Mount_Config::MOUNT_TYPE_USER;
+               $applicable = 'test';
+               $isPersonal = true;
+               $mountConfig = array(
+                       'host' => 'smbhost',
+                       'user' => 'smbuser',
+                       'password' => 'smbpassword',
+                       'share' => 'smbshare',
+                       'root' => 'smbroot'
+               );
+
+               // write config
+               $this->assertTrue(
+                       OC_Mount_Config::addMountPoint(
+                               '/ext',
+                               '\OC\Files\Storage\SMB',
+                               $mountConfig,
+                               $mountType,
+                               $applicable,
+                               $isPersonal
+                       )
+               );
+
+               // note: password re-reading is covered by testReadWritePersonalConfig
+
+               // check that password inside the file is NOT in plain text
+               $config = $this->readUserConfig();
+               $savedConfig = $config[$mountType][$applicable]['/test/files/ext']['options'];
+
+               // no more clear text password in file
+               $this->assertFalse(isset($savedConfig['password']));
+
+               // encrypted password is present
+               $this->assertNotEquals($mountConfig['password'], $savedConfig['password_encrypted']);
+       }
+
+       /**
+        * Test read legacy passwords
+        */
+       public function testReadLegacyPassword() {
+               $mountType = OC_Mount_Config::MOUNT_TYPE_USER;
+               $applicable = 'test';
+               $isPersonal = true;
+               $mountConfig = array(
+                       'host' => 'smbhost',
+                       'user' => 'smbuser',
+                       'password' => 'smbpassword',
+                       'share' => 'smbshare',
+                       'root' => 'smbroot'
+               );
+
+               // write config
+               $this->assertTrue(
+                       OC_Mount_Config::addMountPoint(
+                               '/ext',
+                               '\OC\Files\Storage\SMB',
+                               $mountConfig,
+                               $mountType,
+                               $applicable,
+                               $isPersonal
+                       )
+               );
+
+               $config = $this->readUserConfig();
+               // simulate non-encrypted password situation
+               $config[$mountType][$applicable]['/test/files/ext']['options']['password'] = 'smbpasswd';
+
+               $this->writeUserConfig($config);
+
+               // re-read config, password was read correctly
+               $config = OC_Mount_Config::getPersonalMountPoints();
+               $savedMountConfig = $config['ext']['configuration'];
+               $this->assertEquals($mountConfig, $savedMountConfig);
+       }
 }