Specify listen IP addresses for server

[Ashley]

I think a patch is going to be needed to allow you to specify which IP addresses the server should listen for incoming connections on. Currently it just listens on all the machine's addresses which isn't ideal in more complex situations (especially involving DNS records). At the very least a way to set the address(es) it can send the announce messages on is needed.

I'll see what I can come up with...

[prissi]

But with dns names, one may want to listen on IPv4 and IPv6 as well as on local network and remote networks, which would require different IPs. Why should this be a problem?

[Ashley]

E.g. your server has the IP addresses:

1.2.3.4
1.2.3.5

And the DNS record for the Simutrans server is:

simutrans.test.com

But the simutrans.test.com domain only has one A record, for 1.2.3.4.

The game will use either of the addresses the server has, so when it does an announce it may use 1.2.3.5, this causes the listing server to ignore it as the address doesn't match the DNS entry.

This becomes a real issue if your server has dozens of IP addresses...

An alternative would be to let you specify the addresses it should use to do the announce.

[prissi]

Not every OS lets you know or predetermine by which IP a request is sent in a very portable way. In certain cases (like a two way router) it may be even only decided per connection and outside the computer which route (and thus by which NAT and which IP) a packet is sent.

Imho the best way would be to require the server status packet from the server on the IP adress given and only use this data (which contains a lot of information). That would also remove all the need for any special handling of the first announcement, since the list server fetches the same server state a player would get. Since the server state also contains a minimap anyway and is in simutrans XML, parsing should be straight forward.

[Ashley]

In the case of NAT, the DNS record (or IP address) sent as the server_dns field of the announce would need to have the external IP address, the NAT router would then need to be configured to do port forwarding of the connection to the server hosting the game. In the case where this is unpredictable you cannot, IMO, run a server at all since incoming connections would not be possible.

I don't want to have to parse the gameinfo object for the listings server. The XML format lacks any real definition as to what the fields are (their meaning depends on their order) so if the gameinfo object were changed then the order would change and the listings server would break for the new version. I've written a parser for the gameinfo object so I understand this a bit. Also this is quite a large object to request regularly from the server.

Having the listings server make active outbound connections also significantly increases its complexity. I think it makes sense to keep the HTTP-based communication model since it means the listings server can be implemented easily using any web server if need be.

I have this patch which lets you specify a comma-separated list of IP addresses in the config file which the server will listen on. The same approach should be possible for the IPV4_ONLY section of code too but I've yet to do that. The default preserves the current behaviour of listening on 0.0.0.0 and :: (any IPv4 and any IPv6).


The same approach of a comma-separated list of addresses could be used for the persistent blacklist/whitelist functionality, e.g. blocking connections from banned addresses or only allowing nettool connections from specific addresses).

[prissi]

The gameinfo structure has a version number. Thus the list server can simply discard any misformed message, as it would discard a non-matching IP address.

The listserver needs to call that record only once when a game is started to confirm the given IP address. This is neglible compared to the accumulated informations sended by the servers.

On your patch: Please use vector_tpl instead of std::vector, to avoid mixing two vector template classes. You are also using std::stringstream just to get comma seperated arguments. This will link tons of unneeded library code (the std::io libary) and will not work on any machine. If you are in doubt, I can change that part.

Not your fault, but I just noticed that failing to open a listing channel gives no error.

[Ashley]

I'll find a way to split up the string without using stringstream, that was just a quick way to do it as a proof of concept. I didn't realise Simutrans used its own vector class, I will switch over to that as well.

The HTTP GET and POST methods both need better error handling (e.g. handling of status codes, timeouts etc.) I'll see about that too, since we'll want that to go into the next stable to make it easier to debug.

[Ashley]

This patch does not require std::vector or std::stringstream, and implements IP4_ONLY behaviour as well (in this case v6 addresses are of course ignored).

Still need to implement the same thing for opening outgoing network connections, to ensure that only the addresses defined in listen can be used to make outgoing connections.

[prissi]

Will, inet_addr does not work correctly on Haiku last time I tried. The current IPv4 code was really the optimum that was possible portable. But I will look into this.

[Ashley]

Could use inet_aton instead, either works (I assumed inet_addr would be more portable given that it is older, it's better to use inet_aton).

Or the listen specification functionality can be disabled for the IP4_ONLY build, this is a feature I really need for running my servers and I compile for a more modern platform where inet_ntop is available...

I've nearly finished the non-IP4_ONLY implementation for network_open_address() now, I've cleaned up the code of these two functions quite a bit as well.

[prissi]

The only function that works was gethostbyname(), as was written a little further up in the comments. But since I migrated to windows 7 I have to set up a new virtual machine for Haiku then.

Haiku is still IPv4 only, so disable those address selection for IPv4 would be ok by me.

[Ashley]

Ok, I think I'll leave the IP4_ONLY code alone for the moment then since it's going to require a lot of testing on such platforms and the feature probably isn't as needed there.

[Ashley]

Time to sleep, but I have it working for network_open_address() as well now. Have yet to remove the IP4_ONLY code.


Edit: Of course this breaks nettool a lot, have to fix that too...

[Ashley]

This version doesn't break nettool, and doesn't touch the USE_IP4_ONLY code (except to clean it up a bit, no changed functionality).

I have this patch running on all my servers now, seems to work well. I haven't tested compiling on Windows yet so if someone could test there I'd be grateful, otherwise I can try that on Monday.

[Ashley]

I've tested the patch with Windows, looks to work fine. Can anyone suggest any further improvements? Otherwise I'd like to propose pushing it into the trunk.

[prissi]

I commited it to the SVN.