Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

This plugin may be vulnerable to timing attacks. #3

Merged
merged 3 commits into from Nov 23, 2011
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
29 changes: 26 additions & 3 deletions repoze/who/plugins/sa.py
Expand Up @@ -123,7 +123,12 @@ class SQLAlchemyAuthenticatorPlugin(_BaseSQLAlchemyPlugin):
that verifies the user's password against the password provided through the
login form (it receives the password to be verified as the only argument
and such method is assumed to be called ``validate_password``).


In order to prevent timing attacks, you could provide a validation function
through the ``dummy_validate_password`` translation (see below), which
should use the same algorithm as in ``validate_password``. This function
will also only receive the password provided by the login form.

If you don't want to call the attributes above as ``user_name`` and/or
``validate_password``, respectively, then you have to "translate" them as
in the sample below::
Expand All @@ -133,6 +138,10 @@ class SQLAlchemyAuthenticatorPlugin(_BaseSQLAlchemyPlugin):

# You have User.verify_password instead of User.validate_password:
authenticator.translations['validate_password'] = 'verify_password'

# You have foo.bar.validate as a dummy validation function
import foo.bar.validate
authenticator.translations['dummy_validate_password'] = foo.bar.validate

.. note::

Expand All @@ -145,18 +154,23 @@ class SQLAlchemyAuthenticatorPlugin(_BaseSQLAlchemyPlugin):

default_translations = _BaseSQLAlchemyPlugin.default_translations.copy()
default_translations['validate_password'] = "validate_password"
default_translations['dummy_validate_password'] = None

# IAuthenticator
def authenticate(self, environ, identity):
if not ("login" in identity and "password" in identity):
return None

user = self.get_user(identity['login'])

dummy_validator = self.translations['dummy_validate_password']

if user:
validator = getattr(user, self.translations['validate_password'])
if validator(identity['password']):
return identity['login']
elif dummy_validator:
# To prevent timing attacks
dummy_validator(identity['password'])


class SQLAlchemyUserMDPlugin(_BaseSQLAlchemyPlugin):
Expand Down Expand Up @@ -212,7 +226,9 @@ def _base_plugin_maker(user_class=None, dbsession=None):

def make_sa_authenticator(user_class=None, dbsession=None,
user_name_translation=None,
validate_password_translation=None):
validate_password_translation=None,
dummy_validate_password_translation=None):

"""
Configure :class:`SQLAlchemyAuthenticatorPlugin`.

Expand All @@ -225,6 +241,9 @@ def make_sa_authenticator(user_class=None, dbsession=None,
:param validate_password_translation: The translation for
``validate_password``, if any.
:type validate_password_translation: str
:param dummy_validate_password_translation: The translation for
``dummy_validate_password``, if any.
:type dummy_validate_password_translation: function
:return: The authenticator.
:rtype: SQLAlchemyAuthenticatorPlugin

Expand All @@ -246,6 +265,7 @@ def make_sa_authenticator(user_class=None, dbsession=None,
dbsession = yourcoolproject.model:DBSession
user_name_translation = username
validate_password_translation = verify_password
dummy_validate_password_translation = yourcoolproject.security:validate
# ...

"""
Expand All @@ -259,6 +279,9 @@ def make_sa_authenticator(user_class=None, dbsession=None,
if validate_password_translation:
authenticator.translations['validate_password'] = \
validate_password_translation
if dummy_validate_password_translation:
authenticator.translations['dummy_validate_password'] = \
resolveDotted(dummy_validate_password_translation)

return authenticator

Expand Down
9 changes: 9 additions & 0 deletions tests/fixture/sa_model.py
Expand Up @@ -32,6 +32,15 @@

metadata = DeclarativeBase.metadata

class DummyValidateException(BaseException):
"""Dummy error used for dummy_validate."""
pass

def dummy_validate(password):
"""Dummy validation function that will only raise a dummy error to know
that we reached this execution path."""
raise DummyValidateException()

def init_model(engine):
"""Call me before using any of the tables or classes in the model."""
DBSession.configure(bind=engine)
Expand Down
22 changes: 22 additions & 0 deletions tests/test_authenticator.py
Expand Up @@ -104,6 +104,15 @@ def test_it(self):
identity = {'login': u'rms', 'password': u'freedom'}
self.assertEqual(u'rms', self.plugin.authenticate(None, identity))

def test_dummy_validate(self):
self.plugin = SQLAlchemyAuthenticatorPlugin(sa_model.User,
sa_model.DBSession)
self.plugin.translations['dummy_validate_password'] = \
sa_model.dummy_validate
identity = {'login': u'QWERTY', 'password': u'freedom'}
self.assertRaises(sa_model.DummyValidateException,
self.plugin.authenticate, None, identity)


class TestAuthenticatorWithElixir(TestAuthenticator):

Expand Down Expand Up @@ -179,3 +188,16 @@ def test_passwd_validator_translation(self):
SQLAlchemyAuthenticatorPlugin))
self.assertEqual(password_validator_translation,
authenticator.translations['validate_password'])

def test_dummy_validate_translation(self):
user_class = 'tests.fixture.sa_model:User'
dbsession = 'tests.fixture.sa_model:DBSession'
dummy_val_translation = 'tests.fixture.sa_model:dummy_validate'
authenticator = make_sa_authenticator(
user_class,
dbsession,
dummy_validate_password_translation=dummy_val_translation)
self.assertTrue(isinstance(authenticator,
SQLAlchemyAuthenticatorPlugin))
self.assertEqual(sa_model.dummy_validate,
authenticator.translations['dummy_validate_password'])