Apply review feedback.

src/client/client.cpp

@@ -95,7 +95,7 @@ void PacketCounter::print(std::ostream &o) const
 */
 
 Client::Client(
-		const char *playername,
+		const std::string &playername,
 		const std::string &password,
 		MapDrawControl &control,
 		IWritableTextureSource *tsrc,
@@ -124,15 +124,15 @@ 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_auth(std::string(playername), password),
+	m_auth(playername, password),
 	m_chosen_auth_mech(AUTH_MECHANISM_NONE),
 	m_media_downloader(new ClientMediaDownloader()),
 	m_state(LC_Created),
 	m_game_ui(game_ui),
 	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 =

src/client/client.h

@@ -121,7 +121,7 @@ class Client : public con::PeerHandler, public InventoryManager, public IGameDef
 	*/
 
 	Client(
-			const char *playername,
+			const std::string &playername,
 			const std::string &password,
 			MapDrawControl &control,
 			IWritableTextureSource *tsrc,

src/client/clientauth.cpp

@@ -38,7 +38,7 @@ ClientAuth::~ClientAuth()
 	clear();
 }
 
-void ClientAuth::moveFrom(ClientAuth& other)
+ClientAuth &ClientAuth::operator=(ClientAuth &&other)
 {
 	clear();
 
@@ -52,6 +52,10 @@ void ClientAuth::moveFrom(ClientAuth& other)
 
 	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)
@@ -90,11 +94,11 @@ void * ClientAuth::getAuthData(AuthMechanism chosen_auth_mech) const
 void ClientAuth::clear()
 {
 	if (m_legacy_auth_data != nullptr) {
-		srp_user_delete(static_cast<SRPUser *>(m_legacy_auth_data));
+		srp_user_delete(m_legacy_auth_data);
 		m_legacy_auth_data = nullptr;
 	}
 	if (m_srp_auth_data != nullptr) {
-		srp_user_delete(static_cast<SRPUser *>(m_srp_auth_data));
+		srp_user_delete(m_srp_auth_data);
 		m_srp_auth_data = nullptr;
 	}
 	m_srp_verifier.clear();

src/client/clientauth.h

@@ -23,6 +23,8 @@ with this program; if not, write to the Free Software Foundation, Inc.,
 #include "network/networkprotocol.h"
 #include "util/basic_macros.h"
 
+struct SRPUser;
+
 class ClientAuth
 {
 public:
@@ -32,7 +34,8 @@ class ClientAuth
 	~ClientAuth();
 	DISABLE_CLASS_COPY(ClientAuth);
 
-	void moveFrom(ClientAuth& other);
+	ClientAuth(ClientAuth &&other) { *this = std::move(other); }
+	ClientAuth &operator=(ClientAuth &&other);
 
 	void applyPassword(const std::string &player_name, const std::string &password);
 
@@ -50,6 +53,6 @@ class ClientAuth
 	std::string m_srp_verifier;
 	std::string m_srp_salt;
 
-	void *m_legacy_auth_data = nullptr;
-	void *m_srp_auth_data = nullptr;
+	SRPUser *m_legacy_auth_data = nullptr;
+	SRPUser *m_srp_auth_data = nullptr;
 };

src/client/game.cpp

@@ -1667,7 +1667,7 @@ 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,
 				*draw_control, texture_src, shader_src,
 				itemdef_manager, nodedef_manager, sound_manager.get(), eventmgr,

src/network/clientpackethandler.cpp

@@ -165,10 +165,7 @@ void Client::handleCommand_AuthAccept(NetworkPacket* pkt)
 
 void Client::handleCommand_AcceptSudoMode(NetworkPacket* pkt)
 {
-	deleteAuthData();
-
-	m_auth.moveFrom(m_new_auth);
-	m_new_auth.clear();
+	m_auth = std::move(m_new_auth);
 
 	verbosestream << "Client: Received TOCLIENT_ACCEPT_SUDO_MODE." << std::endl;
 

src/util/srp.cpp

@@ -249,7 +249,8 @@ struct SRPUser {
 
 	char *username;
 	char *username_verifier;
-	unsigned char ucp_hash[SHA512_DIGEST_LENGTH];
+	unsigned char *password;
+	size_t password_len;
 
 	unsigned char M[SHA512_DIGEST_LENGTH];
 	unsigned char H_AMK[SHA512_DIGEST_LENGTH];
@@ -429,9 +430,11 @@ static SRP_Result H_ns(mpz_t result, SRP_HashAlgorithm alg, const unsigned char
 	return SRP_OK;
 }
 
-static void precalculate_x(SRP_HashAlgorithm alg, const char *username,
-	const unsigned char *password, size_t password_len, unsigned char *ucp_hash)
+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)
 {
+	unsigned char ucp_hash[SHA512_DIGEST_LENGTH];
 	HashCTX ctx;
 	hash_init(alg, &ctx);
 
@@ -442,11 +445,8 @@ static void precalculate_x(SRP_HashAlgorithm alg, const char *username,
 	hash_update(alg, &ctx, password, password_len);
 
 	hash_final(alg, &ctx, ucp_hash);
-}
-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));
+
+	return H_ns(result, alg, salt, salt_len, ucp_hash, hash_length(alg));
 }
 
 static SRP_Result update_hash_n(SRP_HashAlgorithm alg, HashCTX *ctx, const mpz_t n)
@@ -612,11 +612,8 @@ SRP_Result srp_create_salted_verification_key( SRP_HashAlgorithm alg,
 		g_rand_idx += size_to_fill;
 	}
 
-	unsigned char ucp_hash[SHA512_DIGEST_LENGTH];
-	precalculate_x(alg, username_for_verifier,
-			password, len_password, ucp_hash);
 	if (!calculate_x(
-			x, alg, *bytes_s, *len_s, ucp_hash))
+			x, alg, *bytes_s, *len_s, username_for_verifier, password, len_password))
 		goto error_and_exit;
 
 	srp_dbg_num(x, "Server calculated x: ");
