feat(proxy): trust the forwarding headers of a set of networks

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

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.                                                                                         |

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

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

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)

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): ...

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")

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 = {