Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

When using SOCKS5 proxy, do name resolution through the proxy #469

Open
yurivict opened this issue Feb 9, 2017 · 35 comments
Open

When using SOCKS5 proxy, do name resolution through the proxy #469

yurivict opened this issue Feb 9, 2017 · 35 comments
Labels
enhancement New feature for the user, not a new feature for build script P3 Low priority security Security
Milestone

Comments

@yurivict
Copy link
Member

yurivict commented Feb 9, 2017

I just did a simple experiment. With 'Enable UDP'=off, 'Proxy type'=SOCKS5, 'Proxy address'=localhost, port=9050, system call log has this:

73692: socket(PF_INET,SOCK_DGRAM|SOCK_CLOEXEC,17) = 38 (0x26)
73692: connect(38,{ AF_INET 104.156.104.15:1 },16) = 0 (0x0)

This is the UDP socket for the clearnet IP 104.156.104.15. So Tox does leak IP despite of the proxy setting, and despite of 'Enable UDP'=off.

With the above network settings all connections should go through the proxy. No UDP should be attempted. No IPs except 127.0.0.1 should appear in the log.

I think I created such bug report before, but can't find it now.

toxcore-0.1.5,1
qTox-1.8.1

@yurivict
Copy link
Member Author

yurivict commented Feb 9, 2017

One other leak is DNS:

85377: socket(PF_INET,SOCK_DGRAM|SOCK_CLOEXEC,0) = 38 (0x26)
85377: connect(38,{ AF_INET 192.168.1.1:53 },16) = 0 (0x0)

It sends DNS requests to the regular DNS server, while SOCKS5 does support DNS, and SOCKS5 should be used for DNS.


Now when I look at the original UDP-connected sockets, they mostly or only call getsockname on such socket, which "probably" doesn't actually leak IP. But Tox doesn't need to even create such sockets in the case of SOCKS5 connectivity. But direct DNS requests certainly leak IP.

@nurupo
Copy link
Member

nurupo commented Feb 10, 2017

I have made a reproducible example https://gist.github.com/nurupo/c2a4eeaedec65dc4e55ef05e4b369f03

192.168.1.1 is my DNS server, 23.226.230.47 and 2604:180:1:4fb:0:10:20:d1ac are the resolved addresses of tox.zodiaclabs.org bootstrap node.

Anyway, it's the getaddrinfo() call which causes those connections.

To fix this, we need to do name resolution over SOCKS5 proxy when SOCKS5 proxy is selected.

@nurupo nurupo changed the title [security] Please stop the IP leak when SOCKS5 proxy is used When using SOCKS5 proxy, do name resolution through the proxy Feb 10, 2017
@nurupo nurupo added enhancement New feature for the user, not a new feature for build script security Security labels Feb 10, 2017
@nurupo
Copy link
Member

nurupo commented Feb 10, 2017

Actually, there are 2 more getaddrinfo() calls in toxcore in addition to the one I mentioned: [1], [2] both in tox.c, which is kind of weird since there is a wrapper for that in network.c -- the first getaddrinfo() I have mentioned in the last post, and it's even used in tox_new().

@nurupo
Copy link
Member

nurupo commented Feb 10, 2017

Here is a summary of what changes need to be done in toxcore in order to resolve this issue.

When using SOCKS5 proxy, there are 3 cases when the name resolution is done:

  1. Resolving address of the SOCKS5 proxy.
  2. Resolving address of bootstrap nodes.
  3. Resolving address of TCP relays.

In case 1, toxcore can't connect at all unless it uses DNS without tunneling it through a proxy in order to resolve proxy's IP address. We could require the SOCKS5 proxy address in Tox_Options to be an IP address. tox_new() would just fail if SOCKS5's address is a hostname. Alternatively, we could add an option to Tox_Options to force DNS resolution though the SOCKS5 proxy. tox_new() would still fail if SOCKS5's address is a hostname and the DNS is forced to go though the SOCKS5 proxy, but it would succeed in case DNS is not forced to go though the proxy.

