Commits

Thomas Waldmann committed dbcadc7

passlib support - strong crypto hashes

updated code, tests and docs

use passlib for:
- wiki user password hashes
- constant time comparison of pw recovery token
- random_string
- recovery token generation

require passlib >= 1.6.0 in setup.py (consteq is new in 1.6)

reduce validity of pw recovery token to 2h, use sha256 (instead of sha1), reduce key length to 32chars

cosmetic other changes

  • Participants
  • Parent commits aff38f7

Comments (0)

Files changed (12)

File MoinMoin/_tests/test_user.py

 # -*- coding: utf-8 -*-
 # Copyright: 2003-2004 by Juergen Hermann <jh@web.de>
 # Copyright: 2009 by ReimarBauer
+# Copyright: 2013 by ThomasWaldmann
 # License: GNU GPL v2 (or any later version), see LICENSE.txt for details.
 
 """
         assert u.exists()
 
 
-class TestLoginWithPassword(object):
-    """user: login tests"""
-
+class TestUser(object):
     def setup_method(self, method):
         # Save original user
         self.saved_user = flaskg.user
         # Restore original user
         flaskg.user = self.saved_user
 
+    # Passwords / Login -----------------------------------------------
+
     def testAsciiPassword(self):
         """ user: login with ascii password """
         # Create test user
         theUser = user.User(name=name, password=password)
         assert theUser.valid
 
-    def test_login(self):
+    def testPasswordHash(self):
         """
-        Create user with some password and check that user can login.
+        Create user, set a specific pw hash and check that user can login
+        with the correct password and can not log in with a wrong password.
         """
         # Create test user
         name = u'Test User'
-        password = '12345'
-        salt = 'salt'
-        pw_hash = ''
+        # sha512_crypt passlib hash for '12345':
+        pw_hash = '$6$rounds=1001$y9ObPHKb8cvRCs5G$39IW1i5w6LqXPRi4xqAu3OKv1UOpVKNkwk7zPnidsKZWqi1CrQBpl2wuq36J/s6yTxjCnmaGzv/2.dAmM8fDY/'
         self.createUser(name, pw_hash, True)
 
-        # Try to "login"
-        theuser = user.User(name=name, password=password)
+        # Try to "login" with correct password
+        theuser = user.User(name=name, password='12345')
         assert theuser.valid
 
+        # Try to "login" with a wrong password
+        theuser = user.User(name=name, password='wrong')
+        assert not theuser.valid
+
+    # Subscriptions ---------------------------------------------------
+
     def testSubscriptionSubscribedPage(self):
         """ user: tests is_subscribed_to  """
         pagename = u'HelpMiscellaneous'

File MoinMoin/_tests/wikiconfig.py

     interwikiname = u'MoinTest'
     interwiki_map = dict(Self='http://localhost:8080/', MoinMoin='http://moinmo.in/')
     interwiki_map[interwikiname] = 'http://localhost:8080/'
+
+    passlib_crypt_context = dict(
+        schemes=["sha512_crypt", ],
+        # for the tests, we don't want to have varying rounds
+        sha512_crypt__vary_rounds=0,
+        # for the tests, we want to have a rather low rounds count,
+        # so the tests run quickly (do NOT use low counts in production!)
+        sha512_crypt__default_rounds=1001,
+    )

File MoinMoin/apps/frontend/views.py

 # Copyright: 2012 MoinMoin:CheerXiao
-# Copyright: 2003-2010 MoinMoin:ThomasWaldmann
+# Copyright: 2003-2013 MoinMoin:ThomasWaldmann
 # Copyright: 2011 MoinMoin:AkashSinha
 # Copyright: 2011 MoinMoin:ReimarBauer
 # Copyright: 2008 MoinMoin:FlorianKrupicka
     """Validator for a valid password recovery form
     """
     passwords_mismatch_msg = L_('The passwords do not match.')
-    password_encoding_problem_msg = L_('New password is unacceptable, encoding trouble.')
+    password_problem_msg = L_('New password is unacceptable, could not get processed.')
 
     def validate(self, element, state):
         if element['password1'].value != element['password2'].value:
             return self.note_error(element, state, 'passwords_mismatch_msg')
 
+        password = element['password1'].value
         try:
-            crypto.crypt_password(element['password1'].value)
-        except UnicodeError:
-            return self.note_error(element, state, 'password_encoding_problem_msg')
+            app.cfg.cache.pwd_context.encrypt(password)
+        except (ValueError, TypeError) as err:
+            return self.note_error(element, state, 'password_problem_msg')
 
         return True
 
     """
     passwords_mismatch_msg = L_('The passwords do not match.')
     current_password_wrong_msg = L_('The current password was wrong.')
