Added password obfuscation for external storage config

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

3 files changed, 148 insertions, 3 deletions

diff --git a/apps/files_external/lib/config.php b/apps/files_external/lib/config.php
index 2c8828c4d51..05cc88c5d57 100755
--- 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 c5fba92ee68..f3f3b3ed7f3 100644
--- 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 090b5f8e5cf..8c255769fc1 100644
--- a/apps/files_external/tests/mountconfig.php
+++ b/apps/files_external/tests/mountconfig.php
@@ -95,6 +95,14 @@ class Test_Mount_Config extends \PHPUnit_Framework_TestCase {
 	}
 
 	/**
+	 * 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
 	 */
 	public function testAddMountPointValidation() {
@@ -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);
+	}
 }