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?