net: Warn when Tor onion service lacks a dedicated onion bind #34892

pull HouseOfHufflepuff wants to merge 1 commits into bitcoin:master from HouseOfHufflepuff:net-warn-bind-missing-onion-socket changing 3 files +21 −1
  1. HouseOfHufflepuff commented at 0:35 am on March 22, 2026: contributor

    Summary

    When -bind is specified without a dedicated onion socket (-bind=<addr>=onion) but -listenonion is enabled, the auto-created Tor hidden service routes incoming connections to the regular bind address. This makes it impossible to distinguish incoming Tor connections from regular IPv4 connections, causing -netinfo to report 0 onion connections even when they exist.

    This adds a startup warning to alert users so they can add -bind=127.0.0.1:8334=onion for accurate connection tracking.

    Fixes #33458

    Test plan

    Start bitcoind with -bind but without a dedicated onion bind:

    0./build/bin/bitcoind -bind=0.0.0.0:8333 -listenonion=1
    

    Before this change: No warning is emitted. Users have no indication that incoming Tor connections will be misidentified.

    After this change: A warning is printed at startup:

    0Warning: The Tor onion service is being directed to a -bind address without a dedicated onion socket (-bind=<addr>=onion). Incoming Tor connections will not be identified as onion connections.
    

    The warning does not appear when a dedicated onion bind is provided:

    0./build/bin/bitcoind -bind=0.0.0.0:8333 -bind=127.0.0.1:8334=onion -listenonion=1
    

    Functional test feature_proxy.py updated to expect the new warning.

  2. DrahtBot added the label P2P on Mar 22, 2026
  3. DrahtBot commented at 0:36 am on March 22, 2026: contributor

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

    Reviews

    See the guideline for information on the review process. A summary of reviews will appear here.

  4. HouseOfHufflepuff commented at 0:36 am on March 22, 2026: contributor
  5. DrahtBot added the label CI failed on Mar 23, 2026
  6. kevkevinpal commented at 8:40 pm on March 23, 2026: contributor

    Looks like the functional test feature_proxy.py is failing

    You can fix it by modifying this bit of code to looks like this

    0self.log.info("Test passing -onlynet=onion without -proxy or -onion but with -listenonion=1 is ok")
    1self.start_node(1, extra_args=["-onlynet=onion", "-listenonion=1"])
    2- self.stop_node(1)
    3+ self.stop_node(1, expected_stderr="Warning: The Tor onion service is being directed to a -bind address without a dedicated on  ion socket (-bind=<addr>=onion). Incoming Tor connections will not be identified as onion connections.")
    
  7. HouseOfHufflepuff commented at 9:33 pm on March 23, 2026: contributor
    Thanks @kevkevinpal! Good catch — applied the fix to feature_proxy.py to pass expected_stderr with the new warning message on stop_node. Test passes locally now. Ready for re-review.
  8. maflcko commented at 5:52 am on March 24, 2026: member

    Please squash your commits according to https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#squashing-commits

    Also, the test plan is clearly wrong.

    Was this LLM generated? What are the steps to test this? What is the output before and after the changes here?

  9. HouseOfHufflepuff commented at 2:40 pm on March 24, 2026: contributor
    Sorry about that @maflcko — you’re right, the test plan was inadequate and there were unrelated changes that shouldn’t have been in the branch. I’ve squashed into a single commit, removed the unrelated changes, and updated the PR description with actual before/after output. Should be ready for another look when you have time.
  10. HouseOfHufflepuff force-pushed on Mar 24, 2026
  11. kevkevinpal commented at 3:49 pm on March 24, 2026: contributor
    I think it would be good to add a test asserting that this warning gets logged. In addition to modifying the existing test that was failing.
  12. maflcko commented at 6:22 am on March 25, 2026: member
    You’ll have to run the tests locally before pushing to CI
  13. HouseOfHufflepuff force-pushed on Mar 25, 2026
  14. net: Warn when Tor onion service lacks a dedicated onion bind
    When -bind is specified without a dedicated -bind=<addr>=onion, the
    auto-created Tor hidden service routes incoming connections to the
    regular bind address. This makes it impossible to distinguish incoming
    Tor connections from regular IPv4 connections, causing -netinfo to
    report 0 onion connections even when they exist.
    
    Add a startup warning to alert users to this misconfiguration so they
    can add an explicit -bind=127.0.0.1:8334=onion if they want accurate
    connection tracking.
    
    fixes #33458
    2e63783ae1
  15. HouseOfHufflepuff force-pushed on Mar 25, 2026
  16. DrahtBot removed the label CI failed on Mar 25, 2026
  17. HouseOfHufflepuff commented at 9:13 pm on March 25, 2026: contributor
    @maflcko Apologies for the messy state earlier — commits are squashed, unrelated changes removed, and tests verified locally before pushing. Lesson learned on the workflow. Ready for re-review whenever you have time.
  18. HouseOfHufflepuff commented at 9:15 pm on March 25, 2026: contributor
    @kevkevinpal Thanks for the proxy test catch and the suggestion to add a warning assertion — both are addressed now. CI is passing clean. Would appreciate another look when you have a moment.
  19. naiyoma commented at 11:15 am on March 27, 2026: contributor
    I’m not sure about this approach. I think the right direction would be to try to get the count right. I’ve explained what I think the two options might be -> #33458 (comment)
  20. naiyoma commented at 10:56 am on March 30, 2026: contributor

    I’m not sure about this approach. I think the right direction would be to try to get the count right. I’ve explained what I think the two options might be -> #33458 (comment)

    I was wrong about the alternative approaches I suggested. I don’t think we can get the correct count. This approach is fine.

  21. in test/functional/feature_proxy.py:439 in 2e63783ae1
    435@@ -436,6 +436,17 @@ def networks_dict(d):
    436 
    437         self.log.info("Test passing -onlynet=onion without -proxy or -onion but with -listenonion=1 is ok")
    438         self.start_node(1, extra_args=["-onlynet=onion", "-listenonion=1"])
    439+        self.stop_node(1, expected_stderr="Warning: The Tor onion service is being directed to a -bind address without a dedicated onion socket (-bind=<addr>=onion). Incoming Tor connections will not be identified as onion connections.")
    


    naiyoma commented at 11:15 am on March 30, 2026:
    Nit: IMO this test would be better placed in feature_bind_extra.py rather than in this file. feature_bind_extra.py also sets bind_to_localhost_only = False, meaning we can explicitly control -bind configuration and not rely on the default. I initially found it confusing why the test was passing, and then I realized that by default, bind_to_localhost_only is true.
  22. naiyoma commented at 11:28 am on March 30, 2026: contributor

    and when using -netinfo it should show a “?” or “-” instead of “0”.

    This is a good suggestion from the issue have you considered implementing it?


github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-03-30 12:13 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me