#36170 net: require a dedicated bind for automatic Tor

ReviewabilityReviewsAgreementSize
#36170 l0rinc net: require a dedicated bind for automatic TorReady3Strong22 + 24 tests
  • Require a dedicated bind for automatic Tor listening when explicit bind arguments are used
  • Prevents inbound Tor peers from being misclassified by IP and inheriting local whitelist permissions
  • Ready for review, with no blockers or unaddressed concerns
  • Strong support with no remaining open objections (vasild, winterrdog)
  • Tested on FreeBSD 15 targeting x86_64 (winterrdog)
  • Thinks adding dedicated container config options is overkill since Docker can set specific IPs (vasild)
  • Answered concern that container environments with dynamic IPs rely on wildcard binds (schildbach)
  • 22 lines added or modified outside tests
  • 24 lines added or modified in tests
  • 11 lines removed in total
  • 7 files, 5 commits
P2P#19 of 71
P2 · bug fix
  • P2 because it prevents inbound Tor connections from inheriting local IP whitelist permissions
  • Stops peer misclassification when automatic onion routing forwards through shared listeners
Ranking notes
Reviewability: ReadyStalePaused
Agreement: StrongPositivePositive w/ caveatsNeutralMildDisputedBlockedCrickets
Reviews: current code-review ACKs, then (+stale ACKs) and -NACKs; greener = more ACKs.
Size: lines added or modified outside tests, then in tests; greener = smaller.

Goal

Require a dedicated, non-wildcard onion bind when automatic Tor onion service listening is enabled alongside explicit `-bind` arguments. Reject startup during initialization if explicit `-bind` parameters lack a specific `-bind=<addr>=onion` listener or specify a wildcard onion bind.

Problem: When automatic onion service creation (`-listenonion`) runs without a dedicated onion bind, Tor forwards inbound traffic to the first normal P2P bind. Bitcoind cannot distinguish these from direct connections, misclassifying Tor peers by IP address and potentially granting them permissions assigned to the Tor daemon's local IP.

Evidence of importance:

  • PR description: 'Tor peers arriving through shared or wildcard binds may be classified by IP and inherit address-based whitelist permissions.'
  • PR description cites consensus from #34892 recommending startup rejection rather than silent fallback.

Scope: This is a breaking startup configuration change: because `-listenonion` defaults to enabled whenever listening, existing node configs specifying `-bind` without Tor options will now fail startup until they add `-bind=<addr>=onion` or `-listenonion=0`.

Risk of a mistake: medium (p2p). Nodes with existing configurations using explicit `-bind` directives will fail to start upon upgrading until their config files or startup arguments are modified.

Changed since the previous assessment: Added release notes, tor documentation updates, and co-authors to commit messages.

Reviewability

Ready

Nothing known that would invalidate a review now.

Waiting on (from the thread read):

  • nothing: no pending reviewer requests or actions awaited

Agreement

Strong

substantive support, no open objection (vasild, winterrdog)

Thread notes: Added ACK from jpk68 and Tested ACK from winterrdog. winterrdog's remark regarding adding 'fixes #33458' to the description is a process suggestion and not recorded as a claim.

Objections, suggestions, and questions:

