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

Experimental Unix socket support #15353

Merged
merged 20 commits into from
Apr 3, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
906acaf
Add IReactorUNIX to ISynapseReactor type hint.
realtyem Mar 27, 2023
aac7fac
Create listen_unix().
realtyem Mar 28, 2023
9abd099
Create UnixListenerConfig and wire it up.
realtyem Mar 28, 2023
43a6ee9
Refactor SynapseRequest to handle logging correctly when using a unix…
realtyem Mar 29, 2023
7b93a29
Make the 'Synapse now listening on Unix socket' log line a little pre…
realtyem Mar 29, 2023
3267581
No silent failures on generic workers when trying to use a unix socke…
realtyem Mar 29, 2023
1e0452f
Inline variables in app/_base.py
realtyem Mar 29, 2023
c0eb5b3
Update docstring for listen_unix() to remove reference to a hardcoded…
realtyem Mar 29, 2023
9fdbb5d
Disallow both a unix socket and a ip/port combo on the same listener …
realtyem Mar 29, 2023
67eecf0
Linting
realtyem Mar 29, 2023
c74ad1d
Changelog
realtyem Mar 29, 2023
2309543
Merge branch 'develop' into exp/unix-reactor
realtyem Mar 29, 2023
3703cd0
review: simplify how listen_unix returns(and get rid of a type: ignore)
realtyem Mar 30, 2023
8876780
review: fix typo from ConfigError in app/homeserver.py
realtyem Mar 30, 2023
d6b88f3
review: roll conditional for http_options.tag into get_site_tag() hel…
realtyem Mar 30, 2023
2acf0a6
review: enhance the conditionals for checking if a port or path is va…
realtyem Mar 30, 2023
2a99e49
review: Try updating comment in get_client_ip_if_available to clarify…
realtyem Mar 31, 2023
547ff5f
Pretty up how 'Synapse now listening on Unix Socket' looks by decodin…
realtyem Mar 31, 2023
d1742a4
review: In parse_listener_def(), raise ConfigError if neither socket_…
realtyem Apr 1, 2023
cfdc5ac
Merge branch 'develop' into exp/unix-reactor
realtyem Apr 1, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 28 additions & 25 deletions synapse/app/_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@
from synapse.config import ConfigError
from synapse.config._base import format_config_error
from synapse.config.homeserver import HomeServerConfig
from synapse.config.server import ListenerConfig, ManholeConfig
from synapse.config.server import ListenerConfig, ManholeConfig, TCPListenerConfig
from synapse.crypto import context_factory
from synapse.events.presence_router import load_legacy_presence_router
from synapse.events.spamcheck import load_legacy_spam_checkers
Expand Down Expand Up @@ -390,44 +390,47 @@ def listen_http(
context_factory: Optional[IOpenSSLContextFactory],
reactor: ISynapseReactor = reactor,
) -> List[Port]:
port = listener_config.port
bind_addresses = listener_config.bind_addresses
tls = listener_config.tls

assert listener_config.http_options is not None

site_tag = listener_config.http_options.tag
if site_tag is None:
site_tag = str(port)
site_tag = listener_config.get_site_tag()
realtyem marked this conversation as resolved.
Show resolved Hide resolved

site = SynapseSite(
"synapse.access.%s.%s" % ("https" if tls else "http", site_tag),
"synapse.access.%s.%s"
% ("https" if listener_config.is_tls() else "http", site_tag),
site_tag,
listener_config,
root_resource,
version_string,
max_request_body_size=max_request_body_size,
reactor=reactor,
)
if tls:
# refresh_certificate should have been called before this.
assert context_factory is not None
ports = listen_ssl(
bind_addresses,
port,
site,
context_factory,
reactor=reactor,
)
logger.info("Synapse now listening on TCP port %d (TLS)", port)

if isinstance(listener_config, TCPListenerConfig):
port = listener_config.port
bind_addresses = listener_config.bind_addresses
if listener_config.is_tls():
# refresh_certificate should have been called before this.
assert context_factory is not None
ports = listen_ssl(
bind_addresses,
port,
site,
context_factory,
reactor=reactor,
)
logger.info("Synapse now listening on TCP port %d (TLS)", port)
else:
ports = listen_tcp(bind_addresses, port, site, reactor=reactor)
logger.info("Synapse now listening on TCP port %d", port)

