#36374 torcontrol: escape backslashes in -torpassword
| Reviewability | Reviews | Agreement | Size | ||||
|---|---|---|---|---|---|---|---|
| #36374 torcontrol: escape backslashes in -torpassword | Ready | 2 | Strong | 2 + 29 tests | |||
|
|
|
|
| |||
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
Escapes backslashes in the Tor control port `AUTHENTICATE` command when using `HASHEDPASSWORD` authentication. Also adds functional test coverage with a mock Tor control server verifying backslash and quote escaping.
Problem: Bitcoin Core sends `AUTHENTICATE "<password>"` under `HASHEDPASSWORD` authentication but only escapes double quotes. Tor treats backslashes in quoted strings as escape characters according to the control specification, causing passwords containing backslashes to be corrupted or malform the command syntax, which fails authentication and prevents onion service creation.
Evidence of importance:
- PR description: 'Tested against Tor 0.4.9.12 with HashedControlPassword set for pa\ss"word\: on master bitcoind logs "tor: Authentication failed", with this change authentication succeeds and ADD_ONION creates the onion service.'
- PR description notes that with '-torpassword=pa\ss"word\', master sends 'AUTHENTICATE "pa\ss\"word\"' whereas the fix sends 'AUTHENTICATE "pa\\ss\"word\\"'.
Risk of a mistake: low (other, tests). A mistake in escaping could cause Tor control port authentication to fail, preventing automatic Tor onion service setup.
Changed since the previous assessment: No substantive changes to the code or test logic compared to the previous version (likely a rebase or commit message edit).
Reviewability
Ready
Nothing known that would invalidate a review now.
Waiting on (from the thread read):
- nothing: no pending items
Agreement
Strong
substantive support, no open objection (winterrdog, winterrdog)
Thread notes: Added jpk68's ACK of 9c9bd8ca.
Objections, suggestions, and questions:
| Reviewer | Type | Harm, request, or question | Status | Must be | Author replied | Quote |
|---|---|---|---|---|---|---|
| fjahr (member) | suggestion, approach | Coordinate with the author of #35292 on existing test coverage | answered · author replied they would coordinate; winterrdog confirmed consensus that this PR should go first | answered | yes | 2026-09-28: 'There is already test coverage suggested for this in #35292, please coordinate with the author there how to proceed.' Status from: 2026-09-28: 0xShadowX: 'I'll ask in #35292 how to combine the test coverage.' |
| fjahr (member) | suggestion, approach | Install Tor and test against it | answered · author installed Tor 0.4.9.12 and tested against it | answered | yes | 2026-09-28: 'You should install it and then test it.' Status from: 2026-09-28: 0xShadowX: 'Installed Tor 0.4.9.12 and tested against it with `HashedControlPassword`' |
| winterrdog (contributor) | suggestion, maintenance | Drop -debug=tor from the test case since it only asserts commands received by the mock | fixed · fix pushed · author dropped -debug=tor in 9c9bd8ca | answered | yes | 2026-10-06: '-debug=tor only enables debug-level logs (_not needed in this case_) and can be dropped here' Status from: 2026-10-06: 0xShadowX: 'Dropped it in 9c9bd8cab0f8f5dfebf8d709e0588b42cbdf57fe, thanks.' |
Support:
- winterrdog (contributor) (Approach ACK, 2026-10-06) [both]: Verified that backslashes are not escaped on master causing the test to fail, verified requirement in TorControl spec, and agreed this PR should land before #35292
- winterrdog (contributor) (Tested ACK, 2026-10-07) [tested]: successfully built and tested with clang++-19 on FreeBSD 15.0/x86_64 and Linux/x86_64 with clang++-18, reproduced test failure on master, and verified successful authentication with local Tor daemon
- jpk68 (contributor) (ACK, 2026-10-21): (no reason given) [not substantive]
Participants:
- fjahr (member, neutral, 1 statements 2026-09-28 to 2026-09-28): suggested coordinating test coverage with #35292 and testing with Tor installed
- winterrdog (contributor, support, 3 statements 2026-10-06 to 2026-10-07): Approach ACK and Tested ACK; verified against TorControl spec, reproduced failure on master, and verified with live Tor daemon on FreeBSD and Linux
- jpk68 (contributor, support, 1 statements 2026-10-21 to 2026-10-21): ACKed commit 9c9bd8ca
Files
29 lines under test/bench/ci.
- test/functional/feature_torcontrol.py +29/-0
- src/torcontrol.cpp +2/-0
Card
The short text the ranking pass compares across PRs.
Escapes backslashes before quotes when transmitting -torpassword in the Tor control AUTHENTICATE command under HASHEDPASSWORD authentication. Currently, Bitcoin Core only escapes quotation marks, which causes Tor's control-spec QuotedString parser to treat backslashes as escape characters; this corrupts the password or breaks command syntax and prevents onion service creation. The author verified against Tor 0.4.9.12 that a password with backslashes fails authentication on master and succeeds with this patch.
Data
thread read through 2026-10-21 (7 statements), head 9c9bd8cab0
labels: none
Categories
P2P
P3 · bug fix
Fixes Tor control authentication failure when passwords contain backslashes. Without escaping, 'Tor treats backslashes in quoted strings as escape characters', causing authentication to fail and preventing onion service creation via ADD_ONION.
Membership: Modifies src/torcontrol.cpp, which implements Tor control port integration covered by P2P.
Factors: security/stability 1, bug 1, performance 0, user value 1, leverage 0