test: Add coverage for Tor control `HASHEDPASSWORD` authentication #35292

winterrdog commented at 6:28 PM on May 14, 2026: contributor

this is a tests-only PR aimed at adding functional test coverage for Tor control HASHEDPASSWORD authentication.

currently, the functional test suite does not explicitly explore this authentication path, which means regressions in tor authentication handling could go unnoticed. for instance incorrectly formatted AUTHENTICATE commands, broken fallback behavior, or authentication attempts being made when no password is configured.

The 4 tests herein extend the existing mock tor control server to simulate METHODS=HASHEDPASSWORD responses and cover successful authentication with the correct password, failing with an incorrect password, behavior when -torpassword is not set, and cases where the server does not choose to advertise HASHEDPASSWORD as a way of authenticating.

tested with the test harness:

./build/test/functional/test_runner.py \
  ./build/test/functional/feature_torcontrol.py \
  --loglevel=debug --failfast

DrahtBot added the label Tests on May 14, 2026

DrahtBot commented at 6:28 PM on May 14, 2026: contributor

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

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/35292.

Reviews

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

#34486 (net: Reduce local network activity when networkactive=0 by willcl-ark)

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.

sedited commented at 6:42 PM on May 14, 2026: contributor

@winterrdog can you re-write the description in your own words without following this LLM-formulaic format? This would give reviewers more confidence that you understand the change.

winterrdog commented at 6:59 PM on May 14, 2026: contributor

re-write the description in your own words without following this LLM-formulaic format?

thanks! done edition (seems like i followed the wrong examples online).

DrahtBot added the label CI failed on May 14, 2026

DrahtBot commented at 7:44 PM on May 14, 2026: contributor

🚧 At least one of the CI tasks failed. <sub>Task lint: https://github.com/bitcoin/bitcoin/actions/runs/25877900203/job/76052468485</sub> <sub>LLM reason (✨ experimental): CI failed due to Python lint errors from ruff (F541: extraneous f-string prefixes) in test/functional/feature_torcontrol.py.</sub>

<details><summary>Hints</summary>

Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:

Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.
A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.
An intermittent issue.

Leave a comment here, if you need help tracking down a confusing failure.

</details>

winterrdog force-pushed on May 14, 2026

DrahtBot removed the label CI failed on May 14, 2026

in test/functional/feature_torcontrol.py:303 in f886aa2434

 298 | +        self.wait_until(lambda: len(mock_tor.received_commands) >= 2, timeout=10)
 299 | +        assert_equal(mock_tor.received_commands[0], "PROTOCOLINFO 1")
 300 | +        assert_equal(mock_tor.received_commands[1], 'AUTHENTICATE "wrong_password"')
 301 | +        # After auth failure, no further commands should be sent and authentication failure should be logged
 302 | +        ensure_for(duration=2, f=lambda: len(mock_tor.received_commands) == 2)
 303 | +        self.wait_until(lambda: any("Authentication failed" in line for line in self.nodes[0].debug_log_path.read_text().splitlines()))

davidgumberg commented at 11:23 PM on May 14, 2026:

What is this testing separately from the correct password test?

winterrdog commented at 8:28 PM on May 15, 2026:

this log check was an extra step i added to fully confirm that the error was actually noticed and reported instead of being silently swallowed. it was not needed in the correct password test since there we already verify that Bitcoin Core continues past authentication by checking that more than 2 commands (AUTHENTICATE and PROTOCOLINFO - the initial commands) are sent.

here comes the long story. let's assume the correct password is the happy path and the wrong password is not.

so in the happy path (correct password), this is what happens in Bitcoin Core:

correct password sent
    -> tor daemon returns `250 OK`
        -> `TorController::auth_cb` hits the `if` branch & continues on to `GETINFO` and `ADD_ONION`
            -> connection stays active

now for the sad path (wrong password):

incorrect password sent
    -> tor daemon returns `515 Bad authentication`
        -> `TorController::auth_cb` hits the `else` branch
            -> `"tor: Authentication failed"` is logged
                -> no further commands are sent
                    -> connection effectively stops

the key difference here is that the wrong password test verifies Bitcoin Core logs and stops trying to do anything else when authentication fails, which is not the case with a correct password as you saw above.

did you have something different in mind ?

winterrdog requested review from davidgumberg on May 19, 2026

winterrdog force-pushed on Jun 4, 2026

sedited requested review from vasild on Jun 8, 2026

winterrdog force-pushed on Jun 9, 2026

winterrdog force-pushed on Aug 3, 2026

test: Add HASHEDPASSWORD authentication tests for Tor control

Add functional test coverage for the HASHEDPASSWORD authentication
method

The Tor control spec (section 3.5) defines HASHEDPASSWORD as one of 3
authentication methods. When a Tor controller advertises HASHEDPASSWORD
support via PROTOCOLINFO, clients must send their password as a quoted
string in the AUTHENTICATE command. These tests verify Bitcoin Core's
torcontrol module correctly implements this protocol flow.

Test scenarios:
- Correct password: Verify AUTHENTICATE sends password as quoted string
  per spec requirement ("AUTHENTICATE \"password\"" format)
- Wrong password: Verify authentication fails with spec-compliant 515
  error and connection stops after failure
- Missing password: Verify no AUTHENTICATE is sent when -torpassword is
  unset, preventing authentication attempts without credentials
- Unsupported method: Verify -torpassword is ignored when Tor does not
  advertise HASHEDPASSWORD support, avoiding unnecessary auth attempts

f24ef5b201

winterrdog force-pushed on Aug 4, 2026

DrahtBot added the label CI failed on Aug 4, 2026

DrahtBot removed the label CI failed on Aug 4, 2026