0000992: Master server code makes incorrect assumptions about the source port of updates.

Notes
(0004467) Torr Samaho (administrator) 2012-08-26 18:54	I changed the report type to "suggestion" since, as far as I can tell, things are working as intended. BTW: What's the benefit of this randomization? You want the master to be available under a certain port, but you don't want to send from the port?

(0004470) jummama (reporter) 2012-08-26 19:22	I'm not entirely sure what the benefit of the source port randomization is, but it has been the default in OpenBSD pf since 3.3 (current version is 5.1). That being said, FreeBSD and NetBSD based firewalls probably also do this by default (haven't checked), since OpenBSD's pf is the grandparent of pf in other OS's. I think it has to do with mitigating certain man-in-the-middle attacks on the DNS protocol. Since it seems to be a security feature, I'm sure there's routers out there that do this, and possibly don't give an option to disable it, since most protocols don't break when the source port is changed. Also, since pf seems to be doing this by default, I would not be surprised if iptables does this in the future, which would then break Zandronum master server updates on quite a number of networks, since most routers seem to be based on Linux.

(0004471) Torr Samaho (administrator) 2012-08-26 19:35 edited on: 2012-08-26 19:35	If there is a security benefit you get by keeping this enabled for the packets the server sends to the master, then I'd say implementing a port override could be justified. So far I don't see any benefit there though. The server wants to be reached on the port its using to send, so what do you gain by hiding the very port to the master you want the master to publish? I may very well be overlooking something here, so please correct me if I'm wrong.

(0004473) jummama (reporter) 2012-08-26 19:44	I doubt there's any security gained from the randomization in regards to the master server, honestly, but the fact that is the default on some NATs tells me it could be beneficial to have such an option. In my case, the randomization can be disabled on the basis of the hosts involved, but I'm just concerned about the possible edge case of a NAT that does this and doesn't give the user control over it, or a super-paranoid admin that's willing to open a port, but not turn this off.

(0004474) jummama (reporter) 2012-08-26 20:07	An added benifit (however small) of a patch that fixes this would be that one could configure the port forward to listen on a different port than the one it directs to on Zandronum. Not sure why one would do THAT, since you can change ports in Zandronum just fine, but, doing so would work on manual connections but break master server updates similarly to how randomized source nat breaks it currently.

(0004476) Torr Samaho (administrator) 2012-08-26 20:44	Of course having the option to override the port wouldn't hurt and I remember one or two occasions during the last half decade where somebody had this kind of problem on the forum. It only makes sense to work on this though, if a few people actually need and use this. So is there anybody else who needs this?

(0004477) jummama (reporter) 2012-08-26 20:56	I am definitely an edge case, running OpenBSD automatically puts that spin on things :) By itself, the handling of odditys when a server talks from a different source port than it's actually listening on is probably not a real large priority, but I think I'm going to grab the source and take a look at adding some features to the master server that apply a little more broadly, while also handling this edge case, and I'm adding a different issue for that for feedback. I'll add another note with the number for it here, since it applies, but isn't entirely the same thing. Hope you don't mind :) (Off subject) It's awesome to see this code finally freed up, thank you for taking on the task.

(0004478) jummama (reporter) 2012-08-26 21:05	Edge case handled by feature proposed in issue 993.

(0004486) Torr Samaho (administrator) 2012-08-27 20:02	Quote from jummama I think I'm going to grab the source and take a look at adding some features to the master server that apply a little more broadly, while also handling this edge case If you implement this on your own, this will greatly enhance the chances of this feature ending up in Zandronum ;). I have a hard time convincing myself to implement something only very few people will ever use, while there are so many requests out there a lot of people are waiting for. But I'll gladly review and add patches for things like this. Quote from jummama It's awesome to see this code finally freed up, thank you for taking on the task. Thanks! It was a long way there, but I'm very glad that we could make it happen after all.

