Merge pull request #3620 from benoitc/fix/asgi-proxy-protocol-trust-and-parsing

fix: enforce proxy_allow_ips and tighten PROXY parsing in ASGI

gunicorn/asgi/parser.py

@@ -9,6 +9,7 @@ Provides callback-based parsing using either the fast C parser (gunicorn_h1c)
 or the pure Python PythonProtocol fallback.
 """
 
+import socket
 import struct
 from enum import IntEnum
 
@@ -319,9 +320,19 @@ class PythonProtocol:
             if len(parts) != 6:
                 raise InvalidProxyLine("Invalid PROXY v1 line for %s" % proto)
 
+            s_addr = parts[2]
+            d_addr = parts[3]
+
+            # Validate addresses with the appropriate family.  WSGI does the
+            # same in gunicorn/http/message.py:_parse_proxy_protocol_v1.
+            af = socket.AF_INET if proto == 'TCP4' else socket.AF_INET6
+            try:
+                socket.inet_pton(af, s_addr)
+                socket.inet_pton(af, d_addr)
+            except (OSError, ValueError):
+                raise InvalidProxyLine("Invalid PROXY v1 %s address" % proto)
+
             try:
-                s_addr = parts[2]
-                d_addr = parts[3]
                 s_port = int(parts[4])
                 d_port = int(parts[5])
             except ValueError as e:
@@ -391,6 +402,13 @@ class PythonProtocol:
         family = (fam_prot & 0xF0) >> 4
         protocol = fam_prot & 0x0F
 
+        # gunicorn is an HTTP server; only TCP (STREAM) makes sense.  WSGI
+        # rejects non-STREAM at gunicorn/http/message.py:_parse_proxy_protocol_v2.
+        if family in (PPFamily.INET, PPFamily.INET6) and protocol != PPProtocol.STREAM:
+            raise InvalidProxyHeader(
+                "PROXY v2: only TCP (STREAM) protocol is supported"
+            )
+
         if family == PPFamily.INET:
             # IPv4
             if len(addr_data) < 12:
@@ -399,7 +417,7 @@ class PythonProtocol:
             d_addr = '.'.join(str(b) for b in addr_data[4:8])
             s_port = struct.unpack('>H', addr_data[8:10])[0]
             d_port = struct.unpack('>H', addr_data[10:12])[0]
-            proto = 'TCP4' if protocol == PPProtocol.STREAM else 'UDP4'
+            proto = 'TCP4'
 
         elif family == PPFamily.INET6:
             # IPv6
@@ -412,7 +430,7 @@ class PythonProtocol:
             d_addr = ':'.join('%x' % w for w in d_words)
             s_port = struct.unpack('>H', addr_data[32:34])[0]
             d_port = struct.unpack('>H', addr_data[34:36])[0]
-            proto = 'TCP6' if protocol == PPProtocol.STREAM else 'UDP6'
+            proto = 'TCP6'
 
         elif family == PPFamily.UNSPEC:
             # Unspecified address family

gunicorn/asgi/protocol.py

@@ -512,7 +512,23 @@ class ASGIProtocol(asyncio.Protocol):
             'permit_unconventional_http_version': self.cfg.permit_unconventional_http_version,
         }
         if parser_class is PythonProtocol:
-            parser_kwargs['proxy_protocol'] = getattr(self.cfg, 'proxy_protocol', 'off')
+            # PROXY framing is only honored when the peer is in
+            # ``proxy_allow_ips`` (the WSGI parser enforces the same gate at
+            # gunicorn/http/message.py:proxy_protocol_access_check).  Untrusted
+            # peers get proxy_protocol='off', so any framing they send is
+            # interpreted as malformed HTTP and rejected with a 400.
+            cfg_proxy = getattr(self.cfg, 'proxy_protocol', 'off')
+            if cfg_proxy != 'off':
+                peername = self.transport.get_extra_info('peername')
+                normalized = _normalize_sockaddr(peername)
+                trusted = _check_trusted_proxy(
+                    normalized,
+                    self.cfg.proxy_allow_ips,
+                    self.cfg.proxy_allow_networks(),
+                )
+                parser_kwargs['proxy_protocol'] = cfg_proxy if trusted else 'off'
+            else:
+                parser_kwargs['proxy_protocol'] = 'off'
         self._callback_parser = parser_class(**parser_kwargs)
 
     def _on_headers_complete(self):
@@ -1286,9 +1302,19 @@ class ASGIProtocol(asyncio.Protocol):
         """Return the client address advertised via PROXY protocol if any.
 
         Falls back to the transport peername when PROXY protocol is disabled,
