Server: Stop opening a TCP connection for external IP detection#1092
Conversation
hoffie
changed the title
hoffie
force-pushed
the
fix-633-stop-cloudflare-traffic
branch
from
c4bbdcb to
76c7ff7
Compare
|
You've got *.qm file changes - you may need to clean those up. |
pljones
left a comment
pljones
left a comment
There was a problem hiding this comment.
Note to self: --serverpublicip could probably be enhanced to take a public port, given where it's processed.
hoffie
force-pushed
the
fix-633-stop-cloudflare-traffic
branch
from
76c7ff7 to
cba5d0d
Compare
Thanks, done.
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: 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. |
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". |
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. |
|
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). |
|
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 |
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. |
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. |
|
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. |
|
@ann0see Thanks for testing!
I agree, 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... |
hoffie
force-pushed
the
fix-633-stop-cloudflare-traffic
branch
2 times, most recently
from
f667479 to
2e2bce6
Compare
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. Credit for finding this approach goes to @atsampson. Fixes jamulussoftware#633. Signed-off-by: Christian Hoffmann <[email protected]>
hoffie
force-pushed
the
fix-633-stop-cloudflare-traffic
branch
from
2e2bce6 to
cd8ba80
Compare
Jamulus as of version 3.7.0 will no longer generate any traffic to CloudFlare. Therefore, remove the warnings from all translations of the Privacy Statement. jamulussoftware/jamulus#633 jamulussoftware/jamulus#1092 Signed-off-by: Christian Hoffmann <[email protected]>
Jamulus as of version 3.7.0 will no longer generate any traffic to CloudFlare. Therefore, remove the warnings from all translations of the Privacy Statement. jamulussoftware/jamulus#633 jamulussoftware/jamulus#1092 Signed-off-by: Christian Hoffmann <[email protected]>
4 participants
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).