Server: Stop opening a TCP connection for external IP detection

[hoffie]

Previously, the code for finding the current machine's external IP address created a TCP connection to Cloudflare's DNS. This is unnecessary.

Instead, just creating a socket to a non-private IP suffices to find that IP. We still use Cloudflare's IP, but we will no longer send any traffic there as we use an UDP socket without actually sending any data.

I have changed the port from DNS/53 to Jamulus' default port. In reality, the port should not matter as no traffic is sent. If the code is changed in the future and should start sending data again, it will be better to use a unique port so that Cloudflare could block such unwanted, non-DNS traffic. :)

Credit for finding this approach goes to @atsampson.
Fixes #633.
Supersedes #1011 (which also used a reserved IP).

[pljones]

You've got *.qm file changes - you may need to clean those up.

[pljones]

Note to self: --serverpublicip could probably be enhanced to take a public port, given where it's processed.

[hoffie]

You've got *.qm file changes - you may need to clean those up.

Thanks, done.

Note to self: --serverpublicip could probably be enhanced to take a public port, given where it's processed.

The processing part could use that, yes. But this would also require a change in what is transmitted on the wire. The relevant field seems to have a dynamic length, so it could work. However, until now it has not been a requirement so I chose to keep it simple.

Could someone confirm that this works properly on Windows (@pljones?) and Mac (@ann0see?) ? This simple test with Jamulus from this PR should suffice:

./Jamulus -e localhost -s --nogui
[...]
Using "<something sane for your machine>" as public IP.

Should this still go into 3.7.0? I'm not feeling strongly.

I'll add a Changelog entry once it is clear if we merge before 3.7.0 or after.

[pljones]

The processing part could use that, yes. But this would also require a change in what is transmitted on the wire. The relevant field seems to have a dynamic length, so it could work. However, until now it has not been a requirement so I chose to keep it simple.

Yep, it's just I've been complaining that port forwarding / mapping doesn't work for registered servers for some time -- this gives a way of passing in the "correct values": we're "faking" the public IP address, we can "fake" the public port number, too.

As I say, it's a "note to self".

[pljones]

Could someone confirm that this works properly on Windows

I almost don't think it's an issue - but worth checking if someone can do it easily. However, my only publicly visible machine is Linux.

[ann0see]

I can test it on macOS/Windows but only locally since I don't own a windows server

[hoffie]

I can test it on macOS/Windows but only locally since I don't own a windows server

That would be cool! Any Windows/macOS machine with network should suffice to validate that the external IP is still properly detected. No Internet or incoming access needed. The test instructions above should be sufficient (just noticed that I messed up Markdown syntax and that the initial line was hidden; fixed it).

[ann0see]

I wouldn't add this to the r 3.7.0 yet. Problem: the privacy page on the documentation would need to be updated. Also I think, a few more tests should be made here. I'd add it to 3.7.1

[ann0see]

Using "10.24.48.110" as public IP.

That's the internal IP of my PC (Windows). Not sure if that's correct? Will test on macOS

Same for macOS.

For debian the .debs seem to be broken. I will focus on fixing that now.

[softins]

Using "10.24.48.110" as public IP.

That's the internal IP of my PC (Windows). Not sure if that's correct? Will test on macOS

Same for macOS.

Yes, that’s correct. It isn’t trying to detect your public (outside of NAT) IP. It’s detecting the IP address of your local interface that is used to connect to your router. The purpose is so that a server can tell the central server what IP address another client coming from the same LAN (same public IP) should use to connect to that server.

I think the problem is that “as public IP” is inaccurate and misleading. Although I’m not sure how better to word it.

[ann0see]

Ah. Ok. So my test succeeded probably. If that’s the case, we could merge this, make changes to the documentation (on the changes branch) and wait for the next release of the documentation to pick it up.

[hoffie]

@ann0see Thanks for testing!

I think the problem is that “as public IP” is inaccurate and misleading. Although I’m not sure how better to word it.

I agree, I have now changed this to "external IP" which may still be ambiguous, but maybe less wrong? :)

[pljones]

I have now changed this to "external IP" which may still be ambiguous, but maybe less wrong? :)

Yup... It's the address of the interface that leads to the outside world: so "internet-facing interface IP" would be unambiguous but cumbersome...