Connection timeout fix

[Liparakis]

Added UDPDiscoveryService
Some Improvements to Program.cs
More detailed comments.

[Extremelyd1]

I haven't looked at all the changes yet, but wanted to leave some comments regardless.

It seems that you are using AI in the code for this pull request. While I do not completely oppose to using AI, I want you to make it clear that you do in future PRs and I want you to thoroughly go over the code that the AI generates and decide whether it is necessary to include.
The reason I say this is that I can now somewhat explain what I have seen in past PRs from you. I've often left comments asking why you've decided to completely rewrite parts of the code that didn't seem the main focus of the PR and why you've decided to change code that didn't need changing. And I suspect this is caused by the AI touching parts of the code that weren't prompted.
So in the future, I want you to have more oversight over what changes are necessary and need committing, and what changes are generated by the AI as "collateral damage".

Again in this PR, you've made the changes that we discussed, but also touched way more code than was necessary. Instead of reviewing the addition of UDP packets to the MMS to obtain NAT port mapping, I now have to review a bunch of other changes and try to figure out what is related to the scope and what is changed for optimization, or other reasons.

Now onto some thoughts related to the implementation.

First of all, UDP is a connectionless, unreliable protocol. This means that sending a single packet to the MMS for port discovery might not be enough. In some cases, the packet may get dropped and I haven't seen anything addressing this edge case in the code. Naturally, because we are both using UDP and TCP, we can simply resend the UDP discovery packet until we receive an acknowledgement from the MMS over TCP.

Secondly, the UDP discovery packet is sent in plaintext, we do not have the security guarantee that TLS gives us with HTTPS. This technically means that an attacker that can attain a MiTM position can intercept discovery packets. Since we use the IP and port from the discovery packet as the endpoint to connect to, an attacker can redirect users trying to join a lobby to a server of their own. Fortunately, the remedy is simple: don't use the IP address from the discovery packet, but only the port. The IP address from the TCP request can be used, since it will be the same as over UDP (in contrast to the port).

[Extremelyd1]

Why are we telling the client in the response that they need to ensure a port is reachable? Shouldn't the port be reachable on our MMS instead?

[Extremelyd1]

This feels like an unnecessary change and when I checked it out further it seems to be an "em dash". This is usually an indication of code generated by an AI.

[Liparakis]

I understand your concerns and I appreciate the thorough review. Yes, i do use AI for many of the boilerplate code and comments, but i always read through the code it produces. The main benefit for me is that it allows for faster iteration during development.

Regarding the number of files that were touched, i also understand your point. In my workflow, I use instructions that guide the LLM to read exsting files and extract sub-functions from larger blocks of code. These are then refactored into separate functions, which helps create a cleaner and more maintainable function topology.

If you like I can redo the implementation with less intrusive edits and a cleaner scope.

Now regarding to the UDP and TCP clever usage and the Man in the middle. They completely flew over my head and you are right on those concerns..but both are easy fixes.

Lemme know if you want me to re-iterate with just the custom STUN implementation and without any other changes.

[Extremelyd1]

Alright, thanks for the explanation. Please re-iterate with only the UDP port discovery and the solution of the issues I outlined.