[tor-commits] [stem/master] Revisions to change for SAFECOOKIE support

atagar at torproject.org atagar at torproject.org
Sun Jun 10 01:27:16 UTC 2012


commit 560923cb7b572d02046c6ca2bd5eb4502fa591b3
Author: Damian Johnson <atagar at torproject.org>
Date:   Sat Jun 9 18:18:00 2012 -0700

    Revisions to change for SAFECOOKIE support
    
    The SAFECOOKIE change is pretty big so, unsurprisingly, it has needed quite a
    few changes. Many are stylistic nitpicks, though this also includes several
    more substantial changes including...
    
    * The SAFECOOKIE AuthMethod was added, but the ProtocolInfoResponse's
      _parse_message didn't recognize it causing it to still be listed as an
      unknown authentication method. Also, we called send/recv on the controller in
      authenticate_safecookie() at one point rather than using _msg, causing it to
      break when called with a Controller. I'm a tad puzzled by these... were the
      integ tests ever ran with the RUN_ALL target?
    
    * Slightly reducing the exhaustiveness of the authentication unit test in the
      name of runtime. The test exercises each combination of exceptions for each
      exception type which led to a combination explosion when we added SAFECOOKIE
      to the mix (raising the unit testing runtime from around two to twelve
      seconds). Made some minor tweaks to bring the runtime back down without
      really impacting what we test much.
    
    * Changing the CookieAuthFailed attribute that indicates if it arose from
      SAFECOOKIE or COOKIE auth to be a boolean. Providing the AuthMethod would
      only make sense if we added even more variants of cookie authentication
      (possible, but not anywhere on the radar).
    
    * Added AUTH_SAFECOOKIE to 'stem.version.Requirement'. This turned out to need
      some fairly big changes to how we check for version requirements (see prior
      commit).
    
    * Few issues with the integ tests, including calling 'remove' on a tuple and a
      bug where it would accidently remove all auth methods from the protocolinfo
      response before calling authenticate().
    
    * We only removed AuthMethod.COOKIE form a protocolinfo response's auth_methods
      when the cookie file was missing.
    
    * Few unused imports and lots 'o whitespace issues. I should probably improve
      the whitespace checker a bit.
    
    * Toned down the 'warning, we're under attack!' errors. The whole point of
      safecookie auth is simply to avoid providing the raw cookie contents to the
      thing we think is tor. Nothing about this handshake should be consitered to
      be an 'attack', nonce mismatches are more likely to just indicate some sort
      of bug.
    
    * No tor reply can be empty (if it was empty then we... well, wouldn't have
      read anything from the socket). Hence the ControlMessage can't be empty.
      It'll be a pita to check for this in every reply, so checking that as part of
      the ControlMessage constructor.
    
    * Added helper function to check if a value is hex digits. We do this a lot, so
      no need to repeat the regex everywhere.
    
    * Looks like we could misidentify IncorrectCookieValue verses
      CookieAuthRejected exceptions when we had both cookie and password auth.
    
    * I was a bit confused about what the AUTHCHALLENGE unit tests for an invalid
      response were attempting to exercise, so replaced with a shorter and simpler
      test for missing components.
---
 stem/connection.py                      |  293 ++++++++++++++++---------------
 stem/descriptor/extrainfo_descriptor.py |    2 +-
 stem/descriptor/server_descriptor.py    |    2 +-
 stem/response/__init__.py               |   10 +-
 stem/response/authchallenge.py          |   74 +++-----
 stem/response/protocolinfo.py           |    2 +
 stem/util/connection.py                 |   31 +---
 stem/util/tor_tools.py                  |   16 ++-
 test/integ/connection/authentication.py |  125 +++++++-------
 test/integ/response/protocolinfo.py     |    7 +-
 test/unit/connection/authentication.py  |   79 +++++----
 test/unit/response/authchallenge.py     |   47 +++---
 12 files changed, 345 insertions(+), 343 deletions(-)

diff --git a/stem/connection.py b/stem/connection.py
index 8234441..ddd4be3 100644
--- a/stem/connection.py
+++ b/stem/connection.py
@@ -52,6 +52,7 @@ the authentication process. For instance...
   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.
+  authenticate_safecookie - Authenticates to a socket supporting safecookie auth.
   
   get_protocolinfo - Issues a PROTOCOLINFO query.
   
@@ -72,8 +73,9 @@ the authentication process. For instance...
     |  |- IncorrectCookieValue - Authentication cookie was rejected.
     |  |- IncorrectCookieSize - Size of the cookie file is incorrect.
     |  |- UnreadableCookieFile - Unable to read the contents of the auth cookie.
-    |  +- AuthChallengeFailed - Failure completing the authchallenge request
-    |     |- AuthSecurityFailure - The computer/network may be compromised.
+    |  +- AuthChallengeFailed - Failure completing the authchallenge request.
+    |     |- AuthChallengeUnsupported - Tor doesn't recognize the AUTHCHALLENGE command.
+    |     |- AuthSecurityFailure - Server provided the wrong nonce credentials.
     |     |- InvalidClientNonce - The client nonce is invalid.
     |     +- UnrecognizedAuthChallengeMethod - AUTHCHALLENGE does not support the given methods.
     |
@@ -83,7 +85,6 @@ the authentication process. For instance...
 """
 
 import os
-import re
 import getpass
 import binascii
 
@@ -97,7 +98,10 @@ import stem.util.connection
 import stem.util.log as log
 from stem.response.protocolinfo import AuthMethod
 
-def connect_port(control_addr = "127.0.0.1", control_port = 9051, password = None, chroot_path = None, controller = None):
+CLIENT_HASH_CONSTANT = "Tor safe cookie authentication controller-to-server hash"
+SERVER_HASH_CONSTANT = "Tor safe cookie authentication server-to-controller hash"
+
+def connect_port(control_addr = "127.0.0.1", control_port = 9051, password = None, chroot_path = None, controller = stem.control.Controller):
   """
   Convenience function for quickly getting a control connection. This is very
   handy for debugging or CLI setup, handling setup and prompting for a password