In case 2, we need to implement a getaddrinfo() replacement that works through SOCKS5. We would need to communicate with DNS servers, meaning we would need to construct request packets and parse the replies according to the DNS protocol. We would also need to know addresses of DNS servers to use. Because there is no single cross-platform way to get the addresses of system DNS servers, toxcore could just offload the burden of getting those to the clients and provide a new option in Tox_Options for specifying addresses of DNS servers for toxcore to use to resolve domain names through SOCKS5 proxy. The way this would work is toxcore would request the SOCKS5 proxy to make a tunnel to TCP port 53 of the DNS specified in Tox_Options, then we would send the name resolution request packet to the DNS server through that tunnel, and receive the resolved IP address back.

In case 3, toxcore could just not resolve the address of TCP relays at all, as, in addition to IPv4 and IPv6, SOCKS5 also allows specifying hostname of the machine you want to establish the tunnel to (DOMAINNAME in the RFC).

@yurivict
Copy link
Member Author

yurivict commented Feb 10, 2017

Another relevant point here is that some SOCKS5 servers do support UDP, and some others don't. Particularly, Tor's SOCKS5 doesn't currently support UDP, only DNS UDP.

'Enable UDP'=true should generally enable UDP through SOCKS5. However, if SOCKS5 will be rejecting UDP associations, client should see some indication of this.

Currently, user can always have 'Enable UDP'=true and it isn't quite clear what happens when UDP isn't supported by SOCKS5.

@nurupo
Copy link
Member

nurupo commented Feb 10, 2017

Currently, user can always have 'Enable UDP'=true and it isn't quite clear what happens when UDP isn't supported by SOCKS5.

Well, toxcore currently supports SOCKS5 in TCP-only. If you don't disable UDP, TCP traffic will go through the proxy, but UDP won't, you'd be making direct connections with UDP.

Disabling UDP support is necessary when using anonymous proxies or Tor.

Because of this some Tox clients automatically disable UDP when you try to use a proxy (I know uTox does this, not sure about other clients).

Also, toxcore doesn't even need SOCKS5 UDP support for the name resolution as DNS servers are required to listen on both UDP 53 and TCP 53, so we could just use SOCKS5 proxy to connect to TCP port 53 of a DNS server.

@yurivict
Copy link
Member Author

we could just use SOCKS5 proxy to connect to TCP port 53 of a DNS server.

This will differ from UDP DNS, because you will have to make a choice for a DNS server, while with UDP this isn't required.

@nurupo
Copy link
Member

nurupo commented Feb 10, 2017

This will differ from UDP DNS, because you will have to make a choice for a DNS server, while with UDP this isn't required.

Could you elaborate more on that? I don't understand what you mean. The way I see it, you will have to chose a DNS server regardless of whether you want to communicate with it through UDP or TCP, since you will need to tell the SOCKS5 proxy the address of the DNS server to connect to in both cases.

@yurivict
Copy link
Member Author

With SOCKS5, you have a choice of the address type. It can be IPv4 (ATYP=X'01'), textual domain name (ATYP=X'03'), or IPv6 (ATYP=X'04'). I am not sure you are supposed to send UDP/53 DNS through SOCKS5. And you shouldn't send DNS TCP either. You just connect to the destination using the address, whatever it is. No need for DNS as such.

@nurupo
Copy link
Member

nurupo commented Feb 10, 2017

Oh, so that's what you mean. I have already mention this before. As I said in #469 (comment) in the 3rd case, you indeed don't have to do the name resolution for the TCP relays you want to connect though SOCKS5 proxy, because you can just tell the SOCKS5 proxy to connect to a hostname (DOMAINNAME in the RFC).

The issue is in the 2nd case of #469 (comment). It seems that toxcore requires you to perform bootstrap even if you use a TCP relay:

This function will attempt to connect to the node using UDP. You must use this function even if Tox_Options.udp_enabled was set to false.

