From 901338e22813b8d47be5045efd40f8a842d7406e Mon Sep 17 00:00:00 2001 From: Fabio Manganiello Date: Sun, 5 May 2024 21:38:27 +0200 Subject: [PATCH] [#397] Replaced bcrypt dependency with native hashlib logic. Closes: #397 --- .../src/components/panels/Settings/Users.vue | 2 +- platypush/install/requirements/alpine.txt | 1 - platypush/install/requirements/arch.txt | 1 - platypush/install/requirements/debian.txt | 1 - platypush/install/requirements/fedora.txt | 1 - ...b_added_password_hashing_parameters_to_.py | 45 +++++++++++ platypush/user/__init__.py | 74 +++++++++++++++---- requirements.txt | 1 - setup.py | 1 - 9 files changed, 106 insertions(+), 21 deletions(-) create mode 100644 platypush/migrations/alembic/versions/0876439530cb_added_password_hashing_parameters_to_.py diff --git a/platypush/backend/http/webapp/src/components/panels/Settings/Users.vue b/platypush/backend/http/webapp/src/components/panels/Settings/Users.vue index 83bba0362..9edad7dc2 100644 --- a/platypush/backend/http/webapp/src/components/panels/Settings/Users.vue +++ b/platypush/backend/http/webapp/src/components/panels/Settings/Users.vue @@ -47,7 +47,7 @@ + @click="selectedUser = user.username; $refs.deleteUserDialog.show()" /> diff --git a/platypush/install/requirements/alpine.txt b/platypush/install/requirements/alpine.txt index 06c917aa0..148934668 100644 --- a/platypush/install/requirements/alpine.txt +++ b/platypush/install/requirements/alpine.txt @@ -1,7 +1,6 @@ python3 py3-pip py3-alembic -py3-bcrypt py3-dateutil py3-docutils py3-flask diff --git a/platypush/install/requirements/arch.txt b/platypush/install/requirements/arch.txt index 2b9623399..e4e576b53 100644 --- a/platypush/install/requirements/arch.txt +++ b/platypush/install/requirements/arch.txt @@ -1,6 +1,5 @@ python python-alembic -python-bcrypt python-croniter python-dateutil python-docutils diff --git a/platypush/install/requirements/debian.txt b/platypush/install/requirements/debian.txt index 0bda73935..f8678bc54 100644 --- a/platypush/install/requirements/debian.txt +++ b/platypush/install/requirements/debian.txt @@ -1,7 +1,6 @@ python3 python3-pip python3-alembic -python3-bcrypt python3-croniter python3-dateutil python3-docutils diff --git a/platypush/install/requirements/fedora.txt b/platypush/install/requirements/fedora.txt index 56b142597..a7148a565 100644 --- a/platypush/install/requirements/fedora.txt +++ b/platypush/install/requirements/fedora.txt @@ -1,7 +1,6 @@ python python-pip python-alembic -python-bcrypt python-croniter python-dateutil python-docutils diff --git a/platypush/migrations/alembic/versions/0876439530cb_added_password_hashing_parameters_to_.py b/platypush/migrations/alembic/versions/0876439530cb_added_password_hashing_parameters_to_.py new file mode 100644 index 000000000..58b168e39 --- /dev/null +++ b/platypush/migrations/alembic/versions/0876439530cb_added_password_hashing_parameters_to_.py @@ -0,0 +1,45 @@ +"""Added password hashing parameters to user table + +Revision ID: 0876439530cb +Revises: c39ac404119b +Create Date: 2024-05-05 20:57:02.820575 + +""" + +from alembic import op +import sqlalchemy as sa + + +# revision identifiers, used by Alembic. +revision = '0876439530cb' +down_revision = 'c39ac404119b' +branch_labels = None +depends_on = None + + +def upgrade() -> None: + conn = op.get_bind() + metadata = sa.MetaData() + metadata.reflect(bind=conn) + user_table = metadata.tables.get('user') + + if user_table is None: + print('The table `user` does not exist, skipping migration') + return + + op.add_column('user', sa.Column('password_salt', sa.String(), nullable=True)) + op.add_column('user', sa.Column('hmac_iterations', sa.Integer(), nullable=True)) + + +def downgrade() -> None: + conn = op.get_bind() + metadata = sa.MetaData() + metadata.reflect(bind=conn) + user_table = metadata.tables.get('user') + + if user_table is None: + print('The table `user` does not exist, skipping migration') + return + + op.drop_column('user', 'password_salt') + op.drop_column('user', 'hmac_iterations') diff --git a/platypush/user/__init__.py b/platypush/user/__init__.py index 432774a42..5691fa1c1 100644 --- a/platypush/user/__init__.py +++ b/platypush/user/__init__.py @@ -2,11 +2,11 @@ import base64 import datetime import hashlib import json +import os import random import time from typing import Optional, Dict -import bcrypt import rsa from sqlalchemy import Column, Integer, String, DateTime, ForeignKey @@ -58,7 +58,7 @@ class UserManager: with self._get_session() as session: return session.query(User).all() - def create_user(self, username, password, **kwargs): + def create_user(self, username: str, password: str, **kwargs): if not username: raise ValueError('Invalid or empty username') if not password: @@ -67,11 +67,17 @@ class UserManager: with self._get_session(locked=True) as session: user = self._get_user(session, username) if user: - raise NameError('The user {} already exists'.format(username)) + raise NameError(f'The user {username} already exists') + password_salt = os.urandom(16) + hmac_iterations = 100_000 record = User( username=username, - password=self._encrypt_password(password), + password=self._encrypt_password( + password, password_salt, hmac_iterations + ), + password_salt=password_salt.hex(), + hmac_iterations=hmac_iterations, created_at=datetime.datetime.utcnow(), **kwargs, ) @@ -88,7 +94,12 @@ class UserManager: return False user = self._get_user(session, username) - user.password = self._encrypt_password(new_password) + user.password_salt = user.password_salt or os.urandom(16).hex() + user.hmac_iterations = user.hmac_iterations or 100_000 + salt = bytes.fromhex(user.password_salt) + user.password = self._encrypt_password( + new_password, salt, user.hmac_iterations + ) session.commit() return True @@ -117,7 +128,7 @@ class UserManager: with self._get_session(locked=True) as session: user = self._get_user(session, username) if not user: - raise NameError('No such user: {}'.format(username)) + raise NameError(f'No such user: {username}') user_sessions = ( session.query(UserSession).filter_by(user_id=user.user_id).all() @@ -172,15 +183,43 @@ class UserManager: def _get_user(session, username): return session.query(User).filter_by(username=username).first() - @staticmethod - def _encrypt_password(pwd): - if isinstance(pwd, str): - pwd = pwd.encode() - return bcrypt.hashpw(pwd, bcrypt.gensalt(12)).decode() + @classmethod + def _encrypt_password( + cls, pwd: str, salt: Optional[bytes] = None, iterations: Optional[int] = None + ) -> str: + # Legacy password check that uses bcrypt if no salt and iterations are provided + # See https://git.platypush.tech/platypush/platypush/issues/397 + if not (salt and iterations): + import bcrypt + + return bcrypt.hashpw(pwd.encode(), bcrypt.gensalt(12)).decode() + + return hashlib.pbkdf2_hmac('sha256', pwd.encode(), salt, iterations).hex() @classmethod - def _check_password(cls, pwd, hashed_pwd): - return bcrypt.checkpw(cls._to_bytes(pwd), cls._to_bytes(hashed_pwd)) + def _check_password( + cls, + pwd: str, + hashed_pwd: str, + salt: Optional[bytes] = None, + iterations: Optional[int] = None, + ) -> bool: + # Legacy password check that uses bcrypt if no salt and iterations are provided + # See https://git.platypush.tech/platypush/platypush/issues/397 + if not (salt and iterations): + import bcrypt + + return bcrypt.checkpw(pwd.encode(), hashed_pwd.encode()) + + return ( + hashlib.pbkdf2_hmac( + 'sha256', + pwd.encode(), + salt, + iterations, + ).hex() + == hashed_pwd + ) @staticmethod def _to_bytes(data) -> bytes: @@ -289,7 +328,12 @@ class UserManager: if not user: return None - if not self._check_password(password, user.password): + if not self._check_password( + password, + user.password, + bytes.fromhex(user.password_salt) if user.password_salt else None, + user.hmac_iterations, + ): return None return user @@ -304,6 +348,8 @@ class User(Base): user_id = Column(Integer, primary_key=True) username = Column(String, unique=True, nullable=False) password = Column(String) + password_salt = Column(String) + hmac_iterations = Column(Integer) created_at = Column(DateTime) diff --git a/requirements.txt b/requirements.txt index c52ae690e..b189c0111 100644 --- a/requirements.txt +++ b/requirements.txt @@ -3,7 +3,6 @@ ### alembic -bcrypt croniter docutils flask diff --git a/setup.py b/setup.py index 5bea8b21a..630dc41a6 100755 --- a/setup.py +++ b/setup.py @@ -67,7 +67,6 @@ setup( ], install_requires=[ 'alembic', - 'bcrypt', 'croniter', 'docutils', 'flask',