commit 76be3997e31d5a56e335a111eb1f3c823dbc9787 Author: Damian Johnson <atagar@torproject.org> Date: Fri Dec 9 09:06:26 2011 -0800 Adding an authentication exception hierarchy For arm a substantial torctl pain point has been poor exception raising from authentication functions. If everything raises a ValueError then the caller needs to parse message responses or re-implement the code to get finer error granularity. Providing a hierarchy of authentication exceptions that can both make life simple for callers, letting them catch the exceptions they want then fall back to a catch-all for the AuthenticationFailure base. One mildly icky point is that this relies on the error message provided from tor to differentiate between when an authentication value is rejected verses the type. That said... - this issue is noted in the pydocs - I have integ testing to check for error message changes - callers are encouraged to use a general authenticate method which doesn't have this flaw since it uses the PROTOCOLINFO response (barring edge cases like a buggy PROTOCOLINFO response or the auth type changing in the split second between PROTOCOLINFO/AUTHENTICATE) - even if the message changes direct callers will still be getting the right category of exception Pydocs reference a general authenticate method but it's not included yet (first draft is written but needs some work and testing). --- stem/connection.py | 232 +++++++++++++++++++++++++------ stem/socket.py | 3 - test/integ/connection/authentication.py | 180 ++++++++++++++---------- 3 files changed, 297 insertions(+), 118 deletions(-) diff --git a/stem/connection.py b/stem/connection.py index 9906b26..850a1f3 100644 --- a/stem/connection.py +++ b/stem/connection.py @@ -1,6 +1,30 @@ """ Functions for connecting and authenticating to the tor process. +AuthenticationFailure - Base exception raised for authentication failures. + |- UnrecognizedAuthMethods - Authentication methods are unsupported. + |- OpenAuthRejected - Tor rejected this method of authentication. + | + |- PasswordAuthFailed - Failure when authenticating by a password. + | |- PasswordAuthRejected - Tor rejected this method of authentication. + | |- IncorrectPassword - Password was rejected. + | +- MissingPassword - Socket supports password auth but wasn't attempted. + | + |- CookieAuthFailed - Failure when authenticating by a cookie. + | |- CookieAuthRejected - Tor rejected this method of authentication. + | |- IncorrectCookieValue - Authentication cookie was rejected. + | |- IncorrectCookieSize - Size of the cookie file is incorrect. + | +- UnreadableCookieFile - Unable to read the contents of the auth cookie. + | + +- MissingAuthInfo - Unexpected PROTOCOLINFO response, missing auth info. + |- NoAuthMethods - Missing any methods for authenticating. + +- NoAuthCookie - Supports cookie auth but doesn't have its path. + +authenticate - Main method for authenticating to a control socket. +authenticate_none - Authenticates to an open control socket. +authenticate_password - Authenticates to a socket supporting password auth. +authenticate_cookie - Authenticates to a socket supporting cookie auth. + get_protocolinfo_by_port - PROTOCOLINFO query via a control port. get_protocolinfo_by_socket - PROTOCOLINFO query via a control socket. ProtocolInfoResponse - Reply from a PROTOCOLINFO query. @@ -9,8 +33,7 @@ ProtocolInfoResponse - Reply from a PROTOCOLINFO query. | |- tor_version | |- auth_methods | |- unknown_auth_methods - | |- cookie_path - | +- socket + | +- cookie_path +- convert - parses a ControlMessage, turning it into a ProtocolInfoResponse """ @@ -40,48 +63,121 @@ LOGGER = logging.getLogger("stem") AuthMethod = stem.util.enum.Enum("NONE", "PASSWORD", "COOKIE", "UNKNOWN") -AUTH_COOKIE_MISSING = "Authentication failed: '%s' doesn't exist" -AUTH_COOKIE_WRONG_SIZE = "Authentication failed: authentication cookie '%s' is the wrong size (%i bytes instead of 32)" +class AuthenticationFailure(Exception): + """ + Base error for authentication failures. + + Attributes: + auth_response (stem.socket.ControlMessage) - AUTHENTICATE response from + the control socket, None if one wasn't received + """ + + def __init__(self, message, auth_response = None): + Exception.__init__(self, message) + self.auth_response = auth_response + +class UnrecognizedAuthMethods(AuthenticationFailure): + "All methods for authenticating aren't recognized." + +class OpenAuthRejected(AuthenticationFailure): + "Attempt to connect to an open control socket was rejected." + +class PasswordAuthFailed(AuthenticationFailure): + "Failure to authenticate with a password." + +class PasswordAuthRejected(PasswordAuthFailed): + "Socket does not support password authentication." + +class IncorrectPassword(PasswordAuthFailed): + "Authentication password incorrect." + +class MissingPassword(PasswordAuthFailed): + "Password authentication is supported but we weren't provided with one." + +class CookieAuthFailed(AuthenticationFailure): + "Failure to authenticate with an authentication cookie." + +class CookieAuthRejected(CookieAuthFailed): + "Socket does not support password authentication." -def authenticate_none(control_socket): +class IncorrectCookieValue(CookieAuthFailed): + "Authentication cookie value was rejected." + +class IncorrectCookieSize(CookieAuthFailed): + "Aborted because the cookie file is the wrong size." + +class UnreadableCookieFile(CookieAuthFailed): + "Error arose in reading the authentication cookie." + +class MissingAuthInfo(AuthenticationFailure): + """ + The PROTOCOLINFO response didn't have enough information to authenticate. + These are valid control responses but really shouldn't happen in practice. + """ + +class NoAuthMethods(MissingAuthInfo): + "PROTOCOLINFO response didn't have any methods for authenticating." + +class NoAuthCookie(MissingAuthInfo): + "PROTOCOLINFO response supports cookie auth but doesn't have its path." + +def authenticate_none(control_socket, suppress_ctl_errors = True): """ Authenticates to an open control socket. All control connections need to authenticate before they can be used, even if tor hasn't been configured to use any authentication. - If authentication fails then tor will close the control socket. + For general usage use the authenticate function instead. If authentication + fails then tor will close the control socket. Arguments: control_socket (stem.socket.ControlSocket) - socket to be authenticated + suppress_ctl_errors (bool) - reports raised stem.socket.ControllerError as + authentication rejection if True, otherwise they're re-raised Raises: - ValueError if the empty authentication credentials aren't accepted - stem.socket.ProtocolError the content from the socket is malformed - stem.socket.SocketError if problems arise in using the socket + stem.connection.OpenAuthRejected if the empty authentication credentials + aren't accepted """ - control_socket.send("AUTHENTICATE") - auth_response = control_socket.recv() - - # if we got anything but an OK response then error - if str(auth_response) != "OK": - raise ValueError(str(auth_response)) + try: + control_socket.send("AUTHENTICATE") + auth_response = control_socket.recv() + + # if we got anything but an OK response then error + if str(auth_response) != "OK": + control_socket.close() + raise OpenAuthRejected(str(auth_response), auth_response) + except stem.socket.ControllerError, exc: + control_socket.close() + + if not suppress_ctl_errors: raise exc + else: raise OpenAuthRejected("Socket failed (%s)" % exc) -def authenticate_password(control_socket, password): +def authenticate_password(control_socket, password, suppress_ctl_errors = True): """ Authenticates to a control socket that uses a password (via the HashedControlPassword torrc option). Quotes in the password are escaped. - If authentication fails then tor will close the control socket. + For general usage use the authenticate function instead. If authentication + fails then tor will close the control socket. + + note: If you use this function directly, rather than authenticate(), we may + mistakenly raise a PasswordAuthRejected rather than IncorrectPassword. This + is because we rely on tor's error messaging which is liable to change in + future versions. Arguments: control_socket (stem.socket.ControlSocket) - socket to be authenticated password (str) - passphrase to present to the socket + suppress_ctl_errors (bool) - reports raised stem.socket.ControllerError as + authentication rejection if True, otherwise they're re-raised Raises: - ValueError if the authentication credentials aren't accepted - stem.socket.ProtocolError the content from the socket is malformed - stem.socket.SocketError if problems arise in using the socket + stem.connection.PasswordAuthRejected if the socket doesn't accept password + authentication + stem.connection.IncorrectPassword if the authentication credentials aren't + accepted """ # Escapes quotes. Tor can include those in the password hash, in which case @@ -90,35 +186,64 @@ def authenticate_password(control_socket, password): password = password.replace('"', '\\"') - control_socket.send("AUTHENTICATE \"%s\"" % password) - auth_response = control_socket.recv() - - # if we got anything but an OK response then error - if str(auth_response) != "OK": - raise ValueError(str(auth_response)) + try: + control_socket.send("AUTHENTICATE \"%s\"" % password) + auth_response = control_socket.recv() + + # if we got anything but an OK response then error + if str(auth_response) != "OK": + control_socket.close() + + # all we have to go on is the error message from tor... + # Password did not match HashedControlPassword value value from configuration... + # Password did not match HashedControlPassword *or*... + + if "Password did not match HashedControlPassword" in str(auth_response): + raise IncorrectPassword(str(auth_response), auth_response) + else: + raise PasswordAuthRejected(str(auth_response), auth_response) + except stem.socket.ControllerError, exc: + control_socket.close() + + if not suppress_ctl_errors: raise exc + else: raise PasswordAuthRejected("Socket failed (%s)" % exc) -def authenticate_cookie(control_socket, cookie_path): +def authenticate_cookie(control_socket, cookie_path, suppress_ctl_errors = True): """ Authenticates to a control socket that uses the contents of an authentication cookie (generated via the CookieAuthentication torrc option). This does basic validation that this is a cookie before presenting the contents to the socket. - If authentication fails then tor will close the control socket. + The IncorrectCookieSize and UnreadableCookieFile exceptions take precidence + over the other types. + + For general usage use the authenticate function instead. If authentication + fails then tor will close the control socket. + + note: If you use this function directly, rather than authenticate(), we may + mistakenly raise a CookieAuthRejected rather than IncorrectCookieValue. This + is because we rely on tor's error messaging which is liable to change in + future versions. Arguments: control_socket (stem.socket.ControlSocket) - socket to be authenticated cookie_path (str) - path of the authentication cookie to send to tor + suppress_ctl_errors (bool) - reports raised stem.socket.ControllerError as + authentication rejection if True, otherwise they're re-raised Raises: - ValueError if the authentication credentials aren't accepted - IOError if the cookie file doesn't exist or we're unable to read it - stem.socket.ProtocolError the content from the socket is malformed - stem.socket.SocketError if problems arise in using the socket + stem.connection.IncorrectCookieSize if the cookie file's size is wrong + stem.connection.UnreadableCookieFile if the cookie file doesn't exist or + we're unable to read it + stem.connection.CookieAuthRejected if cookie authentication is attempted + but the socket doesn't accept it + stem.connection.IncorrectCookieValue if the cookie file's value is rejected """ if not os.path.exists(cookie_path): - raise IOError(AUTH_COOKIE_MISSING % cookie_path) + control_socket.close() + raise UnreadableCookieFile("Authentication failed: '%s' doesn't exist" % cookie_path) # Abort if the file isn't 32 bytes long. This is to avoid exposing arbitrary # file content to the port. @@ -132,18 +257,39 @@ def authenticate_cookie(control_socket, cookie_path): auth_cookie_size = os.path.getsize(cookie_path) if auth_cookie_size != 32: - raise ValueError(AUTH_COOKIE_WRONG_SIZE % (cookie_path, auth_cookie_size)) - - auth_cookie_file = open(cookie_path, "r") - auth_cookie_contents = auth_cookie_file.read() - auth_cookie_file.close() + control_socket.close() + exc_msg = "Authentication failed: authentication cookie '%s' is the wrong size (%i bytes instead of 32)" % (cookie_path, auth_cookie_size) + raise IncorrectCookieSize(exc_msg) - control_socket.send("AUTHENTICATE %s" % binascii.b2a_hex(auth_cookie_contents)) - auth_response = control_socket.recv() + try: + auth_cookie_file = open(cookie_path, "r") + auth_cookie_contents = auth_cookie_file.read() + auth_cookie_file.close() + except IOError, exc: + control_socket.close() + raise UnreadableCookieFile("Authentication failed: unable to read '%s' (%s)" % (cookie_path, exc)) - # if we got anything but an OK response then error - if str(auth_response) != "OK": - raise ValueError(str(auth_response)) + try: + control_socket.send("AUTHENTICATE %s" % binascii.b2a_hex(auth_cookie_contents)) + auth_response = control_socket.recv() + + # if we got anything but an OK response then error + if str(auth_response) != "OK": + control_socket.close() + + # all we have to go on is the error message from tor... + # ... Wrong length on authentication cookie. + # ... *or* authentication cookie. + + if "authentication cookie." in str(auth_response): + raise IncorrectCookieValue(str(auth_response), auth_response) + else: + raise CookieAuthRejected(str(auth_response), auth_response) + except stem.socket.ControllerError, exc: + control_socket.close() + + if not suppress_ctl_errors: raise exc + else: raise CookieAuthRejected("Socket failed (%s)" % exc) def get_protocolinfo_by_port(control_addr = "127.0.0.1", control_port = 9051, get_socket = False): """ diff --git a/stem/socket.py b/stem/socket.py index 0d2a84e..1739aaa 100644 --- a/stem/socket.py +++ b/stem/socket.py @@ -67,15 +67,12 @@ class ControllerError(Exception): class ProtocolError(ControllerError): "Malformed content from the control socket." - pass class SocketError(ControllerError): "Error arose while communicating with the control socket." - pass class SocketClosed(SocketError): "Control socket was closed before completing the message." - pass class ControlSocket: """ diff --git a/test/integ/connection/authentication.py b/test/integ/connection/authentication.py index 9df2db3..d7d0817 100644 --- a/test/integ/connection/authentication.py +++ b/test/integ/connection/authentication.py @@ -38,33 +38,55 @@ class TestAuthenticate(unittest.TestCase): Tests the authenticate_none function. """ - expect_success = self._is_authenticateable(stem.connection.AuthMethod.NONE) - self._check_auth(stem.connection.AuthMethod.NONE, None, expect_success) + auth_type = stem.connection.AuthMethod.NONE + if self._can_authenticate(auth_type): + self._check_auth(auth_type) + else: + self.assertRaises(stem.connection.OpenAuthRejected, self._check_auth, auth_type) + self._assert_auth_rejected_msg(auth_type) def test_authenticate_password(self): """ Tests the authenticate_password function. """ - expect_success = self._is_authenticateable(stem.connection.AuthMethod.PASSWORD) - self._check_auth(stem.connection.AuthMethod.PASSWORD, test.runner.CONTROL_PASSWORD, expect_success) + auth_type = stem.connection.AuthMethod.PASSWORD + auth_value = test.runner.CONTROL_PASSWORD + + if self._can_authenticate(auth_type): + self._check_auth(auth_type, auth_value) + else: + self.assertRaises(stem.connection.PasswordAuthRejected, self._check_auth, auth_type, auth_value) + self._assert_auth_rejected_msg(auth_type, auth_value) # Check with an empty, invalid, and quoted password. These should work if # we have no authentication, and fail otherwise. - expect_success = self._is_authenticateable(stem.connection.AuthMethod.NONE) - self._check_auth(stem.connection.AuthMethod.PASSWORD, "", expect_success) - self._check_auth(stem.connection.AuthMethod.PASSWORD, "blarg", expect_success) - self._check_auth(stem.connection.AuthMethod.PASSWORD, "this has a \" in it", expect_success) + for auth_value in ("", "blarg", "this has a \" in it"): + if self._can_authenticate(stem.connection.AuthMethod.NONE): + self._check_auth(auth_type, auth_value) + else: + if self._can_authenticate(stem.connection.AuthMethod.PASSWORD): + exc_type = stem.connection.IncorrectPassword + else: + exc_type = stem.connection.PasswordAuthRejected + + self.assertRaises(exc_type, self._check_auth, auth_type, auth_value) + self._assert_auth_rejected_msg(auth_type, auth_value) def test_authenticate_cookie(self): """ Tests the authenticate_cookie function. """ - test_path = test.runner.get_runner().get_auth_cookie_path() - expect_success = self._is_authenticateable(stem.connection.AuthMethod.COOKIE) - self._check_auth(stem.connection.AuthMethod.COOKIE, test_path, expect_success) + auth_type = stem.connection.AuthMethod.COOKIE + auth_value = test.runner.get_runner().get_auth_cookie_path() + + if self._can_authenticate(auth_type): + self._check_auth(auth_type, auth_value) + else: + self.assertRaises(stem.connection.CookieAuthRejected, self._check_auth, auth_type, auth_value) + self._assert_auth_rejected_msg(auth_type, auth_value) def test_authenticate_cookie_missing(self): """ @@ -72,9 +94,9 @@ class TestAuthenticate(unittest.TestCase): shouldn't exist. """ - test_path = "/if/this/exists/then/they're/asking/for/a/failure" - expected_exc = IOError(stem.connection.AUTH_COOKIE_MISSING % test_path) - self._check_auth(stem.connection.AuthMethod.COOKIE, test_path, False, expected_exc) + auth_type = stem.connection.AuthMethod.COOKIE + auth_value = "/if/this/exists/then/they're/asking/for/a/failure" + self.assertRaises(stem.connection.UnreadableCookieFile, self._check_auth, auth_type, auth_value) def test_authenticate_cookie_wrong_size(self): """ @@ -83,15 +105,14 @@ class TestAuthenticate(unittest.TestCase): socket. """ - test_path = test.runner.get_runner().get_torrc_path() - auth_cookie_size = os.path.getsize(test_path) + auth_type = stem.connection.AuthMethod.COOKIE + auth_value = test.runner.get_runner().get_torrc_path() - if auth_cookie_size == 32: + if os.path.getsize(auth_value) == 32: # Weird coincidence? Fail so we can pick another file to check against. self.fail("Our torrc is 32 bytes, preventing the test_authenticate_cookie_wrong_size test from running.") else: - expected_exc = ValueError(stem.connection.AUTH_COOKIE_WRONG_SIZE % (test_path, auth_cookie_size)) - self._check_auth(stem.connection.AuthMethod.COOKIE, test_path, False, expected_exc) + self.assertRaises(stem.connection.IncorrectCookieSize, self._check_auth, auth_type, auth_value) def _get_socket_auth(self): """ @@ -108,7 +129,7 @@ class TestAuthenticate(unittest.TestCase): return password_auth, cookie_auth - def _is_authenticateable(self, auth_type): + def _can_authenticate(self, auth_type): """ Checks if the given authentication type should be able to authenticate to our current socket. @@ -131,28 +152,19 @@ class TestAuthenticate(unittest.TestCase): elif auth_type == stem.connection.AuthMethod.COOKIE: return cookie_auth else: return False - def _check_auth(self, auth_type, auth_value, expect_success, failure_exc = None): + def _get_auth_function(self, control_socket, auth_type, *auth_args): """ - Attempts to use the given authentication function against our connection. - If this works then checks that we can use the connection. If not then we - check that the error message is what we'd expect. + Constructs a functor that performs the given authentication without + additional arguments. Arguments: + control_socket (stem.socket.ControlSocket) - socket for the function to + authenticate to auth_type (stem.connection.AuthMethod) - method by which we should authentiate to the control socket - auth_value (str) - value to be provided to the authentication function - expect_success (bool) - true if the authentication should succeed, false - otherwise - failure_exc (Exception) - exception that we want to assert is raised, if - None then we'll check for an auth mismatch error + auth_args (str) - arguments to be passed to the authentication function """ - runner = test.runner.get_runner() - control_socket = runner.get_tor_socket(False) - password_auth, cookie_auth = self._get_socket_auth() - - # construct the function call - if auth_type == stem.connection.AuthMethod.NONE: auth_function = stem.connection.authenticate_none elif auth_type == stem.connection.AuthMethod.PASSWORD: @@ -162,45 +174,69 @@ class TestAuthenticate(unittest.TestCase): else: raise ValueError("unexpected auth type: %s" % auth_type) - if auth_value != None: - auth_function = functools.partial(auth_function, control_socket, auth_value) + if auth_args: + return functools.partial(auth_function, control_socket, *auth_args) + else: + return functools.partial(auth_function, control_socket) + + def _assert_auth_rejected_msg(self, auth_type, *auth_args): + """ + This asserts that authentication will fail with the rejection message given + by tor. Note that this test will need to be updated if tor changes its + rejection reponse. + + Arguments: + auth_type (stem.connection.AuthMethod) - method by which we should + authentiate to the control socket + auth_args (str) - arguments to be passed to the authentication function + """ + + control_socket = test.runner.get_runner().get_tor_socket(False) + auth_function = self._get_auth_function(control_socket, auth_type, *auth_args) + password_auth, cookie_auth = self._get_socket_auth() + + if cookie_auth and password_auth: + failure_msg = MULTIPLE_AUTH_FAIL + elif cookie_auth: + failure_msg = COOKIE_AUTH_FAIL + elif auth_type == stem.connection.AuthMethod.PASSWORD: + failure_msg = INCORRECT_PASSWORD_FAIL else: - auth_function = functools.partial(auth_function, control_socket) + failure_msg = PASSWORD_AUTH_FAIL - if expect_success: + try: auth_function() - - # issues a 'GETINFO config-file' query to confirm that we can use the socket - - control_socket.send("GETINFO config-file") - config_file_response = control_socket.recv() - self.assertEquals("config-file=%s\nOK" % runner.get_torrc_path(), str(config_file_response)) control_socket.close() - else: - # if unset then determine what the general authentication error should - # look like - - if not failure_exc: - if cookie_auth and password_auth: - failure_exc = ValueError(MULTIPLE_AUTH_FAIL) - elif cookie_auth: - failure_exc = ValueError(COOKIE_AUTH_FAIL) - else: - # if we're attempting to authenticate with a password then it's a - # truncated message - - if auth_type == stem.connection.AuthMethod.PASSWORD: - failure_exc = ValueError(INCORRECT_PASSWORD_FAIL) - else: - failure_exc = ValueError(PASSWORD_AUTH_FAIL) - - try: - auth_function() - self.fail() - except Exception, exc: - # we can't check exception equality directly because it contains other - # attributes which will fail - - self.assertEqual(type(failure_exc), type(exc)) - self.assertEqual(str(failure_exc), str(exc)) + self.fail() + except stem.connection.AuthenticationFailure, exc: + self.assertFalse(control_socket.is_alive()) + self.assertEqual(failure_msg, str(exc)) + + def _check_auth(self, auth_type, *auth_args): + """ + Attempts to use the given authentication function against our connection. + If this works then checks that we can use the connection. If not then this + raises the exception. + + Arguments: + auth_type (stem.connection.AuthMethod) - method by which we should + authentiate to the control socket + auth_args (str) - arguments to be passed to the authentication function + + Raises: + stem.connection.AuthenticationFailure if the authentication fails + """ + + runner = test.runner.get_runner() + control_socket = runner.get_tor_socket(False) + auth_function = self._get_auth_function(control_socket, auth_type, *auth_args) + + # run the authentication, letting this raise if there's a problem + auth_function() + + # issues a 'GETINFO config-file' query to confirm that we can use the socket + control_socket.send("GETINFO config-file") + config_file_response = control_socket.recv() + self.assertEquals("config-file=%s\nOK" % runner.get_torrc_path(), str(config_file_response)) + control_socket.close()