Skip to content

Commit 687b0f2

Browse files
bneradtbneradt
committed
Gate PP allowlist by header preface
Flexible Proxy Protocol ports currently use proxy.config.http.proxy_protocol_allowlist as a source-IP gate for every connection, even when traffic never presents a Proxy Protocol header. Mixed PP and non-PP deployments can then reject ordinary HTTP or TLS clients unexpectedly. This changes the allowlist check to run only after a v1 or v2 Proxy Protocol preface is detected, while still applying the gate before parsing or consuming the header. This keeps PP-looking spoof attempts behind the trusted-peer check, leaves non-PP bytes untouched for normal probing or TLS handshakes, and documents the new behavior with focused AuTest coverage.
1 parent 060721f commit 687b0f2

9 files changed

Lines changed: 243 additions & 66 deletions

File tree

‎doc/admin-guide/configuration/proxy-protocol.en.rst‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -49,10 +49,13 @@ configured with :ts:cv:`proxy.config.http.proxy_protocol_allowlist`.
4949

5050
.. important::
5151

52-
If the allowlist is configured, requests will only be accepted from these
53-
IP addresses for all ports designated for Proxy Protocol in the
54-
:ts:cv:`proxy.config.http.server_ports` configuration, regardless of whether
55-
the connections have the Proxy Protocol header.
52+
If the allowlist is configured, connections that begin with a Proxy
53+
Protocol header preface will only be accepted from these IP addresses on
54+
ports designated for Proxy Protocol in the
55+
:ts:cv:`proxy.config.http.server_ports` configuration. Connections
56+
without a Proxy Protocol header preface are not restricted by this
57+
allowlist; use :file:`ip_allow.yaml` for general source-IP access
58+
control.
5659

5760
By default, |TS| uses client's IP address that is from the peer when it applies ACL. If you configure a port to
5861
enable PROXY protocol and want to apply ACL against the IP address delivered by PROXY protocol, you need to have ``PROXY`` in

‎doc/admin-guide/files/records.yaml.en.rst‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2179,10 +2179,13 @@ Proxy User Variables
21792179

21802180
.. ts:cv:: CONFIG proxy.config.http.proxy_protocol_allowlist STRING ```<ip list>```
21812181
2182-
This defines a allowlist of server IPs that are trusted to provide
2183-
connections with Proxy Protocol information. This is a comma delimited list
2184-
of IP addresses. Addressed may be listed individually, in a range separated
2185-
by a dash or by using CIDR notation.
2182+
This defines an allowlist of server IPs that are trusted to provide
2183+
connections with Proxy Protocol information. This allowlist is enforced only
2184+
for connections that begin with a Proxy Protocol header preface; non-Proxy
2185+
Protocol traffic on flexible Proxy Protocol ports is not restricted by this
2186+
setting. Use :file:`ip_allow.yaml` for general source-IP access control. This
2187+
is a comma delimited list of IP addresses. Addresses may be listed
2188+
individually, in a range separated by a dash, or by using CIDR notation.
21862189

21872190
======================= ===========================================================
21882191
Example Effect

‎doc/release-notes/upgrading.en.rst‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,10 @@ The following :file:`records.yaml` changes have been made:
163163
:ts:cv:`proxy.config.http.header_field_max_size` have been changed to 32KB.
164164
- The records.yaml entry :ts:cv:`proxy.config.http.server_ports` now also accepts the
165165
``allow-plain`` option
166+
- The records.yaml entry :ts:cv:`proxy.config.http.proxy_protocol_allowlist` is now enforced
167+
only for connections on Proxy Protocol-enabled ports that begin with a Proxy Protocol
168+
header preface. Non-Proxy Protocol traffic on flexible Proxy Protocol ports is no longer
169+
restricted by this setting; use :file:`ip_allow.yaml` for general source-IP access control.
166170
- The records.yaml entry :ts:cv:`proxy.config.http.cache.max_open_write_retry_timeout` has been added to specify a timeout for starting a write to cache
167171
- The records.yaml entry :ts:cv:`proxy.config.net.per_client.max_connections_in` has
168172
been added to limit the number of connections from a client IP. This works the

‎include/iocore/net/NetVConnection.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -505,6 +505,8 @@ class NetVConnection : public VConnection, public PluginUserArgs<TS_USER_ARGS_VC
505505
void set_proxy_protocol_info(const ProxyProtocol &src);
506506
const ProxyProtocol &get_proxy_protocol_info() const;
507507