-    password_encoding_problem_msg = L_('New password is unacceptable, encoding trouble.')
+    password_problem_msg = L_('New password is unacceptable, could not get processed.')
 
     def validate(self, element, state):
         if not (element['password_current'].valid and element['password1'].valid and element['password2'].valid):
         if element['password1'].value != element['password2'].value:
             return self.note_error(element, state, 'passwords_mismatch_msg')
 
+        password = element['password1'].value
         try:
-            crypto.crypt_password(element['password1'].value)
-        except UnicodeError:
-            return self.note_error(element, state, 'password_encoding_problem_msg')
+            app.cfg.cache.pwd_context.encrypt(password)
+        except (ValueError, TypeError) as err:
+            return self.note_error(element, state, 'password_problem_msg')
         return True
 
 

File MoinMoin/config/default.py

 # -*- coding: utf-8 -*-
 # Copyright: 2000-2004 Juergen Hermann <jh@web.de>
-# Copyright: 2005-2011 MoinMoin:ThomasWaldmann
+# Copyright: 2005-2013 MoinMoin:ThomasWaldmann
 # Copyright: 2008      MoinMoin:JohannesBerg
 # Copyright: 2010      MoinMoin:DiogenesAugusto
 # Copyright: 2011      MoinMoin:AkashSinha
                 raise error.ConfigurationError("You must set a (at least {0} chars long) secret string for secrets['{1}']!".format(
                     secret_min_length, secret_key_name))
 
