diff --git a/changelog.d/20187.feature b/changelog.d/20187.feature new file mode 100644 index 0000000000..d5c53f1510 --- /dev/null +++ b/changelog.d/20187.feature @@ -0,0 +1 @@ +Add support for configuring a `username` for Redis connections, for Redis 6+ ACL authentication. diff --git a/docs/usage/configuration/config_documentation.md b/docs/usage/configuration/config_documentation.md index 4882569b7c..74ac85256f 100644 --- a/docs/usage/configuration/config_documentation.md +++ b/docs/usage/configuration/config_documentation.md @@ -4670,6 +4670,8 @@ _Changed in Synapse 1.85.0: Added path option to use a local Unix socket_ _Changed in Synapse 1.116.0: Added password\_path_ +_Changed in Synapse 1.162.0: Added username_ + This setting has the following sub-options: * `enabled` (boolean): Whether to use Redis support. Defaults to `false`. @@ -4680,6 +4682,8 @@ This setting has the following sub-options: * `path` (string): The full path to a local Unix socket file. **If this is used, `host` and `port` are ignored.** Defaults to `"/tmp/redis.sock"`. +* `username` (string|null): Optional username if configured on the Redis instance (Redis 6+ ACL authentication). Requires `password` (or `password_path`) to also be set. Defaults to `null`. + * `password` (string|null): Optional password if configured on the Redis instance. Defaults to `null`. * `password_path` (string|null): Alternative to `password`, reading the password from an external file. The file should be a plain text file, containing only the password. Synapse reads the password from the given file once at startup. Defaults to `null`. @@ -4702,6 +4706,7 @@ redis: enabled: true host: localhost port: 6379 + username: password_path: dbid: ``` diff --git a/poetry.lock b/poetry.lock index 00a0fddd12..38c00e1af0 100644 --- a/poetry.lock +++ b/poetry.lock @@ -1,4 +1,4 @@ -# This file is automatically @generated by Poetry 2.4.1 and should not be changed by hand. +# This file is automatically @generated by Poetry 2.4.3 and should not be changed by hand. [[package]] name = "annotated-types" @@ -3384,15 +3384,15 @@ windows-platform = ["appdirs (>=1.4.0)", "appdirs (>=1.4.0)", "bcrypt (>=3.1.3)" [[package]] name = "txredisapi" -version = "1.4.11" +version = "1.4.12" description = "non-blocking redis client for python" optional = true python-versions = "*" groups = ["main"] markers = "extra == \"redis\" or extra == \"all\"" files = [ - {file = "txredisapi-1.4.11-py3-none-any.whl", hash = "sha256:ac64d7a9342b58edca13ef267d4fa7637c1aa63f8595e066801c1e8b56b22d0b"}, - {file = "txredisapi-1.4.11.tar.gz", hash = "sha256:3eb1af99aefdefb59eb877b1dd08861efad60915e30ad5bf3d5bf6c5cedcdbc6"}, + {file = "txredisapi-1.4.12-py3-none-any.whl", hash = "sha256:c698fff24a0b82e8932ef18c5623496b0a8b5195e4e94bf8524800a00ca6b7c1"}, + {file = "txredisapi-1.4.12.tar.gz", hash = "sha256:98e2440ff2e297c9048c5f9d516e7b1302d447cd9cd4568c986c35a8523de3c7"}, ] [package.dependencies] @@ -3784,4 +3784,4 @@ url-preview = ["lxml"] [metadata] lock-version = "2.1" python-versions = ">=3.10.0,<4.0.0" -content-hash = "4628b733106afbf7e6306058bcac72c1b273533941eb7cc8621c74b91e947b63" +content-hash = "3e9a70d3ec7ee5edd32ea5e86a0b56e7633e9ed1b59589890a4df8df25a135d5" diff --git a/pyproject.toml b/pyproject.toml index 3606443fcf..e1965b71fd 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -153,7 +153,9 @@ opentracing = [ jwt = ["authlib"] # hiredis is not a *strict* dependency, but it makes things much faster. # (if it is not installed, we fall back to slow code.) -redis = ["txredisapi>=1.4.7", "hiredis>=0.3"] +# txredisapi 1.4.12 is the first release to accept a `username` kwarg for +# Redis ACL auth: https://github.com/IlyaSkriblovsky/txredisapi/releases/tag/1.4.12 +redis = ["txredisapi>=1.4.12", "hiredis>=0.3"] # Required to use experimental `caches.track_memory_usage` config option. cache-memory = ["pympler>=1.0"] # If this is updated, don't forget to update the equivalent lines in @@ -186,7 +188,7 @@ all = [ # opentracing "jaeger-client>=4.2.0", "opentracing>=2.2.0", # redis - "txredisapi>=1.4.7", "hiredis>=0.3", + "txredisapi>=1.4.12", "hiredis>=0.3", # cache-memory # 1.0 added support for python 3.10, our current minimum supported python version "pympler>=1.0", diff --git a/schema/synapse-config.schema.yaml b/schema/synapse-config.schema.yaml index 4a4b9eed3f..b34bc374f3 100644 --- a/schema/synapse-config.schema.yaml +++ b/schema/synapse-config.schema.yaml @@ -5823,6 +5823,9 @@ properties: _Changed in Synapse 1.116.0: Added password\_path_ + + + _Changed in Synapse 1.162.0: Added username_ properties: enabled: type: boolean @@ -5842,6 +5845,13 @@ properties: The full path to a local Unix socket file. **If this is used, `host` and `port` are ignored.** default: /tmp/redis.sock + username: + type: ["string", "null"] + description: >- + Optional username if configured on the Redis instance (Redis 6+ + ACL authentication). Requires `password` (or `password_path`) to + also be set. + default: null password: type: ["string", "null"] description: Optional password if configured on the Redis instance. @@ -5885,6 +5895,7 @@ properties: - enabled: true host: localhost port: 6379 + username: "" password_path: "" dbid: "" worker_app: diff --git a/stubs/txredisapi.pyi b/stubs/txredisapi.pyi index 50ab54037a..7454ad5d14 100644 --- a/stubs/txredisapi.pyi +++ b/stubs/txredisapi.pyi @@ -38,6 +38,7 @@ class RedisProtocol(protocol.Protocol): class SubscriberProtocol(RedisProtocol): def __init__(self, *args: object, **kwargs: object): ... + username: str | None password: str | None def subscribe(self, channels: str | list[str]) -> "Deferred[None]": ... def connectionMade(self) -> None: ... @@ -56,6 +57,7 @@ def lazyConnection( connectTimeout: int | None = ..., replyTimeout: int | None = ..., convertNumbers: bool = ..., + username: str | None = ..., ) -> RedisProtocol: ... # ConnectionHandler doesn't actually inherit from RedisProtocol, but it proxies @@ -82,6 +84,7 @@ class RedisFactory(protocol.ReconnectingClientFactory): password: str | None = None, replyTimeout: int | None = None, convertNumbers: int | None = True, + username: str | None = None, ): ... def buildProtocol(self, addr: IAddress) -> RedisProtocol: ... diff --git a/synapse/config/redis.py b/synapse/config/redis.py index 948c95eef7..cf5707c2c3 100644 --- a/synapse/config/redis.py +++ b/synapse/config/redis.py @@ -49,6 +49,7 @@ class RedisConfig(Config): self.redis_port = redis_config.get("port", 6379) self.redis_path = redis_config.get("path", None) self.redis_dbid = redis_config.get("dbid", None) + self.redis_username = redis_config.get("username", None) self.redis_password = redis_config.get("password") if self.redis_password and not allow_secrets_in_config: raise ConfigError( @@ -67,6 +68,16 @@ class RedisConfig(Config): ), ).strip() + # An empty password is not the same as an unset one: Redis has no wire form + # for a username without a password, but a `nopass` ACL user accepts any. + if self.redis_username and self.redis_password is None: + raise ConfigError( + "`redis.username` was set without `redis.password` (or " + "`redis.password_path`). Redis username authentication requires " + "a password to also be configured.", + ("redis", "username"), + ) + self.redis_use_tls = redis_config.get("use_tls", False) self.redis_certificate = redis_config.get("certificate_file", None) self.redis_private_key = redis_config.get("private_key_file", None) diff --git a/synapse/replication/tcp/redis.py b/synapse/replication/tcp/redis.py index 93ba48b406..705ea4172c 100644 --- a/synapse/replication/tcp/redis.py +++ b/synapse/replication/tcp/redis.py @@ -299,6 +299,7 @@ class SynapseRedisFactory(RedisFactory): isLazy: bool = False, handler: type = ConnectionHandler, charset: str = "utf-8", + username: str | None = None, password: str | None = None, replyTimeout: int = 30, convertNumbers: int | None = True, @@ -310,6 +311,7 @@ class SynapseRedisFactory(RedisFactory): isLazy=isLazy, handler=handler, charset=charset, + username=username, password=password, replyTimeout=replyTimeout, convertNumbers=convertNumbers, @@ -390,6 +392,7 @@ class RedisDirectTcpReplicationClientFactory(SynapseRedisFactory): dbid=None, poolsize=1, replyTimeout=30, + username=hs.config.redis.redis_username, password=hs.config.redis.redis_password, ) @@ -425,6 +428,7 @@ def lazyConnection( port: int = 6379, dbid: int | None = None, reconnect: bool = True, + username: str | None = None, password: str | None = None, replyTimeout: int = 30, ) -> ConnectionHandler: @@ -440,6 +444,7 @@ def lazyConnection( poolsize=1, isLazy=True, handler=ConnectionHandler, + username=username, password=password, replyTimeout=replyTimeout, ) @@ -474,6 +479,7 @@ def lazyUnixConnection( path: str = "/tmp/redis.sock", dbid: int | None = None, reconnect: bool = True, + username: str | None = None, password: str | None = None, replyTimeout: int = 30, ) -> ConnectionHandler: @@ -493,6 +499,7 @@ def lazyUnixConnection( poolsize=1, isLazy=True, handler=UnixConnectionHandler, + username=username, password=password, replyTimeout=replyTimeout, ) diff --git a/synapse/server.py b/synapse/server.py index b756223e54..9422882167 100644 --- a/synapse/server.py +++ b/synapse/server.py @@ -1216,6 +1216,7 @@ class HomeServer(metaclass=abc.ABCMeta): host=self.config.redis.redis_host, port=self.config.redis.redis_port, dbid=self.config.redis.redis_dbid, + username=self.config.redis.redis_username, password=self.config.redis.redis_password, reconnect=True, ) @@ -1229,6 +1230,7 @@ class HomeServer(metaclass=abc.ABCMeta): hs=self, path=self.config.redis.redis_path, dbid=self.config.redis.redis_dbid, + username=self.config.redis.redis_username, password=self.config.redis.redis_password, reconnect=True, ) diff --git a/tests/config/test_redis.py b/tests/config/test_redis.py new file mode 100644 index 0000000000..3a1f810c33 --- /dev/null +++ b/tests/config/test_redis.py @@ -0,0 +1,85 @@ +# +# This file is licensed under the Affero General Public License (AGPL) version 3. +# +# Copyright (C) 2026 Element Creations Ltd +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU Affero General Public License as +# published by the Free Software Foundation, either version 3 of the +# License, or (at your option) any later version. +# +# See the GNU Affero General Public License for more details: +# . +# +# + +import tempfile + +from synapse.config._base import ConfigError +from synapse.config.homeserver import HomeServerConfig + +from tests.unittest import TestCase +from tests.utils import default_config + +try: + import hiredis +except ImportError: + hiredis = None # type: ignore + + +class RedisConfigTestCase(TestCase): + # hiredis is part of the `redis` extra, which `RedisConfig` requires to + # parse an enabled redis config. + if not hiredis: + skip = "Requires hiredis" + + def _make_config(self, redis_config: dict) -> HomeServerConfig: + config_dict = default_config(server_name="test") + config_dict["redis"] = redis_config + config = HomeServerConfig() + config.parse_config_dict(config_dict, "", "") + return config + + def test_username_defaults_to_none(self) -> None: + """`redis.username` is `None` when not configured.""" + config = self._make_config({"enabled": True, "password": "hunter2"}) + self.assertIsNone(config.redis.redis_username) + + def test_username_is_parsed(self) -> None: + """`redis.username` is parsed through to `redis_username` when set + alongside `password`.""" + config = self._make_config( + {"enabled": True, "username": "alice", "password": "hunter2"} + ) + self.assertEqual(config.redis.redis_username, "alice") + + def test_username_is_parsed_with_password_path(self) -> None: + """`redis.username` is also accepted alongside `password_path`, the + documented alternative to an inline `password`.""" + with tempfile.NamedTemporaryFile(buffering=0) as password_file: + password_file.write(b"hunter2") + + config = self._make_config( + { + "enabled": True, + "username": "alice", + "password_path": password_file.name, + } + ) + self.assertEqual(config.redis.redis_username, "alice") + self.assertEqual(config.redis.redis_password, "hunter2") + + def test_username_with_empty_password_is_accepted(self) -> None: + """`redis.username` with an explicitly empty `password` is allowed: that + is how a Redis ACL user declared `nopass` is configured.""" + config = self._make_config( + {"enabled": True, "username": "alice", "password": ""} + ) + self.assertEqual(config.redis.redis_username, "alice") + self.assertEqual(config.redis.redis_password, "") + + def test_username_without_password_is_rejected(self) -> None: + """`redis.username` without any of `password`/`password_path` is + rejected: Redis ACL authentication requires both.""" + with self.assertRaises(ConfigError): + self._make_config({"enabled": True, "username": "alice"}) diff --git a/tests/replication/_base.py b/tests/replication/_base.py index b23696668f..791a6beab5 100644 --- a/tests/replication/_base.py +++ b/tests/replication/_base.py @@ -509,6 +509,10 @@ class FakeRedisPubSubServer: defaultdict(set) ) + # The arguments of every `AUTH` command received, in order: `(password,)` + # when only a password is configured, `(username, password)` with both. + self.auth_attempts: list[tuple[bytes, ...]] = [] + def add_subscriber(self, conn: "FakeRedisPubSubProtocol", channel: bytes) -> None: """A connection has called SUBSCRIBE""" self._subscribers_by_channel[channel].add(conn) @@ -581,6 +585,11 @@ class FakeRedisPubSubProtocol(Protocol): elif command == b"PING": self.send("PONG") + # We don't check the credentials, just record that they were sent. + elif command == b"AUTH": + self._server.auth_attempts.append(args) + self.send("OK") + else: raise Exception(f"Unknown command: {command!r}") diff --git a/tests/replication/tcp/test_redis_auth.py b/tests/replication/tcp/test_redis_auth.py new file mode 100644 index 0000000000..ad025b987e --- /dev/null +++ b/tests/replication/tcp/test_redis_auth.py @@ -0,0 +1,60 @@ +# +# This file is licensed under the Affero General Public License (AGPL) version 3. +# +# Copyright (C) 2026 Element Creations Ltd +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU Affero General Public License as +# published by the Free Software Foundation, either version 3 of the +# License, or (at your option) any later version. +# +# See the GNU Affero General Public License for more details: +# . +# +# + +from typing import Any + +from tests.replication._base import BaseMultiWorkerStreamTestCase + + +class RedisUsernameAuthTestCase(BaseMultiWorkerStreamTestCase): + """Tests that Synapse authenticates to Redis with a username *and* password + when both are configured, rather than with a bare password. + """ + + USERNAME = b"synapse-user" + PASSWORD = b"correct-horse-battery-staple" + + def default_config(self) -> dict[str, Any]: + config = super().default_config() + config["redis"]["username"] = self.USERNAME.decode("utf-8") + config["redis"]["password"] = self.PASSWORD.decode("utf-8") + return config + + def test_auth_sent_with_username(self) -> None: + """Both Redis connections the main process opens (one outbound, one + subscriber) send `AUTH `. + """ + # Let the AUTH replies flow back; nothing here needs virtual time to pass. + self.reactor.advance(0) + + self.assertEqual( + self._redis_server.auth_attempts, + [(self.USERNAME, self.PASSWORD)] * 2, + ) + + def test_workers_authenticate_with_username_too(self) -> None: + """A worker authenticates the same way as the main process, and both end + up subscribed to the replication stream over those connections. + """ + self.make_worker_hs("synapse.app.generic_worker") + + # Let the AUTH and SUBSCRIBE replies flow back. + self.reactor.advance(0) + + self.assertEqual( + self._redis_server.auth_attempts, + [(self.USERNAME, self.PASSWORD)] * 4, + ) + self.assertEqual(len(self._redis_server._subscribers_by_channel[b"test"]), 2)