correct -onion default to -proxy behavior #14729

qubenix · 2018-11-15T04:06:11Z

fixes -onion does not default to -proxy #14722
only use default tor ip:port (127.0.0.1:9050) if -onion and -proxy not set

I'm not sure if this is done correctly, but it does fix the behavior in my tests. I will not be offended if this gets rejected for a more appropriate method.

qubenix · 2018-11-15T16:59:12Z

Pushed a fix to the comment to reflect the new behavior and rebased.

maflcko · 2018-11-15T17:03:34Z

src/torcontrol.cpp

-        if (gArgs.GetArg("-onion", "") == "") {
+        // Now that we know Tor is running, setup the proxy for onion addresses
+        // if -onion and -proxy isn't set to something else.
+        if (gArgs.GetArg("-onion", "-proxy") == "") {


The default must be set to a value, not the name of the arg it should default to.

I think I might have fixed it now.

@MarcoFalke I noticed in another spots this was handled differently. Would it be more preferable like this:

// Now that we know Tor is running setup the proxy for onion addresses // if -onion isn't set to something else. std::string onion = gArgs.GetArg("-onion", ""); std::string proxy = gArgs.GetArg("-proxy", ""); if (onion.empty() && proxy.empty()) {

- fixes #14722 - only use default tor ip:port (127.0.0.1:9050) if -onion and -proxy not set - fix comment to reflect new behavior

qubenix · 2018-11-15T20:30:06Z

Thank you for reviewing.

I think I may have done this the correct way now. If not, I will stop wasting your time and close this so someone more suited can take it up.

bitcoin/src/torcontrol.cpp

Lines 525 to 527 in 38640a0

    
           // Now that we know Tor is running, setup the proxy for onion addresses 
        
           // if -onion and -proxy isn't set to something else. 
        
           if (gArgs.GetArg("-onion", "") == "" && gArgs.GetArg("-proxy", "") == "") {

laanwj · 2018-11-23T07:56:18Z

src/torcontrol.cpp

-        if (gArgs.GetArg("-onion", "") == "") {
+        // Now that we know Tor is running, setup the proxy for onion addresses
+        // if -onion and -proxy isn't set to something else.
+        if (gArgs.GetArg("-onion", "") == "" && gArgs.GetArg("-proxy", "") == "") {


I guess this will work in most cases.

Though ideally we'd query Net's state directly here for "is there a proxy for onion already" instead of indirectly divising it from the arguments. This would really solve the problem of the current code here.

I guess this will work in most cases.

Though ideally we'd query Net's state directly here for "is there a proxy for onion already" instead of indirectly divising it from the arguments. This would really solve the problem of the current code here.

Hi, @qubenix

diff --git a/src/torcontrol.cpp b/src/torcontrol.cpp index bec64fbf2..b632513c3 100644 --- a/src/torcontrol.cpp +++ b/src/torcontrol.cpp @@ -524,7 +524,8 @@ void TorController::auth_cb(TorControlConnection& _conn, const TorControlReply& // Now that we know Tor is running, setup the proxy for onion addresses // if -onion and -proxy isn't set to something else. - if (gArgs.GetArg("-onion", "") == "" && gArgs.GetArg("-proxy", "") == "") { + proxyType addrOnion; + if (gArgs.GetArg("-onion", "") == "" && !GetProxy(NET_ONION, addrOnion)) { CService resolved(LookupNumeric("127.0.0.1", 9050)); proxyType addrOnion = proxyType(resolved, true); SetProxy(NET_ONION, addrOnion);

This litle patch should do the trick and fix the problem and that should be in line what @laanwj suggested, please feel free to copy paste. ;-)

DrahtBot · 2019-02-15T23:31:04Z

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, this pull request conflicts with the following ones:

torcontrol: Create also a V3 ed25519-V3 onion address. #19485 (torcontrol: Create also a V3 ed25519-V3 onion address. by Saibato)
torcontrol : avoid to set wrong outbound proxy and network settings when creating an inbound onion service. #19358 (net: Make sure we do not override proxy settings in hidden service. by Saibato)
torcontrol: Query Tor for correct -onion configuration #15423 (torcontrol: Query Tor for correct -onion configuration by luke-jr)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

luke-jr · 2019-02-16T17:24:23Z

I think when using the Tor controller, we probably want to use the Tor's own listening config (see #15423) over a -proxy setting...

qubenix · 2019-02-18T23:41:13Z

@luke-jr Could be better to listen to the control port, just don't forget about situations where onion-grater is used and the response from the Tor controller intentionally lies to the application. Onion-grater use is considered a Tor best practice and is being used in Whonix and TAILS.

In Whonix there is a dedicated port for Bitcoin traffic, <gateway-ip>:9111, but if you rely on the control port you get a response like this:

user@host:~$ nc -nv 127.0.0.1 9051
(UNKNOWN) [127.0.0.1] 9051 (?) open
AUTHENTICATE
250 OK
GETINFO net/listeners/socks
250-net/listeners/socks="127.0.0.1:9150"
250 OK

Probably best to rely on the controller and systems using onion-grater can use a profile. Maybe an example one can be included in this repo for such systems.

This PR was only meant to correct the defined behavior of defaulting to -proxy:

-onion=ip:port
Use separate SOCKS5 proxy to reach peers via Tor hidden services, set -noonion to disable (default: -proxy)

@laanwj

Though ideally we'd query Net's state directly here for "is there a proxy for onion already" instead of indirectly divising it from the arguments. This would really solve the problem of the current code here.

Sorry, I'm not sure how to do that. I only wanted to help, but I'm afraid the existence of this PR may be keeping others from doing it the ideal way which you suggested. Just let me know if closing this is more help.

DrahtBot · 2019-08-16T13:48:54Z

The last travis run for this pull request was 273 days ago and is thus outdated. To trigger a fresh travis build, this pull request should be closed and re-opened.

adamjonas · 2020-05-18T19:48:55Z

@qubenix bumping here to see if you are planning on continuing to push this forward.

qubenix · 2020-05-18T20:22:08Z

@adamjonas I don't what I can/should be doing to push this forward. This PR fixes the bug, but if it's not done in an acceptable way then there's nothing more I can do. I'm not a coder by trade, I just found a bug and offered a solution that I was able to figure out.
That being said, I'm happy to do whatever is expected of me here as long as it's within my ability. I'm just not sure what that is.

Saibato · 2020-06-23T15:53:12Z

@adamjonas I don't what I can/should be doing to push this forward. This PR fixes the bug, but if it's not done in an acceptable way then there's nothing more I can do. I'm not a coder by trade, I just found a bug and offered a solution that I was able to figure out.
That being said, I'm happy to do whatever is expected of me here as long as it's within my ability. I'm just not sure what that is.

Hi @qubenix might take a look at #19358? I run in the same problem and found that other settings are also ignored by this footgun proxy fallback, so i took the freedom and pushed this alternative fix.

fanquake · 2020-09-29T06:39:21Z

Anyone interested in this PR should instead review #19358.

fanquake added the P2P label Nov 15, 2018

maflcko reviewed Nov 15, 2018

View reviewed changes

correct -onion default to -proxy behavior

38640a0

- fixes #14722 - only use default tor ip:port (127.0.0.1:9050) if -onion and -proxy not set - fix comment to reflect new behavior

laanwj reviewed Nov 23, 2018

View reviewed changes

DrahtBot closed this Aug 16, 2019

DrahtBot reopened this Aug 16, 2019

qubenix mentioned this pull request Sep 23, 2019

doc: Extend onion option's description #16940

Closed

This was referenced Feb 11, 2020

torcontrol: Query Tor for correct -onion configuration #15423

Merged

torcontrol: Launch a private Tor instance when not already running #15421

Closed

DrahtBot mentioned this pull request Jun 23, 2020

torcontrol : avoid to set wrong outbound proxy and network settings when creating an inbound onion service. #19358

Closed

DrahtBot mentioned this pull request Jul 23, 2020

torcontrol: Create also a V3 ed25519-V3 onion address. #19485

Closed

fanquake closed this Sep 29, 2020

bitcoin locked as resolved and limited conversation to collaborators Feb 15, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

correct -onion default to -proxy behavior #14729

correct -onion default to -proxy behavior #14729

Uh oh!

qubenix commented Nov 15, 2018

Uh oh!

qubenix commented Nov 15, 2018 •

edited

Loading

Uh oh!

maflcko Nov 15, 2018

Uh oh!

qubenix Nov 15, 2018

Uh oh!

qubenix Nov 16, 2018

Uh oh!

qubenix commented Nov 15, 2018

Uh oh!

laanwj Nov 23, 2018

Uh oh!

Saibato Jun 26, 2020

Uh oh!

DrahtBot commented Feb 15, 2019 •

edited

Loading

Uh oh!

luke-jr commented Feb 16, 2019

Uh oh!

qubenix commented Feb 18, 2019

Uh oh!

DrahtBot commented Aug 16, 2019

Uh oh!

adamjonas commented May 18, 2020

Uh oh!

qubenix commented May 18, 2020

Uh oh!

Saibato commented Jun 23, 2020

Uh oh!

fanquake commented Sep 29, 2020

Uh oh!

Uh oh!

correct -onion default to -proxy behavior #14729

correct -onion default to -proxy behavior #14729

Uh oh!

Conversation

qubenix commented Nov 15, 2018

Uh oh!

qubenix commented Nov 15, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

maflcko Nov 15, 2018

Choose a reason for hiding this comment

Uh oh!

qubenix Nov 15, 2018

Choose a reason for hiding this comment

Uh oh!

qubenix Nov 16, 2018

Choose a reason for hiding this comment

Uh oh!

qubenix commented Nov 15, 2018

Uh oh!

laanwj Nov 23, 2018

Choose a reason for hiding this comment

Uh oh!

Saibato Jun 26, 2020

Choose a reason for hiding this comment

Uh oh!

DrahtBot commented Feb 15, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Conflicts

Uh oh!

luke-jr commented Feb 16, 2019

Uh oh!

qubenix commented Feb 18, 2019

Uh oh!

DrahtBot commented Aug 16, 2019

Uh oh!

adamjonas commented May 18, 2020

Uh oh!

qubenix commented May 18, 2020

Uh oh!

Saibato commented Jun 23, 2020

Uh oh!

fanquake commented Sep 29, 2020

Uh oh!

Uh oh!

qubenix commented Nov 15, 2018 •

edited

Loading

DrahtBot commented Feb 15, 2019 •

edited

Loading