pytorctl should learn to try both CookieAuth and PasswordAuth

pytorctl apparently suffers from the same problem Vidalia did in #3898 (closed), except backwards -- where Vidalia would try cookie if offered and give up if it fails, apparently pytorctl tries password if offered and gives up if it fails.

https://gitweb.torproject.org/pytorctl.git/blob/HEAD:/TorCtl.py#l532

added component::torctl owner::mikeperry parent::3476 priority::medium resolution::wontfix status::closed type::defect labels

Not quite. TorCtl tries the first authentication type present in the 'METHODS=' entry. Iirc this is COOKIE if both are offered so it'll have the same issue that's discussed in 3898.

When both are offered we should:

1. try the password if we already have it at that point in execution (ie, it's an arg to connect)
2. attempt to read the cookie file and use that
3. prompt the user for a password

This attempts in order of relative attempt speed, prompting the user as a last resort.

Trac:
Owner: N/A to atagar
Status: new to assigned

Replying to atagar:

Not quite. TorCtl tries the first authentication type present in the 'METHODS=' entry. Iirc this is COOKIE if both are offered so it'll have the same issue that's discussed in 3898.

When both are offered we should:

1. try the password if we already have it at that point in execution (ie, it's an arg to connect)
2. attempt to read the cookie file and use that
3. prompt the user for a password

This attempts in order of relative attempt speed, prompting the user as a last resort.

How can we ensure that bandwidth authorities, exit scanners, and other unattended processes that use PyTorCtl never try to 'prompt the user for a password'? (That would cause Nagios scripts to think that the process is running, even if it's doing nothing.)

TorCtl has three methods for connecting...

• connect() which may be interative
• preauth_connect() which just returns the auth type and unauthenticated connection, letting the caller handle that
• the connection can be made manually and authenticated, which is how it was done before the above existed

If starting non-interactively is important then they shouldn't be using the connect() method.

This change will need to break backward compatibility for preauth_connect callers since the return value needs to be an auth type tuple, but arm is probably the only caller so I doubt Mike will mind much.

Trac:
Parent: N/A to #3476 (closed)

Fixed in my bug3958 branch, for details and testing done see the commit description: https://gitweb.torproject.org/atagar/pytorctl.git/commitdiff/3608416cbf00c18aa7c2d1584861246740c549bd

I'll make a similar change in arm so this'll be fixed for its next release without waiting on a merge. Sending this over to Mike for review.

Trac:
Owner: atagar to mikeperry

second trac update to put in into 'needs review' cuz trac isn't smart enough to do it with a reassign in one go

Trac:
Status: assigned to needs_review

Workaround added to arm: https://gitweb.torproject.org/arm.git/commitdiff/8070fd004acde588f4d56d75bf6aa2c3f4d61b77

(... ick)

FYI I can't merge this branch because it contains the diffs for #3679 (moved), which we still are not sure about due to #4015 (moved).

FYI I can't merge this branch

This is what 'git cherry-pick' is for. The changes are completely separate and should merge cleanly.

Tor closes its socket after a failed authentication attempt. This patch seems to work in practice because cookie auth failures are almost always with reading the cookie so we fail before issuing an AUTHENTICATE call.

However, if the auth cookie somehow contains the wrong auth value then...

• torctl will send the cookie value to tor which is rejected, closing the socket
• torctl will then fall back to sending the user's passphrase, which regardless of if it's correct or not will also fail because the socket is closed

From what I recall of vidalia's handling I suspect it has a similar bug. I'm attempting to address this with stem by having socket objects that can reconnect after a failed connection attempt - see the ControlSocket and sublasses of... https://gitweb.torproject.org/stem.git/blob/HEAD:/stem/socket.py

All that said, this patch still gets TorCtl to a better place than it once was.

Cheers! -Damian

Oh, a bug with this patch: if the PROTOCOLINFO response has multiple auth types and the authenticate() caller doesn't provide a passphrase then we abort without trying cookie auth (see lines 1014-1016). This can be trivially fixed by changing...

if AUTH_TYPE.PASSWORD in self._authTypes and secret == "":

to

if secret == "" and self._authTypes == (AUTH_TYPE.PASSWORD,):

This probably also means an issue with the edge case where the user generated an empty password... tor --hash-password ""

so I'd suggest changing the secret default value to None.

Not vital for TorFlow. Resolving.

https://blog.torproject.org/blog/torctl-deprecation-and-stem-plans

Trac:
Status: needs_review to closed
Resolution: N/A to wontfix

closed

mentioned in issue #4003 (moved)

mentioned in issue tpo/network-health/torflow#4003 (closed)