Merge lp:~widelands-dev/widelands/net-pwd-security into lp:widelands

[Notabilis]

Increasing password security by no longer storing and transmitting it in plaintext.

Increasing password security by using CHAP for transmitting the password instead of sending it in plaintext. With CHAP, a challenge (random value) is send to the connecting client and the client responds by sending back the cryptographic hash of the challenge concatenated with its secret password. A cryptographic hash function converts an input to a fixed length output while it is practically impossible to convert the output back to the input. Since the server also knows the password, it can also calculate the hash and can check the hashes whether they are equal. An attacker which captures the send hash can not do anything with it: Extracting the password is not possible and sending the hash to the server does not work since the server will send another challenge on next connect.

To reduce login time, moved TELL_IP functionality for transmitting the IPv4 address to the metaserver to an own thread.

Passwords are now stored as a cryptographic hash in the configuration file. For now, the plaintext password is left in the configuration file if it exists to allow parallel usage of build19 and trunk builds.

Also, removed "max. number of players" network command parameter when opening a game, which has long been obsolete.

This change should only affect new clients, build19 clients should still work with plaintext passwords. This branch should be merged parallel to the respective metaserver branch.

[bunnybot (widelandsofficial)]

Continuous integration builds have changed state:

Travis build 3266. State: errored. Details: https://travis-ci.org/widelands/widelands/builds/348394484.
Appveyor build 3075. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_net_pwd_security-3075.

[GunChleoc (gunchleoc)]

2 little nits, LGTM otherwise.

Which algorithm would sou recommend instead of sha-1? If it's available on Django, we should consider changing the website too.

review: Approve

[bunnybot (widelandsofficial)]

Continuous integration builds have changed state:

Travis build 3271. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/348602418.
Appveyor build 3080. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_net_pwd_security-3080.

[Notabilis (notabilis27)]

Thanks for the review. Reading the config has been fixed. Dropping the close() call is on purpose, it is done automatically in the desctructor of the class.

As secure hash algorithms we could use SHA-2 (over 10 years old but unbroken) or SHA-3 (relatively new, uses different approach). For the website we are using hashlib which seems to support both. Updating the database would probably be messy, though.

A C++ implementation is more of a problem. There are some small repositories where the algorithms are implemented in single files which could easily be integrated into widelands. The problem here is that I guess they never received any review so it might not be secure to use them. Preferably would be a tested library like OpenSSL but that would mean a further dependency for widelands. I haven't tried to extract the needed files out of it yet. (One advantage of SHA-1: It is included in boost.)

[GunChleoc (gunchleoc)]

OK, let's go with sha-1 for now then. We can always revisit this at some other time.

Changing it on the website would mean that everybody would need to use the "I have lost my password" function, I guess.

[kaputtnik (franku)]

I guess the password in this case is the password for Online gaming: https://wl.widelands.org/ggz/changepw which can be accessed through https://wl.widelands.org/profile/edit/ (the last link)

The related model is in wlggz: https://bazaar.launchpad.net/~widelands-dev/widelands-website/trunk/view/head:/wlggz/models.py

Django provides some default hashers for passwords, see https://docs.djangoproject.com/en/1.8/topics/auth/passwords/

I have no idea about creating passwords..., but it think the online gaming password could also be encrypted using django: https://docs.djangoproject.com/en/1.8/topics/auth/passwords/#module-django.contrib.auth.hashers

Maybe there is also a possibility to automatically convert old, not encrypted, passwords to encrypted ones?

[Notabilis (notabilis27)]

Forcing all users to reset their passwords would be a clean but uncomfortable migration step. Other possibilities would include keeping the old password until it is changed (which would probably be not anytime soon) or hashing the already hashed passwords again (would require more complicated code and no idea how secure this is).

Thanks for the links. The password I am talking about is indeed the one for online gaming, though we could consider only using one password for gaming and the website when we store it securely in the configuration file.

Based on https://bazaar.launchpad.net/~widelands-dev/widelands-website/trunk/view/head:/wlggz/forms.py and the code of the metaserver the password seems to be protected by SHA-1 currently.
From your link, Django seems to support SHA-1 and SHA-2/SHA256 but is using an different storage format from what we are using currently. We are simply storing SHA1(password) while the Django-Methods are storing "SHA1"|"salt"|sha1(password|salt) ( "|" being a string concatenation). Wouldn't be a problem but I don't know whether there is an advantage of using the Django methods over using hashlib directly.