In order to bootstrap off the DHT botostrap nodes, toxcore needs to resolve their hostnames. And no, it appears toxcore doesn't connect to the DHT bootstrap nodes through SOCKS5 proxy. DHT is UDP-only. So you can't connect using hostname, you have to actually do the name resolution, get IP address and use that. But toxcore also shouldn't send anything in clearnet when a proxy is enabled and UDP is disabled, so it should connect to those bootstrap nodes directly. I'm somewhat unfamiliar with how toxcore DHT works, but it might be the case that the TCP relay node, to which we are connected through SOCKS5 proxy, acts as a DHT relay and it expects all addresses of DHT nodes you send to it to be in IPv4 or IPv6.

@Mattel88
Copy link

@yurivict

are you up for some testing? I'd like to give you my ID in order to test it. I'd like to see the output.

@yurivict
Copy link
Member Author

Okay.

@Mattel88
Copy link

2ED2CB2E3B98714A2FFA44084CB09E6953FF45E03FBB27BF552D3525FBA5677FBA1BF733A9E4

@Mattel88
Copy link

Well I did a bit of testing with yurivict.

He was not able to see my IP because I were connected over Tor. He was just able to see all the other IPs and exit nodes so I assume it is save to use Tox over Tor.

We didn't test any DNS leaks. I do not know how to test it.

@nurupo
Copy link
Member

nurupo commented Feb 11, 2017

I had a conversation with @irungentoo on IRC about whether we really need to call tox_bootstrap() when having UDP disabled and why. The answer is yes, because in addition to TCP relays (tox_add_tcp_relay()), toxcore also needs to know addresses of UDP onion nodes in order to work correctly. The DHT, however, is not used when UDP is disabled. tox_bootstrap() function resolves the address passed to it as argument and calls onion_add_bs_node_path() and DHT_bootstrap() functions. Although calling DHT_bootstrap() is not really necessary as DHT is not used, we still need to resolve the address of the DHT node in order to populate the onion routes with onion_add_bs_node_path() call. This means that we have to implement our own getaddrinfo() that communicates with DNS servers through a SOCKS5 proxy. (Alternatively, we could modify Tox protocol to allow sending of hostnames, in addition to IPv4 and IPv6.)

@yurivict
Copy link
Member Author

Is there a documentation explaining how tox works? I understand DHT and encryption, but there are "UDP onion nodes", relay nodes, sometimes DHT isn't used. Is this explained anywhere?

@nurupo
Copy link
Member

nurupo commented Feb 11, 2017

There is a documentation written in free form by irungentoo and there is an ongoing effort of making that documentation more structured.

@emdee-is
Copy link

emdee-is commented Oct 6, 2022

This is still an important issue and needs addressing. There's a way of simplifying the problem a bit to move forward: let's assume the SOCKS proxy is Tor, which is probably the majority of users right now.

Corporate SOCKS proxies will require usernames and passwords which are not currently supported - for corporate proxies you will also need to do DNS resolution through the proxy. The normal configuration of Tor does not so we're OK for now. And the normal configuration of Tor has an IP address for the proxy so we don't have to worry about DNS lookup of the proxy.

Additionally, if you configure Tor to do so, it will run a DNS server for you, UDP (and TCP?) on any port you choose using the DNSPort 127.0.0.1:9053 config option.

So you can just write a documentation section for using Tox over Tor to describe DNSPort with Tor as the way of using Tox over Tor without leaking DNS queries if you extend ToxOptions to accept a IP/port for the DNS.
That should be easy to do. But this would be for the BS operators only, not the Tox clients using Tor.

But @nurupo went on to say

(Alternatively, we could modify Tox protocol to allow sending of hostnames, in addition to IPv4 and IPv6.)

This may be really important to do for another reason: adding the ability of Tox to easily use Tor .onion addresses for TCP_RELAY and bootstrap nodes. Adding .onion addresses is crucial for Tox as it is so easy now to block as it has so few relays and nodes. There are workarounds for this problem with Tor configuration for the node operators, but they are not straightforward and user friendly - modifying Tox protocol to allow sending of hostnames would be much better, and would be required if you were to help people in places like Iran, where the country is an Intranet, albeit with Tor.

