added some internal notes about linux/bsd des-crypt border cases

4 files changed, 84 insertions, 158 deletions

diff --git a/passlib/hash/des_crypt.py b/passlib/hash/des_crypt.py
index a9c3a1f..f1bff76 100644
--- a/passlib/hash/des_crypt.py
+++ b/passlib/hash/des_crypt.py
@@ -1,4 +1,53 @@
-"""passlib.hash.des_crypt - traditional unix (DES) crypt"""
+"""passlib.hash.des_crypt - traditional unix (DES) crypt
+
+.. note::
+
+    passlib restricts salt characters to just the hash64 charset,
+    and salt string size to >= 2 chars; since implementations of des-crypt
+    vary in how they handle other characters / sizes...
+
+    linux
+
+        linux crypt() accepts salt characters outside the hash64 charset,
+        and maps them using the following formula (determined by examining crypt's output):
+            chr 0..64:      v = (c-(1-19)) & 63 = (c+18) & 63
+            chr 65..96:     v = (c-(65-12)) & 63 = (c+11) & 63
+            chr 97..127:    v = (c-(97-38)) & 63 = (c+5) & 63
+            chr 128..255:   same as c-128
+
+        invalid salt chars are mirrored back in the resulting hash.
+
+        if the salt is too small, it uses a NUL char for the remaining
+        character (which is treated the same as the char ``G``)
+        when decoding the 12 bit salt. however, it outputs
+        a hash string containing the single salt char twice,
+        resulting in a corrupted hash.
+
+    netbsd
+
+        netbsd crypt() uses a 128-byte lookup table,
+        which is only initialized for the hash64 values.
+        the remaining values < 128 are implicitly zeroed,
+        and values > 128 access past the array bounds
+        (but seem to return 0).
+
+        if the salt string is too small, it reads
+        the NULL char (and continues past the end for bsdi crypt,
+        though the buffer is usually large enough and NULLed).
+        salt strings are output as provided,
+        except for any NULs, which are converted to ``.``.
+
+    openbsd, freebsd
+
+        openbsd crypt() strictly defines the hash64 values as normal,
+        and all other char values as 0. salt chars are reported as provided.
+
+        if the salt or rounds string is too small,
+        it'll read past the end, resulting in unpredictable
+        values, though it'll terminate it's encoding
+        of the output at the first null.
+        this will generally result in a corrupted hash.
+"""
 
 #=========================================================
 #imports
@@ -16,31 +65,25 @@ from passlib.utils.des import mdes_encrypt_int_block
 #pkg
 #local
 __all__ = [
-    "genhash",
-    "genconfig",
-    "encrypt",
-    "identify",
-    "verify",
+    "DesCrypt",
 ]
 
 #=========================================================
 #pure-python backend
 #=========================================================
 def _crypt_secret_to_key(secret):
-    "crypt helper which converts lower 7 bits of first 8 chars of secret -> 56-bit des key"
-    key_value = 0
-    for i, c in enumerate(secret[:8]):
-        key_value |= (ord(c)&0x7f) << (57-8*i)
-    return key_value
+    "crypt helper which converts lower 7 bits of first 8 chars of secret -> 56-bit des key, padded to 64 bits"
+    return sum(
+        (ord(c) & 0x7f) << (57-8*i)
+        for i, c in enumerate(secret[:8])
+    )
 
 def raw_crypt(secret, salt):
     "pure-python fallback if stdlib support not present"
     assert len(salt) == 2
 
-    #NOTE: technically might be able to use
-    #fewer salt chars, not sure what standard behavior is,
-    #so forbidding it for handler.
-
+    #NOTE: technically could accept non-standard salts & single char salt,
+    #but no official spec.
     try:
         salt_value = h64.decode_int12(salt)
     except ValueError:
@@ -58,28 +101,6 @@ def raw_crypt(secret, salt):
     return h64.encode_dc_int64(result)
 
 #=========================================================
