Skip to content

Commit f21e8ec

Browse files
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 dcec715 commit f21e8ec

8 files changed

Lines changed: 235 additions & 62 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
@@ -47,10 +47,13 @@ configured with :ts:cv:`proxy.config.http.proxy_protocol_allowlist`.
4747

4848
.. important::
4949

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

5558
By default, |TS| uses client's IP address that is from the peer when it applies ACL. If you configure a port to
5659
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
@@ -2168,10 +2168,13 @@ Proxy User Variables
21682168

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

21762179
======================= ===========================================================
21772180
Example Effect

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: 31 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -353,41 +353,43 @@ 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 dst;
380-
dst.sa = *(this->get_proxy_protocol_dst_addr());
381-
ip_port_text_buffer ipb1;
382-
ats_ip_nptop(&dst, ipb1, sizeof(ipb1));
383-
DbgPrint(dbg_ctl_proxyprotocol, "ssl_has_proxy_v1, dest IP received [%s]", ipb1);
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 dst;
382+
dst.sa = *(this->get_proxy_protocol_dst_addr());
383+
ip_port_text_buffer ipb1;
384+
ats_ip_nptop(&dst, ipb1, sizeof(ipb1));
385+
DbgPrint(dbg_ctl_proxyprotocol, "ssl_has_proxy_v1, dest IP received [%s]", ipb1);
386+
}
387+
} else {
388+
Dbg(dbg_ctl_proxyprotocol, "proxy protocol preface was present, but Proxy Protocol header could not be parsed");
389+
r = stored_r;
384390
}
385391
} else {
386392
Dbg(dbg_ctl_proxyprotocol, "proxy protocol was enabled, but Proxy Protocol header was not present");
387-
// We are flexible with the Proxy Protocol designation. Maybe not all
388-
// connections include Proxy Protocol. Revert to the stored value of r so
389-
// we can process the bytes that are on the wire (likely a CLIENT_HELLO).
390-
r = stored_r;
391393
}
392394
}
393395
} // 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
@@ -58,8 +58,8 @@ def setupTS(self, name, enable_cp):
5858
{
5959
"proxy.config.http.proxy_protocol_allowlist": "127.0.0.1",
6060
"proxy.config.http.insert_forwarded": "for|by=ip|proto",
61-
"proxy.config.ssl.server.cert.path": f"{self.ts.Variables.SSLDir}",
62-
"proxy.config.ssl.server.private_key.path": f"{self.ts.Variables.SSLDir}",
61+
"proxy.config.ssl.server.cert.path": self.ts.Variables.SSLDir,
62+
"proxy.config.ssl.server.private_key.path": self.ts.Variables.SSLDir,
6363
"proxy.config.diags.debug.enabled": 1,
6464
"proxy.config.diags.debug.tags": "proxyprotocol",
6565
})
@@ -106,6 +106,79 @@ def run(self):
106106
self.checkAccessLog()
107107

108108

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

@@ -164,8 +237,8 @@ def setupTS(self, tr: 'TestRun') -> None:
164237

165238
self._ts.Disk.records_config.update(
166239
{
167-
"proxy.config.ssl.server.cert.path": f"{self._ts.Variables.SSLDir}",
168-
"proxy.config.ssl.server.private_key.path": f"{self._ts.Variables.SSLDir}",
240+
"proxy.config.ssl.server.cert.path": self._ts.Variables.SSLDir,
241+
"proxy.config.ssl.server.private_key.path": self._ts.Variables.SSLDir,
169242
"proxy.config.diags.debug.enabled": 1,
170243
"proxy.config.diags.debug.tags": "http|proxyprotocol",
171244
"proxy.config.http.proxy_protocol_out": self._pp_version,
@@ -238,6 +311,7 @@ def run(self) -> None:
238311

239312
ProxyProtocolInTest("nocp", False).run()
240313
ProxyProtocolInTest("cp", True).run()
314+
ProxyProtocolAllowlistTest().run()
241315

242316
# non-tunnling HTTP to origin
243317
ProxyProtocolOutTest(pp_version=-1, is_tunnel=False, is_tls_to_origin=False).run()
Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
# Licensed to the Apache Software Foundation (ASF) under one
2+
# or more contributor license agreements. See the NOTICE file
3+
# distributed with this work for additional information
4+
# regarding copyright ownership. The ASF licenses this file
5+
# to you under the Apache License, Version 2.0 (the
6+
# "License"); you may not use this file except in compliance
7+
# with the License. You may obtain a copy of the License at
8+
#
9+
# http://www.apache.org/licenses/LICENSE-2.0
10+
#
11+
# Unless required by applicable law or agreed to in writing, software
12+
# distributed under the License is distributed on an "AS IS" BASIS,
13+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
# See the License for the specific language governing permissions and
15+
# limitations under the License.
16+
17+
meta:
18+
version: "1.0"
19+
20+
sessions:
21+
- protocol:
22+
stack: http
23+
transactions:
24+
- client-request:
25+
method: GET
26+
version: "1.1"
27+
url: /get
28+
headers:
29+
fields:
30+
- [uuid, 1]
31+
32+
server-response:
33+
status: 200
34+
35+
proxy-response:
36+
status: 200
37+
38+
- protocol:
39+
stack: https
40+
transactions:
41+
- client-request:
42+
method: GET
43+
version: "1.1"
44+
url: /get
45+
headers:
46+
fields:
47+
- [uuid, 2]
48+
49+
server-response:
50+
status: 200
51+
52+
proxy-response:
53+
status: 200

0 commit comments

Comments
 (0)