+        from passlib.context import CryptContext
+        try:
+            self.cache.pwd_context = CryptContext(**self.passlib_crypt_context)
+        except ValueError as err:
+            raise error.ConfigurationError("passlib_crypt_context configuration is invalid [{0}].".format(err))
+
     def _config_check(self):
         """ Check namespace and warn about unknown names
 
 
     ('password_checker', DefaultExpression('_default_password_checker'),
      'checks whether a password is acceptable (default check is length >= 6, at least 4 different chars, no keyboard sequence, not username used somehow (you can switch this off by using `None`)'),
+
+    ('passlib_crypt_context', dict(
+        # schemes we want to support (or deprecated schemes for which we still have
+        # hashes in our storage).
+        # note about bcrypt: it needs additional code (that is not pure python and
+        # thus either needs compiling or installing platform-specific binaries)
+        schemes=["sha512_crypt", ],
+        # default scheme for creating new pw hashes (if not given, passlib uses first from schemes)
+        #default="sha512_crypt",
+        # deprecated schemes get auto-upgraded to the default scheme at login
+        # time or when setting a password (including doing a moin account pwreset).
+        #deprecated=["auto"],
+        # vary rounds parameter randomly when creating new hashes...
+        #all__vary_rounds=0.1,
+     ),
+     "passlib CryptContext arguments, see passlib docs"),
   )),
   # ==========================================================================
   'spam_leech_dos': ('Anti-Spam / Leech / DOS',
         scroll_page_after_edit=True,
         show_comments=False,
         want_trivial=False,
+        enc_password=u'',  # empty value == invalid hash
         disabled=False,
         bookmarks={},
         quicklinks=[],

File MoinMoin/script/account/resetpw.py

-# Copyright: 2006 MoinMoin:ThomasWaldmann
+# Copyright: 2006-2013 MoinMoin:ThomasWaldmann
 # Copyright: 2008 MoinMoin:JohannesBerg
 # Copyright: 2011 MoinMoin:ReimarBauer
 # License: GNU GPL v2 (or any later version), see LICENSE.txt for details.
 
 from MoinMoin import user
 from MoinMoin.app import before_wiki
-from MoinMoin.util import crypto
 
 
 class Set_Password(Command):
             print 'This user "{0!r}" does not exists!'.format(u.name)
             return
 
-        u.enc_password = crypto.crypt_password(password)
-        u.save()
-        print 'Password set.'
+        try:
+            u.enc_password = app.cfg.cache.pwd_context.encrypt(password)
+        except (TypeError, ValueError) as err:
+            print "Error: Password could not get processed, aborting."
+        else:
+            u.save()
+            print 'Password set.'

File MoinMoin/script/migration/moin19/import19.py

                 'editor_default', # not used any more
                 'editor_ui', # not used any more
                 'external_target', # ancient, not used any more
-                'passwd', # ancient, not used any more (use enc_passwd)
+                'passwd', # ancient, not used any more (use enc_password)
                 'show_emoticons', # ancient, not used any more
                 'show_fancy_diff', # kind of diff display now depends on mimetype
                 'show_fancy_links', # not used any more (now link rendering depends on theme)
             if key in metadata and metadata[key] in [u'', tuple(), {}, [], ]:
                 del metadata[key]
 
+        # moin2 only supports passlib generated hashes, drop everything else
+        # (users need to do pw recovery in case they are affected)
+        pw = metadata.get('enc_password')
+        if pw is not None:
+            if pw.startswith('{PASSLIB}'):
+                # take it, but strip the prefix as moin2 does not use that any more
+                metadata['enc_password'] = pw[len('{PASSLIB}'):]
+            else:
+                # drop old, unsupported (and also more or less unsafe) hashing scheme
+                del metadata['enc_password']
+
         # TODO quicklinks and subscribed_items - check for non-interwiki elements and convert them to interwiki
 
         return metadata

File MoinMoin/user.py

 # Copyright: 2000-2004 Juergen Hermann <jh@web.de>
-# Copyright: 2003-2012 MoinMoin:ThomasWaldmann
+# Copyright: 2003-2013 MoinMoin:ThomasWaldmann
 # Copyright: 2007 MoinMoin:JohannesBerg
 # Copyright: 2007 MoinMoin:HeinrichWendel
 # Copyright: 2008 MoinMoin:ChristopherDenter
 
 from whoosh.query import Term, And, Or
 
+from MoinMoin import log
+logging = log.getLogger(__name__)
+
 from MoinMoin import wikiutil
 from MoinMoin.config import CONTENTTYPE_USER
 from MoinMoin.constants.keys import *
 from MoinMoin.i18n import _, L_, N_
 from MoinMoin.mail import sendmail
 from MoinMoin.util.interwiki import getInterwikiHome, getInterwikiName, is_local_wiki
-from MoinMoin.util.crypto import crypt_password, upgrade_password, valid_password, \
-                                 generate_token, valid_token, make_uuid
+from MoinMoin.util.crypto import generate_token, valid_token, make_uuid
 from MoinMoin.storage.error import NoSuchItemError, ItemAlreadyExistsError, NoSuchRevisionError
 
 
         if pw_error:
             return _("Password not acceptable: %(msg)s", msg=pw_error)
 
-    try:
-        theuser.set_password(password, is_encrypted)
-    except UnicodeError as err:
-        # Should never happen
-        return "Can't encode password: %(msg)s" % dict(msg=str(err))
+    theuser.set_password(password, is_encrypted)
 
     # try to get the email, for new users it is required
     if validate and not email:
         if not pw_hash or not password:
             return False, False
 
-        # check the password against the password hash
-        if not valid_password(password, pw_hash):
-            return False, False
+        pwd_context = self._cfg.cache.pwd_context
+        password_correct = False
+        recomputed_hash = None
+        try:
+            password_correct, recomputed_hash = pwd_context.verify_and_update(password, pw_hash)
+        except (ValueError, TypeError) as err:
+            logging.error('in user profile %r, verifying the passlib pw hash raised an Exception [%s]' % (self.id, str(err)))
+        else:
+            if recomputed_hash is not None:
+                data[ENC_PASSWORD] = recomputed_hash
+        return password_correct, bool(recomputed_hash)
 
-        new_pw_hash = upgrade_password(password, pw_hash)
-        if not new_pw_hash:
-            return True, False
-
-        data[ENC_PASSWORD] = new_pw_hash
-        return True, True
-
-    def set_password(self, password, is_encrypted=False):
+    def set_password(self, password, is_encrypted=False, salt=None):
         if not is_encrypted:
-            password = crypt_password(password)
+            password = self._cfg.cache.pwd_context.encrypt(password, salt=salt)
         self.profile[ENC_PASSWORD] = password
         # Invalidate all other browser sessions except this one.
         session['user.session_token'] = self.generate_session_token(False)

File MoinMoin/util/_tests/test_crypto.py

 # -*- coding: utf-8 -*-
-# Copyright: 2011 by MoinMoin:ThomasWaldmann
+# Copyright: 2011-2013 by MoinMoin:ThomasWaldmann
 # License: GNU GPL v2 (or any later version), see LICENSE.txt for details.
 
 """
         result = crypto.valid_token(test_key, test_token)
         assert not result
 
