Remove nonce and double hashing as it's no longer needed.

This commit is contained in:
Pentarctagon 2021-06-01 23:37:47 -05:00 committed by Pentarctagon
parent ce2eba73ac
commit e7a4869c4a
8 changed files with 25 additions and 107 deletions

View file

@ -444,8 +444,6 @@ std::unique_ptr<wesnothd_connection> mp_manager::open_connection(std::string hos
error_message = VGETTEXT("The nickname $nick is registered on this server.", i18n_symbols)
+ "\n\n" + _("WARNING: There is already a client using this nickname, "
"logging in will cause that client to be kicked!");
} else if(ec == MP_NO_SEED_ERROR) {
error_message = _("Error in the login procedure (the server had no seed for your connection).");
} else if(ec == MP_INCORRECT_PASSWORD_ERROR) {
error_message = _("The password you provided was incorrect.");
} else if(ec == MP_TOO_MANY_ATTEMPTS_ERROR) {

View file

@ -32,7 +32,6 @@
#define MP_PASSWORD_REQUEST "200"
#define MP_PASSWORD_REQUEST_FOR_LOGGED_IN_NAME "201"
#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"

View file

@ -49,31 +49,21 @@ fuh::fuh(const config& c)
}
}
bool fuh::login(const std::string& name, const std::string& password, const std::string& nonce) {
bool fuh::login(const std::string& name, const std::string& password) {
// Retrieve users' password as hash
std::string hash;
try {
hash = get_hashed_password_from_db(name);
std::string hash = get_hashed_password_from_db(name);
if(utils::md5::is_valid_hash(hash) || utils::bcrypt::is_valid_prefix(hash)) { // md5 hash
return password == hash;
} else {
ERR_UH << "Invalid hash for user '" << name << "'" << std::endl;
return false;
}
} catch (const error& e) {
ERR_UH << "Could not retrieve hash for user '" << name << "' :" << e.message << std::endl;
return false;
}
std::string valid_hash;
if(utils::md5::is_valid_hash(hash)) { // md5 hash
valid_hash = utils::md5(hash.substr(12,34), nonce).base64_digest();
} else if(utils::bcrypt::is_valid_prefix(hash)) { // bcrypt hash
valid_hash = utils::md5(hash, nonce).base64_digest();
} else {
ERR_UH << "Invalid hash for user '" << name << "'" << std::endl;
return false;
}
if(password == valid_hash) return true;
return false;
}
std::string fuh::extract_salt(const std::string& name) {

View file

@ -37,11 +37,10 @@ public:
*
* @param name The username used to login.
* @param password The hashed password sent by the client.
* @param nonce The nonce created for this login attempt.
* @see server::send_password_request().
* @return Whether the hashed password sent by the client matches the hash retrieved from the phpbb database.
*/
bool login(const std::string& name, const std::string& password, const std::string& nonce);
bool login(const std::string& name, const std::string& password);
/**
* Needed because the hashing algorithm used by phpbb requires some info

View file

@ -20,7 +20,6 @@
#include "serialization/parser.hpp"
#include "serialization/base64.hpp"
#include "filesystem.hpp"
#include "random.hpp"
#ifdef HAVE_CONFIG_H
#include "config.h"
@ -49,12 +48,6 @@
#include <sstream>
#include <string>
#ifndef __APPLE__
#include <openssl/rand.h>
#else
#include <cstdlib>
#endif
static lg::log_domain log_server("server");
#define ERR_SERVER LOG_STREAM(err, log_server)
@ -586,56 +579,15 @@ void server_base::load_tls_config(const config& cfg)
if(!cfg["tls_dh"].str().empty()) tls_context_.use_tmp_dh_file(cfg["tls_dh"].str());
}
std::string server_base::create_unsecure_nonce(int length)
std::string server_base::hash_password(const std::string& pw, const std::string& salt, const std::string& username)
{
srand(static_cast<unsigned>(std::time(nullptr)));
std::stringstream ss;
for(int i = 0; i < length; i++) {
ss << randomness::rng::default_instance().get_random_int(0, 9);
if(salt.length() < 12) {
ERR_SERVER << "Bad salt found for user: " << username << std::endl;
return "";
}
return ss.str();
}
#ifndef __APPLE__
namespace
{
class RAND_bytes_exception : public std::exception
{
};
} // namespace
#endif
std::string server_base::create_secure_nonce()
{
// Must be full base64 encodings (3 bytes = 4 chars) else we skew the PRNG results
std::array<unsigned char, (3 * 32) / 4> buf;
#ifndef __APPLE__
if(!RAND_bytes(buf.data(), buf.size())) {
throw RAND_bytes_exception();
}
#else
arc4random_buf(buf.data(), buf.size());
#endif
return base64::encode({buf.data(), buf.size()});
}
std::pair<std::string, std::string> server_base::hash_password(const std::string& pw, const std::string& plain_salt, const std::string& username)
{
std::string password = pw;
// if using crypt_blowfish hashing, create a properly secure nonce
// otherwise if using MD5 hashing, create an insecure nonce
std::string nonce{(plain_salt.length() > 1 && plain_salt[1] == '2')
? create_secure_nonce()
: create_unsecure_nonce()};
std::string salt = plain_salt + 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.
@ -653,36 +605,21 @@ std::pair<std::string, std::string> server_base::hash_password(const std::string
password.replace(pos, 1, "&gt;");
}
if(salt.length() < 12) {
ERR_SERVER << "Bad salt found for user: " << username << 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);
std::string hash = utils::md5(password, utils::md5::get_salt(salt), utils::md5::get_iteration_count(salt)).base64_digest();
return salt+hash;
} 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
);
return hash.base64_digest();
} catch(const utils::hash_error& err) {
ERR_SERVER << "bcrypt hash failed for user " << username << ": " << err.what() << std::endl;
return std::make_pair("", "");
return "";
}
} else {
ERR_SERVER << "Unable to determine how to hash the password for user: " << username << std::endl;
return std::make_pair("", "");
return "";
}
}

View file

@ -136,11 +136,11 @@ public:
* 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 plain_salt The salt as retrieved from the forum database.
* @param salt The salt as retrieved from the forum database.
* @param username The player attempting to log in.
* @return The hashed password, or empty if the password couldn't be hashed.
*/
std::pair<std::string, std::string> hash_password(const std::string& pw, const std::string& plain_salt, const std::string& username);
std::string hash_password(const std::string& pw, const std::string& salt, const std::string& username);
protected:
unsigned short port_;

View file

@ -60,7 +60,7 @@ public:
* Currently the login procedure in the server and client code is hardcoded
* for the forum_user_handler implementation
*/
virtual bool login(const std::string& name, const std::string& password, const std::string& seed) = 0;
virtual bool login(const std::string& name, const std::string& password) = 0;
/** Executed when the user with the given name logged in. */
virtual void user_logged_in(const std::string& name) = 0;

View file

@ -905,7 +905,7 @@ template<class SocketPtr> bool server::authenticate(
"may fix this problem.");
return false;
}
const auto [hashed_password, nonce] = hash_password(password, salt, username);
const std::string hashed_password = hash_password(password, salt, username);
// This name is registered and no password provided
if(password.empty()) {
@ -923,19 +923,14 @@ template<class SocketPtr> bool server::authenticate(
return false;
}
// 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()) {
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, hashed_password, nonce))) {
else if(!(user_handler_->login(username, hashed_password))) {
const std::time_t now = std::time(nullptr);
login_log login_ip { client_address(socket), 0, now };