I just realized that security model as implemented in PR is broken when reusing same keypair on multiple servers.
- Victim secures their company on legitimate server with pubkey.
- Attacker lures victim to connect to malicious server.
- Malicious server simultaneously connects to legitimate server and proxies KEYAUTH packets.
- After KEYAUTH is completed malicious server is left with authorized socket and can access the company, profit.
Of course it also applies to rcon access if we migrate it to pubkey too.
- Generate unique keypair for each IP/port. (permanently enabled privacy mode. cons: friends lists are nonviable, if somebody hosts server on dynamic IP it could lock people out periodically when IP changes)
- Client includes server target IP/port in signature, and server verifies that. (doesn't have cons of mitigation 1., but difficult to implement in practice: server might not necessarily know all valid IPs from which it's reachable)
- Implement secure key-exchange and authenticate and encrypt all communication.
That obviously for discussion, but I vote it is. It only requires knowing that someone is admin on some server, and luring them into malicious server to gain access to their rcon. Splitting rcon keys lowers the risk, but: it still isn't perfect, and complicates implementation (it needs some another variant of NEED_KEYAUTH/KEYAUTH that is only requested with first use of rcon). I don't like adding extra code and then still ending up with mediocre solution.
DH exchange gives pubkey without extra work. If we implement pubkey auth without encryption, we need these NEED_KEYAUTH/KEYAUTH packets, which are vulnerable to discussed attack. To protect against this attack pubkey must be derived from DH exchange. We can PR pubkey first and encryption later, but then insecure NEED_KEYAUTH/KEYAUTH/(separate variants for rcon keys) would need to be added only to be removed later.
Encrypted communication is implemented there: (quick implementation for demo, likely needs discussion if encapsulating into PACKET_CIPHERTEXT is right approach and whether it should be in tcp_game or somewhere else) (this removes NEED_KEYAUTH/KEYAUTH and instead uses pubkey from DH exchange)
https://github.com/Milek7/OpenTTD/commits/pubkey_encrypt