-
Notifications
You must be signed in to change notification settings - Fork 37.7k
Net: Do not re-enable Onion network when it was disabled via onlynet #14425
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
Conversation
I tested also the following case: |
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ConflictsReviewers, this pull request conflicts with the following ones:
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. |
Needs rebase, but also I don't think simply removing |
Could be closed? |
Thanks for reaction. What's Your objective for closing? Is the issue solved, or do You think there is no issue, or do You think the PR is not so good, in that case any other idea? |
Rebased (I hope I made it right, was my first stale PR rebase). |
The last travis run for this pull request was 352 days ago and is thus outdated. To trigger a fresh travis build, this pull request should be closed and re-opened. |
Hi @wodry You are almost correct, those settings are total meaningless since they are true How do you like the diff, this will fix both your issue and the problem with torcontrol and different proxy settings other than 127.0.0.1:9050? diff --git a/src/init.cpp b/src/init.cpp
index 8d9566edc..00354c90f 100644
--- a/src/init.cpp
+++ b/src/init.cpp
@@ -1397,6 +1397,9 @@ bool AppInitMain(const util::Ref& context, NodeContext& node)
strSubVersion.size(), MAX_SUBVERSION_LENGTH));
}
+ // Future devs please note: the underlying structure is default all nets are reachable
+ // So here if onlynet defined we set all routeable false and then mask in all desired nets to *true*.
+ // Use IsReachable(netname) to determine later this user settings.
if (gArgs.IsArgSet("-onlynet")) {
std::set<enum Network> nets;
for (const std::string& snet : gArgs.GetArgs("-onlynet")) {
@@ -1419,7 +1422,6 @@ bool AppInitMain(const util::Ref& context, NodeContext& node)
// -proxy sets a proxy for all outgoing network traffic
// -noproxy (or -proxy=0) as well as the empty string can be used to not set a proxy, this is the default
std::string proxyArg = gArgs.GetArg("-proxy", "");
- SetReachable(NET_ONION, false);
if (proxyArg != "" && proxyArg != "0") {
CService proxyAddr;
if (!Lookup(proxyArg, proxyAddr, 9050, fNameLookup)) {
@@ -1430,16 +1432,18 @@ bool AppInitMain(const util::Ref& context, NodeContext& node)
if (!addrProxy.IsValid())
return InitError(strprintf(_("Invalid -proxy address or hostname: '%s'"), proxyArg));
- SetProxy(NET_IPV4, addrProxy);
- SetProxy(NET_IPV6, addrProxy);
- SetProxy(NET_ONION, addrProxy);
+ if (IsReachable(NET_IPV4)) SetProxy(NET_IPV4, addrProxy);
+ if (IsReachable(NET_IPV6)) SetProxy(NET_IPV6, addrProxy);
+ if (IsReachable(NET_ONION)) SetProxy(NET_ONION, addrProxy);
+ // -onlynet will at least leave one allowed, so we now set the name proxy
SetNameProxy(addrProxy);
- SetReachable(NET_ONION, true); // by default, -proxy sets onion as reachable, unless -noonion later
+ // by default, -proxy sets also an onion proxy, if allowed by -netonly flags, unless -noonion later
}
// -onion can be used to set only a proxy for .onion, or override normal proxy for .onion addresses
// -noonion (or -onion=0) disables connecting to .onion entirely
// An empty string is used to not override the onion proxy (in which case it defaults to -proxy set above, or none)
+
std::string onionArg = gArgs.GetArg("-onion", "");
if (onionArg != "") {
if (onionArg == "0") { // Handle -noonion/-onion=0
@@ -1452,10 +1456,15 @@ bool AppInitMain(const util::Ref& context, NodeContext& node)
proxyType addrOnion = proxyType(onionProxy, proxyRandomize);
if (!addrOnion.IsValid())
return InitError(strprintf(_("Invalid -onion address or hostname: '%s'"), onionArg));
- SetProxy(NET_ONION, addrOnion);
- SetReachable(NET_ONION, true);
+ if (IsReachable(NET_ONION)) {
+ SetProxy(NET_ONION, addrOnion);
+ } else return InitError(strprintf(_("Invalid request network not allowed: '%s'"), onionArg));
}
}
+ for (int n = 0; n < NET_MAX; n++) {
+ Network net = (enum Network)n;
+ if (net != NET_UNROUTABLE) LogPrintf("Net [%s] outbound allowed %d\n", GetNetworkName(net), IsReachable(net));
+ }
// see Step 2: parameter interactions for more information about these
fListen = gArgs.GetBoolArg("-listen", DEFAULT_LISTEN);
diff --git a/src/torcontrol.cpp b/src/torcontrol.cpp
index 84118b36e..7bcb96d67 100644
--- a/src/torcontrol.cpp
+++ b/src/torcontrol.cpp
@@ -522,15 +522,21 @@ void TorController::auth_cb(TorControlConnection& _conn, const TorControlReply&
if (reply.code == 250) {
LogPrint(BCLog::TOR, "tor: Authentication successful\n");
- // Now that we know Tor is running setup the proxy for onion addresses
+ // Now that we know Tor is running setup the proxy for onion addresses
// if -onion isn't set to something else.
- if (gArgs.GetArg("-onion", "") == "") {
- CService resolved(LookupNumeric("127.0.0.1", 9050));
- proxyType addrOnion = proxyType(resolved, true);
- SetProxy(NET_ONION, addrOnion);
- SetReachable(NET_ONION, true);
+ // Or if we have already an outbound proxy defined. we skip this footgun gracefully.
+ proxyType addrOnion;
+ if (IsReachable(NET_ONION)) {
+ if (gArgs.GetArg("-onion", "") == "" && !GetProxy(NET_ONION, addrOnion)) {
+ CService resolved(LookupNumeric("127.0.0.1", 9050));
+ addrOnion = proxyType(resolved, gArgs.GetBoolArg("-proxyrandomize", true));
+ if (SetProxy(NET_ONION, addrOnion)) {
+ // Here the 'do not default dox the service' line?
+ SetNameProxy(addrOnion);
+ LogPrintf("tor: Default outbound tor .onion address proxy is set to 127.0.0.1:9050\n");
+ }
+ }
}
-
// Finally - now create the service
if (private_key.empty()) // No private key, generate one
private_key = "NEW:RSA1024"; // Explicitly request RSA1024 - see issue #9214 |
@Saibato I had created this PR quite long ago and have forgotten what this is all about in detail, and it looks according to You that things changed also. So, please feel free to create a new PR, You could mention this PR there, and this PR could be closed then. |
Yup, will mention this. you clearly found and fixed a missbehave of the node that is not fixed since #7553 and your fix would have worked then. Things change a bit and I will add a fix to the PR #19358. Thx. 👍 |
🐙 This pull request conflicts with the target branch and needs rebase. Want to unsubscribe from rebase notifications on this pull request? Just convert this pull request to a "draft". |
|
Fixes #13378
If set
onlynet=ipv4
oronlynet=ipv6
(implicating the attempt to disable onion network) without this patch, Bitcoin Core would re-enable onion network (setting onion network to status "reachable", when it was initially set to "unreachable" (aka "limited" in the code). This happened whenproxy
oronion
parameters where set, or whenlistenonion
was set to 1 (which is the default).I think, the users initial choice of disabling onion network should be honored instead of quietly being overwritten.
I have testet with
onlynet=ipv4
onlynet=ipv6
onlynet=onion
(each for itself, no multiple
onlynet
settings)and without specifying
onlynet
With this patch, the node only connects to the specified network (or to all networks in the last test case)