clarify behavior for secret=None and hash=None

* passing a non-string secret or non-string hash to any
  CryptContext or handler method will now reliably result
  in a TypeError.

  previously, passing hash=None to many handler identify() and verify()
  methods would return False, while others would raise a TypeError.
  other handler methods would alternately throw ValueError or TypeError
  when passed a value that wasn't unicode or bytes.
  the various CryptContext methods also behaved inconsistently,
  depending on the behavior of the underlying handler.

  all of these behaviors are gone, they should all raise the same TypeError.

* redid many of the from_string() methods to verify the hash type.

* moved secret type & size validation to GenericHandler's encrypt/genhash/verify methods.
  this cheaply made the secret validation global to all hashes, and lets
  _calc_digest() implementations trust that the secret is valid.

* updated the CryptContext and handler unittests to verify the above behavior is adhered to.

3 files changed, 95 insertions, 38 deletions

diff --git a/passlib/tests/test_context.py b/passlib/tests/test_context.py
index f9edafc..373d066 100644
--- a/passlib/tests/test_context.py
+++ b/passlib/tests/test_context.py
@@ -521,7 +521,7 @@ admin__context__deprecated = des_crypt, bsdi_crypt
 #CryptContext
 #=========================================================
 class CryptContextTest(TestCase):
-    "test CryptContext object's behavior"
+    "test CryptContext class"
     descriptionPrefix = "CryptContext"
 
     #=========================================================
@@ -893,10 +893,6 @@ class CryptContextTest(TestCase):
         self.assertEqual(cc.identify('$9$232323123$1287319827'), None)
         self.assertRaises(ValueError, cc.identify, '$9$232323123$1287319827', required=True)
 
-        #make sure "None" is accepted
-        self.assertEqual(cc.identify(None), None)
-        self.assertRaises(ValueError, cc.identify, None, required=True)
-
     def test_22_verify(self):
         "test verify() scheme kwd"
         handlers = ["md5_crypt", "des_crypt", "bsdi_crypt"]
@@ -915,14 +911,6 @@ class CryptContextTest(TestCase):
         #check verify using wrong alg
         self.assertRaises(ValueError, cc.verify, 'test', h, scheme='bsdi_crypt')
 
-    def test_23_verify_empty_hash(self):
-        "test verify() allows hash=None"
-        handlers = [hash.md5_crypt, hash.des_crypt, hash.bsdi_crypt]
-        cc = CryptContext(handlers, policy=None)
-        self.assertTrue(not cc.verify("test", None))
-        for handler in handlers:
-            self.assertTrue(not cc.verify("test", None, scheme=handler.name))
-
     def test_24_min_verify_time(self):
         "test verify() honors min_verify_time"
         #NOTE: this whole test assumes time.sleep() and tick()
@@ -1008,6 +996,58 @@ class CryptContextTest(TestCase):
         self.assertIs(new_hash, None)
 
     #=========================================================
+    # border cases
+    #=========================================================
+    def test_30_nonstring_hash(self):
+        "test non-string hash values cause error"
+        #
+        # test hash=None or some other non-string causes TypeError
+        # and that explicit-scheme code path behaves the same.
+        #
+        cc = CryptContext(["des_crypt"])
+        for hash, kwds in [
+                (None, {}),
+                (None, {"scheme": "des_crypt"}),
+                (1, {}),
+                ((), {}),
+                ]:
+
+            self.assertRaises(TypeError, cc.identify, hash, **kwds)
+            self.assertRaises(TypeError, cc.genhash, 'stub', hash, **kwds)
+            self.assertRaises(TypeError, cc.verify, 'stub', hash, **kwds)
+            self.assertRaises(TypeError, cc.verify_and_update, 'stub', hash, **kwds)
+            self.assertRaises(TypeError, cc.hash_needs_update, hash, **kwds)
+
+        #
+        # but genhash *should* accept None if default scheme lacks config string.
+        #
+        cc2 = CryptContext(["mysql323"])
+        self.assertRaises(TypeError, cc2.identify, None)
+        self.assertIsInstance(cc2.genhash("stub", None), str)
+        self.assertRaises(TypeError, cc2.verify, 'stub', None)
+        self.assertRaises(TypeError, cc2.verify_and_update, 'stub', None)
+        self.assertRaises(TypeError, cc2.hash_needs_update, None)
+
+
+    def test_31_nonstring_secret(self):
+        "test non-string password values cause error"
+        cc = CryptContext(["des_crypt"])
+        hash = cc.encrypt("stub")
+        #
+        # test secret=None, or some other non-string causes TypeError
+        #
+        for secret, kwds in [
+                (None, {}),
+                (None, {"scheme": "des_crypt"}),
+                (1, {}),
+                ((), {}),
+                ]:
+            self.assertRaises(TypeError, cc.encrypt, secret, **kwds)
+            self.assertRaises(TypeError, cc.genhash, secret, hash, **kwds)
+            self.assertRaises(TypeError, cc.verify, secret, hash, **kwds)
+            self.assertRaises(TypeError, cc.verify_and_update, secret, hash, **kwds)
+
+    #=========================================================
     # other
     #=========================================================
     def test_90_bcrypt_normhash(self):