@@ -230,28 +234,29 @@ def authenticate(controller, password = None, chroot_path = None, protocolinfo_r
       Tor allows for authentication by reading it a cookie file, but rejected
       the contents of that file.
     
+    * **\***:class:`stem.connection.AuthChallengeUnsupported`
+    
+      Tor doesn't recognize the AUTHCHALLENGE command. This is probably a Tor
+      version prior to SAFECOOKIE being implement, but this exception shouldn't
+      arise because we won't attempt SAFECOOKIE auth unless Tor claims to
+      support it.
+    
     * **\***:class:`stem.connection.UnrecognizedAuthChallengeMethod`
-
+    
       Tor couldn't recognize the AUTHCHALLENGE method Stem sent to it. This
       shouldn't happen at all.
-
+    
     * **\***:class:`stem.connection.InvalidClientNonce`
-
+    
       Tor says that the client nonce provided by Stem during the AUTHCHALLENGE
       process is invalid.
-
+    
     * **\***:class:`stem.connection.AuthSecurityFailure`
-
-      Raised when self is a possibility self security having been compromised.
     
-    * **\***:class:`stem.connection.AuthChallengeFailed`
-
-      The AUTHCHALLENGE command has failed (probably because Stem is connecting
-      to an old version of Tor which doesn't support Safe cookie authentication,
-      could also be because of other reasons).
+      Nonce value provided by the server was invalid.
     
     * **\***:class:`stem.connection.OpenAuthRejected`
-
+    
       Tor says that it allows for authentication without any credentials, but
       then rejected our authentication attempt.
     
@@ -308,19 +313,17 @@ def authenticate(controller, password = None, chroot_path = None, protocolinfo_r
   if protocolinfo_response.cookie_path is None:
     for cookie_auth_method in (AuthMethod.COOKIE, AuthMethod.SAFECOOKIE):
       if cookie_auth_method in auth_methods:
-        try:
-          auth_methods.remove(AuthMethod.COOKIE)
-        except ValueError:
-          pass
-        auth_exceptions.append(NoAuthCookie("our PROTOCOLINFO response did not have the location of our authentication cookie"))
+        auth_methods.remove(cookie_auth_method)
+        
+        exc_msg = "our PROTOCOLINFO response did not have the location of our authentication cookie"
+        auth_exceptions.append(NoAuthCookie(exc_msg, cookie_auth_method == AuthMethod.SAFECOOKIE))
   
   if AuthMethod.PASSWORD in auth_methods and password is None:
     auth_methods.remove(AuthMethod.PASSWORD)
     auth_exceptions.append(MissingPassword("no passphrase provided"))
   
   # iterating over AuthMethods so we can try them in this order
-  for auth_type in (AuthMethod.NONE, AuthMethod.PASSWORD, AuthMethod.SAFECOOKIE,
-      AuthMethod.COOKIE):
+  for auth_type in (AuthMethod.NONE, AuthMethod.PASSWORD, AuthMethod.SAFECOOKIE, AuthMethod.COOKIE):
     if not auth_type in auth_methods: continue
     
     try:
@@ -328,7 +331,7 @@ def authenticate(controller, password = None, chroot_path = None, protocolinfo_r
         authenticate_none(controller, False)
       elif auth_type == AuthMethod.PASSWORD:
         authenticate_password(controller, password, False)
-      elif auth_type == AuthMethod.COOKIE or auth_type == AuthMethod.SAFECOOKIE:
+      elif auth_type in (AuthMethod.COOKIE, AuthMethod.SAFECOOKIE):
         cookie_path = protocolinfo_response.cookie_path
         
         if chroot_path:
@@ -350,19 +353,17 @@ def authenticate(controller, password = None, chroot_path = None, protocolinfo_r
       log.debug("The authenticate_password method raised a PasswordAuthRejected when password auth should be available. Stem may need to be corrected to recognize this response: %s" % exc)
       auth_exceptions.append(IncorrectPassword(str(exc)))
     except AuthSecurityFailure, exc:
-      log.info("The authenticate_safecookie method raised an AuthSecurityFailure. Security might have been compromised - attack? (%s)" % exc)
+      log.info("Tor failed to provide the nonce expected for safecookie authentication. (%s)" % exc)
       auth_exceptions.append(exc)
-    except (InvalidClientNonce, UnrecognizedAuthChallengeMethod,
-        AuthChallengeFailed), exc:
+    except (InvalidClientNonce, UnrecognizedAuthChallengeMethod, AuthChallengeFailed), exc:
       auth_exceptions.append(exc)
     except (IncorrectCookieSize, UnreadableCookieFile, IncorrectCookieValue), exc:
       auth_exceptions.append(exc)
     except CookieAuthRejected, exc:
-      auth_func = "authenticate_cookie"
-      if exc.auth_type == AuthMethod.SAFECOOKIE:
-        auth_func = "authenticate_safecookie"
+      auth_func = "authenticate_safecookie" if exc.is_safecookie else "authenticate_cookie"
+      
       log.debug("The %s method raised a CookieAuthRejected when cookie auth should be available. Stem may need to be corrected to recognize this response: %s" % (auth_func, exc))
-      auth_exceptions.append(IncorrectCookieValue(str(exc), exc.cookie_path))
+      auth_exceptions.append(IncorrectCookieValue(str(exc), exc.cookie_path, exc.is_safecookie))
     except stem.socket.ControllerError, exc:
       auth_exceptions.append(AuthenticationFailure(str(exc)))
   
@@ -473,47 +474,6 @@ def authenticate_password(controller, password, suppress_ctl_errors = True):
     if not suppress_ctl_errors: raise exc
     else: raise PasswordAuthRejected("Socket failed (%s)" % exc)
 
-def _read_cookie(cookie_path, auth_type):
-  """
-  Provides the contents of a given cookie file. If unable to do so this raises
-  an exception of a given type.
-  
-  :param str cookie_path: absolute path of the cookie file
-  :param str auth_type: cookie authentication type (from the AuthMethod Enum)
-  
-  :raises:
-    * :class:`stem.connection.UnreadableCookieFile` if the cookie file is unreadable
-    * :class:`stem.connection.IncorrectCookieSize` if the cookie size is incorrect (not 32 bytes)
-  """
-
-  if not os.path.exists(cookie_path):
-    raise UnreadableCookieFile("Authentication failed: '%s' doesn't exist" % 
-        cookie_path, cookie_path, auth_type = auth_type)
-  
-  # Abort if the file isn't 32 bytes long. This is to avoid exposing arbitrary
-  # file content to the port.
-  #
-  # Without this a malicious socket could, for instance, claim that
-  # '~/.bash_history' or '~/.ssh/id_rsa' was its authentication cookie to trick
-  # us into reading it for them with our current permissions.
-  #
-  # https://trac.torproject.org/projects/tor/ticket/4303
-  
-  auth_cookie_size = os.path.getsize(cookie_path)
-  
-  if auth_cookie_size != 32:
-    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, cookie_path, auth_type = auth_type)
-  
-  try:
-    with file(cookie_path, 'rb', 0) as f:
-      cookie_data = f.read()
-      return cookie_data
-  except IOError, exc:
-    raise UnreadableCookieFile("Authentication failed: unable to read '%s' (%s)"
-        % (cookie_path, exc), cookie_path, auth_type = auth_type)
-
 def authenticate_cookie(controller, cookie_path, suppress_ctl_errors = True):
   """
   Authenticates to a control socket that uses the contents of an authentication
@@ -549,7 +509,7 @@ def authenticate_cookie(controller, cookie_path, suppress_ctl_errors = True):
     * :class:`stem.connection.IncorrectCookieValue` if the cookie file's value is rejected
   """
   
-  cookie_data = _read_cookie(cookie_path, AuthMethod.COOKIE)
+  cookie_data = _read_cookie(cookie_path, False)
   
   try:
     msg = "AUTHENTICATE %s" % binascii.b2a_hex(cookie_data)
@@ -566,33 +526,37 @@ def authenticate_cookie(controller, cookie_path, suppress_ctl_errors = True):
       
       if "*or* authentication cookie." in str(auth_response) or \
          "Authentication cookie did not match expected value." in str(auth_response):
-        raise IncorrectCookieValue(str(auth_response), cookie_path, auth_response)
+        raise IncorrectCookieValue(str(auth_response), cookie_path, False, auth_response)
       else:
-        raise CookieAuthRejected(str(auth_response), cookie_path, auth_response)
+        raise CookieAuthRejected(str(auth_response), cookie_path, False, auth_response)
   except stem.socket.ControllerError, exc:
     try: controller.connect()
     except: pass
     
     if not suppress_ctl_errors: raise exc
-    else: raise CookieAuthRejected("Socket failed (%s)" % exc, cookie_path)
+    else: raise CookieAuthRejected("Socket failed (%s)" % exc, cookie_path, False)
 
 def authenticate_safecookie(controller, cookie_path, suppress_ctl_errors = True):
   """
   Authenticates to a control socket using the safe cookie method, which is
   enabled by setting the CookieAuthentication torrc option on Tor client's which
-  support it. This uses a two-step process - first, it sends a nonce to the
-  server and receives a challenge from the server of the cookie's contents.
-  Next, it generates a hash digest using the challenge received in the first
-  step and uses it to authenticate to 'controller'.
+  support it.
+  
+  Authentication with this is a two-step process...
+  
+  1. send a nonce to the server and receives a challenge from the server for
+     the cookie's contents
+  2. generate a hash digest using the challenge received in the first step, and
+     use it to authenticate the controller
   
   The IncorrectCookieSize and UnreadableCookieFile exceptions take 
   precedence over the other exception types.
-
-  The UnrecognizedAuthChallengeMethod, AuthChallengeFailed, InvalidClientNonce
-  and CookieAuthRejected exceptions are next in the order of precedence.
-  Depending on the reason, one of these is raised if the first (AUTHCHALLENGE) step
-  fails.
-
+  
+  The AuthChallengeUnsupported, UnrecognizedAuthChallengeMethod,
+  InvalidClientNonce and CookieAuthRejected exceptions are next in the order of
+  precedence. Depending on the reason, one of these is raised if the first
+  (AUTHCHALLENGE) step fails.
+  
   In the second (AUTHENTICATE) step, IncorrectCookieValue or
   CookieAuthRejected maybe raised.
   
@@ -612,74 +576,77 @@ def authenticate_safecookie(controller, cookie_path, suppress_ctl_errors = True)
     * :class:`stem.connection.CookieAuthRejected` if cookie authentication is attempted but the socket doesn't accept it
     * :class:`stem.connection.IncorrectCookieValue` if the cookie file's value is rejected
     * :class:`stem.connection.UnrecognizedAuthChallengeMethod` if the Tor client fails to recognize the AuthChallenge method
-    * :class:`stem.connection.AuthChallengeFailed` if AUTHCHALLENGE is unimplemented, or if unable to parse AUTHCHALLENGE response
+    * :class:`stem.connection.AuthChallengeUnsupported` if AUTHCHALLENGE is unimplemented, or if unable to parse AUTHCHALLENGE response
     * :class:`stem.connection.AuthSecurityFailure` if AUTHCHALLENGE's response looks like a security attack
     * :class:`stem.connection.InvalidClientNonce` if stem's AUTHCHALLENGE client nonce is rejected for being invalid
   """
   
-  cookie_data = _read_cookie(cookie_path, AuthMethod.SAFECOOKIE)
-  client_nonce =  stem.util.connection.random_bytes(32)
-  client_hash_const = "Tor safe cookie authentication controller-to-server hash"
-  server_hash_const = "Tor safe cookie authentication server-to-controller hash"
-
-  try:
-    challenge_response = _msg(controller, "AUTHCHALLENGE SAFECOOKIE %s" %
-        binascii.b2a_hex(client_nonce))
+  cookie_data = _read_cookie(cookie_path, True)
+  client_nonce = os.urandom(32)
   
-    if not challenge_response.is_ok():
+  try:
+    client_nonce_hex = binascii.b2a_hex(client_nonce)
+    authchallenge_response = _msg(controller, "AUTHCHALLENGE SAFECOOKIE %s" % client_nonce_hex)
+    
+    if not authchallenge_response.is_ok():
       try: controller.connect()
       except: pass
       
-      challenge_response_str = str(challenge_response)
-      if "AUTHCHALLENGE only supports" in challenge_response_str:
-        raise UnrecognizedAuthChallengeMethod(challenge_response_str, cookie_path, AuthMethod.SAFECOOKIE)
-      elif "Invalid base16 client nonce" in challenge_response_str:
-        raise InvalidClientNonce(challenge_response_str, cookie_path)
-      elif "Authentication required." in challenge_response_str:
-        raise AuthChallengeFailed("SAFECOOKIE Authentication unimplemented", cookie_path)
-      elif "Cookie authentication is disabled." in challenge_response_str:
-        raise CookieAuthRejected(challenge_response_str, cookie_path, auth_type = AuthMethod.SAFECOOKIE)
+      authchallenge_response_str = str(authchallenge_response)
+      if "Authentication required." in authchallenge_response_str:
+        raise AuthChallengeUnsupported("SAFECOOKIE authentication isn't supported", cookie_path)
+      elif "AUTHCHALLENGE only supports" in authchallenge_response_str:
+        raise UnrecognizedAuthChallengeMethod(authchallenge_response_str, cookie_path)
+      elif "Invalid base16 client nonce" in authchallenge_response_str:
+        raise InvalidClientNonce(authchallenge_response_str, cookie_path)
+      elif "Cookie authentication is disabled" in authchallenge_response_str:
+        raise CookieAuthRejected(authchallenge_response_str, cookie_path, True)
       else:
-        raise AuthChallengeFailed(challenge_response, cookie_path)
+        raise AuthChallengeFailed(authchallenge_response, cookie_path)
   except stem.socket.ControllerError, exc:
     try: controller.connect()
     except: pass
     
     if not suppress_ctl_errors: raise exc
-    else: raise AuthChallengeFailed("Socket failed (%s)" % exc, cookie_path)
-
+    else: raise AuthChallengeFailed("Socket failed (%s)" % exc, cookie_path, True)
+  
   try:
-    stem.response.convert("AUTHCHALLENGE", challenge_response)
+    stem.response.convert("AUTHCHALLENGE", authchallenge_response)
   except stem.socket.ProtocolError, exc:
     if not suppress_ctl_errors: raise exc
     else: raise AuthChallengeFailed("Unable to parse AUTHCHALLENGE response: %s" % exc, cookie_path)
-
-  expected_server_hash = stem.util.connection.hmac_sha256(server_hash_const,
-      cookie_data + client_nonce + challenge_response.server_nonce)
-  if not stem.util.connection.cryptovariables_equal(challenge_response.server_hash, expected_server_hash):
-    raise AuthSecurityFailure("Server hash is wrong -- attack?", cookie_path)
-
+  
+  expected_server_hash = stem.util.connection.hmac_sha256(SERVER_HASH_CONSTANT,
+      cookie_data + client_nonce + authchallenge_response.server_nonce)
+  
+  if not stem.util.connection.cryptovariables_equal(authchallenge_response.server_hash, expected_server_hash):
+    raise AuthSecurityFailure("Tor provided the wrong server nonce", cookie_path)
+  
   try:
-    client_hash = stem.util.connection.hmac_sha256(client_hash_const,
-        cookie_data + client_nonce + challenge_response.server_nonce)
-    controller.send("AUTHENTICATE %s" % (binascii.b2a_hex(client_hash)))
-    auth_response = controller.recv()
+    client_hash = stem.util.connection.hmac_sha256(CLIENT_HASH_CONSTANT,
+        cookie_data + client_nonce + authchallenge_response.server_nonce)
+    auth_response = _msg(controller, "AUTHENTICATE %s" % (binascii.b2a_hex(client_hash)))
   except stem.socket.ControllerError, exc:
     try: controller.connect()
     except: pass
     
     if not suppress_ctl_errors: raise exc
-    else: raise CookieAuthRejected("Socket failed (%s)" % exc, cookie_path, auth_response, AuthMethod.SAFECOOKIE)
-
+    else: raise CookieAuthRejected("Socket failed (%s)" % exc, cookie_path, True, auth_response)
+  
   # if we got anything but an OK response then err
   if not auth_response.is_ok():
     try: controller.connect()
     except: pass
     
-    if 'Safe cookie response did not match expected value' in auth_response[0]: #Cookie doesn't match
-      raise IncorrectCookieValue(str(auth_response), cookie_path, auth_response, AuthMethod.SAFECOOKIE)
+    # all we have to go on is the error message from tor...
+    # ... Safe cookie response did not match expected value
+    # ... *or* authentication cookie.
+    
+    if "*or* authentication cookie." in str(auth_response) or \
+       "Safe cookie response did not match expected value" in str(auth_response):
+      raise IncorrectCookieValue(str(auth_response), cookie_path, True, auth_response)
     else:
-      raise CookieAuthRejected(str(auth_response), cookie_path, auth_response, AuthMethod.SAFECOOKIE)
+      raise CookieAuthRejected(str(auth_response), cookie_path, True, auth_response)
 
 def get_protocolinfo(controller):
   """
@@ -755,6 +722,44 @@ def _msg(controller, message):
   else:
     return controller.msg(message)
 
+def _read_cookie(cookie_path, is_safecookie):
+  """
+  Provides the contents of a given cookie file.
+  
+  :param str cookie_path: absolute path of the cookie file
+  :param bool is_safecookie: True if this was for SAFECOOKIE authentication, False if for COOKIE
+  
+  :raises:
+    * :class:`stem.connection.UnreadableCookieFile` if the cookie file is unreadable
+    * :class:`stem.connection.IncorrectCookieSize` if the cookie size is incorrect (not 32 bytes)
+  """
+  
+  if not os.path.exists(cookie_path):
+    exc_msg = "Authentication failed: '%s' doesn't exist" % cookie_path
+    raise UnreadableCookieFile(exc_msg, cookie_path, is_safecookie)
+  
+  # Abort if the file isn't 32 bytes long. This is to avoid exposing arbitrary
+  # file content to the port.
+  #
+  # Without this a malicious socket could, for instance, claim that
+  # '~/.bash_history' or '~/.ssh/id_rsa' was its authentication cookie to trick
+  # us into reading it for them with our current permissions.
+  #
+  # https://trac.torproject.org/projects/tor/ticket/4303
+  
+  auth_cookie_size = os.path.getsize(cookie_path)
+  
+  if auth_cookie_size != 32:
+    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, cookie_path, is_safecookie)
+  
+  try:
+    with file(cookie_path, 'rb', 0) as f:
+      return f.read()
+  except IOError, exc:
+    exc_msg = "Authentication failed: unable to read '%s' (%s)" % (cookie_path, exc)
+    raise UnreadableCookieFile(exc_msg, cookie_path, is_safecookie)
+
 def _expand_cookie_path(protocolinfo_response, pid_resolver, pid_resolution_arg):
   """
   Attempts to expand a relative cookie path with the given pid resolver. This
@@ -832,15 +837,14 @@ class CookieAuthFailed(AuthenticationFailure):
   Failure to authenticate with an authentication cookie.
   
   :param str cookie_path: location of the authentication cookie we attempted
-  :param str auth_type:  cookie authentication type (from the AuthMethod Enum)
+  :param bool is_safecookie: True if this was for SAFECOOKIE authentication, False if for COOKIE
+  :param stem.response.ControlMessage auth_response: reply to our authentication attempt
   """
   
-  def __init__(self, message, cookie_path, auth_response = None, auth_type = AuthMethod.COOKIE):
-    """
-    """
+  def __init__(self, message, cookie_path, is_safecookie, auth_response = None):
     AuthenticationFailure.__init__(self, message, auth_response)
+    self.is_safecookie = is_safecookie
     self.cookie_path = cookie_path
-    self.auth_type = auth_type
 
 class CookieAuthRejected(CookieAuthFailed):
   "Socket does not support password authentication."
@@ -857,29 +861,26 @@ class UnreadableCookieFile(CookieAuthFailed):
 class AuthChallengeFailed(CookieAuthFailed):
   """
   AUTHCHALLENGE command has failed.
-
-  :param str cookie_path: path to the cookie file
-  :param str auth_type: cookie authentication type (from the AuthMethod Enum)
   """
   
-  def __init__(self, message, cookie_path, auth_type = AuthMethod.SAFECOOKIE):
-    CookieAuthFailed.__init__(self, message, cookie_path)
-    self.auth_type = auth_type
+  def __init__(self, message, cookie_path):
+    CookieAuthFailed.__init__(self, message, cookie_path, True)
 
+class AuthChallengeUnsupported(AuthChallengeFailed):
+  """
+  AUTHCHALLENGE isn't implemented.
+  """
 
 class UnrecognizedAuthChallengeMethod(AuthChallengeFailed):
   """
   Tor couldn't recognize our AUTHCHALLENGE method.
   
   :var str authchallenge_method: AUTHCHALLENGE method that Tor couldn't recognize
-  :var str cookie_path: path to the cookie file
-  :var str auth_type: cookie authentication type (from the AuthMethod Enum)
   """
   
-  def __init__(self, message, cookie_path, authchallenge_method, auth_type = AuthMethod.SAFECOOKIE):
-    CookieAuthFailed.__init__(self, message, cookie_path)
+  def __init__(self, message, cookie_path, authchallenge_method):
+    AuthChallengeFailed.__init__(self, message, cookie_path)
     self.authchallenge_method = authchallenge_method
-    self.auth_type = auth_type
 
 class AuthSecurityFailure(AuthChallengeFailed):
   "AUTHCHALLENGE response is invalid."
@@ -897,7 +898,15 @@ 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."
+  """
+  PROTOCOLINFO response supports cookie auth but doesn't have its path.
+  
+  :param bool is_safecookie: True if this was for SAFECOOKIE authentication, False if for COOKIE
+  """
+  
+  def __init__(self, message, is_safecookie):
+    AuthenticationFailure.__init__(self, message)
+    self.is_safecookie = is_safecookie
 
 # authentication exceptions ordered as per the authenticate function's pydocs
 AUTHENTICATE_EXCEPTIONS = (
@@ -908,10 +917,10 @@ AUTHENTICATE_EXCEPTIONS = (
   IncorrectCookieSize,
   UnreadableCookieFile,
   IncorrectCookieValue,
+  AuthChallengeUnsupported,
   UnrecognizedAuthChallengeMethod,
   InvalidClientNonce,
   AuthSecurityFailure,
-  AuthChallengeFailed,
   OpenAuthRejected,
   MissingAuthInfo,
   AuthenticationFailure
diff --git a/stem/descriptor/extrainfo_descriptor.py b/stem/descriptor/extrainfo_descriptor.py
index 0a4e893..9fbdd1d 100644
--- a/stem/descriptor/extrainfo_descriptor.py
+++ b/stem/descriptor/extrainfo_descriptor.py
@@ -420,7 +420,7 @@ class ExtraInfoDescriptor(stem.descriptor.Descriptor):
       elif keyword == "geoip-db-digest":
         # "geoip-db-digest" Digest
         
-        if validate and not re.match("^[0-9a-fA-F]{40}$", value):
+        if validate and not stem.util.tor_tools.is_hex_digits(value, 40):
           raise ValueError("Geoip digest line had an invalid sha1 digest: %s" % line)
         
         self.geoip_db_digest = value
diff --git a/stem/descriptor/server_descriptor.py b/stem/descriptor/server_descriptor.py
index bbce9d1..6f8079b 100644
--- a/stem/descriptor/server_descriptor.py
+++ b/stem/descriptor/server_descriptor.py
@@ -678,7 +678,7 @@ class BridgeDescriptor(ServerDescriptor):
       line = "%s %s" % (keyword, value)
       
       if keyword == "router-digest":
-        if validate and not re.match("^[0-9a-fA-F]{40}$", value):
+        if validate and not stem.util.tor_tools.is_hex_digits(value, 40):
           raise ValueError("Router digest line had an invalid sha1 digest: %s" % line)
         
         self._digest = value
diff --git a/stem/response/__init__.py b/stem/response/__init__.py
index 788658b..a4eac7a 100644
--- a/stem/response/__init__.py
+++ b/stem/response/__init__.py
@@ -81,10 +81,14 @@ def convert(response_type, message):
 class ControlMessage:
   """
   Message from the control socket. This is iterable and can be stringified for
-  individual message components stripped of protocol formatting.
+  individual message components stripped of protocol formatting. Messages are
+  never empty.
   """
   
   def __init__(self, parsed_content, raw_content):
+    if not parsed_content:
+      raise ValueError("ControlMessages can't be empty")
+    
     self._parsed_content = parsed_content
     self._raw_content = raw_content
   
@@ -174,14 +178,14 @@ class ControlMessage:
     """
     :returns: Number of ControlLines
     """
-
+    
     return len(self._parsed_content)
   
   def __getitem__(self, index):
     """
     :returns: ControlLine at index
     """
-
+    
     return ControlLine(self._parsed_content[index][2])
 
 class ControlLine(str):
diff --git a/stem/response/authchallenge.py b/stem/response/authchallenge.py
index 61c9ae8..84df58f 100644
--- a/stem/response/authchallenge.py
+++ b/stem/response/authchallenge.py
@@ -1,67 +1,53 @@
-
 import re
 import binascii
 
 import stem.socket
 import stem.response
+import stem.util.tor_tools
 
 class AuthChallengeResponse(stem.response.ControlMessage):
   """
   AUTHCHALLENGE query response.
   
-  :var str server_hash: server hash returned by Tor
-  :var str server_nonce: server nonce returned by Tor
+  :var str server_hash: server hash provided by tor
+  :var str server_nonce: server nonce provided by tor
   """
   
   def _parse_message(self):
     # Example:
     #   250 AUTHCHALLENGE SERVERHASH=680A73C9836C4F557314EA1C4EDE54C285DB9DC89C83627401AEF9D7D27A95D5 SERVERNONCE=F8EA4B1F2C8B40EF1AF68860171605B910E3BBCABADF6FC3DB1FA064F4690E85
     
-    _ProtocolError = stem.socket.ProtocolError
-
-    try:
-      line = self[0]
-    except IndexError:
-      raise _ProtocolError("Received empty AUTHCHALLENGE response")
-
+    self.server_hash = None
+    self.server_nonce = None
+    
+    if not self.is_ok():
+      raise stem.socket.ProtocolError("AUTHCHALLENGE response didn't have an OK status:\n%s" % self)
+    elif len(self) > 1:
+      raise stem.socket.ProtocolError("Received multiline AUTHCHALLENGE response:\n%s" % self)
+    
+    line = self[0]
+    
     # sanity check that we're a AUTHCHALLENGE response
     if not line.pop() == "AUTHCHALLENGE":
-      raise _ProtocolError("Message is not an AUTHCHALLENGE response (%s)" % self)
-
-    if len(self) > 1:
-      raise _ProtocolError("Received multiline AUTHCHALLENGE response (%s)" % line)
-
-    self.server_hash, self.server_nonce = None, None
-
-    try:
-      key, value = line.pop_mapping()
-    except (IndexError, ValueError), exc:
-      raise _ProtocolError(exc.message)
-    if key == "SERVERHASH":
-      if not re.match("^[A-Fa-f0-9]{64}$", value):
-        raise _ProtocolError("SERVERHASH has an invalid value: %s" % value)
-          
+      raise stem.socket.ProtocolError("Message is not an AUTHCHALLENGE response (%s)" % self)
+    
+    if line.is_next_mapping("SERVERHASH"):
+      value = line.pop_mapping()[1]
+      
+      if not stem.util.tor_tools.is_hex_digits(value, 64):
+        raise stem.socket.ProtocolError("SERVERHASH has an invalid value: %s" % value)
+      
       self.server_hash = binascii.a2b_hex(value)
-
-    try:
-      key, value = line.pop_mapping()
-    except (IndexError, ValueError), exc:
-      raise _ProtocolError(exc.message)
-    if key == "SERVERNONCE":
-      if not re.match("^[A-Fa-f0-9]{64}$", value):
-        raise _ProtocolError("SERVERNONCE has an invalid value: %s" % value)
+    else:
+      raise stem.socket.ProtocolError("Missing SERVERHASH mapping: %s" % line)
+    
+    if line.is_next_mapping("SERVERNONCE"):
+      value = line.pop_mapping()[1]
       
-      self.server_nonce = binascii.a2b_hex(value)
+      if not stem.util.tor_tools.is_hex_digits(value, 64):
+        raise stem.socket.ProtocolError("SERVERNONCE has an invalid value: %s" % value)
       
-    msg = ""
-    if not self.server_hash:
-      msg.append("SERVERHASH")
-      if not self.server_nonce:
-        msg.append("and SERVERNONCE")
+      self.server_nonce = binascii.a2b_hex(value)
     else:
-      if not self.server_nonce:
-        msg.append("SERVERNONCE")
-
-    if msg:
-      raise _ProtocolError("AUTHCHALLENGE response is missing %s." % msg)
+      raise stem.socket.ProtocolError("Missing SERVERNONCE mapping: %s" % line)
 
diff --git a/stem/response/protocolinfo.py b/stem/response/protocolinfo.py
index 23bef69..eccad88 100644
--- a/stem/response/protocolinfo.py
+++ b/stem/response/protocolinfo.py
@@ -112,6 +112,8 @@ class ProtocolInfoResponse(stem.response.ControlMessage):
             auth_methods.append(AuthMethod.PASSWORD)
           elif method == "COOKIE":
             auth_methods.append(AuthMethod.COOKIE)
+          elif method == "SAFECOOKIE":
+            auth_methods.append(AuthMethod.SAFECOOKIE)
           else:
             unknown_auth_methods.append(method)
             message_id = "stem.response.protocolinfo.unknown_auth_%s" % method
diff --git a/stem/util/connection.py b/stem/util/connection.py
index ed339c9..03dced9 100644
--- a/stem/util/connection.py
+++ b/stem/util/connection.py
@@ -8,9 +8,10 @@ but for now just moving the parts we need.
 import os
 import re
 import hmac
-import random
 import hashlib
 
+CRYPTOVARIABLE_EQUALITY_COMPARISON_NONCE = os.urandom(32)
+
 def is_valid_ip_address(address):
   """
   Checks if a string is a valid IPv4 address.
@@ -82,44 +83,28 @@ def is_valid_port(entry, allow_zero = False):
   
   return entry > 0 and entry < 65536
 
-
 def hmac_sha256(key, msg):
   """
   Generates a sha256 digest using the given key and message.
-
+  
   :param str key: starting key for the hash
   :param str msg: message to be hashed
-
+  
   :returns; A sha256 digest of msg, hashed using the given key.
   """
-
-  return hmac.new(key, msg, hashlib.sha256).digest()
-
-def random_bytes(length):
-  """
-  Generates and returns a 'length' byte random string.
-
-  :param int length: length of random string to be returned in bytes.
-
-  :returns: A string of length 'length' bytes.
-  """
   
-  return os.urandom(length)
+  return hmac.new(key, msg, hashlib.sha256).digest()
 
-CRYPTOVARIABLE_EQUALITY_COMPARISON_NONCE = random_bytes(32)
 def cryptovariables_equal(x, y):
   """
   Compares two strings for equality securely.
-
+  
   :param str x: string to be compared.
   :param str y: the other string to be compared.
-
+  
   :returns: True if both strings are equal, False otherwise.
   """
-
-  ## Like all too-high-level languages, Python sucks for secure coding.
-  ## I'm not even going to try to compare strings in constant time.
-  ## Fortunately, I have HMAC and a random number generator. -- rransom
+  
   return (hmac_sha256(CRYPTOVARIABLE_EQUALITY_COMPARISON_NONCE, x) ==
       hmac_sha256(CRYPTOVARIABLE_EQUALITY_COMPARISON_NONCE, y))
 
diff --git a/stem/util/tor_tools.py b/stem/util/tor_tools.py
index a4f63c3..7a980c7 100644
--- a/stem/util/tor_tools.py
+++ b/stem/util/tor_tools.py
@@ -13,7 +13,8 @@ import re
 # case insensitive. Tor doesn't define this in the spec so flipping a coin
 # and going with case insensitive.
 
-FINGERPRINT_PATTERN = re.compile("^[0-9a-fA-F]{40}$")
+HEX_DIGIT = "[0-9a-fA-F]"
+FINGERPRINT_PATTERN = re.compile("^%s{40}$" % HEX_DIGIT)
 NICKNAME_PATTERN = re.compile("^[a-zA-Z0-9]{1,19}$")
 
 def is_valid_fingerprint(entry, check_prefix = False):
@@ -45,3 +46,16 @@ def is_valid_nickname(entry):
   
   return bool(NICKNAME_PATTERN.match(entry))
 
+def is_hex_digits(entry, count):
+  """
+  Checks if a string is the given number of hex digits. Digits represented by
+  letters are case insensitive.
+  
+  :param str entry: string to be checked
+  :param int count: number of hex digits to be checked for
+  
+  :returns: True if the string matches this number
+  """
+  
+  return bool(re.match("^%s{%i}$" % (HEX_DIGIT, count), entry))
+
diff --git a/test/integ/connection/authentication.py b/test/integ/connection/authentication.py
index aa7d6c1..ee7bb32 100644
--- a/test/integ/connection/authentication.py
+++ b/test/integ/connection/authentication.py
@@ -9,7 +9,7 @@ import unittest
 import test.runner
 import stem.connection
 import stem.socket
-from stem.version import Version
+import stem.version
 from stem.response.protocolinfo import AuthMethod
 
 # Responses given by tor for various authentication failures. These may change
@@ -39,11 +39,15 @@ def _can_authenticate(auth_type):
     bool that's True if we should be able to authenticate and False otherwise
   """
   
-  tor_options = test.runner.get_runner().get_options()
+  runner = test.runner.get_runner()
+  tor_options = runner.get_options()
   password_auth = test.runner.Torrc.PASSWORD in tor_options
-  safecookie_auth = cookie_auth = test.runner.Torrc.COOKIE in tor_options
+  cookie_auth = test.runner.Torrc.COOKIE in tor_options
+  safecookie_auth = cookie_auth and runner.get_tor_version().meets_requirements(stem.version.Requirement.AUTH_SAFECOOKIE)
   
-  if not password_auth and not cookie_auth: return True # open socket
+  if not password_auth and not cookie_auth:
+    # open socket, anything but safecookie will work
+    return auth_type != stem.connection.AuthMethod.SAFECOOKIE
   elif auth_type == stem.connection.AuthMethod.PASSWORD: return password_auth
   elif auth_type == stem.connection.AuthMethod.COOKIE: return cookie_auth
   elif auth_type == stem.connection.AuthMethod.SAFECOOKIE: return safecookie_auth
@@ -64,26 +68,31 @@ def _get_auth_failure_message(auth_type):
   
   tor_options = test.runner.get_runner().get_options()
   password_auth = test.runner.Torrc.PASSWORD in tor_options
-  safecookie_auth = cookie_auth = test.runner.Torrc.COOKIE in tor_options
+  cookie_auth = test.runner.Torrc.COOKIE in tor_options
   
   if cookie_auth and password_auth:
     return MULTIPLE_AUTH_FAIL
   elif cookie_auth:
     if auth_type == stem.connection.AuthMethod.COOKIE:
-        return INCORRECT_COOKIE_FAIL
+      return INCORRECT_COOKIE_FAIL
     elif auth_type == stem.connection.AuthMethod.SAFECOOKIE:
-        return INCORRECT_SAFECOOKIE_FAIL
+      return INCORRECT_SAFECOOKIE_FAIL
     else:
-        return COOKIE_AUTH_FAIL
+      return COOKIE_AUTH_FAIL
   elif password_auth:
     if auth_type == stem.connection.AuthMethod.PASSWORD:
       return INCORRECT_PASSWORD_FAIL
     else:
       return PASSWORD_AUTH_FAIL
   else:
-    # shouldn't happen unless safecookie, if so then the test has a bug
+    # The only way that we should fail to authenticate to an open control
+    # socket is if we attempt via safecookie (since we get an 'unsupported'
+    # response via the AUTHCHALLENGE call rather than AUTHENTICATE). For
+    # anything else if we get here it indicates that this test has a bug.
+    
     if auth_type == stem.connection.AuthMethod.SAFECOOKIE:
       return SAFECOOKIE_AUTHCHALLENGE_FAIL
+    
     raise ValueError("No methods of authentication. If this is an open socket then auth shouldn't fail.")
 
 class TestAuthenticate(unittest.TestCase):
@@ -92,8 +101,7 @@ class TestAuthenticate(unittest.TestCase):
     self.cookie_auth_methods = [AuthMethod.COOKIE]
     
     tor_version = test.runner.get_runner().get_tor_version()
-    if tor_version >= Version("0.2.2.36") and tor_version < Version("0.2.3.0") \
-        or tor_version >= Version("0.2.3.13-alpha"):
+    if tor_version.meets_requirements(stem.version.Requirement.AUTH_SAFECOOKIE):
       self.cookie_auth_methods.append(AuthMethod.SAFECOOKIE)
   
   def test_authenticate_general_socket(self):
@@ -187,9 +195,14 @@ class TestAuthenticate(unittest.TestCase):
   
   def test_authenticate_general_cookie(self):
     """
-    Tests the authenticate function's password argument.
+    Tests the authenticate function with only cookie authentication methods.
+    This manipulates our PROTOCOLINFO response to test each method
+    individually.
     """
     
+    
+    
+    
     runner = test.runner.get_runner()
     tor_options = runner.get_options()
     is_cookie_only = test.runner.Torrc.COOKIE in tor_options and not test.runner.Torrc.PASSWORD in tor_options
@@ -197,10 +210,15 @@ class TestAuthenticate(unittest.TestCase):
     # test both cookie authentication mechanisms
     with runner.get_tor_socket(False) as control_socket:
       if is_cookie_only:
-        for method in self.cookie_auth_methods:
+        for method in (AuthMethod.COOKIE, AuthMethod.SAFECOOKIE):
           protocolinfo_response = stem.connection.get_protocolinfo(control_socket)
-          protocolinfo_response.auth_methods.remove(method)
-          stem.connection.authenticate(control_socket, chroot_path = runner.get_chroot(), protocolinfo_response = protocolinfo_response)
+          
+          if method in protocolinfo_response.auth_methods:
+            # narrow to *only* use cookie auth or safecooke, so we exercise
+            # both independently
+            
+            protocolinfo_response.auth_methods = (method, )
+            stem.connection.authenticate(control_socket, chroot_path = runner.get_chroot(), protocolinfo_response = protocolinfo_response)
   
   def test_authenticate_none(self):
     """
@@ -246,9 +264,9 @@ class TestAuthenticate(unittest.TestCase):
     Tests the authenticate_cookie function.
     """
     
+    auth_value = test.runner.get_runner().get_auth_cookie_path()
+    
     for auth_type in self.cookie_auth_methods:
-      auth_value = test.runner.get_runner().get_auth_cookie_path()
-      
       if not os.path.exists(auth_value):
         # If the authentication cookie doesn't exist then we'll be getting an
         # error for that rather than rejection. This will even fail if
@@ -260,7 +278,7 @@ class TestAuthenticate(unittest.TestCase):
       elif _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.assertRaises(stem.connection.CookieAuthRejected, self._check_auth, auth_type, auth_value, False)
   
   def test_authenticate_cookie_invalid(self):
     """
@@ -268,34 +286,35 @@ class TestAuthenticate(unittest.TestCase):
     value.
     """
     
+    auth_value = test.runner.get_runner().get_test_dir("fake_cookie")
+    
+    # we need to create a 32 byte cookie file to load from
+    fake_cookie = open(auth_value, "w")
+    fake_cookie.write("0" * 32)
+    fake_cookie.close()
+    
     for auth_type in self.cookie_auth_methods:
-      auth_value = test.runner.get_runner().get_test_dir("fake_cookie")
-      
-      # we need to create a 32 byte cookie file to load from
-      fake_cookie = open(auth_value, "w")
-      fake_cookie.write("0" * 32)
-      fake_cookie.close()
-      
       if _can_authenticate(stem.connection.AuthMethod.NONE):
-        # authentication will work anyway
+        # authentication will work anyway unless this is safecookie
         if auth_type == AuthMethod.COOKIE:
           self._check_auth(auth_type, auth_value)
-        #unless you're trying the safe cookie method
         elif auth_type == AuthMethod.SAFECOOKIE:
-          exc_type = stem.connection.AuthChallengeFailed
+          exc_type = stem.connection.CookieAuthRejected
           self.assertRaises(exc_type, self._check_auth, auth_type, auth_value)
-      
       else:
-        if _can_authenticate(auth_type):
+        if auth_type == AuthMethod.SAFECOOKIE:
+          if _can_authenticate(auth_type):
+            exc_type = stem.connection.AuthSecurityFailure
+          else:
+            exc_type = stem.connection.CookieAuthRejected
+        elif _can_authenticate(auth_type):
           exc_type = stem.connection.IncorrectCookieValue
         else:
           exc_type = stem.connection.CookieAuthRejected
-          if auth_type == AuthMethod.SAFECOOKIE:
-            exc_type = stem.connection.AuthChallengeFailed
         
-        self.assertRaises(exc_type, self._check_auth, auth_type, auth_value)
-      
-      os.remove(auth_value)
+        self.assertRaises(exc_type, self._check_auth, auth_type, auth_value, False)
+    
+    os.remove(auth_value)
   
   def test_authenticate_cookie_missing(self):
     """
@@ -314,33 +333,14 @@ class TestAuthenticate(unittest.TestCase):
     socket.
     """
     
-    auth_type = AuthMethod.COOKIE
-    auth_value = test.runner.get_runner().get_torrc_path(True)
-    
-    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:
-      self.assertRaises(stem.connection.IncorrectCookieSize, self._check_auth, auth_type, auth_value, False)
-  
-  def test_authenticate_safecookie_wrong_size(self):
-    """
-    Tests the authenticate_safecookie function with our torrc as an auth cookie.
-    This is to confirm that we won't read arbitrary files to the control
-    socket.
-    """
-    
-    auth_type = AuthMethod.SAFECOOKIE
     auth_value = test.runner.get_runner().get_torrc_path(True)
     
-    auth_value = test.runner.get_runner().get_test_dir("fake_cookie")
-    
-    # we need to create a 32 byte cookie file to load from
-    fake_cookie = open(auth_value, "w")
-    fake_cookie.write("0" * 48)
-    fake_cookie.close()
-    self.assertRaises(stem.connection.IncorrectCookieSize,
-        stem.connection.authenticate_safecookie, auth_type, auth_value, False)
+    for auth_type in self.cookie_auth_methods:
+      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:
+        self.assertRaises(stem.connection.IncorrectCookieSize, self._check_auth, auth_type, auth_value, False)
   
   def _check_auth(self, auth_type, auth_arg = None, check_message = True):
     """
@@ -378,10 +378,7 @@ class TestAuthenticate(unittest.TestCase):
         
         # check that we got the failure message that we'd expect
         if check_message:
-          if auth_type != AuthMethod.SAFECOOKIE:
-            failure_msg = _get_auth_failure_message(auth_type)
-          else:
-            failure_msg = _get_auth_failure_message(auth_type)
+          failure_msg = _get_auth_failure_message(auth_type)
           self.assertEqual(failure_msg, str(exc))
         
         raise exc
diff --git a/test/integ/response/protocolinfo.py b/test/integ/response/protocolinfo.py
index f9c96b0..6b7e397 100644
--- a/test/integ/response/protocolinfo.py
+++ b/test/integ/response/protocolinfo.py
@@ -8,6 +8,7 @@ import unittest
 import test.runner
 import stem.socket
 import stem.connection
+import stem.version
 import stem.util.system
 import test.mocking as mocking
 from test.integ.util.system import filter_system_call
@@ -116,11 +117,15 @@ class TestProtocolInfo(unittest.TestCase):
     
     runner = test.runner.get_runner()
     tor_options = runner.get_options()
+    tor_version = runner.get_tor_version()
     auth_methods, auth_cookie_path = [], None
     
     if test.runner.Torrc.COOKIE in tor_options:
       auth_methods.append(stem.response.protocolinfo.AuthMethod.COOKIE)
-      auth_methods.append(stem.response.protocolinfo.AuthMethod.SAFECOOKIE)
+      
+      if tor_version.meets_requirements(stem.version.Requirement.AUTH_SAFECOOKIE):
+        auth_methods.append(stem.response.protocolinfo.AuthMethod.SAFECOOKIE)
+      
       chroot_path = runner.get_chroot()
       auth_cookie_path = runner.get_auth_cookie_path()
       
diff --git a/test/unit/connection/authentication.py b/test/unit/connection/authentication.py
index bd56727..085eae6 100644
--- a/test/unit/connection/authentication.py
+++ b/test/unit/connection/authentication.py
@@ -12,8 +12,6 @@ various error conditions, and make sure that the right exception is raised.
 import unittest
 
 import stem.connection
-import stem.response
-import stem.response.authchallenge
 import stem.util.log as log
 import test.mocking as mocking
 
@@ -92,12 +90,10 @@ class TestAuthenticate(unittest.TestCase):
       stem.connection.IncorrectPassword(None))
     
     all_auth_cookie_exc = (None,
-      stem.connection.IncorrectCookieSize(None, None),
-      stem.connection.UnreadableCookieFile(None, None),
-      stem.connection.CookieAuthRejected(None, None),
-      stem.connection.IncorrectCookieValue(None, None))
-    
-    all_auth_safecookie_exc = all_auth_cookie_exc + (
+      stem.connection.IncorrectCookieSize(None, False, None),
+      stem.connection.UnreadableCookieFile(None, False, None),
+      stem.connection.CookieAuthRejected(None, False, None),
+      stem.connection.IncorrectCookieValue(None, False, None),
       stem.connection.UnrecognizedAuthChallengeMethod(None, None, None),
       stem.connection.AuthChallengeFailed(None, None),
       stem.connection.AuthSecurityFailure(None, None),
@@ -114,7 +110,6 @@ class TestAuthenticate(unittest.TestCase):
     all_auth_none_exc += control_exc
     all_auth_password_exc += control_exc
     all_auth_cookie_exc += control_exc
-    all_auth_safecookie_exc += control_exc
     
     for protocolinfo_auth_methods in _get_all_auth_method_combinations():
       # protocolinfo input for the authenticate() call we'll be making
@@ -126,38 +121,46 @@ class TestAuthenticate(unittest.TestCase):
       for auth_none_exc in all_auth_none_exc:
         for auth_password_exc in all_auth_password_exc:
           for auth_cookie_exc in all_auth_cookie_exc:
-            for auth_safecookie_exc in all_auth_cookie_exc:
-              # determine if the authenticate() call will succeed and mock each
-              # of the authenticate_* function to raise its given exception
-              
-              expect_success = False
-              auth_mocks = {
-                stem.connection.AuthMethod.NONE:
-                  (stem.connection.authenticate_none, auth_none_exc),
-                stem.connection.AuthMethod.PASSWORD:
-                  (stem.connection.authenticate_password, auth_password_exc),
-                stem.connection.AuthMethod.COOKIE:
-                  (stem.connection.authenticate_cookie, auth_cookie_exc),
-                stem.connection.AuthMethod.SAFECOOKIE:
-                  (stem.connection.authenticate_safecookie, auth_safecookie_exc),
-              }
+            # Determine if the authenticate() call will succeed and mock each
+            # of the authenticate_* function to raise its given exception.
+            #
+            # This implementation is slightly inaccurate in a couple regards...
+            # a. it raises safecookie exceptions from authenticate_cookie()
+            # b. exceptions raised by authenticate_cookie() and
+            #    authenticate_safecookie() are always the same
+            #
+            # However, adding another loop for safe_cookie exceptions means
+            # multiplying our runtime many fold. This exercises everything that
+            # matters so the above inaccuracies seem fine.
+            
+            expect_success = False
+            auth_mocks = {
+              stem.connection.AuthMethod.NONE:
+                (stem.connection.authenticate_none, auth_none_exc),
+              stem.connection.AuthMethod.PASSWORD:
+                (stem.connection.authenticate_password, auth_password_exc),
+              stem.connection.AuthMethod.COOKIE:
+                (stem.connection.authenticate_cookie, auth_cookie_exc),
+              stem.connection.AuthMethod.SAFECOOKIE:
+                (stem.connection.authenticate_safecookie, auth_cookie_exc),
+            }
+            
+            for auth_method in auth_mocks:
+              auth_function, raised_exc = auth_mocks[auth_method]
               
-              for auth_method in auth_mocks:
-                auth_function, raised_exc = auth_mocks[auth_method]
+              if not raised_exc:
+                # Mocking this authentication method so it will succeed. If
+                # it's among the protocolinfo methods then expect success.
                 
-                if not raised_exc:
-                  # Mocking this authentication method so it will succeed. If
-                  # it's among the protocolinfo methods then expect success.
-                  
-                  mocking.mock(auth_function, mocking.no_op())
-                  expect_success |= auth_method in protocolinfo_auth_methods
-                else:
-                  mocking.mock(auth_function, mocking.raise_exception(raised_exc))
-              
-              if expect_success:
-                stem.connection.authenticate(None, "blah", None, protocolinfo_arg)
+                mocking.mock(auth_function, mocking.no_op())
+                expect_success |= auth_method in protocolinfo_auth_methods
               else:
-                self.assertRaises(stem.connection.AuthenticationFailure, stem.connection.authenticate, None, "blah", None, protocolinfo_arg)
+                mocking.mock(auth_function, mocking.raise_exception(raised_exc))
+            
+            if expect_success:
+              stem.connection.authenticate(None, "blah", None, protocolinfo_arg)
+            else:
+              self.assertRaises(stem.connection.AuthenticationFailure, stem.connection.authenticate, None, "blah", None, protocolinfo_arg)
     
     # revert logging back to normal
     stem_logger.setLevel(log.logging_level(log.TRACE))
diff --git a/test/unit/response/authchallenge.py b/test/unit/response/authchallenge.py
index 593d3ac..f85856b 100644
--- a/test/unit/response/authchallenge.py
+++ b/test/unit/response/authchallenge.py
@@ -9,26 +9,31 @@ import stem.response
 import stem.response.authchallenge
 import test.mocking as mocking
 
+VALID_RESPONSE = "250 AUTHCHALLENGE \
+SERVERHASH=B16F72DACD4B5ED1531F3FCC04B593D46A1E30267E636EA7C7F8DD7A2B7BAA05 \
+SERVERNONCE=653574272ABBB49395BD1060D642D653CFB7A2FCE6A4955BCFED819703A9998C"
+
+VALID_HASH = "\xb1or\xda\xcdK^\xd1S\x1f?\xcc\x04\xb5\x93\xd4j\x1e0&~cn\xa7\xc7\xf8\xddz+{\xaa\x05"
+VALID_NONCE = "e5t'*\xbb\xb4\x93\x95\xbd\x10`\xd6B\xd6S\xcf\xb7\xa2\xfc\xe6\xa4\x95[\xcf\xed\x81\x97\x03\xa9\x99\x8c"
+INVALID_RESPONSE = "250 AUTHCHALLENGE \
+SERVERHASH=FOOBARB16F72DACD4B5ED1531F3FCC04B593D46A1E30267E636EA7C7F8DD7A2B7BAA05 \
+SERVERNONCE=FOOBAR653574272ABBB49395BD1060D642D653CFB7A2FCE6A4955BCFED819703A9998C"
+
 class TestAuthChallengeResponse(unittest.TestCase):
-  VALID_RESPONSE = "250 AUTHCHALLENGE SERVERHASH=B16F72DACD4B5ED1531F3FCC04B593D46A1E30267E636EA7C7F8DD7A2B7BAA05 SERVERNONCE=653574272ABBB49395BD1060D642D653CFB7A2FCE6A4955BCFED819703A9998C"
-  VALID_HASH = "\xb1or\xda\xcdK^\xd1S\x1f?\xcc\x04\xb5\x93\xd4j\x1e0&~cn\xa7\xc7\xf8\xddz+{\xaa\x05"
-  VALID_NONCE = "e5t'*\xbb\xb4\x93\x95\xbd\x10`\xd6B\xd6S\xcf\xb7\xa2\xfc\xe6\xa4\x95[\xcf\xed\x81\x97\x03\xa9\x99\x8c"
-  INVALID_RESPONSE = "250 AUTHCHALLENGE SERVERHASH=FOOBARB16F72DACD4B5ED1531F3FCC04B593D46A1E30267E636EA7C7F8DD7A2B7BAA05 SERVERNONCE=FOOBAR653574272ABBB49395BD1060D642D653CFB7A2FCE6A4955BCFED819703A9998C"
-  
   def test_valid_response(self):
     """
     Parses valid AUTHCHALLENGE responses.
     """
     
-    control_message = mocking.get_message(self.VALID_RESPONSE)
+    control_message = mocking.get_message(VALID_RESPONSE)
     stem.response.convert("AUTHCHALLENGE", control_message)
     
     # now this should be a AuthChallengeResponse (ControlMessage subclass)
     self.assertTrue(isinstance(control_message, stem.response.ControlMessage))
     self.assertTrue(isinstance(control_message, stem.response.authchallenge.AuthChallengeResponse))
     
-    self.assertEqual(self.VALID_HASH, control_message.server_hash)
-    self.assertEqual(self.VALID_NONCE, control_message.server_nonce)
+    self.assertEqual(VALID_HASH, control_message.server_hash)
+    self.assertEqual(VALID_NONCE, control_message.server_nonce)
   
   def test_invalid_responses(self):
     """
@@ -36,22 +41,14 @@ class TestAuthChallengeResponse(unittest.TestCase):
     appropriate exceptions.
     """
     
-    valid_resp = self.VALID_RESPONSE.split()
-    
-    control_message = mocking.get_message(' '.join(valid_resp[0:1] + [valid_resp[3]]))
-    self.assertRaises(stem.socket.ProtocolError, stem.response.convert, "AUTHCHALLENGE", control_message)
-    
-    control_message = mocking.get_message(' '.join(valid_resp[0:1] + [valid_resp[3], valid_resp[2]]))
-    self.assertRaises(stem.socket.ProtocolError, stem.response.convert, "AUTHCHALLENGE", control_message)
-    
-    control_message = mocking.get_message(' '.join(valid_resp[0:2]))
-    self.assertRaises(stem.socket.ProtocolError, stem.response.convert, "AUTHCHALLENGE", control_message)
+    auth_challenge_comp = VALID_RESPONSE.split()
     
-    for begin in range(4):
-      for end in range(4):
-        try:
-          control_message = mocking.get_message(' '.join(self.VALID_RESPONSE.split()[begin:end]))
-        except stem.socket.ProtocolError:
-          continue
-        self.assertRaises(stem.socket.ProtocolError, stem.response.convert, "AUTHCHALLENGE", control_message)
+    for i in xrange(1, len(auth_challenge_comp)):
+      # Attempts to parse a message without this item. The first item is
+      # skipped because, without the 250 code, the message won't be
+      # constructed.
+      
+      remaining_comp = auth_challenge_comp[:i] + auth_challenge_comp[i + 1:]
+      control_message = mocking.get_message(' '.join(remaining_comp))
+      self.assertRaises(stem.socket.ProtocolError, stem.response.convert, "AUTHCHALLENGE", control_message)
 



More information about the tor-commits mailing list