-        the framing was absent, or the parser is the C variant (which currently
+        the framing was absent, the parser is the C variant (which currently
-        does not surface PROXY metadata).
+        does not surface PROXY metadata), or the transport peer is not in
+        ``proxy_allow_ips`` (defense-in-depth: ``_setup_callback_parser``
+        already disables PROXY parsing for untrusted peers).
         """
+        if getattr(self.cfg, 'proxy_protocol', 'off') == 'off':
+            return peername
+        if not _check_trusted_proxy(
+            peername,
+            self.cfg.proxy_allow_ips,
+            self.cfg.proxy_allow_networks(),
+        ):
+            return peername
         parser = self._callback_parser
         info = getattr(parser, 'proxy_protocol_info', None) if parser else None
         if not info:

tests/requests/invalid/pp_03.http

@@ -0,0 +1 @@
+PROXY TCP4 not-an-ip 192.168.0.11 56324 443\r\n

tests/requests/invalid/pp_03.py

@@ -0,0 +1,11 @@
+#
+# This file is part of gunicorn released under the MIT license.
+# See the NOTICE for more information.
+
+from gunicorn.config import Config
+from gunicorn.http.errors import InvalidProxyLine
+
+cfg = Config()
+cfg.set('proxy_protocol', True)
+
+request = InvalidProxyLine

tests/test_asgi_proxy_protocol.py

@@ -0,0 +1,86 @@
+#
+# This file is part of gunicorn released under the MIT license.
+# See the NOTICE for more information.
+
+"""ASGI PROXY protocol parser tests.
+
+Covers the validation gaps that the WSGI parser already enforces:
+- v1 TCP4/TCP6 addresses must be valid IP addresses (inet_pton).
+- v2 must reject non-STREAM (UDP) protocols when family is INET/INET6.
+"""
+
+import struct
+
+import pytest
+
+from gunicorn.asgi.parser import (
+    PythonProtocol,
+    PP_V2_SIGNATURE,
+    InvalidProxyLine,
+    InvalidProxyHeader,
+)
+
+
+class TestProxyV1AddressValidation:
+    """v1 must validate IPv4/IPv6 source/destination addresses."""
+
+    def test_v1_invalid_ipv4_source_rejected(self):
+        parser = PythonProtocol(proxy_protocol='v1')
+        with pytest.raises(InvalidProxyLine):
+            parser.feed(b"PROXY TCP4 not-an-ip 192.168.0.1 1 2\r\n")
+
+    def test_v1_invalid_ipv4_destination_rejected(self):
+        parser = PythonProtocol(proxy_protocol='v1')
+        with pytest.raises(InvalidProxyLine):
+            parser.feed(b"PROXY TCP4 192.168.0.1 999.999.999.999 1 2\r\n")
+
+    def test_v1_invalid_ipv6_source_rejected(self):
+        parser = PythonProtocol(proxy_protocol='v1')
+        with pytest.raises(InvalidProxyLine):
+            parser.feed(b"PROXY TCP6 not::an::ip ::1 1 2\r\n")
+
+    def test_v1_valid_ipv4_accepted(self):
+        parser = PythonProtocol(proxy_protocol='v1')
+        parser.feed(b"PROXY TCP4 192.168.0.1 192.168.0.11 56324 443\r\n")
+        assert parser.proxy_protocol_info['client_addr'] == '192.168.0.1'
+        assert parser.proxy_protocol_info['proxy_protocol'] == 'TCP4'
+
+
+class TestProxyV2NonStreamRejected:
+    """v2 must reject DGRAM (UDP) when family is INET or INET6."""
+
+    @staticmethod
+    def _v2_header(fam_proto, addr_payload):
+        ver_cmd = 0x21  # version 2, command PROXY
+        length = len(addr_payload)
+        header = struct.pack('>BBH', ver_cmd, fam_proto, length)
+        return PP_V2_SIGNATURE + header + addr_payload
+
+    def test_v2_inet_dgram_rejected(self):
+        # family=0x10 (INET), protocol=0x02 (DGRAM)
+        fam_proto = 0x12
+        addr_payload = b'\x01\x02\x03\x04\x05\x06\x07\x08' + b'\x00\x50\x01\xbb'
+        data = self._v2_header(fam_proto, addr_payload)
+        parser = PythonProtocol(proxy_protocol='v2')
+        with pytest.raises(InvalidProxyHeader):
+            parser.feed(data)
+
+    def test_v2_inet6_dgram_rejected(self):
+        # family=0x20 (INET6), protocol=0x02 (DGRAM)
+        fam_proto = 0x22
+        addr_payload = b'\x00' * 32 + b'\x00\x50\x01\xbb'
+        data = self._v2_header(fam_proto, addr_payload)
+        parser = PythonProtocol(proxy_protocol='v2')
+        with pytest.raises(InvalidProxyHeader):
+            parser.feed(data)
+
+    def test_v2_inet_stream_accepted(self):
+        # family=0x10 (INET), protocol=0x01 (STREAM)
+        fam_proto = 0x11
+        addr_payload = b'\x01\x02\x03\x04\x05\x06\x07\x08' + b'\x00\x50\x01\xbb'
+        data = self._v2_header(fam_proto, addr_payload)
+        parser = PythonProtocol(proxy_protocol='v2')
+        # Followed by an HTTP request so the parser can transition out of
+        # the proxy_protocol state without hanging on more data.
+        parser.feed(data + b"GET / HTTP/1.1\r\nHost: e\r\n\r\n")
+        assert parser.proxy_protocol_info['proxy_protocol'] == 'TCP4'

tests/test_asgi_worker.py

@@ -597,11 +597,15 @@ class TestASGIProtocol:
         assert protocol._effective_peername(peer) == peer
 
     def test_effective_peername_with_proxy(self):
-        """PROXY-supplied client address overrides the transport peername."""
+        """PROXY-supplied client address overrides the transport peername
+        when both proxy_protocol is enabled AND the peer is in
+        proxy_allow_ips (matches the WSGI gate)."""
         from gunicorn.asgi.protocol import ASGIProtocol
 
         worker = mock.Mock()
         worker.cfg = Config()
+        worker.cfg.set('proxy_protocol', True)
+        worker.cfg.set('proxy_allow_ips', '10.0.0.1')
         worker.log = mock.Mock()
         worker.asgi = mock.Mock()
         protocol = ASGIProtocol(worker)
@@ -615,6 +619,34 @@ class TestASGIProtocol:
 
         assert protocol._effective_peername(("10.0.0.1", 1)) == ("203.0.113.5", 56324)
 
+    def test_effective_peername_untrusted_peer_ignored(self):
+        """A peer outside proxy_allow_ips MUST NOT be allowed to spoof its
+        client address via PROXY metadata, even if framing reached the
+        parser somehow.  Defense-in-depth for the trust gate that is
+        also enforced in _setup_callback_parser."""
+        from gunicorn.asgi.protocol import ASGIProtocol
+
+        worker = mock.Mock()
+        worker.cfg = Config()
+        worker.cfg.set('proxy_protocol', True)
+        worker.cfg.set('proxy_allow_ips', '10.0.0.1')
+        worker.log = mock.Mock()
+        worker.asgi = mock.Mock()
+        protocol = ASGIProtocol(worker)
+        protocol._callback_parser = mock.Mock(proxy_protocol_info={
+            'proxy_protocol': 'TCP4',
+            'client_addr': '203.0.113.99',
+            'client_port': 56324,
+            'proxy_addr': '198.51.100.1',
+            'proxy_port': 443,
+        })
+
+        # Peer is 198.51.100.1 (NOT in 10.0.0.1/32) — must fall back to
+        # the transport peername instead of trusting the spoofed PROXY
+        # metadata.
+        peer = ("198.51.100.1", 1234)
+        assert protocol._effective_peername(peer) == peer
+
     def test_effective_peername_unknown_proxy(self):
         """UNKNOWN PROXY framing has no client info; fall back to transport peername."""
         from gunicorn.asgi.protocol import ASGIProtocol