[limiter] Rate limiting does not work

[mrpaulblack]

Version of SearXNG, commit number if you are using on master branch and stipulate if you forked SearXNG

Leons instance: 2022.05.24-fddbc5ed (39077fc0)
ALso my instance: 2022.05.08-85c1c14f 

How did you install SearXNG?

Using the scripts as well as with docker.

What happened?

The limiter plugin does not rate limit requests and uses the X-Forwarded-For header which could be easily set by a client request.

How To Reproduce
Make 15 /search requests in 20sec to trigger rate limiting.
Make another search request, which is going to get blocked with a HTTP 429 response.
Make another /search request which is going to get not blocked again.

Expected behavior
The IP should get blocked for a specific amount of time with a timeout value for example. So a offending host could get blocked for like 2 hours for example.

Screenshots & Logs

Additional context
Another problem is that the IP is read from a header. In a improper setup the IP could maybe not be in that header or a client could abuse this and set the header themselves. The IP should be gotten from a trusted source... (Maube the request lib has something to get the IP?)

[mrpaulblack]

for vis: @dalf @return42 @tiekoetter

[tiekoetter]

Tested on my instance :

IPv4:

• Refresh about 15 times -> 429
• Wait about 10 seconds -> 200
• Refresh about once a second after getting blocked initially -> 429

IPv6:

• Refresh about 15 times -> 429
• Wait about 10 seconds -> 200
• Refresh about once a second after getting blocked initially -> 429

[dalf]

Another problem is that the IP is read from a header.

• Caddy: https://caddyserver.com/docs/caddyfile/directives/reverse_proxy#defaults
• Traefik: https://doc.traefik.io/traefik/v2.1/routing/entrypoints/#forwarded-header

[return42]

How To Reproduce
Make 15 /search requests in 20sec to trigger rate limiting.
Make another search request, which is going to get blocked with a HTTP 429 response.
Make another /search request which is going to get not blocked again.

"is going to get not blocked again." .. I can't reproduce this behaviour .. in my tests, the sliding window works and blocks the following request .. @paul, can you please verify your test.

[mrpaulblack]

well yeah can you try with my instance? https://paulgo.io it happens there for me like this... (Tho I did not update my instance for over 2 weeks => so searxng container and redis is running for over 2 weeks now)

[return42]

About nginx -- from https://nginx.org/en/docs/http/ngx_http_proxy_module.html#variables

 $proxy_add_x_forwarded_for
    the “X-Forwarded-For” client request header field with the $remote_addr variable
    appended to it, separated by a comma. If the “X-Forwarded-For” field is not present
    in the client request header, the $proxy_add_x_forwarded_for variable is equal to
    the $remote_addr variable.

Does it mean the client can overwrite “X-Forwarded-For” ?

[return42]

well yeah can you try with my instance? https://paulgo.io/ it happens there for me like this...

I tested your instance and for me it works like @tiekoetter described "Refresh about once a second after getting blocked initially -> 429" .. what I mean; the sliding window works and blocks the following requests .. the limiter itself is IMO not buggy.

---

The issue might be that the limiter identifies the request origin by the entire string of:

searxng/searx/plugins/limiter.py

Line 42 in de0c4d7

x_forwarded_for = request.headers.get('X-Forwarded-For', '')

If the client set a “X-Forwarded-For” and change the value with each request, the entire string changes by each request.

[return42]

The leak is described here :-) ..

Any security-related use of X-Forwarded-For -- such as for rate limiting or IP-based access control -- must only use IP addresses added by a trusted proxy. Using untrustworthy values can result in rate-limiter avoidance, access-control bypass, memory exhaustion, or other negative security or availability consequences.

[ononoki1]

If the client set a “X-Forwarded-For” and change the value with each request, the entire string changes by each request.

I think it should be handled by web server like nginx, not backend application like searxng. For example, using set_real_ip_from directive properly or simply using $remote_addr as X-Forwarded-For header are both solutions.

---

It should be the web server's (or reverse proxy's) responsibility that the X-Forwarded-For header passed to backend application is reliable. SearXNG cannot and should not verify it.

[return42]

It should be the web server's (or reverse proxy's) responsibility that the X-Forwarded-For header passed to backend application is reliable. SearXNG cannot and should not verify it.

That is absolutely true .. but what when the proxy in front of the app is just one proxy in a chain of proxies .. at its best the first incoming HTTP server should fix (reset) the X-Forwarded-For header .. in most HTTP setups we will find something like this:

searxng/utils/templates/etc/nginx/default.apps-available/searxng.conf:filtron

Lines 8 to 9 in ea0cddb

	proxy_set_header X-Real-IP \$remote_addr;
	proxy_set_header X-Forwarded-For \$proxy_add_x_forwarded_for;

We (the developer) do not know how many proxies in front of the app so I thought it might be better we add a configuration option to select what position in the X-Forwarded-For identifies the IP of the request (client).