search

jitsi / docker-jitsi-meet

Jitsi Meet on Docker
https://hub.docker.com/u/jitsi/
Apache License 2.0
3k stars 1.34k forks source link

Prosody configuration does not set trusted proxies directive #1042

Open millaguie opened 3 years ago

millaguie commented 3 years ago

According to Prosody documentation trusted reverse proxies has to be set in order to be able to know the real IP address of a client. This is preventing the log_auth module to show the real IP address of the client as it always shows the IP address of the web server (who is acting as a proxy).

There is no easy way to solve this issue, as it depends on the IP address of another container, a workaround is to set a depends_on tag on the prosody container forcing it to start after the web service, and change the configuration script to resolve the web server and set that IP (or IPs) in trusted_proxies. Another option is patch the prosody http_module to accept CIDRs (currently it only accept a list of IP address) and set the docker CIDR as trusted.

Has somebody else face this problem?

Thanks!

saghul commented 3 years ago

Good point. I wonder if setting the trusted proxy to "*" would work.

bgrozev commented 3 years ago

Good point. I wonder if setting the trusted proxy to "*" would work.

This would allow users to bypass the rate limiting we have in prosody (I'm not sure it's currently used with docker). Can we pin down the address of the nginx container to a smaller network, e.g. 192.168.0.0/16?

saghul commented 3 years ago

In this setup clients never connect directly to prosody, it always goes through the web container.

Does your concern still apply in this scenario @bgrozev ?

bgrozev commented 3 years ago

@saghul yes, a client could just include the header in the original request, and prosody will trust it.

@millaguie I just remembered we added support for CIDR to prosody for just this reason. I don't know if it's made it to 0.11 or the version used here though.

saghul commented 3 years ago

@saghul yes, a client could just include the header in the original request, and prosody will trust it.

We'd set that header ourselves when proxying inwards, of course.

bgrozev commented 3 years ago

We'd set that header ourselves when proxying inwards, of course.

Most proxies are configured to append to it

saghul commented 3 years ago

I'm a bit confused :-) This setup has Nginx in front of Prosody. Prosody is not exposed directly, ever. If we set this header in Nginx and configure Prosody to trust anything, do you still have concerns?

bgrozev commented 3 years ago

No, unless there are other proxies in front. If there are, prosody will detect their address as the client address, and someone could easily get the address rate limited.

saghul commented 3 years ago

Gotcha.

millaguie commented 3 years ago

Hi,

Thanks @bgrozev!

@millaguie I just remembered we added support for CIDR to prosody for just this reason. I don't know if it's made it to 0.11 or the version used here though.

Yes, I think this is the proper way, but that code is not in 0.11.x only in trunk :(

saghul commented 3 years ago

Yes, I think this is the proper way, but that code is not in 0.11.x only in trunk :(

Ah, that's unfortunate. Once it lands in a release, holler and we'll update and make it configurable.