mirror of
https://github.com/element-hq/synapse.git
synced 2026-09-17 01:34:28 +00:00
Support configuring a username for Redis connections (#20187)
Adds a `redis.username` config option. Details: A `username` without a `password` (or `password_path`) is refused at startup. Redis has no wire form for a username without a password, and txredisapi only sends `AUTH` when a password is set, so the username would otherwise be silently ignored. An explicitly empty password is accepted, since that is how a `nopass` ACL user is configured. This relies on txredisapi 1.4.12, the first release to accept a `username` kwarg. That upstream support was contributed by @karolyi specifically to unblock this. Fixes #19238. ### Pull Request Checklist <!-- Please read https://element-hq.github.io/synapse/latest/development/contributing_guide.html before submitting your pull request --> * [X] Pull request is based on the develop branch * [X] Pull request includes a [changelog file](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#changelog). The entry should: - Be a short description of your change which makes sense to users. "Fixed a bug that prevented receiving messages from other servers." instead of "Moved X method from `EventStore` to `EventWorkerStore`.". - Use markdown where necessary, mostly for `code blocks`. - End with either a period (.) or an exclamation mark (!). - Start with a capital letter. - Feel free to credit yourself, by adding a sentence "Contributed by @github_username." or "Contributed by [Your Name]." to the end of the entry. * [X] [Code style](https://element-hq.github.io/synapse/latest/code_style.html) is correct (run the [linters](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#run-the-linters)) --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Sonnet 5
parent
c343c78182
commit
f7c16d0148
@@ -0,0 +1 @@
|
||||
Add support for configuring a `username` for Redis connections, for Redis 6+ ACL authentication.
|
||||
@@ -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: <username>
|
||||
password_path: <path_to_the_password_file>
|
||||
dbid: <dbid>
|
||||
```
|
||||
|
||||
Generated
+5
-5
@@ -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"
|
||||
|
||||
+4
-2
@@ -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",
|
||||
|
||||
@@ -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: "<username>"
|
||||
password_path: "<path_to_the_password_file>"
|
||||
dbid: "<dbid>"
|
||||
worker_app:
|
||||
|
||||
@@ -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: ...
|
||||
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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,
|
||||
)
|
||||
|
||||
@@ -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,
|
||||
)
|
||||
|
||||
@@ -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:
|
||||
# <https://www.gnu.org/licenses/agpl-3.0.html>.
|
||||
#
|
||||
#
|
||||
|
||||
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"})
|
||||
@@ -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}")
|
||||
|
||||
|
||||
@@ -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:
|
||||
# <https://www.gnu.org/licenses/agpl-3.0.html>.
|
||||
#
|
||||
#
|
||||
|
||||
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 <username> <password>`.
|
||||
"""
|
||||
# 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)
|
||||
Reference in New Issue
Block a user