So we can move forward by :

  1. Modifying Tox_Options to take a DNS ip/port.
  2. Modifying Tox protocol to allow sending of hostnames, including onions.

@emdee-is
Copy link

The code to do this is almost trivial: it's a oneliner sent to the tor 9050 port.

There's an implementation in Python you can follow in sTorResolve https://git.plastiras.org/emdee/toxygen_wrapper/src/branch/main/src/toxygen_wrapper/tests/support_onions.py

@nurupo
Copy link
Member

nurupo commented Feb 14, 2024

@emdee-is if it's so trivial, then feel free to open a PR.

However, I assure you that the change required to use a SOCKS5 proxy for name resolution in toxcore is far from being a trivial one-liner. It would require making addr_resolve(), and by extension addr_resolve_or_parse_ip(), asynchronous. Them not being able to return the resolved address to the caller but instead having to schedule a DNS resolution, requiring a few tox_iterate()/do_tcp_connection() loops in order to resolve it, already makes the change non-trivial as the current code expects to get the addresses resolved right there and then, in some cases even before tox_iterate()/do_tcp_connection() is called for the first time and before any proxy connection is established.


If someone decides to tackle this for the general case of using a custom user-specified DNS server, via clearnet and proxy, they would find out that there is no standard API for asynchronous DNS: glibc has getaddrinfo_a(), but that's a GNU extension and musl libc doesn't support it, and obviously Windows has its own different way of doing async DNS, and maybe macOS too, and so on. If the DNS spec is simple enough, it would be preferable to implement our own async stub DNS resolver. Otherwise we could use a 3rd party cross-platform asynchronous DNS resolution library for this, like c-ares or something else.

@emdee-is
Copy link

emdee-is commented Feb 14, 2024

@emdee-is if it's so trivial, then feel free to open a PR.

I stand corrected: it was close to trivial for me to implement in Python so I thought the same could be implemented in C. Shows you how much I know about C, and why I'll never open a PR in C. I assumed getting an answer would be no more complex than the current SOCK5 connect negotiation because it's the same port and protocol.

Still I think this is an important issue and should be mentioned in the README; many Tor users are naive about DNS leaks while using Tor, and very few with have UDP+TCP firewalls in place, which are hard to do and even harder to test. We've had help requests in IRC from people in places like Iran so it can be crucial. I'll publicize the fact that you need a firewall if you want anonymity with Tor so people don't get burned.

(I see 8 packages on my Linux that use c-ares and it runs on POSIX including arm sparc s390 and macos, Windows, and Android )

Do all the complications you listed apply to having the code get its DNS from other than 127,0.0.1:53? Tor is easily configured to provide DNS on 127.0.0.1:9053 UDP if that helps; we could "easily" add a SOCKS_DNS_HOST and SOCKS_DNS_PORT.

I leave it in your capable hands but I'd hate for it to take another 6 years for this to be fixed.

@iphydf
Copy link
Member

iphydf commented Feb 14, 2024

Clients should use the IP address of bootstrap nodes instead of host names. No leaking happens then. Proxy host similar, and it usually already is localhost.

@emdee-is
Copy link

Clients should use the IP address of bootstrap nodes instead of host names. No leaking happens then. Proxy host similar, and it usually already is localhost.

Good point and easy to arrange client side. I don't know the workings of the DHT: if my tox instance starts with only IP addresses, is there anything over time that wil cause it to DNSlook up some other addresses?

In fact I already do this in Toxygen using the code I linked to above, if Tor is asked for. In python it's a cheap and easy call and the result is cached.

Can we add this to the client spec and encourage clients to use external nodelists of IPs?

Could you set an example by changing ctest: ideally break the list of nodes out of the C code #2467 and perhaps lookup the IP's at build time? If socks_proxy is set then on a tor system there's a standalone executable called tor-resolve -4 that resolves the IP.