else:
ports = listen_tcp(
bind_addresses,
port,
site,
reactor=reactor,
)
logger.info("Synapse now listening on TCP port %d", port)
path = listener_config.path
mode = listener_config.mode
ports = listen_unix(path, mode, site, reactor=reactor)
logger.info(f"Synapse now listening on Unix Socket at: {ports[0]}")

return ports


Expand Down
24 changes: 13 additions & 11 deletions synapse/app/generic_worker.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@
from synapse.config._base import ConfigError
from synapse.config.homeserver import HomeServerConfig
from synapse.config.logger import setup_logging
from synapse.config.server import ListenerConfig
from synapse.config.server import ListenerConfig, TCPListenerConfig
from synapse.federation.transport.server import TransportLayerServer
from synapse.http.server import JsonResource, OptionsResource
from synapse.logging.context import LoggingContext
Expand Down Expand Up @@ -236,23 +236,25 @@ def start_listening(self) -> None:
if listener.type == "http":
self._listen_http(listener)
elif listener.type == "manhole":
_base.listen_manhole(
listener.bind_addresses,
listener.port,
manhole_settings=self.config.server.manhole_settings,
manhole_globals={"hs": self},
)
if isinstance(listener, TCPListenerConfig):
_base.listen_manhole(
listener.bind_addresses,
listener.port,
manhole_settings=self.config.server.manhole_settings,
manhole_globals={"hs": self},
)
elif listener.type == "metrics":
if not self.config.metrics.enable_metrics:
logger.warning(
"Metrics listener configured, but "
"enable_metrics is not True!"
)
else:
_base.listen_metrics(
listener.bind_addresses,
listener.port,
)
if isinstance(listener, TCPListenerConfig):
_base.listen_metrics(
listener.bind_addresses,
listener.port,
)
else:
logger.warning("Unsupported listener type: %s", listener.type)

Expand Down
40 changes: 26 additions & 14 deletions synapse/app/homeserver.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@
)
from synapse.config._base import ConfigError, format_config_error
from synapse.config.homeserver import HomeServerConfig
from synapse.config.server import ListenerConfig
from synapse.config.server import ListenerConfig, TCPListenerConfig
from synapse.federation.transport.server import TransportLayerServer
from synapse.http.additional_resource import AdditionalResource
from synapse.http.server import (
Expand Down Expand Up @@ -78,14 +78,15 @@ class SynapseHomeServer(HomeServer):
DATASTORE_CLASS = DataStore # type: ignore

def _listener_http(
self, config: HomeServerConfig, listener_config: ListenerConfig
self,
config: HomeServerConfig,
listener_config: ListenerConfig,
) -> Iterable[Port]:
port = listener_config.port
# Must exist since this is an HTTP listener.
assert listener_config.http_options is not None
site_tag = listener_config.http_options.tag
if site_tag is None:
site_tag = str(port)
site_tag = listener_config.get_site_tag()

# We always include a health resource.
resources: Dict[str, Resource] = {"/health": HealthResource()}
Expand Down Expand Up @@ -252,23 +253,34 @@ def start_listening(self) -> None:
self._listener_http(self.config, listener)
)
elif listener.type == "manhole":
_base.listen_manhole(
listener.bind_addresses,
listener.port,
manhole_settings=self.config.server.manhole_settings,
manhole_globals={"hs": self},
)
if isinstance(listener, TCPListenerConfig):
_base.listen_manhole(
listener.bind_addresses,
listener.port,
manhole_settings=self.config.server.manhole_settings,
manhole_globals={"hs": self},
)
else:
raise ConfigError(
"Can not using a unix socket for manhole at this time."
realtyem marked this conversation as resolved.
Show resolved Hide resolved
)
elif listener.type == "metrics":
if not self.config.metrics.enable_metrics:
logger.warning(
"Metrics listener configured, but "
"enable_metrics is not True!"
)
else:
_base.listen_metrics(
listener.bind_addresses,
listener.port,
)
if isinstance(listener, TCPListenerConfig):
_base.listen_metrics(
listener.bind_addresses,
listener.port,
)
else:
raise ConfigError(
"Can not use a unix socket for metrics at this time."
)

