jamulussoftware / jamulus

Jamulus enables musicians to perform real-time jam sessions over the internet.
https://jamulus.io
Other
997 stars 222 forks source link

Use a different approach to get external IP address #633

Closed atsampson closed 3 years ago

atsampson commented 4 years ago

From discussion in #572...

NetworkUtil::GetLocalAddress is used by CServerListManager's constructor to provide an idea of the server's own address for the first entry in the server list. Currently it works by opening a TCP connection to Google's public DNS server and using Qt's wrapper for getsockname to get the local end's address, which isn't ideal for privacy reasons.

We could rework this to use a connect()-ed UDP socket, which would achieve the same effect without actually sending any packets (on Linux/FreeBSD at least; I don't know if this works on Windows/MacOS). Or we might be able to get the address from one of the Jamulus server's existing sockets when replying to a server list request.

pljones commented 4 years ago

Given the version lookup is done to the Default Central Server list server, that might as well be the address used, if the TCP connection is going to work.

(Note that many other methods were investigated and not found to be available cross-platform. This is the most reliable, as far as I could tell.)

pljones commented 4 years ago

Well, jamulus.fischvolk.de doesn't have port 80 open.

The requirements here are:

gilgongo commented 4 years ago

Given the fact that this ticket originally came from the POV of privacy, I see CloudFlare "doesn't just promise that it won't use your browsing data to serve ads; it commits that it will never write the querying IP address (yours) to disk. Any logs that do exist will be deleted within 24 hours."

So maybe 1.1.1.1 is a good enough alternative?

ann0see commented 4 years ago

As a quick fix 1.1.1.1 might work.

corrados commented 4 years ago

@pljones Do you want to apply the change (since you have written that code)?

pljones commented 4 years ago

Done: #662

Running on jamulus.drealm.info and I can connect locally okay (i.e. it uses my LAN address) using the server list entry.

gilgongo commented 4 years ago

Note to self: update privacy statement when this gets released.

ann0see commented 4 years ago

I think it has been merged

corrados commented 4 years ago

Ok, should we close this one then?

ann0see commented 4 years ago

Not sure. We still rely on a third party.

pljones commented 4 years ago

The default here needs to be to use the Well Known service. Perhaps, in some circumstances, someone is running a server list with registered servers that do not expose their public IP addresses - but that case is going to be incredibly rare if it ever happens. Should it happen, perhaps an IP/port rewriting rule could be inserted in a firewall, or similar, to have the lookup handled some other way, without being routed outwards.

gilgongo commented 4 years ago

We still rely on a third party.

From what I understand, not relying on a third party isn't going to be possible if the central server(s) don't have well known TCP ports open. Which they don't.

atsampson commented 4 years ago

I haven't had a chance to try implementing this yet, but I've had a look through the Qt documentation today and found a few more potential ways of doing it if my first suggestion above doesn't work on all platforms. So we could:

So it looks like there are plenty of options that don't need a remote server or any network communication at all.

gilgongo commented 4 years ago

Well. Rocking good news!

pljones commented 3 years ago

I tried the first option and it didn't appear to work - hence using TCP. I may well have done something wrong...

A central server could be started with a "known" set of hosts, so there wasn't an option to wait without significantly rewriting the start up code, which wasn't a priority.

And the third option is not only messy, it's unreliable in many situations (e.g. within cloud data centres, it can give completely the wrong answer).

So either a working version for option 1, which is simplest, or a substantial - hence risky - rewrite of a core part of the Jamulus server list processing.

pljones commented 3 years ago

Has anyone managed to get option 1 working on Windows, Mac and Linux yet?

corrados commented 3 years ago

There is another closed Issue https://github.com/corrados/jamulus/issues/690 in which it was recommended to make the IP lookup time out higher (from 500 ms to 5 s). But when there is another method of getting the local IP is going to be supported, that Issue would no longer be relevant.

hoffie commented 3 years ago

When reading the code, I stumbled upon the 1.1.1.1 address and found this issue. I fully agree with @atsampson. For finding a machine's external IP¹, no actual network traffic should be necessary. I quickly tried what @atsampson proposed and it seems to work as expected (#1011).

¹ We are still talking about the existing approach which just finds the external IP. This will be equal to the public IP for Internet-facing machines. For machines behind NAT, it will be the private IP within the NAT environment. If one really wanted to find the external IP, one would need to contact an external service (STUN, an API). But I do not think that this is in the scope of this issue.

pljones commented 3 years ago

Just to note @softins (https://github.com/jamulussoftware/jamulus/pull/1011#discussion_r578548118) points out that no exposure happens whilst retaining functionality simply changing the protocol to UDP. (In case people aren't following the code, too.)

hoffie commented 3 years ago

Will merge in a second. Thanks for anyone involved, especially @atsampson! :)

Note to self: update privacy statement when this gets released.

https://github.com/jamulussoftware/jamuluswebsite/pull/333