Fixed race condition with CM_MAC_ADDRESS

LS could send account info before the game client would send `CM_MAC_ADDRESS`, resulting in invalid checks (added in 1e0080f) due to missing data. Refactored everything to request account info only after receiving `CM_MAC_ADDRESS`.
Also removed pointless todo in CM_RECONNECT_AUTH, as this packet's validStates already take care of it.

Author: neon-dev

game-server/src/com/aionemu/gameserver/network/aion/AionConnection.java

@@ -242,13 +242,9 @@ protected final void onDisconnect() {
 			return;
 		}
 
-		String msg = "";
-		Account account = getAccount();
-		if (account != null) {
-			msg += " " + account;
-			LoginServer.getInstance().aionClientDisconnected(account.getId());
-		}
+		LoginServer.getInstance().onDisconnect(this);
 
+		String msg = getAccount() == null ? "" : " " + getAccount();
 		Player player = getActivePlayer();
 		if (player != null) {
 			msg += " " + player + " (client crash or connection loss)";

game-server/src/com/aionemu/gameserver/network/aion/clientpackets/CM_L2AUTH_LOGIN_CHECK.java

@@ -59,6 +59,6 @@ protected void readImpl() {
 
 	@Override
 	protected void runImpl() {
-		LoginServer.getInstance().requestAuthenticationOfClient(accountId, getConnection(), loginOk, playOk1, playOk2);
+		LoginServer.getInstance().registerLoginRequest(accountId, getConnection(), loginOk, playOk1, playOk2);
 	}
 }

game-server/src/com/aionemu/gameserver/network/aion/clientpackets/CM_MAC_ADDRESS.java

@@ -7,6 +7,7 @@
 import com.aionemu.gameserver.network.aion.AionClientPacket;
 import com.aionemu.gameserver.network.aion.AionConnection;
 import com.aionemu.gameserver.network.aion.AionConnection.State;
+import com.aionemu.gameserver.network.loginserver.LoginServer;
 
 /**
  * In this packet client is sending connection and system information (IPs, MAC Address and HDD serial).
@@ -37,6 +38,7 @@ protected void runImpl() {
 		AionConnection con = getConnection();
 		con.setMacAddress(macAddress);
 		con.setHddSerial(hddSerial);
+		LoginServer.getInstance().authenticateClient(con);
 	}
 
 	private static String fixHddSerial(String hddSerial) {

game-server/src/com/aionemu/gameserver/network/aion/clientpackets/CM_RECONNECT_AUTH.java

@@ -25,7 +25,6 @@ protected void readImpl() {
 	@Override
 	protected void runImpl() {
 		AionConnection client = getConnection();
-		// TODO! check if may reconnect
 		LoginServer.getInstance().requestAuthReconnection(client.getAccount().getId(), client);
 	}
 }

game-server/src/com/aionemu/gameserver/network/loginserver/LoginServer.java

@@ -41,15 +41,8 @@ public class LoginServer {
 
 	private static final Logger log = LoggerFactory.getLogger(LoginServer.class);
 
-	/**
-	 * Map<accountId,Connection> for waiting request. This request is send to LoginServer and GameServer is waiting for response.
-	 */
-	private Map<Integer, AionConnection> loginRequests = new ConcurrentHashMap<>();
-
-	/**
-	 * Map<accountId,Connection> for all logged in accounts.
-	 */
-	private Map<Integer, AionConnection> loggedInAccounts = new ConcurrentHashMap<>();
+	private final Map<Integer, LoginRequest> loginRequests = new ConcurrentHashMap<>();
+	private final Map<Integer, AionConnection> loggedInAccounts = new ConcurrentHashMap<>();
 	private LoginServerConnection lsCon = null;
 	private NioServer nioServer = null;
 	private int gameServerCount = 1;
@@ -98,8 +91,8 @@ public void disconnect() {
 			lsCon.close();
 			lsCon = null;
 		}
-		for (AionConnection client : loginRequests.values())
-			client.close(/* closePacket */); // TODO! some error packet!
+		for (LoginRequest loginRequest : loginRequests.values())
+			loginRequest.connection.close();
 		loginRequests.clear();
 	}
 