A short explanation: Encryption and hashing are two completely different things. With Encryption you have a secret key and can later on decrypt the encrypted message by again using the key. So no information is lost by encryption, but the encrypted data is protected against everyone not owning the secret key.
With hashing, you are "compressing" your data into a byte sequence of fixed length. Naturally, information is lost this way and for cryptographic hash functions it is considered (practically) impossible to find some data which results in the same hash value. So you can calculate hash("text")=>abc but you can't do magic(abc)=>"text". Additionally, it is not feasible to find a string so hash("text1532")=>abc. Also note that there is no key involved.

At least the passwords for internet gaming are stored as SHA1 hashes in the database and I guess the passwords for the forum will also be hashed. So it is not possible to extract the password the user entered from the database and hash it again with a more secure algorithm.
What would be possible: The user is entering its password in plaintext when logging in (forum as well as lobby). We could use this plaintext password to update the database entry. This wouldn't help with inactive users, so we would need to run a ... hybrid ... system for quite some time.

[Notabilis (notabilis27)]

@bunnybot merge

=== modified file 'src/network/internet_gaming.cc'
--- src/network/internet_gaming.cc	2018-02-04 11:41:45 +0000
+++ src/network/internet_gaming.cc	2018-03-11 10:02:09 +0000
@@ -20,6 +20,7 @@
 #include "network/internet_gaming.h"
 
 #include <memory>
+#include <thread>
 
 #include <boost/algorithm/string.hpp>
 #include <boost/format.hpp>
@@ -159,7 +160,7 @@
 	s.string(clientname_);
 	s.string(build_id());
 	s.string(bool2str(reg_));
-	s.string(authenticator_);
+	s.string(reg_ ? "" : authenticator_);
 	net->send(s);
 
 	// Now let's see, whether the metaserver is answering
