Server: Stop using Cloudflare for IP detection

[hoffie]

Previously, the code for finding the current machine's external IP address created a TCP connection to Cloudflare. This is unnecessary. Instead, just creating a socket to a non-private IP suffices to find
that IP. We now use a reserved IP for that and use UDP in order to avoid triggering any traffic at all.

All credit goes to @atsampson.
Fixes #633.

To be done:

• Verify functionality on Windows/Mac
• Decide whether to keep the new qDebug() line
• Confirm that using a reserved IP is the best option
• Confirm that using the Jamulus default port is a good idea

[pljones]

There's no guarantee what 192.0.2.x will use. It could be localnet. You might get back 127.0.0.1. The RFC says it's reserved for documentation and examples. This is an actual usage and hence violates the RFC!

[hoffie]

Hm, yeah, you're right. I am wondering what address to use instead?

• Keep using 1.1.1.1 although we would no longer perform a real connect?
• Use jamulus.io (the DNS name)? Has the downside that it will probably trigger network packets for the DNS resolution.
• Use current jamulus.io's IP? Might change at some time without anybody remembering to update the code.

[pljones]

I'm also not convinced that a UDP connectToHost will generate the source IP address at all.

[hoffie]

Why do you think that? In theory it can work (it's just required to prepare a socket; no need to send anything to the wire at all). I also verified that it does work on Linux, at least, no actual packet is generated to the target IP (using tcpdump) and the newly added debug message otuputs my "external" IP (some 10.10.0.x IP for me, which is what the previous code did as well).

Would you prefer some other approach?

[softins]

If you are not generating traffic, then provided the local address gets filled in by the OS to sockname (must check this happens on all platforms), it doesn't matter what public IP address gets used. All you need is an address that will get routed via the default route. Whether it's 1.1.1.1, 8.8.8.8 or something else non-RFC1918 doesn't matter.

Basically, it boils down to a platform-independent way to determine the IP address of the local interface by which the default route goes out.

AFAIK, the reason for this is to tell the central server the LAN address of the registering server, so that if a client on the same LAN (i.e. talking to the central server from the same public IP) wants to connect to the server, the client gets told the LAN address instead of the public one. Is that correct?

[softins]

Just done some tests on my Pi. Firstly set up a tcpdump to capture everything to or from 1.1.1.1:

tcpdump -C 1 -i $INTERFACE -nn -p -s0 -w $FILE 'host 1.1.1.1' </dev/null >/dev/null 2>&1 &

Then started up my installed 3.6.2 version as a server. In wireshark afterwards, I could see the six packets for the TCP connection to 1.1.1.1 port 53.

Then I took the current master, changed QTcpSocket to QUdpSocket in util.h and util.cpp, as per this PR (but I didn't change the address or port). Also in util.cpp I added the following debug:

--- a/src/util.cpp
+++ b/src/util.cpp
@@ -1029,6 +1029,9 @@ CHostAddress NetworkUtil::GetLocalAddress()

     if ( socket.waitForConnected ( IP_LOOKUP_TIMEOUT ) )
     {
+        qDebug() << "Local address: " << socket.localAddress();
+        qDebug() << "Peer address:  " << socket.peerAddress();
+
         return CHostAddress ( socket.localAddress(), 0 );
     }
     else

After building, with the tcpdump still running, I ran the new build as a server. No packets at all got captured, but the local address was still filled in correctly:

tony@pi:~/jamulus $ ./Jamulus -s
- server mode chosen
qt5ct: using qt5ct plugin
Local address:  QHostAddress("192.168.20.14")
Peer address:   QHostAddress("1.1.1.1")

So my conclusion is that it doesn't matter whether the address is 1.1.1.1 or 192.0.2.1 or any other public address, as no traffic is generated and there is therefore no "privacy" concern.

[pljones]

OK, I'll just say that an address that "looks like a well known IP address" is less likely to have strange routing behaviour than one that an RFC explicitly reserves for non-standard usage. So, if simply using a connected UDP port like that works on all platforms, then I'm all for just changing the protocol. (Sometimes the Qt documentation is utterly inscrutable - I think, when I read it, it seemed quite clear you could not call any of the connection-oriented socket methods on a UDP socket with any expectation of success. But, as I say, if it works - it works.)

[pljones]

And on this basis, either we close this branch or just re-title to "Stop using TCP for IP detection"?

[hoffie]

AFAIK, the reason for this is to tell the central server the LAN address of the registering server, so that if a client on the same LAN (i.e. talking to the central server from the same public IP) wants to connect to the server, the client gets told the LAN address instead of the public one. Is that correct?

Yes, I think so. The function is only called in that single place:
https://github.com/jamulussoftware/jamulus/blob/master/src/serverlist.cpp#L49

So my conclusion is that it doesn't matter whether the address is 1.1.1.1 or 192.0.2.1 or any other public address, as no traffic is generated and there is therefore no "privacy" concern.

Agreed.

And on this basis, either we close this branch or just re-title to "Stop using TCP for IP detection"?

Works for me. I can update the PR on the weekend (if someone else wants to, feel free to and comment here).

[pljones]

Please use a compliant address.

[hoffie]

Closing in favor of #1092.