From b5a1ea911d0cfff2e9f55a72d74eaf4866be42cb Mon Sep 17 00:00:00 2001 From: Johannes Berg Date: Sat, 30 Aug 2025 21:59:22 +0200 Subject: [PATCH] auth: dovecot: pass remote IP (rip=) to auth server If known, let the auth server know where the client came from, using REMOTE_ADDR or, optionally/configurably, the X-Remote-Addr header value (which is needed when running behind a trusted proxy.) Addresses #1859. --- CHANGELOG.md | 1 + DOCUMENTATION.md | 20 ++++++++++++ config | 3 ++ radicale/app/__init__.py | 13 ++++++-- radicale/auth/__init__.py | 30 ++++++++++++++++-- radicale/auth/dovecot.py | 18 +++++++++-- radicale/config.py | 4 +++ radicale/tests/test_auth.py | 62 +++++++++++++++++++++++++++++++++---- 8 files changed, 136 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f5de8c23..c3f3cb51 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ * Fix: broken start when UID does not exist (potential container startup case) * Improve: user/group retrievement for running service and directories * Extend/Improve: [auth] ldap: group membership lookup +* Add: option [auth] dovecot_rip_x_remote_addr ## 3.5.5 * Improve: [auth] ldap: do not read server info by bind to avoid needless network traffic diff --git a/DOCUMENTATION.md b/DOCUMENTATION.md index 632c477a..4f6b7ed0 100644 --- a/DOCUMENTATION.md +++ b/DOCUMENTATION.md @@ -1187,6 +1187,26 @@ Port of via network exposed dovecot socket Default: `12345` +##### dovecot_rip_x_remote_addr + +_(>= 3.5.6)_ + +Use the `X-Remote-Addr` value for the remote IP (rip) parameter in the +dovecot authentication protocol. + +If set, Radicale must be running behind a proxy that you control and +that sets/overwrites the `X-Remote-Addr` header (doesn't pass it) so +that the value passed to dovecot is reliable. For example, for nginx, +add + +``` + proxy_set_header X-Remote-Addr $remote_addr; +``` + +to the configuration sample. + +Default: `False` + ##### imap_host _(>= 3.4.1)_ diff --git a/config b/config index b51c5dfc..37040eff 100644 --- a/config +++ b/config @@ -136,6 +136,9 @@ # Port of via network exposed dovecot socket #dovecot_port = 12345 +# Use X-Remote-Addr for remote IP (rip) in dovecot authentication +#dovecot_rip_x_remote_addr = False + # IMAP server hostname # Syntax: address | address:port | [address]:port | imap.server.tld #imap_host = localhost diff --git a/radicale/app/__init__.py b/radicale/app/__init__.py index 9a06d34e..0fcbd328 100644 --- a/radicale/app/__init__.py +++ b/radicale/app/__init__.py @@ -49,6 +49,7 @@ from radicale.app.propfind import ApplicationPartPropfind from radicale.app.proppatch import ApplicationPartProppatch from radicale.app.put import ApplicationPartPut from radicale.app.report import ApplicationPartReport +from radicale.auth import AuthContext from radicale.log import logger # Combination of types.WSGIStartResponse and WSGI application return value @@ -156,6 +157,8 @@ class Application(ApplicationPartDelete, ApplicationPartHead, unsafe_path = environ.get("PATH_INFO", "") https = environ.get("HTTPS", "") + context = AuthContext() + """Manage a request.""" def response(status: int, headers: types.WSGIResponseHeaders, answer: Union[None, str, bytes]) -> _IntermediateResponse: @@ -201,12 +204,16 @@ class Application(ApplicationPartDelete, ApplicationPartHead, remote_host = "unknown" if environ.get("REMOTE_HOST"): remote_host = repr(environ["REMOTE_HOST"]) - elif environ.get("REMOTE_ADDR"): - remote_host = environ["REMOTE_ADDR"] + if environ.get("REMOTE_ADDR"): + if remote_host == 'unknown': + remote_host = environ["REMOTE_ADDR"] + context.remote_addr = environ["REMOTE_ADDR"] if environ.get("HTTP_X_FORWARDED_FOR"): reverse_proxy = True remote_host = "%s (forwarded for %r)" % ( remote_host, environ["HTTP_X_FORWARDED_FOR"]) + if environ.get("HTTP_X_REMOTE_ADDR"): + context.x_remote_addr = environ["HTTP_X_REMOTE_ADDR"] if environ.get("HTTP_X_FORWARDED_HOST") or environ.get("HTTP_X_FORWARDED_PROTO") or environ.get("HTTP_X_FORWARDED_SERVER"): reverse_proxy = True remote_useragent = "" @@ -295,7 +302,7 @@ class Application(ApplicationPartDelete, ApplicationPartHead, self.configuration, environ, base64.b64decode( authorization.encode("ascii"))).split(":", 1) - (user, info) = self._auth.login(login, password) or ("", "") if login else ("", "") + (user, info) = self._auth.login(login, password, context) or ("", "") if login else ("", "") if self.configuration.get("auth", "type") == "ldap": try: logger.debug("Groups received from LDAP: %r", ",".join(self._auth._ldap_groups)) diff --git a/radicale/auth/__init__.py b/radicale/auth/__init__.py index 2de8c4e9..d6371383 100644 --- a/radicale/auth/__init__.py +++ b/radicale/auth/__init__.py @@ -91,6 +91,15 @@ def load(configuration: "config.Configuration") -> "BaseAuth": configuration) +class AuthContext: + remote_addr: str + x_remote_addr: str + + def __init__(self): + self.remote_addr = None + self.x_remote_addr = None + + class BaseAuth: _ldap_groups: Set[str] = set([]) @@ -187,6 +196,21 @@ class BaseAuth: raise NotImplementedError + def _login_ext(self, login: str, password: str, context: AuthContext) -> str: + """Check credentials and map login to internal user + + ``login`` the login name + + ``password`` the password + + ``context`` additional data for the login, e.g. IP address used + + Returns the username or ``""`` for invalid credentials. + """ + + # override this method instead of _login() if you want the context + return self._login(login, password) + def _sleep_for_constant_exec_time(self, time_ns_begin: int): """Sleep some time to reach a constant execution time for failed logins @@ -216,7 +240,7 @@ class BaseAuth: time.sleep(sleep) @final - def login(self, login: str, password: str) -> Tuple[str, str]: + def login(self, login: str, password: str, context: AuthContext) -> Tuple[str, str]: time_ns_begin = time.time_ns() result_from_cache = False if self._lc_username: @@ -284,7 +308,7 @@ class BaseAuth: if result == "": # verify login+password via configured backend logger.debug("Login verification for user+password via backend: '%s'", login) - result = self._login(login, password) + result = self._login_ext(login, password, context) if result != "": logger.debug("Login successful for user+password via backend: '%s'", login) if digest == "": @@ -314,7 +338,7 @@ class BaseAuth: return (result, self._type) else: # self._cache_logins is False - result = self._login(login, password) + result = self._login_ext(login, password, context) if result == "": self._sleep_for_constant_exec_time(time_ns_begin) return (result, self._type) diff --git a/radicale/auth/dovecot.py b/radicale/auth/dovecot.py index b3f3fb81..b4d28c32 100644 --- a/radicale/auth/dovecot.py +++ b/radicale/auth/dovecot.py @@ -19,6 +19,7 @@ import base64 import itertools import os +import re import socket from contextlib import closing @@ -32,6 +33,8 @@ class Auth(auth.BaseAuth): self.timeout = 5 self.request_id_gen = itertools.count(1) + self.use_x_remote_addr = configuration.get("auth", "dovecot_rip_x_remote_addr") + config_family = configuration.get("auth", "dovecot_connection_type") if config_family == "AF_UNIX": self.family = socket.AF_UNIX @@ -46,7 +49,7 @@ class Auth(auth.BaseAuth): else: self.family = socket.AF_INET6 - def _login(self, login, password): + def _login_ext(self, login, password, context): """Validate credentials. Check if the ``login``/``password`` pair is valid according to Dovecot. @@ -148,10 +151,19 @@ class Auth(auth.BaseAuth): "Authenticating with request id: '{}'" .format(request_id) ) + rip = b'' + if self.use_x_remote_addr and context.x_remote_addr: + rip = context.x_remote_addr.encode('ascii') + elif context.remote_addr: + rip = context.remote_addr.encode('ascii') + # squash all whitespace - shouldn't be there and auth protocol + # is sensitive to whitespace (in particular \t and \n) + if rip: + rip = b'\trip=' + re.sub(br'\s', b'', rip) sock.send( - b'AUTH\t%u\tPLAIN\tservice=radicale\tresp=%b\n' % + b'AUTH\t%u\tPLAIN\tservice=radicale%s\tresp=%b\n' % ( - request_id, base64.b64encode( + request_id, rip, base64.b64encode( b'\0%b\0%b' % (login.encode(), password.encode()) ) diff --git a/radicale/config.py b/radicale/config.py index adab9567..a228ed97 100644 --- a/radicale/config.py +++ b/radicale/config.py @@ -253,6 +253,10 @@ DEFAULT_CONFIG_SCHEMA: types.CONFIG_SCHEMA = OrderedDict([ "value": "12345", "help": "dovecot auth port", "type": int}), + ("dovecot_rip_x_remote_addr", { + "value": "False", + "help": "use X-Remote-Addr for dovecot auth remote IP (rip) parameter", + "type": bool}), ("realm", { "value": "Radicale - Password Required", "help": "message displayed when a password is needed", diff --git a/radicale/tests/test_auth.py b/radicale/tests/test_auth.py index 88cd3ea4..e34256ed 100644 --- a/radicale/tests/test_auth.py +++ b/radicale/tests/test_auth.py @@ -282,13 +282,23 @@ class TestBaseAuthRequests(BaseTest): @pytest.mark.skipif(sys.platform == 'win32', reason="Not supported on Windows") def _test_dovecot( - self, user, password, expected_status, - response=b'FAIL\n1\n', mech=[b'PLAIN'], broken=None): + self, user, password, expected_status, expected_rip=None, + response=b'FAIL\t1', mech=[b'PLAIN'], broken=None, + extra_config=None, extra_env=None): import socket from unittest.mock import DEFAULT, patch - self.configure({"auth": {"type": "dovecot", - "dovecot_socket": "./dovecot.sock"}}) + if extra_env is None: + extra_env = {} + if extra_config is None: + extra_config = {} + + config = {"auth": {"type": "dovecot", + "dovecot_socket": "./dovecot.sock"}} + for toplvl, entries in extra_config.items(): + for key, val in entries.items(): + config[toplvl][key] = val + self.configure(config) if broken is None: broken = [] @@ -311,10 +321,18 @@ class TestBaseAuthRequests(BaseTest): if "done" not in broken: handshake += b'DONE\n' + sent_rip = None + + def record_sent_data(s, data, flags=None): + nonlocal sent_rip + if b'\trip=' in data: + sent_rip = data.split(b'\trip=')[1].split(b'\t')[0] + return len(data) + with patch.multiple( 'socket.socket', connect=DEFAULT, - send=DEFAULT, + send=record_sent_data, recv=DEFAULT ) as mock_socket: if "socket" in broken: @@ -325,7 +343,9 @@ class TestBaseAuthRequests(BaseTest): status, _, answer = self.request( "PROPFIND", "/", HTTP_AUTHORIZATION="Basic %s" % base64.b64encode( - ("%s:%s" % (user, password)).encode()).decode()) + ("%s:%s" % (user, password)).encode()).decode(), + **extra_env) + assert sent_rip == expected_rip assert status == expected_status @pytest.mark.skipif(sys.platform == 'win32', reason="Not supported on Windows") @@ -392,6 +412,36 @@ class TestBaseAuthRequests(BaseTest): def test_dovecot_auth_id_mismatch(self): self._test_dovecot("user", "password", 401, response=b'OK\t2') + @pytest.mark.skipif(sys.platform == 'win32', reason="Not supported on Windows") + def test_dovecot_remote_addr(self): + self._test_dovecot("user", "password", 401, expected_rip=b'172.17.16.15', + extra_env={ + 'REMOTE_ADDR': '172.17.16.15', + 'HTTP_X_REMOTE_ADDR': '127.0.0.1', + }) + + @pytest.mark.skipif(sys.platform == 'win32', reason="Not supported on Windows") + def test_dovecot_x_remote_addr(self): + self._test_dovecot("user", "password", 401, expected_rip=b'172.17.16.15', + extra_env={ + 'REMOTE_ADDR': '127.0.0.1', + 'HTTP_X_REMOTE_ADDR': '172.17.16.15', + }, + extra_config={ + 'auth': {"dovecot_rip_x_remote_addr": "True"}, + }) + + @pytest.mark.skipif(sys.platform == 'win32', reason="Not supported on Windows") + def test_dovecot_x_remote_addr_whitespace(self): + self._test_dovecot("user", "password", 401, expected_rip=b'172.17.16.15rip=127.0.0.1', + extra_env={ + 'REMOTE_ADDR': '127.0.0.1', + 'HTTP_X_REMOTE_ADDR': '172.17.16.15\trip=127.0.0.1', + }, + extra_config={ + 'auth': {"dovecot_rip_x_remote_addr": "True"}, + }) + def test_custom(self) -> None: """Custom authentication.""" self.configure({"auth": {"type": "radicale.tests.custom.auth"}})