Merge b0c08bf370 into 2d36d32da8

games/devtest/mods/util_commands/init.lua

@@ -241,3 +241,11 @@ core.register_chatcommand("set_saturation", {
 		core.get_player_by_name(player_name):set_lighting({saturation = saturation })
 	end
 })
+
+core.register_chatcommand("shutdown_with_reconnect", {
+	params = "",
+	description = "Shutdown server with reconnect request.",
+	func = function(player_name, param)
+		minetest.request_shutdown("Shutdown with reconnect.", true, 5)
+	end
+})

src/client/CMakeLists.txt

@@ -38,6 +38,7 @@ set(client_SRCS
 	${CMAKE_CURRENT_SOURCE_DIR}/activeobjectmgr.cpp
 	${CMAKE_CURRENT_SOURCE_DIR}/camera.cpp
 	${CMAKE_CURRENT_SOURCE_DIR}/client.cpp
+	${CMAKE_CURRENT_SOURCE_DIR}/clientauth.cpp
 	${CMAKE_CURRENT_SOURCE_DIR}/clientenvironment.cpp
 	${CMAKE_CURRENT_SOURCE_DIR}/clientlauncher.cpp
 	${CMAKE_CURRENT_SOURCE_DIR}/clientmap.cpp

src/client/client.cpp

@@ -86,8 +86,9 @@ void PacketCounter::print(std::ostream &o) const
 */
 
 Client::Client(
-		const char *playername,
+		const std::string &playername,
-		const std::string &password,
+		//const std::string &password,
+		ClientAuth *auth,
 		MapDrawControl &control,
 		IWritableTextureSource *tsrc,
 		IWritableShaderSource *shsrc,
@@ -116,14 +117,14 @@ Client::Client(
 	m_allow_login_or_register(allow_login_or_register),
 	m_server_ser_ver(SER_FMT_VER_INVALID),
 	m_last_chat_message_sent(time(NULL)),
-	m_password(password),
+	m_auth(auth),
 	m_chosen_auth_mech(AUTH_MECHANISM_NONE),
 	m_media_downloader(new ClientMediaDownloader()),
 	m_state(LC_Created),
 	m_modchannel_mgr(new ModChannelMgr())
 {
 	// Add local player
-	m_env.setLocalPlayer(new LocalPlayer(this, playername));
+	m_env.setLocalPlayer(new LocalPlayer(this, playername.c_str()));
 
 	// Make the mod storage database and begin the save for later
 	m_mod_storage_database =
@@ -1109,20 +1110,7 @@ void Client::interact(InteractAction action, const PointedThing& pointed)
 
 void Client::deleteAuthData()
 {
-	if (!m_auth_data)
+	m_auth->clearSessionData();
-		return;
-
-	switch (m_chosen_auth_mech) {
-		case AUTH_MECHANISM_FIRST_SRP:
-			break;
-		case AUTH_MECHANISM_SRP:
-		case AUTH_MECHANISM_LEGACY_PASSWORD:
-			srp_user_delete((SRPUser *) m_auth_data);
-			m_auth_data = NULL;
-			break;
-		case AUTH_MECHANISM_NONE:
-			break;
-	}
 	m_chosen_auth_mech = AUTH_MECHANISM_NONE;
 }
 
@@ -1161,36 +1149,34 @@ void Client::startAuth(AuthMechanism chosen_auth_mechanism)
 	switch (chosen_auth_mechanism) {
 		case AUTH_MECHANISM_FIRST_SRP: {
 			// send srp verifier to server
-			std::string verifier;
+			const std::string &verifier = m_auth->getSrpVerifier();
-			std::string salt;
+			const std::string &salt = m_auth->getSrpSalt();
-			generate_srp_verifier_and_salt(playername, m_password,
-				&verifier, &salt);
 
 			NetworkPacket resp_pkt(TOSERVER_FIRST_SRP, 0);
-			resp_pkt << salt << verifier << (u8)((m_password.empty()) ? 1 : 0);
+			resp_pkt << salt << verifier << (u8)((m_auth->getIsEmpty()) ? 1 : 0);
 
 			Send(&resp_pkt);
 			break;
 		}
 		case AUTH_MECHANISM_SRP:
 		case AUTH_MECHANISM_LEGACY_PASSWORD: {
-			u8 based_on = 1;
+			u8 based_on;
+			SRPUser *auth_data;
 
 			if (chosen_auth_mechanism == AUTH_MECHANISM_LEGACY_PASSWORD) {
-				m_password = translate_password(playername, m_password);
 				based_on = 0;
+				auth_data = m_auth->getLegacyAuthData();
+			}
+			else {
+				based_on = 1;
+				auth_data = m_auth->getSrpAuthData();
 			}
 
-			std::string playername_u = lowercase(playername);
-			m_auth_data = srp_user_new(SRP_SHA256, SRP_NG_2048,
-				playername.c_str(), playername_u.c_str(),
-				(const unsigned char *) m_password.c_str(),
-				m_password.length(), NULL, NULL);
 			char *bytes_A = 0;
 			size_t len_A = 0;
 			SRP_Result res = srp_user_start_authentication(
-				(struct SRPUser *) m_auth_data, NULL, NULL, 0,
+				auth_data, NULL, NULL, 0,
-				(unsigned char **) &bytes_A, &len_A);
+				reinterpret_cast<unsigned char **>(&bytes_A), &len_A);
 			FATAL_ERROR_IF(res != SRP_OK, "Creating local SRP user failed.");
 
 			NetworkPacket resp_pkt(TOSERVER_SRP_BYTES_A, 0);
@@ -1342,16 +1328,25 @@ void Client::clearOutChatQueue()
 	m_out_chat_queue = std::queue<std::wstring>();
 }
 
-void Client::sendChangePassword(const std::string &oldpassword,
+void Client::sendChangePassword(std::string &oldpassword,
-	const std::string &newpassword)
+	std::string &newpassword)
 {
 	LocalPlayer *player = m_env.getLocalPlayer();
-	if (player == NULL)
+	if (player == NULL) {
+		porting::secure_clear_string(oldpassword);
+		porting::secure_clear_string(newpassword);
 		return;
+	}
 
 	// get into sudo mode and then send new password to server
-	m_password = oldpassword;
+	std::string playername = m_env.getLocalPlayer()->getName();
-	m_new_password = newpassword;
+	m_auth->applyPassword(playername, oldpassword);
+	m_new_auth.applyPassword(playername, newpassword);
+
+	// we do not need to keep passwords in memory
+	porting::secure_clear_string(oldpassword);
+	porting::secure_clear_string(newpassword);
+
 	startAuth(choseAuthMech(m_sudo_auth_methods));
 }
 

src/client/client.h

@@ -19,6 +19,8 @@
 #include "network/peerhandler.h"
 #include "gameparams.h"
 #include "script/common/c_types.h" // LuaError
+#include "clientdynamicinfo.h"
+#include "clientauth.h"
 #include "util/numeric.h"
 #include "util/string.h" // StringMap
 #include "config.h"
@@ -109,8 +111,9 @@ public:
 	*/
 
 	Client(
-			const char *playername,
+			const std::string &playername,
-			const std::string &password,
+			//const std::string &password,
+			ClientAuth *auth,
 			MapDrawControl &control,
 			IWritableTextureSource *tsrc,
 			IWritableShaderSource *shsrc,
@@ -233,8 +236,8 @@ public:
 	void sendInventoryAction(InventoryAction *a);
 	void sendChatMessage(const std::wstring &message);
 	void clearOutChatQueue();
-	void sendChangePassword(const std::string &oldpassword,
+	void sendChangePassword(std::string &oldpassword,
-		const std::string &newpassword);
+		std::string &newpassword);
 	void sendDamage(u16 damage);
 	void sendRespawnLegacy();
 	void sendReady();
@@ -525,12 +528,11 @@ private:
 
 	// Auth data
 	std::string m_playername;
-	std::string m_password;
+	ClientAuth *m_auth;
 	// If set, this will be sent (and cleared) upon a TOCLIENT_ACCEPT_SUDO_MODE
-	std::string m_new_password;
+	ClientAuth m_new_auth;
 	// Usable by auth mechanisms.
 	AuthMechanism m_chosen_auth_mech;
-	void *m_auth_data = nullptr;
 
 	bool m_access_denied = false;
 	bool m_access_denied_reconnect = false;

src/client/clientauth.cpp

@@ -0,0 +1,113 @@
+// Luanti
+// SPDX-License-Identifier: LGPL-2.1-or-later
+// Copyright (C) 2024-2025 SFENCE, <sfence.software@gmail.com>
+
+#include "clientauth.h"
+#include "exceptions.h"
+#include "util/auth.h"
+#include "util/srp.h"
+#include "util/string.h"
+
+ClientAuth::ClientAuth() :
+		m_is_empty(true)
+{
+}
+
+ClientAuth::ClientAuth(const std::string &player_name, const std::string &password)
+{
+	applyPassword(player_name, password);
+}
+
+ClientAuth::~ClientAuth()
+{
+	clear();
+}
+
+ClientAuth &ClientAuth::operator=(ClientAuth &&other)
+{
+	clear();
+
+	m_is_empty = other.m_is_empty;
+
+	m_srp_verifier = other.m_srp_verifier;
+	m_srp_salt = other.m_srp_salt;
+
+	m_legacy_auth_data = other.m_legacy_auth_data;
+	m_srp_auth_data = other.m_srp_auth_data;
+
+	other.m_legacy_auth_data = nullptr;
+	other.m_srp_auth_data = nullptr;
+
+	other.clear();
+
+	return *this;
+}
+
+void ClientAuth::applyPassword(const std::string &player_name, const std::string &password)
+{
+	clear();
+	// AUTH_MECHANISM_FIRST_SRP
+	generate_srp_verifier_and_salt(player_name, password, &m_srp_verifier, &m_srp_salt);
+	m_is_empty = password.empty();
+
+	std::string player_name_u = lowercase(player_name);
+	// AUTH_MECHANISM_SRP
+	m_srp_auth_data = srp_user_new(SRP_SHA256, SRP_NG_2048,
+			player_name.c_str(), player_name_u.c_str(),
+			reinterpret_cast<const unsigned char *>(password.c_str()),
+			password.length(), NULL, NULL);
+	// AUTH_MECHANISM_LEGACY_PASSWORD
+	std::string translated = translate_password(player_name, password);
+	m_legacy_auth_data = srp_user_new(SRP_SHA256, SRP_NG_2048,
+			player_name.c_str(), player_name_u.c_str(),
+			reinterpret_cast<const unsigned char *>(translated.c_str()),
+			translated.length(), NULL, NULL);
+}
+
+SRPUser * ClientAuth::getAuthData(AuthMechanism chosen_auth_mech) const
+{
+	SRPUser *chosen = nullptr;
+	switch (chosen_auth_mech) {
+		case AUTH_MECHANISM_LEGACY_PASSWORD:
+			chosen = m_legacy_auth_data;
+			break;
+		case AUTH_MECHANISM_SRP:
+			chosen = m_srp_auth_data;
+			break;
+		case AUTH_MECHANISM_FIRST_SRP:
+			return nullptr;
+		default:
+			throw AuthError("Not valid auth data request.");
+	}
+	if (!chosen)
+		throw AuthError("Not valid autch data found.");
+	return chosen;
+}
+
+void ClientAuth::clear()
+{
+	if (m_legacy_auth_data != nullptr) {
+		srp_user_delete(m_legacy_auth_data);
+		m_legacy_auth_data = nullptr;
+	}
+	if (m_srp_auth_data != nullptr) {
+		srp_user_delete(m_srp_auth_data);
+		m_srp_auth_data = nullptr;
+	}
+	m_srp_verifier.clear();
+	m_srp_salt.clear();
+}
+
+void ClientAuth::clearSessionData()
+{
+	if (m_legacy_auth_data != nullptr) {
+		srp_user_clear_sessiondata(m_legacy_auth_data);
+	}
+	if (m_srp_auth_data != nullptr) {
+		srp_user_clear_sessiondata(m_srp_auth_data);
+	}
+	// This is need only for first login to server.
+	// So, there is no need to keep this for reconnect purposes.
+	m_srp_verifier.clear();
+	m_srp_salt.clear();
+}

src/client/clientauth.h

@@ -0,0 +1,44 @@
+// Luanti
+// SPDX-License-Identifier: LGPL-2.1-or-later
+// Copyright (C) 2024-2025 SFENCE, <sfence.software@gmail.com>
+
+#pragma once
+
+#include <string>
+#include "network/networkprotocol.h"
+#include "util/basic_macros.h"
+
+struct SRPUser;
+
+class ClientAuth
+{
+public:
+	ClientAuth();
+	ClientAuth(const std::string &player_name, const std::string &password);
+
+	~ClientAuth();
+	DISABLE_CLASS_COPY(ClientAuth);
+
+	ClientAuth(ClientAuth &&other) { *this = std::move(other); }
+	ClientAuth &operator=(ClientAuth &&other);
+
+	void applyPassword(const std::string &player_name, const std::string &password);
+
+	bool getIsEmpty() const { return m_is_empty; }
+	const std::string &getSrpVerifier() const { return m_srp_verifier; }
+	const std::string &getSrpSalt() const { return m_srp_salt; }
+	SRPUser * getLegacyAuthData() const { return m_legacy_auth_data; }
+	SRPUser * getSrpAuthData() const { return m_srp_auth_data; }
+	SRPUser * getAuthData(AuthMechanism chosen_auth_mech) const;
+
+	void clear();
+	void clearSessionData();
+private:
+	bool m_is_empty;
+
+	std::string m_srp_verifier;
+	std::string m_srp_salt;
+
+	SRPUser *m_legacy_auth_data = nullptr;
+	SRPUser *m_srp_auth_data = nullptr;
+};

src/client/clientgamestartdata.h

@@ -0,0 +1,34 @@
+// Luanti
+// SPDX-License-Identifier: LGPL-2.1-or-later
+// Copyright (C) 2024-2025 SFENCE, <sfence.software@gmail.com>
+
+#pragma once
+
+#include "gameparams.h"
+#include "clientauth.h"
+
+// Information processed by main menu
+struct ClientGameStartData : GameParams
+{
+	ClientGameStartData(const GameStartData &start_data):
+		GameParams(start_data),
+		name(start_data.name),
+		address(start_data.address),
+		local_server(start_data.local_server),
+		allow_login_or_register(start_data.allow_login_or_register),
+		world_spec(start_data.world_spec)
+	{
+	}
+
+	bool isSinglePlayer() const { return address.empty() && !local_server; }
+
+	std::string name;
+	ClientAuth auth;
+	std::string address;
+	bool local_server;
+
+	ELoginRegister allow_login_or_register = ELoginRegister::Any;
+
+	// "world_path" must be kept in sync!
+	WorldSpec world_spec;
+};

src/client/clientlauncher.cpp

@@ -87,7 +87,8 @@ bool ClientLauncher::run(GameStartData &start_data, const Settings &cmd_args)
 	 *   - Local server (for main menu only)
 	*/
 
-	init_args(start_data, cmd_args);
+	ClientGameStartData client_start_data(start_data);
+	init_args(client_start_data, cmd_args);
 
 #if USE_SOUND
 	g_sound_manager_singleton = createSoundManagerSingleton();
@@ -176,7 +177,7 @@ bool ClientLauncher::run(GameStartData &start_data, const Settings &cmd_args)
 				core::rect<s32>(0, 0, 10000, 10000));
 
 			bool should_run_game = launch_game(error_message, reconnect_requested,
-				start_data, cmd_args);
+				client_start_data, cmd_args);
 
 			// Reset the reconnect_requested flag
 			reconnect_requested = false;
@@ -200,7 +201,7 @@ bool ClientLauncher::run(GameStartData &start_data, const Settings &cmd_args)
 				kill,
 				input,
 				m_rendering_engine,
-				start_data,
+				client_start_data,
 				error_message,
 				chat_backend,
 				&reconnect_requested
@@ -255,7 +256,7 @@ bool ClientLauncher::run(GameStartData &start_data, const Settings &cmd_args)
 	return retval;
 }
 
-void ClientLauncher::init_args(GameStartData &start_data, const Settings &cmd_args)
+void ClientLauncher::init_args(ClientGameStartData &start_data, const Settings &cmd_args)
 {
 	skip_main_menu = cmd_args.getFlag("go");
 
@@ -390,20 +391,22 @@ void ClientLauncher::config_guienv()
 }
 
 bool ClientLauncher::launch_game(std::string &error_message,
-		bool reconnect_requested, GameStartData &start_data,
+		bool reconnect_requested, ClientGameStartData &start_data,
 		const Settings &cmd_args)
 {
 	// Prepare and check the start data to launch a game
 	std::string error_message_lua = error_message;
 	error_message.clear();
 
+	std::string password;
+
 	if (cmd_args.exists("password"))
-		start_data.password = cmd_args.get("password");
+		password = cmd_args.get("password");
 
 	if (cmd_args.exists("password-file")) {
 		std::ifstream passfile(cmd_args.get("password-file"));
 		if (passfile.good()) {
-			std::getline(passfile, start_data.password);
+			std::getline(passfile, password);
 		} else {
 			error_message = gettext("Provided password file "
 					"failed to open: ")
@@ -432,7 +435,7 @@ bool ClientLauncher::launch_game(std::string &error_message,
 		MainMenuData menudata;
 		menudata.address                         = start_data.address;
 		menudata.name                            = start_data.name;
-		menudata.password                        = start_data.password;
+		menudata.password                        = password;
 		menudata.port                            = itos(start_data.socket_port);
 		menudata.script_data.errormessage        = std::move(error_message_lua);
 		menudata.script_data.reconnect_requested = reconnect_requested;
@@ -464,12 +467,15 @@ bool ClientLauncher::launch_game(std::string &error_message,
 		}
 
 		start_data.name = menudata.name;
-		start_data.password = menudata.password;
+		password = menudata.password;
 		start_data.address = std::move(menudata.address);
 		start_data.allow_login_or_register = menudata.allow_login_or_register;
 		server_name = menudata.servername;
 		server_description = menudata.serverdescription;
 
+		/* make sure that password will not stay somewhere in memory */
+		porting::secure_clear_string(menudata.password);
+
 		start_data.local_server = !menudata.simple_singleplayer_mode &&
 			start_data.address.empty();
 	} else {
@@ -477,24 +483,36 @@ bool ClientLauncher::launch_game(std::string &error_message,
 			start_data.address.empty() && !start_data.name.empty();
 	}
 
-	if (!m_rendering_engine->run())
+	if (!m_rendering_engine->run()) {
+		/* make sure that password will not stay somewhere in memory */
+		porting::secure_clear_string(password);
 		return false;
+	}
 
 	if (!start_data.isSinglePlayer() && start_data.name.empty()) {
 		error_message = gettext("Please choose a name!");
 		errorstream << error_message << std::endl;
+		/* make sure that password will not stay somewhere in memory */
+		porting::secure_clear_string(password);
 		return false;
 	}
 
 	// If using simple singleplayer mode, override
 	if (start_data.isSinglePlayer()) {
 		start_data.name = "singleplayer";
-		start_data.password = "";
+		password = "";
 		start_data.socket_port = myrand_range(49152, 65535);
 	} else {
 		g_settings->set("name", start_data.name);
 	}
 
+	if (!reconnect_requested) {
+		// This should not be call on reconnect request, because password has been erased.
+		start_data.auth.applyPassword(start_data.name, password);
+	}
+	/* make sure that password will not stay somewhere in memory */
+	porting::secure_clear_string(password);
+
 	if (start_data.name.length() > PLAYERNAME_SIZE - 1) {
 		error_message = gettext("Player name too long.");
 		start_data.name.resize(PLAYERNAME_SIZE);

src/client/clientlauncher.h

@@ -11,6 +11,7 @@ class Settings;
 class MyEventReceiver;
 class InputHandler;
 struct GameStartData;
+struct ClientGameStartData;
 struct MainMenuData;
 
 class ClientLauncher
@@ -23,7 +24,7 @@ public:
 	bool run(GameStartData &start_data, const Settings &cmd_args);
 
 private:
-	void init_args(GameStartData &start_data, const Settings &cmd_args);
+	void init_args(ClientGameStartData &start_data, const Settings &cmd_args);
 	bool init_engine();
 	void init_input();
 	void init_joysticks();
@@ -32,7 +33,7 @@ private:
 	void config_guienv();
 
 	bool launch_game(std::string &error_message, bool reconnect_requested,
-		GameStartData &start_data, const Settings &cmd_args);
+		ClientGameStartData &start_data, const Settings &cmd_args);
 
 	void main_menu(MainMenuData *menudata);
 

src/client/game.cpp

@@ -30,6 +30,8 @@
 #include "log.h"
 #include "log_internal.h"
 #include "gameparams.h"
+#include "filesys.h"
+#include "client/clientgamestartdata.h"
 #include "gettext.h"
 #include "gui/guiChatConsole.h"
 #include "texturesource.h"
@@ -566,7 +568,7 @@ public:
 	bool startup(volatile std::sig_atomic_t *kill,
 			InputHandler *input,
 			RenderingEngine *rendering_engine,
-			const GameStartData &game_params,
+			ClientGameStartData &game_params,
 			std::string &error_message,
 			bool *reconnect,
 			ChatBackend *chat_backend);
@@ -585,11 +587,11 @@ protected:
 	void copyServerClientCache();
 
 	// Client creation
-	bool createClient(const GameStartData &start_data);
+	bool createClient(ClientGameStartData &start_data);
 	bool initGui();
 
 	// Client connection
-	bool connectToServer(const GameStartData &start_data,
+	bool connectToServer(ClientGameStartData &start_data,
 			bool *connect_ok, bool *aborted);
 	bool getServerContent(bool *aborted);
 
@@ -937,7 +939,7 @@ Game::~Game()
 bool Game::startup(volatile std::sig_atomic_t *kill,
 		InputHandler *input,
 		RenderingEngine *rendering_engine,
-		const GameStartData &start_data,
+		ClientGameStartData &start_data,
 		std::string &error_message,
 		bool *reconnect,
 		ChatBackend *chat_backend)
@@ -1330,7 +1332,7 @@ void Game::copyServerClientCache()
 		<< std::endl;
 }
 
-bool Game::createClient(const GameStartData &start_data)
+bool Game::createClient(ClientGameStartData &start_data)
 {
 	showOverlayMessage(N_("Creating client..."), 0, 10);
 
@@ -1460,7 +1462,7 @@ bool Game::initGui()
 	return true;
 }
 
-bool Game::connectToServer(const GameStartData &start_data,
+bool Game::connectToServer(ClientGameStartData &start_data,
 		bool *connect_ok, bool *connection_aborted)
 {
 	*connect_ok = false;	// Let's not be overly optimistic
@@ -1515,8 +1517,8 @@ bool Game::connectToServer(const GameStartData &start_data,
 
 
 	try {
-		client = new Client(start_data.name.c_str(),
+		client = new Client(start_data.name,
-				start_data.password,
+				&start_data.auth,
 				*draw_control, texture_src, shader_src,
 				itemdef_manager, nodedef_manager, sound_manager.get(), eventmgr,
 				m_rendering_engine,
@@ -4245,7 +4247,7 @@ void Game::readSettings()
 void the_game(volatile std::sig_atomic_t *kill,
 		InputHandler *input,
 		RenderingEngine *rendering_engine,
-		const GameStartData &start_data,
+		ClientGameStartData &start_data,
 		std::string &error_message,
 		ChatBackend &chat_backend,
 		bool *reconnect_requested) // Used for local game

src/client/game.h

@@ -8,6 +8,7 @@
 #include "config.h"
 #include <csignal>
 #include <string>
+#include "client/clientgamestartdata.h"
 
 #if !IS_CLIENT_BUILD
 #error Do not include in server builds
@@ -40,7 +41,7 @@ struct CameraOrientation {
 void the_game(volatile std::sig_atomic_t *kill,
 		InputHandler *input,
 		RenderingEngine *rendering_engine,
-		const GameStartData &start_data,
+		ClientGameStartData &start_data,
 		std::string &error_message,
 		ChatBackend &chat_backend,
 		bool *reconnect_requested);

src/exceptions.h

@@ -87,6 +87,11 @@ public:
 	ModError(const std::string &s): BaseException(s) {}
 };
 
+class AuthError : public BaseException {
+public:
+	AuthError(const std::string &s): BaseException(s) {}
+};
+
 
 /*
 	Some "old-style" interrupts:

src/gameparams.h

@@ -6,6 +6,7 @@
 
 #include "irrlichttypes.h"
 #include "content/subgames.h"
+#include "porting.h"
 
 // Information provided from "main"
 struct GameParams
@@ -33,7 +34,6 @@ struct GameStartData : GameParams
 	bool isSinglePlayer() const { return address.empty() && !local_server; }
 
 	std::string name;
-	std::string password;
 	// If empty, we're hosting a server.
 	// This may or may not be in "simple singleplayer mode".
 	std::string address;

src/gui/guiPasswordChange.cpp

@@ -176,12 +176,20 @@ void GUIPasswordChange::acceptInput()
 bool GUIPasswordChange::processInput()
 {
 	if (m_newpass != m_newpass_confirm) {
+		porting::secure_clear_string(m_oldpass);
+		porting::secure_clear_string(m_newpass);
+		porting::secure_clear_string(m_newpass_confirm);
 		gui::IGUIElement *e = getElementFromId(ID_message);
 		if (e != NULL)
 			e->setVisible(true);
 		return false;
 	}
-	m_client->sendChangePassword(wide_to_utf8(m_oldpass), wide_to_utf8(m_newpass));
+	std::string old_pass = wide_to_utf8(m_oldpass);
+	std::string new_pass = wide_to_utf8(m_newpass);
+	porting::secure_clear_string(m_oldpass);
+	porting::secure_clear_string(m_newpass);
+	porting::secure_clear_string(m_newpass_confirm);
+	m_client->sendChangePassword(old_pass, new_pass);
 	return true;
 }
 

src/network/clientpackethandler.cpp

@@ -95,8 +95,7 @@ void Client::handleCommand_Hello(NetworkPacket* pkt)
 			<< "(chosen_mech=" << m_chosen_auth_mech << ")." << std::endl;
 		if (m_chosen_auth_mech == AUTH_MECHANISM_SRP ||
 				m_chosen_auth_mech == AUTH_MECHANISM_LEGACY_PASSWORD) {
-			srp_user_delete((SRPUser *) m_auth_data);
+			m_auth->clear();
-			m_auth_data = 0;
 		}
 	}
 
@@ -157,9 +156,7 @@ void Client::handleCommand_AuthAccept(NetworkPacket* pkt)
 
 void Client::handleCommand_AcceptSudoMode(NetworkPacket* pkt)
 {
-	deleteAuthData();
+	*m_auth = std::move(m_new_auth);
-
-	m_password = m_new_password;
 
 	verbosestream << "Client: Received TOCLIENT_ACCEPT_SUDO_MODE." << std::endl;
 
@@ -186,6 +183,8 @@ void Client::handleCommand_AccessDenied(NetworkPacket* pkt)
 	// not been agreed yet, the same as TOCLIENT_INIT.
 	m_access_denied = true;
 
+	deleteAuthData();
+
 	if (pkt->getCommand() != TOCLIENT_ACCESS_DENIED) {
 		// Servers older than 5.6 still send TOCLIENT_ACCESS_DENIED_LEGACY sometimes.
 		// see commit a65f6f07f3a5601207b790edcc8cc945133112f7
@@ -1548,16 +1547,23 @@ void Client::handleCommand_SrpBytesSandB(NetworkPacket* pkt)
 
 	char *bytes_M = 0;
 	size_t len_M = 0;
-	SRPUser *usr = (SRPUser *) m_auth_data;
+	SRPUser *usr = nullptr;
+	try
+	{
+		usr = m_auth->getAuthData(m_chosen_auth_mech);
+	} catch (AuthError &e) {
+		errorstream << "Client: Bad SRP data: " << e.what() << std::endl;
+		return;
+	}
 	std::string s;
 	std::string B;
 	*pkt >> s >> B;
 
 	infostream << "Client: Received TOCLIENT_SRP_BYTES_S_B." << std::endl;
 
-	srp_user_process_challenge(usr, (const unsigned char *) s.c_str(), s.size(),
+	srp_user_process_challenge(usr, reinterpret_cast<const unsigned char *>(s.c_str()), s.size(),
-		(const unsigned char *) B.c_str(), B.size(),
+		reinterpret_cast<const unsigned char *>(B.c_str()), B.size(),
-		(unsigned char **) &bytes_M, &len_M);
+		reinterpret_cast<unsigned char **>(&bytes_M), &len_M);
 
 	if ( !bytes_M ) {
 		errorstream << "Client: SRP-6a S_B safety check violation!" << std::endl;

src/porting.cpp

@@ -8,6 +8,9 @@
 	See comments in porting.h
 */
 
+// enable include of memset_s function
+#define __STDC_WANT_LIB_EXT1__ 1
+
 #include "porting.h"
 
 #if defined(__FreeBSD__)  || defined(__NetBSD__) || defined(__DragonFly__) || defined(__OpenBSD__)
@@ -968,4 +971,33 @@ void TriggerMemoryTrim()
 
 #endif
 
+/// Override every byte before clearing
+void secure_clear_memory(volatile void *ptr, size_t size)
+{
+#ifdef __STDC_LIB_EXT1__
+	memset_s(ptr, size, '0', size);
+#elif _WIN32
+	SecureZeroMemory((PVOID)ptr, size);
+#else
+	volatile char *ch = (char *)ptr;
+	for (;size>0;size--) {
+		*ch = 0;
+		ch++;
+	}
+#endif
+}
+
+/// Override every character before clearing
+void secure_clear_string(std::string &text)
+{
+	secure_clear_memory((void *)text.data(), text.size());
+	text.clear();
+}
+
+void secure_clear_string(std::wstring &text)
+{
+	secure_clear_memory((void *)text.data(), text.size() * sizeof(wchar_t));
+	text.clear();
+}
+
 } //namespace porting

src/porting.h

@@ -341,6 +341,14 @@ bool open_url(const std::string &url);
  */
 bool open_directory(const std::string &path);
 
+
+/// Clear memory by overwriting every character (used for security)
+void secure_clear_memory(volatile void *ptr, size_t size);
+
+/// Clear string by overwriting every character (used for security)
+void secure_clear_string(std::string &text);
+void secure_clear_string(std::wstring &text);
+
 } // namespace porting
 
 #ifdef __ANDROID__

src/util/srp.cpp

@@ -232,8 +232,7 @@ struct SRPUser {
 
 	char *username;
 	char *username_verifier;
-	unsigned char *password;
+	unsigned char ucp_hash[CSRP_MAX_HASH];
-	size_t password_len;
 
 	unsigned char M[CSRP_MAX_HASH];
 	unsigned char H_AMK[CSRP_MAX_HASH];
@@ -413,11 +412,9 @@ static SRP_Result H_ns(mpz_t result, SRP_HashAlgorithm alg, const unsigned char
 	return SRP_OK;
 }
 
-static int calculate_x(mpz_t result, SRP_HashAlgorithm alg, const unsigned char *salt,
+static void precalculate_x(SRP_HashAlgorithm alg, const char *username,
-	size_t salt_len, const char *username, const unsigned char *password,
+	const unsigned char *password, size_t password_len, unsigned char *ucp_hash)
-	size_t password_len)
 {
-	unsigned char ucp_hash[CSRP_MAX_HASH];
 	HashCTX ctx;
 	hash_init(alg, &ctx);
 
@@ -428,8 +425,11 @@ static int calculate_x(mpz_t result, SRP_HashAlgorithm alg, const unsigned char
 	hash_update(alg, &ctx, password, password_len);
 
 	hash_final(alg, &ctx, ucp_hash);
-
+}
-	return H_ns(result, alg, salt, salt_len, ucp_hash, hash_length(alg));
+static int calculate_x(mpz_t result, SRP_HashAlgorithm alg, const unsigned char *salt,
+	size_t salt_len, const unsigned char *ucp_hash)
+{
+	return H_ns(result, alg, salt, salt_len, (const unsigned char *)ucp_hash, hash_length(alg));
 }
 
 static SRP_Result update_hash_n(SRP_HashAlgorithm alg, HashCTX *ctx, const mpz_t n)
@@ -553,8 +553,11 @@ SRP_Result srp_create_salted_verification_key( SRP_HashAlgorithm alg,
 			goto error_and_exit;
 	}
 
+	unsigned char ucp_hash[CSRP_MAX_HASH];
+	precalculate_x(alg, username_for_verifier,
+			password, len_password, ucp_hash);
 	if (!calculate_x(
-			x, alg, *bytes_s, *len_s, username_for_verifier, password, len_password))
+			x, alg, *bytes_s, *len_s, ucp_hash))
 		goto error_and_exit;
 
 	srp_dbg_num(x, "Server calculated x: ");
@@ -768,14 +771,13 @@ struct SRPUser *srp_user_new(SRP_HashAlgorithm alg, SRP_NGType ng_type,
 
 	usr->username = (char *)malloc(ulen);
 	usr->username_verifier = (char *)malloc(uvlen);
-	usr->password = (unsigned char *)malloc(len_password);
-	usr->password_len = len_password;
 
-	if (!usr->username || !usr->password || !usr->username_verifier) goto err_exit;
+	if (!usr->username || !usr->username_verifier) goto err_exit;
 
 	memcpy(usr->username, username, ulen);
 	memcpy(usr->username_verifier, username_for_verifier, uvlen);
-	memcpy(usr->password, bytes_password, len_password);
+
+	precalculate_x(alg, username_for_verifier, bytes_password, len_password, usr->ucp_hash);
 
 	usr->authenticated = 0;
 
@@ -791,10 +793,6 @@ err_exit:
 		delete_ng(usr->ng);
 		free(usr->username);
 		free(usr->username_verifier);
-		if (usr->password) {
-			memset(usr->password, 0, usr->password_len);
-			free(usr->password);
-		}
 		free(usr);
 	}
 
@@ -810,11 +808,8 @@ void srp_user_delete(struct SRPUser *usr)
 
 		delete_ng(usr->ng);
 
-		memset(usr->password, 0, usr->password_len);
-
 		free(usr->username);
 		free(usr->username_verifier);
-		free(usr->password);
 
 		if (usr->bytes_A) free(usr->bytes_A);
 
@@ -823,6 +818,18 @@ void srp_user_delete(struct SRPUser *usr)
 	}
 }
 
+void srp_user_clear_sessiondata(struct SRPUser *usr)
+{
+	if (usr) {
+		if (usr->bytes_A) free(usr->bytes_A);
+		usr->bytes_A = nullptr;
+
+		memset(usr->M, 0, CSRP_MAX_HASH);
+		memset(usr->H_AMK, 0, CSRP_MAX_HASH);
+		memset(usr->session_key, 0, CSRP_MAX_HASH);
+	}
+}
+
 int srp_user_is_authenticated(struct SRPUser *usr)
 {
 	return usr->authenticated;
@@ -899,8 +906,7 @@ void  srp_user_process_challenge(struct SRPUser *usr,
 
 	srp_dbg_num(u, "Client calculated u: ");
 
-	if (!calculate_x(x, usr->hash_alg, bytes_s, len_s, usr->username_verifier,
+	if (!calculate_x(x, usr->hash_alg, bytes_s, len_s, usr->ucp_hash))
-			usr->password, usr->password_len))
 		goto cleanup_and_exit;
 
 	srp_dbg_num(x, "Client calculated x: ");

src/util/srp.h

@@ -150,6 +150,8 @@ struct SRPUser *srp_user_new(SRP_HashAlgorithm alg, SRP_NGType ng_type,
 
 void srp_user_delete(struct SRPUser *usr);
 
+void srp_user_clear_sessiondata(struct SRPUser *usr);
+
 int srp_user_is_authenticated(struct SRPUser *usr);
 
 const char *srp_user_get_username(struct SRPUser *usr);