ReviewerTypeHarm, request, or questionStatusMust beAuthor repliedQuote
jeanpablojp (contributor)objection, correctnessWildcard onion binds pass the new check but fail to tag incoming peers as onion, causing them to retain IP-network permissions such as nobanfixed · fix pushed · author implemented a wildcard-bind check and testchangedyes2026-09-06: '-bind=0.0.0.0:8334=onion passes the new check while tagging nothing'
Status from: 2026-09-24: l0rinc: 'added a wildcard-bind check and test'
jeanpablojp (contributor)suggestion, maintenanceExplain in the -bind help and doc/tor.md when an explicit dedicated onion bind is requiredfixed · fix pushed · author added explanation to -bind help and doc/tor.mdansweredyes2026-09-06: 'Would a line in one of the two be worth it?'
Status from: 2026-09-24: l0rinc: 'added this to both the `-bind` help and section 2 of `doc/tor.md`'
jeanpablojp (contributor)suggestion, styleUse assert_start_raises_init_error helper in the test to drop unused importsfixed · fix pushed · author switched to the test helperansweredyes2026-09-06: 'this could use the helper the file already uses further down'
Status from: 2026-09-24: l0rinc: 'Switched to `assert_start_raises_init_error`, thanks.'
vasild (member)suggestion, interfaceClarify the error message so it does not imply the requirement only applies when -bind is setfixed · fix pushed · author shortened the error messageansweredyes2026-10-01: 'With this PR the automatic Tor service requires a dedicated bind, always, unconditionally. Not only "when -bind is set".'
Status from: 2026-10-07: l0rinc: 'Used your shorter version, thanks.'
vasild (member)suggestion, maintenanceRemove the duplicate check in AppInitParameterInteraction and keep only the check using connOptions.onion_bindsfixed · fix pushed · author removed the duplicate checkansweredyes2026-10-01: 'Isn't this check redundant? Seems to be the same as the added check in `AppInitParameterInteraction()`. Better have just one.'
Status from: 2026-10-07: l0rinc: 'I agree that duplicating the argument parsing just for this case isn’t worth it - removed, thanks'
vasild (member)suggestion, correctnessCorrect the error message for wildcard onion binds because wildcard addresses still identify incoming Tor connections but Tor cannot forward to themaccepted · vasild accepted that wildcard onion binds fail to match and misclassify connectionsansweredyes2026-10-01: 'This message is technically not correct. If wildcard address is used, incoming Tor connections will still be identified just fine.'
Status from: 2026-10-19: vasild: 'I got this wrong initially, thanks for correcting me.'
vasild (member)suggestion, maintenanceClarify in the release notes that any specific address can be used, not only 127.0.0.1fixed · fix pushed · author clarified the release notesansweredyes2026-10-01: 'The impression is that `127.0.0.1` must be used.'
Status from: 2026-10-07: l0rinc: 'clarified the release notes'
schildbach (contributor)objection, approachRequiring a specific IP rather than wildcard binds conflicts with container environments where dynamic IPs are usedanswered · author prefers to keep rejecting wildcard configurations in this PR and handle container support separatelyansweredyes2026-10-06: 'when running in a container, it is considered best practise to bind to `0.0.0.0` / `[::]` rather than a specific IP.'
Status from: 2026-10-07: l0rinc: 'I'd keep rejecting these configurations in this PR and handle that support separately.'

Support:

  • jeanpablojp (contributor) (Concept ACK, 2026-09-06): (no reason given) [not substantive]
  • winterrdog (contributor) (Approach ACK, 2026-09-08): (no reason given) [not substantive]
  • vasild (member) (Approach ACK, 2026-10-01): (no reason given) [not substantive]
  • vasild (member) (ACK, 2026-10-19) [read]: adding another configuration option for containers would be overkill; docker can assign a specific IP
  • jpk68 (contributor) (ACK, 2026-10-21): (no reason given) [not substantive]
  • winterrdog (contributor) (Tested ACK, 2026-10-21) [tested]: successfully built & tested with `clang++-19` on FreeBSD `15.0`, targeting `x86_64`

Participants:

  • jeanpablojp (contributor, objection, 3 statements 2026-09-06 to 2026-09-06): noted wildcard onion binds bypass classification checks and suggested doc updates
  • winterrdog (contributor, support, 2 statements 2026-09-08 to 2026-10-21): Approach ACK and Tested ACK on FreeBSD
  • vasild (member, support, 12 statements 2026-10-01 to 2026-10-19): ACK; suggested eliminating a redundant check and accepted author's rationale on wildcard misclassification
  • schildbach (contributor, objection, 1 statements 2026-10-06 to 2026-10-06): noted container environments with dynamic IPs rely on wildcard binds and suggested solving via a second config option
  • jpk68 (contributor, support, 1 statements 2026-10-21 to 2026-10-21): ACK

Files

26 lines under test/bench/ci.

  • src/init.cpp +11/-9
  • test/functional/feature_bind_extra.py +16/-0
  • doc/release-notes-36170.md +8/-0
  • test/functional/feature_proxy.py +5/-2
  • doc/tor.md +3/-0
  • test/functional/feature_torcontrol.py +2/-0
  • test/functional/p2p_private_broadcast.py +1/-0

Card

The short text the ranking pass compares across PRs.

PR 36170 causes bitcoind to refuse startup if automatic Tor listening is enabled alongside explicit -bind options without a dedicated, non-wildcard onion bind (-bind=<addr>=onion). Without a dedicated bind, incoming Tor connections land on standard listening sockets where they are misclassified by IP, potentially inheriting whitelist permissions assigned to the Tor daemon. Because -listenonion is enabled by default when listening, existing node setups using -bind will fail to start until they explicitly configure an onion bind or pass -listenonion=0.

Data

dossier JSON · extract JSON · model ledger, generated 2026-10-22T01:15, confidence high, input hash dc8582b757c4ac50
thread read through 2026-10-21 (27 statements), head 5527295cb4
labels: P2P

Categories

P2P

P2 · bug fix

Prevents Tor peers from being misclassified and inheriting local IP whitelist permissions when automatic onion listening routes inbound traffic through shared binds. The PR description notes that without a dedicated bind, Tor peers 'may be classified by IP and inherit address-based whitelist permissions.'

Membership: Carries the P2P label and modifies connection handling and Tor listener binding configurations.

Factors: security/stability 2, bug 1, performance 0, user value 1, leverage 0