From 0cd5600163ecca9bfee6dfb9fb0c83bc7ae26168 Mon Sep 17 00:00:00 2001 From: ReAnzu Date: Sat, 24 Feb 2018 23:45:00 -0600 Subject: [PATCH] Changed password setup to use libsodium and argon2id * Regular SHA256 hashing for passwords is inadequate as modern GPU's can hash generate billions of hashes per second. * Added code to auto migrate old passwords to the new password_hash if the existing password_hash matches either of the legacy password generation schemes (SHA1 or SHA256). * Added migration to support new password_hash format length * Added auth tests --- server/requirements.txt | 1 + server/szurubooru/func/auth.py | 39 ++++++++++++++----- server/szurubooru/migrations/env.py | 10 ++++- ...increase_password_hash_max_field_length.py | 30 ++++++++++++++ server/szurubooru/model/user.py | 2 +- server/szurubooru/tests/conftest.py | 6 +-- server/szurubooru/tests/func/test_auth.py | 37 ++++++++++++++++++ 7 files changed, 110 insertions(+), 15 deletions(-) create mode 100644 server/szurubooru/migrations/versions/9ef1a1643c2a_increase_password_hash_max_field_length.py create mode 100644 server/szurubooru/tests/func/test_auth.py diff --git a/server/requirements.txt b/server/requirements.txt index 2cd15ec1..7cc47868 100644 --- a/server/requirements.txt +++ b/server/requirements.txt @@ -11,3 +11,4 @@ scipy>=0.18.1 elasticsearch>=5.0.0 elasticsearch-dsl>=5.0.0 scikit-image>=0.12 +pynacl>=1.2.1 \ No newline at end of file diff --git a/server/szurubooru/func/auth.py b/server/szurubooru/func/auth.py index 25c991c4..d2bb69ac 100644 --- a/server/szurubooru/func/auth.py +++ b/server/szurubooru/func/auth.py @@ -1,8 +1,11 @@ import hashlib import random from collections import OrderedDict -from szurubooru import config, model, errors +from nacl.exceptions import InvalidkeyError + +from szurubooru import config, model, errors, db from szurubooru.func import util +from nacl.pwhash import argon2id, verify RANK_MAP = OrderedDict([ @@ -17,7 +20,14 @@ RANK_MAP = OrderedDict([ def get_password_hash(salt: str, password: str) -> str: - ''' Retrieve new-style password hash. ''' + """ Retrieve argon2id password hash.""" + return argon2id.str( + (config.config['secret'] + salt + password).encode('utf8') + ).decode('utf8') + + +def get_sha256_legacy_password_hash(salt: str, password: str) -> str: + """ Retrieve old-style sha256 password hash.""" digest = hashlib.sha256() digest.update(config.config['secret'].encode('utf8')) digest.update(salt.encode('utf8')) @@ -25,8 +35,8 @@ def get_password_hash(salt: str, password: str) -> str: return digest.hexdigest() -def get_legacy_password_hash(salt: str, password: str) -> str: - ''' Retrieve old-style password hash. ''' +def get_sha1_legacy_password_hash(salt: str, password: str) -> str: + """ Retrieve old-style sha1 password hash.""" digest = hashlib.sha1() digest.update(b'1A2/$_4xVa') digest.update(salt.encode('utf8')) @@ -47,11 +57,22 @@ def create_password() -> str: def is_valid_password(user: model.User, password: str) -> bool: assert user salt, valid_hash = user.password_salt, user.password_hash - possible_hashes = [ - get_password_hash(salt, password), - get_legacy_password_hash(salt, password) - ] - return valid_hash in possible_hashes + + try: + return verify(user.password_hash.encode('utf8'), + (config.config['secret'] + salt + password).encode('utf8')) + except InvalidkeyError: + possible_hashes = [ + get_sha256_legacy_password_hash(salt, password), + get_sha1_legacy_password_hash(salt, password) + ] + if valid_hash in possible_hashes: + # Convert the user password hash to the new hash + user.password_hash = get_password_hash(salt, password) + db.session.commit() + return True + + return False def has_privilege(user: model.User, privilege_name: str) -> bool: diff --git a/server/szurubooru/migrations/env.py b/server/szurubooru/migrations/env.py index 7065a69e..61b20f4e 100644 --- a/server/szurubooru/migrations/env.py +++ b/server/szurubooru/migrations/env.py @@ -35,7 +35,11 @@ def run_migrations_offline(): ''' url = alembic_config.get_main_option('sqlalchemy.url') alembic.context.configure( - url=url, target_metadata=target_metadata, literal_binds=True) + url=url, + target_metadata=target_metadata, + literal_binds=True, + compare_type=True + ) with alembic.context.begin_transaction(): alembic.context.run_migrations() @@ -56,7 +60,9 @@ def run_migrations_online(): with connectable.connect() as connection: alembic.context.configure( connection=connection, - target_metadata=target_metadata) + target_metadata=target_metadata, + compare_type=True + ) with alembic.context.begin_transaction(): alembic.context.run_migrations() diff --git a/server/szurubooru/migrations/versions/9ef1a1643c2a_increase_password_hash_max_field_length.py b/server/szurubooru/migrations/versions/9ef1a1643c2a_increase_password_hash_max_field_length.py new file mode 100644 index 00000000..137b5911 --- /dev/null +++ b/server/szurubooru/migrations/versions/9ef1a1643c2a_increase_password_hash_max_field_length.py @@ -0,0 +1,30 @@ +''' +Alter the password_hash field to work with larger output. Particularly libsodium output for greater password security. + +Revision ID: 9ef1a1643c2a +Created at: 2018-02-24 23:00:32.848575 +''' + +import sqlalchemy as sa +from alembic import op + + +revision = '9ef1a1643c2a' +down_revision = '02ef5f73f4ab' +branch_labels = None +depends_on = None + + +def upgrade(): + + op.alter_column('user', 'password_hash', + existing_type=sa.VARCHAR(length=64), + type_=sa.Unicode(length=128), + existing_nullable=False) + + +def downgrade(): + op.alter_column('user', 'password_hash', + existing_type=sa.Unicode(length=128), + type_=sa.VARCHAR(length=64), + existing_nullable=False) diff --git a/server/szurubooru/model/user.py b/server/szurubooru/model/user.py index dd7c0629..d355a143 100644 --- a/server/szurubooru/model/user.py +++ b/server/szurubooru/model/user.py @@ -23,7 +23,7 @@ class User(Base): last_login_time = sa.Column('last_login_time', sa.DateTime) version = sa.Column('version', sa.Integer, default=1, nullable=False) name = sa.Column('name', sa.Unicode(50), nullable=False, unique=True) - password_hash = sa.Column('password_hash', sa.Unicode(64), nullable=False) + password_hash = sa.Column('password_hash', sa.Unicode(128), nullable=False) password_salt = sa.Column('password_salt', sa.Unicode(32)) email = sa.Column('email', sa.Unicode(64), nullable=True) rank = sa.Column('rank', sa.Unicode(32), nullable=False) diff --git a/server/szurubooru/tests/conftest.py b/server/szurubooru/tests/conftest.py index e71f9609..aa26b731 100644 --- a/server/szurubooru/tests/conftest.py +++ b/server/szurubooru/tests/conftest.py @@ -115,11 +115,11 @@ def config_injector(): @pytest.fixture def user_factory(): - def factory(name=None, rank=model.User.RANK_REGULAR, email='dummy'): + def factory(name=None, rank=model.User.RANK_REGULAR, email='dummy', password_salt=None, password_hash=None): user = model.User() user.name = name or get_unique_name() - user.password_salt = 'dummy' - user.password_hash = 'dummy' + user.password_salt = password_salt or 'dummy' + user.password_hash = password_hash or 'dummy' user.email = email user.rank = rank user.creation_time = datetime(1997, 1, 1) diff --git a/server/szurubooru/tests/func/test_auth.py b/server/szurubooru/tests/func/test_auth.py new file mode 100644 index 00000000..9bc78d7a --- /dev/null +++ b/server/szurubooru/tests/func/test_auth.py @@ -0,0 +1,37 @@ +from szurubooru.func import auth +import pytest + + +@pytest.fixture(autouse=True) +def inject_config(config_injector): + config_injector({'secret': 'testSecret'}) + + +def test_get_password_hash(): + salt, password = ('testSalt', 'pass') + result = auth.get_password_hash(salt, password) + assert result + hash_parts = list(filter(lambda e: e is not None and e != '', result.split('$'))) + assert len(hash_parts) == 5 + assert hash_parts[0] == 'argon2id' + + +def test_get_sha256_legacy_password_hash(): + salt, password = ('testSalt', 'pass') + result = auth.get_sha256_legacy_password_hash(salt, password) + assert result == '2031ac9631353ac9303719a7f808a24f79aa1d71712c98523e4bb4cce579428a' + + +def test_get_sha1_legacy_password_hash(): + salt, password = ('testSalt', 'pass') + result = auth.get_sha1_legacy_password_hash(salt, password) + assert result == '1eb1f953d9be303a1b54627e903e6124cfb1245b' + + +def test_is_valid_password_auto_upgrades_user_password_hash_on_success_of_legacy_hash(user_factory): + salt, password = ('testSalt', 'pass') + legacy_password_hash = auth.get_sha256_legacy_password_hash(salt, password) + user = user_factory(password_salt=salt, password_hash=legacy_password_hash) + result = auth.is_valid_password(user, password) + assert result is True + assert user.password_hash != legacy_password_hash