diff --git a/server/szurubooru/func/auth.py b/server/szurubooru/func/auth.py index d2bb69ac..55b4db39 100644 --- a/server/szurubooru/func/auth.py +++ b/server/szurubooru/func/auth.py @@ -1,3 +1,5 @@ +from typing import Tuple + import hashlib import random from collections import OrderedDict @@ -5,7 +7,7 @@ from nacl.exceptions import InvalidkeyError from szurubooru import config, model, errors, db from szurubooru.func import util -from nacl.pwhash import argon2id, verify +from nacl import pwhash RANK_MAP = OrderedDict([ @@ -19,29 +21,29 @@ RANK_MAP = OrderedDict([ ]) -def get_password_hash(salt: str, password: str) -> str: - """ Retrieve argon2id password hash.""" - return argon2id.str( +def get_password_hash(salt: str, password: str) -> Tuple[str, int]: + ''' Retrieve argon2id password hash. ''' + return pwhash.argon2id.str( (config.config['secret'] + salt + password).encode('utf8') - ).decode('utf8') + ).decode('utf8'), 3 -def get_sha256_legacy_password_hash(salt: str, password: str) -> str: - """ Retrieve old-style sha256 password hash.""" +def get_sha256_legacy_password_hash(salt: str, password: str) -> Tuple[str, int]: + ''' Retrieve old-style sha256 password hash. ''' digest = hashlib.sha256() digest.update(config.config['secret'].encode('utf8')) digest.update(salt.encode('utf8')) digest.update(password.encode('utf8')) - return digest.hexdigest() + return digest.hexdigest(), 2 -def get_sha1_legacy_password_hash(salt: str, password: str) -> str: - """ Retrieve old-style sha1 password hash.""" +def get_sha1_legacy_password_hash(salt: str, password: str) -> Tuple[str, int]: + ''' Retrieve old-style sha1 password hash. ''' digest = hashlib.sha1() digest.update(b'1A2/$_4xVa') digest.update(salt.encode('utf8')) digest.update(password.encode('utf8')) - return digest.hexdigest() + return digest.hexdigest(), 1 def create_password() -> str: @@ -59,16 +61,19 @@ def is_valid_password(user: model.User, password: str) -> bool: salt, valid_hash = user.password_salt, user.password_hash try: - return verify(user.password_hash.encode('utf8'), - (config.config['secret'] + salt + password).encode('utf8')) + return pwhash.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) + get_sha256_legacy_password_hash(salt, password)[0], + get_sha1_legacy_password_hash(salt, password)[0] ] if valid_hash in possible_hashes: # Convert the user password hash to the new hash - user.password_hash = get_password_hash(salt, password) + new_hash, revision = get_password_hash(salt, password) + user.password_hash = new_hash + user.password_revision = revision db.session.commit() return True diff --git a/server/szurubooru/func/users.py b/server/szurubooru/func/users.py index ba6f67f2..2b744e28 100644 --- a/server/szurubooru/func/users.py +++ b/server/szurubooru/func/users.py @@ -243,7 +243,9 @@ def update_user_password(user: model.User, password: str) -> None: raise InvalidPasswordError( 'Password must satisfy regex %r.' % password_regex) user.password_salt = auth.create_password() - user.password_hash = auth.get_password_hash(user.password_salt, password) + hash, revision = auth.get_password_hash(user.password_salt, password) + user.password_hash = hash + user.password_revision = revision def update_user_email(user: model.User, email: str) -> None: @@ -308,5 +310,7 @@ def reset_user_password(user: model.User) -> str: assert user password = auth.create_password() user.password_salt = auth.create_password() - user.password_hash = auth.get_password_hash(user.password_salt, password) + hash, revision = auth.get_password_hash(user.password_salt, password) + user.password_hash = hash + user.password_revision = revision return password 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 deleted file mode 100644 index 137b5911..00000000 --- a/server/szurubooru/migrations/versions/9ef1a1643c2a_increase_password_hash_max_field_length.py +++ /dev/null @@ -1,30 +0,0 @@ -''' -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/migrations/versions/9ef1a1643c2a_update_user_table_for_hardened_passwords.py b/server/szurubooru/migrations/versions/9ef1a1643c2a_update_user_table_for_hardened_passwords.py new file mode 100644 index 00000000..4ae9bf2b --- /dev/null +++ b/server/szurubooru/migrations/versions/9ef1a1643c2a_update_user_table_for_hardened_passwords.py @@ -0,0 +1,34 @@ +''' +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) + op.add_column('user', sa.Column('password_revision', + sa.SmallInteger(), + nullable=False)) + + +def downgrade(): + op.alter_column('user', 'password_hash', + existing_type=sa.Unicode(length=128), + type_=sa.VARCHAR(length=64), + existing_nullable=False) + op.drop_column('user', 'password_revision') diff --git a/server/szurubooru/model/user.py b/server/szurubooru/model/user.py index d355a143..be95b0b1 100644 --- a/server/szurubooru/model/user.py +++ b/server/szurubooru/model/user.py @@ -25,6 +25,8 @@ class User(Base): name = sa.Column('name', sa.Unicode(50), nullable=False, unique=True) password_hash = sa.Column('password_hash', sa.Unicode(128), nullable=False) password_salt = sa.Column('password_salt', sa.Unicode(32)) + password_revision = sa.Column('password_revision', sa.SmallInteger, + default=0, nullable=False) email = sa.Column('email', sa.Unicode(64), nullable=True) rank = sa.Column('rank', sa.Unicode(32), nullable=False) avatar_style = sa.Column( diff --git a/server/szurubooru/tests/conftest.py b/server/szurubooru/tests/conftest.py index aa26b731..44ded329 100644 --- a/server/szurubooru/tests/conftest.py +++ b/server/szurubooru/tests/conftest.py @@ -115,7 +115,11 @@ def config_injector(): @pytest.fixture def user_factory(): - def factory(name=None, rank=model.User.RANK_REGULAR, email='dummy', password_salt=None, password_hash=None): + 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 = password_salt or 'dummy' diff --git a/server/szurubooru/tests/func/test_auth.py b/server/szurubooru/tests/func/test_auth.py index 9bc78d7a..5d0955ad 100644 --- a/server/szurubooru/tests/func/test_auth.py +++ b/server/szurubooru/tests/func/test_auth.py @@ -9,29 +9,35 @@ def inject_config(config_injector): def test_get_password_hash(): salt, password = ('testSalt', 'pass') - result = auth.get_password_hash(salt, password) + result, revision = auth.get_password_hash(salt, password) assert result - hash_parts = list(filter(lambda e: e is not None and e != '', result.split('$'))) + assert revision == 3 + 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' + result, revision = auth.get_sha256_legacy_password_hash(salt, password) + hash = '2031ac9631353ac9303719a7f808a24f79aa1d71712c98523e4bb4cce579428a' + assert result == hash + assert revision == 2 def test_get_sha1_legacy_password_hash(): salt, password = ('testSalt', 'pass') - result = auth.get_sha1_legacy_password_hash(salt, password) + result, revision = auth.get_sha1_legacy_password_hash(salt, password) assert result == '1eb1f953d9be303a1b54627e903e6124cfb1245b' + assert revision == 1 -def test_is_valid_password_auto_upgrades_user_password_hash_on_success_of_legacy_hash(user_factory): +def test_is_valid_password_auto_upgrades_user_password_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) + hash, revision = auth.get_sha256_legacy_password_hash(salt, password) + user = user_factory(password_salt=salt, password_hash=hash) result = auth.is_valid_password(user, password) assert result is True - assert user.password_hash != legacy_password_hash + assert user.password_hash != hash + assert user.password_revision > revision diff --git a/server/szurubooru/tests/func/test_users.py b/server/szurubooru/tests/func/test_users.py index 76c52c8b..55061276 100644 --- a/server/szurubooru/tests/func/test_users.py +++ b/server/szurubooru/tests/func/test_users.py @@ -320,10 +320,11 @@ def test_update_user_password(user_factory, config_injector): with patch('szurubooru.func.auth.create_password'), \ patch('szurubooru.func.auth.get_password_hash'): auth.create_password.return_value = 'salt' - auth.get_password_hash.return_value = 'hash' + auth.get_password_hash.return_value = ('hash', 3) users.update_user_password(user, 'a') assert user.password_salt == 'salt' assert user.password_hash == 'hash' + assert user.password_revision == 3 def test_update_user_email_with_too_long_string(user_factory): @@ -447,7 +448,8 @@ def test_reset_user_password(user_factory): patch('szurubooru.func.auth.get_password_hash'): user = user_factory() auth.create_password.return_value = 'salt' - auth.get_password_hash.return_value = 'hash' + auth.get_password_hash.return_value = ('hash', 3) users.reset_user_password(user) assert user.password_salt == 'salt' assert user.password_hash == 'hash' + assert user.password_revision == 3