whitelist_ratelimiter: @dalf patch

- it calls ip_address and ip_network only once for the settings.
- init_whitelist to make tests easier
- get_remote_addr get the IP address for the function is_accepted_request
- for reference: https://esd.io/blog/flask-apps-heroku-real-ip-spoofing.html

manage

@@ -698,6 +698,7 @@ test.pylint() {
         build_msg TEST "[pylint] searx tests"
         python ${PYLINT_OPTIONS} ${PYLINT_VERBOSE} \
             --disable="${PYLINT_SEARXNG_DISABLE_OPTION}" \
+            --additional-builtins="${PYLINT_ADDITIONAL_BUILTINS_FOR_ENGINES}" \
 	    --ignore=searx/engines \
 	    searx tests
     )

searx/plugins/limiter.py

@@ -5,9 +5,9 @@
 
 To monitor rate limits and protect privacy the IP addresses are getting stored
 with a hash so the limiter plugin knows who to block.  A redis database is
-needed to store the hash values. 
+needed to store the hash values.
 
-It is also possible to bypass the limiter for a specific IP address or subnet 
+It is also possible to bypass the limiter for a specific IP address or subnet
 using the `whitelist_ip` and `whitelist_subnet` settings.
 
 Enable the plugin in ``settings.yml``:
@@ -23,6 +23,7 @@ Enable the plugin in ``settings.yml``:
 
 import ipaddress
 import re
+from typing import List, cast
 from flask import request
 
 from searx import get_setting, redisdb
@@ -45,35 +46,37 @@ re_bot = re.compile(
 )
 
 
-WHITELISTED_IPS = get_setting('server.limiter_whitelist_ip', default=[])
-WHITELISTED_SUBNET = get_setting('server.limiter_whitelist_subnet', default=[])
+WHITELISTED_IPS = []
+WHITELISTED_SUBNET = []
 
 
-def is_whitelist_ip(ip: str) -> bool:
+def is_whitelist_ip(ip_str: str) -> bool:
     """Check if the given IP address belongs to the whitelisted list of IP addresses or subnets."""
     # if ip is empty use the source ip
-    if ip == "" or ip is None:
-        ip = request.remote_addr or ""
-    logger.debug("checking whitelist rules for: %s", ip)
-    whitelisted = False
     try:
-        whitelisted = ip in WHITELISTED_IPS or any(
-            ipaddress.ip_address(ip) in ipaddress.ip_network(subnet) for subnet in WHITELISTED_SUBNET
-        )
+        ip_a = ipaddress.ip_address(ip_str)
     except ValueError as e:
         logger.error("Error while checking ratelimiter whitelist: %s", e)
+        return False
+    return ip_a in WHITELISTED_IPS or any(ip_a in subnet for subnet in WHITELISTED_SUBNET)
 
-    return whitelisted
+
+def get_remote_addr() -> str:
+    x_forwarded_for = request.headers.getlist('X-Forwarded-For')
+    if len(x_forwarded_for) > 0:
+        return x_forwarded_for[-1]
+    return request.remote_addr or ''
 
 
 def is_accepted_request() -> bool:
     # pylint: disable=too-many-return-statements
     redis_client = redisdb.client()
     user_agent = request.headers.get('User-Agent', '')
-    x_forwarded_for = request.headers.get('X-Forwarded-For', '')
+    remote_addr = get_remote_addr()
 
     # if the request source ip belongs to the whitelisted list of ip addresses or subnets
-    if is_whitelist_ip(x_forwarded_for):
+    if is_whitelist_ip(remote_addr):
+        logger.debug("whitelist IP")
         return True
 
     if request.path == '/image_proxy':
@@ -82,8 +85,8 @@ def is_accepted_request() -> bool:
         return True
 
     if request.path == '/search':
-        c_burst = incr_sliding_window(redis_client, 'IP limit, burst' + x_forwarded_for, 20)
-        c_10min = incr_sliding_window(redis_client, 'IP limit, 10 minutes' + x_forwarded_for, 600)
+        c_burst = incr_sliding_window(redis_client, 'IP limit, burst' + remote_addr, 20)
+        c_10min = incr_sliding_window(redis_client, 'IP limit, 10 minutes' + remote_addr, 600)
         if c_burst > 15 or c_10min > 150:
             logger.debug("to many request")  # pylint: disable=undefined-variable
             return False
@@ -110,7 +113,7 @@ def is_accepted_request() -> bool:
             return False
 
         if request.args.get('format', 'html') != 'html':
