matrix-org / synapse

Synapse: Matrix homeserver written in Python/Twisted.
https://matrix-org.github.io/synapse
Apache License 2.0
11.83k stars 2.12k forks source link

Enable connecting to appservices through unix sockets #16399

Open Sir-Photch opened 1 year ago

Sir-Photch commented 1 year ago

Description:

When the URL provided in an appservice-registration is a unix socket of the schema unix:/path/to.sock, the server refuses to connect and reports "Unsupported schema: 'unix'".

For deployments where appservices are on the same machine as the server, it would make sense to enable this functionality.

(Since this seems to be handled specifically by the server, I assume it's not a bug.)

clokep commented 1 year ago

I wonder if this is even allowed by the spec, that defines the url as:

Required: The URL for the application service. May include a path after the domain name. Optionally set to null if no traffic is required.

It doesn't explicitly forbid it, but 🤷 anyway, it is likely fine.

realtyem commented 1 year ago

I am pretty unread into the appservice chapters of this book, but from what I'm understanding from out-of-band conversations with others is that it's probably not a good idea. I can get the SimpleHttpClient to speak to sockets like was done for ReplicationClient, but what happens when Synapse is using workers and if the endpoints it needs to talk to are load-balanced? Or sharded? (Does a bridge ever have to speak directly to a stream writer?) Since the reverse proxy handles which worker gets what work, it may be better to just 'not' do this. And like I said, I'm not fully read in to how they work so I could be completely off-base.

I have to admit though, it does sound attractive.

clokep commented 1 year ago

but what happens when Synapse is using workers and if the endpoints it needs to talk to are load-balanced? Or sharded?

I'm pretty sure this issue is about the Synapse to appservice communication, not the other way around.

realtyem commented 1 year ago

I've been giving this a look-over, and there is some information I think I'm missing to make sure I don't break anything.

Does any appservice:

EDIT: Also, I think treq which is part of the request machinery was hindering using 'relaxed' ascii for hostnames(in the most general case, for worker names). Would that be something useful/helpful?(Thinking of usage of things like docker compose/kubernetes here, not sure how strict the internal DNS of docker is, etc.)

EDIT 2: (deleted above) No that was ensureValidURI() that was restricting hostnames by IDNA2008. Still a valid question though.

realtyem commented 1 year ago

After a few days of poking/staring at the appservice code, I think it's fair to mark a few things off the list of requirements:

None of those seem to make sense when using a appservice. Which means treq doesn't need to be part of the equation. The question of connecting through a proxy remains on the table.

That being said, using the request() machinery of Twisted(as it's the most widely used in Synapse) means there are some restrictions on how to handle passing the filename of the unix socket into that machinery. I think the simplest method will be to append a : to the socket filename internally. It follows along with how Nginx(as a single example) allows for separating the path_to_socket_file(unix:/path/to.socket) and path_to_url_request(/_matrix/app/v1/whatever).

realtyem commented 1 year ago

And it looks like a proxy isn't going to happen/be needed as even the docs say so: https://matrix-org.github.io/synapse/latest/setup/forward_proxy.html#connection-types

(Thanks @clokep !!)