@@ -839,13 +836,14 @@ struct SRPUser *srp_user_new(SRP_HashAlgorithm alg, SRP_NGType ng_type,
 
 	usr->username = (char *)srp_alloc(ulen);
 	usr->username_verifier = (char *)srp_alloc(uvlen);
+	usr->password = (unsigned char *)srp_alloc(len_password);
+	usr->password_len = len_password;
 
-	if (!usr->username || !usr->username_verifier) goto err_exit;
+	if (!usr->username || !usr->password || !usr->username_verifier) goto err_exit;
 
 	memcpy(usr->username, username, ulen);
 	memcpy(usr->username_verifier, username_for_verifier, uvlen);
-
-	precalculate_x(alg, username_for_verifier, bytes_password, len_password, usr->ucp_hash);
+	memcpy(usr->password, bytes_password, len_password);
 
 	usr->authenticated = 0;
 
@@ -861,6 +859,10 @@ struct SRPUser *srp_user_new(SRP_HashAlgorithm alg, SRP_NGType ng_type,
 		delete_ng(usr->ng);
 		srp_free(usr->username);
 		srp_free(usr->username_verifier);
+		if (usr->password) {
+			memset(usr->password, 0, usr->password_len);
+			srp_free(usr->password);
+		}
 		srp_free(usr);
 	}
 
@@ -876,8 +878,11 @@ void srp_user_delete(struct SRPUser *usr)
 
 		delete_ng(usr->ng);
 
+		memset(usr->password, 0, usr->password_len);
+
 		srp_free(usr->username);
 		srp_free(usr->username_verifier);
+		srp_free(usr->password);
 
 		if (usr->bytes_A) srp_free(usr->bytes_A);
 
@@ -962,7 +967,8 @@ 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->ucp_hash))
+	if (!calculate_x(x, usr->hash_alg, bytes_s, len_s, usr->username_verifier,
+			usr->password, usr->password_len))
 		goto cleanup_and_exit;
 
 	srp_dbg_num(x, "Client calculated x: ");