@iphydf
Copy link
Member

iphydf commented Feb 14, 2024

The C compiler can't do DNS lookups so we can't do that at compile time.

No, the dht never does DNS lookups outside of the client driven bootstrap call.

@emdee-is
Copy link

emdee-is commented Feb 15, 2024

The C compiler can't do DNS lookups so we can't do that at compile time.

No but the ctest run can be wrapped in a bash/python script that parses BSnodes.json and does the lookups and writes them into a file of IP:ports that auto_test.c is told to look for them in. (You might even put the C code that reads a file of IP:port BSnodes into the library so all clients can use it.) The bash script could look at socks_proxy in the environment and write the IP:ports file accordingly before ctest is run. In most cases this is a once-and-done step.

To deal with this (six year old) security issue one solution would be to make the library code deal only with IP addresses: no calls by c-toxcore to do DNS lookups. That would be the client's job, and you could write it into the client spec.

The clients could do the lookup of the nodes list, using tor-resolve or not, and cache the result. The clients could each make its decision on what async DNS library code to use, e.g. c-ares, or python code or java libraries etc. Hopefully all the clients will stop using compiled in nodelists at the same time. It's not a problem for toxygen: it already either uses it's own code for tor, or I call something for non-SOCKs. The c-toxcore stays clean and safe of this security issue.

You'd have to do some planning for this:

  • Make the decision
  • Modify the client spec
  • Get the clients to plan implementation schedules
  • Set a schedule for the phaseout of DNS lookups by c-toxcore.
  • Cut off DNS lookups and test.
  • Mark this issue as completed.

There's an advantage to doing this: for tox-in-tor

This may be really important to do for another reason: adding the ability of Tox to easily use Tor .onion addresses for TCP_RELAY and bootstrap nodes. Adding .onion addresses is crucial for Tox as it is so easy now to block as it has so few relays and nodes.

The code that resolves names to IPs in tor will handle .onion addresses for Tox-in-Tor, something I doubt c-toxcore will ever do. So you would get defacto .onion address compatability if you did this.

@nurupo
Copy link
Member

nurupo commented Feb 15, 2024

Regarding my previous comment. I gave it a bit of thought and I think it is possible to make SOCKS5 hostname lookups synchronous without breaking a lot of things. We could create a TCP_Client-like object that is just TCP socket with proxy connection logic (i.e. TCP_Client sans all the Tox crypto and handshake bits), which would be used exclusively for SOCKS5 hostname resolution within addr_resolve(). Created once and reused in all consecutive addr_resolve() calls. addr_resolve() would send a DOMAINNAME requests to the proxy (after authenticating with it if required) and block until it receives a reply with the corresponding IP address, or an error, or some timeout passes. Blocking the call awaiting a networking reply is widely inefficient than doing so asynchronously, but it should be somewhat comparable to getaddrinfo() addr_resolve() already does, albeit slower, as the initial lookup will require a TCP handshake and might require SOCKS5 authentication (with following lookups reusing the connection), and TCP is generally slower anyway.

@emdee-is
Copy link

emdee-is commented Feb 15, 2024

Regarding my previous comment. I gave it a bit of thought and I think it is possible to make SOCKS5 hostname lookups synchronous without breaking a lot of things...

I think so, especially if you cache the result and it's only probably a half dozen lookups per Tox lifetime. I looked at the C-toxcore code to do a SOCKs connect once and although I'm no good at C, I know the Python code is dead simple and must be less complex than the existing c-core SOCKs connect code. The existing connect code should be reusable with some command-response byte changes. And you have a working "reference" implementation in Python.

Tor circuits can be slow but the lookups are immediate. And if the user is using tox over tor, he's in no hurry anyway :=,)

If you go that route can you keep open to the likelihood that the node names may be .onion addresses for tox-in-tor? I don't see tox caring about what's in a name, except they are long (62b). You can do longterm mapping of onion addresses to spoofed IPs in a given reserved range in tor with a little tinkering on torrc.

