diff --git a/apps/files_sharing/lib/connector/publicauth.php b/apps/files_sharing/lib/connector/publicauth.php index c9d545180b31..4309c573bde7 100644 --- a/apps/files_sharing/lib/connector/publicauth.php +++ b/apps/files_sharing/lib/connector/publicauth.php @@ -48,13 +48,7 @@ protected function validateUserPass($username, $password) { if (isset($linkItem['share_with'])) { if ($linkItem['share_type'] == \OCP\Share::SHARE_TYPE_LINK) { // Check Password - $forcePortable = (CRYPT_BLOWFISH != 1); - $hasher = new \PasswordHash(8, $forcePortable); - if (!$hasher->CheckPassword($password . $this->config->getSystemValue('passwordsalt', ''), $linkItem['share_with'])) { - return false; - } else { - return true; - } + return password_verify($password, $linkItem['share_with']); } else { return false; } diff --git a/apps/files_sharing/lib/helper.php b/apps/files_sharing/lib/helper.php index e7ca4fcccd4e..7f98a9e64cad 100644 --- a/apps/files_sharing/lib/helper.php +++ b/apps/files_sharing/lib/helper.php @@ -3,7 +3,6 @@ namespace OCA\Files_Sharing; use OC_Config; -use PasswordHash; class Helper { @@ -99,10 +98,7 @@ public static function authenticate($linkItem, $password) { if ($password !== null) { if ($linkItem['share_type'] == \OCP\Share::SHARE_TYPE_LINK) { // Check Password - $forcePortable = (CRYPT_BLOWFISH != 1); - $hasher = new PasswordHash(8, $forcePortable); - if (!($hasher->CheckPassword($password.OC_Config::getValue('passwordsalt', ''), - $linkItem['share_with']))) { + if (!(password_verify($password, $linkItem['share_with']))) { return false; } else { // Save item id in session for future requests diff --git a/apps/files_sharing/public.php b/apps/files_sharing/public.php index bf90c0b5dfcb..169815feefdb 100644 --- a/apps/files_sharing/public.php +++ b/apps/files_sharing/public.php @@ -1,7 +1,5 @@ getAppConfig(); @@ -13,12 +11,8 @@ exit(); } -// Legacy sharing links via public.php have the token in $GET['t'] if (isset($_GET['t'])) { $token = $_GET['t']; -} - -if (isset($token)) { $linkItem = OCP\Share::getShareByToken($token, false); if (is_array($linkItem) && isset($linkItem['uid_owner'])) { // seems to be a valid share @@ -57,10 +51,7 @@ $password = $_POST['password']; if ($linkItem['share_type'] == OCP\Share::SHARE_TYPE_LINK) { // Check Password - $forcePortable = (CRYPT_BLOWFISH != 1); - $hasher = new PasswordHash(8, $forcePortable); - if (!($hasher->CheckPassword($password.OC_Config::getValue('passwordsalt', ''), - $linkItem['share_with']))) { + if (!password_verify($password, $linkItem['share_with'])) { OCP\Util::addStyle('files_sharing', 'authenticate'); $tmpl = new OCP\Template('files_sharing', 'authenticate', 'guest'); $tmpl->assign('URL', $url); @@ -69,11 +60,11 @@ exit(); } else { // Save item id in session for future requests - \OC::$server->getSession()->set('public_link_authenticated', $linkItem['id']); + \OC::$session->set('public_link_authenticated', $linkItem['id']); } } else { - OCP\Util::writeLog('share', 'Unknown share type '.$linkItem['share_type'] - .' for share id '.$linkItem['id'], \OCP\Util::ERROR); + OCP\Util::writeLog('share', 'Unknown share type ' . $linkItem['share_type'] + . ' for share id ' . $linkItem['id'], \OCP\Util::ERROR); header('HTTP/1.0 404 Not Found'); $tmpl = new OCP\Template('', '404', 'guest'); $tmpl->printPage(); @@ -82,8 +73,8 @@ } else { // Check if item id is set in session - if ( ! \OC::$server->getSession()->exists('public_link_authenticated') - || \OC::$server->getSession()->get('public_link_authenticated') !== $linkItem['id'] + if (!\OC::$session->exists('public_link_authenticated') + || \OC::$session->get('public_link_authenticated') !== $linkItem['id'] ) { // Prompt for password OCP\Util::addStyle('files_sharing', 'authenticate'); @@ -95,7 +86,7 @@ } } $basePath = $path; - $rootName = \OC_Util::basename($path); + $rootName = basename($path); if (isset($_GET['path']) && \OC\Files\Filesystem::isReadable($basePath . $_GET['path'])) { $getPath = \OC\Files\Filesystem::normalizePath($_GET['path']); $path .= $getPath; @@ -106,15 +97,12 @@ $file = basename($path); // Download the file if (isset($_GET['download'])) { - if (!\OCP\App::isEnabled('files_encryption')) { - // encryption app requires the session to store the keys in - \OC::$server->getSession()->close(); - } + \OC::$server->getSession()->close(); if (isset($_GET['files'])) { // download selected files $files = urldecode($_GET['files']); $files_list = json_decode($files); // in case we get only a single file - if ($files_list === NULL ) { + if ($files_list === null) { $files_list = array($files); } OC_Files::get($path, $files_list, $_SERVER['REQUEST_METHOD'] == 'HEAD'); @@ -123,6 +111,7 @@ } exit(); } else { + OCP\Util::addScript('files', 'file-upload'); OCP\Util::addStyle('files_sharing', 'public'); OCP\Util::addStyle('files_sharing', 'mobile'); @@ -130,7 +119,7 @@ OCP\Util::addScript('files', 'fileactions'); OCP\Util::addScript('files', 'jquery.iframe-transport'); OCP\Util::addScript('files', 'jquery.fileupload'); - $maxUploadFilesize=OCP\Util::maxUploadFilesize($path); + $maxUploadFilesize = OCP\Util::maxUploadFilesize($path); $tmpl = new OCP\Template('files_sharing', 'public', 'base'); $tmpl->assign('displayName', \OCP\User::getDisplayName($shareOwner)); $tmpl->assign('filename', $file); @@ -138,12 +127,11 @@ $tmpl->assign('mimetype', \OC\Files\Filesystem::getMimeType($path)); $tmpl->assign('dirToken', $linkItem['token']); $tmpl->assign('sharingToken', $token); - $tmpl->assign('server2serversharing', Helper::isOutgoingServer2serverShareEnabled()); $tmpl->assign('protected', isset($linkItem['share_with']) ? 'true' : 'false'); - $urlLinkIdentifiers= (isset($token)?'&t='.$token:'') - .(isset($_GET['dir'])?'&dir='.$_GET['dir']:'') - .(isset($_GET['file'])?'&file='.$_GET['file']:''); + $urlLinkIdentifiers = (isset($token) ? '&t=' . $token : '') + . (isset($_GET['dir']) ? '&dir=' . $_GET['dir'] : '') + . (isset($_GET['file']) ? '&file=' . $_GET['file'] : ''); // Show file list if (\OC\Files\Filesystem::is_dir($path)) { $tmpl->assign('dir', $getPath); @@ -157,10 +145,10 @@ OCP\Util::addscript('files', 'keyboardshortcuts'); $files = array(); $rootLength = strlen($basePath) + 1; - $maxUploadFilesize=OCP\Util::maxUploadFilesize($path); + $maxUploadFilesize = OCP\Util::maxUploadFilesize($path); - $freeSpace=OCP\Util::freeSpace($path); - $uploadLimit=OCP\Util::uploadLimit(); + $freeSpace = OCP\Util::freeSpace($path); + $uploadLimit = OCP\Util::uploadLimit(); $folder = new OCP\Template('files', 'list', ''); $folder->assign('dir', $getPath); $folder->assign('dirToken', $linkItem['token']); @@ -185,7 +173,7 @@ $tmpl->assign('downloadURL', OCP\Util::linkToPublic('files') . $urlLinkIdentifiers . '&download'); } else { $tmpl->assign('downloadURL', OCP\Util::linkToPublic('files') - .$urlLinkIdentifiers.'&download&path='.urlencode($getPath)); + . $urlLinkIdentifiers . '&download&path=' . urlencode($getPath)); } } $tmpl->printPage(); diff --git a/config/config.sample.php b/config/config.sample.php index 71105a8b10d8..ff9ed2e708c3 100755 --- a/config/config.sample.php +++ b/config/config.sample.php @@ -32,6 +32,9 @@ /* Prefix for the ownCloud tables in the database */ "dbtableprefix" => "", +/* Define the cost used to hash the user passwords. */ +"hashingCost" => "", + /* Define the salt used to hash the user passwords. All your user passwords are lost if you lose this string. */ "passwordsalt" => "", diff --git a/lib/private/share/share.php b/lib/private/share/share.php index 8441e6a94c44..11d06a5d0392 100644 --- a/lib/private/share/share.php +++ b/lib/private/share/share.php @@ -612,9 +612,7 @@ public static function shareItem($itemType, $itemSource, $shareType, $shareWith, // Generate hash of password - same method as user passwords if (!empty($shareWith)) { - $forcePortable = (CRYPT_BLOWFISH != 1); - $hasher = new \PasswordHash(8, $forcePortable); - $shareWith = $hasher->HashPassword($shareWith.\OC_Config::getValue('passwordsalt', '')); + $shareWith = password_hash($shareWith, PASSWORD_BCRYPT, array('cost' => OC::$server->getConfig()->getSystemValue('hashingCost'))); } else { // reuse the already set password, but only if we change permissions // otherwise the user disabled the password protection diff --git a/lib/private/user/database.php b/lib/private/user/database.php index 3a76adbe7633..05d04d272755 100644 --- a/lib/private/user/database.php +++ b/lib/private/user/database.php @@ -33,30 +33,16 @@ * */ -require_once 'phpass/PasswordHash.php'; - /** * Class for user management in a SQL Database (e.g. MySQL, SQLite) */ class OC_User_Database extends OC_User_Backend { - /** - * @var PasswordHash - */ - private static $hasher = null; private $cache = array(); - private function getHasher() { - if (!self::$hasher) { - //we don't want to use DES based crypt(), since it doesn't return a hash with a recognisable prefix - $forcePortable = (CRYPT_BLOWFISH != 1); - self::$hasher = new PasswordHash(8, $forcePortable); - } - return self::$hasher; - } - /** * Create a new user + * * @param string $uid The username of the user to create * @param string $password The password of the new user * @return bool @@ -66,8 +52,7 @@ private function getHasher() { */ public function createUser($uid, $password) { if (!$this->userExists($uid)) { - $hasher = $this->getHasher(); - $hash = $hasher->HashPassword($password . OC_Config::getValue('passwordsalt', '')); + $hash = password_hash($password, PASSWORD_BCRYPT, array('cost' => OC::$server->getConfig()->getSystemValue('hashingCost'))); $query = OC_DB::prepare('INSERT INTO `*PREFIX*users` ( `uid`, `password` ) VALUES( ?, ? )'); $result = $query->execute(array($uid, $hash)); @@ -79,6 +64,7 @@ public function createUser($uid, $password) { /** * delete a user + * * @param string $uid The username of the user to delete * @return bool * @@ -98,6 +84,7 @@ public function deleteUser($uid) { /** * Set password + * * @param string $uid The username * @param string $password The new password * @return bool @@ -106,8 +93,7 @@ public function deleteUser($uid) { */ public function setPassword($uid, $password) { if ($this->userExists($uid)) { - $hasher = $this->getHasher(); - $hash = $hasher->HashPassword($password . OC_Config::getValue('passwordsalt', '')); + $hash = password_hash($password, PASSWORD_BCRYPT, array('cost' => OC::$server->getConfig()->getSystemValue('hashingCost'))); $query = OC_DB::prepare('UPDATE `*PREFIX*users` SET `password` = ? WHERE `uid` = ?'); $result = $query->execute(array($hash, $uid)); @@ -119,6 +105,7 @@ public function setPassword($uid, $password) { /** * Set display name + * * @param string $uid The username * @param string $displayName The new display name * @return bool @@ -139,6 +126,7 @@ public function setDisplayName($uid, $displayName) { /** * get display name of the user + * * @param string $uid user ID of the user * @return string display name */ @@ -149,6 +137,7 @@ public function getDisplayName($uid) { /** * Get a list of all display names + * * @return array an array of all displayNames (value) and the correspondig uids (key) * * Get a list of all display names and user ids. @@ -169,6 +158,7 @@ public function getDisplayNames($search = '', $limit = null, $offset = null) { /** * Check if the password is correct + * * @param string $uid The username * @param string $password The password * @return string @@ -183,14 +173,21 @@ public function checkPassword($uid, $password) { $row = $result->fetchRow(); if ($row) { $storedHash = $row['password']; - if ($storedHash[0] === '$') { //the new phpass based hashing - $hasher = $this->getHasher(); - if ($hasher->CheckPassword($password . OC_Config::getValue('passwordsalt', ''), $storedHash)) { - return $row['uid']; - } - //old sha1 based hashing - } elseif (sha1($password) === $storedHash) { + if (password_verify($password, $storedHash)) { + // Check and see if our password needs to be rehashed + if (password_needs_rehash($storedHash, PASSWORD_BCRYPT)) { + $this->setPassword($row['uid'], $password); + } + return $row['uid']; + // Check to see if we are dealing with a legacy password (phpass) + } elseif (password_verify($password . OC::$server->getConfig()->getSystemValue('passwordsalt'), $storedHash)) { + // Check and see if our password needs to be rehashed + if (password_needs_rehash($storedHash, PASSWORD_BCRYPT)) { + $this->setPassword($row['uid'], $password); + } + return $row['uid']; + } elseif (sha1($password) === $storedHash) { //old sha1 based hashing //upgrade to new hashing $this->setPassword($row['uid'], $password); return $row['uid']; @@ -202,6 +199,7 @@ public function checkPassword($uid, $password) { /** * Load an user in the cache + * * @param string $uid the username * @return boolean */ @@ -226,6 +224,7 @@ private function loadUser($uid) { /** * Get a list of all users + * * @return array an array of all uids * * Get a list of all users. @@ -242,6 +241,7 @@ public function getUsers($search = '', $limit = null, $offset = null) { /** * check if a user exists + * * @param string $uid the username * @return boolean */ @@ -252,6 +252,7 @@ public function userExists($uid) { /** * get the user's home directory + * * @param string $uid the username * @return string|false */ diff --git a/tests/lib/user.php b/tests/lib/user.php index e2c3282a19f4..7f653c7691d7 100644 --- a/tests/lib/user.php +++ b/tests/lib/user.php @@ -9,25 +9,52 @@ namespace Test; + class User extends \PHPUnit_Framework_TestCase { /** * @var \OC_User_Backend | \PHPUnit_Framework_MockObject_MockObject $backend */ private $backend; - - protected function setUp(){ + + protected function setUp() { $this->backend = $this->getMock('\OC_User_Dummy'); $manager = \OC_User::getManager(); $manager->registerBackend($this->backend); } - + public function testCheckPassword() { + /** + * Verify that password_compat can accept legacy phpass hashes + * key = plaintext password + * val = hash from phpass + */ + $passes = array( + 'r1TLAYYPIhzqpRH' => '$2a$08$MK8CNRbaqlcjbQVchklG1eHmqsm2Qge00bUrq5SMo7Swn11AVO4o2', + 'jio0FuK1c2YIpc' => '$2a$08$jUIoz.AYeiJ5k4cOGKyPb.VIt4wnTeaNY460Jc7.FyyCz/nZjHuHi', + 'yRHhiflGpDWOa' => '$2a$08$huQVCsKeTPquzYNfyvDQR.jp9d6tNGcSYwTmV1InpkOTQXoT0qigO', + 'bqwyBEXVmyHz' => '$2a$08$sLrFBq86qlOU28bL.XuBgewwBVGR7wUO0QR7qEaCFAqnj/atIW6FO', + '6SiKUhEeHiR' => '$2a$08$XHvJdJC7d4Q11QIYR7Gl4OC.FNkJOewhitRFDEVoYaerCv17EqA/e', + 'pjN4ijSYtl' => '$2a$08$9.C/6byxWNBFAEEKnDlzOu//wYhrP/hJ0uZH2DQDhtYEhX.nwfYoW', + 'wJGnOLJGC' => '$2a$08$nci16BoJcX71Q6Yiievk4uEN8mUcaLzQx8tCUiiW4eZ1R8x1DJQM6', + 'ALqkm3bv' => '$2a$08$vhaV5SXQT9pkN14xa2H5xu2rs1fYQQJTO1ZlUYR5a/Na8URnfbzVu', + 'LGZNEEN' => '$2a$08$YLxcohzMXo3oxp9WFCn3PevRrozP1kiH3u3ZPAImLtcTnxREA5HfS', + 'EMu3Tu' => '$2a$08$qMhZKB2xOQq3xY/chWiGj.6NcDUEpig9wOhA2zcQI.K7rAWzOJzum' + ); + + // Statically set salt as it would differ between each installation and make this untestable + $salt = 'b710fa552160d82221b5871ce73979'; + + + foreach ($passes as $key => $val) { + $this->assertTrue(password_verify($key . $salt, $val)); + } + + $this->backend->expects($this->once()) ->method('checkPassword') ->with($this->equalTo('foo'), $this->equalTo('bar')) - ->will($this->returnValue('foo')) - ; + ->will($this->returnValue('foo')); $this->backend->expects($this->any()) ->method('implementsActions') @@ -42,18 +69,18 @@ public function testCheckPassword() { $uid = \OC_User::checkPassword('foo', 'bar'); $this->assertEquals($uid, 'foo'); } - + public function testDeleteUser() { $fail = \OC_User::deleteUser('victim'); $this->assertFalse($fail); - + $success = \OC_User::createUser('victim', 'password'); - + $success = \OC_User::deleteUser('victim'); $this->assertTrue($success); } - - public function testCreateUser(){ + + public function testCreateUser() { $this->backend->expects($this->any()) ->method('implementsActions') ->will($this->returnCallback(function ($actions) { @@ -63,7 +90,7 @@ public function testCreateUser(){ return false; } })); - + $user = \OC_User::createUser('newuser', 'newpassword'); $this->assertEquals('newuser', $user->getUid()); }