diff --git a/passlib/tests/test_utils_handlers.py b/passlib/tests/test_utils_handlers.py
index 30194a6..5044d1e 100644
--- a/passlib/tests/test_utils_handlers.py
+++ b/passlib/tests/test_utils_handlers.py
@@ -14,7 +14,7 @@ from passlib.hash import ldap_md5, sha256_crypt
 from passlib.registry import _unload_handler_name as unload_handler_name, \
     register_crypt_handler, get_crypt_handler
 from passlib.exc import MissingBackendError, PasslibHashWarning
-from passlib.utils import getrandstr, JYTHON, rng, to_unicode
+from passlib.utils import getrandstr, JYTHON, rng
 from passlib.utils.compat import b, bytes, bascii_to_str, str_to_uascii, \
                                  uascii_to_str, unicode, PY_MAX_25
 import passlib.utils.handlers as uh
diff --git a/passlib/tests/utils.py b/passlib/tests/utils.py
index b7dce87..5665259 100644
--- a/passlib/tests/utils.py
+++ b/passlib/tests/utils.py
@@ -42,7 +42,7 @@ from passlib.utils import has_rounds_info, has_salt_info, rounds_cost_values, \
                           classproperty, rng, getrandstr, is_ascii_safe, to_native_str, \
                           repeat_string
 from passlib.utils.compat import b, bytes, iteritems, irange, callable, \
-                                 sb_types, exc_err, u, unicode
+                                 base_string_types, exc_err, u, unicode
 import passlib.utils.handlers as uh
 #local
 __all__ = [
@@ -374,7 +374,7 @@ class TestCase(unittest.TestCase):
             # 3.0 and <= 2.6 didn't have this method at all
             def assertRegex(self, text, expected_regex, msg=None):
                 """Fail the test unless the text matches the regular expression."""
-                if isinstance(expected_regex, sb_types):
+                if isinstance(expected_regex, base_string_types):
                     assert expected_regex, "expected_regex must not be empty."
                     expected_regex = re.compile(expected_regex)
                 if not expected_regex.search(text):
@@ -662,6 +662,10 @@ class HandlerCase(TestCase):
                 msg = "verify failed: secret=%r, hash=%r" % (secret, hash)
             raise self.failureException(msg)
 
+    def check_returned_native_str(self, result, func_name):
+        self.assertIsInstance(result, str,
+            "%s() failed to return native string: %r" % (func_name, result,))
+
     #=========================================================
     # internal class attrs
     #=========================================================
@@ -765,8 +769,7 @@ class HandlerCase(TestCase):
 
             # encrypt should generate hash...
             result = self.do_encrypt(secret)
-            self.assertIsInstance(result, str,
-                                  "encrypt must return native str:")
+            self.check_returned_native_str(result, "encrypt")
 
             # which should be positively identifiable...
             self.assertTrue(self.do_identify(result))
@@ -1202,17 +1205,22 @@ class HandlerCase(TestCase):
             self.assertNotEqual(h2, h1,
                                 "genhash() should be case sensitive")
 
-    def test_62_secret_null(self):
-        "test password=None"
-        _, hash = self.get_sample_hash()
+    def test_62_secret_border(self):
+        "test non-string passwords are rejected"
+        hash = self.get_sample_hash()[1]
+
+        # secret=None
         self.assertRaises(TypeError, self.do_encrypt, None)
         self.assertRaises(TypeError, self.do_genhash, None, hash)
         self.assertRaises(TypeError, self.do_verify, None, hash)
 
-    def test_63_max_password_size(self):
+        # secret=int (picked as example of entirely wrong class)
+        self.assertRaises(TypeError, self.do_encrypt, 1)
+        self.assertRaises(TypeError, self.do_genhash, 1, hash)
+        self.assertRaises(TypeError, self.do_verify, 1, hash)
+
+    def test_63_large_secret(self):
         "test MAX_PASSWORD_SIZE is enforced"
-        if self.is_disabled_handler:
-            raise self.skipTest("not applicable")
         from passlib.exc import PasswordSizeError
         from passlib.utils import MAX_PASSWORD_SIZE
         secret = '.' * (1+MAX_PASSWORD_SIZE)
@@ -1400,25 +1408,28 @@ class HandlerCase(TestCase):
                     __msg__= "genhash() failed to throw error for hash "
                     "belonging to %s: %r" % (name, hash))
 
