Merge pull request #1860 from jmberg/dovecot-auth-ip

auth: dovecot: pass rip= to auth server

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

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)_

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

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"):
+        if environ.get("REMOTE_ADDR"):
-            remote_host = environ["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))

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)

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())
                             )

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",

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,
+            self, user, password, expected_status, expected_rip=None,
-            response=b'FAIL\n1\n', mech=[b'PLAIN'], broken=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",
+        if extra_env is None:
-                                 "dovecot_socket": "./dovecot.sock"}})
+            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"}})