From 6359259dbb1e8d5b569f569a7089abffd9259d30 Mon Sep 17 00:00:00 2001 From: Andrew Dolgov Date: Mon, 1 Mar 2021 15:24:18 +0300 Subject: [PATCH] simplify internal authentication code and bump default algo to SSHA-512 --- api/index.php | 2 +- classes/auth/base.php | 4 +- classes/pref/prefs.php | 24 +----- classes/pref/users.php | 22 ++--- classes/rpc.php | 2 +- classes/userhelper.php | 65 +++++++++++++- include/functions.php | 10 --- js/PrefUsers.js | 27 +++--- js/common.js | 15 +++- plugins/auth_internal/init.php | 150 +++++++++++---------------------- 10 files changed, 152 insertions(+), 169 deletions(-) diff --git a/api/index.php b/api/index.php index 430082f16..750a95721 100644 --- a/api/index.php +++ b/api/index.php @@ -46,7 +46,7 @@ UserHelper::load_user_plugins($_SESSION["uid"]); } - $method = strtolower($_REQUEST["op"]); + $method = strtolower($_REQUEST["op"] ?? ""); $handler = new API($_REQUEST); diff --git a/classes/auth/base.php b/classes/auth/base.php index f18cc2d2d..9b2f630c0 100644 --- a/classes/auth/base.php +++ b/classes/auth/base.php @@ -23,8 +23,8 @@ abstract class Auth_Base extends Plugin implements IAuthModule { if (!$password) $password = make_password(); - $salt = substr(bin2hex(get_random_bytes(125)), 0, 250); - $pwd_hash = encrypt_password($password, $salt, true); + $salt = UserHelper::get_salt(); + $pwd_hash = UserHelper::hash_password($password, $salt, UserHelper::HASH_ALGOS[0]); $sth = $this->pdo->prepare("INSERT INTO ttrss_users (login,access_level,last_login,created,pwd_hash,salt) diff --git a/classes/pref/prefs.php b/classes/pref/prefs.php index 5fe4f1bbf..f61f0f038 100644 --- a/classes/pref/prefs.php +++ b/classes/pref/prefs.php @@ -1038,19 +1038,6 @@ class Pref_Prefs extends Handler_Protected { } } - static function _is_default_password() { - $authenticator = PluginHost::getInstance()->get_plugin($_SESSION["auth_module"]); - - if ($authenticator && - method_exists($authenticator, "check_password") && - $authenticator->check_password($_SESSION["uid"], "password")) { - - return true; - } - - return false; - } - function otpdisable() { $password = clean($_REQUEST["password"]); @@ -1404,12 +1391,6 @@ class Pref_Prefs extends Handler_Protected { _encrypt_app_password($new_password); + $new_salt = UserHelper::get_salt(); + $new_password_hash = UserHelper::hash_password($new_password, $new_salt, UserHelper::HASH_ALGOS[0]); print_warning(T_sprintf("Generated password %s for %s. Please remember it for future reference.", $new_password, $title)); @@ -1432,7 +1414,7 @@ class Pref_Prefs extends Handler_Protected { VALUES (?, ?, ?, NOW(), ?)"); - $sth->execute([$title, $new_password_hash, Auth_Base::AUTH_SERVICE_API, $_SESSION['uid']]); + $sth->execute([$title, "$new_password_hash:$new_salt", Auth_Base::AUTH_SERVICE_API, $_SESSION['uid']]); $this->appPasswordList(); } diff --git a/classes/pref/users.php b/classes/pref/users.php index 13f808cb3..111cabdca 100644 --- a/classes/pref/users.php +++ b/classes/pref/users.php @@ -107,7 +107,7 @@ class Pref_Users extends Handler_Administrative { function editSave() { $login = clean($_REQUEST["login"]); - $uid = clean($_REQUEST["id"]); + $uid = (int) clean($_REQUEST["id"]); $access_level = (int) clean($_REQUEST["access_level"]); $email = clean($_REQUEST["email"]); $password = clean($_REQUEST["password"]); @@ -118,19 +118,13 @@ class Pref_Users extends Handler_Administrative { // forbid renaming admin if ($uid == 1) $login = "admin"; - if ($password) { - $salt = substr(bin2hex(get_random_bytes(125)), 0, 250); - $pwd_hash = encrypt_password($password, $salt, true); - $pass_query_part = "pwd_hash = ".$this->pdo->quote($pwd_hash).", - salt = ".$this->pdo->quote($salt).","; - } else { - $pass_query_part = ""; - } - - $sth = $this->pdo->prepare("UPDATE ttrss_users SET $pass_query_part login = LOWER(?), - access_level = ?, email = ?, otp_enabled = false WHERE id = ?"); + $sth = $this->pdo->prepare("UPDATE ttrss_users SET login = LOWER(?), + access_level = ?, email = ?, otp_enabled = false WHERE id = ?"); $sth->execute([$login, $access_level, $email, $uid]); + if ($password) { + UserHelper::reset_password($uid, false, $password); + } } function remove() { @@ -153,8 +147,8 @@ class Pref_Users extends Handler_Administrative { function add() { $login = clean($_REQUEST["login"]); $tmp_user_pwd = make_password(); - $salt = substr(bin2hex(get_random_bytes(125)), 0, 250); - $pwd_hash = encrypt_password($tmp_user_pwd, $salt, true); + $salt = UserHelper::get_salt(); + $pwd_hash = UserHelper::hash_password($tmp_user_pwd, $salt, UserHelper::HASH_ALGOS[0]); if (!$login) return; // no blank usernames diff --git a/classes/rpc.php b/classes/rpc.php index 65612ec34..633e3a86e 100755 --- a/classes/rpc.php +++ b/classes/rpc.php @@ -442,7 +442,7 @@ class RPC extends Handler_Protected { $params["default_view_limit"] = (int) get_pref(Prefs::_DEFAULT_VIEW_LIMIT); $params["default_view_order_by"] = get_pref(Prefs::_DEFAULT_VIEW_ORDER_BY); $params["bw_limit"] = (int) $_SESSION["bw_limit"]; - $params["is_default_pw"] = Pref_Prefs::_is_default_password(); + $params["is_default_pw"] = UserHelper::is_default_password(); $params["label_base_index"] = LABEL_BASE_INDEX; $theme = get_pref(Prefs::USER_CSS_THEME); diff --git a/classes/userhelper.php b/classes/userhelper.php index 44406b959..e3f39a7f8 100644 --- a/classes/userhelper.php +++ b/classes/userhelper.php @@ -3,6 +3,20 @@ use OTPHP\TOTP; class UserHelper { + const HASH_ALGO_SSHA512 = 'SSHA-512'; + const HASH_ALGO_SSHA256 = 'SSHA-256'; + const HASH_ALGO_MODE2 = 'MODE2'; + const HASH_ALGO_SHA1X = 'SHA1X'; + const HASH_ALGO_SHA1 = 'SHA1'; + + const HASH_ALGOS = [ + self::HASH_ALGO_SSHA512, + self::HASH_ALGO_SSHA256, + self::HASH_ALGO_MODE2, + self::HASH_ALGO_SHA1X, + self::HASH_ALGO_SHA1 + ]; + static function authenticate(string $login = null, string $password = null, bool $check_only = false, string $service = null) { if (!Config::get(Config::SINGLE_USER_MODE)) { $user_id = false; @@ -190,7 +204,11 @@ class UserHelper { session_commit(); } - static function reset_password($uid, $format_output = false) { + static function get_salt() { + return substr(bin2hex(get_random_bytes(125)), 0, 250); + } + + static function reset_password($uid, $format_output = false, $new_password = "") { $pdo = Db::pdo(); @@ -201,10 +219,10 @@ class UserHelper { $login = $row["login"]; - $new_salt = substr(bin2hex(get_random_bytes(125)), 0, 250); - $tmp_user_pwd = make_password(); + $new_salt = self::get_salt(); + $tmp_user_pwd = $new_password ? $new_password : make_password(); - $pwd_hash = encrypt_password($tmp_user_pwd, $new_salt, true); + $pwd_hash = self::hash_password($tmp_user_pwd, $new_salt, self::HASH_ALGOS[0]); $sth = $pdo->prepare("UPDATE ttrss_users SET pwd_hash = ?, salt = ?, otp_enabled = false @@ -276,4 +294,43 @@ class UserHelper { return null; } + + static function is_default_password() { + $authenticator = PluginHost::getInstance()->get_plugin($_SESSION["auth_module"]); + + if ($authenticator && + method_exists($authenticator, "check_password") && + $authenticator->check_password($_SESSION["uid"], "password")) { + + return true; + } + return false; + } + + static function hash_password(string $pass, string $salt, string $algo) { + $pass_hash = ""; + + switch ($algo) { + case self::HASH_ALGO_SHA1: + $pass_hash = sha1($pass); + break; + case self::HASH_ALGO_SHA1X: + $pass_hash = sha1("$salt:$pass"); + break; + case self::HASH_ALGO_MODE2: + case self::HASH_ALGO_SSHA256: + $pass_hash = hash('sha256', $salt . $pass); + break; + case self::HASH_ALGO_SSHA512: + $pass_hash = hash('sha512', $salt . $pass); + break; + default: + user_error("hash_password: unknown hash algo: $algo", E_USER_ERROR); + } + + if ($pass_hash) + return "$algo:$pass_hash"; + else + return false; + } } diff --git a/include/functions.php b/include/functions.php index 275caac69..4892d5320 100644 --- a/include/functions.php +++ b/include/functions.php @@ -361,16 +361,6 @@ return vsprintf(_ngettext(array_shift($args), array_shift($args), array_shift($args)), $args); } - function encrypt_password($pass, $salt = '', $mode2 = false) { - if ($salt && $mode2) { - return "MODE2:" . hash('sha256', $salt . $pass); - } else if ($salt) { - return "SHA1X:" . sha1("$salt:$pass"); - } else { - return "SHA1:" . sha1($pass); - } - } // function encrypt_password - function init_plugins() { PluginHost::getInstance()->load(Config::get(Config::PLUGINS), PluginHost::KIND_ALL); diff --git a/js/PrefUsers.js b/js/PrefUsers.js index 3eb83b02a..109782f6c 100644 --- a/js/PrefUsers.js +++ b/js/PrefUsers.js @@ -1,16 +1,18 @@ 'use strict' -/* global __ */ -/* global xhrPost, xhr, dijit, Notify, Tables, App, fox */ +/* global __, xhr, dijit, Notify, Tables, App, fox */ const Users = { reload: function(sort) { - const user_search = App.byId("user_search"); - const search = user_search ? user_search.value : ""; + return new Promise((resolve, reject) => { + const user_search = App.byId("user_search"); + const search = user_search ? user_search.value : ""; - xhr.post("backend.php", { op: "pref-users", sort: sort, search: search }, (reply) => { - dijit.byId('usersTab').attr('content', reply); - Notify.close(); + xhr.post("backend.php", { op: "pref-users", sort: sort, search: search }, (reply) => { + dijit.byId('usersTab').attr('content', reply); + Notify.close(); + resolve(); + }, (e) => { reject(e) }); }); }, add: function() { @@ -20,8 +22,9 @@ const Users = { Notify.progress("Adding user..."); xhr.post("backend.php", {op: "pref-users", method: "add", login: login}, (reply) => { - alert(reply); - Users.reload(); + Users.reload().then(() => { + Notify.info(reply); + }) }); } @@ -38,9 +41,11 @@ const Users = { if (this.validate()) { Notify.progress("Saving data...", true); - xhr.post("backend.php", this.attr('value'), () => { + xhr.post("backend.php", this.attr('value'), (reply) => { dialog.hide(); - Users.reload(); + Users.reload().then(() => { + Notify.info(reply); + }); }); } }, diff --git a/js/common.js b/js/common.js index 1544e6d0b..59231010e 100755 --- a/js/common.js +++ b/js/common.js @@ -154,7 +154,7 @@ String.prototype.stripTags = function() { /* exported xhr */ const xhr = { - post: function(url, params = {}, complete = undefined) { + post: function(url, params = {}, complete = undefined, failed = undefined) { console.log('xhr.post', '>>>', params); return new Promise((resolve, reject) => { @@ -165,6 +165,9 @@ const xhr = { postData: dojo.objectToQuery(params), handleAs: "text", error: function(error) { + if (failed != undefined) + failed(error); + reject(error); }, load: function(data, ioargs) { @@ -178,7 +181,7 @@ const xhr = { ); }); }, - json: function(url, params = {}, complete = undefined) { + json: function(url, params = {}, complete = undefined, failed = undefined) { return new Promise((resolve, reject) => this.post(url, params).then((data) => { let obj = null; @@ -187,6 +190,10 @@ const xhr = { obj = JSON.parse(data); } catch (e) { console.error("xhr.json", e, xhr); + + if (failed != undefined) + failed(e); + reject(e); } @@ -194,6 +201,10 @@ const xhr = { if (obj && typeof App != "undefined") if (!App.handleRpcJson(obj)) { + + if (failed != undefined) + failed(obj); + reject(obj); return; } diff --git a/plugins/auth_internal/init.php b/plugins/auth_internal/init.php index 0cff439cf..9a8417d4f 100644 --- a/plugins/auth_internal/init.php +++ b/plugins/auth_internal/init.php @@ -19,8 +19,6 @@ class Auth_Internal extends Auth_Base { function authenticate($login, $password, $service = '') { - $pwd_hash1 = encrypt_password($password); - $pwd_hash2 = encrypt_password($password, $login); $otp = (int) ($_REQUEST["otp"] ?? 0); // don't bother with null/null logins for auth_external etc @@ -135,117 +133,54 @@ class Auth_Internal extends Auth_Base { return $user_id; } - if (get_schema_version() > 87) { + if ($login) { + $try_user_id = $this->find_user_by_login($login); - $sth = $this->pdo->prepare("SELECT salt FROM ttrss_users WHERE LOWER(login) = LOWER(?)"); - $sth->execute([$login]); - - if ($row = $sth->fetch()) { - $salt = $row['salt']; - - if ($salt == "") { - - $sth = $this->pdo->prepare("SELECT id FROM ttrss_users WHERE - LOWER(login) = LOWER(?) AND (pwd_hash = ? OR pwd_hash = ?)"); - - $sth->execute([$login, $pwd_hash1, $pwd_hash2]); - - // verify and upgrade password to new salt base - - if ($row = $sth->fetch()) { - // upgrade password to MODE2 - - $user_id = $row['id']; - - $salt = substr(bin2hex(get_random_bytes(125)), 0, 250); - $pwd_hash = encrypt_password($password, $salt, true); - - $sth = $this->pdo->prepare("UPDATE ttrss_users SET - pwd_hash = ?, salt = ? WHERE LOWER(login) = LOWER(?)"); - - $sth->execute([$pwd_hash, $salt, $login]); - - return $user_id; - - } else { - return false; - } - - } else { - $pwd_hash = encrypt_password($password, $salt, true); - - $sth = $this->pdo->prepare("SELECT id - FROM ttrss_users WHERE - LOWER(login) = LOWER(?) AND pwd_hash = ?"); - $sth->execute([$login, $pwd_hash]); - - if ($row = $sth->fetch()) { - return $row['id']; - } - } - - } else { - $sth = $this->pdo->prepare("SELECT id - FROM ttrss_users WHERE - LOWER(login) = LOWER(?) AND (pwd_hash = ? OR pwd_hash = ?)"); - - $sth->execute([$login, $pwd_hash1, $pwd_hash2]); - - if ($row = $sth->fetch()) { - return $row['id']; - } - } - } else { - $sth = $this->pdo->prepare("SELECT id - FROM ttrss_users WHERE - LOWER(login) = LOWER(?) AND (pwd_hash = ? OR pwd_hash = ?)"); - - $sth->execute([$login, $pwd_hash1, $pwd_hash2]); - - if ($row = $sth->fetch()) { - return $row['id']; + if ($try_user_id) { + return $this->check_password($try_user_id, $password); } } return false; } - function check_password($owner_uid, $password, $service = '') { + function check_password(int $owner_uid, string $password, string $service = '') { + + if (get_schema_version() > 87) { + $sth = $this->pdo->prepare("SELECT salt,login,otp_enabled,pwd_hash FROM ttrss_users WHERE id = ?"); + } else { + $sth = $this->pdo->prepare("SELECT login,otp_enabled,pwd_hash FROM ttrss_users WHERE id = ?"); + } - $sth = $this->pdo->prepare("SELECT salt,login,otp_enabled FROM ttrss_users WHERE - id = ?"); $sth->execute([$owner_uid]); if ($row = $sth->fetch()) { - $salt = $row['salt']; + $salt = $row['salt'] ?? ""; $login = $row['login']; + $pwd_hash = $row['pwd_hash']; + + list ($pwd_algo, $raw_hash) = explode(":", $pwd_hash, 2); // check app password only if service is specified if ($service && get_schema_version() > 138) { return $this->check_app_password($login, $password, $service); } - if (!$salt) { - $password_hash1 = encrypt_password($password); - $password_hash2 = encrypt_password($password, $login); + $test_hash = UserHelper::hash_password($password, $salt, $pwd_algo); - $sth = $this->pdo->prepare("SELECT id FROM ttrss_users WHERE - id = ? AND (pwd_hash = ? OR pwd_hash = ?)"); + if (hash_equals($pwd_hash, $test_hash)) { + if ($pwd_algo != UserHelper::HASH_ALGOS[0]) { + Logger::log(E_USER_NOTICE, "Upgrading password of user $login to " . UserHelper::HASH_ALGOS[0]); - $sth->execute([$owner_uid, $password_hash1, $password_hash2]); + $new_hash = UserHelper::hash_password($password, $salt, UserHelper::HASH_ALGOS[0]); - return $sth->fetch(); - - } else { - $password_hash = encrypt_password($password, $salt, true); - - $sth = $this->pdo->prepare("SELECT id FROM ttrss_users WHERE - id = ? AND pwd_hash = ?"); - - $sth->execute([$owner_uid, $password_hash]); - - return $sth->fetch(); + if ($new_hash) { + $usth = $this->pdo->prepare("UPDATE ttrss_users SET pwd_hash = ? WHERE id = ?"); + $usth->execute([$new_hash, $owner_uid]); + } + } + return $owner_uid; } } @@ -256,15 +191,16 @@ class Auth_Internal extends Auth_Base { if ($this->check_password($owner_uid, $old_password)) { - $new_salt = substr(bin2hex(get_random_bytes(125)), 0, 250); - $new_password_hash = encrypt_password($new_password, $new_salt, true); + $new_salt = UserHelper::get_salt(); + $new_password_hash = UserHelper::hash_password($new_password, $new_salt, UserHelper::HASH_ALGOS[0]); $sth = $this->pdo->prepare("UPDATE ttrss_users SET pwd_hash = ?, salt = ?, otp_enabled = false WHERE id = ?"); $sth->execute([$new_password_hash, $new_salt, $owner_uid]); - $_SESSION["pwd_hash"] = $new_password_hash; + if ($_SESSION["uid"] ?? 0 == $owner_uid) + $_SESSION["pwd_hash"] = $new_password_hash; $sth = $this->pdo->prepare("SELECT email, login FROM ttrss_users WHERE id = ?"); $sth->execute([$owner_uid]); @@ -303,19 +239,27 @@ class Auth_Internal extends Auth_Base { $sth->execute([$login, $service]); while ($row = $sth->fetch()) { - list ($algo, $hash, $salt) = explode(":", $row["pwd_hash"]); + list ($pwd_algo, $raw_hash, $salt) = explode(":", $row["pwd_hash"]); - if ($algo == "SSHA-512") { - $test_hash = hash('sha512', $salt . $password); + $test_hash = UserHelper::hash_password($password, $salt, $pwd_algo); - if ($test_hash == $hash) { - $usth = $this->pdo->prepare("UPDATE ttrss_app_passwords SET last_used = NOW() WHERE id = ?"); - $usth->execute([$row['id']]); + if (hash_equals("$pwd_algo:$raw_hash", $test_hash)) { + $usth = $this->pdo->prepare("UPDATE ttrss_app_passwords SET last_used = NOW() WHERE id = ?"); + $usth->execute([$row['id']]); - return $row['uid']; + if ($pwd_algo != UserHelper::HASH_ALGOS[0]) { + // upgrade password to current algo + Logger::log(E_USER_NOTICE, "Upgrading app password of user $login to " . UserHelper::HASH_ALGOS[0]); + + $new_hash = UserHelper::hash_password($password, $salt, UserHelper::HASH_ALGOS[0]); + + if ($new_hash) { + $usth = $this->pdo->prepare("UPDATE ttrss_app_passwords SET pwd_hash = ? WHERE id = ?"); + $usth->execute(["$new_hash:$salt", $row['id']]); + } } - } else { - user_error("Got unknown algo of app password for user $login: $algo"); + + return $row['uid']; } }