bcrypt padding work:

* added changelog entry re: issue & fix
* bcrypt now warns about padding bits in digest as well
* bcrypt.normhash() now normalizes salt padding bits, digest padding bits, and rounds zero-padding
* hash_needs_update() will now flag unclean bcrypt hashes as needing an update
* UTs for hash_needs_update(), and digest padding bit handling

7 files changed, 140 insertions, 18 deletions

diff --git a/.hgignore b/.hgignore
index ac25f48..7e3cc34 100644
--- a/.hgignore
+++ b/.hgignore
@@ -7,3 +7,4 @@ glob:build
 glob:dist
 glob:*$py.class
 glob:MANIFEST
+glob:.tox
diff --git a/CHANGES b/CHANGES
index 8728a98..cba4a44 100644
--- a/CHANGES
+++ b/CHANGES
@@ -4,6 +4,51 @@
 Release History
 ===============
 
+**1.5.3** (NOT YET RELEASED)
+======================
+
+    Bugfix release -- fixes BCrypt padding/verification issue
+
+    .. _bcrypt-padding-issue:
+
+    This release fixes a single issue with Passlib's BCrypt support: Many
+    BCrypt hashes generated by Passlib (<= 1.5.2) will not successfully verify
+    under some of the other BCrypt implementations, such as OpenBSD's
+    ``/etc/master.passwd``.
+
+    *In detail:*    
+
+        BCrypt hashes contain 4 "padding" bits in the encoded salt, and Passlib
+        (<= 1.5.2) generated salts in a manner which frequently set some of the
+        padding bits to 1.  While Passlib ignores these bits, many BCrypt
+        implementations perform password verification in a way will reject
+        *all* passwords, if any of the padding bits are set.  Thus Passlib's
+        BCrypt salt generation needed to be corrected to ensure compatibility,
+        and a route provided to fix existing hashes already out in the wild
+        [issue 25].
+        
+    *Changes in this release:*
+    
+    .. currentmodule:: passlib.context
+      
+    * BCrypt hashes generated by Passlib now have all padding bits cleared.
+    
+    * Passlib will continue to accept BCrypt hashes that have padding bits
+      set, but when it encounters them, it will issue a :exc:`UserWarning`
+      recommending that the hash should be fixed (see below).
+      
+    * Applications which use :meth:`CryptContext.verify_and_update` will
+      have any such hashes automatically re-encoded the next time the user
+      logs in.
+
+    *To fix existing hashes:*
+    
+        If you have BCrypt hashes which might have their padding bits set,
+        you can import :class:`!passlib.hash.bcrypt`, and
+        call ``clean_hash = bcrypt.normhash(hash)``.
+        This function will clear the padding bits of any BCrypt hashes,
+        and should leave all other strings alone.
+
 **1.5.2** (2011-09-19)
 ======================
 
diff --git a/docs/lib/passlib.hash.bcrypt.rst b/docs/lib/passlib.hash.bcrypt.rst
index 2b9a22d..27027b4 100644
--- a/docs/lib/passlib.hash.bcrypt.rst
+++ b/docs/lib/passlib.hash.bcrypt.rst
@@ -90,6 +90,19 @@ This implementation of bcrypt differs from others in a few ways:
   encoding is desired by an application, the password should be encoded
   before handing it to PassLib.
 