-#choose backend
-#=========================================================
-backend = "builtin"
-
-try:
-    #try stdlib module, which is only present under posix
-    from crypt import crypt
-    if crypt("test", "ab") == 'abgOeLfPimXQo':
-        backend = "os-crypt"
-    else:
-        #shouldn't be any unix os which has crypt but doesn't support this format.
-        warn("crypt() failed runtime test for DES-CRYPT support")
-        crypt = None
-except ImportError:
-    #XXX: could check for openssl passwd -des support in libssl
-
-    #TODO: need to reconcile our implementation's behavior
-    # with the stdlib's behavior so error types, messages, and limitations
-    # are the same. (eg: handling of None and unicode chars)
-    crypt = None
-
-#=========================================================
 #handler
 #=========================================================
 class DesCrypt(BackendBaseHandler):
@@ -130,35 +151,21 @@ class DesCrypt(BackendBaseHandler):
         return os_crypt and os_crypt("test", "ab") == 'abgOeLfPimXQo'
 
     def _calc_checksum_builtin(self, secret):
+        #forbidding nul chars because linux crypt (and most C implementations) won't accept it either.
         if '\x00' in secret:
             raise ValueError, "null char in secret"
+        #gotta do something - no official policy since des-crypt predates unicode
         if isinstance(secret, unicode):
             secret = secret.encode("utf-8")
         return raw_crypt(secret, self.salt)
 
     def _calc_checksum_os_crypt(self, secret):
-        #forbidding nul chars because linux crypt (and most C implementations) won't accept it either.
+        #os_crypt() would raise less useful error
         if '\x00' in secret:
             raise ValueError, "null char in secret"
-
-        #XXX: des-crypt predates unicode, not sure if there's an official policy for handing it.
-        #for now, just coercing to utf-8.
+        #gotta do something - no official policy since des-crypt predates unicode
         if isinstance(secret, unicode):
             secret = secret.encode("utf-8")
-
-        #XXX: given a single letter salt, linux crypt returns a hash with the original salt doubled,
-        #     but appears to calculate the hash based on the letter + "G" as the second byte.
-        #     this results in a hash that won't validate, which is DEFINITELY wrong.
-        #     need to find out it's underlying logic, and if it's part of spec,
-        #     or just weirdness that should actually be an error.
-        #     until then, passlib raises an error in genconfig()
-
-        #XXX: given salt chars outside of h64.CHARS range, linux crypt
-        #     does something unknown when decoding salt to 12 bit int,
-        #     successfully creates a hash, but reports the original salt.
-        #     need to find out it's underlying logic, and if it's part of spec,
-        #     or just weirdness that should actually be an error.
-        #     until then, passlib raises an error for bad salt chars.
         return os_crypt(secret, self.salt)[2:]
 
     #=========================================================
diff --git a/passlib/hash/ext_des_crypt.py b/passlib/hash/ext_des_crypt.py
index 08ce485..0fcbe58 100644
--- a/passlib/hash/ext_des_crypt.py
+++ b/passlib/hash/ext_des_crypt.py
@@ -10,17 +10,13 @@ from warnings import warn
 #libs
 from passlib.base import register_crypt_handler
 from passlib.utils.handlers import BaseHandler
-from passlib.utils import norm_rounds, norm_salt, h64, autodocument
+from passlib.utils import h64, autodocument
 from passlib.utils.des import mdes_encrypt_int_block
 from passlib.hash.des_crypt import _crypt_secret_to_key
 #pkg
 #local
 __all__ = [
-    "genhash",
-    "genconfig",
-    "encrypt",
-    "identify",
-    "verify",
+    "ExtDesCrypt",
 ]
 
 #=========================================================
@@ -41,10 +37,6 @@ def raw_ext_crypt(secret, rounds, salt):
         #XXX: would make more sense to raise ValueError, but want to be compatible w/ stdlib crypt
         raise ValueError, "secret must be string without null bytes"
 
-    #XXX: doesn't match stdlib, but just to useful to not add in
-    if isinstance(secret, unicode):
-        secret = secret.encode("utf-8")
-
     #convert secret string into an integer
     key_value = _crypt_secret_to_key(secret)
     idx = 8
