net: require a dedicated bind for automatic Tor #36170

l0rinc commented at 9:25 PM on September 4, 2026: contributor

Problem: When the automatic onion service has no dedicated -bind=<addr>=onion, Tor forwards incoming connections to the first normal P2P bind. A wildcard onion bind also fails to identify incoming Tor connections. In either case, network classification is incorrect and Tor peers may inherit permissions granted to the Tor daemon's IP address.

Fix: Refuse startup when -listenonion is enabled and an explicit -bind configuration lacks a dedicated, non-wildcard onion bind or includes a wildcard onion bind, following [the recommendation in #34892](/bitcoin-bitcoin/34892/#issuecomment-5541662236). Users should add a specific -bind=<addr>=onion or set -listenonion=0.

Because -listenonion is enabled by default when listening, existing configurations with only a normal -bind must also be updated. Nodes without explicit -bind options retain the default dedicated onion listener.

test: give Tor tests dedicated onion binds

Several functional tests exercise Tor behavior without relying on a normal bind shared with the automatic onion service.

Configure dedicated onion binds for those scenarios. This prepares the tests for rejecting shared Tor binds.

Co-authored-by: HouseOfHufflepuff <ahrens@gmail.com>

f004c68d80

DrahtBot added the label P2P on Sep 4, 2026

DrahtBot commented at 9:25 PM on September 4, 2026: contributor

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

External sites

Code coverage & benchmarks: https://corecheck.dev/bitcoin/bitcoin/pulls/36170
Ralph review <img src=https://review.fish.foo/assets/bitcoin/bitcoin/36170.png alt="Ralph review status"> https://git.fish.foo/bitcoin/bitcoin/pulls/36170#author-ralph

Reviews

See the guideline and AI policy for information on the review process.

Type	Reviewers
Concept ACK	jeanpablojp
Approach ACK	winterrdog, vasild

If your review is incorrectly listed, please copy-paste <code></code> into the comment that the bot should ignore.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#36257 <sub><img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/36257.svg"></sub> (qa: assert_equals -> assert_true/assert_false by hodlinator)

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.

jeanpablojp commented at 12:55 AM on September 6, 2026: contributor

Concept ACK

0.0.0.0 is the default for -bind itself, and -bind=0.0.0.0:8334=onion passes the new check while tagging nothing. addr_bind comes from GetBindAddress on the accepted socket, so what CreateNodeFromAcceptedSocket compares against m_onion_binds is the concrete address the peer reached.

Same node, same arriving address, same -whitelist. With -bind=0.0.0.0:P=onion the inbound peer is classified by its IP network in getpeerinfo and keeps its noban. Spelling that same address out instead of the wildcard gives onion with no permissions. [::] is classified the same way, and either wildcard is what reaches Tor as the ADD_ONION target.

Rejecting a wildcard onion bind would close that. Naming 127.0.0.1:8334=onion in the message would only make it less likely. Is the first one in scope here?

in doc/release-notes-36170.md:9 in 7fb68e3927

   0 | @@ -0,0 +1,9 @@
   1 | +P2P and network changes
   2 | +-----------------------
   3 | +
   4 | +Nodes configured with `-bind` but no dedicated `-bind=<addr>=onion` now refuse
   5 | +to start when `-listenonion` is enabled, which is the default when listening.
   6 | +A shared bind cannot distinguish Tor-forwarded connections from direct connections,
   7 | +which can grant Tor peers unintended IP-based whitelist permissions.
   8 | +Users should add a dedicated onion bind to accept incoming Tor connections, or set
   9 | +`-listenonion=0` to disable automatic onion service creation.

jeanpablojp commented at 12:55 AM on September 6, 2026:

The release note covers upgraders. For someone setting Tor up from scratch, the -bind help advertises default: 127.0.0.1:8334=onion, which still holds with no explicit -bind but not once one is given, and section 2 of doc/tor.md does not mention the dedicated bind. Would a line in one of the two be worth it?

l0rinc commented at 5:17 AM on September 24, 2026:

Thanks, added this to both the -bind help and section 2 of doc/tor.md. They now explain when the default onion bind is added and when an explicit, non-wildcard =onion bind is required.

in test/functional/feature_bind_extra.py:103 in 7fb68e3927

  96 | @@ -95,6 +97,14 @@ def run_test(self):
  97 |  
  98 |          self.stop_node(0)
  99 |  
 100 | +        self.log.info("Test -listenonion with a normal bind and no dedicated onion bind")
 101 | +        self.stop_node(2)
 102 | +        assert_raises(FailedToStartError, self.start_node, 2, extra_args=self.expected[2][0] + ["-listenonion=1"])
 103 | +        self.nodes[2].wait_until_stopped(expected_ret_code=1, expected_stderr=re.compile("Tor onion service cannot share"))

jeanpablojp commented at 12:55 AM on September 6, 2026:

nit: this could use the helper the file already uses further down, and then re, assert_raises and FailedToStartError drop out of the imports. It does loosen the exit status from exactly 1 to any non-zero. Ran it that way and it passes.

        self.nodes[2].assert_start_raises_init_error(
                    self.expected[2][0] + ["-listenonion=1"],
                    "Error: The Tor onion service cannot share a -bind address",
                    match=ErrorMatch.PARTIAL_REGEX)

l0rinc commented at 5:18 AM on September 24, 2026:

Switched to assert_start_raises_init_error, thanks.

winterrdog commented at 7:12 AM on September 8, 2026: contributor

Approach ACK

l0rinc force-pushed on Sep 24, 2026

l0rinc commented at 5:19 AM on September 24, 2026: contributor

Rejecting a wildcard onion bind would close that. Naming 127.0.0.1:8334=onion in the message would only make it less likely. Is the first one in scope here?

Thanks , added a wildcard-bind check and test (added you as coauthor), but I’d appreciate a second look from e.g. @vasild who has more experience with this part of the networking code than I do.

in test/functional/feature_bind_extra.py:101 in 69116ee97c

  94 | @@ -95,12 +95,28 @@ def run_test(self):
  95 |  
  96 |          self.stop_node(0)
  97 |  
  98 | +        self.log.info("Test -listenonion with a normal bind and no dedicated onion bind")
  99 | +        self.stop_node(2)
 100 | +        self.nodes[2].assert_start_raises_init_error(
 101 | +            self.expected[2][0] + ["-listenonion=1", "-torcontrol=127.0.0.1:1"],

vasild commented at 11:31 AM on October 1, 2026:

            self.extra_args[2] + ["-listenonion=1", "-torcontrol=127.0.0.1:1"],

(not tested)

in test/functional/feature_bind_extra.py:106 in 69116ee97c

 101 | +            self.expected[2][0] + ["-listenonion=1", "-torcontrol=127.0.0.1:1"],
 102 | +            "Error: The automatic Tor onion service requires a dedicated onion bind when -bind is set. Use a specific address such as -bind=127.0.0.1:<port>=onion, or disable the service with -listenonion=0.",
 103 | +        )
 104 | +
 105 | +        self.log.info("Test -bind with dedicated onion bind starts when -listenonion=1")
 106 | +        self.restart_node(1, extra_args=self.expected[1][0] + ["-listenonion=1", "-torcontrol=127.0.0.1:1"])

vasild commented at 11:32 AM on October 1, 2026:

        self.restart_node(1, extra_args=self.extra_args[1] + ["-listenonion=1", "-torcontrol=127.0.0.1:1"])

(not tested)

in src/init.cpp:1066 in 69116ee97c

1061 | @@ -1060,6 +1062,11 @@ bool AppInitParameterInteraction(const ArgsManager& args)
1062 |          return InitError(Untranslated("Cannot set -listen=0 together with -listenonion=1"));
1063 |      }
1064 |  
1065 | +    const auto binds{args.GetArgs("-bind")};
1066 | +    if (args.GetBoolArg("-listenonion", DEFAULT_LISTEN_ONION) && binds.size() && std::ranges::none_of(binds, [](auto& b) { return b.ends_with("=onion"); })) {

vasild commented at 11:38 AM on October 1, 2026:

    if (args.GetBoolArg("-listenonion", DEFAULT_LISTEN_ONION) && !binds.empty() && std::ranges::none_of(binds, [](auto& b) { return b.ends_with("=onion"); })) {

l0rinc commented at 5:34 AM on October 7, 2026:

I prefer non-negated versions, it was deliberate

in src/init.cpp:1067 in 69116ee97c

1061 | @@ -1060,6 +1062,11 @@ bool AppInitParameterInteraction(const ArgsManager& args)
1062 |          return InitError(Untranslated("Cannot set -listen=0 together with -listenonion=1"));
1063 |      }
1064 |  
1065 | +    const auto binds{args.GetArgs("-bind")};
1066 | +    if (args.GetBoolArg("-listenonion", DEFAULT_LISTEN_ONION) && binds.size() && std::ranges::none_of(binds, [](auto& b) { return b.ends_with("=onion"); })) {
1067 | +        return InitError(_("The automatic Tor onion service requires a dedicated onion bind when -bind is set. Use a specific address such as -bind=127.0.0.1:<port>=onion, or disable the service with -listenonion=0."));

vasild commented at 11:45 AM on October 1, 2026:

        return InitError(_("The automatic Tor onion service requires a dedicated onion bind. Use a specific address such as -bind=127.0.0.1:<port>=onion, or disable the service with -listenonion=0."));

With this PR the automatic Tor service requires a dedicated bind, always, unconditionally. Not only "when -bind is set".

To clarify - if the user did not set -bind, then the defaults do provide both -bind and -bind=...=onion.

in src/init.cpp:2281 in 69116ee97c outdated

2284 |      }
2285 |  
2286 |      if (listenonion) {
2287 | +        if (connOptions.onion_binds.empty()) {
2288 | +            return InitError(_("The automatic Tor onion service requires a dedicated onion bind when -bind is set. Use a specific address such as -bind=127.0.0.1:<port>=onion, or disable the service with -listenonion=0."));
2289 | +        }

vasild commented at 12:17 PM on October 1, 2026:

Isn't this check redundant? Seems to be the same as the added check in AppInitParameterInteraction(). Better have just one. I prefer this one because it uses connOptions.onion_binds. The other one manually "parses" the arguments and checks for =onion suffix which is redundant with the code that sets connOptions.onion_binds. That is, if =onion would have to be changed to e.g. to support case insensitive match, then it would have to be changed in two places, not nice.

l0rinc commented at 5:30 AM on October 7, 2026:

I originally added it to reject the configuration before loading chainstate, but I agree that duplicating the argument parsing just for this case isn’t worth it - removed, thanks

in src/init.cpp:2284 in 69116ee97c outdated

2287 | +        if (connOptions.onion_binds.empty()) {
2288 | +            return InitError(_("The automatic Tor onion service requires a dedicated onion bind when -bind is set. Use a specific address such as -bind=127.0.0.1:<port>=onion, or disable the service with -listenonion=0."));
2289 | +        }
2290 | +        if (std::ranges::any_of(connOptions.onion_binds, [](auto& b) { return b.IsBindAny(); })) {
2291 | +            return InitError(_("The automatic Tor onion service cannot use a wildcard onion bind because incoming Tor connections would not be identified. Use a specific address such as -bind=127.0.0.1:<port>=onion, or disable the service with -listenonion=0."));
2292 | +        }

vasild commented at 12:24 PM on October 1, 2026:

This message is technically not correct. If wildcard address is used, incoming Tor connections will still be identified just fine. For example: -bind=1.2.3.4:8333 -bind=0.0.0.0:8334=onion - anything that arrives on port 8334 will be correctly identified as incoming Tor.

The problem, however, is that 0.0.0.0:8334 in addition to being used for binding and classifying connections is also used to instruct the Tor daemon to forward incoming connections to it. And telling the Tor daemon to forward connections to 0.0.0.0:8334 is ill.

So, maybe something like: "The automatic Tor onion service cannot use a wildcard onion bind because the Tor daemon wouldn't be able to forward incoming connections to us. Use a specific address such as -bind=127.0.0.1:<port>=onion, or disable the service with -listenonion=0."

Or just: "The automatic Tor onion service cannot use a wildcard onion bind. Use a specific address such as -bind=127.0.0.1:<port>=onion, or disable the service with -listenonion=0."

l0rinc commented at 5:31 AM on October 7, 2026:

Used your shorter version, thanks.

in doc/release-notes-36170.md:4 in 69116ee97c

   0 | @@ -0,0 +1,8 @@
   1 | +P2P and network changes
   2 | +-----------------------
   3 | +
   4 | +Nodes configured with `-bind` but no specific `-bind=<addr>=onion` now refuse to start when `-listenonion` is enabled.

vasild commented at 12:33 PM on October 1, 2026:

Nodes configured with `-bind` but no specific `-bind=<addr:port>=onion` now refuse to start when `-listenonion` is enabled.

in doc/release-notes-36170.md:6 in 69116ee97c outdated

   0 | @@ -0,0 +1,8 @@
   1 | +P2P and network changes
   2 | +-----------------------
   3 | +
   4 | +Nodes configured with `-bind` but no specific `-bind=<addr>=onion` now refuse to start when `-listenonion` is enabled.
   5 | +This includes nodes without Tor configured, since `-listenonion` is enabled by default when listening.
   6 | +Shared and wildcard binds cannot distinguish Tor-forwarded connections from direct connections, which can grant Tor peers unintended IP-based whitelist permissions.

vasild commented at 12:35 PM on October 1, 2026:

Shared binds cannot distinguish Tor-forwarded connections from direct connections, which can grant Tor peers unintended IP-based whitelist permissions.

same reasoning as in the comment above, wildcard can distinguish.

l0rinc commented at 5:32 AM on October 7, 2026:

I kept "wildcard" here because the IPv4 and IPv6 checks described above both showed misclassification and retained whitelist permissions.

in doc/release-notes-36170.md:8 in 69116ee97c

   0 | @@ -0,0 +1,8 @@
   1 | +P2P and network changes
   2 | +-----------------------
   3 | +
   4 | +Nodes configured with `-bind` but no specific `-bind=<addr>=onion` now refuse to start when `-listenonion` is enabled.
   5 | +This includes nodes without Tor configured, since `-listenonion` is enabled by default when listening.
   6 | +Shared and wildcard binds cannot distinguish Tor-forwarded connections from direct connections, which can grant Tor peers unintended IP-based whitelist permissions.
   7 | +Nodes without `-bind`, including those using only `-whitebind`, continue to get the default onion target.
   8 | +Users should add a specific `-bind=127.0.0.1:<port>=onion` to accept incoming Tor connections, or set `-listenonion=0` to disable automatic onion service creation.

vasild commented at 1:03 PM on October 1, 2026:

The impression is that 127.0.0.1 must be used.

Users should add a specific `-bind=<addr:port>=onion` to accept incoming Tor connections, or set `-listenonion=0` to disable automatic onion service creation.

vasild commented at 1:03 PM on October 1, 2026: contributor

Approach ACK 69116ee97cefa089f9eb6665918fb6c8b1ff62d4

knst referenced this in commit 5f40d56f4d on Oct 5, 2026

schildbach commented at 2:21 PM on October 6, 2026: contributor

Keep in mind that when running in a container, it is considered best practise to bind to 0.0.0.0 / [::] rather than a specific IP. That is because most container engines (e.g. Docker) use dynamic IPs.

The issue that 0.0.0.0 / [::] are unsuitable to pass via Tor control should imho be solved by a second configuration option, as suggested in #25094.

PastaPastaPasta referenced this in commit d721befcf0 on Oct 6, 2026

PastaPastaPasta referenced this in commit 4eb82f6c6d on Oct 7, 2026

PastaPastaPasta referenced this in commit 0364ed626e on Oct 7, 2026

test: characterize Tor bind startup

Record that shared and wildcard onion binds currently allow startup so both configurations can be rejected by the fix.

Co-authored-by: HouseOfHufflepuff <ahrens@gmail.com>
Co-authored-by: JP <jeanpablo.jp@hotmail.com>

400ad822a8

init: reject shared and wildcard Tor binds

Tor peers arriving through shared or wildcard binds may be classified by IP and inherit address-based whitelist permissions.
Reject those configurations when the automatic onion service is enabled.

Because `-listenonion` is enabled by default when listening, existing plain `-bind` configurations also need a dedicated onion bind or `-listenonion=0`, even if they do not use Tor.

Co-authored-by: Vasil Dimov <vd@FreeBSD.org>
Co-authored-by: JP <jeanpablo.jp@hotmail.com>

079a29da46

refactor: remove obsolete Tor bind fallback

With shared binds rejected, select the onion target directly and keep the default target for configurations without `-bind`.

Co-authored-by: Vasil Dimov <vd@FreeBSD.org>

4f537f8580

doc: note dedicated Tor bind requirement 5527295cb4

l0rinc force-pushed on Oct 7, 2026

l0rinc commented at 5:40 AM on October 7, 2026: contributor

Addressed @vasild’s suggestions: reused the configured test arguments, removed the duplicate early check, shortened the errors, and clarified the release notes.

The issue that 0.0.0.0 / [::] are unsuitable to pass via Tor control should imho be solved by a second configuration option

I'd keep rejecting these configurations in this PR and handle that support separately. What do you think @vasild?