508+
bool has_proxy_protocol_preface(IOBufferReader *) const;
509+
bool has_proxy_protocol_preface(const char *, int64_t) const;
508510
bool has_proxy_protocol(IOBufferReader *, int max_header_size);
509511
bool has_proxy_protocol(char *, int64_t *);
510512

‎src/iocore/net/NetVConnection.cc‎

Lines changed: 33 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,38 @@ DbgCtl dbg_ctl_ssl{"ssl"};
4545
// NetVConnection
4646
//
4747

48+
/**
49+
PROXY Protocol preface check with IOBufferReader.
50+
*/
51+
bool
52+
NetVConnection::has_proxy_protocol_preface(IOBufferReader *reader) const
53+
{
54+
if (reader == nullptr) {
55+
return false;
56+
}
57+
58+
swoc::TextView tv;
59+
60+
char preface[PPv2_CONNECTION_HEADER_LEN];
61+
tv.assign(preface, reader->memcpy(preface, sizeof(preface), 0));
62+
return proxy_protocol_detect(tv);
63+
}
64+
65+
/**
66+
PROXY Protocol preface check with a raw buffer.
67+
*/
68+
bool
69+
NetVConnection::has_proxy_protocol_preface(const char *buffer, int64_t bytes_r) const
70+
{
71+
if (buffer == nullptr || bytes_r <= 0) {
72+
return false;
73+
}
74+
75+
swoc::TextView tv;
76+
tv.assign(buffer, static_cast<size_t>(bytes_r));
77+
return proxy_protocol_detect(tv);
78+
}
79+
4880
/**
4981
PROXY Protocol check with IOBufferReader
5082
@@ -55,9 +87,7 @@ NetVConnection::has_proxy_protocol(IOBufferReader *reader, int max_header_size)
5587
{
5688
swoc::TextView tv;
5789

58-
char preface[PPv2_CONNECTION_HEADER_LEN];
59-
tv.assign(preface, reader->memcpy(preface, sizeof(preface), 0));
60-
if (!proxy_protocol_detect(tv)) {
90+
if (!this->has_proxy_protocol_preface(reader)) {
6191
return false;
6292
}
6393

‎src/iocore/net/SSLNetVConnection.cc‎

Lines changed: 35 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -353,45 +353,47 @@ SSLNetVConnection::read_raw_data()
353353

354354
if (this->get_is_proxy_protocol() && this->get_proxy_protocol_version() == ProxyProtocolVersion::UNDEFINED) {
355355
Dbg(dbg_ctl_proxyprotocol, "proxy protocol is enabled on this port");
356-
if (pp_ipmap->count() > 0) {
357-
Dbg(dbg_ctl_proxyprotocol, "proxy protocol has a configured allowlist of trusted IPs - checking");
358-
359-
// Using get_remote_addr() will return the ip of the
360-
// proxy source IP, not the Proxy Protocol client ip.
361-
if (!pp_ipmap->contains(swoc::IPAddr(get_remote_addr()))) {
362-
Dbg(dbg_ctl_proxyprotocol, "Source IP is NOT in the configured allowlist of trusted IPs - closing connection");
363-
r = -ENOTCONN; // Need a quick close/exit here to refuse the connection!!!!!!!!!
364-
goto proxy_protocol_bypass;
356+
if (this->has_proxy_protocol_preface(buffer, r)) {
357+
if (pp_ipmap->count() > 0) {
358+
Dbg(dbg_ctl_proxyprotocol, "proxy protocol has a configured allowlist of trusted IPs - checking");
359+
360+
// Using get_remote_addr() will return the ip of the
361+
// proxy source IP, not the Proxy Protocol client ip.
362+
if (!pp_ipmap->contains(swoc::IPAddr(get_remote_addr()))) {
363+
Dbg(dbg_ctl_proxyprotocol, "Source IP is NOT in the configured allowlist of trusted IPs - closing connection");
364+
r = -ENOTCONN; // Need a quick close/exit here to refuse the connection!!!!!!!!!
365+
goto proxy_protocol_bypass;
366+
} else {
367+
char new_host[INET6_ADDRSTRLEN];
368+
Dbg(dbg_ctl_proxyprotocol, "Source IP [%s] is in the trusted allowlist for proxy protocol",
369+
ats_ip_ntop(this->get_remote_addr(), new_host, sizeof(new_host)));
370+
}
365371
} else {
366-
char new_host[INET6_ADDRSTRLEN];
367-
Dbg(dbg_ctl_proxyprotocol, "Source IP [%s] is in the trusted allowlist for proxy protocol",
368-
ats_ip_ntop(this->get_remote_addr(), new_host, sizeof(new_host)));
372+
Dbg(dbg_ctl_proxyprotocol, "proxy protocol DOES NOT have a configured allowlist of trusted IPs but "
373+
"proxy protocol is enabled on this port - processing all connections with Proxy Protocol "
374+
"headers");
369375
}
370-
} else {
371-
Dbg(dbg_ctl_proxyprotocol, "proxy protocol DOES NOT have a configured allowlist of trusted IPs but "
372-
"proxy protocol is enabled on this port - processing all connections");
373-
}
374376

375-
auto const stored_r = r;
376-
if (this->has_proxy_protocol(buffer, &r)) {
377-
Dbg(dbg_ctl_proxyprotocol, "ssl has proxy protocol header");
378-
if (dbg_ctl_proxyprotocol.on()) {
379-
IpEndpoint src;
380-
src.sa = *(this->get_proxy_protocol_src_addr());
381-
IpEndpoint dst;
382-
dst.sa = *(this->get_proxy_protocol_dst_addr());
383-
ip_port_text_buffer src_ipb, dst_ipb;
384-
ats_ip_nptop(&src, src_ipb, sizeof(src_ipb));
385-
ats_ip_nptop(&dst, dst_ipb, sizeof(dst_ipb));
386-
DbgPrint(dbg_ctl_proxyprotocol, "ssl proxy protocol v%d header parsed: src=[%s] dst=[%s]",
387-
static_cast<int>(this->get_proxy_protocol_version()), src_ipb, dst_ipb);
377+
auto const stored_r = r;
378+
if (this->has_proxy_protocol(buffer, &r)) {
379+
Dbg(dbg_ctl_proxyprotocol, "ssl has proxy protocol header");
380+
if (dbg_ctl_proxyprotocol.on()) {
381+
IpEndpoint src;
382+
src.sa = *(this->get_proxy_protocol_src_addr());
383+
IpEndpoint dst;
384+
dst.sa = *(this->get_proxy_protocol_dst_addr());
385+
ip_port_text_buffer src_ipb, dst_ipb;
386+
ats_ip_nptop(&src, src_ipb, sizeof(src_ipb));
387+
ats_ip_nptop(&dst, dst_ipb, sizeof(dst_ipb));
388+
DbgPrint(dbg_ctl_proxyprotocol, "ssl proxy protocol v%d header parsed: src=[%s] dst=[%s]",
389+
static_cast<int>(this->get_proxy_protocol_version()), src_ipb, dst_ipb);
390+
}
391+
} else {
392+
Dbg(dbg_ctl_proxyprotocol, "proxy protocol preface was present, but Proxy Protocol header could not be parsed");
393+
r = stored_r;
388394
}
389395
} else {
390396
Dbg(dbg_ctl_proxyprotocol, "proxy protocol was enabled, but Proxy Protocol header was not present");
391-
// We are flexible with the Proxy Protocol designation. Maybe not all
392-
// connections include Proxy Protocol. Revert to the stored value of r so
393-
// we can process the bytes that are on the wire (likely a CLIENT_HELLO).
394-
r = stored_r;
395397
}
396398
}
397399
} // end of Proxy Protocol processing

‎src/proxy/ProtocolProbeSessionAccept.cc‎

Lines changed: 24 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -111,27 +111,33 @@ struct ProtocolProbeTrampoline : public Continuation, public ProtocolProbeSessio
111111

112112
if (netvc->get_is_proxy_protocol() && netvc->get_proxy_protocol_version() == ProxyProtocolVersion::UNDEFINED) {
113113
Dbg(dbg_ctl_proxyprotocol, "ioCompletionEvent: proxy protocol is enabled on this port");
114-
if (pp_ipmap->count() > 0) {
115-
Dbg(dbg_ctl_proxyprotocol, "ioCompletionEvent: proxy protocol has a configured allowlist of trusted IPs - checking");
116-
if (!pp_ipmap->contains(swoc::IPAddr(netvc->get_remote_addr()))) {
114+
if (netvc->has_proxy_protocol_preface(reader)) {
115+
if (pp_ipmap->count() > 0) {
116+
Dbg(dbg_ctl_proxyprotocol, "ioCompletionEvent: proxy protocol has a configured allowlist of trusted IPs - checking");
117+
if (!pp_ipmap->contains(swoc::IPAddr(netvc->get_remote_addr()))) {
118+
Dbg(dbg_ctl_proxyprotocol,
119+
"ioCompletionEvent: Source IP is NOT in the configured allowlist of trusted IPs - closing connection");
120+
goto done;
121+
} else {
122+
char new_host[INET6_ADDRSTRLEN];
123+
Dbg(dbg_ctl_proxyprotocol, "ioCompletionEvent: Source IP [%s] is trusted in the allowlist for proxy protocol",
124+
ats_ip_ntop(netvc->get_remote_addr(), new_host, sizeof(new_host)));
125+
}
126+
} else {
117127
Dbg(dbg_ctl_proxyprotocol,
118-
"ioCompletionEvent: Source IP is NOT in the configured allowlist of trusted IPs - closing connection");
119-
goto done;
128+
"ioCompletionEvent: proxy protocol DOES NOT have a configured allowlist of trusted IPs but proxy protocol is "
129+
"enabled on this port - processing all connections with Proxy Protocol headers");
130+
}
131+
132+
HttpConfigParams *param = HttpConfig::acquire();
133+
int max_header_size = param->pp_hdr_max_size;
134+
HttpConfig::release(param);
135+
if (netvc->has_proxy_protocol(reader, max_header_size)) {
136+
Dbg(dbg_ctl_proxyprotocol, "ioCompletionEvent: http has proxy protocol header");
120137
} else {
121-
char new_host[INET6_ADDRSTRLEN];
122-
Dbg(dbg_ctl_proxyprotocol, "ioCompletionEvent: Source IP [%s] is trusted in the allowlist for proxy protocol",
123-
ats_ip_ntop(netvc->get_remote_addr(), new_host, sizeof(new_host)));
138+
Dbg(dbg_ctl_proxyprotocol,
139+
"ioCompletionEvent: proxy protocol preface was present, but Proxy Protocol header could not be parsed");
124140
}
125-
} else {
126-
Dbg(dbg_ctl_proxyprotocol,
127-
"ioCompletionEvent: proxy protocol DOES NOT have a configured allowlist of trusted IPs but proxy protocol is "
128-
"enabled on this port - processing all connections");
129-
}
130-
HttpConfigParams *param = HttpConfig::acquire();
131-
int max_header_size = param->pp_hdr_max_size;
132-
HttpConfig::release(param);
133-
if (netvc->has_proxy_protocol(reader, max_header_size)) {
134-
Dbg(dbg_ctl_proxyprotocol, "ioCompletionEvent: http has proxy protocol header");
135141
} else {
136142
Dbg(dbg_ctl_proxyprotocol, "ioCompletionEvent: proxy protocol was enabled, but Proxy Protocol header was not present");
137143
}

‎tests/gold_tests/proxy_protocol/proxy_protocol.test.py‎

Lines changed: 78 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -60,8 +60,8 @@ def setupTS(self, name, enable_cp):
6060
"proxy.config.http.insert_forwarded": "for|by=ip|proto",
6161
"proxy.config.http.insert_client_ip": 2,
6262
"proxy.config.http.insert_squid_x_forwarded_for": 1,
63-
"proxy.config.ssl.server.cert.path": f"{self.ts.Variables.SSLDir}",
64-
"proxy.config.ssl.server.private_key.path": f"{self.ts.Variables.SSLDir}",
63+
"proxy.config.ssl.server.cert.path": self.ts.Variables.SSLDir,
64+
"proxy.config.ssl.server.private_key.path": self.ts.Variables.SSLDir,
6565
"proxy.config.diags.debug.enabled": 1,
6666
"proxy.config.diags.debug.tags": "proxyprotocol",
6767
})
@@ -108,6 +108,79 @@ def run(self):
108108
self.checkAccessLog()
109109

110110

111+
class ProxyProtocolAllowlistTest:
112+
"""Test that the PROXY Protocol allowlist applies only to PP-prefaced traffic."""
113+
114+
replay_file = "replay/proxy_protocol_allowlist.replay.yaml"
115+
116+
def __init__(self):
117+
self.setupOriginServer()
118+
self.setupTS()
119+
120+
def setupOriginServer(self):
121+
self.server = Test.MakeVerifierServerProcess("pp-allowlist-server", self.replay_file)
122+
123+
def setupTS(self):
124+
self.ts = Test.MakeATSProcess("ts_pp_allowlist", enable_tls=True, enable_cache=False, enable_proxy_protocol=True)
125+
126+
self.ts.addDefaultSSLFiles()
127+
self.ts.Disk.ssl_multicert_yaml.AddLines(
128+
"""
129+
ssl_multicert:
130+
- dest_ip: "*"
131+
ssl_cert_name: server.pem
132+
ssl_key_name: server.key
133+
""".split("\n"))
134+
135+
self.ts.Disk.remap_config.AddLine(f"map / http://127.0.0.1:{self.server.Variables.http_port}/")
136+
137+
self.ts.Disk.records_config.update(
138+
{
139+
"proxy.config.http.proxy_protocol_allowlist": "192.0.2.1",
140+
"proxy.config.ssl.server.cert.path": self.ts.Variables.SSLDir,
141+
"proxy.config.ssl.server.private_key.path": self.ts.Variables.SSLDir,
142+
"proxy.config.diags.debug.enabled": 1,
143+
"proxy.config.diags.debug.tags": "proxyprotocol",
144+
})
145+
146+
def addCurlRun(self, name, args, return_code=0, expect_status=None, start_processes=False):
147+
tr = Test.AddTestRun(name)
148+
tr.TimeOut = 10
149+
tr.MakeCurlCommand(args, ts=self.ts)
150+
tr.Processes.Default.ReturnCode = return_code
151+
152+
if expect_status is not None:
153+
tr.Processes.Default.Streams.stdout = Testers.ContainsExpression(expect_status, f"Expected HTTP {expect_status}")
154+
155+
if start_processes:
156+
tr.Processes.Default.StartBefore(self.server)
157+
tr.Processes.Default.StartBefore(self.ts)
158+
159+
tr.StillRunningAfter = self.server
160+
tr.StillRunningAfter = self.ts
161+
162+
def run(self):
163+
self.addCurlRun(
164+
"Non-PP HTTP traffic bypasses proxy_protocol_allowlist",
165+
f'-sS -o /dev/null -w "%{{http_code}}" -H "uuid: 1" http://127.0.0.1:{self.ts.Variables.proxy_protocol_port}/get',
166+
expect_status="200",
167+
start_processes=True)
168+
self.addCurlRun(
169+
"Non-PP TLS traffic bypasses proxy_protocol_allowlist", f'-k -sS -o /dev/null -w "%{{http_code}}" -H "uuid: 2" '
170+
f'https://127.0.0.1:{self.ts.Variables.proxy_protocol_ssl_port}/get',
171+
expect_status="200")
172+
self.addCurlRun(
173+
"PP-prefaced HTTP traffic is rejected when peer is not allowlisted",
174+
f'-sS -o /dev/null --max-time 5 --haproxy-protocol '
175+
f'http://127.0.0.1:{self.ts.Variables.proxy_protocol_port}/get',
176+
return_code=Any(52, 56))
177+
self.addCurlRun(
178+
"PP-prefaced TLS traffic is rejected when peer is not allowlisted",
179+
f'-k -sS -o /dev/null --max-time 5 --haproxy-protocol '
180+
f'https://127.0.0.1:{self.ts.Variables.proxy_protocol_ssl_port}/get',
181+
return_code=Any(35, 52, 56))
182+
183+
111184
class ProxyProtocolOutTest:
112185
"""Test that ATS can send Proxy Protocol."""
113186

@@ -166,8 +239,8 @@ def setupTS(self, tr: 'TestRun') -> None:
166239

167240
self._ts.Disk.records_config.update(
168241
{
169-
"proxy.config.ssl.server.cert.path": f"{self._ts.Variables.SSLDir}",
170-
"proxy.config.ssl.server.private_key.path": f"{self._ts.Variables.SSLDir}",
242+
"proxy.config.ssl.server.cert.path": self._ts.Variables.SSLDir,
243+
"proxy.config.ssl.server.private_key.path": self._ts.Variables.SSLDir,
171244
"proxy.config.diags.debug.enabled": 1,
172245
"proxy.config.diags.debug.tags": "http|proxyprotocol",
173246
"proxy.config.http.proxy_protocol_out": self._pp_version,
@@ -240,6 +313,7 @@ def run(self) -> None:
240313

241314
ProxyProtocolInTest("nocp", False).run()
242315
ProxyProtocolInTest("cp", True).run()
316+
ProxyProtocolAllowlistTest().run()
243317

244318
# non-tunnling HTTP to origin
245319
ProxyProtocolOutTest(pp_version=-1, is_tunnel=False, is_tls_to_origin=False).run()

0 commit comments

Comments
 (0)