@@ -119,13 +112,13 @@ public boolean isUp() {
 	/**
 	 * Notify that client is disconnected - we must clear waiting request to LoginServer if any to prevent leaks. Also notify LoginServer that this
 	 * account is no longer on GameServer side.
-	 * 
-	 * @param accountId
 	 */
-	public void aionClientDisconnected(int accountId) {
-		loginRequests.remove(accountId);
-		loggedInAccounts.remove(accountId);
-		sendPacket(new SM_ACCOUNT_DISCONNECTED(accountId));
+	public void onDisconnect(AionConnection connection) {
+		loginRequests.values().removeIf(r -> r.connection == connection);
+		if (connection.getAccount() != null) {
+			loggedInAccounts.remove(connection.getAccount().getId());
+			sendPacket(new SM_ACCOUNT_DISCONNECTED(connection.getAccount().getId()));
+		}
 	}
 
 	public void setGameServerCount(int gameServerCount) {
@@ -136,18 +129,16 @@ public int getGameServerCount() {
 		return gameServerCount;
 	}
 
+	public void registerLoginRequest(int accountId, AionConnection client, int loginOk, int playOk1, int playOk2) {
+		loginRequests.putIfAbsent(accountId, new LoginRequest(client, new SM_ACCOUNT_AUTH(accountId, loginOk, playOk1, playOk2)));
+	}
+
 	/**
 	 * Starts authentication procedure of this client - LoginServer will send response with information about account name if authentication is ok.
-	 * 
-	 * @param accountId
-	 * @param client
-	 * @param loginOk
-	 * @param playOk1
-	 * @param playOk2
 	 */
-	public void requestAuthenticationOfClient(int accountId, AionConnection client, int loginOk, int playOk1, int playOk2) {
-		if (isUp() && loginRequests.putIfAbsent(accountId, client) == null)
-			lsCon.sendPacket(new SM_ACCOUNT_AUTH(accountId, loginOk, playOk1, playOk2));
+	public void authenticateClient(AionConnection client) {
+		if (isUp())
+			loginRequests.values().stream().filter(r -> r.connection == client).findAny().ifPresent(r -> lsCon.sendPacket(r.lsAuthResponse));
 		else
 			client.close(new SM_L2AUTH_LOGIN_CHECK(false, null)); // disconnect this client since authentication will not happen
 	}
@@ -157,10 +148,11 @@ public void requestAuthenticationOfClient(int accountId, AionConnection client,
 	 */
 	public void accountAuthenticationResponse(int accountId, String accountName, boolean result, long creationDate, AccountTime accountTime,
 		byte accessLevel, byte membership, long toll, String allowedHddSerial) {
-		AionConnection client = loginRequests.remove(accountId);
-		if (client == null)
+		LoginRequest loginRequest = loginRequests.remove(accountId);
+		if (loginRequest == null)
 			return;
 
+		AionConnection client = loginRequest.connection;
 		if (!result || !validateMacAndHddSerial(client, allowedHddSerial)) {
 			client.close(new SM_L2AUTH_LOGIN_CHECK(false, accountName)); // LS sends no accName when result is false
 			sendPacket(new SM_ACCOUNT_DISCONNECTED(accountId)); // disconnect manually from login server because account isn't attached to connection yet
@@ -181,10 +173,7 @@ public void accountAuthenticationResponse(int accountId, String accountName, boo
 	}
 
 	private boolean validateMacAndHddSerial(AionConnection client, String allowedHddSerial) {
-		if (client.getMacAddress() == null || client.getHddSerial() == null) {
-			log.warn(client + " did not send CM_MAC_ADDRESS during login (modified client or hack)");
-			return false;
-		} else if (!client.getMacAddress().matches("^([0-9A-F]{2}-){5}[0-9A-F]{2}$")) {
+		if (!client.getMacAddress().matches("^([0-9A-F]{2}-){5}[0-9A-F]{2}$")) {
 			log.warn(client + " sent an invalid MAC address (modified client or hack): " + client.getMacAddress());
 			return false;
 		} else if (BannedMacManager.getInstance().isBanned(client.getMacAddress())) {
@@ -214,12 +203,9 @@ private void kickOnlineCharacters(Account account) {
 
 	/**
 	 * Starts reconnection to LoginServer procedure. LoginServer in response will send reconnection key.
-	 * 
-	 * @param accountId
-	 * @param client
 	 */
 	public void requestAuthReconnection(int accountId, AionConnection client) {
-		if (isUp() && loggedInAccounts.containsKey(accountId) && loginRequests.putIfAbsent(accountId, client) == null)
+		if (isUp() && client.equals(loggedInAccounts.get(accountId)))
 			lsCon.sendPacket(new SM_ACCOUNT_RECONNECT_KEY(client.getAccount().getId()));
 		else
 			client.close(/* closePacket */);
@@ -228,12 +214,9 @@ public void requestAuthReconnection(int accountId, AionConnection client) {
 	/**
 	 * This method is called by CM_ACCOUNT_RECONNECT_KEY LoginServer packets to give GameServer reconnection key for client that was requesting
 	 * reconnection.
-	 * 
-	 * @param accountId
-	 * @param reconnectKey
 	 */
 	public void authReconnectionResponse(int accountId, int reconnectKey) {
-		AionConnection client = loginRequests.remove(accountId);
+		AionConnection client = loggedInAccounts.get(accountId);
 		if (client == null)
 			return;
 		client.close(new SM_RECONNECT_KEY(reconnectKey));
@@ -287,4 +270,6 @@ private static class SingletonHolder {
 
 		protected static final LoginServer instance = new LoginServer();
 	}
+
+	private record LoginRequest(AionConnection connection, SM_ACCOUNT_AUTH lsAuthResponse) {}
 }