-    def test_76_none(self):
-        "test empty hashes"
+    def test_76_hash_border(self):
+        "test non-string hashes are rejected"
         #
-        # test hash=None
+        # test hash=None is rejected (except if config=None)
         #
-        # FIXME: allowing value or type error to simplify implementation,
-        # but TypeError is really the correct one here.
-        self.assertFalse(self.do_identify(None))
-        self.assertRaises((ValueError, TypeError), self.do_verify, 'stub', None)
+        self.assertRaises(TypeError, self.do_identify, None)
+        self.assertRaises(TypeError, self.do_verify, 'stub', None)
         if self.supports_config_string:
-            self.assertRaises((ValueError, TypeError), self.do_genhash,
-                'stub', None)
+            self.assertRaises(TypeError, self.do_genhash, 'stub', None)
         else:
             result = self.do_genhash('stub', None)
-            self.assertIsInstance(result, str,
-                "genhash() failed to return native string: %r" % (result,))
+            self.check_returned_native_str(result, "genhash")
 
         #
-        # test hash=''
+        # test hash=int is rejected (picked as example of entirely wrong type)
+        #
+        self.assertRaises(TypeError, self.do_identify, 1)
+        self.assertRaises(TypeError, self.do_verify, 'stub', 1)
+        self.assertRaises(TypeError, self.do_genhash, 'stub', 1)
+
+        #
+        # test hash='' is rejected for all but the plaintext hashes
         #
         for hash in [u(''), b('')]:
             if self.accepts_all_hashes:
@@ -1426,9 +1437,9 @@ class HandlerCase(TestCase):
                 self.assertTrue(self.do_identify(hash))
                 self.do_verify('stub', hash)
                 result = self.do_genhash('stub', hash)
-                self.assertIsInstance(result, str,
-                    "genhash() failed to return native string: %r" % (result,))
+                self.check_returned_native_str(result, "genhash")
             else:
+                # otherwise it should reject them
                 self.assertFalse(self.do_identify(hash),
                     "identify() incorrectly identified empty hash")
                 self.assertRaises(ValueError, self.do_verify, 'stub', hash,
@@ -1436,6 +1447,12 @@ class HandlerCase(TestCase):
                 self.assertRaises(ValueError, self.do_genhash, 'stub', hash,
                     __msg__="genhash() failed to reject empty hash")
 
+        #
+        # test identify doesn't throw decoding errors on 8-bit input
+        #
+        self.do_identify('\xe2\x82\xac\xc2\xa5$') # utf-8
+        self.do_identify('abc\x91\x00') # non-utf8
+
     #---------------------------------------------------------
     # fuzz testing
     #---------------------------------------------------------