+* Padding Bits
+
+  BCrypt's base64 encoding results in the last character of the salt
+  encoding only 2 bits of data, the remaining 4 are "padding" bits.
+  Similarly, the last character of the digest contains 4 bits of data,
+  and 2 padding bits. Because of the way they are coded, many BCrypt implementations
+  will reject all passwords if these padding bits are not set to 0.
+  Due to a legacy issue with Passlib <= 1.5.2,
+  Passlib instead prints a warning if it encounters hashes with any padding bits set,
+  and will then validate them correctly. 
+  (This behavior will eventually be deprecated and such hashes
+  will throw a :exc:`ValueError` instead).
+
 .. rubric:: Footnotes
 
 .. [#f1] `<http://www.usenix.org/event/usenix99/provos/provos_html/>`_ - the bcrypt format specification
diff --git a/passlib/context.py b/passlib/context.py
index 46bcec3..f27e6c0 100644
--- a/passlib/context.py
+++ b/passlib/context.py
@@ -954,6 +954,11 @@ class CryptContext(object):
 
         #XXX: could check if handler provides it's own helper, eg getattr(handler, "hash_needs_update", None),
         #and call that instead of the following default behavior
+        if hasattr(handler, "_hash_needs_update"):
+            #NOTE: hacking this in for the sake of bcrypt & issue 25,
+            #      will formalize (and possibly change) interface later.
+            if handler._hash_needs_update(hash, **opts):
+                return True
 
         if opts:
             #check if we can parse hash to check it's rounds parameter
diff --git a/passlib/handlers/bcrypt.py b/passlib/handlers/bcrypt.py
index 28a4eea..e17282c 100644
--- a/passlib/handlers/bcrypt.py
+++ b/passlib/handlers/bcrypt.py
@@ -26,7 +26,7 @@ except ImportError: #pragma: no cover - though should run whole suite w/o bcrypt
     bcryptor_engine = None
 #libs
 from passlib.utils import safe_os_crypt, classproperty, handlers as uh, \
-    h64, to_hash_str, rng, getrandstr
+    h64, to_hash_str, rng, getrandstr, bytes
 
 #pkg
 #local
@@ -36,11 +36,12 @@ __all__ = [
 
 # base64 character->value mapping used by bcrypt.
 # this is same as as H64_CHARS, but the positions are different.
-BCHARS = "./ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789"
+BCHARS = u"./ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789"
 
 # last bcrypt salt char should have 4 padding bits set to 0.
 # thus, only the following chars are allowed:
-BCLAST = ".Oeu"
+BSLAST = u".Oeu"
+BHLAST = u'.CGKOSWaeimquy26'
 
 #=========================================================
 #handler
@@ -96,7 +97,7 @@ class bcrypt(uh.HasManyIdents, uh.HasRounds, uh.HasSalt, uh.HasManyBackends, uh.
     #--HasSalt--
     min_salt_size = max_salt_size = 22
     salt_chars = BCHARS
-    #NOTE: 22nd salt char must be in BCLAST, not full BCHARS
+    #NOTE: 22nd salt char must be in BSLAST, not full BCHARS
 
     #--HasRounds--
     default_rounds = 12 #current passlib default
@@ -109,7 +110,7 @@ class bcrypt(uh.HasManyIdents, uh.HasRounds, uh.HasSalt, uh.HasManyBackends, uh.
     #=========================================================
 
     @classmethod
-    def from_string(cls, hash):
+    def from_string(cls, hash, strict=True):
         if not hash:
             raise ValueError("no hash specified")
         if isinstance(hash, bytes):
@@ -121,7 +122,7 @@ class bcrypt(uh.HasManyIdents, uh.HasRounds, uh.HasSalt, uh.HasManyBackends, uh.
             raise ValueError("invalid bcrypt hash")
         rounds, data = hash[len(ident):].split(u"$")
         rval = int(rounds)
-        if rounds != u'%02d' % (rval,):
+        if strict and rounds != u'%02d' % (rval,):
             raise ValueError("invalid bcrypt hash (no rounds padding)")
         salt, chk = data[:22], data[22:]
         return cls(
@@ -129,7 +130,7 @@ class bcrypt(uh.HasManyIdents, uh.HasRounds, uh.HasSalt, uh.HasManyBackends, uh.
             salt=salt,
             checksum=chk or None,
             ident=ident,
-            strict=bool(chk),
+            strict=strict and bool(chk),
         )
 
     def to_string(self, native=True):
@@ -141,10 +142,18 @@ class bcrypt(uh.HasManyIdents, uh.HasRounds, uh.HasSalt, uh.HasManyBackends, uh.
     #=========================================================
 
     @classmethod
+    def _hash_needs_update(cls, hash, **opts):
+        if isinstance(hash, bytes):
+            hash = hash.decode("ascii")
+        if hash.startswith(u"$2a$") and hash[28] not in BSLAST:
+            return True
+        return False
+
+    @classmethod
     def normhash(cls, hash):
         "helper to normalize hash, correcting any bcrypt padding bits"
         if cls.identify(hash):
-            return cls.from_string(hash).to_string()
+            return cls.from_string(hash, strict=False).to_string()
         else:
             return hash
 
@@ -153,21 +162,34 @@ class bcrypt(uh.HasManyIdents, uh.HasRounds, uh.HasSalt, uh.HasManyBackends, uh.
         assert cls.min_salt_size == cls.max_salt_size == cls.default_salt_size == 22
         if salt_size is not None and salt_size != 22:
             raise ValueError("bcrypt salts must be 22 characters in length")
-        return getrandstr(rng, BCHARS, 21) + getrandstr(rng, BCLAST, 1)
+        return getrandstr(rng, BCHARS, 21) + getrandstr(rng, BSLAST, 1)
 
     @classmethod
     def norm_salt(cls, *args, **kwds):
         salt = super(bcrypt, cls).norm_salt(*args, **kwds)
-        if salt and salt[-1] not in BCLAST:
+        if salt and salt[-1] not in BSLAST:
             salt = salt[:-1] + BCHARS[BCHARS.index(salt[-1]) & ~15]
-            assert salt[-1] in BCLAST
+            assert salt[-1] in BSLAST
             warn(
                 "encountered a bcrypt hash with incorrectly set padding bits; "
-                "you may want to run bcrypt.normhash() "
-                "to fix these hashes (see Passlib issue 25)."
+                "you may want to use bcrypt.normhash() "
+                "to fix this; see Passlib 1.5.3 changelog."
                 )
         return salt
 
+    @classmethod
+    def norm_checksum(cls, *args, **kwds):
+        checksum = super(bcrypt, cls).norm_checksum(*args, **kwds)
+        if checksum and checksum[-1] not in BHLAST:
+            checksum = checksum[:-1] + BCHARS[BCHARS.index(checksum[-1]) & ~3]
+            assert checksum[-1] in BHLAST
+            warn(
+                "encountered a bcrypt hash with incorrectly set padding bits; "
+                "you may want to use bcrypt.normhash() "
+                "to fix this; see Passlib 1.5.3 changelog."
+                )
+        return checksum
+    
     #=========================================================
     #primary interface
     #=========================================================
diff --git a/passlib/tests/test_context.py b/passlib/tests/test_context.py
index 10c7b9c..2d461d2 100644
--- a/passlib/tests/test_context.py
+++ b/passlib/tests/test_context.py
@@ -863,6 +863,31 @@ class CryptContextTest(TestCase):
         self.assertIs(new_hash, None)
 
     #=========================================================
+    # other
+    #=========================================================
+    def test_90_bcrypt_normhash(self):
+        "teset verify_and_update / hash_needs_update corrects bcrypt padding"
+        # see issue 25.
+        bcrypt = hash.bcrypt
+        
+        PASS1 = "loppux"
+        BAD1  = "$2a$12$oaQbBqq8JnSM1NHRPQGXORm4GCUMqp7meTnkft4zgSnrbhoKdDV0C"
+        GOOD1 = "$2a$12$oaQbBqq8JnSM1NHRPQGXOOm4GCUMqp7meTnkft4zgSnrbhoKdDV0C"
+        ctx = CryptContext(["bcrypt"])
+        
+        with catch_warnings(record=True) as wlog:
+            warnings.simplefilter("always")
+
+            self.assertTrue(ctx.hash_needs_update(BAD1))
+            self.assertFalse(ctx.hash_needs_update(GOOD1))
+    
+            if bcrypt.has_backend():            
+                self.assertEquals(ctx.verify_and_update(PASS1,GOOD1), (True,None))
+                self.assertEquals(ctx.verify_and_update("x",BAD1), (False,None))
+                res = ctx.verify_and_update(PASS1, BAD1)
+                self.assertTrue(res[0] and res[1] and res[1] != BAD1)
+
+    #=========================================================
     #eoc
     #=========================================================
 
diff --git a/passlib/tests/test_drivers.py b/passlib/tests/test_drivers.py
index 5dcfbb7..526c9be 100644
--- a/passlib/tests/test_drivers.py
+++ b/passlib/tests/test_drivers.py
@@ -132,9 +132,9 @@ class _BCryptTest(HandlerCase):
         # correct provided salts to handle ending correctly,
         # so test_33_genconfig_saltchars doesn't throw warnings.
         if 'salt' in kwds:
-            from passlib.handlers.bcrypt import BCHARS, BCLAST
+            from passlib.handlers.bcrypt import BCHARS, BSLAST
             salt = kwds['salt']
-            if salt and salt[-1] not in BCLAST:
+            if salt and salt[-1] not in BSLAST:
                 salt = salt[:-1] + BCHARS[BCHARS.index(salt[-1])&~15]
             kwds['salt'] = salt
         return self.handler.genconfig(**kwds)
@@ -152,13 +152,13 @@ class _BCryptTest(HandlerCase):
         def check_padding(hash):
             "check bcrypt hash doesn't have salt padding bits set"
             assert hash.startswith("$2a$") and len(hash) >= 28
-            self.assertTrue(hash[28] in BCLAST,
+            self.assertTrue(hash[28] in BSLAST,
                             "padding bits set in hash: %r" % (hash,))
 
         #===============================================================
         # test generated salts
         #===============================================================
-        from passlib.handlers.bcrypt import BCHARS, BCLAST
+        from passlib.handlers.bcrypt import BCHARS, BSLAST
 
         # make sure genconfig & encrypt don't return bad hashes.
         # bug had 15/16 chance of occurring every time salt generated.
@@ -184,14 +184,21 @@ class _BCryptTest(HandlerCase):
         # test handling existing hashes
         #===============================================================
 
+        # 2 bits of salt padding set
         PASS1 = "loppux"
         BAD1  = "$2a$12$oaQbBqq8JnSM1NHRPQGXORm4GCUMqp7meTnkft4zgSnrbhoKdDV0C"
         GOOD1 = "$2a$12$oaQbBqq8JnSM1NHRPQGXOOm4GCUMqp7meTnkft4zgSnrbhoKdDV0C"
 
+        # all 4 bits of salt padding set
         PASS2 = "Passlib11"
         BAD2  = "$2a$12$M8mKpW9a2vZ7PYhq/8eJVcUtKxpo6j0zAezu0G/HAMYgMkhPu4fLK"
         GOOD2 = "$2a$12$M8mKpW9a2vZ7PYhq/8eJVOUtKxpo6j0zAezu0G/HAMYgMkhPu4fLK"
 
+        # bad checksum padding
+        PASS3 = "test"
+        BAD3  = "$2a$04$yjDgE74RJkeqC0/1NheSSOrvKeu9IbKDpcQf/Ox3qsrRS/Kw42qIV"
+        GOOD3 = "$2a$04$yjDgE74RJkeqC0/1NheSSOrvKeu9IbKDpcQf/Ox3qsrRS/Kw42qIS"
+
         # make sure genhash() corrects input
         with catch_warnings(record=True) as wlog:
             warnings.simplefilter("always")
@@ -204,7 +211,11 @@ class _BCryptTest(HandlerCase):
 
             self.assertEqual(bcrypt.genhash(PASS2, GOOD2), GOOD2)
             self.assertFalse(wlog)
-
+            
+            self.assertEqual(bcrypt.genhash(PASS3, BAD3), GOOD3)
+            check_warning(wlog)
+            self.assertFalse(wlog)
+            
         # make sure verify works on both bad and good hashes
         with catch_warnings(record=True) as wlog:
             warnings.simplefilter("always")