superseriousbusiness / gotosocial

Fast, fun, small ActivityPub server.
https://docs.gotosocial.org
GNU Affero General Public License v3.0
3.61k stars 304 forks source link

[bug] Rate limiting fails to group by IP with reverse proxying #1125

Closed dequis closed 1 year ago

dequis commented 1 year ago

Describe the bug with a clear and concise description of what the bug is.

Every client in a reverse proxy setup is seen as the same IP leading to rate limiting blocking requests as soon as 1000 requests from anyone are received within 5 minutes.

What's your GoToSocial Version?

0.5.1-SNAPSHOT git-81c1fe0

GoToSocial Arch

x86_64 docker

Browser version

No response

What happened?

My single-user instance (with docker+nginx) returned http 429 when i tried to use it, and it looks like every request in the logs comes from clientIP=172.19.0.1.

The docs for nginx proxying do have this line:

proxy_set_header X-Forwarded-For $remote_addr;

But it appears to be unused.

Note that usage of x-forwarded-for should be explicitly opt-in in the config of gotosocial, otherwise instances with no reverse proxies would blindly trust a header specified by users.

Since as far as I can tell this config doesn't exist, this is currently failing in the more secure way (rejecting more requests than it should)

What you expected to happen?

clientIP says more than that IP. A config for enabling x-forwarded-for exists.

How to reproduce it?

Have an instance with a reverse proxy setup and use it.

Anything else we need to know?

The apache and caddy docs lack x-forwarded-for entirely.

tsmethurst commented 1 year ago

check the trusted-proxies value in the configuration yaml ;)

dequis commented 1 year ago

Oh excellent, then this is a documentation issue

tsmethurst commented 1 year ago

then this is a documentation issue

Yeah, I think we need to add a line in each of the reverse proxy installation guides about setting trusted-proxies correctly. Fancy making a pr? :D

dequis commented 1 year ago

There, https://github.com/superseriousbusiness/gotosocial/pull/1127

dequis commented 1 year ago

1127 merged!