From 3d4a9ac29cf10f545bd68772156265ad6bc94d2c Mon Sep 17 00:00:00 2001 From: Florent Daigniere Date: Thu, 8 Aug 2024 09:35:27 +0200 Subject: [PATCH 1/9] Fix #3364 (cherry picked from commit ee243ea735744b296bb90b2c1e6a1fded8915c8d) --- core/nginx/dovecot/login.lua | 13 +++++++++++-- towncrier/newsfragments/3364.bugfix | 1 + 2 files changed, 12 insertions(+), 2 deletions(-) create mode 100644 towncrier/newsfragments/3364.bugfix diff --git a/core/nginx/dovecot/login.lua b/core/nginx/dovecot/login.lua index d24de149..a93b4b29 100644 --- a/core/nginx/dovecot/login.lua +++ b/core/nginx/dovecot/login.lua @@ -10,15 +10,24 @@ local http_client = dovecot.http.client { max_attempts = 3; } +-- on the other end we use urllib.parse.unquote() +function urlEncode(str) + return str:gsub("[^%w_.-~]", function(c) + return string.format("%%%02X", string.byte(c)) + end) +end + function auth_passdb_lookup(req) local auth_request = http_client:request { url = "http://{{ ADMIN_ADDRESS }}:8080/internal/auth/email"; } auth_request:add_header('Auth-Port', req.local_port) - auth_request:add_header('Auth-User', req.user) + local user = urlEncode(req.user) + auth_request:add_header('Auth-User', user) if req.password ~= nil then - auth_request:add_header('Auth-Pass', req.password) + local password = urlEncode(req.password) + auth_request:add_header('Auth-Pass', password) end auth_request:add_header('Auth-Protocol', req.service) auth_request:add_header('Client-IP', req.remote_ip) diff --git a/towncrier/newsfragments/3364.bugfix b/towncrier/newsfragments/3364.bugfix new file mode 100644 index 00000000..10e56cf3 --- /dev/null +++ b/towncrier/newsfragments/3364.bugfix @@ -0,0 +1 @@ +Fix a bug preventing percent characters from being used in passwords From 77c7d2f6919423cd6de62f64e3482b867d2701c7 Mon Sep 17 00:00:00 2001 From: Florent Daigniere Date: Thu, 8 Aug 2024 09:49:21 +0200 Subject: [PATCH 2/9] Ensure we test this (cherry picked from commit 1cbd31c7bbccede04970901edf27e3160844aa09) --- tests/compose/core/00_create_users.sh | 2 +- tests/compose/core/05_connectivity.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/compose/core/00_create_users.sh b/tests/compose/core/00_create_users.sh index 142c3cb3..3832c03b 100755 --- a/tests/compose/core/00_create_users.sh +++ b/tests/compose/core/00_create_users.sh @@ -8,5 +8,5 @@ docker compose -f tests/compose/core/docker-compose.yml exec -T admin flask mail docker compose -f tests/compose/core/docker-compose.yml exec -T admin flask mailu admin admin mailu.io 'password' --mode=update || exit 1 docker compose -f tests/compose/core/docker-compose.yml exec -T admin flask mailu user user mailu.io 'password' || exit 1 docker compose -f tests/compose/core/docker-compose.yml exec -T admin flask mailu user 'user/with/slash' mailu.io 'password' || exit 1 -docker compose -f tests/compose/core/docker-compose.yml exec -T admin flask mailu user 'user_UTF8' mailu.io 'password€' || exit 1 +docker compose -f tests/compose/core/docker-compose.yml exec -T admin flask mailu user 'user_UTF8' mailu.io 'pass%e9word€' || exit 1 echo "User testing successful!" diff --git a/tests/compose/core/05_connectivity.py b/tests/compose/core/05_connectivity.py index 5cc12069..6ae62487 100755 --- a/tests/compose/core/05_connectivity.py +++ b/tests/compose/core/05_connectivity.py @@ -8,7 +8,7 @@ import managesieve SERVER='localhost' USERNAME='user_UTF8@mailu.io' -PASSWORD='password€' +PASSWORD='pass%e9word€' #https://github.com/python/cpython/issues/73936 #SMTPlib does not support UTF8 passwords. USERNAME_ASCII='user@mailu.io' @@ -139,4 +139,4 @@ if __name__ == '__main__': test_SMTP(SERVER, USERNAME_ASCII, PASSWORD_ASCII) test_managesieve(SERVER, USERNAME, PASSWORD) #https://github.com/python/cpython/issues/73936 -#SMTPlib does not support UTF8 passwords. \ No newline at end of file +#SMTPlib does not support UTF8 passwords. From 00ef3cb950606dbf8dce8b48044028d3f1691fad Mon Sep 17 00:00:00 2001 From: Florent Daigniere Date: Thu, 8 Aug 2024 10:24:43 +0200 Subject: [PATCH 3/9] Remove this insanity since we don't use nginx (cherry picked from commit 148c8f9ede32e649129eaec3eba0e3886db1d0a5) --- core/admin/mailu/internal/nginx.py | 12 +++--------- core/admin/mailu/internal/views/auth.py | 11 ++--------- 2 files changed, 5 insertions(+), 18 deletions(-) diff --git a/core/admin/mailu/internal/nginx.py b/core/admin/mailu/internal/nginx.py index ebd677d0..d236513d 100644 --- a/core/admin/mailu/internal/nginx.py +++ b/core/admin/mailu/internal/nginx.py @@ -91,20 +91,14 @@ def handle_authentication(headers): # Authenticated user elif method in ['plain', 'login']: is_valid_user = False - # According to RFC2616 section 3.7.1 and PEP 3333, HTTP headers should - # be ASCII and are generally considered ISO8859-1. However when passing - # the password, nginx does not transcode the input UTF string, thus - # we need to manually decode. - raw_user_email = urllib.parse.unquote(headers["Auth-User"]) - raw_password = urllib.parse.unquote(headers["Auth-Pass"]) user_email = 'invalid' password = 'invalid' try: - user_email = raw_user_email.encode("iso8859-1").decode("utf8") - password = raw_password.encode("iso8859-1").decode("utf8") + user_email = urllib.parse.unquote(headers["Auth-User"]) + password = urllib.parse.unquote(headers["Auth-Pass"]) ip = urllib.parse.unquote(headers["Client-Ip"]) except: - app.logger.warn(f'Received undecodable user/password from nginx: {raw_user_email!r}/{raw_password!r}') + app.logger.warn(f'Received undecodable user/password from nginx: {headers["Auth-User"]!r}/{headers["Auth-Pass"]!r}') else: try: user = models.User.query.get(user_email) if '@' in user_email else None diff --git a/core/admin/mailu/internal/views/auth.py b/core/admin/mailu/internal/views/auth.py index 4aa31407..3182188c 100644 --- a/core/admin/mailu/internal/views/auth.py +++ b/core/admin/mailu/internal/views/auth.py @@ -29,7 +29,6 @@ def nginx_authentication(): response.headers['Auth-Status'] = status response.headers['Auth-Error-Code'] = code return response - raw_password = urllib.parse.unquote(headers['Auth-Pass']) if 'Auth-Pass' in headers else '' headers = nginx.handle_authentication(flask.request.headers) response = flask.Response() for key, value in headers.items(): @@ -50,14 +49,8 @@ def nginx_authentication(): if not is_port_25: utils.limiter.exempt_ip_from_ratelimits(client_ip) elif is_valid_user: - password = None - try: - password = raw_password.encode("iso8859-1").decode("utf8") - except: - app.logger.warn(f'Received undecodable password for {username} from nginx: {raw_password!r}') - utils.limiter.rate_limit_user(username, client_ip, password=None) - else: - utils.limiter.rate_limit_user(username, client_ip, password=password) + password = urllib.parse.unquote(headers['Auth-Pass']) if 'Auth-Pass' in headers else '' + utils.limiter.rate_limit_user(username, client_ip, password=password) elif not is_from_webmail: utils.limiter.rate_limit_ip(client_ip, username) return response From a5af42a6efb17cd5729413a4514d68ef0f181c5b Mon Sep 17 00:00:00 2001 From: Florent Daigniere Date: Thu, 8 Aug 2024 10:27:35 +0200 Subject: [PATCH 4/9] Better (cherry picked from commit 38ea029bd9b118e58627055a900ae8357e9e48e5) --- core/admin/mailu/internal/views/auth.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/admin/mailu/internal/views/auth.py b/core/admin/mailu/internal/views/auth.py index 3182188c..fed10953 100644 --- a/core/admin/mailu/internal/views/auth.py +++ b/core/admin/mailu/internal/views/auth.py @@ -49,7 +49,7 @@ def nginx_authentication(): if not is_port_25: utils.limiter.exempt_ip_from_ratelimits(client_ip) elif is_valid_user: - password = urllib.parse.unquote(headers['Auth-Pass']) if 'Auth-Pass' in headers else '' + password = urllib.parse.unquote(headers.get('Auth-Pass'], None)) utils.limiter.rate_limit_user(username, client_ip, password=password) elif not is_from_webmail: utils.limiter.rate_limit_ip(client_ip, username) From 1366ee3fc7afc3571f21c56cb34964e7f9bbbcfd Mon Sep 17 00:00:00 2001 From: Florent Daigniere Date: Thu, 8 Aug 2024 10:33:19 +0200 Subject: [PATCH 5/9] doh (cherry picked from commit d7c6528f045e50b939675fcb6730ab4813aeb6d4) --- core/admin/mailu/internal/views/auth.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/admin/mailu/internal/views/auth.py b/core/admin/mailu/internal/views/auth.py index fed10953..d163cb80 100644 --- a/core/admin/mailu/internal/views/auth.py +++ b/core/admin/mailu/internal/views/auth.py @@ -49,7 +49,7 @@ def nginx_authentication(): if not is_port_25: utils.limiter.exempt_ip_from_ratelimits(client_ip) elif is_valid_user: - password = urllib.parse.unquote(headers.get('Auth-Pass'], None)) + password = urllib.parse.unquote(headers.get('Auth-Pass', None)) utils.limiter.rate_limit_user(username, client_ip, password=password) elif not is_from_webmail: utils.limiter.rate_limit_ip(client_ip, username) From e8ded2f203ed7b7b3bb449d532c7101c65c4be1b Mon Sep 17 00:00:00 2001 From: Florent Daigniere Date: Thu, 8 Aug 2024 10:50:32 +0200 Subject: [PATCH 6/9] More tests See what happens with a character that will url decode to longer (cherry picked from commit 2526d43a98c899f9742228e31ffb0e2c1a1c115d) --- tests/compose/core/00_create_users.sh | 2 +- tests/compose/core/05_connectivity.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/compose/core/00_create_users.sh b/tests/compose/core/00_create_users.sh index 3832c03b..0bed6baf 100755 --- a/tests/compose/core/00_create_users.sh +++ b/tests/compose/core/00_create_users.sh @@ -8,5 +8,5 @@ docker compose -f tests/compose/core/docker-compose.yml exec -T admin flask mail docker compose -f tests/compose/core/docker-compose.yml exec -T admin flask mailu admin admin mailu.io 'password' --mode=update || exit 1 docker compose -f tests/compose/core/docker-compose.yml exec -T admin flask mailu user user mailu.io 'password' || exit 1 docker compose -f tests/compose/core/docker-compose.yml exec -T admin flask mailu user 'user/with/slash' mailu.io 'password' || exit 1 -docker compose -f tests/compose/core/docker-compose.yml exec -T admin flask mailu user 'user_UTF8' mailu.io 'pass%e9word€' || exit 1 +docker compose -f tests/compose/core/docker-compose.yml exec -T admin flask mailu user 'user_UTF8' mailu.io 'pa…ss%e9word€' || exit 1 echo "User testing successful!" diff --git a/tests/compose/core/05_connectivity.py b/tests/compose/core/05_connectivity.py index 6ae62487..44bb4553 100755 --- a/tests/compose/core/05_connectivity.py +++ b/tests/compose/core/05_connectivity.py @@ -8,7 +8,7 @@ import managesieve SERVER='localhost' USERNAME='user_UTF8@mailu.io' -PASSWORD='pass%e9word€' +PASSWORD='pa…ss%e9word€' #https://github.com/python/cpython/issues/73936 #SMTPlib does not support UTF8 passwords. USERNAME_ASCII='user@mailu.io' From 14196e5054699a150d64274da2e5aa2fc8b6e5d0 Mon Sep 17 00:00:00 2001 From: Florent Daigniere Date: Thu, 8 Aug 2024 11:18:50 +0200 Subject: [PATCH 7/9] Do the same with Client-Ip (cherry picked from commit 98f671dc2e4be4fbc2ed64a5e36263caa217183c) --- core/nginx/dovecot/login.lua | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/core/nginx/dovecot/login.lua b/core/nginx/dovecot/login.lua index a93b4b29..442833f1 100644 --- a/core/nginx/dovecot/login.lua +++ b/core/nginx/dovecot/login.lua @@ -30,7 +30,8 @@ function auth_passdb_lookup(req) auth_request:add_header('Auth-Pass', password) end auth_request:add_header('Auth-Protocol', req.service) - auth_request:add_header('Client-IP', req.remote_ip) + local client_ip = urlEncode(req.remote_ip) + auth_request:add_header('Client-Ip', client_ip) auth_request:add_header('Client-Port', req.remote_port) auth_request:add_header('Auth-SSL', req.secured) auth_request:add_header('Auth-Method', req.mechanism) From f2c0a147faac0fdd3c442865081f5b0479de3a2b Mon Sep 17 00:00:00 2001 From: Florent Daigniere Date: Fri, 9 Aug 2024 15:29:51 +0200 Subject: [PATCH 8/9] as per review (cherry picked from commit 78c5d34227c7e42b4f7aaef8cd431697726f0485) --- core/admin/mailu/internal/nginx.py | 2 +- core/admin/mailu/internal/views/auth.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/core/admin/mailu/internal/nginx.py b/core/admin/mailu/internal/nginx.py index d236513d..3638645b 100644 --- a/core/admin/mailu/internal/nginx.py +++ b/core/admin/mailu/internal/nginx.py @@ -98,7 +98,7 @@ def handle_authentication(headers): password = urllib.parse.unquote(headers["Auth-Pass"]) ip = urllib.parse.unquote(headers["Client-Ip"]) except: - app.logger.warn(f'Received undecodable user/password from nginx: {headers["Auth-User"]!r}/{headers["Auth-Pass"]!r}') + app.logger.warn(f'Received undecodable user/password from nginx: {headers.get("Auth-User", "")!r}') else: try: user = models.User.query.get(user_email) if '@' in user_email else None diff --git a/core/admin/mailu/internal/views/auth.py b/core/admin/mailu/internal/views/auth.py index d163cb80..c74bcc9e 100644 --- a/core/admin/mailu/internal/views/auth.py +++ b/core/admin/mailu/internal/views/auth.py @@ -49,7 +49,7 @@ def nginx_authentication(): if not is_port_25: utils.limiter.exempt_ip_from_ratelimits(client_ip) elif is_valid_user: - password = urllib.parse.unquote(headers.get('Auth-Pass', None)) + password = urllib.parse.unquote(headers.get('Auth-Pass', '')) utils.limiter.rate_limit_user(username, client_ip, password=password) elif not is_from_webmail: utils.limiter.rate_limit_ip(client_ip, username) From 79a393d6017f5ac357a706ed19d7e09f1f9ac20c Mon Sep 17 00:00:00 2001 From: Florent Daigniere Date: Fri, 9 Aug 2024 15:55:33 +0200 Subject: [PATCH 9/9] s/nginx/front (cherry picked from commit 5cfec650dfcee49b840134bd61292ace8fe3ea15) --- core/admin/mailu/internal/nginx.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/admin/mailu/internal/nginx.py b/core/admin/mailu/internal/nginx.py index 3638645b..daef8b9e 100644 --- a/core/admin/mailu/internal/nginx.py +++ b/core/admin/mailu/internal/nginx.py @@ -98,7 +98,7 @@ def handle_authentication(headers): password = urllib.parse.unquote(headers["Auth-Pass"]) ip = urllib.parse.unquote(headers["Client-Ip"]) except: - app.logger.warn(f'Received undecodable user/password from nginx: {headers.get("Auth-User", "")!r}') + app.logger.warn(f'Received undecodable user/password from front: {headers.get("Auth-User", "")!r}') else: try: user = models.User.query.get(user_email) if '@' in user_email else None