Rate limit with ip doesn't work on Django 3/Nginx/Gunicorn

[bzimor]

Ratelimit based on user ip doesn't work completely on Django 3 app. Initially, it raised an error due to the absence of REMOTE_ADDR and this error was fixed with a new patch on main branch. Although, it can access to the user ip, it doesn't block any over limit on production. However, it works when I run the project using runserver 0.0.0.0:8000.
I deployed my django app with the following configurations:
nginx:

upstream my_app_server {
    server unix:/home/projects/my_app/src/gunicorn.sock fail_timeout=0;
}

server {
    location / {
        proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for;
        proxy_set_header Host $http_host;
        proxy_set_header X-Real-IP $remote_addr;
        proxy_redirect off;
...

gunicorn:

#!/bin/bash

NAME="my_app"
DJANGODIR=/home/projects/my_app/src
SOCKFILE=/home/projects/my_app/src/gunicorn.sock 
NUM_WORKERS=5 
DJANGO_SETTINGS_MODULE=my_app.settings 
DJANGO_WSGI_MODULE=my_app.wsgi
TIMEOUT=300

echo "Starting $NAME as `whoami`"

cd $DJANGODIR
source ../env/bin/activate
export DJANGO_SETTINGS_MODULE=$DJANGO_SETTINGS_MODULE
export PYTHONPATH=$DJANGODIR:$PYTHONPATH

exec ../env/bin/gunicorn ${DJANGO_WSGI_MODULE}:application \\
--name $NAME \
--workers $NUM_WORKERS \
--timeout $TIMEOUT \
--bind=unix:$SOCKFILE \
--log-level=debug \
--log-file=-

I added this setting:

RATELIMIT_IP_META_KEY = 'HTTP_X_REAL_IP'

Also, I checked with PyMemcacheCache and LocMemCache, even I tried with custom middleware to change REMOTE_ADDR header, but no luck.
Any ideas?
Django version is 3.1.5.

[jsocol]

Do limits based on other fields such as user ID work?

[bzimor]

I checked, it doesn't work either.

[jsocol]

Thanks for checking. Can you share your cache settings?

You mentioned LocMemCache That definitely shouldn’t be used in production since it’s far too volatile and not shared across processes or hosts.

[bzimor]

I think I fixed my issue, before I used PyMemecacheCache with this setting:

CACHES = {
    'default': {
        'BACKEND': 'djpymemcache.backend.PyMemcacheCache',
        'LOCATION': [
           '127.0.0.1:11211',
        ],
    },

Then I found one Stackoverflow question about that multiple gunicorn workers can have their own cache dict and because of it they cannot share the saved values. So, I changed my Memcached config to unix socket with following setting:

'default': {
        'BACKEND': 'django.core.cache.backends.memcached.MemcachedCache',
        'LOCATION': 'unix:/tmp/memcached.sock',
    },

Then it worked. Thanks for your attention

UPD. Sorry, actually it didn't fix my issue anyway.

This is my memcached configuration:

-m 256
-u memcache
-P /var/run/memcached/memcached.pid
-s /tmp/memcached.sock
-a 0766

[SimonHurst]

Silly question @bzimor : but are you setting block=True in the decorator.

I only ask as I found your issue when trying to see why the rate limiter didn't work for me. Not sure why True in this instance isn't the default as otherwise it appears to just do nothing and I'm not sure of the point! If you've already set that then, ignore my comment and I'll let someone with more knowledge go ;)

[bzimor]

Thanks for your advice. Strange, if I use block=True, it ignores the limit and it always shows 403 error whenever I access that url.

[SimonHurst]

Can you show what you’re setting for the view?

…

On Sun, 31 Jan 2021 at 13:04, Boburmirzo Hamraqulov < ***@***.***> wrote: Thanks for your advice. Strange, if I use block=True, it ignores the limit and it always shows 403 error whenever I access that url. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#222 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAIDNATHVRDI4KXDTAYT623S4VIPTANCNFSM4WTAJWMQ> .

[jsocol]

Huh wow I’m going to be honest, I haven’t been working in Django lately and the default has been block=False for so long that I’d completely forgotten about it.

Digging way back into my memory, I believe the intent was twofold

1. if you block on the first rate in a stack of @ratelimits, the subsequent ones don’t get checked or counted. That can be surprising.
2. if you block on the wrong type of field, like key='post:username' on a login field (which is a good way to limit login attempts) you can create a denial of service vector where I can lock someone else out of their account. These are good use cases for a captcha.

But thinking about it now, with eyes that haven’t seen it in a while, I feel like block=False is more surprising as a default and from what I can tell the login form use case is not the 80% use case.

Obviously this would be a major API change and require a major version bump, but we may need to do v4 anyway to namespace the package.

[bzimor]

@SimonHurst , this is my view:

@ratelimit(key='ip', rate='10/m', block=True)
def test_view(request):
    s = request.META.get('HTTP_X_REAL_IP')
    return HttpResponse('your ip is ' + s)

[SimonHurst]

You might want to set a value for m. '200/<n>m' n being the number of minutes.

…

On Sun, 31 Jan 2021 at 16:25, Boburmirzo Hamraqulov < ***@***.***> wrote: @SimonHurst <https://github.com/SimonHurst> , this is my view: @ratelimit(key='ip', rate='200/m', block=True) def test_view(request): s = request.META.get('HTTP_X_REAL_IP') return HttpResponse('your ip is ' + s) — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#222 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAIDNAVBZELEH3DI7PCUSQLS4WAAHANCNFSM4WTAJWMQ> .

[bzimor]

I logged ratelimit variables on core.py and I got this:

user_ip: 162.158.183.130
method: my_app.views.test_view
cache_name: default
rate: 10/m
cache_key: rl:22f71a140f65c22dfdf73b15b85c4f2c
period: 60
limit: 10
count: None

I found that I got blocked because count is None. It means that cache cannot be found by key...

[SimonHurst]

Sorry deleted that last comment, didn't read it properly.

I would still be explicit:

@ratelimit(key='ip', rate='10/1m', block=True)

Only because it's a little more obvious if someone else has tor read it later.

My next question is what is your caching policy on that view?

[jsocol]

To keep issues cleaner but still help with debugging, I’m going to move this to Discussions (it’ll be the very first one!)

[jsocol]

Can you go into the Django shell and write/read something to/from the cache? It seems like it’s still having issues there.

If you’re running memcached with -s, it disables TCP/UDP network interfaces, so the PyMemcached settings weren’t able to connect to memcached at all. It looks like your other settings should work, but a couple of things could be happening. It’s possible the file permissions on the socket are wrong (does it need +x?). Or that the file is getting cleaned up since it’s in /temp/ (a lot of times sockets end up somewhere in /var/run).

Given that it’s not finding a count at all, it really sounds to me like the cache connection doesn’t work.

[bzimor]

Thank you, I changed python memcached lib , then I checked with django shell and now I can set and get cache by key. Hovewer, I have another problem now. This is from my custom ratelimit log:

user_ip: 162.158.183.128
method: my_app.views.test_view2
cache_name: default
rate: 20/m
cache_key: rl:181ec4f6cb60cbadac89d3426b4e1038
period: 60
limit: 20
count: 1

user_ip: 162.158.183.128
method: my_app.views.test_view2
cache_name: default
rate: 20/m
cache_key: rl:181ec4f6cb60cbadac89d3426b4e1038
period: 60
limit: 20
count: 1

You can see the same keys but no count increment. I checked that key, its value was 1. What can be wrong then? I thought this was because of multiple workers of gunicorn, but I am not sure.

[jsocol]

Can I ask where you put this instrumentation in core.py? It’s hard to know what to expect without knowing when/where you’re logging

[bzimor]

Inside the get_usage() method, after this line

django-ratelimit/django_ratelimit/core.py

Line 215 in 6972fa2

count = None

:

...
cache_name = getattr(settings, 'RATELIMIT_USE_CACHE', 'default')
    cache = caches[cache_name]
    cache_key = _make_cache_key(group, window, rate, value, method)
    log_ratelimit('\nuser_ip: ', value)
    log_ratelimit('old_key_value: ', cache.get(cache_key))
    

    count = None
    added = cache.add(cache_key, initial_value, period + EXPIRATION_FUDGE)
    log_ratelimit('added: ', added)
    if added:
        count = initial_value
    else:
        if increment:
            try:
                # python3-memcached will throw a ValueError if the server is
                # unavailable or (somehow) the key doesn't exist. redis, on the
                # other hand, simply returns None.
                count = cache.incr(cache_key)
            except ValueError:
                pass
        else:
            count = cache.get(cache_key, initial_value)

    
    log_ratelimit('method: ', group)
    log_ratelimit('cache_name: ', cache_name)
    log_ratelimit('rate: ', rate)
    log_ratelimit('cache_key: ', cache_key)
    log_ratelimit('period: ', period)
    log_ratelimit('limit: ', limit)
    log_ratelimit('count: ', count)
    # Getting or setting the count from the cache failed
    if count is None:
...

I found another interesting moment, although log_ratelimit('old_key_value: ', cache.get(cache_key)) logs 1 (which means key already exists), cache.add() returns True, which means it can override the old one i think

[bzimor]

I checked again, even there is a record with a key, cache.add() returns true. So, I added condition to check the key exists or not and it works well:

    count = None
    if cache.get(cache_key):
        added = False
    else:
        added = cache.add(cache_key, initial_value, period + EXPIRATION_FUDGE)
    log_ratelimit('added: ', added)
    if added:
        count = initial_value
    else:
        if increment:
            try:
                # python3-memcached will throw a ValueError if the server is
                # unavailable or (somehow) the key doesn't exist. redis, on the
                # other hand, simply returns None.
                count = cache.incr(cache_key)
            except ValueError:
                pass
        else:
            count = cache.get(cache_key, initial_value)

According to the documentation, cache.add() should return True if the value was stored, False otherwise, but in my code it returns True in both cases:

user_ip: 162.158.183.198
old_key: None
added: True
method: my_app.views.test_view2
cache_name: default
rate: 20/h
cache_key: rl:75cbef6de1bebb129aa70c4b125540e5
period: 3600
limit: 20
count: 1

user_ip: 162.158.183.198
old_key: 1
added: True
method: my_app.views.test_view2
cache_name: default
rate: 20/h
cache_key: rl:75cbef6de1bebb129aa70c4b125540e5
period: 3600
limit: 20
count: 1

[bzimor]

Strange, this is the result from django shell: