Support UNIX sockets for HTTP interfaces

[adrianheine]

Should probably be implemented in a wrapper around

synapse/synapse/app/_base.py

Line 162 in 9482a84

def listen_tcp(bind_addresses, port, factory, reactor=reactor, backlog=50):

See https://twistedmatrix.com/documents/8.2.0/api/twisted.internet.interfaces.IReactorUNIX.html.

[richvdh]

support for what? the CS-API? I'm struggling to see how that would be useful.

[adrianheine]

It would be useful for people who don't want to open a localhost TCP port when they have a proxy in front of synapse.

[richvdh]

Fair enough. I wasn't aware that http-over-unix-sockets was even a thing.

[syndicalist]

+1 to this request. It's really common practice for app servers like gunicorn, to skip local networking and just use sockets between the frontend (like nginx) and the app server, e.g. http://docs.gunicorn.org/en/stable/deploy.html

[wvffle]

I'm also looking forward to see this getting implemented.

[ilmaisin]

This would be cool. I am not really a fan of using unprivileged TCP ports for loopbacks. Any unprivileged local user could seize the port if the server shuts down for a while. This issue could perhaps therefore be tagged with the "security" tag?

[1848]

This is a quick hack I am using currently.
Seems to work but I did not do any testing...

--- a/synapse/app/_base.py
+++ b/synapse/app/_base.py
@@ -163,7 +163,10 @@ def listen_tcp(bind_addresses, port, factory, reactor=reactor, backlog=50):
     r = []
     for address in bind_addresses:
         try:
-            r.append(reactor.listenTCP(port, factory, backlog, address))
+            if address[0] == "/":
+                r.append(reactor.listenUNIX(address, factory, backlog))
+            else:
+                r.append(reactor.listenTCP(port, factory, backlog, address))
         except error.CannotListenError as e:
             check_bind_error(e, address, bind_addresses)

[auscompgeek]

Nice hack.

It probably doesn't make a terrible amount of sense to require all the TCP options for a UNIX listener. Having thought about it for a bit, here's what I imagine would happen:

1. Change parse_listener_def and ListenerConfig in synapse.config.server for a listener config that only has path, type, and http.
   • Specifying any of the TCP options is an error.
   • Split the TCP fields from ListenerConfig into a TcpListenerConfig?
   • (We could make the manhole support UNIX sockets too, but baby steps.)
2. Change start_listening and _listen_http to take the new ListenerConfig and listen on TCP or a UNIX socket as appropriate.

If this seems sensible I might try working on this in the next few days.

[clokep]

That sounds like a reasonable approach on the face of it. Not sure how gnarly it'll end up being.

I suspect the worst part is figuring out how to do the configuration without bloating the config (and being backwards compatible).

[1848]

My suggestion for config modifications:

Dont Change:

• bind_address is good enough, no need to change it to path imo.

Drop (ignore or throw error if set):

• tls
• port

Add:

• bool: socket_abstract (useful?)
• int: socket_mode (socket permissions)
• str: socket_user (owning user, if process has privileges to chown it)
• str: socket_group

Another idea, allow to set ipaddrs and unix_paths side by side in bind_addresses...

[auscompgeek]

@1848 I don't see a good reason to allow listening on multiple paths and/or TCP sockets for a UNIX listener. Surely if you're specifying a UNIX socket to listen on then you intend on using a reverse proxy, right?

[1848]

@auscompgeek I am with you. I could imagine corner cases where it would make sense (e.g. multiple frontends, some of them unable to use unix sockets) but yeah, corner cases...

[kevincox]

Yeah, you could give your metric scraper access to one socket and your reverse proxy access to the others. I can definitely see the use cases.

Ideally it would be supported to pass these in from systemd (or similar runners) so that synapse doesn't need to worry about permissions and socket options and just let the service manager deal with that. This also allows hitless restarts, socket activation and other niceties.

[DMRobertson]

Do we consider this closed by #15353?

[realtyem]

Do we consider this closed by #15353?

Almost. There's a little more to do yet.
Edit: Docs for one, a little more testing hookup, and replication listener enablement.

Note for those watching, metrics won't happen at this time. That's a completely separate problem this won't help with.