+
+class TestCacheKey(object):
+    """ tests for cache key generation """
+
     def test_cache_key(self):
         """ The key must be different for different <kw> """
         test_kw1 = {'MoinMoin': 'value1'}
         result2 = crypto.cache_key(**test_kw2)
         assert result1 != result2, ("Expected different keys for different <kw> but got the same")
 
+
 coverage_modules = ['MoinMoin.util.crypto']

File MoinMoin/util/crypto.py

-# Copyright: 2000-2004 Juergen Hermann <jh@web.de>
-# Copyright: 2003-2011 MoinMoin:ThomasWaldmann
-# Copyright: 2007 MoinMoin:JohannesBerg
-# Copyright: 2007 MoinMoin:HeinrichWendel
-# Copyright: 2008 MoinMoin:ChristopherDenter
-# Copyright: 2010 MoinMoin:DiogenesAugusto
+# Copyright: 2012-2013 MoinMoin:ThomasWaldmann
 # License: GNU GPL v2 (or any later version), see LICENSE.txt for details.
 
 """
 
 Features:
 
-- generate strong, salted cryptographic password hashes for safe pw storage
-- verify cleartext password against any supported crypto (see METHODS)
-- supports password hash upgrades to stronger methods if the cleartext
-  password is available (usually at login time)
 - generate password recovery tokens
 - verify password recovery tokens
 - generate random strings of given length (for salting)
 
 import hashlib
 import hmac
-import random
 import time
 
 from uuid import uuid4
 make_uuid = lambda: unicode(uuid4().hex)
 UUID_LEN = len(make_uuid())
 
-# random stuff
+from passlib.utils import rng, getrandstr, getrandbytes, consteq, generate_password
+
 
 def random_string(length, allowed_chars=None):
     """
     Generate a random string with given length consisting of the given characters.
 
+    Note: this is now just a little wrapper around passlib's randomness code.
+
     :param length: length of the string
     :param allowed_chars: string with allowed characters or None
                           to indicate all 256 byte values should be used
     :returns: random string
     """
     if allowed_chars is None:
-        s = ''.join([chr(random.randint(0, 255)) for dummy in xrange(length)])
+        s = getrandbytes(rng, length)
     else:
-        s = ''.join([random.choice(allowed_chars) for dummy in xrange(length)])
+        s = getrandstr(rng, allowed_chars, length)
     return s
 
 
-# password stuff
-
-def crypt_password(password, salt=None):
-    """
-    Crypt/Hash a cleartext password
-
-    :param password: cleartext password [unicode]
-    :param salt: salt for the password [str] or None to generate a random salt
-    :rtype: str
-    :returns: the password hash
-    """
-    return 'foobar' # TODO
-
-
-def upgrade_password(password, pw_hash):
-    """
-    Upgrade a password to a better hash, if needed
-
-    :param password: cleartext password [unicode]
-    :param pw_hash: password hash (with hash type prefix)
-    :rtype: str
-    :returns: new password hash (or None, if unchanged)
-    """
-    # TODO
-
-def valid_password(password, pw_hash):
-    """
-    Validate a user password.
-
-    :param password: cleartext password to verify [unicode]
-    :param pw_hash: password hash (with hash type prefix)
-    :rtype: bool
-    :returns: password is valid
-    """
-    return True # TODO
-
-
 # password recovery token
 
 def generate_token(key=None, stamp=None):
     :param key: give it to recompute some specific token for verification
     :param stamp: give it to recompute some specific token for verification
     :rtype: 2-tuple
-    :returns: key, token
+    :returns: key, token (both unicode)
     """
     if key is None:
