diff --git a/games/devtest/mods/util_commands/init.lua b/games/devtest/mods/util_commands/init.lua index a413e7fa78584..17b76f61619ed 100644 --- a/games/devtest/mods/util_commands/init.lua +++ b/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 +}) diff --git a/src/client/CMakeLists.txt b/src/client/CMakeLists.txt index c17148a355f4e..43e71921d8dd0 100644 --- a/src/client/CMakeLists.txt +++ b/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 diff --git a/src/client/client.cpp b/src/client/client.cpp index 7a370b34df2ab..23c94cae8c938 100644 --- a/src/client/client.cpp +++ b/src/client/client.cpp @@ -85,8 +85,9 @@ void PacketCounter::print(std::ostream &o) const */ Client::Client( - const char *playername, - const std::string &password, + const std::string &playername, + //const std::string &password, + ClientAuth *auth, MapDrawControl &control, IWritableTextureSource *tsrc, IWritableShaderSource *shsrc, @@ -113,14 +114,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 = @@ -1104,20 +1105,7 @@ void Client::interact(InteractAction action, const PointedThing& pointed) void Client::deleteAuthData() { - if (!m_auth_data) - 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_auth->clearSessionData(); m_chosen_auth_mech = AUTH_MECHANISM_NONE; } @@ -1156,36 +1144,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; - std::string salt; - generate_srp_verifier_and_salt(playername, m_password, - &verifier, &salt); + const std::string &verifier = m_auth->getSrpVerifier(); + const std::string &salt = m_auth->getSrpSalt(); 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, - (unsigned char **) &bytes_A, &len_A); + auth_data, NULL, NULL, 0, + reinterpret_cast(&bytes_A), &len_A); FATAL_ERROR_IF(res != SRP_OK, "Creating local SRP user failed."); NetworkPacket resp_pkt(TOSERVER_SRP_BYTES_A, 0); @@ -1337,16 +1323,25 @@ void Client::clearOutChatQueue() m_out_chat_queue = std::queue(); } -void Client::sendChangePassword(const std::string &oldpassword, - const std::string &newpassword) +void Client::sendChangePassword(std::string &oldpassword, + 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; - m_new_password = newpassword; + std::string playername = m_env.getLocalPlayer()->getName(); + 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)); } diff --git a/src/client/client.h b/src/client/client.h index a02a7d8c6481e..a3946e18c09b5 100644 --- a/src/client/client.h +++ b/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" @@ -108,8 +110,9 @@ class Client : public con::PeerHandler, public InventoryManager, public IGameDef */ Client( - const char *playername, - const std::string &password, + const std::string &playername, + //const std::string &password, + ClientAuth *auth, MapDrawControl &control, IWritableTextureSource *tsrc, IWritableShaderSource *shsrc, @@ -231,8 +234,8 @@ class Client : public con::PeerHandler, public InventoryManager, public IGameDef void sendInventoryAction(InventoryAction *a); void sendChatMessage(const std::wstring &message); void clearOutChatQueue(); - void sendChangePassword(const std::string &oldpassword, - const std::string &newpassword); + void sendChangePassword(std::string &oldpassword, + std::string &newpassword); void sendDamage(u16 damage); void sendRespawnLegacy(); void sendReady(); @@ -515,12 +518,11 @@ class Client : public con::PeerHandler, public InventoryManager, public IGameDef // 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; diff --git a/src/client/clientauth.cpp b/src/client/clientauth.cpp new file mode 100644 index 0000000000000..e1a0a29522a90 --- /dev/null +++ b/src/client/clientauth.cpp @@ -0,0 +1,104 @@ +// Luanti +// SPDX-License-Identifier: LGPL-2.1-or-later +// Copyright (C) 2024-2025 SFENCE, + +#include "clientauth.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(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(translated.c_str()), + translated.length(), NULL, NULL); +} + +SRPUser * ClientAuth::getAuthData(AuthMechanism chosen_auth_mech) const +{ + switch (chosen_auth_mech) { + case AUTH_MECHANISM_LEGACY_PASSWORD: + return m_legacy_auth_data; + case AUTH_MECHANISM_SRP: + return m_srp_auth_data; + default: + return nullptr; + } +} + +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(); +} diff --git a/src/client/clientauth.h b/src/client/clientauth.h new file mode 100644 index 0000000000000..774f43b281bd6 --- /dev/null +++ b/src/client/clientauth.h @@ -0,0 +1,59 @@ +/* +Minetest +Copyright (C) 2024 SFENCE, + +This program is free software; you can redistribute it and/or modify +it under the terms of the GNU Lesser General Public License as published by +the Free Software Foundation; either version 2.1 of the License, or +(at your option) any later version. + +This program is distributed in the hope that it will be useful, +but WITHOUT ANY WARRANTY; without even the implied warranty of +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +GNU Lesser General Public License for more details. + +You should have received a copy of the GNU Lesser General Public License along +with this program; if not, write to the Free Software Foundation, Inc., +51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. +*/ + +#pragma once + +#include +#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; +}; diff --git a/src/client/clientgamestartdata.h b/src/client/clientgamestartdata.h new file mode 100644 index 0000000000000..6b397d02b0628 --- /dev/null +++ b/src/client/clientgamestartdata.h @@ -0,0 +1,49 @@ +/* +Minetest +Copyright (C) 2024 SFENCE, + +This program is free software; you can redistribute it and/or modify +it under the terms of the GNU Lesser General Public License as published by +the Free Software Foundation; either version 2.1 of the License, or +(at your option) any later version. + +This program is distributed in the hope that it will be useful, +but WITHOUT ANY WARRANTY; without even the implied warranty of +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +GNU Lesser General Public License for more details. + +You should have received a copy of the GNU Lesser General Public License along +with this program; if not, write to the Free Software Foundation, Inc., +51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. +*/ + +#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; +}; diff --git a/src/client/clientlauncher.cpp b/src/client/clientlauncher.cpp index 725ff4323a3db..fc1120fcec63b 100644 --- a/src/client/clientlauncher.cpp +++ b/src/client/clientlauncher.cpp @@ -88,7 +88,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(); @@ -180,7 +181,7 @@ bool ClientLauncher::run(GameStartData &start_data, const Settings &cmd_args) core::rect(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; @@ -204,7 +205,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 @@ -259,7 +260,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"); @@ -383,20 +384,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: ") @@ -425,7 +428,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; @@ -457,12 +460,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 { @@ -470,24 +476,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); diff --git a/src/client/clientlauncher.h b/src/client/clientlauncher.h index 21e459db720ff..3f2cf56e9db88 100644 --- a/src/client/clientlauncher.h +++ b/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 @@ class ClientLauncher 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 @@ class ClientLauncher 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); diff --git a/src/client/game.cpp b/src/client/game.cpp index 00808803f6dd1..77ee419b0ea04 100644 --- a/src/client/game.cpp +++ b/src/client/game.cpp @@ -31,6 +31,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" @@ -494,7 +496,7 @@ class Game { bool startup(bool *kill, InputHandler *input, RenderingEngine *rendering_engine, - const GameStartData &game_params, + ClientGameStartData &game_params, std::string &error_message, bool *reconnect, ChatBackend *chat_backend); @@ -513,11 +515,11 @@ class Game { 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); @@ -862,7 +864,7 @@ Game::~Game() bool Game::startup(bool *kill, InputHandler *input, RenderingEngine *rendering_engine, - const GameStartData &start_data, + ClientGameStartData &start_data, std::string &error_message, bool *reconnect, ChatBackend *chat_backend) @@ -1252,7 +1254,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); @@ -1382,7 +1384,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 @@ -1439,8 +1441,8 @@ bool Game::connectToServer(const GameStartData &start_data, try { - client = new Client(start_data.name.c_str(), - start_data.password, + client = new Client(start_data.name, + &start_data.auth, *draw_control, texture_src, shader_src, itemdef_manager, nodedef_manager, sound_manager.get(), eventmgr, m_rendering_engine, @@ -4137,7 +4139,7 @@ void Game::readSettings() void the_game(bool *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 diff --git a/src/client/game.h b/src/client/game.h index 6b838e2bcaeb3..27403d64caf72 100644 --- a/src/client/game.h +++ b/src/client/game.h @@ -7,6 +7,7 @@ #include "irrlichttypes.h" #include "config.h" #include +#include "client/clientgamestartdata.h" #if !IS_CLIENT_BUILD #error Do not include in server builds @@ -39,7 +40,7 @@ struct CameraOrientation { void the_game(bool *kill, InputHandler *input, RenderingEngine *rendering_engine, - const GameStartData &start_data, + ClientGameStartData &start_data, std::string &error_message, ChatBackend &chat_backend, bool *reconnect_requested); diff --git a/src/gameparams.h b/src/gameparams.h index 7c20cccf32d00..1a3161e3ebbad 100644 --- a/src/gameparams.h +++ b/src/gameparams.h @@ -6,6 +6,7 @@ #include "irrlichttypes.h" #include "content/subgames.h" +#include "porting.h" // Information provided from "main" struct GameParams @@ -32,7 +33,6 @@ struct GameStartData : GameParams bool isSinglePlayer() const { return address.empty() && !local_server; } std::string name; - std::string password; std::string address; bool local_server; diff --git a/src/gui/guiPasswordChange.cpp b/src/gui/guiPasswordChange.cpp index a9b73dfe8324a..e7be11aa8f463 100644 --- a/src/gui/guiPasswordChange.cpp +++ b/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; } diff --git a/src/network/clientpackethandler.cpp b/src/network/clientpackethandler.cpp index 1024988bdceea..56b7f8aaa34c6 100644 --- a/src/network/clientpackethandler.cpp +++ b/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_data = 0; + m_auth->clear(); } } @@ -164,9 +163,7 @@ void Client::handleCommand_AuthAccept(NetworkPacket* pkt) void Client::handleCommand_AcceptSudoMode(NetworkPacket* pkt) { - deleteAuthData(); - - m_password = m_new_password; + *m_auth = std::move(m_new_auth); verbosestream << "Client: Received TOCLIENT_ACCEPT_SUDO_MODE." << std::endl; @@ -192,6 +189,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) { // Legacy code from 0.4.12 and older but is still used // in some places of the server code @@ -1567,16 +1566,16 @@ void Client::handleCommand_SrpBytesSandB(NetworkPacket* pkt) char *bytes_M = 0; size_t len_M = 0; - SRPUser *usr = (SRPUser *) m_auth_data; + SRPUser *usr = m_auth->getAuthData(m_chosen_auth_mech); 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(), - (const unsigned char *) B.c_str(), B.size(), - (unsigned char **) &bytes_M, &len_M); + srp_user_process_challenge(usr, reinterpret_cast(s.c_str()), s.size(), + reinterpret_cast(B.c_str()), B.size(), + reinterpret_cast(&bytes_M), &len_M); if ( !bytes_M ) { errorstream << "Client: SRP-6a S_B safety check violation!" << std::endl; diff --git a/src/porting.cpp b/src/porting.cpp index faef75b7c938b..1cd03fb2f8476 100644 --- a/src/porting.cpp +++ b/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__) @@ -970,4 +973,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 diff --git a/src/porting.h b/src/porting.h index 4963916f0f6bb..c0bad887d7998 100644 --- a/src/porting.h +++ b/src/porting.h @@ -342,6 +342,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__ diff --git a/src/util/srp.cpp b/src/util/srp.cpp index b1dfa76a4d13b..0f4186a4f2ccb 100644 --- a/src/util/srp.cpp +++ b/src/util/srp.cpp @@ -232,8 +232,7 @@ struct SRPUser { char *username; char *username_verifier; - unsigned char *password; - size_t password_len; + unsigned char ucp_hash[CSRP_MAX_HASH]; 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, - size_t salt_len, const char *username, const unsigned char *password, - size_t password_len) +static void precalculate_x(SRP_HashAlgorithm alg, const char *username, + const unsigned char *password, size_t password_len, unsigned char *ucp_hash) { - 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 @@ struct SRPUser *srp_user_new(SRP_HashAlgorithm alg, SRP_NGType ng_type, 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, - usr->password, usr->password_len)) + if (!calculate_x(x, usr->hash_alg, bytes_s, len_s, usr->ucp_hash)) goto cleanup_and_exit; srp_dbg_num(x, "Client calculated x: "); diff --git a/src/util/srp.h b/src/util/srp.h index fc4d2dc89b736..93ce453a2c8d5 100644 --- a/src/util/srp.h +++ b/src/util/srp.h @@ -148,6 +148,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);