@iphydf
Copy link
Member

iphydf commented Feb 15, 2024

I'd like to remove DNS code from toxcore for 0.3.0. For that, the first step would be to deprecate passing host names to bootstrap and proxy hosts (in documentation, maybe adding an options flag to make it not work so clients can test compatibility with future changes).

@Green-Sky
Copy link
Member

I agree with @iphydf here. Clients usually are in a better position to perform DNS lookups because they might already perform other types of out-of-band network connections like push notifications using more accessible libraries (@emdee-is mentioned it was pretty easy in python land).

I think we discussed this a while back in chat and also came to the conclusion to allow clients to provide their own network functions. The idea here was that toxcore would by default not provide DNS lookup functions, but a client implementer could still provide their own. Not sure if this is worth doing, considering that the client implementer can just resolve them before handing them to toxcore though.

@emdee-is
Copy link

Dropping DNS support is fine with me - toxygen already has.
And I did some testing today on .onions and although the IP resolution can be fast, there are complications as to whether the IP address is reachable (introduction points), so it's better done client side.

Can I get a yes on the idea of c-toxcore supporting reading a list of IP:port so clients can have that standardized?

Can I get a yes on the idea of c-toxcore ctests reading a list of IP:port:prot so BSnodes are broken out of automatic.c?

@robinlinden Are you putting together some release notes for the 0.2.19 release? If so, could you do a section on 'upcoming changes' and put your version of the steps I outlined? There could be a list of planned incompatabilities for 0.3.0.

At the current rate, it may be another ~6 years before 0.3.0 lands; and I'd rather this security issue isn't left open until then.

@nurupo it seems to (ignorant) me that the existing SOCKS code could be repurposed to do, admittedly blocking, tor lookups. Could you look into the possibility of doing it? Then this issue could get resolved and maybe testing can start on tox-in-tor.

@nurupo
Copy link
Member

nurupo commented Feb 17, 2024

Can I get a yes on the idea of c-toxcore supporting reading a list of IP:port so clients can have that standardized?

No. This has to be done at the client level, not in toxcore. Toxcore doesn't read files. It provides API allowing to specify IP and port, so just have clients read a file and call the appropriate API functions. Clients can decide on their own on whatever file format they want to standardize, if any -- this does not concern toxcore.

Can I get a yes on the idea of c-toxcore ctests reading a list of IP:port:prot so BSnodes are broken out of automatic.c?

There is no "automatic.c" in toxcore.

@nurupo it seems to (ignorant) me that the existing SOCKS code could be repurposed to do, admittedly blocking, tor lookups. Could you look into the possibility of doing it?

Not sure what you mean by "tor lookups". If you mean using a Tor-aware SOCKS5 proxy to connect to TCP relays via their .onion addresses by passing those .onion directly to the proxy without toxcore doing the name resolution, then yes, this is possible. This doesn't fit the current tox_add_tcp_relay() API behavior though, some sort of a Tox_Options flag or a new versions of tox_add_tcp_relay(), e.g. tox_add_tcp_relay_proxyresolve() would need to be added to make this work. It's also at odds with #469 (comment) not accepting any hostnames in API starting with 0.3.0, though maybe it could make an exception for them to be passed to proxies?

@robinlinden Are you putting together some release notes for the 0.2.19 release? If so, could you do a section on 'upcoming changes' and put your version of the steps I outlined? There could be a list of planned incompatabilities for 0.3.0.

It doesn't make much sense to announce a list of planned changes for 0.3.0 in 0.2.19's release notes if we are unsure of what they will be yet ourselves. We have some ideas, but they might change when implementing them or when they clash with other changes we want to see in 0.3.0. It's also not known when 0.3.0 will release, for all I know we might continue working on 0.2.20, 0.2.21, etc. for a while now, working on changes that don't necessitate API breakage, meanwhile doing a better job on documenting what breaking changes we want to do in 0.3.0 by opening milestoned issues so that we don't forget about them and so that we could discuss them before they are implemented. API breaking changes are important changes, having them being scattered all around IRC, groupchats, random PR and Issue discussions (like the mentioning of #469 (comment) in here), or not even not being discussed at all but being in developers' heads and then dropping as a surprise fully implemented PR, is not very good. While it works for regular changes, for the API-breaking ones we really should keep a list of them all and be able to discuss if needed. Anyway, I got a little side-tracked here.

