Patching current vulnerabilities and moving on to 0.7.0 (Page 2 of 2)

I'm sure there are a lot of features we would like to see in 0.7.0, but surely this is by no means a final release. A release as-is has been long due and would never get done if we expect everything to be complete by the next milestone.

I welcome efforts to finally release 0.7, but realistically, 0.6 stuff will keep running for a year or so at least, so it's important to also fix 0.6.

@Stitch626 Okay i understand.

@WaterMalone LOL, yes i have left the dark side, it is a waste of time and energy. I am glad teeworlds will be better.

@Oy okay, but currently you are spending one master for 0.7 only, 2 or 3 for 0.6 and one is able to manage 0.4/0.5... thats why I wrote that ^^

So....

Version server is vulnerable, to what extent is not clear right now, however this is a minor issue as it is not working anyway (version.teeworlds.com for example doesn't even resolve to an address). Hence i mostly skipped this part.

master server has a few obvious vulnerabilities that can be fairly easily resolved.
  -- SERVERBROWSE_GETCOUNT is vulnerable to amplification. simple fix: enlarge SERVERBROWSE_GETCOUNT by at least 2 bytes. (or make it a struct with two empty bytes whose existence are checked but not the values (only size is checked)
  -- Heartbeat checking is prone to amplification. i'd suggest making the try count amount of tries a constant and enforcing SERVERBROWSE_HEARTBEAT size of MAX_TRIES*sizeof(SERVERBROWSE_FWCHECK). Alternatively, enforce a size of any*sizeof(SERVERBROWSE_FWCHECK) and use it like "credits", if the request is 4*8 bytes large, you allow up to 4 check packets, if its 32*8 bytes large, you allow for 32 checks. (however "used" credits must not be re-used, additional heartbeat checks must only be performed if enough "traffic credits" are there. just an example. (as you see, the basic idea is never sending out more bytes than you received from a given address)
  -- UPDATE: Probably a better alternative would be a three way handshake here as connections to the master server are long-lived so once a connection is secured with a token, it can stay that way and you don't waste traffic.
  -- SERVERBROWSE_GETLIST is vulnerable. As the client determines the server count anyway, a simple approach would be to enlarge the GETLIST packet to SERVERCOUNT*SERVERINFO bytes (and headers are equal size, however note multiple packets would be sent in response, so (SERVERCOUNT*sizeof(SERVERINFO))+sizeof(HEADER)*num_packets_for_SERVERCOUNT. the master server would store this value and cut off the response if GETLIST was unsufficiently large.

Additionally i would like to re-think the anti spoof tokens. game traffic should ideally be authenticated and encrypted if this is possible with low ping penalties. i'd prefer using either authenticated encryption (AES-GCM) or (packet+hmac(token,packet)) instead of just (packet+token). However, this is not so much a problem for the master server.

Thats my thoughts about the master server. The reasons i picked simply enlarging packets instead of a complicated handshake is that a handshake is a bit much overhead for short lived "connections" such as fetching a serverlist once.
UPDATE: for server registration and heartbeat i recommend an actual handshake. this connection is long-lived.

updates on game protocol will follow.

Yes, it would be unlikely to be abused, but still, simple fix, just to make sure.

Encryption for the sake of doing it. If it does not cost a lot of time (especially compared to hmac, since aes gcm is authenticated encryption). Hmac would prevent the token to be actually transferred (if you ignore key exchange). And finally, key exchange can be done securely using (ephemeral) keys.

Thinking ahead, this could allow for the client checking server integrity (using the server public key for example), if wanted. It could surely allow for securely identifying and recognizing individual clients, making better rcon access control as well as maybe server passwords possible (or: use password once, server recognizes you). Additionally, if not wanted, ephemeral one time keys can be used for key exchange.

As this all happens during initial connection, overhead wouldn't be so much of a problem. For me, I would discuss those things a lot, but no matter what you choose, I vote for added security. The main reason for packet authentication here is by the way to prevent address spoofing, if every packet is signed (or authenticated by other means) you are sure it's from its original owner.

EDIT: just a side note after i took a quick look at the current token code. MD5 is unsafe, however that isn't even the limiting factor as the protocol only reserves 20 bits for a token. yes, there is only 1 million different tokens, which can be brute forced in very short time (as there is for example no penalty on sending wrong tokens), especially as this could be prone to timing attacks, reducing the necessary attempts a lot.

For that tiny nit. I don't regard md5 as the biggest problem. Packets are relatively small, and reserving only 20 bits for packet authentication makes it entirely possible to send all the million packets within 10 seconds until token rotation. Or, if that's just barely not possible, you retain a high percentage of win and will probably crack the next seed. As for https,as much as I despise it (it adds _a lot_ of unnecessary bloat instead of just using DTLS or TLS for example), i see that it would allow far better load balancing and reliability.

As for protocol, I would favor a simple approach. No heartbeat checking as that can be worked around quite easily. No server info as it would greatly limit teeworld's openness. Alternate serverlists? Must be supported by the individual server. Favorite list just storing ip addresse (or domains)? Nop. Custom server info for custom clients? Nop.
I would suggest a protocol that consists of a simple post request to put your server in the masterlist for say 1 or 5 minutes. The master server checks if the given ip matches the source ip, or if a domain was given, that domain resolved to the server. Get provides a list of addresses.

Posts relevant to patching the vulnerability and forward development were split and moved here.

From my understanding, they cannot be fixed without breaking compatibility/moving to 0.7, right?

I believe Schwertspize comments on how they weren't fixed in 0.7, so they cannot be fixed without breaking compatibility/moving to 0.8.

See above.