diff --git a/gunicorn/asgi/parser.py b/gunicorn/asgi/parser.py index 2fc4a3d1..11e944ad 100644 --- a/gunicorn/asgi/parser.py +++ b/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 diff --git a/gunicorn/asgi/protocol.py b/gunicorn/asgi/protocol.py index 085a4604..5d6301d3 100644 --- a/gunicorn/asgi/protocol.py +++ b/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 - does not surface PROXY metadata). + the framing was absent, the parser is the C variant (which currently + 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: diff --git a/tests/requests/invalid/pp_03.http b/tests/requests/invalid/pp_03.http new file mode 100644 index 00000000..14ee7632 --- /dev/null +++ b/tests/requests/invalid/pp_03.http @@ -0,0 +1 @@ +PROXY TCP4 not-an-ip 192.168.0.11 56324 443\r\n diff --git a/tests/requests/invalid/pp_03.py b/tests/requests/invalid/pp_03.py new file mode 100644 index 00000000..0999e652 --- /dev/null +++ b/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 diff --git a/tests/test_asgi_proxy_protocol.py b/tests/test_asgi_proxy_protocol.py new file mode 100644 index 00000000..576f6873 --- /dev/null +++ b/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' diff --git a/tests/test_asgi_worker.py b/tests/test_asgi_worker.py index fbe9a6e6..f718d5fe 100644 --- a/tests/test_asgi_worker.py +++ b/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