@emdee-is
Copy link

Can I get a yes on the idea of c-toxcore supporting reading a list of IP:port so clients can have that standardized?

No. This has to be done at the client level, not in toxcore. Toxcore doesn't read files. It provides API allowing to specify IP and port, so just have clients read a file and call the appropriate API functions. Clients can decide on their own on whatever file format they want to standardize, if any -- this does not concern toxcore.

I disagree - if you had standardized support for reading a list and the format of the list, it would encourage the use of external nodelists, rather than the groddy habit of compiled in lists, which are brittle and inhibit the testing in non-canonical situtations like tox-in-tor.

Can I get a yes on the idea of c-toxcore ctests reading a list of IP:port:prot so BSnodes are broken out of automatic.c?

There is no "automatic.c" in toxcore.

My bad - I meant to say auto_test_support.c - which I referred to above in #469 (comment) :

Could you set an example by changing ctest: ideally break the list of nodes out of the C code #2467 and perhaps lookup the IP's at build time? If socks_proxy is set then on a tor system there's a standalone executable called tor-resolve -4 that resolves the IP.

grep 334 /usr/local/src/c-toxcore/auto_tests/auto_test_support.c        "tox.abilinski.com", 33445,
        "tox.initramfs.io", 33445,
        "tox.plastiras.org", 33445,
        "tox.novg.net", 33445,
    // test to get the default port 33445.
    const uint16_t start_port = lan_discovery ? 33445 : 33545;

These should be broken out into a IP:port list, and if you standardized how to do that it would encourgage clients to use that stanardized function of reading an external Nodeslist.

c-toxcore does not currently read files, but in this case, I think it should.

@emdee-is
Copy link

Not sure what you mean by "tor lookups". If you mean using a Tor-aware SOCKS5 proxy to connect to TCP relays via their .onion addresses by passing those .onion directly to the proxy without toxcore doing the name resolution, then yes, this is possible.

A "tor lookup" is when you resolve an .onion address to an IP that is stable during the lifetime of a Tor process (usually long - longer than a Tox client except maybe a BSdaemon).

tor-resolve  -4 facebookwkhpilnemxj7asaniu7vnjjbiltxjqhye3mhbshg7kx5tfyd.onion
172.18.155.197

That IP is good for the lifetime of the Tor instance and the IP is what would be passed to Tox for a BS .onion via an external nodelist for .onion BS addresses with tox-in-tor. But you also do that lookup internally by the simple query of the socks proxy which is required anyway of non-onion BS adresseses, which is needed to fix DNS leak in this issue.

This doesn't fit the current tox_add_tcp_relay() API behavior though, some sort of a Tox_Options flag or a new versions of tox_add_tcp_relay(), e.g. tox_add_tcp_relay_proxyresolve() would need to be added to make this work

New versions of tox_add_tcp_relay(), are required to fix this issue and avoid the DNS lookups that leak in current Tox over Tor usage, hence #469 (comment) That code would also resolve .onion BS addresses as an incidental side benefit.

@emdee-is
Copy link

I think this issue gets resolved by #2694 which is OK I guess - the clients will have to do the lookups, which toxygen already does.

This is an API break as it changes the structure of Tox_Options and should be merged in a new version number of c-toxcore.

@zoff99
Copy link

zoff99 commented Mar 15, 2024

trifa for android does dns lookups via tor, when you set tor (orbot) proxy.
and only gives IP addresses to toxcore. thats not a fix, but at least a mitigation for now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature for the user, not a new feature for build script P3 Low priority security Security
Projects
None yet
Development

No branches or pull requests

7 participants