Make the server sets Steamids to clients after the validation by Steam servers

[Splatt581]

This pr fixes the Steam App Ownership Ticket hijacking/spoofing vulnerability and related exploits. Now the server will sets the steamid from the ticket only after the client connection has been validated by the Steam servers. Before this, the client will be assigned STEAM_ID_PENDING (sid 0).

In fact, the GoldSrc server followed similar logic before the Steamworks updates.

[Splatt581]

In a good way, we also needs to replace the SendUserConnectAndAuthenticate function with BeginAuthSession, which using in Source1/S2 engines. The SendUserConnectAndAuthenticate function is deprecated in the Steamworks SDK and has poor client auth ticket fields validation.

But for now in the local rebuild of rehlds, I have the the BeginAuthSession function returns k_EBeginAuthSessionResultInvalidTicket even with a valid auth ticket for appid 10.

[tupec]

You can't spoof someone's ticket, there's a reason why - it's RSA signed with a private key. You can only reuse them (until the expiration, which is 21 days).

[Splatt581]

You can't spoof someone's ticket, there's a reason why - it's RSA signed with a private key. You can only reuse them (until the expiration, which is 21 days).

That's right.

But in the future, attackers may obtain the private key or otherwise start signing tickets. GoldSrc game servers must be ready for this.

“Reusing” of a ticket not with the owner’s account has been the main exploit used by attackers for 10 years; by stealing a ticket from server admins (mostly using social engineering), they gain admin rights on the server.

There are also other exploits that allow you to hide your steam avatar in the scoreboard, as well as a complete bypass of the steamid ban via the banid engine command.

This pr fixes all currently known vulnerabilities with steam tickets on the server.

[anzz1]

Would this be better as a rehlds plugin?

Hooking the functions with the hookchain API and changing the result?
If not, at very least all this should be wrapped in #ifdef REHLDS_FIXES like all functionality that doesn't match original HLDS.

I could see potential compatibility issue with plugins, let's say you're running a zombie mod which does stuff on client connect and expects the user to have a id instead of STEAM_ID_PENDING.

It seems that CSteam3Server::OnGSClientApprove() which validates the connection doesn't call any rehlds hookchain or isn't connected to Metamod either, but it does seem that AMX Mod X still has support for this from pre-steamworks era and should only call client_authorized() after the id is actually validated.

On ClientConnect(post) it does only call amxx's client_authorized() if the steam id is not STEAM_ID_PENDING, and if it is adds the player to a list which is then iterated in StartFrame(post) and client_authorized() is called and the player removed from list if he has got the validation since.
That all is from a quick glance though, no idea if it actually still works or could have been broken since considering we've already had Steamworks for a small eternity.

This change would no longer return valid id in client_connect() and the order of client_putinserver() and client_authorized() could now happen in any order instead of the current client_connect=client_authorized, and client_putinserver afterwards.
Making a simple code change to affected plugins so that whatever player id related stuffs like loading loadouts or whatever would happen in client_putinserver() or client_authorized() whichever gets called later, would then "guarantee" (unless theres a hickup in the validation server) both a valid steamid and that the user is connected to the server.

In any case changes like this can't be just pushed through like this, they should be definitely at least in REHLDS_FIXES considering this changes original functionality, or as it breaks plugin compatibility maybe introduced as a plugin or wrapped in a cvar like sv_rehlds_id_defer_validation.