@@ -78,9 +70,10 @@ class ExtDesCrypt(BaseHandler):
     rounds_cost = "linear"
 
     checksum_chars = 11
+    checksum_charset = h64.CHARS
 
     # NOTE: OpenBSD login.conf reports 7250 as minimum allowed rounds,
-    # but not sure if that's strictly enforced, or silently clipped.
+    # but that seems to be an OS policy, not a algorithm limitation.
 
     #=========================================================
     #internal helpers
@@ -121,6 +114,8 @@ class ExtDesCrypt(BaseHandler):
     #TODO: check if os_crypt supports ext-des-crypt.
 
     def calc_checksum(self, secret):
+        if isinstance(secret, unicode):
+            secret = secret.encode("utf-8")
         return raw_ext_crypt(secret, self.rounds, self.salt)
 
     #=========================================================
@@ -128,7 +123,7 @@ class ExtDesCrypt(BaseHandler):
     #=========================================================
 
 autodocument(ExtDesCrypt)
-register_crypt_handler(ExtDesCrypt, force=True)
+register_crypt_handler(ExtDesCrypt)
 #=========================================================
 #eof
 #=========================================================
diff --git a/passlib/tests/test_hash_des_crypt.py b/passlib/tests/test_hash_des_crypt.py
index 4e538a3..c79aa6f 100644
--- a/passlib/tests/test_hash_des_crypt.py
+++ b/passlib/tests/test_hash_des_crypt.py
@@ -10,8 +10,8 @@ from logging import getLogger
 #pkg
 from passlib.tests.utils import TestCase, enable_option
 from passlib.tests.handler_utils import _HandlerTestCase
-import passlib.hash.des_crypt as mod
-import passlib.hash.ext_des_crypt as mod2
+from passlib.hash.des_crypt import DesCrypt
+from passlib.hash.ext_des_crypt import ExtDesCrypt
 #module
 log = getLogger(__name__)
 
@@ -20,7 +20,7 @@ log = getLogger(__name__)
 #=========================================================
 class DesCryptTest(_HandlerTestCase):
     "test DesCrypt algorithm"
-    handler = mod.DesCrypt
+    handler = DesCrypt
     secret_chars = 8
 
     #TODO: test
@@ -40,23 +40,24 @@ class DesCryptTest(_HandlerTestCase):
         '!gAwTx2l6NADI',
         ]
 
-if mod.backend != "builtin" and enable_option("all-backends"):
-
-    #monkeypatch des-crypt mod so it uses builtin backend
+if enable_option("all-backends") and DesCrypt.get_backend() != "builtin":
 
     class BuiltinDesCryptTest(DesCryptTest):
         case_prefix = "des-crypt (builtin backend)"
 
         def setUp(self):
-            self.tmp = mod.crypt
-            mod.crypt = None
+            self.tmp = self.handler.get_backend()
+            self.handler.set_backend("builtin")
 
         def cleanUp(self):
-            mod.crypt = self.tmp
+            self.handler.set_backend(self.tmp)
 
+#=========================================================
+#bsdi crypt
+#=========================================================
 class ExtDesCryptTest(_HandlerTestCase):
     "test ExtDesCrypt algorithm"
-    handler = mod2.ExtDesCrypt
+    handler = ExtDesCrypt
     known_correct = [
         (" ", "_K1..crsmZxOLzfJH8iw"),
         ("my", '_KR/.crsmykRplHbAvwA'), #<- to detect old 12-bit rounds bug
@@ -70,86 +71,5 @@ class ExtDesCryptTest(_HandlerTestCase):
     ]
 
 #=========================================================