(0004689) Zalewa (developer) 2012-09-16 10:37	I think I just became a victim of this problem. I have a shitty Thomson TWG870UG router which was given to me by my ISP. I've setup port forwarding to forward some special ports to one computer (A) and the rest of them to another one (B). Hoping that the settings lower on the list will overwrite settings higher on the list I setup port forwarding for Zandronum at the very bottom of my list to forward a range of two ports to A. One server appeared on the master server list properly, but the other one appeared with port above 60000. Honestly, I have no idea what this has to do with port forwarding itself, except that ports in this range were already caught in a range that was already forwarded higher on the list to B. Eventually I managed to fix the problem by disabling forwarding for all ranges but the Zandronum's one and then re-enabling them. Visibly nothing changed on the list, but now it works as intended (probably the router re-wrote it's internal forwarding table differently). My personal opinion on this subject is this: Every article I've read instructed to always allow the operating system to choose the port for the client socket. Game server is a client of the master server, correct? Following these instructions no server should ever assume that the port of an incoming connection is also a compatible port for an outgoing connection. The opposite is true for IP address (incoming IP address == outgoing IP address). Because programs know at which port they host their listening sockets I think it's best to send this port explicitly when advertising to an entity such as the master server. An override option, where the port number would be sent explicitly in the packet to the master server, seems to be the best solution; just in case if someone would want to use the current behavior for some reason. I believe however that this option should be set to enabled by default.

(0007459) Konar6 (reporter) 2013-10-24 00:27 edited on: 2013-10-24 00:50	This problem seems to be rather common. I can see users experiencing it once every week now. Well, how about this patch I've attempted to make (untested, needs review, eg. I'm not sure if I got it right that NETWORK_ReadShort will actually return -1 there): 'http://sickedwick.net/k6/a331eda0128853f604ba60af3fea409b/00000054.txt [^]'

(0009621) Torr Samaho (administrator) 2014-06-21 15:33	Hard coding the sent port to NETWORK_GetLocalPort() is not an option. If you want the user to be able to override the port reported on the master, this should be optional (and opt-in) and the user should be able to freely select the port.

(0009630) Konar6 (reporter) 2014-06-21 19:58	Can you elaborate why it should be optional? What are the downsides? I presumed that this is a standard method. I found that ZDaemon did just this, in 2003: "Provided a solution for hosting the server in a NAPT environ- ment: the heartbeat message from the server to the master server now contains the actual listening port of the server; ideally, the master server should use that value instead of checking the actual port used to send the heartbeat message (the actual port may have been translated by NAPT; it is a standard procedure). [...]"

(0009643) Torr Samaho (administrator) 2014-06-22 09:43	Quote from Konar6 Can you elaborate why it should be optional? What are the downsides? Both, guessing that the external port is the local port and that the external port is the port used to send the hearbeats may be wrong. The user is the only one who knows the external port. So if you change the current default behavior, people who didn't have problems before may get problems after the change.

(0009675) Konar6 (reporter) 2014-06-22 17:48 edited on: 2014-06-22 17:50	Would switching the hardcoded element to a CVAR based one suffice then? I.e. is the approach good?

Only registered users can voice their support. Click here to register, or here to log in.
Supporters:	eco Monsterovich
Opponents:	No one explicitly opposes this issue yet.

Issue History
Date Modified	Username	Field	Change
2012-08-26 18:17	jummama	New Issue
2012-08-26 18:50	Torr Samaho	Category	Bug => Suggestion
2012-08-26 18:54	Torr Samaho	Note Added: 0004467
2012-08-26 18:54	Torr Samaho	Status	new => feedback
2012-08-26 19:22	jummama	Note Added: 0004470
2012-08-26 19:22	jummama	Status	feedback => new
2012-08-26 19:35	Torr Samaho	Note Added: 0004471
2012-08-26 19:35	Torr Samaho	Note Edited: 0004471	View Revisions
2012-08-26 19:44	jummama	Note Added: 0004473
2012-08-26 20:07	jummama	Note Added: 0004474
2012-08-26 20:44	Torr Samaho	Note Added: 0004476
2012-08-26 20:44	Torr Samaho	Status	new => feedback
2012-08-26 20:56	jummama	Note Added: 0004477
2012-08-26 20:56	jummama	Status	feedback => new
2012-08-26 21:05	jummama	Note Added: 0004478
2012-08-27 20:02	Torr Samaho	Note Added: 0004486
2012-08-27 20:20	Torr Samaho	Relationship added	related to 0000993
2012-09-16 10:37	Zalewa	Note Added: 0004689
2013-10-24 00:27	Konar6	Note Added: 0007459
2013-10-24 00:50	Konar6	Note Edited: 0007459	View Revisions
2013-10-24 13:02	Dusk	Status	new => needs review
2014-04-27 13:09	eco	Note Added: 0008646
2014-04-28 23:51	Dusk	Note Added: 0008651
2014-05-01 15:43	Dusk	Note Deleted: 0008646
2014-05-01 15:43	Dusk	Note Deleted: 0008651
2014-06-21 15:33	Torr Samaho	Note Added: 0009621
2014-06-21 15:33	Torr Samaho	Assigned To	=> Torr Samaho
2014-06-21 15:33	Torr Samaho	Status	needs review => feedback
2014-06-21 19:58	Konar6	Note Added: 0009630
2014-06-22 09:43	Torr Samaho	Note Added: 0009643
2014-06-22 17:48	Konar6	Note Added: 0009675
2014-06-22 17:50	Konar6	Note Edited: 0009675	View Revisions
2016-11-21 21:11	Torr Samaho	Relationship added	parent of 0002928