Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Add redis SSL configuration options #15312

Merged
merged 11 commits into from
May 11, 2023
1 change: 1 addition & 0 deletions changelog.d/15312.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Add redis TLS configuration options.
4 changes: 4 additions & 0 deletions contrib/docker_compose_workers/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,10 @@ redis:
port: 6379
# dbid: <redis_logical_db_id>
# password: <secret_password>
# use_tls: True
# certificate_file: <path_to_certificate>
# private_key_file: <path_to_private_key>
# ca_file: <path_to_ca_certificate>
```

This assumes that your Redis service is called `redis` in your Docker Compose file.
Expand Down
9 changes: 9 additions & 0 deletions docs/usage/configuration/config_documentation.md
Original file line number Diff line number Diff line change
Expand Up @@ -3938,6 +3938,11 @@ This setting has the following sub-options:
localhost and 6379
* `password`: Optional password if configured on the Redis instance.
* `dbid`: Optional redis dbid if needs to connect to specific redis logical db.
* `use_tls`: Whether to use tls connection. Defaults to false.
* `certificate_file`: Optional path to the certificate file
* `private_key_file`: Optional path to the private key file
* `ca_file`: Optional path to the CA certificate file. Use this one or:
* `ca_path`: Optional path to the folder containing the CA certificate file

_Added in Synapse 1.78.0._
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm guessing this refers to the dbid option specifically?

We should have explicitly point out when the new options were added.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah yes, that's a good point, I didn't spot that since it was floating.
I guess it would be sensible to put _Added in Synapse 1.78.0._ on the original options and add (I believe) _Added in Synapse 1.83.0._ on the new ones.

Otherwise might be worth seeing what the other option groups do if the 'sub-options' were added at different times.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If in doubt, my advice would be to copy what the Python docs do, e.g. https://docs.python.org/3/library/subprocess.html?highlight=subprocess#subprocess.TimeoutExpired

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added the changelog with version. I used version 1.84.0, since 1.83.0 is already in RC so I assume it won't end up in that one.


Expand All @@ -3949,6 +3954,10 @@ redis:
port: 6379
password: <secret_password>
dbid: <dbid>
#use_tls: True
#certificate_file: <path_to_the_certificate_file>
#private_key_file: <path_to_the_private_key_file>
#ca_file: <path_to_the_ca_certificate_file>
```
---
## Individual worker configuration
Expand Down
6 changes: 6 additions & 0 deletions synapse/config/redis.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,3 +35,9 @@ def read_config(self, config: JsonDict, **kwargs: Any) -> None:
self.redis_port = redis_config.get("port", 6379)
self.redis_dbid = redis_config.get("dbid", None)
self.redis_password = redis_config.get("password")

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)
self.redis_ca_file = redis_config.get("ca_file", None)
self.redis_ca_path = redis_config.get("ca_path", None)
34 changes: 34 additions & 0 deletions synapse/replication/tcp/context.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
# Copyright 2023 The Matrix.org Foundation C.I.C.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
from OpenSSL.SSL import Context
from twisted.internet import ssl

from synapse.config.redis import RedisConfig


class ClientContextFactory(ssl.ClientContextFactory):
def __init__(self, redis_config: RedisConfig):
self.redis_config = redis_config

def getContext(self) -> Context:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm slightly suspicious of Contexts because I remember the defaults being a bit iffy.

Before merging, we should check the correct TLS verification flags are set on this.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(I looked into this and it seems sane. Notably, it's the Twisted SSL context, not the vanilla Python one 😵‍💫.)

ctx = super().getContext()
if self.redis_config.redis_certificate:
ctx.use_certificate_file(self.redis_config.redis_certificate)
if self.redis_config.redis_private_key:
ctx.use_privatekey_file(self.redis_config.redis_private_key)
if self.redis_config.redis_ca_file:
ctx.load_verify_locations(cafile=self.redis_config.redis_ca_file)
elif self.redis_config.redis_ca_path:
ctx.load_verify_locations(capath=self.redis_config.redis_ca_path)
return ctx
29 changes: 22 additions & 7 deletions synapse/replication/tcp/handler.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@
UserIpCommand,
UserSyncCommand,
)
from synapse.replication.tcp.context import ClientContextFactory
from synapse.replication.tcp.protocol import IReplicationConnection
from synapse.replication.tcp.streams import (
STREAMS_MAP,
Expand Down Expand Up @@ -348,13 +349,27 @@ def start_replication(self, hs: "HomeServer") -> None:
outbound_redis_connection,
channel_names=self._channels_to_subscribe_to,
)
hs.get_reactor().connectTCP(
hs.config.redis.redis_host,
hs.config.redis.redis_port,
self._factory,
timeout=30,
bindAddress=None,
)

reactor = hs.get_reactor()
redis_config = hs.config.redis
if hs.config.redis.redis_use_tls:
ssl_context_factory = ClientContextFactory(hs.config.redis)
reactor.connectSSL(
redis_config.redis_host,
redis_config.redis_port,
self._factory,
ssl_context_factory,
timeout=30,
bindAddress=None,
)
else:
reactor.connectTCP(
redis_config.redis_host,
redis_config.redis_port,
self._factory,
timeout=30,
bindAddress=None,
)

def get_streams(self) -> Dict[str, Stream]:
"""Get a map from stream name to all streams."""
Expand Down
27 changes: 20 additions & 7 deletions synapse/replication/tcp/redis.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
ReplicateCommand,
parse_command_from_line,
)
from synapse.replication.tcp.context import ClientContextFactory
from synapse.replication.tcp.protocol import (
IReplicationConnection,
tcp_inbound_commands_counter,
Expand Down Expand Up @@ -386,12 +387,24 @@ def lazyConnection(
factory.continueTrying = reconnect

reactor = hs.get_reactor()
reactor.connectTCP(
host,
port,
factory,
timeout=30,
bindAddress=None,
)

if hs.config.redis.redis_use_tls:
ssl_context_factory = ClientContextFactory(hs.config.redis)
reactor.connectSSL(
host,
port,
factory,
ssl_context_factory,
timeout=30,
bindAddress=None,
)
else:
reactor.connectTCP(
host,
port,
factory,
timeout=30,
bindAddress=None,
)

return factory.handler