diff --git a/CHANGES.rst b/CHANGES.rst index dd65538..a133234 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -2,6 +2,15 @@ Changes ======= +0.9.2 +----- + + * Handle IPv6 in client_addr tween. ClientAddr tween was crashing when getting + IPv6 addresses. Use IP version agnostic parsing. Fetch the list of + CloudFlare's IPv6 IPs. + [am-on] + + 0.9.1 ----- diff --git a/pyproject.toml b/pyproject.toml index bb1a80f..8a72999 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [tool.poetry] name = "pyramid_heroku" -version = "0.9.1" +version = "0.9.2" description = "A bunch of helpers for successfully running Pyramid on Heroku." readme = "README.rst" include = [ "CHANGES.rst", "COPYING.txt" ] diff --git a/pyramid_heroku/client_addr.py b/pyramid_heroku/client_addr.py index eb0b8c2..f2a9ce7 100644 --- a/pyramid_heroku/client_addr.py +++ b/pyramid_heroku/client_addr.py @@ -1,6 +1,6 @@ """Set client_addr IP that we can trust.""" -from ipaddress import IPv4Address -from ipaddress import IPv4Network +from ipaddress import ip_address +from ipaddress import ip_network import logging import requests @@ -41,6 +41,14 @@ class ClientAddr(object): def __init__(self, handler, registry): self.handler = handler self.registry = registry + + if self.registry.settings.get("pyramid_heroku.structlog"): + import structlog + + self.logger = structlog.getLogger(__name__) + else: + self.logger = logging.getLogger(__name__) + self.ignored_ip_networks = self.get_cloudflare_ip_networks() def __call__(self, request): @@ -56,26 +64,33 @@ def __call__(self, request): def remove_ignored_ips(self, ips): """Remove ignored IPs from the list.""" for ip in ips: + try: + address = ip_address(ip) + except ValueError as e: + self.logger.info("Ignoring invalid IP address", exc_info=e) + continue + is_ignored = any( - [IPv4Address(ip) in network for network in self.ignored_ip_networks] + [address in network for network in self.ignored_ip_networks] ) if not is_ignored: yield ip - def get_cloudflare_ip_networks(self): - """Get the list of Cloudflare's current IP ranges.""" + def get_ip_networks_from_url(self, url): + """Get the list of IP networks from given url.""" try: - res = requests.get("https://www.cloudflare.com/ips-v4", timeout=10) + res = requests.get(url, timeout=10) res.raise_for_status() - return [IPv4Network(n) for n in res.text.split()] + return [ip_network(n) for n in res.text.split()] except Exception as e: - if self.registry.settings.get("pyramid_heroku.structlog"): - import structlog - - logger = structlog.getLogger(__name__) - else: - - logger = logging.getLogger(__name__) - logger.exception("Failed getting a list of Cloudflare IPs", exc_info=e) + self.logger.exception( + f"Failed getting a list of IPs from {url}", exc_info=e + ) return [] + + def get_cloudflare_ip_networks(self): + """Get the list of Cloudflare's current IP ranges.""" + ipv4 = self.get_ip_networks_from_url("https://www.cloudflare.com/ips-v4") + ipv6 = self.get_ip_networks_from_url("https://www.cloudflare.com/ips-v6") + return ipv4 + ipv6 diff --git a/pyramid_heroku/tests/test_client_addr.py b/pyramid_heroku/tests/test_client_addr.py index 50ef891..b349f2a 100644 --- a/pyramid_heroku/tests/test_client_addr.py +++ b/pyramid_heroku/tests/test_client_addr.py @@ -26,16 +26,23 @@ def setUp(self): self.request = request.Request({}) self.handler = mock.Mock() - self.registry = None + self.registry = mock.Mock() + self.registry.settings = {} self.responses = responses.RequestsMock() self.responses.start() self.responses.add( responses.GET, - "https://www.cloudflare.com/ips-v4", # noqa + "https://www.cloudflare.com/ips-v4", status=200, body="8.8.8.0/24\n9.9.9.0/24", ) + self.responses.add( + responses.GET, + "https://www.cloudflare.com/ips-v6", + status=200, + body="2400:cb00::/32\n2606:4700::/32", + ) structlog.configure(processors=[self.wrap_logger], context_class=dict) @@ -77,7 +84,45 @@ def test_cloudflare_ip_ignored(self): from pyramid_heroku.client_addr import ClientAddr self.request.environ["REMOTE_ADDR"] = "127.0.0.1" # load balancer - self.request.headers["X-Forwarded-For"] = " 6.6.6.6, 1.2.3.4, 9.9.9.9" + self.request.headers["X-Forwarded-For"] = "6.6.6.6, 1.2.3.4, 9.9.9.9" + + ClientAddr(self.handler, self.registry)(self.request) + self.handler.assert_called_with(self.request) + self.assertEqual(self.request.client_addr, "1.2.3.4") + + def test_cloudflare_ipv6_ignored(self): + from pyramid_heroku.client_addr import ClientAddr + + self.request.environ["REMOTE_ADDR"] = "127.0.0.1" # load balancer + self.request.headers["X-Forwarded-For"] = ( + "2600:cb00:0000:0000:0000:0000:0000:0001, " + "2500:cb00:0000:0000:0000:0000:0000:0001, " + "2400:cb00:0000:0000:0000:0000:0000:0001" + ) + + ClientAddr(self.handler, self.registry)(self.request) + self.handler.assert_called_with(self.request) + self.assertEqual( + self.request.client_addr, "2500:cb00:0000:0000:0000:0000:0000:0001" + ) + + def test_invalid_ips(self): + from pyramid_heroku.client_addr import ClientAddr + + self.request.environ["REMOTE_ADDR"] = "127.0.0.1" # load balancer + self.request.headers["X-Forwarded-For"] = "1.2.3.4, invalid_ip not_ip" + + ClientAddr(self.handler, self.registry)(self.request) + self.handler.assert_called_with(self.request) + self.assertEqual(self.request.client_addr, "1.2.3.4") + + def test_cloudflare_ipv4_6_ignored(self): + from pyramid_heroku.client_addr import ClientAddr + + self.request.environ["REMOTE_ADDR"] = "127.0.0.1" # load balancer + self.request.headers[ + "X-Forwarded-For" + ] = "1.2.3.4, 9.9.9.9, 2400:cb00:0000:0000:0000:0000:0000:0001" ClientAddr(self.handler, self.registry)(self.request) self.handler.assert_called_with(self.request) @@ -88,10 +133,10 @@ def test_cloudflare_ip_list_get_error(self): self.responses.reset() self.responses.add( - responses.GET, - "https://www.cloudflare.com/ips-v4", # noqa - status=501, - body="error", + responses.GET, "https://www.cloudflare.com/ips-v4", status=501, body="error" + ) + self.responses.add( + responses.GET, "https://www.cloudflare.com/ips-v6", status=501, body="error" ) self.request.environ["REMOTE_ADDR"] = "127.0.0.1" # load balancer @@ -103,9 +148,14 @@ def test_cloudflare_ip_list_get_error(self): self.handler.assert_called_with(self.request) self.assertEqual(self.request.client_addr, "9.9.9.9") - self.assertEqual(len(tweens_handler.records), 1) + self.assertEqual(len(tweens_handler.records), 2) self.assertEqual( - "Failed getting a list of Cloudflare IPs", tweens_handler.records[0].msg + "Failed getting a list of IPs from https://www.cloudflare.com/ips-v4", + tweens_handler.records[0].msg, + ) + self.assertEqual( + "Failed getting a list of IPs from https://www.cloudflare.com/ips-v6", + tweens_handler.records[1].msg, ) # structlog logging @@ -117,13 +167,21 @@ def test_cloudflare_ip_list_get_error(self): status=501, body="error", ) + self.responses.add( + responses.GET, "https://www.cloudflare.com/ips-v6", status=501, body="error" + ) registry.settings = {"pyramid_heroku.structlog": True} ClientAddr(self.handler, registry)(self.request) self.handler.assert_called_with(self.request) self.assertEqual(self.request.client_addr, "9.9.9.9") - self.assertEqual(len(tweens_handler.records), 1) + self.assertEqual(len(tweens_handler.records), 2) + self.assertEqual( + "Failed getting a list of IPs from https://www.cloudflare.com/ips-v4", + tweens_handler.records[0].msg, + ) self.assertEqual( - "Failed getting a list of Cloudflare IPs", tweens_handler.records[0].msg + "Failed getting a list of IPs from https://www.cloudflare.com/ips-v6", + tweens_handler.records[1].msg, )