-        key = random_string(64, "abcdefghijklmnopqrstuvwxyz0123456789")
+        key = generate_password(size=32)
     if stamp is None:
         stamp = int(time.time())
-    h = hmac.new(str(key), str(stamp), digestmod=hashlib.sha1).hexdigest()
-    token = str(stamp) + '-' + h
-    return key, token
+    h = hmac.new(str(key), str(stamp), digestmod=hashlib.sha256).hexdigest()
+    token = u"{0}-{1}".format(stamp, h)
+    return unicode(key), token
 
 
-def valid_token(key, token, timeout=12*60*60):
+def valid_token(key, token, timeout=2*60*60):
     """
     check if token is valid with respect to the secret key,
     the token must not be older than timeout seconds.
     if timeout and stamp + timeout < time.time():
         return False
     expected_token = generate_token(key, stamp)[1]
-    return token == expected_token
+    return consteq(token, expected_token)
 
 
 # miscellaneous

File docs/admin/configure.rst

 
 Password storage
 ----------------
-Moin never stores passwords in clear text.
+Moin never stores wiki user passwords in clear text, but uses strong
+cryptographic hashes provided by the "passlib" library, see there for details:
+
+    http://packages.python.org/passlib/.
+
+The passlib docs recommend 3 hashing schemes that have good security:
+sha512_crypt, pbkdf2_sha512 and bcrypt (bcrypt has additional binary/compiled
+package requirements, please refer to the passlib docs in case you want to use
+it).
+
+By default, we use sha512_crypt hashes with default parameters as provided
+by passlib (this is same algorithm as moin >= 1.9.7 used by default).
+
+In case you experience slow logins or feel that you might need to tweak the
+hash generation for other reasons, please read the passlib docs. moin allows
+you to configure passlib's CryptContext params within the wiki config, the
+default is this:
+
+::
+    passlib_crypt_context = dict(
+        schemes=["sha512_crypt", ],
+    )
 
 
 Authorization

File docs/admin/upgrade.rst

 
 From moin < 1.9
 ===============
-If you run an older moin version than 1.9, please first upgrade to moin 1.9.x
-before upgrading to moin2. 
-You may want to run 1.9.x for a while to be sure everything is working as expected.
+If you run an older moin version than 1.9, please first upgrade to a recent
+moin 1.9.x version (preferably >= 1.9.7) before upgrading to moin2.
+You may want to run that for a while to be sure everything is working as expected.
 
 Note: Both moin 1.9.x and moin2 are WSGI applications.
 Upgrading to 1.9 first also makes sense concerning the WSGI / server side.
 
 From moin 1.9.x
 ===============
+
+If you want to keep your user's password hashes and migrate them to moin2,
+make sure you use moin >= 1.9.7 WITH enabled passlib support and that all
+password hashes stored in user profiles are {PASSLIB} hashes. Other hashes
+will get removed in the migration process and users will need to do password
+recovery via email (or with admin help, if that does not work).
+
+
 Backup
 ------
 Have a backup of everything, so you can go back in case it doesn't do what
     sitename = u'...' # same as in 1.9
     item_root = u'...' # see page_front_page in 1.9
 
+    # if you had a custom passlib_crypt_context in 1.9, put it here
+
     # configure backend and ACLs to use in future
     # TODO
 
         'whoosh>=2.4.0', # needed for indexed search
         'sphinx>=1.1', # needed to build the docs
         'pdfminer', # pdf -> text/plain conversion
+        'passlib>=1.6.0', # strong password hashing (1.6 needed for consteq)
         'XStatic>=0.0.2', # support for static file pypi packages
         'XStatic-CKEditor>=3.6.1.2',
         'XStatic-jQuery>=1.8.2',