tor: allow authenticating to control port using hashedpassword auth

[3nprob]

• add configuration for tor_control_password for onion services
  • if this is configured, it will use HashedControlPassword instead of CookieAuthentication for the tor control port.

Resolves:

• feature request: add support for tor HashedControlPassword authentication #1401
• Q: Is being a maker connecting to a remote tor daemon supported? #1315

[roshii]

Suggesting a few changes before merging. New functionality should be covered by unit tests too.

[roshii]

Suggested change

	if self.tor_control_password is None:
	d = txtorcon.connect(reactor, control_endpoint)
	else:
	d = txtorcon.connect(reactor, control_endpoint, password_function=lambda : self.tor_control_password)
	password_function = lambda : self.tor_control_password if self.tor_control_password else None
	d = txtorcon.connect(reactor, control_endpoint, password_function=password_function)

[3nprob]

I find the former more readable. If maintainer prefer we can go with suggestion.

[roshii]

this looks overall ok, but needs to be covered by unit tests. we want to assess whether it behaves as intended: authenticating via password when provided in config, and via cookie when not.

[3nprob]

this looks overall ok, but needs to be covered by unit tests. we want to assess whether it behaves as intended: authenticating via password when provided in config, and via cookie when not.

That sounds more like an integration test than a unit test?

Currently AFAICT no part of the tor configuration (including host and port) are currently tested this way and while I agree testing for all of them would be good, it requires significant new testing setup that IMO should be done separately and hopefully not be a blocker for supporting remote tor node.

[roshii]

That sounds more like an integration test than a unit test?

should be unit tests.

1. making sure config reads and loads password when set (may already be covered somehow)
2. making sure service calls tor with intended config -- tor should be mocked here, we just it to assessed it'd called as expected