diff --git a/api/api/settings.py b/api/api/settings.py index 0b72db231..ecabc1917 100644 --- a/api/api/settings.py +++ b/api/api/settings.py @@ -95,7 +95,7 @@ "ALLOWED_VERSIONS": ["v1", "v2"], "DEFAULT_THROTTLE_CLASSES": [ "desecapi.throttling.ScopedRatesThrottle", - "rest_framework.throttling.UserRateThrottle", + "desecapi.throttling.UserRateThrottle", ], "DEFAULT_THROTTLE_RATES": { # When changing rate limits, make sure to keep docs/rate-limits.rst consistent # ScopedRatesThrottle @@ -124,7 +124,7 @@ "300/d", ], # DNS API requests affecting RRset(s) of a single domain # UserRateThrottle - "user": "10000/d", # hard limit on requests by a) an authenticated user, b) an unauthenticated IP address + "user": "2000/d", # hard limit on requests by a) an authenticated user, b) an unauthenticated IP address }, "NUM_PROXIES": 0, # Do not use X-Forwarded-For header when determining IP for throttling } diff --git a/api/api/settings_quick_test.py b/api/api/settings_quick_test.py index bc394f1cd..513073b30 100644 --- a/api/api/settings_quick_test.py +++ b/api/api/settings_quick_test.py @@ -30,9 +30,7 @@ } REST_FRAMEWORK["PAGE_SIZE"] = 20 -REST_FRAMEWORK["DEFAULT_THROTTLE_CLASSES"] = [ - "rest_framework.throttling.UserRateThrottle" -] +REST_FRAMEWORK["DEFAULT_THROTTLE_CLASSES"] = ["desecapi.throttling.UserRateThrottle"] REST_FRAMEWORK["DEFAULT_THROTTLE_RATES"] = {"user": "1000/s"} # Carry email backend connection over to test mail outbox diff --git a/api/desecapi/management/commands/stop-abuse.py b/api/desecapi/management/commands/stop-abuse.py index 97c13e6f1..dc3f3655e 100644 --- a/api/desecapi/management/commands/stop-abuse.py +++ b/api/desecapi/management/commands/stop-abuse.py @@ -47,10 +47,11 @@ def handle(self, *args, **options): ).exists(): try: blocked_subnet = BlockedSubnet.from_ip(rr.content) - except dns.resolver.NXDOMAIN: # for unallocated IP addresses + except dns.resolver.NXDOMAIN: # IP address unallocated/private continue - blocked_subnet.save() - blocked_subnets.append(blocked_subnet) + if not blocked_subnet.subnet.is_private: + blocked_subnet.save() + blocked_subnets.append(blocked_subnet) # Print summary print( diff --git a/api/desecapi/migrations/0038_user_throttle_daily_rate.py b/api/desecapi/migrations/0038_user_throttle_daily_rate.py new file mode 100644 index 000000000..a36e20392 --- /dev/null +++ b/api/desecapi/migrations/0038_user_throttle_daily_rate.py @@ -0,0 +1,18 @@ +# Generated by Django 5.0.6 on 2024-06-17 21:19 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("desecapi", "0037_remove_tokendomainpolicy_perm_dyndns"), + ] + + operations = [ + migrations.AddField( + model_name="user", + name="throttle_daily_rate", + field=models.PositiveIntegerField(null=True), + ), + ] diff --git a/api/desecapi/models/abuse.py b/api/desecapi/models/abuse.py index ab36ae958..ed19777b7 100644 --- a/api/desecapi/models/abuse.py +++ b/api/desecapi/models/abuse.py @@ -29,11 +29,16 @@ def from_ip(cls, ip): qname = IPv4Address(ip).reverse_pointer.replace( "in-addr.arpa", "origin.asn.cymru.com" ) - answer = dns.resolver.resolve(qname, "TXT")[0] - parts = str(answer).strip('"').split("|") + try: + answer = dns.resolver.resolve(qname, "TXT")[0] + parts = str(answer).strip('"').split("|") + except dns.resolver.LifetimeTimeout: + # In over a year of operation, there was never a smaller network than /24 + print(f"Could not determine ASN and subnet for {ip}, using 0 and /24") + parts = ["0", f"{ip}/24", "", "", str(date.today())] return cls( asn=int(parts[0].strip()), - subnet=IPv4Network(parts[1].strip()), + subnet=IPv4Network(parts[1].strip(), strict=False), country=parts[2].strip(), registry=parts[3].strip(), allocation_date=date.fromisoformat(parts[4].strip()), diff --git a/api/desecapi/models/users.py b/api/desecapi/models/users.py index 5effa0669..884640ccb 100644 --- a/api/desecapi/models/users.py +++ b/api/desecapi/models/users.py @@ -49,6 +49,7 @@ def _limit_domains_default(): ) needs_captcha = models.BooleanField(default=True) outreach_preference = models.BooleanField(default=True) + throttle_daily_rate = models.PositiveIntegerField(null=True) objects = MyUserManager() diff --git a/api/desecapi/tests/test_throttling.py b/api/desecapi/tests/test_throttling_scoped.py similarity index 85% rename from api/desecapi/tests/test_throttling.py rename to api/desecapi/tests/test_throttling_scoped.py index 57bc76eba..a67524657 100644 --- a/api/desecapi/tests/test_throttling.py +++ b/api/desecapi/tests/test_throttling_scoped.py @@ -1,3 +1,4 @@ +from contextlib import contextmanager from unittest import mock import time @@ -9,13 +10,14 @@ from rest_framework.test import APIRequestFactory -def override_rates(rates): - return override_settings( - REST_FRAMEWORK={ - "DEFAULT_THROTTLE_CLASSES": ["desecapi.throttling.ScopedRatesThrottle"], - "DEFAULT_THROTTLE_RATES": {"test_scope": rates}, - } - ) +@contextmanager +def override_bucket(bucket): + old_bucket = getattr(MockView, "throttle_scope_bucket", None) + MockView.throttle_scope_bucket = bucket + try: + yield + finally: + MockView.throttle_scope_bucket = old_bucket class MockView(APIView): @@ -65,12 +67,14 @@ def do_test(): cache.clear() request = self.factory.get("/") - with override_rates(rates): + with override_settings( + REST_FRAMEWORK={"DEFAULT_THROTTLE_RATES": {MockView.throttle_scope: rates}} + ): do_test() if buckets is not None: for bucket in buckets: - MockView.throttle_scope_bucket = bucket - do_test() + with override_bucket(bucket): + do_test() def test_requests_are_throttled_4sec(self): self._test_requests_are_throttled(["4/sec"], [(0, 4, 1), (1, 4, 1)]) diff --git a/api/desecapi/tests/test_throttling_user.py b/api/desecapi/tests/test_throttling_user.py new file mode 100644 index 000000000..4b5a99cd7 --- /dev/null +++ b/api/desecapi/tests/test_throttling_user.py @@ -0,0 +1,79 @@ +from unittest import mock +import time + +from django.core.cache import cache +from django.test import TestCase, override_settings +from rest_framework import status +from rest_framework.response import Response +from rest_framework.views import APIView +from rest_framework.test import APIRequestFactory, force_authenticate + +from desecapi.models import User + + +class MockView(APIView): + @property + def throttle_classes(self): + # Need to import here so that the module is only loaded once the settings override is in effect + from desecapi.throttling import UserRateThrottle + + return (UserRateThrottle,) + + def get(self, request): + return Response("foo") + + +class ThrottlingTestCase(TestCase): + """ + Based on DRF's test_throttling.py. + """ + + def setUp(self): + super().setUp() + self.factory = APIRequestFactory() + + def _test_requests_are_throttled(self, counts, user=None): + cache.clear() + request = self.factory.get("/") + if user is not None: + force_authenticate(request, user=user) + with override_settings( + REST_FRAMEWORK={"DEFAULT_THROTTLE_RATES": {"user": "10/d"}} + ): + view = MockView.as_view() + sum_delay = 0 + for delay, count, max_wait in counts: + sum_delay += delay + with mock.patch( + "desecapi.throttling.UserRateThrottle.timer", + return_value=time.time() + sum_delay, + ): + for _ in range(count): + response = view(request) + self.assertEqual(response.status_code, status.HTTP_200_OK) + + response = view(request) + self.assertEqual( + response.status_code, status.HTTP_429_TOO_MANY_REQUESTS + ) + self.assertTrue( + max_wait - 1 <= float(response["Retry-After"]) <= max_wait + ) + + def test_requests_are_throttled_unauthenticated(self): + self._test_requests_are_throttled( + [(0, 10, 86400), (86399, 0, 1), (1, 10, 86400)] + ) + + def test_requests_are_throttled_user(self): + for email, throttle_daily_rate in [ + ("foo@bar.com", None), + ("foo@bar.net", 3), + ("foo@bar.org", 30), + ]: + user = User.objects.create_user( + email=email, password="", throttle_daily_rate=throttle_daily_rate + ) + self._test_requests_are_throttled( + [(0, throttle_daily_rate or 10, 86400)], user=user + ) diff --git a/api/desecapi/throttling.py b/api/desecapi/throttling.py index 695ab7aae..d48573597 100644 --- a/api/desecapi/throttling.py +++ b/api/desecapi/throttling.py @@ -76,3 +76,31 @@ def THROTTLE_RATES(self): def get_cache_key(self, request, view): key = super().get_cache_key(request, view) return [f"{key}_{duration}" for duration in self.duration] + + +class UserRateThrottle(throttling.UserRateThrottle): + """ + Like DRF's UserRateThrottle, but supports individual rates per user. + """ + + def __init__(self): + pass # defer to allow_request() where request object is available + + def allow_request(self, request, view): + self.request = request + super().__init__() # gets and parses rate + return super().allow_request(request, view) + + def get_rate(self): + try: + return f"{self.request.user.throttle_daily_rate:d}/d" + except ( + AttributeError, # request.user is AnonymousUser + TypeError, # .throttle_daily_rate is None + ): + return super().get_rate() + + # Override the static attribute of the parent class so that we can dynamically apply override settings for testing + @property + def THROTTLE_RATES(self): + return api_settings.DEFAULT_THROTTLE_RATES diff --git a/api/requirements.txt b/api/requirements.txt index 067671b8f..53d59b366 100644 --- a/api/requirements.txt +++ b/api/requirements.txt @@ -1,9 +1,9 @@ captcha~=0.5.0 celery~=5.4.0 -coverage~=7.5.1 -cryptography~=42.0.6 +coverage~=7.5.4 +cryptography~=42.0.8 Django~=5.0.6 -django-cors-headers~=4.3.1 +django-cors-headers~=4.4.0 djangorestframework~=3.14.0 django-celery-email~=3.0.0 django-netfields~=1.3.2 @@ -16,5 +16,5 @@ psycopg~=3.1.19 psl-dns~=1.1.0 pylibmc~=1.6.3 pyyaml~=6.0.1 -requests~=2.31.0 -uwsgi~=2.0.25 +requests~=2.32.3 +uwsgi~=2.0.26 diff --git a/test/e2e2/spec/test_www.py b/test/e2e2/spec/test_www.py index 6be9d15cf..2288bdb65 100644 --- a/test/e2e2/spec/test_www.py +++ b/test/e2e2/spec/test_www.py @@ -85,7 +85,8 @@ def test_unknown_hosts(api_anon, protocol, hostname): assert 'RemoteDisconnected' in str(excinfo) -def test_security_headers(api_anon): +@pytest.mark.parametrize("url", [https_url + relpath for relpath in ('', 'signup')]) +def test_security_headers(api_anon, url): api_anon.headers = {} # CSP hashes are for legacy browser support. # Source: https://github.com/vitejs/vite/tree/v5.0.10/packages/plugin-legacy#content-security-policy @@ -101,7 +102,7 @@ def test_security_headers(api_anon): 'Referrer-Policy': 'strict-origin-when-cross-origin', 'X-XSS-Protection': '1; mode=block', } - response = api_anon.get(https_url) + response = api_anon.get(url) for k, v in expected_headers.items(): assert response.headers[k] == v diff --git a/www/conf/sites-available/90-desec.static.location b/www/conf/sites-available/90-desec.static.location index cc4d570f3..e7f0fd4fb 100644 --- a/www/conf/sites-available/90-desec.static.location +++ b/www/conf/sites-available/90-desec.static.location @@ -8,7 +8,7 @@ location / { gzip on; gzip_types *; - location /index.html { # Also includes / via internal redirect, see https://nginx.org/en/docs/http/ngx_http_index_module.html#index + location /index.html { expires epoch; add_header Strict-Transport-Security "max-age=31536000; includeSubDomains; preload" always; # CSP hashes are for legacy browser support. @@ -21,8 +21,8 @@ location / { } location / { # all other files - index index.html; - try_files $uri $uri/ /index.html =404; + index index.html; # causes internal redirect, i.e. above location applies + try_files $uri $uri/ /index.html; # only last parameter causes internal redirect expires 1M; } }