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

Setting torcontrol overrides proxy address #25265

Open
kroese opened this issue Jun 1, 2022 · 7 comments
Open

Setting torcontrol overrides proxy address #25265

kroese opened this issue Jun 1, 2022 · 7 comments

Comments

@kroese
Copy link

kroese commented Jun 1, 2022

When using Bitcoin in combination with Tor, to generate a hidden service to allow for inbound connections, it is needed to add torcontrol like this:

proxy=1.2.3.4:9050
torcontrol=1.2.3.4:29501

The problem is that adding torcontrol results in overriding the proxy address with 127.0.0.1 and the end-result is that Bitcoin cannot connect to Tor any longer:

connect() to 127.0.0.1:9050 failed after wait: Connection refused (111)

It should have connected to 1.2.3.4 instead.

So why does adding torcontrol overrides the configured address for proxy?

@kroese kroese added the Bug label Jun 1, 2022
@kroese kroese changed the title Setting torcontrol overrides proxy Setting torcontrol overrides proxy address Jun 1, 2022
@fjahr
Copy link
Contributor

fjahr commented Jun 19, 2022

@kroese can you share any relevant logs outputs and also what other settings you are using that are networking related?

@laanwj laanwj added the P2P label Jun 20, 2022
@laanwj
Copy link
Member

laanwj commented Jun 20, 2022

So why does adding torcontrol overrides the configured address for proxy?

To be precise, it overrides the specific proxy for NET_ONION, e.g. -onion.
See https://github.com/bitcoin/bitcoin/blob/master/src/torcontrol.cpp#L385

I guess it shouldn't be doing so if an explicit one was provided on the command line.

@fjahr
Copy link
Contributor

fjahr commented Jun 20, 2022

So why does adding torcontrol overrides the configured address for proxy?

To be precise, it overrides the specific proxy for NET_ONION, e.g. -onion. See https://github.com/bitcoin/bitcoin/blob/master/src/torcontrol.cpp#L385

I guess it shouldn't be doing so if an explicit one was provided on the command line.

That's a good hint! I will look into it unless you have already started working on it.

@laanwj
Copy link
Member

laanwj commented Jun 21, 2022

No I haven't, feel free to pick it up!

@luke-jr
Copy link
Member

luke-jr commented Jul 26, 2022

If you want to override -torcontrol, you should be using -onion. Overriding -proxy is expected behaviour (otherwise why use -torcontrol at all?).

@fjahr
Copy link
Contributor

fjahr commented Jul 26, 2022

If you want to override -torcontrol, you should be using -onion. Overriding -proxy is expected behaviour (otherwise why use -torcontrol at all?).

To me, this doesn't make sense. -onion is the more specific version of -proxy, so it makes sense that -onion overrides -proxy. But the interactions with any other parameter should be consistent between the two. Everything else is very confusing.

@vasild
Copy link
Contributor

vasild commented Aug 18, 2022

This was briefly discussed in #15423 (comment).
Related PR: #22830.

IMO it is ok that the auto-discovered Tor proxy overrides the generic one from -proxy because -proxy could be just an ordinary SOCKS5 proxy unrelated to the Tor network. If that is not ok, then the user has the option to override the auto-discovered one by using -onion. That is,
-proxy < -torcontrol (auto-discover) < -onion
looks reasonable to me.

If it is changed to
-torcontrol (auto-discover) < -proxy < -onion
then there is no way for the user to setup a configuration that uses auto-discovery + a manually specified non-Tor-related SOCKS5 proxy (with -proxy). The auto-discovery is nice because it returns a correct location, even after configuration changes in the network or the Tor daemon, without user interaction.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

12 participants
@laanwj @vasild @luke-jr @fjahr @kroese and others