From 7137ba6ff18c87185e25b9913377d8e7ce3fa8b6 Mon Sep 17 00:00:00 2001 From: Florent Daigniere Date: Tue, 2 Feb 2021 20:10:18 +0100 Subject: [PATCH] Misc improvements to PASSWORD_SCHEME - remove PASSWORD_SCHEME altogether - introduce CREDENTIAL_ROUNDS - migrate all old hashes to the current format - auto-detect/enable all hash types that passlib supports - upgrade passlib to 1.7.4 (see #1706: ldap_salted_sha512 support) --- core/admin/mailu/configuration.py | 2 +- core/admin/mailu/manage.py | 24 +++++++-------------- core/admin/mailu/models.py | 35 ++++++++++++++++++++----------- core/admin/requirements-prod.txt | 2 +- docs/cli.rst | 1 - docs/compose/.env | 5 ++--- docs/configuration.rst | 4 +--- 7 files changed, 35 insertions(+), 38 deletions(-) diff --git a/core/admin/mailu/configuration.py b/core/admin/mailu/configuration.py index 3d4d8668..cdee1084 100644 --- a/core/admin/mailu/configuration.py +++ b/core/admin/mailu/configuration.py @@ -33,6 +33,7 @@ DEFAULT_CONFIG = { 'TLS_FLAVOR': 'cert', 'AUTH_RATELIMIT': '10/minute;1000/hour', 'AUTH_RATELIMIT_SUBNET': True, + 'CREDENTIAL_ROUNDS': 12, 'DISABLE_STATISTICS': False, # Mail settings 'DMARC_RUA': None, @@ -52,7 +53,6 @@ DEFAULT_CONFIG = { 'RECAPTCHA_PUBLIC_KEY': '', 'RECAPTCHA_PRIVATE_KEY': '', # Advanced settings - 'PASSWORD_SCHEME': 'PBKDF2', 'LOG_LEVEL': 'WARNING', 'SESSION_COOKIE_SECURE': True, # Host settings diff --git a/core/admin/mailu/manage.py b/core/admin/mailu/manage.py index 62f214d3..9c576404 100644 --- a/core/admin/mailu/manage.py +++ b/core/admin/mailu/manage.py @@ -86,13 +86,10 @@ def admin(localpart, domain_name, password, mode='create'): @click.argument('localpart') @click.argument('domain_name') @click.argument('password') -@click.argument('hash_scheme', required=False) @flask_cli.with_appcontext -def user(localpart, domain_name, password, hash_scheme=None): +def user(localpart, domain_name, password): """ Create a user """ - if hash_scheme is None: - hash_scheme = app.config['PASSWORD_SCHEME'] domain = models.Domain.query.get(domain_name) if not domain: domain = models.Domain(name=domain_name) @@ -102,7 +99,7 @@ def user(localpart, domain_name, password, hash_scheme=None): domain=domain, global_admin=False ) - user.set_password(password, hash_scheme=hash_scheme) + user.set_password(password) db.session.add(user) db.session.commit() @@ -111,17 +108,14 @@ def user(localpart, domain_name, password, hash_scheme=None): @click.argument('localpart') @click.argument('domain_name') @click.argument('password') -@click.argument('hash_scheme', required=False) @flask_cli.with_appcontext -def password(localpart, domain_name, password, hash_scheme=None): +def password(localpart, domain_name, password): """ Change the password of an user """ email = '{0}@{1}'.format(localpart, domain_name) user = models.User.query.get(email) - if hash_scheme is None: - hash_scheme = app.config['PASSWORD_SCHEME'] if user: - user.set_password(password, hash_scheme=hash_scheme) + user.set_password(password) else: print("User " + email + " not found.") db.session.commit() @@ -148,13 +142,10 @@ def domain(domain_name, max_users=-1, max_aliases=-1, max_quota_bytes=0): @click.argument('localpart') @click.argument('domain_name') @click.argument('password_hash') -@click.argument('hash_scheme') @flask_cli.with_appcontext -def user_import(localpart, domain_name, password_hash, hash_scheme = None): +def user_import(localpart, domain_name, password_hash): """ Import a user along with password hash. """ - if hash_scheme is None: - hash_scheme = app.config['PASSWORD_SCHEME'] domain = models.Domain.query.get(domain_name) if not domain: domain = models.Domain(name=domain_name) @@ -164,7 +155,7 @@ def user_import(localpart, domain_name, password_hash, hash_scheme = None): domain=domain, global_admin=False ) - user.set_password(password_hash, hash_scheme=hash_scheme, raw=True) + user.set_password(password_hash, raw=True) db.session.add(user) db.session.commit() @@ -217,7 +208,6 @@ def config_update(verbose=False, delete_objects=False): localpart = user_config['localpart'] domain_name = user_config['domain'] password_hash = user_config.get('password_hash', None) - hash_scheme = user_config.get('hash_scheme', None) domain = models.Domain.query.get(domain_name) email = '{0}@{1}'.format(localpart, domain_name) optional_params = {} @@ -239,7 +229,7 @@ def config_update(verbose=False, delete_objects=False): else: for k in optional_params: setattr(user, k, optional_params[k]) - user.set_password(password_hash, hash_scheme=hash_scheme, raw=True) + user.set_password(password_hash, raw=True) db.session.add(user) aliases = new_config.get('aliases', []) diff --git a/core/admin/mailu/models.py b/core/admin/mailu/models.py index 164312ad..905af4a2 100644 --- a/core/admin/mailu/models.py +++ b/core/admin/mailu/models.py @@ -1,7 +1,7 @@ from mailu import dkim from sqlalchemy.ext import declarative -from passlib import context, hash +from passlib import context, hash, registry from datetime import datetime, date from email.mime import text from flask import current_app as app @@ -370,17 +370,30 @@ class User(Base, Email): 'CRYPT': "des_crypt"} def get_password_context(self): + schemes = registry.list_crypt_handlers() + # scrypt throws a warning if the native wheels aren't found + schemes.remove('scrypt') + # we can't leave plaintext schemes as they will be misidentified + for scheme in schemes: + if scheme.endswith('plaintext'): + schemes.remove(scheme) return context.CryptContext( - schemes=self.scheme_dict.values(), - default=self.scheme_dict[app.config['PASSWORD_SCHEME']], + schemes=schemes, + default='bcrypt_sha256', + bcrypt_sha256__rounds=app.config['CREDENTIAL_ROUNDS'], + deprecated='auto' ) def check_password(self, password): context = self.get_password_context() - reference = re.match('({[^}]+})?(.*)', self.password).group(2) - result = context.verify(password, reference) - if result and context.identify(reference) != context.default_scheme(): - self.set_password(password) + # {scheme} will most likely be migrated on first use + reference = self.password + if self.password.startswith("{"): + reference = re.match('({[^}]+})?(.*)', reference).group(2) + + result, new_hash = context.verify_and_update(password, reference) + if new_hash: + self.password = new_hash db.session.add(self) db.session.commit() return result @@ -389,13 +402,11 @@ class User(Base, Email): """Set password for user with specified encryption scheme @password: plain text password to encrypt (if raw == True the hash itself) """ - if hash_scheme is None: - hash_scheme = app.config['PASSWORD_SCHEME'] - # for the list of hash schemes see https://wiki2.dovecot.org/Authentication/PasswordSchemes if raw: - self.password = '{'+hash_scheme+'}' + password + self.password = password else: - self.password = '{'+hash_scheme+'}' + self.get_password_context().encrypt(password, self.scheme_dict[hash_scheme]) + self.password = self.get_password_context().hash(password) + app.cache.delete(self.get_id()) def get_managed_domains(self): if self.global_admin: diff --git a/core/admin/requirements-prod.txt b/core/admin/requirements-prod.txt index a3c32855..f767f431 100644 --- a/core/admin/requirements-prod.txt +++ b/core/admin/requirements-prod.txt @@ -29,7 +29,7 @@ limits==1.3 Mako==1.0.9 MarkupSafe==1.1.1 mysqlclient==1.4.2.post1 -passlib==1.7.1 +passlib==1.7.4 psycopg2==2.8.2 pycparser==2.19 pyOpenSSL==19.0.0 diff --git a/docs/cli.rst b/docs/cli.rst index a9cff41c..8e94026b 100644 --- a/docs/cli.rst +++ b/docs/cli.rst @@ -85,7 +85,6 @@ where mail-config.yml looks like: - localpart: foo domain: example.com password_hash: klkjhumnzxcjkajahsdqweqqwr - hash_scheme: MD5-CRYPT aliases: - localpart: alias1 diff --git a/docs/compose/.env b/docs/compose/.env index 7f91c270..432b20b0 100644 --- a/docs/compose/.env +++ b/docs/compose/.env @@ -144,9 +144,8 @@ LOG_DRIVER=json-file # Docker-compose project name, this will prepended to containers names. COMPOSE_PROJECT_NAME=mailu -# Default password scheme used for newly created accounts and changed passwords -# (value: PBKDF2, BLF-CRYPT, SHA512-CRYPT, SHA256-CRYPT) -PASSWORD_SCHEME=PBKDF2 +# Number of rounds used by the password hashing scheme +CREDENTIAL_ROUNDS=12 # Header to take the real ip from REAL_IP_HEADER= diff --git a/docs/configuration.rst b/docs/configuration.rst index 9123054c..bc2027c6 100644 --- a/docs/configuration.rst +++ b/docs/configuration.rst @@ -138,9 +138,7 @@ Depending on your particular deployment you most probably will want to change th Advanced settings ----------------- -The ``PASSWORD_SCHEME`` is the password encryption scheme. You should use the -default value, unless you are importing password from a separate system and -want to keep using the old password encryption scheme. +The ``CREDENTIAL_ROUNDS`` (default: 12) is the number of rounds used by the password hashing scheme. You should use the default value. The ``SESSION_COOKIE_SECURE`` (default: True) setting controls the secure flag on the cookies of the administrative interface. It should only be turned off if you intend to access it over plain HTTP.