From 64f4f8fc84fd8fc27f0e9e316a2c4c2500c7134f Mon Sep 17 00:00:00 2001 From: Ross Nicoll Date: Fri, 19 Dec 2014 17:23:24 +0000 Subject: [PATCH] Add support for SFTP key authentication Add support for external files accessed via SFTP using public key exchange authentication. Keys are generated automatically when the configuration is added, or can be regenerated on demand if a key is compromised. Creation of a new configuration row now triggers focus on that row. This is used to trigger auto-configuration for SFTP keys. Generated public keys are saved in user's data directory for easy retrieval by an external application. Add controller for SFTP key generation AJAX SFTP class initialisation no longer produces a warning if the password field is missing. Add unit tests for SFTP with key authentication backend --- apps/files_external/appinfo/app.php | 13 ++ apps/files_external/appinfo/application.php | 33 +++ apps/files_external/appinfo/routes.php | 20 +- .../controller/ajaxcontroller.php | 48 +++++ apps/files_external/js/sftp_key.js | 53 +++++ apps/files_external/l10n/en_GB.json | 2 +- apps/files_external/lib/sftp.php | 21 +- apps/files_external/lib/sftp_key.php | 194 ++++++++++++++++++ .../tests/backends/sftp_key.php | 85 ++++++++ apps/files_external/tests/config.php | 10 +- 10 files changed, 472 insertions(+), 7 deletions(-) create mode 100644 apps/files_external/appinfo/application.php create mode 100644 apps/files_external/controller/ajaxcontroller.php create mode 100644 apps/files_external/js/sftp_key.js create mode 100644 apps/files_external/lib/sftp_key.php create mode 100644 apps/files_external/tests/backends/sftp_key.php diff --git a/apps/files_external/appinfo/app.php b/apps/files_external/appinfo/app.php index 0aafcad559..74f0e337a1 100644 --- a/apps/files_external/appinfo/app.php +++ b/apps/files_external/appinfo/app.php @@ -18,6 +18,7 @@ OC::$CLASSPATH['OC\Files\Storage\SMB_OC'] = 'files_external/lib/smb_oc.php'; OC::$CLASSPATH['OC\Files\Storage\AmazonS3'] = 'files_external/lib/amazons3.php'; OC::$CLASSPATH['OC\Files\Storage\Dropbox'] = 'files_external/lib/dropbox.php'; OC::$CLASSPATH['OC\Files\Storage\SFTP'] = 'files_external/lib/sftp.php'; +OC::$CLASSPATH['OC\Files\Storage\SFTP_Key'] = 'files_external/lib/sftp_key.php'; OC::$CLASSPATH['OC_Mount_Config'] = 'files_external/lib/config.php'; OC::$CLASSPATH['OCA\Files\External\Api'] = 'files_external/lib/api.php'; @@ -177,5 +178,17 @@ OC_Mount_Config::registerBackend('\OC\Files\Storage\SFTP', array( 'password' => '*'.$l->t('Password'), 'root' => '&'.$l->t('Remote subfolder')))); +OC_Mount_Config::registerBackend('\OC\Files\Storage\SFTP_Key', [ + 'backend' => 'SFTP with secret key login', + 'priority' => 100, + 'configuration' => array( + 'host' => (string)$l->t('Host'), + 'user' => (string)$l->t('Username'), + 'public_key' => (string)$l->t('Public key'), + 'private_key' => '#private_key', + 'root' => '&'.$l->t('Remote subfolder')), + 'custom' => 'sftp_key', + ] +); $mountProvider = new \OCA\Files_External\Config\ConfigAdapter(); \OC::$server->getMountProviderCollection()->registerProvider($mountProvider); diff --git a/apps/files_external/appinfo/application.php b/apps/files_external/appinfo/application.php new file mode 100644 index 0000000000..b1605bb98a --- /dev/null +++ b/apps/files_external/appinfo/application.php @@ -0,0 +1,33 @@ + + * This file is licensed under the Affero General Public License version 3 or + * later. + * See the COPYING-README file. + */ + +namespace OCA\Files_External\Appinfo; + +use \OCA\Files_External\Controller\AjaxController; +use \OCP\AppFramework\App; +use \OCP\IContainer; + + /** + * @package OCA\Files_External\Appinfo + */ +class Application extends App { + public function __construct(array $urlParams=array()) { + parent::__construct('files_external', $urlParams); + $container = $this->getContainer(); + + /** + * Controllers + */ + $container->registerService('AjaxController', function (IContainer $c) { + return new AjaxController( + $c->query('AppName'), + $c->query('Request') + ); + }); + } +} diff --git a/apps/files_external/appinfo/routes.php b/apps/files_external/appinfo/routes.php index b852b34c5d..5c7c4eca90 100644 --- a/apps/files_external/appinfo/routes.php +++ b/apps/files_external/appinfo/routes.php @@ -20,6 +20,23 @@ * */ +namespace OCA\Files_External\Appinfo; + +$application = new Application(); +$application->registerRoutes( + $this, + array( + 'routes' => array( + array( + 'name' => 'Ajax#getSshKeys', + 'url' => '/ajax/sftp_key.php', + 'verb' => 'POST', + 'requirements' => array() + ) + ) + ) +); + /** @var $this OC\Route\Router */ $this->create('files_external_add_mountpoint', 'ajax/addMountPoint.php') @@ -37,10 +54,11 @@ $this->create('files_external_dropbox', 'ajax/dropbox.php') $this->create('files_external_google', 'ajax/google.php') ->actionInclude('files_external/ajax/google.php'); + $this->create('files_external_list_applicable', '/applicable') ->actionInclude('files_external/ajax/applicable.php'); -OC_API::register('get', +\OC_API::register('get', '/apps/files_external/api/v1/mounts', array('\OCA\Files\External\Api', 'getUserMounts'), 'files_external'); diff --git a/apps/files_external/controller/ajaxcontroller.php b/apps/files_external/controller/ajaxcontroller.php new file mode 100644 index 0000000000..141fc7817d --- /dev/null +++ b/apps/files_external/controller/ajaxcontroller.php @@ -0,0 +1,48 @@ + + * This file is licensed under the Affero General Public License version 3 or + * later. + * See the COPYING-README file. + */ + +namespace OCA\Files_External\Controller; + +use OCP\AppFramework\Controller; +use OCP\IRequest; +use OCP\AppFramework\Http\JSONResponse; + +class AjaxController extends Controller { + public function __construct($appName, IRequest $request) { + parent::__construct($appName, $request); + } + + private function generateSshKeys() { + $rsa = new \Crypt_RSA(); + $rsa->setPublicKeyFormat(CRYPT_RSA_PUBLIC_FORMAT_OPENSSH); + $rsa->setPassword(\OC::$server->getConfig()->getSystemValue('secret', '')); + + $key = $rsa->createKey(); + // Replace the placeholder label with a more meaningful one + $key['publicKey'] = str_replace('phpseclib-generated-key', gethostname(), $key['publickey']); + + return $key; + } + + /** + * Generates an SSH public/private key pair. + * + * @NoAdminRequired + */ + public function getSshKeys() { + $key = $this->generateSshKeys(); + return new JSONResponse( + array('data' => array( + 'private_key' => $key['privatekey'], + 'public_key' => $key['publickey'] + ), + 'status' => 'success' + )); + } + +} diff --git a/apps/files_external/js/sftp_key.js b/apps/files_external/js/sftp_key.js new file mode 100644 index 0000000000..2b39628247 --- /dev/null +++ b/apps/files_external/js/sftp_key.js @@ -0,0 +1,53 @@ +$(document).ready(function() { + + $('#externalStorage tbody tr.\\\\OC\\\\Files\\\\Storage\\\\SFTP_Key').each(function() { + var tr = $(this); + var config = $(tr).find('.configuration'); + if ($(config).find('.sftp_key').length === 0) { + setupTableRow(tr, config); + } + }); + + // We can't catch the DOM elements being added, but we can pick up when + // they receive focus + $('#externalStorage').on('focus', 'tbody tr.\\\\OC\\\\Files\\\\Storage\\\\SFTP_Key', function() { + var tr = $(this); + var config = $(tr).find('.configuration'); + + if ($(config).find('.sftp_key').length === 0) { + setupTableRow(tr, config); + } + }); + + $('#externalStorage').on('click', '.sftp_key', function(event) { + event.preventDefault(); + var tr = $(this).parent().parent(); + generateKeys(tr); + }); + + function setupTableRow(tr, config) { + $(config).append($(document.createElement('input')).addClass('button sftp_key') + .attr('type', 'button') + .attr('value', t('files_external', 'Generate keys'))); + // If there's no private key, build one + if (0 === $(config).find('[data-parameter="private_key"]').val().length) { + generateKeys(tr); + } + } + + function generateKeys(tr) { + var config = $(tr).find('.configuration'); + + $.post(OC.filePath('files_external', 'ajax', 'sftp_key.php'), {}, function(result) { + if (result && result.status === 'success') { + $(config).find('[data-parameter="public_key"]').val(result.data.public_key); + $(config).find('[data-parameter="private_key"]').val(result.data.private_key); + OC.MountConfig.saveStorage(tr, function() { + // Nothing to do + }); + } else { + OC.dialogs.alert(result.data.message, t('files_external', 'Error generating key pair') ); + } + }); + } +}); diff --git a/apps/files_external/l10n/en_GB.json b/apps/files_external/l10n/en_GB.json index 21a5881c94..4b4da2f2cb 100644 --- a/apps/files_external/l10n/en_GB.json +++ b/apps/files_external/l10n/en_GB.json @@ -69,4 +69,4 @@ "Enable User External Storage" : "Enable User External Storage", "Allow users to mount the following external storage" : "Allow users to mount the following external storage" },"pluralForm" :"nplurals=2; plural=(n != 1);" -} \ No newline at end of file +} diff --git a/apps/files_external/lib/sftp.php b/apps/files_external/lib/sftp.php index f6c5666973..2a762ad068 100644 --- a/apps/files_external/lib/sftp.php +++ b/apps/files_external/lib/sftp.php @@ -20,7 +20,7 @@ class SFTP extends \OC\Files\Storage\Common { /** * @var \Net_SFTP */ - private $client; + protected $client; private static $tempFiles = array(); @@ -42,7 +42,8 @@ class SFTP extends \OC\Files\Storage\Common { $this->host = substr($this->host, $proto+3); } $this->user = $params['user']; - $this->password = $params['password']; + $this->password + = isset($params['password']) ? $params['password'] : ''; $this->root = isset($params['root']) ? $this->cleanPath($params['root']) : '/'; @@ -101,6 +102,18 @@ class SFTP extends \OC\Files\Storage\Common { return 'sftp::' . $this->user . '@' . $this->host . '/' . $this->root; } + public function getHost() { + return $this->host; + } + + public function getRoot() { + return $this->root; + } + + public function getUser() { + return $this->user; + } + /** * @param string $path */ @@ -121,7 +134,7 @@ class SFTP extends \OC\Files\Storage\Common { return false; } - private function writeHostKeys($keys) { + protected function writeHostKeys($keys) { try { $keyPath = $this->hostKeysPath(); if ($keyPath && file_exists($keyPath)) { @@ -137,7 +150,7 @@ class SFTP extends \OC\Files\Storage\Common { return false; } - private function readHostKeys() { + protected function readHostKeys() { try { $keyPath = $this->hostKeysPath(); if (file_exists($keyPath)) { diff --git a/apps/files_external/lib/sftp_key.php b/apps/files_external/lib/sftp_key.php new file mode 100644 index 0000000000..6113f88a8f --- /dev/null +++ b/apps/files_external/lib/sftp_key.php @@ -0,0 +1,194 @@ + + * This file is licensed under the Affero General Public License version 3 or + * later. + * See the COPYING-README file. + */ +namespace OC\Files\Storage; + +/** +* Uses phpseclib's Net_SFTP class and the Net_SFTP_Stream stream wrapper to +* provide access to SFTP servers. +*/ +class SFTP_Key extends \OC\Files\Storage\SFTP { + private $publicKey; + private $privateKey; + + public function __construct($params) { + parent::__construct($params); + $this->publicKey = $params['public_key']; + $this->privateKey = $params['private_key']; + } + + /** + * Returns the connection. + * + * @return \Net_SFTP connected client instance + * @throws \Exception when the connection failed + */ + public function getConnection() { + if (!is_null($this->client)) { + return $this->client; + } + + $hostKeys = $this->readHostKeys(); + $this->client = new \Net_SFTP($this->getHost()); + + // The SSH Host Key MUST be verified before login(). + $currentHostKey = $this->client->getServerPublicHostKey(); + if (array_key_exists($this->getHost(), $hostKeys)) { + if ($hostKeys[$this->getHost()] !== $currentHostKey) { + throw new \Exception('Host public key does not match known key'); + } + } else { + $hostKeys[$this->getHost()] = $currentHostKey; + $this->writeHostKeys($hostKeys); + } + + $key = $this->getPrivateKey(); + if (is_null($key)) { + throw new \Exception('Secret key could not be loaded'); + } + if (!$this->client->login($this->getUser(), $key)) { + throw new \Exception('Login failed'); + } + return $this->client; + } + + /** + * Returns the private key to be used for authentication to the remote server. + * + * @return \Crypt_RSA instance or null in case of a failure to load the key. + */ + private function getPrivateKey() { + $key = new \Crypt_RSA(); + $key->setPassword(\OC::$server->getConfig()->getSystemValue('secret', '')); + if (!$key->loadKey($this->privateKey)) { + // Should this exception rather than return null? + return null; + } + return $key; + } + + /** + * Throws an exception if the provided host name/address is invalid (cannot be resolved + * and is not an IPv4 address). + * + * @return true; never returns in case of a problem, this return value is used just to + * make unit tests happy. + */ + public function assertHostAddressValid($hostname) { + // TODO: Should handle IPv6 addresses too + if (!preg_match('/^\d+\.\d+\.\d+\.\d+$/', $hostname) && gethostbyname($hostname) === $hostname) { + // Hostname is not an IPv4 address and cannot be resolved via DNS + throw new \InvalidArgumentException('Cannot resolve hostname.'); + } + return true; + } + + /** + * Throws an exception if the provided port number is invalid (cannot be resolved + * and is not an IPv4 address). + * + * @return true; never returns in case of a problem, this return value is used just to + * make unit tests happy. + */ + public function assertPortNumberValid($port) { + if (!preg_match('/^\d+$/', $port)) { + throw new \InvalidArgumentException('Port number must be a number.'); + } + if ($port < 0 || $port > 65535) { + throw new \InvalidArgumentException('Port number must be between 0 and 65535 inclusive.'); + } + return true; + } + + /** + * Replaces anything that's not an alphanumeric character or "." in a hostname + * with "_", to make it safe for use as part of a file name. + */ + protected function sanitizeHostName($name) { + return preg_replace('/[^\d\w\._]/', '_', $name); + } + + /** + * Replaces anything that's not an alphanumeric character or "_" in a username + * with "_", to make it safe for use as part of a file name. + */ + protected function sanitizeUserName($name) { + return preg_replace('/[^\d\w_]/', '_', $name); + } + + public function test() { + if (empty($this->getHost())) { + \OC::$server->getLogger()->warning('Hostname has not been specified'); + return false; + } + if (empty($this->getUser())) { + \OC::$server->getLogger()->warning('Username has not been specified'); + return false; + } + if (!isset($this->privateKey)) { + \OC::$server->getLogger()->warning('Private key was missing from the request'); + return false; + } + + // Sanity check the host + $hostParts = explode(':', $this->getHost()); + try { + if (count($hostParts) == 1) { + $hostname = $hostParts[0]; + $this->assertHostAddressValid($hostname); + } else if (count($hostParts) == 2) { + $hostname = $hostParts[0]; + $this->assertHostAddressValid($hostname); + $this->assertPortNumberValid($hostParts[1]); + } else { + throw new \Exception('Host connection string is invalid.'); + } + } catch(\Exception $e) { + \OC::$server->getLogger()->warning($e->getMessage()); + return false; + } + + // Validate the key + $key = $this->getPrivateKey(); + if (is_null($key)) { + \OC::$server->getLogger()->warning('Secret key could not be loaded'); + return false; + } + + try { + if ($this->getConnection()->nlist() === false) { + return false; + } + } catch(\Exception $e) { + // We should be throwing a more specific error, so we're not just catching + // Exception here + \OC::$server->getLogger()->warning($e->getMessage()); + return false; + } + + // Save the key somewhere it can easily be extracted later + if (\OC::$server->getUserSession()->getUser()) { + $view = new \OC\Files\View('/'.\OC::$server->getUserSession()->getUser()->getUId().'/files_external/sftp_keys'); + if (!$view->is_dir('')) { + if (!$view->mkdir('')) { + \OC::$server->getLogger()->warning('Could not create secret key directory.'); + return false; + } + } + $key_filename = $this->sanitizeUserName($this->getUser()).'@'.$this->sanitizeHostName($hostname).'.pub'; + $key_file = $view->fopen($key_filename, "w"); + if ($key_file) { + fwrite($key_file, $this->publicKey); + fclose($key_file); + } else { + \OC::$server->getLogger()->warning('Could not write secret key file.'); + } + } + + return true; + } +} diff --git a/apps/files_external/tests/backends/sftp_key.php b/apps/files_external/tests/backends/sftp_key.php new file mode 100644 index 0000000000..4e55cc37ca --- /dev/null +++ b/apps/files_external/tests/backends/sftp_key.php @@ -0,0 +1,85 @@ +. + */ + +namespace Test\Files\Storage; + +class SFTP_Key extends Storage { + private $config; + + protected function setUp() { + parent::setUp(); + + $id = $this->getUniqueID(); + $this->config = include('files_external/tests/config.php'); + if ( ! is_array($this->config) or ! isset($this->config['sftp_key']) or ! $this->config['sftp_key']['run']) { + $this->markTestSkipped('SFTP with key backend not configured'); + } + $this->config['sftp_key']['root'] .= '/' . $id; //make sure we have an new empty folder to work in + $this->instance = new \OC\Files\Storage\SFTP_Key($this->config['sftp_key']); + $this->instance->mkdir('/'); + } + + protected function tearDown() { + if ($this->instance) { + $this->instance->rmdir('/'); + } + + parent::tearDown(); + } + + /** + * @expectedException InvalidArgumentException + */ + public function testInvalidAddressShouldThrowException() { + # I'd use example.com for this, but someone decided to break the spec and make it resolve + $this->instance->assertHostAddressValid('notarealaddress...'); + } + + public function testValidAddressShouldPass() { + $this->assertTrue($this->instance->assertHostAddressValid('localhost')); + } + + /** + * @expectedException InvalidArgumentException + */ + public function testNegativePortNumberShouldThrowException() { + $this->instance->assertPortNumberValid('-1'); + } + + /** + * @expectedException InvalidArgumentException + */ + public function testNonNumericalPortNumberShouldThrowException() { + $this->instance->assertPortNumberValid('a'); + } + + /** + * @expectedException InvalidArgumentException + */ + public function testHighPortNumberShouldThrowException() { + $this->instance->assertPortNumberValid('65536'); + } + + public function testValidPortNumberShouldPass() { + $this->assertTrue($this->instance->assertPortNumberValid('22222')); + } +} diff --git a/apps/files_external/tests/config.php b/apps/files_external/tests/config.php index 62aff4d1bc..cf9cdfeead 100644 --- a/apps/files_external/tests/config.php +++ b/apps/files_external/tests/config.php @@ -88,5 +88,13 @@ return array( 'user'=>'test', 'password'=>'test', 'root'=>'/test' - ) + ), + 'sftp_key' => array ( + 'run'=>false, + 'host'=>'localhost', + 'user'=>'test', + 'public_key'=>'ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAAAgQDJPTvz3OLonF2KSGEKP/nd4CPmRYvemG2T4rIiNYjDj0U5y+2sKEWbjiUlQl2bsqYuVoJ+/UNJlGQbbZ08kQirFeo1GoWBzqioaTjUJfbLN6TzVVKXxR9YIVmH7Ajg2iEeGCndGgbmnPfj+kF9TR9IH8vMVvtubQwf7uEwB0ALhw== phpseclib-generated-key', + 'private_key'=>'test', + 'root'=>'/test' + ), );