From 505bb79a78203da87cde5f9f3b31c579f3bcfac1 Mon Sep 17 00:00:00 2001 From: Florent Daigniere Date: Mon, 14 Nov 2022 15:03:57 +0100 Subject: [PATCH 1/6] Don't set the secure Cookie flag if TLS_FLAVOR=notls --- core/admin/mailu/sso/views/base.py | 2 +- core/admin/mailu/utils.py | 2 +- core/admin/run_dev.sh | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/core/admin/mailu/sso/views/base.py b/core/admin/mailu/sso/views/base.py index 6fa9403f..a4218ac6 100644 --- a/core/admin/mailu/sso/views/base.py +++ b/core/admin/mailu/sso/views/base.py @@ -39,7 +39,7 @@ def login(): flask.session.regenerate() flask_login.login_user(user) response = flask.redirect(destination) - response.set_cookie('rate_limit', utils.limiter.device_cookie(username), max_age=31536000, path=flask.url_for('sso.login'), secure=app.config['SESSION_COOKIE_SECURE'], httponly=True) + response.set_cookie('rate_limit', utils.limiter.device_cookie(username), max_age=31536000, path=flask.url_for('sso.login'), secure=False if app.config['TLS_FLAVOR'] == 'notls' else app.config['SESSION_COOKIE_SECURE'], httponly=True) flask.current_app.logger.info(f'Login succeeded for {username} from {client_ip} pwned={form.pwned.data}.') if msg := utils.isBadOrPwned(form): flask.flash(msg, "error") diff --git a/core/admin/mailu/utils.py b/core/admin/mailu/utils.py index f160fe3f..a330a7d6 100644 --- a/core/admin/mailu/utils.py +++ b/core/admin/mailu/utils.py @@ -408,7 +408,7 @@ class MailuSessionInterface(SessionInterface): httponly=self.get_cookie_httponly(app), domain=self.get_cookie_domain(app), path=self.get_cookie_path(app), - secure=self.get_cookie_secure(app), + secure=False if app.config['TLS_FLAVOR'] == 'notls' else self.get_cookie_secure(app), samesite=self.get_cookie_samesite(app) ) diff --git a/core/admin/run_dev.sh b/core/admin/run_dev.sh index 4ab76e74..65841f2c 100755 --- a/core/admin/run_dev.sh +++ b/core/admin/run_dev.sh @@ -68,12 +68,12 @@ ENV \ FLASK_ENV="development" \ MEMORY_SESSIONS="true" \ RATELIMIT_STORAGE_URL="memory://" \ - SESSION_COOKIE_SECURE="false" \ \ DEBUG="true" \ DEBUG_PROFILER="${DEV_PROFILER}" \ DEBUG_ASSETS="/app/static" \ DEBUG_TB_ENABLED="true" \ + DEBUG_TB_INTERCEPT_REDIRECTS=False \ \ IMAP_ADDRESS="127.0.0.1" \ POP3_ADDRESS="127.0.0.1" \ From a566cb07d6181a846962c74aa7e35884fa76df7c Mon Sep 17 00:00:00 2001 From: Florent Daigniere Date: Mon, 14 Nov 2022 16:51:05 +0100 Subject: [PATCH 2/6] fix --- core/admin/run_dev.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/admin/run_dev.sh b/core/admin/run_dev.sh index 7c6e8467..05da792e 100755 --- a/core/admin/run_dev.sh +++ b/core/admin/run_dev.sh @@ -82,7 +82,7 @@ ENV \ REDIS_ADDRESS="127.0.0.1" \ WEBMAIL_ADDRESS="127.0.0.1" -CMD ["/bin/bash", "-c", "flask db upgrade &>/dev/null && flask mailu admin '${DEV_ADMIN/@*}' '${DEV_ADMIN#*@}' '${DEV_PASSWORD}' --mode ifmissing >/dev/null; flask --debug run --host=0.0.0.0 --port=8080"] +CMD ["/bin/bash", "-c", "flask db upgrade &>/dev/null && flask mailu admin '${DEV_ADMIN/@*}' '${DEV_ADMIN#*@}' '${DEV_PASSWORD}' --mode ifmissing >/dev/null; flask --debugger run --host=0.0.0.0 --port=8080"] EOF # build From 7aad1158fb10df2f61596e56bfe336a255ac9952 Mon Sep 17 00:00:00 2001 From: Florent Daigniere Date: Mon, 14 Nov 2022 17:31:31 +0100 Subject: [PATCH 3/6] @ghostwheel42 will fix it in another PR --- core/admin/run_dev.sh | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/core/admin/run_dev.sh b/core/admin/run_dev.sh index 05da792e..cbe625f6 100755 --- a/core/admin/run_dev.sh +++ b/core/admin/run_dev.sh @@ -73,7 +73,6 @@ ENV \ DEBUG_PROFILER="${DEV_PROFILER}" \ DEBUG_ASSETS="/app/static" \ DEBUG_TB_ENABLED="true" \ - DEBUG_TB_INTERCEPT_REDIRECTS=False \ \ IMAP_ADDRESS="127.0.0.1" \ POP3_ADDRESS="127.0.0.1" \ @@ -82,7 +81,7 @@ ENV \ REDIS_ADDRESS="127.0.0.1" \ WEBMAIL_ADDRESS="127.0.0.1" -CMD ["/bin/bash", "-c", "flask db upgrade &>/dev/null && flask mailu admin '${DEV_ADMIN/@*}' '${DEV_ADMIN#*@}' '${DEV_PASSWORD}' --mode ifmissing >/dev/null; flask --debugger run --host=0.0.0.0 --port=8080"] +CMD ["/bin/bash", "-c", "flask db upgrade &>/dev/null && flask mailu admin '${DEV_ADMIN/@*}' '${DEV_ADMIN#*@}' '${DEV_PASSWORD}' --mode ifmissing >/dev/null; flask --debug run --host=0.0.0.0 --port=8080"] EOF # build From 76f8517e00a33a3e51126a7a54cdaa7ce3b9dd26 Mon Sep 17 00:00:00 2001 From: Florent Daigniere Date: Mon, 14 Nov 2022 19:38:17 +0100 Subject: [PATCH 4/6] This is still required (as TLS_FLAVOR isn't set) --- core/admin/run_dev.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/core/admin/run_dev.sh b/core/admin/run_dev.sh index cbe625f6..dbe3368a 100755 --- a/core/admin/run_dev.sh +++ b/core/admin/run_dev.sh @@ -68,6 +68,7 @@ ENV \ FLASK_DEBUG="true" \ MEMORY_SESSIONS="true" \ RATELIMIT_STORAGE_URL="memory://" \ + SESSION_COOKIE_SECURE="false" \ \ DEBUG="true" \ DEBUG_PROFILER="${DEV_PROFILER}" \ From 66de1dcec88f6a961a4a5d792045be8bc8774347 Mon Sep 17 00:00:00 2001 From: Florent Daigniere Date: Tue, 15 Nov 2022 10:47:20 +0100 Subject: [PATCH 5/6] Change the logic The idea here is that if you have set SESSION_COOKIE_SECURE we should honor that... and if you haven't we should try to do the right thing. --- core/admin/mailu/configuration.py | 4 +++- core/admin/mailu/sso/views/base.py | 2 +- core/admin/mailu/utils.py | 2 +- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/core/admin/mailu/configuration.py b/core/admin/mailu/configuration.py index b941e95c..eb27aee0 100644 --- a/core/admin/mailu/configuration.py +++ b/core/admin/mailu/configuration.py @@ -73,7 +73,7 @@ DEFAULT_CONFIG = { 'SESSION_KEY_BITS': 128, 'SESSION_TIMEOUT': 3600, 'PERMANENT_SESSION_LIFETIME': 30*24*3600, - 'SESSION_COOKIE_SECURE': True, + 'SESSION_COOKIE_SECURE': False, 'CREDENTIAL_ROUNDS': 12, 'TLS_PERMISSIVE': True, 'TZ': 'Etc/UTC', @@ -156,6 +156,8 @@ class ConfigManager: self.config['SESSION_STORAGE_URL'] = f'redis://{self.config["REDIS_ADDRESS"]}/3' self.config['SESSION_COOKIE_SAMESITE'] = 'Strict' self.config['SESSION_COOKIE_HTTPONLY'] = True + if self.config['TLS_FLAVOR'] != 'notls' and not self.config['SESSION_COOKIE_SECURE']: + self.config['SESSION_COOKIE_SECURE'] = True self.config['SESSION_PERMANENT'] = True self.config['SESSION_TIMEOUT'] = int(self.config['SESSION_TIMEOUT']) self.config['PERMANENT_SESSION_LIFETIME'] = int(self.config['PERMANENT_SESSION_LIFETIME']) diff --git a/core/admin/mailu/sso/views/base.py b/core/admin/mailu/sso/views/base.py index a4218ac6..6fa9403f 100644 --- a/core/admin/mailu/sso/views/base.py +++ b/core/admin/mailu/sso/views/base.py @@ -39,7 +39,7 @@ def login(): flask.session.regenerate() flask_login.login_user(user) response = flask.redirect(destination) - response.set_cookie('rate_limit', utils.limiter.device_cookie(username), max_age=31536000, path=flask.url_for('sso.login'), secure=False if app.config['TLS_FLAVOR'] == 'notls' else app.config['SESSION_COOKIE_SECURE'], httponly=True) + response.set_cookie('rate_limit', utils.limiter.device_cookie(username), max_age=31536000, path=flask.url_for('sso.login'), secure=app.config['SESSION_COOKIE_SECURE'], httponly=True) flask.current_app.logger.info(f'Login succeeded for {username} from {client_ip} pwned={form.pwned.data}.') if msg := utils.isBadOrPwned(form): flask.flash(msg, "error") diff --git a/core/admin/mailu/utils.py b/core/admin/mailu/utils.py index a330a7d6..f160fe3f 100644 --- a/core/admin/mailu/utils.py +++ b/core/admin/mailu/utils.py @@ -408,7 +408,7 @@ class MailuSessionInterface(SessionInterface): httponly=self.get_cookie_httponly(app), domain=self.get_cookie_domain(app), path=self.get_cookie_path(app), - secure=False if app.config['TLS_FLAVOR'] == 'notls' else self.get_cookie_secure(app), + secure=self.get_cookie_secure(app), samesite=self.get_cookie_samesite(app) ) From b9e5560fb6942bde84419024cf53d04263dd8a03 Mon Sep 17 00:00:00 2001 From: Florent Daigniere Date: Tue, 15 Nov 2022 12:47:38 +0100 Subject: [PATCH 6/6] Better way to express the same thing Thanks @ghostwheel42 --- core/admin/mailu/configuration.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/core/admin/mailu/configuration.py b/core/admin/mailu/configuration.py index eb27aee0..fa638520 100644 --- a/core/admin/mailu/configuration.py +++ b/core/admin/mailu/configuration.py @@ -73,7 +73,7 @@ DEFAULT_CONFIG = { 'SESSION_KEY_BITS': 128, 'SESSION_TIMEOUT': 3600, 'PERMANENT_SESSION_LIFETIME': 30*24*3600, - 'SESSION_COOKIE_SECURE': False, + 'SESSION_COOKIE_SECURE': None, 'CREDENTIAL_ROUNDS': 12, 'TLS_PERMISSIVE': True, 'TZ': 'Etc/UTC', @@ -156,8 +156,8 @@ class ConfigManager: self.config['SESSION_STORAGE_URL'] = f'redis://{self.config["REDIS_ADDRESS"]}/3' self.config['SESSION_COOKIE_SAMESITE'] = 'Strict' self.config['SESSION_COOKIE_HTTPONLY'] = True - if self.config['TLS_FLAVOR'] != 'notls' and not self.config['SESSION_COOKIE_SECURE']: - self.config['SESSION_COOKIE_SECURE'] = True + if self.config['SESSION_COOKIE_SECURE'] is None: + self.config['SESSION_COOKIE_SECURE'] = self.config['TLS_FLAVOR'] != 'notls' self.config['SESSION_PERMANENT'] = True self.config['SESSION_TIMEOUT'] = int(self.config['SESSION_TIMEOUT']) self.config['PERMANENT_SESSION_LIFETIME'] = int(self.config['PERMANENT_SESSION_LIFETIME'])