From a19137f29db899cad5880c81e15fa5938f139697 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Magalh=C3=A3es?= Date: Sun, 13 Sep 2026 15:13:06 +0100 Subject: [PATCH] feat(proxy): trust the forwarding headers of a set of networks - A peer in TRUST_NETWORKS has its X-Real-IP and X-Forwarded-* headers kept - A repeated or empty forwarding header no longer breaks the client address - A network written with its host part set now matches every address of it --- CHANGELOG.md | 4 +- doc/configuration.md | 1 + src/netius/common/util.py | 2 +- src/netius/extra/proxy_r.py | 26 +++--- src/netius/servers/proxy.py | 46 ++++++++++ src/netius/servers/proxy.pyi | 4 + src/netius/test/common/util.py | 18 ++++ src/netius/test/extra/proxy_r.py | 149 +++++++++++++++++++++++++++++++ src/netius/test/servers/proxy.py | 73 +++++++++++++++ 9 files changed, 311 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 90e221e0..2fac3dd1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,7 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added -* +* Support for trusting the forwarding headers of the proxies that sit in a given network, such as the client address ### Changed @@ -18,7 +18,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed * A configuration file is now read under Python 3.5, instead of failing on it +* A forwarding header that is repeated or empty no longer breaks the client address passed on by the reverse proxy * A header that is set by its position now replaces the one that is there +* A network written with the address of an interface in it now matches every address of the network * A string that carries no null byte is now given back whole instead of raising * A TFTP file that is served under an absolute name no longer fails to open * A TFTP peer can no longer reach a file outside the root that is served diff --git a/doc/configuration.md b/doc/configuration.md index 3e8a8c0d..d232b1a7 100644 --- a/doc/configuration.md +++ b/doc/configuration.md @@ -116,6 +116,7 @@ Accumulating the payload before each flush improves the compression ratio of a c | **DYNAMIC** | `bool` | `True` | In case this value is active dynamic connection encoding is applied, meaning that extra heuristics will be applied on a response basis to determine the proper encoding of the response (eg: plain, chunked, gzip, etc.). | | **THROTTLE** | `bool` | `True` | If throttling of the connection stream should be applied on both ways to avoid starvation of the producer consumer relation. | | **TRUST_ORIGIN** | `bool` | `False` | If the origin connection (eg: http client, proxy client, etc.) is meant to be trusted meaning that its information is considered reliable, this value is especially important for proxy to proxy relations. | +| **TRUST_NETWORKS** | `list` | `[]` | The addresses or IPv4 networks (eg: `172.17.0.0/16`) of the peers that are trusted, so that the forwarding headers they send (eg: `X-Real-IP`) are kept and passed on, meant for the proxies in front of this one. | | **COMPRESS_FORWARD_ACCEPT** | `bool` | `False` | If the `Accept-Encoding` of the client should be forwarded to the back-end instead of asking it for the `identity` coding, use it when the back-end does better than the proxy. | | **COMPRESS_BUFFER** | `bool` | `True` | If the encoding decision should be deferred for the back-ends that stream a payload with no declared length, holding the response until the minimum size is crossed, when disabled such responses are forwarded untouched. | | **COMPRESS_TIMEOUT** | `float` | `1.0` | The maximum amount of time (in seconds) that a response may be held while waiting for enough payload to decide on the compression. | diff --git a/src/netius/common/util.py b/src/netius/common/util.py index 1e23cc62..83206976 100644 --- a/src/netius/common/util.py +++ b/src/netius/common/util.py @@ -137,7 +137,7 @@ def in_subnet_ip4(address, subnet): subnet, length = subnet.split("/", 1) size_i = 32 - int(length) address_a = ip4_to_addr(address) - subnet_a = ip4_to_addr(subnet) + subnet_a = ip4_to_addr(subnet) >> size_i << size_i limit_a = subnet_a + pow(2, size_i) in_subnet = (address_a & subnet_a) == subnet_a in_subnet &= address_a < limit_a diff --git a/src/netius/extra/proxy_r.py b/src/netius/extra/proxy_r.py index 615e792b..8fceb6f1 100644 --- a/src/netius/extra/proxy_r.py +++ b/src/netius/extra/proxy_r.py @@ -221,29 +221,35 @@ def on_headers(self, connection, parser): host = self.alias.get(host_s, host) host = self.alias.get(host, host) + # determines if the peer of the connection is considered "trustable", + # either because every origin is trusted or because its address is part + # of the trusted networks (eg: another proxy in front of this one) + trusted = self.is_trusted(connection) + # tries to discover the proper address representation of the current # connections, note that the forwarded for header is only used in case - # the current "origin" is considered "trustable" + # the current "origin" is considered "trustable" and that a repeated + # one is joined, as the first of its values is the client one address = connection.address[0] - if self.trust_origin: - address = headers.get("x-forwarded-for", address) - address = headers.get("x-client-ip", address) - address = headers.get("x-real-ip", address) + if trusted: + address = self._prx_header(headers, "x-forwarded-for", join=True) or address + address = self._prx_header(headers, "x-client-ip") or address + address = self._prx_header(headers, "x-real-ip") or address address = address.split(",", 1)[0].strip() # tries to retrieve the string based port version taking into account # the port of the server bind port and the host based port and falling # back to the forwarded port in case the "origin" is considered "trustable" port = port_s or str(self.port) - if self.trust_origin: - port = headers.get("x-forwarded-port", port) + if trusted: + port = self._prx_header(headers, "x-forwarded-port") or port # tries to discover the protocol representation of the current # connections, note that the forwarded for header is only used in case # the current "origin" is considered "trustable" protocol = "https" if is_secure else "http" - if self.trust_origin: - protocol = headers.get("x-forwarded-proto", protocol) + if trusted: + protocol = self._prx_header(headers, "x-forwarded-proto") or protocol # constructs the URL that is going to be used by the rule engine and any # other internal resolution process as the canonical URL of the request @@ -431,7 +437,7 @@ def on_headers(self, connection, parser): # a client supplied forwarded header may only be taken into account # in case the origin is considered a trustable one, otherwise it's # removed so that the back-end is not misled by a spoofed value - if not self.trust_origin and "forwarded" in headers: + if not trusted and "forwarded" in headers: del headers["forwarded"] # updates the various headers that are related with the reverse diff --git a/src/netius/servers/proxy.py b/src/netius/servers/proxy.py index 8513975a..abc74560 100644 --- a/src/netius/servers/proxy.py +++ b/src/netius/servers/proxy.py @@ -68,6 +68,12 @@ relay towards an arbitrary service, set it to an empty sequence so that the restriction is removed (not recommended) """ +TRUST_NETWORKS = () +""" The sequence of addresses and networks (in CIDR notation) of the +peers that are considered trustable, meaning that the forwarding +information they provide (eg: X-Real-IP) is taken as reliable, this +is meant for the other proxies that sit in front of this one """ + PORT_REGEX = re.compile(r"^[0-9]+$") """ Regular expression to be used in the validation of the port of the target of a tunnel, only the ASCII digits are considered valid ones as @@ -197,6 +203,7 @@ def __init__( dynamic=True, throttle=True, trust_origin=False, + trust_networks=TRUST_NETWORKS, max_pending=MAX_PENDING, compress_forward_accept=False, compress_buffer=True, @@ -215,6 +222,7 @@ def __init__( self.dynamic = dynamic self.throttle = throttle self.trust_origin = trust_origin + self.trust_networks = trust_networks self.max_pending = max_pending self.min_pending = int(max_pending * MIN_RATIO) self.compress_forward_accept = compress_forward_accept @@ -452,6 +460,35 @@ def is_upgrade(self, parser): tokens = [value.strip() for value in connection.lower().split(",")] return "upgrade" in tokens and upgrade.lower() == "websocket" + def is_trusted(self, connection): + """ + Determines if the peer of the provided connection is trustable, + meaning that the forwarding information it provides (eg: the + X-Real-IP and X-Forwarded-For headers) may be taken as reliable. + + A peer is trustable when every origin is trusted or when its + address is one of the trusted networks, note that under the + PROXY protocol the address is the one reported by the front-end. + + :type connection: Connection + :param connection: The connection (or stream) whose peer is going + to be verified for trust. + :rtype: bool + :return: If the peer of the connection is considered trustable. + """ + + if self.trust_origin: + return True + if not self.trust_networks: + return False + + # only an IPv4 address is able to be matched against the trusted + # networks, any other address (eg: IPv6) is never trusted + address = connection.address[0] + if not netius.common.is_ip4(address): + return False + return netius.common.assert_ip4(address, self.trust_networks, default=False) + def on_data(self, connection, data): netius.StreamServer.on_data(self, connection, data) @@ -546,8 +583,17 @@ def on_serve(self): "CONNECT_PORTS", self.connect_ports, cast=list ) self.connect_ports = tuple(int(port) for port in self.connect_ports) + if self.env: + self.trust_networks = self.get_env( + "TRUST_NETWORKS", self.trust_networks, cast=list + ) if self.trust_origin: self.info('Origin is considered "trustable" by proxy') + elif self.trust_networks: + self.info( + 'Networks %s are considered "trustable" by proxy', + ", ".join(self.trust_networks), + ) def on_data_http(self, connection, parser): http2.HTTP2Server.on_data_http(self, connection, parser) diff --git a/src/netius/servers/proxy.pyi b/src/netius/servers/proxy.pyi index 0af40898..974c53bb 100644 --- a/src/netius/servers/proxy.pyi +++ b/src/netius/servers/proxy.pyi @@ -12,6 +12,7 @@ BUFFER_RATIO: float MIN_RATIO: float MAX_PENDING: int CONNECT_PORTS: tuple[int, ...] +TRUST_NETWORKS: tuple[str, ...] PORT_REGEX: Pattern[str] HOP_HEADERS: tuple[str, ...] COMPRESS_TIMEOUT: float @@ -29,6 +30,7 @@ class ProxyServer(HTTP2Server): dynamic: bool throttle: bool trust_origin: bool + trust_networks: Sequence[str] max_pending: int min_pending: int compress_forward_accept: bool @@ -45,6 +47,7 @@ class ProxyServer(HTTP2Server): dynamic: bool = ..., throttle: bool = ..., trust_origin: bool = ..., + trust_networks: Sequence[str] = ..., max_pending: int = ..., compress_forward_accept: bool = ..., compress_buffer: bool = ..., @@ -73,6 +76,7 @@ class ProxyServer(HTTP2Server): def reason_connection(self, _connection: Connection | Protocol, reason: str): ... def pair_connection(self, _connection: Connection | Protocol): ... def is_upgrade(self, parser: HTTPParser | HTTP2Stream) -> bool: ... + def is_trusted(self, connection: Connection | HTTP2Stream) -> bool: ... def on_data(self, connection: Connection, data: bytes): ... def on_connection_d(self, connection: Connection): ... def on_stream_d(self, stream: Stream): ... diff --git a/src/netius/test/common/util.py b/src/netius/test/common/util.py index 1bcaf559..15ccb6e1 100644 --- a/src/netius/test/common/util.py +++ b/src/netius/test/common/util.py @@ -103,6 +103,24 @@ def test_in_subnet_ip4(self): result = netius.common.in_subnet_ip4("127.0.0.1", "128.0.0.0/24") self.assertEqual(result, False) + def test_in_subnet_ip4_host(self): + # a subnet written with the host part of its address set (eg: the + # address of an interface) must still match every address in it + result = netius.common.in_subnet_ip4("172.17.5.2", "172.17.0.1/16") + self.assertEqual(result, True) + + result = netius.common.in_subnet_ip4("172.17.5.3", "172.17.0.1/16") + self.assertEqual(result, True) + + result = netius.common.in_subnet_ip4("172.17.0.0", "172.17.0.1/16") + self.assertEqual(result, True) + + result = netius.common.in_subnet_ip4("172.18.0.1", "172.17.0.1/16") + self.assertEqual(result, False) + + result = netius.common.in_subnet_ip4("172.16.255.255", "172.17.0.1/16") + self.assertEqual(result, False) + def test_addr_to_ip4(self): result = netius.common.addr_to_ip4(2130706433) self.assertEqual(result, "127.0.0.1") diff --git a/src/netius/test/extra/proxy_r.py b/src/netius/test/extra/proxy_r.py index 7498dd9d..65c97de7 100644 --- a/src/netius/test/extra/proxy_r.py +++ b/src/netius/test/extra/proxy_r.py @@ -787,6 +787,9 @@ def test_dynamic_config(self): def test_trust_origin_config(self): self.assertFalse(self.server.trust_origin) + def test_trust_networks_config(self): + self.assertEqual(self.server.trust_networks, ()) + def test_http_client_event_relay(self): events_received = [] client = self.server.http_client @@ -1415,6 +1418,140 @@ def capture_method(method, url, **kwargs): self.assertEqual(captured_headers.get("x-real-ip"), "192.168.1.100") self.assertEqual(captured_headers.get("x-client-ip"), "192.168.1.100") + def test_x_forwarded_headers_trusted_network(self): + if mock == None: + self.skipTest("Skipping test: mock unavailable") + + self.server.trust_networks = ("172.17.0.0/16",) + + frontend = self._make_frontend() + frontend.address = ("172.17.0.5", 54321) + request_parser = self._make_forwarded_parser() + backend = self._make_backend() + + captured_headers = {} + + def capture_method(method, url, **kwargs): + captured_headers.update(kwargs.get("headers", {})) + return (None, backend) + + with mock.patch.object( + self.server.http_client, "method", side_effect=capture_method + ): + self.server.on_headers(frontend, request_parser) + + # a peer from a trusted network is another proxy, so the address of + # the client that it reports is the one that is passed on, together + # with the protocol, the port and the standard forwarded header + self.assertEqual(captured_headers.get("x-real-ip"), "203.0.113.7") + self.assertEqual(captured_headers.get("x-client-ip"), "203.0.113.7") + self.assertEqual(captured_headers.get("x-forwarded-for"), "203.0.113.7") + self.assertEqual(captured_headers.get("x-forwarded-proto"), "https") + self.assertEqual(captured_headers.get("x-forwarded-port"), "443") + self.assertEqual(captured_headers.get("forwarded"), "for=203.0.113.7") + + def test_x_forwarded_headers_trusted_repeated(self): + if mock == None: + self.skipTest("Skipping test: mock unavailable") + + self.server.trust_networks = ("172.17.0.0/16",) + + frontend = self._make_frontend() + frontend.address = ("172.17.0.5", 54321) + request_parser = self._make_request_parser(host="host.com") + request_parser.headers = { + "host": "host.com", + "x-forwarded-for": ["203.0.113.7", "172.17.0.1"], + "x-forwarded-proto": ["http", "https"], + "x-forwarded-port": ["80", "443"], + } + backend = self._make_backend() + + captured_headers = {} + + def capture_method(method, url, **kwargs): + captured_headers.update(kwargs.get("headers", {})) + return (None, backend) + + with mock.patch.object( + self.server.http_client, "method", side_effect=capture_method + ): + self.server.on_headers(frontend, request_parser) + + # a repeated header is stored as a sequence of values by the parser, + # the forwarded for ones are joined (the first being the client) and + # for the others the last definition is the one that prevails + self.assertEqual(captured_headers.get("x-real-ip"), "203.0.113.7") + self.assertEqual(captured_headers.get("x-forwarded-for"), "203.0.113.7") + self.assertEqual(captured_headers.get("x-forwarded-proto"), "https") + self.assertEqual(captured_headers.get("x-forwarded-port"), "443") + + def test_x_forwarded_headers_trusted_empty(self): + if mock == None: + self.skipTest("Skipping test: mock unavailable") + + self.server.trust_networks = ("172.17.0.0/16",) + + frontend = self._make_frontend() + frontend.address = ("172.17.0.5", 54321) + request_parser = self._make_request_parser(host="host.com") + request_parser.headers = { + "host": "host.com", + "x-real-ip": "", + "x-forwarded-proto": "", + "x-forwarded-port": "", + } + backend = self._make_backend() + + captured_headers = {} + + def capture_method(method, url, **kwargs): + captured_headers.update(kwargs.get("headers", {})) + return (None, backend) + + with mock.patch.object( + self.server.http_client, "method", side_effect=capture_method + ): + self.server.on_headers(frontend, request_parser) + + # a trusted peer that provides empty forwarding information is taken + # as the client itself, instead of an empty client being passed on + self.assertEqual(captured_headers.get("x-real-ip"), "172.17.0.5") + self.assertEqual(captured_headers.get("x-forwarded-for"), "172.17.0.5") + self.assertEqual(captured_headers.get("x-forwarded-proto"), "http") + self.assertEqual(captured_headers.get("x-forwarded-port"), "80") + + def test_x_forwarded_headers_untrusted_network(self): + if mock == None: + self.skipTest("Skipping test: mock unavailable") + + self.server.trust_networks = ("172.17.0.0/16",) + + frontend = self._make_frontend() + frontend.address = ("198.51.100.20", 54321) + request_parser = self._make_forwarded_parser() + backend = self._make_backend() + + captured_headers = {} + + def capture_method(method, url, **kwargs): + captured_headers.update(kwargs.get("headers", {})) + return (None, backend) + + with mock.patch.object( + self.server.http_client, "method", side_effect=capture_method + ): + self.server.on_headers(frontend, request_parser) + + # a peer outside the trusted networks may be spoofing the headers, + # so they are replaced by what the proxy itself sees of the peer + self.assertEqual(captured_headers.get("x-real-ip"), "198.51.100.20") + self.assertEqual(captured_headers.get("x-client-ip"), "198.51.100.20") + self.assertEqual(captured_headers.get("x-forwarded-for"), "198.51.100.20") + self.assertEqual(captured_headers.get("x-forwarded-proto"), "http") + self.assertEqual(captured_headers.get("x-forwarded-port"), "80") + self.assertEqual("forwarded" in captured_headers, False) + def test_accept_encoding_auto(self): if mock == None: self.skipTest("Skipping test: mock unavailable") @@ -1522,6 +1659,18 @@ def _make_request_parser(self, host="host.com", method="GET", path="/test"): parser.headers = {"host": host} return parser + def _make_forwarded_parser(self, host="host.com"): + parser = self._make_request_parser(host=host) + parser.headers = { + "host": host, + "x-real-ip": "203.0.113.7", + "x-forwarded-for": "203.0.113.7, 172.17.0.1", + "x-forwarded-proto": "https", + "x-forwarded-port": "443", + "forwarded": "for=203.0.113.7", + } + return parser + def _make_upgrade_parser(self, host="host.com", path="/socket"): parser = self._make_request_parser(host=host, method="GET", path=path) parser.headers = { diff --git a/src/netius/test/servers/proxy.py b/src/netius/test/servers/proxy.py index d605b2ea..a450a458 100644 --- a/src/netius/test/servers/proxy.py +++ b/src/netius/test/servers/proxy.py @@ -282,6 +282,51 @@ def test_is_upgrade(self): parser = Parser(headers={}) self.assertEqual(self.server.is_upgrade(parser), False) + def test_is_trusted(self): + Connection = collections.namedtuple("Connection", "address") + + # with no trusted network and no trusted origin no peer is trusted, + # not even the ones that sit in a private network + self.assertEqual(self.server.is_trusted(Connection(("172.17.0.5", 80))), False) + + self.server.trust_networks = ("172.17.0.0/16", "10.0.0.1") + + # a peer is trusted when its address is part of one of the networks + # or when it matches one of the plain addresses exactly + self.assertEqual(self.server.is_trusted(Connection(("172.17.0.5", 80))), True) + self.assertEqual( + self.server.is_trusted(Connection(("172.17.255.255", 80))), True + ) + self.assertEqual(self.server.is_trusted(Connection(("10.0.0.1", 80))), True) + self.assertEqual(self.server.is_trusted(Connection(("172.18.0.5", 80))), False) + self.assertEqual(self.server.is_trusted(Connection(("172.16.0.5", 80))), False) + self.assertEqual(self.server.is_trusted(Connection(("10.0.0.2", 80))), False) + + # an address that is not a valid IPv4 one is never matched against + # the networks, instead of failing on it + self.assertEqual(self.server.is_trusted(Connection(("::1", 80))), False) + self.assertEqual( + self.server.is_trusted(Connection(("::ffff:172.17.0.5", 80))), False + ) + self.assertEqual( + self.server.is_trusted(Connection(("172.17.0.256", 80))), False + ) + + self.server.trust_networks = ("172.17.0.1/16",) + + # a network may be written as the address of the interface that sits + # in it, which must still trust every peer of the network + self.assertEqual(self.server.is_trusted(Connection(("172.17.0.1", 80))), True) + self.assertEqual(self.server.is_trusted(Connection(("172.17.0.2", 80))), True) + self.assertEqual(self.server.is_trusted(Connection(("172.18.0.2", 80))), False) + + self.server.trust_networks = () + self.server.trust_origin = True + + # trusting every origin trusts every peer whatever its address + self.assertEqual(self.server.is_trusted(Connection(("8.8.8.8", 80))), True) + self.assertEqual(self.server.is_trusted(Connection(("::1", 80))), True) + def test_tunnel(self): if mock == None: self.skipTest("Skipping test: mock unavailable") @@ -752,6 +797,34 @@ def test_on_stream_d(self): self.assertEqual(stream.proxy_c, None) self.assertEqual(stream.encoding_b, None) + def test_on_serve(self): + server = netius.servers.ProxyServer(trust_networks=["172.17.0.0/16"]) + server.env = False + + try: + with netius.conf_override("TRUST_NETWORKS", "10.0.0.0/8"): + server.on_serve() + + # with no environment to read from, the networks are the ones + # that were given on the creation of the server + self.assertEqual(server.trust_networks, ["172.17.0.0/16"]) + finally: + server.cleanup() + + def test_on_serve_env(self): + server = netius.servers.ProxyServer() + server.env = True + + try: + with netius.conf_override("TRUST_NETWORKS", "172.17.0.0/16;10.0.0.1"): + server.on_serve() + + # the environment is able to set the networks, naming them as the + # semicolon separated list that a variable holds + self.assertEqual(server.trust_networks, ["172.17.0.0/16", "10.0.0.1"]) + finally: + server.cleanup() + def test_on_headers(self): if mock == None: self.skipTest("Skipping test: mock unavailable")