else:
# this shouldn't happen, as the listener type should have been checked
# during parsing
Expand Down
103 changes: 75 additions & 28 deletions synapse/config/server.py
Original file line number Diff line number Diff line change
Expand Up @@ -214,17 +214,47 @@ class HttpListenerConfig:


@attr.s(slots=True, frozen=True, auto_attribs=True)
class ListenerConfig:
"""Object describing the configuration of a single listener."""
class TCPListenerConfig:
"""Object describing the configuration of a single TCP listener."""

port: int = attr.ib(validator=attr.validators.instance_of(int))
bind_addresses: List[str]
bind_addresses: List[str] = attr.ib(validator=attr.validators.instance_of(List))
type: str = attr.ib(validator=attr.validators.in_(KNOWN_LISTENER_TYPES))
tls: bool = False

# http_options is only populated if type=http
http_options: Optional[HttpListenerConfig] = None

def get_site_tag(self) -> str:
return str(self.port)

def is_tls(self) -> bool:
return self.tls
Comment on lines +235 to +236
Copy link
Member

Choose a reason for hiding this comment

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

I don't see why we need this? It doesn't look like we ever try to call get_tls on the UnixListenerConfig?

Copy link
Member

Choose a reason for hiding this comment

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

Oh, hmm... maybe it is used in one spot? has_tls_listener is that the spot?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Three places it gets used at:

  1. The one you mention(has_tls_listener()
  2. When constructing the SynapseSite (for deciding if the scheme needs to be http or `https)(in app/_base.py)
  3. Just below that when parsing the TCP section to decide if going to call listen_ssl() vs listen_tcp(). The isinstance allow asserting that it is appropriate to be in the TCP section at all. This place isn't technically a use case that showcases it's usefulness unlike the other two.

Copy link
Member

Choose a reason for hiding this comment

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

FWIW I suspect the second one (in SynapseSite) would be better replaced with a property to calculate the scheme. 😄 Regardless, seems fine! Thanks for detailing the spots.



@attr.s(slots=True, frozen=True, auto_attribs=True)
class UnixListenerConfig:
"""Object describing the configuration of a single Unix socket listener."""

# Note: unix sockets can not be tls encrypted, so HAVE to be behind a tls-handling
# reverse proxy
path: str = attr.ib()
mode: int
type: str = attr.ib(validator=attr.validators.in_(KNOWN_LISTENER_TYPES))

# http_options is only populated if type=http
http_options: Optional[HttpListenerConfig] = None

def get_site_tag(self) -> str:
return "unix"

def is_tls(self) -> bool:
"""Unix sockets can't have TLS"""
return False


ListenerConfig = Union[TCPListenerConfig, UnixListenerConfig]


@attr.s(slots=True, frozen=True, auto_attribs=True)
class ManholeConfig:
Expand Down Expand Up @@ -531,12 +561,12 @@ def read_config(self, config: JsonDict, **kwargs: Any) -> None:

self.listeners = [parse_listener_def(i, x) for i, x in enumerate(listeners)]

# no_tls is not really supported any more, but let's grandfather it in
# here.
# no_tls is not really supported anymore, but let's grandfather it in here.
if config.get("no_tls", False):
l2 = []
for listener in self.listeners:
if listener.tls:
if isinstance(listener, TCPListenerConfig) and listener.tls:
# Use isinstance() as the assertion this *has* a listener.port
realtyem marked this conversation as resolved.
Show resolved Hide resolved
logger.info(
"Ignoring TLS-enabled listener on port %i due to no_tls",
listener.port,
Expand Down Expand Up @@ -577,7 +607,7 @@ def read_config(self, config: JsonDict, **kwargs: Any) -> None:
)

self.listeners.append(
ListenerConfig(
TCPListenerConfig(
port=bind_port,
bind_addresses=[bind_host],
tls=True,
Expand All @@ -589,7 +619,7 @@ def read_config(self, config: JsonDict, **kwargs: Any) -> None:
unsecure_port = config.get("unsecure_port", bind_port - 400)
if unsecure_port:
self.listeners.append(
ListenerConfig(
TCPListenerConfig(
port=unsecure_port,
bind_addresses=[bind_host],
tls=False,
Expand All @@ -601,7 +631,7 @@ def read_config(self, config: JsonDict, **kwargs: Any) -> None:
manhole = config.get("manhole")
if manhole:
self.listeners.append(
ListenerConfig(
TCPListenerConfig(
port=manhole,
bind_addresses=["127.0.0.1"],
type="manhole",
Expand Down Expand Up @@ -648,7 +678,7 @@ def read_config(self, config: JsonDict, **kwargs: Any) -> None:
logger.warning(METRICS_PORT_WARNING)

self.listeners.append(
ListenerConfig(
TCPListenerConfig(
port=metrics_port,
bind_addresses=[config.get("metrics_bind_host", "127.0.0.1")],
type="http",
Expand Down Expand Up @@ -724,7 +754,7 @@ def read_config(self, config: JsonDict, **kwargs: Any) -> None:
self.delete_stale_devices_after = None

def has_tls_listener(self) -> bool:
return any(listener.tls for listener in self.listeners)
return any(listener.is_tls() for listener in self.listeners)

def generate_config_section(
self,
Expand Down Expand Up @@ -904,25 +934,15 @@ def parse_listener_def(num: int, listener: Any) -> ListenerConfig:
raise ConfigError(DIRECT_TCP_ERROR, ("listeners", str(num), "type"))

port = listener.get("port")
if type(port) is not int:
socket_path = listener.get("path")
# Either a port or a path should be declared at a minimum. Using both would be bad.
realtyem marked this conversation as resolved.
Show resolved Hide resolved
if port and not isinstance(port, int):
raise ConfigError("Listener configuration is lacking a valid 'port' option")
if socket_path and not isinstance(socket_path, str):
realtyem marked this conversation as resolved.
Show resolved Hide resolved
raise ConfigError("Listener configuration is lacking a valid 'path' option")

tls = listener.get("tls", False)

bind_addresses = listener.get("bind_addresses", [])
bind_address = listener.get("bind_address")
# if bind_address was specified, add it to the list of addresses
if bind_address:
bind_addresses.append(bind_address)

# if we still have an empty list of addresses, use the default list
if not bind_addresses:
if listener_type == "metrics":
# the metrics listener doesn't support IPv6
bind_addresses.append("0.0.0.0")
else:
bind_addresses.extend(DEFAULT_BIND_ADDRESSES)

http_config = None
if listener_type == "http":
try:
Expand All @@ -932,16 +952,43 @@ def parse_listener_def(num: int, listener: Any) -> ListenerConfig:
except ValueError as e:
raise ConfigError("Unknown listener resource") from e

# For a unix socket, default x_forwarded to True, as this is the only way of
# getting a client IP.
# Note: a reverse proxy is required anyway, as there is no way of exposing a
# unix socket to the internet.
http_config = HttpListenerConfig(
x_forwarded=listener.get("x_forwarded", False),
x_forwarded=listener.get("x_forwarded", (True if socket_path else False)),
resources=resources,
additional_resources=listener.get("additional_resources", {}),
tag=listener.get("tag"),
request_id_header=listener.get("request_id_header"),
experimental_cors_msc3886=listener.get("experimental_cors_msc3886", False),
)

return ListenerConfig(port, bind_addresses, listener_type, tls, http_config)
if socket_path:
# TODO: Add in path validation, like if the directory exists and is writable?
# TODO: Set this default in a better place, maybe UnixListenerConfig?
realtyem marked this conversation as resolved.
Show resolved Hide resolved
socket_mode = listener.get("mode", 0o666)

return UnixListenerConfig(socket_path, socket_mode, listener_type, http_config)

else:
assert port is not None
bind_addresses = listener.get("bind_addresses", [])
bind_address = listener.get("bind_address")
# if bind_address was specified, add it to the list of addresses
if bind_address:
bind_addresses.append(bind_address)

# if we still have an empty list of addresses, use the default list
if not bind_addresses:
if listener_type == "metrics":
# the metrics listener doesn't support IPv6
bind_addresses.append("0.0.0.0")
else:
bind_addresses.extend(DEFAULT_BIND_ADDRESSES)

return TCPListenerConfig(port, bind_addresses, listener_type, tls, http_config)


_MANHOLE_SETTINGS_SCHEMA = {
Expand Down
Loading