simplify internal authentication code and bump default algo to SSHA-512

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 {
 		<?php
 	}
 
-	private function _encrypt_app_password($password) {
-		$salt = substr(bin2hex(get_random_bytes(24)), 0, 24);
-
-		return "SSHA-512:".hash('sha512', $salt . $password). ":$salt";
-	}
-
 	function deleteAppPassword() {
 		$ids = explode(",", clean($_REQUEST['ids']));
 		$ids_qmarks = arr_qmarks($ids);
@@ -1423,7 +1404,8 @@ class Pref_Prefs extends Handler_Protected {
 	function generateAppPassword() {
 		$title = clean($_REQUEST['title']);
 		$new_password = make_password(16);
-		$new_password_hash = $this->_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 <strong>%s</strong> 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) {
-
-			$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'];
-					}
-				}
+		if ($login) {
+			$try_user_id = $this->find_user_by_login($login);
 
-			} 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);
-
-				$sth = $this->pdo->prepare("SELECT id FROM ttrss_users WHERE
-					id = ? AND (pwd_hash = ? OR pwd_hash = ?)");
-
-				$sth->execute([$owner_uid, $password_hash1, $password_hash2]);
+			$test_hash = UserHelper::hash_password($password, $salt, $pwd_algo);
 
-				return $sth->fetch();
+			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]);
 
-			} else {
-				$password_hash = encrypt_password($password, $salt, true);
+					$new_hash = UserHelper::hash_password($password, $salt, UserHelper::HASH_ALGOS[0]);
 
-				$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"]);
+
+			$test_hash = UserHelper::hash_password($password, $salt, $pwd_algo);
+
+			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']]);
 
-			if ($algo == "SSHA-512") {
-				$test_hash = hash('sha512', $salt . $password);
+				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]);
 
-				if ($test_hash == $hash) {
-					$usth = $this->pdo->prepare("UPDATE ttrss_app_passwords SET last_used = NOW() WHERE id = ?");
-					$usth->execute([$row['id']]);
+					$new_hash = UserHelper::hash_password($password, $salt, UserHelper::HASH_ALGOS[0]);
 
-					return $row['uid'];
+					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'];
 			}
 		}