search

BZFlag-Dev / bzflag

3D multi-player tank battle game
https://www.bzflag.org
Other
297 stars 85 forks source link

Change UDP Connection System #115

Open blast007 opened 6 years ago

blast007 commented 6 years ago

Currently the game tries to start UDP at the very start of the connection process. This makes it hard for the server to know what UDP endpoints are tied to players and what are attacks.

The UDP connection should be moved to much later in the connection cycle and be given a 2 byte security prefix. Only UDP packets that have associated TCP connections that have passed authentication will be accepted. The 2 byte security prefix is randomly generated by the server, sent over TCP during the connection handshake and must be included at the start of every UDP packet. Packets that do not have this will be trivially rejected.

It would also be good/better to have the server tell the client what UDP port to use, so it could be different or randomly assigned. This will prevent an attacker from finding the UDP port. This could complicate firewall configuration so there should be a way to pool/limit to one or a small number of ports if needed.

(From @JeffM2501)

timriker commented 6 years ago

I don't think a random udp port is a good idea. We have enough trouble supporting people getting to the one port. The 2-byte random client key does sound like a good thing.

JeffM2501 commented 6 years ago

The UDP port used for the connection outbound from clients is not an issue. The issue is servers being configured to route inbound UDP. They already have to do that now, so adding an optional port range (Yes I should have written this up as an optional definable range, not "random"). is not an real extra work (if you can route one, you can route a range). The main "big" servers are no longer hosted on home connections but actual data center machines, so this type of configuration is not out of line for that kind of setup.

I agree that by default it should probably stick with a single port to make home servers simpler to configure, but these features are needed for the more popular servers.

timriker commented 6 years ago

I'm not seeing why these are "needed for the more popular servers". A single UDP port can handle millions of connections, practically unlimited. Adding more listening ports does not add any additional capacity. What are you trying to accomplish with more listening ports? It seems additional complexity both for the administrator and in the code, with no win whatsoever.

JeffM2501 commented 6 years ago

1) It hides the UDP port. There are many attacks these days that simply use the list server and spam the corresponding UDP port blindly (usually using amplification attacks). by allowing the server to specify the UDP port, it can be moved to something different. This makes the attack require a full valid TCP connection first, something most script systems don't want to do (this is also why the UDP should be pushed as late as possible in the connection sequence).

2) For a multi-threaded server there may be benefit in breaking the players into UDP groups. The issue is not the UDP throughput of the socket (though on some system buffering may be an issue on a single port), the issue is the processing that happens with the socket data. Having multiples may allow more options in this area, such as thread pools doing message processing and sending separate from the main thread.

3) There could be valid reasons why a server wants one port for TCP, and another for UDP, this just allows it.

I agree that there is no need to add complexity for the administrator. I mostly want the protocol changed to allow experimentation with existing clients. It's very hard to do load and thread testing on branch clients because nobody has them.

I propose that the port be added to the UDP connection message, but bzfs simply set it to the TCP port. Then the current BZFS can continue to do what it does, and operate normally. The client should read this port and it use it when creating it's UDP socket back to the server. If desired an optional server config argument can be added to specify the UDP port, if it is not specified the TCP port is used. This could be potentially helpful in dealing with attacks.

This will cause zero change for server owners and end users unless they want to try new things, but it will allow new things to be tried.

timriker commented 6 years ago
  1. kind of pointless, as the port is easy to discover anyway.
  2. invalid. Multiple thread can already listen on the same UDP port. This gains nothing other than added complexity
  3. I suppose some admins may WANT a more complex setup, but do we want to support it? They can already do their own port forwarding somewhere and complicate their system if they want to.

It does seem reasonable to add a shared client secret and reject packets that do not contain valid secrets. It also makes sense to start the UDP handshake after the login process.