Secure authentication - prevent joins as a different player

[elexis]

The current implementation of the NetServer allows users to join as a different player.

Reproduce:

1. Alice, bob and carol start a game, (no matter if lobby or rated)
2. Bob disconnects
3. Eve notices that there are less players connected than the list displays
4. Eve tries to join the game, but can't connect as the game already started
5. Eve clicks on multiplayer -> join. The IP address is the one of alice' game
6. Eve tries to connect with one of the names (including the rating as that's part of the name)
7. After some tries, eve successfuly joins as Bob. Carol doesn't know its eve, as even the chat messages indicate it's bob.

The code we need to change for this can be found in CNetServerWorker::AddPlayer:

	// Try to match username next
	for (PlayerAssignmentMap::iterator it = m_PlayerAssignments.begin(); it != m_PlayerAssignments.end(); ++it)
	{
		if (!it->second.m_Enabled && it->second.m_Name == name && usedIDs.find(it->second.m_PlayerID) == usedIDs.end())
		{
			playerID = it->second.m_PlayerID;
			m_PlayerAssignments.erase(it); // delete the old mapping, since we've got a new one now
			goto found;
		}
	}

But notice we have this other TODO in CNetServerWorker::OnAuthenticate:

		// Search for an old disconnected player of the same name
		// (TODO: if GUIDs were stable, we should use them instead)

We might need some secure authentication token, like the jabber ID for lobbied games.

[scythetwirler]

We need to have more host-side validation; simple redefining a couple variables in session.js will allow one to impersonate other players.

Raising this to release blocker for Alpha 19 (for attention, doesn't have to stay there).

Abuse of this in the multiplayer lobby has been increasing.

[Josh]

Quick fix for A19: Don't save lobby IPs. It's bad practice to expose them like that anyway.

[Josh]

Don't save the host ip or player name if joining through the lobby

[Josh]

I'll commit this later tonight if there are no objections.

[elexis]

Notice the rating is part of the playername and can be faked too by joining by IP.

---

Exposing IPs:

Replying to Josh:

Quick fix for A19: Don't save lobby IPs. It's bad practice to expose them like that anyway.

Yes, that works for less sophisticated attackers and I agree it can be commited. The ticket should not be closed as the actual issue is not addressd.

The problem of exposing user IPs can be solved if we have only dedicated wfg-owned servers hosting. Then attackers will only get IPs of those servers instead of the player ones. Only the host sees the client IPs.

---

Proposed Solution for this ticket: My idea for solving this problem is to let the lobby server authenticate the user in game-join:

• Prohibit rejoins by IP for lobby games.
• If a player wants to join a lobby game, it sends that info to the lobby server
• The lobby server sends the info (including the players IP) to the host
• The host assigns the playername + rating to that client as provided by the lobby server

I see no other possiblity.

[Josh]

Patched in r17147. Doesn't fix the root issue, but repairs a tangential one. Should reduce the abuse enough for A19.

[elexis]

Extending comment:5:

Prohibit rejoins by IP for lobby games

Therefore users without accounts can't participate in lobbied games

As every gamejoin is registered with the lobby server

• can improve the prevention, clarification of abuse and "proving the point" a bit
• more troll accounts will have to be banned though

[elexis]

Backlogging due to lack of progress.

[elexis]

I would recommend to not release without Imaroks patch. The trolls and smurfs are annoying and this is an effective means to mitigate the impact. Incidentally also helps with the "unbannable nickname" hack.

[Imarok]

In 21520:

Secure lobby authentication - prevent joins as a different player

Reviewed by: Dunedan, elexis, Itms
Fixes #3549
Differential Revision: ​https://code.wildfiregames.com/D897

[elexis]

In 21877:

Always require lobby authentication for lobby matches, refs #3549 / rP21520 / D897.

This is due to too many oversteppings of the lobby Terms of Use following JS mods that implemented an UI for players to join lobby games with arbitrary nicknames or 'replace' / impersonate other players in lobby games.

Agreed with: user1, Dunedan
Code proofread by: Vladislav
Minor discussions with: Imarok, Hannibal_Barca, smiley, fpre, bb, nani
refs ​https://wildfiregames.com/forum/index.php?/topic/24722-improving-mod-security/