No check is done whether the binding for Tor succeeded before using it for ADD_ONION #22727

issue vasild openend this issue on August 17, 2021
  1. vasild commented at 1:33 pm on August 17, 2021: contributor

    As long as the ordinary bind succeeds, a failure to bind on the onion listen address (127.0.0.1:8334 by default) is ignored and later the supposed listening address is used in ADD_ONION to tell the Tor control daemon where to send incoming connections. This could result in a messy situation where two Bitcoin Cores run on the same machine and one of them is receiving the incoming Tor connections destined for the other, in addition to own ones. For example:

    0# Will bind also to 127.0.0.1:8334.
    1bitcoind -datadir=datadir1 -rpcbind=127.0.0.1:2001 -rpcallowip=127.0.0.1 -bind=1.2.3.4:2000
    2
    3# Will try to bind to 127.0.0.1:8334, will fail but nevertheless will instruct
    4# the Tor daemon to send incoming connections to 127.0.0.1:8334 where the other
    5# bitcoind is listening.
    6bitcoind -datadir=datadir2 -rpcbind=127.0.0.1:3001 -rpcallowip=127.0.0.1 -bind=[5:6::7]:3000
    

    Expected behavior

    Currently it is required that the ordinary bind succeeds (-bind=...) and that the RPC bind succeeds (-rpcbind=...). -bind=...=onion should be treated in the same way.

  2. vasild added the label Bug on Aug 17, 2021
  3. vasild referenced this in commit 05db6b4aac on Aug 17, 2021
  4. fanquake added the label P2P on Aug 18, 2021
  5. vasild referenced this in commit f89f80ef2d on Aug 18, 2021
  6. MDrollette commented at 6:18 pm on September 14, 2021: none
    Just to add an anecdote to make sure it’s covered in the fix - When running a node with port=8333 and rpcport=8334 I was surprised to find the rpc exposed via the onion address. So it’s not just the case of sending incoming onion traffic to a different node running on the same machine, but a very simple and seemingly correct config can result in exposing the rpc publicly.
  7. vasild commented at 8:50 am on September 15, 2021: contributor
    @MDrollette thanks for reporting, that is indeed unexpected and undesirable! This is supposed to be fixed in #22729. If you can test that PR and confirm it fixes the problem for you, maybe that would speed up its merge.
  8. vasild referenced this in commit 8e18fc433b on Sep 15, 2021
  9. MDrollette commented at 4:41 pm on September 15, 2021: none

    @vasild That PR does cause bitcoind to exit in my current configuration instead of continuing to run with the rpc port exposed over tor. So that’s an improvement.

    0./src/bitcoind -disablewallet -listenonion -onion=127.0.0.1:9050 -torcontrol=127.0.0.1:9051 -port=8333 -rpcport=8334
    

    But adding -listen=0 (the “suggestion” in the error message) causes it to start fully, and the rpc port is still exposed over the onion address - which still seems like an incorrect outcome.

  10. vasild referenced this in commit 980af167ec on Sep 16, 2021
  11. vasild referenced this in commit 57e0c5d32e on Sep 16, 2021
  12. vasild commented at 1:14 pm on September 16, 2021: contributor

    @MDrollette, how could you start bitcoind with -listen=0 -listenonion!? I think the following code should prevent that:

    https://github.com/bitcoin/bitcoin/blob/87780dfee8c82ea09b84e74d331db2ad6f9e75ab/src/init.cpp#L864-L867

  13. MDrollette commented at 4:48 pm on September 16, 2021: none
    For some reason I didn’t reach that error on the previous version of the PR. But at 57e0c5d32e7a1d86c9d8bccf088773b96029c338 I confirmed it does now exit correctly with either -listen=0 or -listen=1.
  14. vasild referenced this in commit 3d54f24f02 on Jun 13, 2022
  15. vasild referenced this in commit 7180859e7e on Oct 25, 2022
  16. vasild referenced this in commit 6a51eaf4a9 on Aug 16, 2023
  17. vasild referenced this in commit d0c8109dd0 on Jan 18, 2024
  18. vasild referenced this in commit 2e5e69a2d6 on Jun 1, 2024
  19. vasild referenced this in commit 8c3087150c on Jun 1, 2024
  20. vasild referenced this in commit ffe9b27498 on Jul 1, 2024
  21. vasild referenced this in commit bca346a970 on Jul 2, 2024
  22. achow101 closed this on Jul 16, 2024

  23. achow101 referenced this in commit 45750f61d6 on Jul 16, 2024

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: 2024-10-18 10:12 UTC

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