@@ -337,34 +338,76 @@
 }
 
 void InternetGaming::create_second_connection() {
-	NetAddress addr;
-	net->get_remote_address(&addr);
-	if (!addr.is_ipv6()) {
+	NetAddress addr_net;
+	net->get_remote_address(&addr_net);
+	if (!addr_net.is_ipv6()) {
 		// Primary connection already is IPv4, abort
 		return;
 	}
 
-	if (!NetAddress::resolve_to_v4(&addr, meta_, port_)) {
-		// Could not get the IPv4 address of the metaserver? Strange :-/
-		return;
-	}
-
-	std::unique_ptr<NetClient> tmpNet = NetClient::connect(addr);
-	if (!tmpNet || !tmpNet->is_connected()) {
-		// Connecting by IPv4 doesn't work? Well, nothing to do then
-		return;
-	}
-
-	// Okay, we have a connection. Send the login message and terminate the connection
-	SendPacket s;
-	s.string(IGPCMD_TELL_IP);
-	s.string(boost::lexical_cast<std::string>(kInternetGamingProtocolVersion));
-	s.string(clientname_);
-	s.string(authenticator_);
-	tmpNet->send(s);
-
-	// Close the connection
-	tmpNet->close();
+	// Do real work in thread to reduce freezing of GUI
+	// $this cannot become invalid since it is a global variable
+	// Member variables of $this might change their values but it does not really matter if the thread fails
+	std::thread([this]() {
+
+		NetAddress addr;
+		if (!NetAddress::resolve_to_v4(&addr, meta_, port_)) {
+			// Could not get the IPv4 address of the metaserver? Strange :-/
+			return;
+		}
+
+		std::unique_ptr<NetClient> tmpNet = NetClient::connect(addr);
+		if (!tmpNet || !tmpNet->is_connected()) {
+			// Connecting by IPv4 doesn't work? Well, nothing to do then
+			return;
+		}
+
+		// Okay, we have a connection. Send the login message and terminate the connection
+		SendPacket s;
+		s.string(IGPCMD_TELL_IP);
+		s.string(boost::lexical_cast<std::string>(kInternetGamingProtocolVersion));
+		s.string(clientname_);
+		s.string(reg_ ? "" : authenticator_);
+		tmpNet->send(s);
+		if (!reg_) {
+			// Not registered: We are done here
+			return;
+		}
+
+		// Wait for the challenge
+		uint32_t const secs = time(nullptr);
+		try {
+			while (kInternetGamingTimeout > time(nullptr) - secs) {
+				// Check if the connection is still open
+				if (!tmpNet->is_connected()) {
+					return;
+				}
+				// Try to get a packet
+				std::unique_ptr<RecvPacket> packet = tmpNet->try_receive();
+				if (!packet) {
+					continue;
+				}
+				const std::string cmd = packet->string();
+				if (cmd != IGPCMD_PWD_CHALLENGE) {
+					// Wrong command, abort
+					return;
+				}
+				const std::string challenge = packet->string();
+				// Got a challenge. Calculate the response and send it
+				SendPacket s2;
+				s2.string(IGPCMD_PWD_CHALLENGE);
+				s2.string(crypto::sha1(challenge + authenticator_));
+				tmpNet->send(s2);
+				// Our work is done
+				return;
+			}
+		} catch (const std::exception& e) {
+			log("InternetGaming: Error when trying to transmit secondary IP.\n");
+			return;
+		}
+
+		log("InternetGaming: Timeout when trying to transmit secondary IP.\n");
+	}).detach();
 }
 
 /// Handle one packet received from the metaserver.
@@ -389,7 +432,15 @@
 
 	// Are we already online?
 	if (state_ == CONNECTING) {
-		if (cmd == IGPCMD_LOGIN) {
+		if (cmd == IGPCMD_PWD_CHALLENGE) {
+			const std::string nonce = packet.string();
+			SendPacket s;
+			s.string(IGPCMD_PWD_CHALLENGE);
+			s.string(crypto::sha1(nonce + authenticator_));
+			net->send(s);
+			return;
+
+		} else if (cmd == IGPCMD_LOGIN) {
 			// Clients request to login was granted
 			const std::string assigned_name = packet.string();
 			if (clientname_ != assigned_name) {
@@ -401,10 +452,11 @@
 			}
 			clientname_ = assigned_name;
 			clientrights_ = packet.string();
-			if (clientrights_ == INTERNET_CLIENT_UNREGISTERED) {
-				// Might happen that we log in with less rights than we wanted to.
+			if (reg_ && clientrights_ == INTERNET_CLIENT_UNREGISTERED) {
+				// Permission downgrade: We logged in with less rights than we wanted to.
 				// Happens when we are already logged in with another client.
 				reg_ = false;
+				authenticator_ = crypto::sha1(clientname_ + authenticator_);
 			}
 			state_ = LOBBY;
 			log("InternetGaming: Client %s logged in.\n", clientname_.c_str());
@@ -412,7 +464,7 @@
 
 		} else if (cmd == IGPCMD_ERROR) {
 			std::string errortype = packet.string();
-			if (errortype != "LOGIN" && errortype != "RELOGIN") {
+			if (errortype != IGPCMD_LOGIN && errortype != IGPCMD_PWD_CHALLENGE) {
 				log("InternetGaming: Strange ERROR in connecting state: %s\n", errortype.c_str());
 				throw WLWarning(
 				   _("Mixed up"), _("The metaserver sent a strange ERROR during connection"));
@@ -425,11 +477,12 @@
 		} else {
 			logout();
 			set_error();
+			log("InternetGaming: Expected a LOGIN, PWD_CHALLENGE or ERROR packet from server, but received "
+				"command %s. Maybe the metaserver is using a different protocol version?", cmd.c_str());
 			throw WLWarning(
 			   _("Unexpected packet"),
-			   _("Expected a LOGIN, RELOGIN or REJECTED packet from server, but received command "
-			     "%s. Maybe the metaserver is using a different protocol version ?"),
-			   cmd.c_str());
+			   _("Received an unexpected network packet from the metaserver. The metaserver could be "
+				 "using a different protocol version. If the error persists, try updating your game."));
 		}
 	}
 	try {
@@ -589,6 +642,9 @@
 			if (waitcmd_ == IGPCMD_GAME_OPEN) {
 				waitcmd_ = "";
 			}
+			// Get the challenge
+			std::string challenge = packet.string();
+			relay_password_ = crypto::sha1(challenge + authenticator_);
 			// Save the received IP(s), so the client can connect to the game
 			NetAddress::parse_ip(&gameips_.first, packet.string(), kInternetRelayPort);
 			// If the next value is true, a secondary IP follows
@@ -666,7 +722,7 @@
 }
 
 const std::string InternetGaming::relay_password() {
-	return authenticator_;
+	return relay_password_;
 }
 
 /// called by a client to join the game \arg gamename
@@ -701,7 +757,6 @@
 	SendPacket s;
 	s.string(IGPCMD_GAME_OPEN);
 	s.string(gamename_);
-	s.string("1024");  // Used to be maxclients, no longer used.
 	net->send(s);
 	log("InternetGaming: Client opened a game with the name %s.\n", gamename_.c_str());
 

=== modified file 'src/network/internet_gaming.h'
--- src/network/internet_gaming.h	2017-11-28 20:39:35 +0000
+++ src/network/internet_gaming.h	2018-03-11 10:02:09 +0000
@@ -218,6 +218,9 @@
 	std::string authenticator_;
 	bool reg_;
 
+	/// Password for connecting as host to a game on the relay server
+	std::string relay_password_;
+
 	std::string meta_;
 	uint16_t port_;
 

=== modified file 'src/network/internet_gaming_protocol.h'
--- src/network/internet_gaming_protocol.h	2017-11-29 20:43:35 +0000
+++ src/network/internet_gaming_protocol.h	2018-03-11 10:02:09 +0000
@@ -36,9 +36,10 @@
  * 2: Between build 19 and build 20 - Added UUID to allow reconnect with same username after
  *    crashes. When logging twice with a registered account, the second connection gets a free
  *     username assigned. Dropping RELOGIN command.
- * 3: Between build 19 and build 20 - Added network relay for internet games [supported]
+ * 3: Between build 19 and build 20 - Added network relay for internet games
+ * 4: Between build 19 and build 20 - Using CHAP for password authentication [supported]
  */
-constexpr unsigned int kInternetGamingProtocolVersion = 3;
+constexpr unsigned int kInternetGamingProtocolVersion = 4;
 
 /**
  * The default timeout time after which the client tries to resend a package or even finally closes
@@ -110,10 +111,8 @@
  * 1) Linking connections with IPv4 and IPv6
  * The UUID is used on the metaserver to link multiple connections by the same client. This
  * normally happens when the client supports IPv4 and IPv6 and connects with both protocol versions.
- * This
- * way, the metaserver knows that the client supports both versions and can show games / offer its
- * game
- * of/for clients with both protocol versions.
+ * This way, the metaserver knows that the client supports both versions and can show games / offer its
+ * game of/for clients with both protocol versions.
  *
  * When a network client connects to the metaserver with (RE)LOGIN it also sends the UUID.
  * When "another" netclient connects to the metaserver and sends TELL_IP containing the same UUID,
@@ -122,11 +121,9 @@
  *
  * 2) Reconnect after crash / network problems.
  * When Widelands breaks the connection without logging out, the server still assumes that the old
- * connection is active. So when the player reconnects, another name is chosen. Sending the UUID
- * allows
+ * connection is active. So when the player reconnects, another name is chosen. Sending the UUID allows
  * to reclaim the old name, since the server recognizes that there isn't a second player trying to
- * use
- * the same name.
+ * use the same name.
  */
 
 /**
@@ -144,8 +141,7 @@
  *
  * \note If you want to change the payload of this command, change it only by appending new items.
  *       The reason is that this is the only command that can be sent by the metaserver even when
- * the
- *       protocol versions differ.
+ *       the protocol versions differ.
  *
  */
 static const std::string IGPCMD_DISCONNECT = "DISCONNECT";
@@ -153,25 +149,30 @@
 /**
  * Initiate a connection.
  *
- * The first communication across the network stream is a LOGIN command
+ * The first communication across the network stream is a IGPCMD_LOGIN command
  * sent by the client, with the following payload:
- * \li string:    protocol version
+ * \li string:    protocol version (see kInternetGamingProtocolVersion)
  * \li string:    client name
  * \li string:    build_id of the client
  * \li string:    whether the client wants to login in to a registered account
  *                ("true" or "false" as string)
- * \li string:    for registered accounts: password in clear text
+ * \li string:    for registered accounts: string of length 0
  *                for unregistered users the UUID to recognize the matching IPv4 and IPv6
  *                connections or to reclaim the username after a unintended disconnect.
  *                For an explanation of the UUID, see above.
  *
- * If the metaserver accepts, it replies with a LOGIN command with the following payload:
- * \li string:    client name (might be different to the previously chosen one, if the client did
- *                NOT login to a registered account and either the chosen is registered or already
- *                used.)
- * \li string:    clients rights  (see client rights section above)
- *
- * If no answer is received in \ref kInternetGamingTimeout s the client will again try to login
+ * If the user tries to login to a registered account, a IGPCMD_PWD_CHALLENGE exchange follows before
+ * the server replies with a IGPCMD_LOGIN or IGPCMD_ERROR message.
+ *
+ * If the metaserver accepts, it replies with a IGPCMD_LOGIN command with the following payload:
+ * \li string:    client name. Might be different to the previously chosen one, if the chosen
+ *                name already used or is registered (and the connecting client is not registered).
+ * \li string:    clients rights (see client rights section above)
+ *
+ * When the client is downgraded to an unregistered user on login, a special UUID value
+ * of sha1(assignedName | passwordHash) has to be used on reconnects.
+ *
+ * If no answer is received in \ref kInternetGamingTimeout seconds the client will again try to login
  * \ref INTERNET_GAMING_RETRIES times until it finally bails out something like "server does not
  * answer"
  *
@@ -184,19 +185,37 @@
  *
  * Assuming the client already has a connection over IPv6 and tries to establish a secondary
  * connection over IPv4, this is the only message sent.
- * It should be sent as soon as a connection is established, immediately followed by closing
- * the connection. No answer from the server should be expected.
+ * It should be sent as soon as a connection is established.
+ * For unregistered users, the connection should be closed immediately following the command. No
+ * answer from the server should be expected.
+ * For registered users, an exchange of IGPCMD_PWD_CHALLENGE commands is done before closing the
+ * connection.
  *
  * Is sent by the client, with the following payload:
- * \li string:    protocol version
+ * \li string:    protocol version (see kInternetGamingProtocolVersion)
  * \li string:    client name - the one the metaserver replied at the first login
- * \li string:    for registered accounts: password in clear text
+ * \li string:    for registered accounts: string of length 0
  *                for unregistered users the UUID used on login
  *                for an explanation of the UUID, see above.
  */
 static const std::string IGPCMD_TELL_IP = "TELL_IP";
 
 /**
+ * This is sent by the metaserver after a IGPCMD_LOGIN or IGPCMD_TELL_IP by a registered client.
+ * This is the first message of the a protocol similar to the challenge handshake authentication
+ * protocol (CHAP) for secure transmission of the users password.
+ * The server sends the nonce for hashing:
+ * \li string:    a nonce for hashing
+ *
+ * The client should answer the message by an own IGPCMD_PWD_CHALLENGE containing the hashed password:
+ * \li string:    HASH_SHA1(nonce | HASH_SHA1(password))
+ *
+ * If the transmitted value is correct, the normal IGPCMD_LOGIN/IGPCMD_TELL_IP sequence continues. If the
+ * value is wrong (e.g., wrong password) the connection is terminated by the servers IGPCMD_DISCONNECT.
+ */
+static const std::string IGPCMD_PWD_CHALLENGE = "PWD_CHALLENGE";
+
+/**
  * This command is sent by the metaserver if something went wrong.
  * At least the following payload:
  * \li string:    IGPCMD code of the message that lead to the ERROR message or ERROR
@@ -321,11 +340,12 @@
 
 /**
  * Sent by the client to announce the startup of a game with following payload:
- * \li string:    name
- * \li string:    number of maximal clients
- * \note build_id is not necessary, as this is in every way the build_id of the hosting client.
+ * \li string:    name of the game
+ * \note build_id is not necessary, as this is the build_id of the hosting client anyway.
  *
  * Sent by the metaserver to acknowledge the startup of a new game with the following payload:
+ * \li string:    a challenge that has to be "solved" to work as host of the new game.
+ *                See IGPCMD_PWD_CHALLENGE. The response is send to the relay
  * \li string:    primary ip of relay server for the game.
  * \li string:    whether a secondary ip for the relay follows ("true" or "false" as string)
  * \li string:    secondary ip of the relay - only valid if previous was true

=== modified file 'src/network/relay_protocol.h'
--- src/network/relay_protocol.h	2017-12-17 14:45:23 +0000
+++ src/network/relay_protocol.h	2018-03-11 10:02:09 +0000
@@ -123,7 +123,8 @@
     * Has the following payload:
     * \li unsigned_8: Protocol version.
     * \li string:     Game name.
-    * \li string:     For the host: Password that was set on start of the relay.
+    * \li string:     For the host: Password that was set on start of the relay. Is the "solution"
+    *                 for the challenge send by the metaserver on IGPCMD_GAME_OPEN
     *                 For clients/observers: String "client".
     *
     * Is answered by kWelcome or kDisconnect (if a parameter is wrong/unknown).

=== modified file 'src/ui_fsmenu/multiplayer.cc'
--- src/ui_fsmenu/multiplayer.cc	2017-11-28 20:54:16 +0000
+++ src/ui_fsmenu/multiplayer.cc	2018-03-11 10:02:09 +0000
@@ -105,13 +105,22 @@
 	Section& s = g_options.pull_section("global");
 	if (auto_log_) {
 		nickname_ = s.get_string("nickname", _("nobody"));
-		password_ = s.get_string("password", "nobody");
+		password_ = s.get_string("password_sha1", "nobody");
 		register_ = s.get_bool("registered", false);
 	} else {
 		LoginBox lb(*this);
 		if (lb.run<UI::Panel::Returncodes>() == UI::Panel::Returncodes::kOk) {
 			nickname_ = lb.get_nickname();
-			password_ = lb.get_password();
+			/// NOTE: The password is only stored (in memory and on disk) and transmitted (over the network
+			/// to the metaserver) as cryptographic hash. This does NOT mean that the password is stored
+			/// securely on the local disk. While the password should be secure while transmitted to the
+			/// metaserver (no-one can use the transmitted data to log in as the user) this is not the case
+			/// for local storage. The stored hash of the password makes it hard to look at the configuration
+			/// file and figure out the plaintext password to, e.g., log in on the forum. However, the
+			/// stored hash can be copied to another system and used to log in as the user on the metaserver.
+			// Further note: SHA-1 is considered broken and shouldn't be used anymore. But since the
+			// passwords on the server are protected by SHA-1 we have to use it here, too
+			password_ = crypto::sha1(lb.get_password());
 			register_ = lb.registered();
 
 			s.set_bool("registered", lb.registered());
@@ -139,7 +148,7 @@
 
 		// Reset InternetGaming and passwort and show internet login again
 		InternetGaming::ref().reset();
-		s.set_string("password", "");
+		s.set_string("password_sha1", "");
 		show_internet_login();
 	}
 }

=== modified file 'src/wlapplication.cc'
--- src/wlapplication.cc	2017-12-14 09:02:31 +0000
+++ src/wlapplication.cc	2018-03-11 10:02:09 +0000
@@ -71,6 +71,7 @@
 #include "logic/single_player_game_controller.h"
 #include "logic/single_player_game_settings_provider.h"
 #include "map_io/map_loader.h"
+#include "network/crypto.h"
 #include "network/gameclient.h"
 #include "network/gamehost.h"
 #include "network/internet_gaming.h"
@@ -747,7 +748,10 @@
 	s.get_string("uuid");
 	s.get_string("registered");
 	s.get_string("nickname");
+	// TODO(Notabilis): Remove next line after build 20.
+	// Currently left in to avoid removing stored passwords for users of both build 19 and trunk
 	s.get_string("password");
+	s.get_string("password_sha1");
 	s.get_string("emailadd");
 	s.get_string("auto_log");
 	s.get_string("lasthost");
@@ -765,6 +769,12 @@
 	}
 	s.set_int("last_start", time(nullptr));
 
+	// Replace the stored plaintext password with its SHA-1 hashed version
+	// Used to upgrade the stored password when upgrading widelands
+	if (strlen(s.get_string("password", "")) > 0 && strlen(s.get_string("password_sha1", "")) == 0) {
+		s.set_string("password_sha1", crypto::sha1(s.get_string("password")));
+	}
+
 	// Save configuration now. Otherwise, the UUID is not saved
 	// when the game crashes, loosing part of its advantage
 	try {
@@ -1175,8 +1185,9 @@
 			Section& s = g_options.pull_section("global");
 			s.set_string("nickname", playername);
 			// Only change the password if we use a registered account
-			if (registered)
-				s.set_string("password", password);
+			if (registered) {
+				s.set_string("password_sha1", password);
+			}
 
 			// reinitalise in every run, else graphics look strange
 			FullscreenMenuInternetLobby ns(playername.c_str(), password.c_str(), registered);

=== modified file 'src/wui/login_box.cc'
--- src/wui/login_box.cc	2017-02-26 12:16:09 +0000
+++ src/wui/login_box.cc	2018-03-11 10:02:09 +0000
@@ -63,7 +63,6 @@
 
 	Section& s = g_options.pull_section("global");
 	eb_nickname->set_text(s.get_string("nickname", _("nobody")));
-	eb_password->set_text(s.get_string("password", ""));
 	cb_register->set_state(s.get_bool("registered", false));
 	eb_nickname->focus();
 }