-            c = incr_sliding_window(redis_client, 'API limit' + x_forwarded_for, 3600)
+            c = incr_sliding_window(redis_client, 'API limit' + remote_addr, 3600)
             if c > 4:
                 logger.debug("API limit exceeded")  # pylint: disable=undefined-variable
                 return False
@@ -123,6 +126,20 @@ def pre_request():
     return None
 
 
+def init_whitelist(limiter_whitelist_ip: List[str], limiter_whitelist_subnet: List[str]):
+    global WHITELISTED_IPS, WHITELISTED_SUBNET  # pylint: disable=global-statement
+    if isinstance(limiter_whitelist_ip, str):
+        limiter_whitelist_ip = [limiter_whitelist_ip]
+    if isinstance(limiter_whitelist_subnet, str):
+        limiter_whitelist_subnet = [limiter_whitelist_subnet]
+    if not isinstance(limiter_whitelist_ip, list):
+        raise ValueError('server.limiter_whitelist_ip is not a list')
+    if not isinstance(limiter_whitelist_subnet, list):
+        raise ValueError('server.limiter_whitelist_subnet is not a list')
+    WHITELISTED_IPS = [ipaddress.ip_address(ip) for ip in limiter_whitelist_ip]
+    WHITELISTED_SUBNET = [ipaddress.ip_network(subnet, strict=False) for subnet in limiter_whitelist_subnet]
+
+
 def init(app, settings):
     if not settings['server']['limiter']:
         return False
@@ -131,5 +148,10 @@ def init(app, settings):
         logger.error("The limiter requires Redis")  # pylint: disable=undefined-variable
         return False
 
+    init_whitelist(
+        cast(list, get_setting('server.limiter_whitelist_ip', default=[])),
+        cast(list, get_setting('server.limiter_whitelist_subnet', default=[])),
+    )
+
     app.before_request(pre_request)
     return True

searx/settings_defaults.py

@@ -174,8 +174,8 @@ SCHEMA = {
         'port': SettingsValue((int, str), 8888, 'SEARXNG_PORT'),
         'bind_address': SettingsValue(str, '127.0.0.1', 'SEARXNG_BIND_ADDRESS'),
         'limiter': SettingsValue(bool, False),
-        'limiter_whitelist_ip': SettingsValue(list, []),
-        'limiter_whitelist_subnet': SettingsValue(list, []),
+        'limiter_whitelist_ip': SettingsValue((str, list), []),
+        'limiter_whitelist_subnet': SettingsValue((str, list), []),
         'secret_key': SettingsValue(str, environ_name='SEARXNG_SECRET'),
         'base_url': SettingsValue((False, str), False, 'SEARXNG_BASE_URL'),
         'image_proxy': SettingsValue(bool, False),

tests/unit/test_plugins.py

@@ -171,8 +171,7 @@ class LimiterPluginTest(SearxTestCase):
         self.assertTrue(len(store.plugins) == 1)
 
         def test_whitelist_case(case):
-            plugins.limiter.WHITELISTED_SUBNET = case[1]['whitelist_subnet']
-            plugins.limiter.WHITELISTED_IPS = case[1]['whitelist_ip']
+            plugins.limiter.init_whitelist(case[1]['whitelist_ip'], case[1]['whitelist_subnet'])
             ret = store.call(store.plugins, 'is_whitelist_ip', case[0])
             self.assertEqual(ret, case[2])
 
@@ -186,12 +185,6 @@ class LimiterPluginTest(SearxTestCase):
             )
         )
 
-        # not an ip
-        test_cases.append(('192.0.43.22', {'whitelist_ip': 'not an ip', 'whitelist_subnet': []}, False))
-
-        # not a subnet
-        test_cases.append(('192.0.43.22', {'whitelist_ip': [], 'whitelist_subnet': 'not a subnet'}, False))
-
         # test single ip
         test_cases.append(('192.0.43.22', {'whitelist_ip': '192.0.43.22', 'whitelist_subnet': []}, True))
 
@@ -211,3 +204,11 @@ class LimiterPluginTest(SearxTestCase):
 
         for case in test_cases:
             test_whitelist_case(case)
+
+        # not an ip
+        with self.assertRaises(ValueError):
+            test_whitelist_case(('192.0.43.22', {'whitelist_ip': ['not an ip'], 'whitelist_subnet': []}, False))
+
+        # not a subnet
+        with self.assertRaises(ValueError):
+            test_whitelist_case(('192.0.43.22', {'whitelist_ip': [], 'whitelist_subnet': ['not a subnet']}, False))