From 22af5b8432b1f229d8a4a29e5ee46b1fe0c62bba Mon Sep 17 00:00:00 2001 From: Florent Daigniere Date: Sat, 20 Feb 2021 19:10:20 +0100 Subject: [PATCH 1/7] Switch to server-side sessions in redis --- core/admin/mailu/__init__.py | 4 ++++ core/admin/requirements-prod.txt | 1 + core/admin/requirements.txt | 1 + 3 files changed, 6 insertions(+) diff --git a/core/admin/mailu/__init__.py b/core/admin/mailu/__init__.py index 4de3e580..4d7efba1 100644 --- a/core/admin/mailu/__init__.py +++ b/core/admin/mailu/__init__.py @@ -1,5 +1,8 @@ import flask import flask_bootstrap +import redis +from flask_kvsession import KVSessionExtension +from simplekv.memory.redisstore import RedisStore from mailu import utils, debug, models, manage, configuration @@ -17,6 +20,7 @@ def create_app_from_config(config): # Initialize application extensions config.init_app(app) models.db.init_app(app) + KVSessionExtension(RedisStore(redis.StrictRedis().from_url('redis://{0}/3'.format(config['REDIS_ADDRESS']))), app) utils.limiter.init_app(app) utils.babel.init_app(app) utils.login.init_app(app) diff --git a/core/admin/requirements-prod.txt b/core/admin/requirements-prod.txt index f767f431..54cf9a14 100644 --- a/core/admin/requirements-prod.txt +++ b/core/admin/requirements-prod.txt @@ -13,6 +13,7 @@ Flask==1.0.2 Flask-Babel==0.12.2 Flask-Bootstrap==3.3.7.1 Flask-DebugToolbar==0.10.1 +Flask-KVSession==0.6.2 Flask-Limiter==1.0.1 Flask-Login==0.4.1 Flask-Migrate==2.4.0 diff --git a/core/admin/requirements.txt b/core/admin/requirements.txt index 9739ed3f..abb37234 100644 --- a/core/admin/requirements.txt +++ b/core/admin/requirements.txt @@ -3,6 +3,7 @@ Flask-Login Flask-SQLAlchemy Flask-bootstrap Flask-Babel +Flask-KVSession Flask-migrate Flask-script Flask-wtf From d459c374322f219ab12801d17817c12c628f1fdc Mon Sep 17 00:00:00 2001 From: Florent Daigniere Date: Mon, 22 Feb 2021 20:34:06 +0100 Subject: [PATCH 2/7] make session IDs 128bits --- core/admin/mailu/configuration.py | 1 + 1 file changed, 1 insertion(+) diff --git a/core/admin/mailu/configuration.py b/core/admin/mailu/configuration.py index 429e778c..a9ab937f 100644 --- a/core/admin/mailu/configuration.py +++ b/core/admin/mailu/configuration.py @@ -135,6 +135,7 @@ class ConfigManager(dict): self.config['QUOTA_STORAGE_URL'] = 'redis://{0}/1'.format(self.config['REDIS_ADDRESS']) self.config['SESSION_COOKIE_SAMESITE'] = 'Strict' self.config['SESSION_COOKIE_HTTPONLY'] = True + self.config['SESSION_KEY_BITS'] = 128 # update the app config itself app.config = self From a1d32568d6aceec8b0a2a0fd0514714585020edc Mon Sep 17 00:00:00 2001 From: Florent Daigniere Date: Mon, 22 Feb 2021 20:43:52 +0100 Subject: [PATCH 3/7] Regenerate session-ids to prevent session fixation --- core/admin/mailu/ui/views/base.py | 2 ++ core/admin/mailu/ui/views/users.py | 2 ++ 2 files changed, 4 insertions(+) diff --git a/core/admin/mailu/ui/views/base.py b/core/admin/mailu/ui/views/base.py index 7501a883..625c02e1 100644 --- a/core/admin/mailu/ui/views/base.py +++ b/core/admin/mailu/ui/views/base.py @@ -17,6 +17,7 @@ def login(): if form.validate_on_submit(): user = models.User.login(form.email.data, form.pw.data) if user: + flask.session.regenerate() flask_login.login_user(user) endpoint = flask.request.args.get('next', '.index') return flask.redirect(flask.url_for(endpoint) @@ -30,6 +31,7 @@ def login(): @access.authenticated def logout(): flask_login.logout_user() + flask.session.destroy() return flask.redirect(flask.url_for('.index')) diff --git a/core/admin/mailu/ui/views/users.py b/core/admin/mailu/ui/views/users.py index 8830ff5b..2784fe53 100644 --- a/core/admin/mailu/ui/views/users.py +++ b/core/admin/mailu/ui/views/users.py @@ -119,6 +119,7 @@ def user_password(user_email): if form.pw.data != form.pw2.data: flask.flash('Passwords do not match', 'error') else: + flask.session.regenerate() user.set_password(form.pw.data) models.db.session.commit() flask.flash('Password updated for %s' % user) @@ -186,6 +187,7 @@ def user_signup(domain_name=None): if domain.has_email(form.localpart.data): flask.flash('Email is already used', 'error') else: + flask.session.regenerate() user = models.User(domain=domain) form.populate_obj(user) user.set_password(form.pw.data) From b9becd86497fa685e80cca2ccbe20d54405e6d24 Mon Sep 17 00:00:00 2001 From: Florent Daigniere Date: Mon, 22 Feb 2021 21:15:25 +0100 Subject: [PATCH 4/7] make sessions expire --- core/admin/mailu/configuration.py | 3 +++ docs/configuration.rst | 2 ++ 2 files changed, 5 insertions(+) diff --git a/core/admin/mailu/configuration.py b/core/admin/mailu/configuration.py index a9ab937f..0f50bc95 100644 --- a/core/admin/mailu/configuration.py +++ b/core/admin/mailu/configuration.py @@ -1,5 +1,6 @@ import os +from datetime import timedelta from socrate import system DEFAULT_CONFIG = { @@ -53,6 +54,7 @@ DEFAULT_CONFIG = { 'RECAPTCHA_PRIVATE_KEY': '', # Advanced settings 'LOG_LEVEL': 'WARNING', + 'SESSION_LIFETIME': 24, 'SESSION_COOKIE_SECURE': True, 'CREDENTIAL_ROUNDS': 12, # Host settings @@ -136,6 +138,7 @@ class ConfigManager(dict): self.config['SESSION_COOKIE_SAMESITE'] = 'Strict' self.config['SESSION_COOKIE_HTTPONLY'] = True self.config['SESSION_KEY_BITS'] = 128 + self.config['PERMANENT_SESSION_LIFETIME'] = timedelta(hours=int(self.config['SESSION_LIFETIME'])) # update the app config itself app.config = self diff --git a/docs/configuration.rst b/docs/configuration.rst index 26bdb024..7cb53d13 100644 --- a/docs/configuration.rst +++ b/docs/configuration.rst @@ -142,6 +142,8 @@ The ``CREDENTIAL_ROUNDS`` (default: 12) setting is the number of rounds used by 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. +``SESSION_LIFETIME`` (default: 24) is the length in hours a session is valid for on the administrative interface. + The ``LOG_LEVEL`` setting is used by the python start-up scripts as a logging threshold. Log messages equal or higher than this priority will be printed. Can be one of: CRITICAL, ERROR, WARNING, INFO, DEBUG or NOTSET. From 481cb6739216d168e6c439852a7db2e441b13f68 Mon Sep 17 00:00:00 2001 From: Florent Daigniere Date: Mon, 22 Feb 2021 21:18:06 +0100 Subject: [PATCH 5/7] cleanup old sessions on startup --- core/admin/mailu/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/admin/mailu/__init__.py b/core/admin/mailu/__init__.py index 4d7efba1..f9ca2466 100644 --- a/core/admin/mailu/__init__.py +++ b/core/admin/mailu/__init__.py @@ -20,7 +20,7 @@ def create_app_from_config(config): # Initialize application extensions config.init_app(app) models.db.init_app(app) - KVSessionExtension(RedisStore(redis.StrictRedis().from_url('redis://{0}/3'.format(config['REDIS_ADDRESS']))), app) + KVSessionExtension(RedisStore(redis.StrictRedis().from_url('redis://{0}/3'.format(config['REDIS_ADDRESS']))), app).cleanup_sessions(app) utils.limiter.init_app(app) utils.babel.init_app(app) utils.login.init_app(app) From 64d757582d2f3531604503d3608dd2815a591c72 Mon Sep 17 00:00:00 2001 From: Florent Daigniere Date: Mon, 22 Feb 2021 21:59:15 +0100 Subject: [PATCH 6/7] Disable anti-csrf on the login form The rationale is that the attacker doesn't have the password... and that doing it this way we avoid creating useless sessions --- core/admin/mailu/ui/forms.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/core/admin/mailu/ui/forms.py b/core/admin/mailu/ui/forms.py index 356137e8..32bb31ab 100644 --- a/core/admin/mailu/ui/forms.py +++ b/core/admin/mailu/ui/forms.py @@ -46,6 +46,8 @@ class ConfirmationForm(flask_wtf.FlaskForm): class LoginForm(flask_wtf.FlaskForm): + class Meta: + csrf = False email = fields.StringField(_('E-mail'), [validators.Email()]) pw = fields.PasswordField(_('Password'), [validators.DataRequired()]) submit = fields.SubmitField(_('Sign in')) From b872b46097f507ae48fbb2a102207530678c36d7 Mon Sep 17 00:00:00 2001 From: Florent Daigniere Date: Tue, 9 Mar 2021 20:13:31 +0100 Subject: [PATCH 7/7] towncrier --- towncrier/newsfragments/1783.misc | 1 + 1 file changed, 1 insertion(+) create mode 100644 towncrier/newsfragments/1783.misc diff --git a/towncrier/newsfragments/1783.misc b/towncrier/newsfragments/1783.misc new file mode 100644 index 00000000..2ee4c97f --- /dev/null +++ b/towncrier/newsfragments/1783.misc @@ -0,0 +1 @@ +Switch from client side sessions (cookies) to server-side sessions (Redis). This simplies the security model a lot and allows for an easier recovery should a cookie ever land in the hands of an attacker.