0ct0pu5/wesnoth
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Hash passwords server-side instead of client-side.

Browse source 
This removes the need for the Wesnoth client to need to know how to hash the password to match what is expected by the forum database.
This commit is contained in:
Pentarctagon 2021-05-19 19:48:24 -05:00 • committed by Pentarctagon
parent 4285a339eb
commit ac39ffdcd3
9 changed files with 142 additions and 105 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
src/commandline_options.cpp
View file

@ -82,6 +82,7 @@ commandline_options::commandline_options(const std::vector<std::string>& args)
, debug(false)
, debug_lua(false)
, strict_lua(false)
, allow_insecure(false)
#ifdef DEBUG_WINDOW_LAYOUT_GRAPHS
, debug_dot_domain()
, debug_dot_level()
@ -185,6 +186,7 @@ commandline_options::commandline_options(const std::vector<std::string>& args)
("debug,d", "enables additional command mode options in-game.")
("debug-lua", "enables some Lua debugging mechanisms")
("strict-lua", "disallow deprecated Lua API calls")
("allow-insecure", "Allows sending a plaintext password over an unencrypted connection. Should only ever be used for local testing.")
#ifdef DEBUG_WINDOW_LAYOUT_GRAPHS
("debug-dot-level", po::value<std::string>(), "sets the level of the debug dot files. <arg> should be a comma separated list of levels. These files are used for debugging the widgets especially the for the layout engine. When enabled the engine will produce dot files which can be converted to images with the dot tool. Available levels: size (generate the size info of the widget), state (generate the state info of the widget).")
("debug-dot-domain", po::value<std::string>(), "sets the domain of the debug dot files. <arg> should be a comma separated list of domains. See --debug-dot-level for more info. Available domains: show (generate the data when the dialog is about to be shown), layout (generate the data during the layout phase - might result in multiple files). The data can also be generated when the F12 is pressed in a dialog.")
@ -350,6 +352,8 @@ commandline_options::commandline_options(const std::vector<std::string>& args)
debug_lua = true;
if (vm.count("strict-lua"))
strict_lua = true;
if (vm.count("allow-insecure"))
allow_insecure = true;
#ifdef DEBUG_WINDOW_LAYOUT_GRAPHS
if (vm.count("debug-dot-domain")) {
debug_dot_domain = vm["debug-dot-domain"].as<std::string>();

6
src/commandline_options.hpp
View file

@ -76,6 +76,12 @@ public:
bool debug_lua;
/** True if --strict-lua was given in the commandline. Disallows use of deprecated APIs. */
bool strict_lua;
/**
* True if --allow-insecure was given in the commandline.
* Allows sending a plaintext password over an unencrypted connection.
* Should only ever be used for local testing.
*/
bool allow_insecure;
#ifdef DEBUG_WINDOW_LAYOUT_GRAPHS
/** Non-empty if --debug-dot-domain was given on the command line. */
std::optional<std::string> debug_dot_domain;

2
src/game_config.cpp
View file

@ -92,6 +92,8 @@ const std::size_t max_loop = 65536;
std::vector<server_info> server_list;
bool allow_insecure = false;
//
// Gamestate flags
//

2
src/game_config.hpp
View file

@ -62,6 +62,8 @@ namespace game_config
extern bool debug_lua, strict_lua, editor, ignore_replay_errors, mp_debug,
exit_at_end, no_delay, disable_autosave, no_addons;
extern bool allow_insecure;
extern const bool& debug;
void set_debug(bool new_debug);

69
src/game_initialization/multiplayer.cpp
View file

@ -31,7 +31,6 @@
#include "gui/dialogs/multiplayer/mp_join_game.hpp"
#include "gui/dialogs/multiplayer/mp_login.hpp"
#include "gui/dialogs/multiplayer/mp_staging.hpp"
#include "hash.hpp"
#include "log.hpp"
#include "map_settings.hpp"
#include "multiplayer_error_codes.hpp"
@ -361,58 +360,20 @@ std::unique_ptr<wesnothd_connection> mp_manager::open_connection(std::string hos
? (gui2::show_message(_("Confirm"), (*error)["message"], gui2::dialogs::message::ok_cancel_buttons) == gui2::retval::CANCEL)
: false;
const bool is_pw_request = !((*error)["password_request"].empty()) && !(password.empty());
// If:
// * the server asked for a password
// * the password isn't empty
// * the user didn't press Cancel
// * the connection is secure or the client was started with the option to use insecure connections
// send the password to the server
// otherwise go directly to the username/password dialog
if(!(*error)["password_request"].empty() && !password.empty() && !fall_through && (conn->using_tls() || game_config::allow_insecure)) {
// the possible cases here are that either:
// 1) TLS encryption is enabled, thus sending the plaintext password is still secure
// 2) TLS encryption is not enabled, in which case the server should not be requesting a password in the first place
// 3) This is being used for local testing/development, so using an insecure connection is enabled manually
// If the server asks for a password, provide one if we can
// or request a password reminder.
// Otherwise or if the user pressed 'cancel' in the confirmation dialog
// above go directly to the username/password dialog
if(is_pw_request && !fall_through) {
if((*error)["phpbb_encryption"].to_bool()) {
// Apparently HTML key-characters are passed to the hashing functions of phpbb in this escaped form.
// I will do closer investigations on this, for now let's just hope these are all of them.
// Note: we must obviously replace '&' first, I wasted some time before I figured that out... :)
for(std::string::size_type pos = 0; (pos = password.find('&', pos)) != std::string::npos; ++pos)
password.replace(pos, 1, "&amp;");
for(std::string::size_type pos = 0; (pos = password.find('\"', pos)) != std::string::npos; ++pos)
password.replace(pos, 1, "&quot;");
for(std::string::size_type pos = 0; (pos = password.find('<', pos)) != std::string::npos; ++pos)
password.replace(pos, 1, "&lt;");
for(std::string::size_type pos = 0; (pos = password.find('>', pos)) != std::string::npos; ++pos)
password.replace(pos, 1, "&gt;");
const std::string salt = (*error)["salt"];
if(salt.length() < 12) {
throw wesnothd_error(_("Bad data received from server"));
}
if(utils::md5::is_valid_prefix(salt)) {
sp["password"] = utils::md5(
utils::md5(password, utils::md5::get_salt(salt), utils::md5::get_iteration_count(salt)).base64_digest(),
salt.substr(12, 8)
).base64_digest();
} else if(utils::bcrypt::is_valid_prefix(salt)) {
try {
auto bcrypt_salt = utils::bcrypt::from_salted_salt(salt);
auto hash = utils::bcrypt::hash_pw(password, bcrypt_salt);
const std::string outer_salt = salt.substr(bcrypt_salt.iteration_count_delim_pos + 23);
if(outer_salt.size() != 32) {
throw utils::hash_error("salt wrong size");
}
sp["password"] = utils::md5(hash.base64_digest(), outer_salt).base64_digest();
} catch(const utils::hash_error& err) {
ERR_MP << "bcrypt hash failed: " << err.what() << std::endl;
throw wesnothd_error(_("Bad data received from server"));
}
} else {
throw wesnothd_error(_("Bad data received from server"));
}
} else {
sp["password"] = password;
}
// Once again send our request...
conn->send_data(response);
@ -442,7 +403,9 @@ std::unique_ptr<wesnothd_connection> mp_manager::open_connection(std::string hos
const std::string ec = (*error)["error_code"];
if(ec == MP_MUST_LOGIN) {
if(!(*error)["password_request"].empty() && !conn->using_tls() && !game_config::allow_insecure) {
error_message = _("The remote server requested a password while using an insecure connection.");
} else if(ec == MP_MUST_LOGIN) {
error_message = _("You must login first.");
} else if(ec == MP_NAME_TAKEN_ERROR) {
error_message = VGETTEXT("The nickname ‘$nick’ is already taken.", i18n_symbols);
@ -487,6 +450,8 @@ std::unique_ptr<wesnothd_connection> mp_manager::open_connection(std::string hos
error_message = _("The password you provided was incorrect.");
} else if(ec == MP_TOO_MANY_ATTEMPTS_ERROR) {
error_message = _("You have made too many login attempts.");
} else if(ec == MP_HASHING_PASSWORD_FAILED) {
error_message = _("Password hashing failed.");
} else {
error_message = (*error)["message"].str();
}

1
src/multiplayer_error_codes.hpp
View file

@ -35,3 +35,4 @@
#define MP_NO_SEED_ERROR "202"
#define MP_INCORRECT_PASSWORD_ERROR "203"
#define MP_TOO_MANY_ATTEMPTS_ERROR "204"
#define MP_HASHING_PASSWORD_FAILED "205"

114
src/server/wesnothd/server.cpp
View file

@ -22,13 +22,13 @@
#include "config.hpp"
#include "filesystem.hpp"
#include "game_config.hpp"
#include "hash.hpp"
#include "log.hpp"
#include "multiplayer_error_codes.hpp"
#include "serialization/parser.hpp"
#include "serialization/preprocessor.hpp"
#include "serialization/string_utils.hpp"
#include "serialization/unicode.hpp"
#include <functional>
#include "utils/general.hpp"
#include "utils/iterable_pair.hpp"
#include "game_version.hpp"
@ -53,12 +53,14 @@
#include <cerrno>
#include <csignal>
#include <cstdlib>
#include <functional>
#include <iomanip>
#include <iostream>
#include <map>
#include <queue>
#include <set>
#include <sstream>
#include <string>
#include <vector>
static lg::log_domain log_server("server");
@ -880,17 +882,14 @@ template<class SocketPtr> bool server::authenticate(
{
// Current login procedure for registered nicks is:
// - Client asks to log in with a particular nick
// - Server sends client random nonce plus some info
// generated from the original hash that is required to
// regenerate the hash
// - Client generates hash for the user provided password
// and mixes it with the received random nonce
// - Server received password hash hashed with the nonce,
// applies the nonce to the valid hash and compares the results
// - Server sends client a password request (if TLS/database support is enabled)
// - Client sends the plaintext password
// - Server receives plaintext password, hashes it, and compares it to the password in the forum database
registered = false;
if(user_handler_) {
const auto [hashed_password, nonce] = hash_password(password, username, socket);
const bool exists = user_handler_->user_exists(username);
// This name is registered but the account is not active
@ -903,34 +902,34 @@ template<class SocketPtr> bool server::authenticate(
// This name is registered and no password provided
if(password.empty()) {
if(!name_taken) {
send_password_request(socket, "The nickname '" + username + "' is registered on this server.",
username, MP_PASSWORD_REQUEST);
send_password_request(socket, "The nickname '" + username + "' is registered on this server.", MP_PASSWORD_REQUEST);
} else {
send_password_request(socket,
"The nickname '" + username + "' is registered on this server."
"\n\nWARNING: There is already a client using this username, "
"logging in will cause that client to be kicked!",
username, MP_PASSWORD_REQUEST_FOR_LOGGED_IN_NAME, true
MP_PASSWORD_REQUEST_FOR_LOGGED_IN_NAME, true
);
}
return false;
}
// A password (or hashed password) was provided, however
// there is no seed
if(seeds_[socket.get()].empty()) {
send_password_request(socket, "Please try again.", username, MP_NO_SEED_ERROR);
// A password was provided, however the generated nonce is empty for some reason
if(nonce.empty()) {
send_password_request(socket, "Please try again.", MP_NO_SEED_ERROR);
return false;
}
// hashing the password failed
// note: this could be due to other related problems other than *just* the hashing step failing
else if(hashed_password.empty()) {
async_send_error(socket, "Password hashing failed.", MP_HASHING_PASSWORD_FAILED);
return false;
}
// This name is registered and an incorrect password provided
else if(!(user_handler_->login(username, password, seeds_[socket.get()]))) {
else if(!(user_handler_->login(username, hashed_password, nonce))) {
const std::time_t now = std::time(nullptr);
// Reset the random seed
seeds_.erase(socket.get());
login_log login_ip { client_address(socket), 0, now };
auto i = std::find(failed_logins_.begin(), failed_logins_.end(), login_ip);
@ -960,7 +959,7 @@ template<class SocketPtr> bool server::authenticate(
async_send_error(socket, "You have made too many failed login attempts.", MP_TOO_MANY_ATTEMPTS_ERROR);
} else {
send_password_request(socket,
"The password you provided for the nickname '" + username + "' was incorrect.", username,
"The password you provided for the nickname '" + username + "' was incorrect.",
MP_INCORRECT_PASSWORD_ERROR);
}
@ -974,7 +973,6 @@ template<class SocketPtr> bool server::authenticate(
registered = true;
// Reset the random seed
seeds_.erase(socket.get());
user_handler_->user_logged_in(username);
}
}
@ -982,38 +980,86 @@ template<class SocketPtr> bool server::authenticate(
return true;
}
template<class SocketPtr> void server::send_password_request(SocketPtr socket,
const std::string& msg,
const std::string& user,
const char* error_code,
bool force_confirmation)
template<class SocketPtr> std::pair<std::string, std::string> server::hash_password(const std::string& pw, const std::string& user, SocketPtr socket)
{
std::string salt = user_handler_->extract_salt(user);
std::string plain_salt = user_handler_->extract_salt(user);
std::string password = pw;
// If using crypt_blowfish, use 32 random Base64 characters, cryptographic-strength, 192 bits entropy
// else (phppass, MD5, $H$), use 8 random integer digits, not secure, do not use, this is crap, 29.8 bits entropy
std::string nonce{(salt[1] == '2')
std::string nonce{(plain_salt[1] == '2')
? user_handler_->create_secure_nonce()
: user_handler_->create_unsecure_nonce()};
std::string password_challenge = salt + nonce;
std::string salt = plain_salt + nonce;
if(salt.empty()) {
async_send_error(socket,
"Even though your nickname is registered on this server you "
"cannot log in due to an error in the hashing algorithm. "
"Logging into your forum account on https://forums.wesnoth.org "
"may fix this problem.");
return;
return std::make_pair("", "");
}
seeds_[socket.get()] = nonce;
// Apparently HTML key-characters are passed to the hashing functions of phpbb in this escaped form.
// I will do closer investigations on this, for now let's just hope these are all of them.
// Note: we must obviously replace '&' first, I wasted some time before I figured that out... :)
for(std::string::size_type pos = 0; (pos = password.find('&', pos)) != std::string::npos; ++pos) {
password.replace(pos, 1, "&amp;");
}
for(std::string::size_type pos = 0; (pos = password.find('\"', pos)) != std::string::npos; ++pos) {
password.replace(pos, 1, "&quot;");
}
for(std::string::size_type pos = 0; (pos = password.find('<', pos)) != std::string::npos; ++pos) {
password.replace(pos, 1, "&lt;");
}
for(std::string::size_type pos = 0; (pos = password.find('>', pos)) != std::string::npos; ++pos) {
password.replace(pos, 1, "&gt;");
}
if(salt.length() < 12) {
ERR_SERVER << "Bad salt found for user: " << user << std::endl;
return std::make_pair("", "");
}
if(utils::md5::is_valid_prefix(salt)) {
std::string md5_1 = utils::md5(password, utils::md5::get_salt(salt), utils::md5::get_iteration_count(salt)).base64_digest();
std::string md5_2 = utils::md5(md5_1, salt.substr(12, 8)).base64_digest();
return std::make_pair(md5_2, nonce);
} else if(utils::bcrypt::is_valid_prefix(salt)) {
try {
auto bcrypt_salt = utils::bcrypt::from_salted_salt(salt);
auto hash = utils::bcrypt::hash_pw(password, bcrypt_salt);
const std::string outer_salt = salt.substr(bcrypt_salt.iteration_count_delim_pos + 23);
if(outer_salt.size() != 32) {
throw utils::hash_error("salt wrong size");
}
return std::make_pair(
utils::md5(hash.base64_digest(), outer_salt).base64_digest(),
nonce
);
} catch(const utils::hash_error& err) {
ERR_SERVER << "bcrypt hash failed: " << err.what() << std::endl;
return std::make_pair("", "");
}
} else {
ERR_SERVER << "Unable to determine how to hash the password for user: " << user << std::endl;
return std::make_pair("", "");
}
}
template<class SocketPtr> void server::send_password_request(SocketPtr socket,
const std::string& msg,
const char* error_code,
bool force_confirmation)
{
simple_wml::document doc;
simple_wml::node& e = doc.root().add_child("error");
e.set_attr_dup("message", msg.c_str());
e.set_attr("password_request", "yes");
e.set_attr("phpbb_encryption", "yes");
e.set_attr_dup("salt", password_challenge.c_str());
e.set_attr("force_confirmation", force_confirmation ? "yes" : "no");
if(*error_code != '\0') {

13
src/server/wesnothd/server.hpp
View file

@ -43,8 +43,7 @@ private:
template<class SocketPtr> void login_client(boost::asio::yield_context yield, SocketPtr socket);
template<class SocketPtr> bool is_login_allowed(SocketPtr socket, const simple_wml::node* const login, const std::string& username, bool& registered, bool& is_moderator);
template<class SocketPtr> bool authenticate(SocketPtr socket, const std::string& username, const std::string& password, bool name_taken, bool& registered);
template<class SocketPtr> void send_password_request(SocketPtr socket, const std::string& msg,
const std::string& user, const char* error_code = "", bool force_confirmation = false);
template<class SocketPtr> void send_password_request(SocketPtr socket, const std::string& msg, const char* error_code = "", bool force_confirmation = false);
bool accepting_connections() const { return !graceful_restart; }
template<class SocketPtr> void handle_player(boost::asio::yield_context yield, SocketPtr socket, const player& player);
@ -59,6 +58,15 @@ private:
void handle_join_game(player_iterator player, simple_wml::node& join);
void disconnect_player(player_iterator player);
void remove_player(player_iterator player);
/**
* Handles hashing the password provided by the player before comparing it to the hashed password in the forum database.
*
* @param pw The plaintext password.
* @param user The player attempting to log in.
* @param socket The socket the player is connected with.
* @return The hashed password, or empty if the password couldn't be hashed.
*/
template<class SocketPtr> std::pair<std::string, std::string> hash_password(const std::string& pw, const std::string& user, SocketPtr socket);
public:
template<class SocketPtr> void send_server_message(SocketPtr socket, const std::string& message, const std::string& type);
@ -115,7 +123,6 @@ private:
std::deque<login_log> failed_logins_;
std::unique_ptr<user_handler> user_handler_;
std::map<void*, std::string> seeds_;
std::mt19937 die_;

4
src/wesnoth.cpp
View file

@ -429,6 +429,10 @@ static int process_command_args(const commandline_options& cmdline_opts)
game_config::debug_lua = true;
}
if(cmdline_opts.allow_insecure) {
game_config::allow_insecure = true;
}
if(cmdline_opts.strict_lua) {
game_config::strict_lua = true;
}