-#test activate backend (stored in mod._crypt)
-#=========================================================
-#TODO: make these tests work again, or merge them into above.
-##class _DesCryptBackendTest(TestCase):
-##    "test builtin unix crypt backend"
-##
-##    def get_crypt(self):
-##        raise NotImplementedError
-##
-##    known_correct = DesCryptTest.known_correct
-##
-##    def test_knowns(self):
-##        "test known crypt results"
-##        crypt = self.get_crypt()
-##        for secret, result in self.known_correct:
-##
-##            #make sure crypt verifies preserving just salt
-##            out = crypt(secret, result[:2])
-##            self.assertEqual(out, result, "secret=%r using salt alone:" % (secret,))
-##
-##            #make sure crypt verifies preseving salt + fragment of known hash
-##            out = crypt(secret, result[:6])
-##            self.assertEqual(out, result, "secret=%r using salt + fragment:" % (secret,))
-##
-##            #make sure crypt verifies using whole known hash
-##            out = crypt(secret, result)
-##            self.assertEqual(out, result, "secret=%r using whole hash:" % (secret,))
-##
-##    #TODO: deal with border cases where host crypt & bps crypt differ
-##    # (none of which should impact the normal use cases)
-##    #border cases:
-##    #   no salt given, empty salt given, 1 char salt
-##    #   salt w/ non-b64 chars (linux crypt handles this _somehow_)
-##    #test that \x00 is NOT allowed
-##    #test that other chars _are_ allowed
-##
-##    def test_null_in_key(self):
-##        "test null chars in secret"
-##        crypt = self.get_crypt()
-##        #NOTE: this is done to match stdlib crypt behavior.
-##        # would raise ValueError if otherwise had free choice
-##        self.assertRaises(ValueError, crypt, "hello\x00world", "ab")
-##
-##    def test_invalid_salt(self):
-##        "test invalid salts"
-##        crypt = self.get_crypt()
-##
-##        #NOTE: stdlib crypt's behavior is to return "" in this case.
-##        # passlib wraps stdlib crypt so it raises ValueError
-##        self.assertRaises(ValueError, crypt, "fooey","")
-##
-##        #NOTE: stdlib crypt's behavior is rather bizarre in this case
-##        # (see wrapper in passlib.unix_crypt).
-##        # passlib wraps stdlib crypt so it raises ValueError
-##        self.assertRaises(ValueError, crypt, "fooey","f")
-##
-##        #FIXME: stdlib crypt does something unpredictable
-##        #if passed salt chars outside of H64.CHARS range.
-##        #not sure *what* it's algorithm is. should figure that out.
-##        # until then, passlib wraps stdlib crypt so this causes ValueError
-##        self.assertRaises(ValueError, crypt, "fooey", "a@")
-##
-##if mod.backend != "builtin" and enable_option("fallback-backend"):
-##    class BuiltinDesCryptBackendTest(_DesCryptBackendTest):
-##        "test builtin des-crypt backend"
-##        case_prefix = "builtin des-crypt() backend"
-##
-##        def get_crypt(self):
-##            return builtin_crypt
-##
-##if enable_option("backends"):
-##    #NOTE: this will generally be the stdlib implementation,
-##    #which of course is correct, so doing this more to detect deviations in builtin implementation
-##    class ActiveDesCryptBackendTest(_DesCryptBackendTest):
-##        "test active des-crypt backend"
-##        case_prefix = mod.backend + " des-crypt() backend"
-##
-##        def get_crypt(self):
-##            return mod.crypt
-
-#=========================================================
 #EOF
 #=========================================================
diff --git a/passlib/utils/des.py b/passlib/utils/des.py
index 2617305..4335ed8 100644
--- a/passlib/utils/des.py
+++ b/passlib/utils/des.py
@@ -31,6 +31,10 @@ The copyright & license for that source is as follows::
     simple password protection.
     @version $Id: UnixCrypt2.txt,v 1.1.1.1 2005/09/13 22:20:13 christos Exp $
     @author Greg Wilkins (gregw)
+
+netbsd des-crypt implementation,
+which has some nice notes on how this all works -
+    http://fxr.googlebit.com/source/lib/libcrypt/crypt.c?v=NETBSD-CURRENT
 """
 
 #TODO: could use an accelerated C version of this module to speed up lmhash, des-crypt, and ext-des-crypt