You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by bn...@apache.org on 2023/02/07 19:41:31 UTC
[trafficserver] branch master updated: Proxy Protocol: Allow for no Proxy Protocol header (#9383)
This is an automated email from the ASF dual-hosted git repository.
bneradt pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/trafficserver.git
The following commit(s) were added to refs/heads/master by this push:
new 5d9c75e714 Proxy Protocol: Allow for no Proxy Protocol header (#9383)
5d9c75e714 is described below
commit 5d9c75e7143cdc6672b8d5b7cffd786442e1a238
Author: Brian Neradt <br...@gmail.com>
AuthorDate: Tue Feb 7 13:41:22 2023 -0600
Proxy Protocol: Allow for no Proxy Protocol header (#9383)
Prior to this patch, if the user specified that a listening port may
have Proxy Protocol headers on connections via `pp` in the server_ports
configuration, the connection would be dropped if such a header was not
sent. This makes ATS flexible in such situations: if the Proxy Protocol
port is there, then it is processed, otherwise the connection is
processed according to the other specifications in the server_ports
configuration for the port.
---
.../configuration/proxy-protocol.en.rst | 14 +++++++----
iocore/net/ProxyProtocol.cc | 2 +-
iocore/net/SSLNetVConnection.cc | 8 +++++--
proxy/ProtocolProbeSessionAccept.cc | 5 +---
tests/gold_tests/proxy_protocol/gold/access.gold | 2 ++
.../proxy_protocol/gold/test_case_3_stderr.gold | 10 ++++++++
.../proxy_protocol/gold/test_case_3_stdout.gold | 9 +++++++
.../proxy_protocol/gold/test_case_4_stderr.gold | 10 ++++++++
.../proxy_protocol/gold/test_case_4_stdout.gold | 9 +++++++
.../proxy_protocol/proxy_protocol.test.py | 28 ++++++++++++++++++++++
10 files changed, 85 insertions(+), 12 deletions(-)
diff --git a/doc/admin-guide/configuration/proxy-protocol.en.rst b/doc/admin-guide/configuration/proxy-protocol.en.rst
index a83fe5b137..5c5907988d 100644
--- a/doc/admin-guide/configuration/proxy-protocol.en.rst
+++ b/doc/admin-guide/configuration/proxy-protocol.en.rst
@@ -33,11 +33,13 @@ TLS connections.
The current implementation doesn't support TLV fields of Version 2.
-The Proxy Protocol must be enabled on each port. See
+The Proxy Protocol must be enabled on each port for which connections with the
+Proxy Protocol header are expected. See
:ts:cv:`proxy.config.http.server_ports` for information on how to enable the
-Proxy Protocol on a port. Once enabled, all incoming requests must be prefaced
-with the PROXY v1/v2 header. Any request not preface by this header will be
-dropped.
+Proxy Protocol on a port. Once enabled, incoming requests may be prefaced with
+either the PROXY v1 or v2 header. Any request not prefaced by this header will
+be handled according to the other directives in the associated port
+configuration.
As a security measure, an optional list of trusted IP addresses may be
configured with :ts:cv:`proxy.config.http.proxy_protocol_allowlist`.
@@ -45,7 +47,9 @@ configured with :ts:cv:`proxy.config.http.proxy_protocol_allowlist`.
.. important::
If the allowlist is configured, requests will only be accepted from these
- IP addresses and must be prefaced with the PROXY v1/v2 header.
+ IP addresses for all ports designated for Proxy Protocol in the
+ :ts:cv:`proxy.config.http.server_ports` configuration, regardless of whether
+ the connections have the Proxy Protocol header.
1. HTTP Forwarded Header
diff --git a/iocore/net/ProxyProtocol.cc b/iocore/net/ProxyProtocol.cc
index c18f563b88..5c431f226c 100644
--- a/iocore/net/ProxyProtocol.cc
+++ b/iocore/net/ProxyProtocol.cc
@@ -465,7 +465,7 @@ proxy_protocol_parse(ProxyProtocol *pp_info, ts::TextView tv)
} else {
// if we don't have the PROXY preface, we don't have a ProxyProtocol header
// TODO: print hexdump of buffer safely
- Debug("proxyprotocol", "failed to find ProxyProtocol preface");
+ Debug("proxyprotocol", "failed to find ProxyProtocol preface in the first %zu bytes", tv.size());
}
return len;
diff --git a/iocore/net/SSLNetVConnection.cc b/iocore/net/SSLNetVConnection.cc
index 0e118b7fa5..db355bb80e 100644
--- a/iocore/net/SSLNetVConnection.cc
+++ b/iocore/net/SSLNetVConnection.cc
@@ -453,6 +453,7 @@ SSLNetVConnection::read_raw_data()
"proxy protocol is enabled on this port - processing all connections");
}
+ auto const stored_r = r;
if (this->has_proxy_protocol(buffer, &r)) {
Debug("proxyprotocol", "ssl has proxy protocol header");
if (is_debug_tag_set("proxyprotocol")) {
@@ -463,8 +464,11 @@ SSLNetVConnection::read_raw_data()
Debug("proxyprotocol", "ssl_has_proxy_v1, dest IP received [%s]", ipb1);
}
} else {
- Debug("proxyprotocol", "proxy protocol was enabled, but required header was not present in the "
- "transaction - closing connection");
+ Debug("proxyprotocol", "proxy protocol was enabled, but Proxy Protocol header was not present");
+ // We are flexible with the Proxy Protocol designation. Maybe not all
+ // connections include Proxy Protocol. Revert to the stored value of r so
+ // we can process the bytes that are on the wire (likely a CLIENT_HELLO).
+ r = stored_r;
}
} // end of Proxy Protocol processing
diff --git a/proxy/ProtocolProbeSessionAccept.cc b/proxy/ProtocolProbeSessionAccept.cc
index dad9de9c9c..59e0767bb8 100644
--- a/proxy/ProtocolProbeSessionAccept.cc
+++ b/proxy/ProtocolProbeSessionAccept.cc
@@ -124,10 +124,7 @@ struct ProtocolProbeTrampoline : public Continuation, public ProtocolProbeSessio
if (netvc->has_proxy_protocol(reader)) {
Debug("proxyprotocol", "ioCompletionEvent: http has proxy protocol header");
} else {
- Debug("proxyprotocol",
- "ioCompletionEvent: proxy protocol was enabled, but required header was not present in the transaction - "
- "closing connection");
- goto done;
+ Debug("proxyprotocol", "ioCompletionEvent: proxy protocol was enabled, but Proxy Protocol header was not present");
}
} // end of Proxy Protocol processing
diff --git a/tests/gold_tests/proxy_protocol/gold/access.gold b/tests/gold_tests/proxy_protocol/gold/access.gold
index 54d08479dc..dcae156a3c 100644
--- a/tests/gold_tests/proxy_protocol/gold/access.gold
+++ b/tests/gold_tests/proxy_protocol/gold/access.gold
@@ -1,3 +1,5 @@
127.0.0.1 127.0.0.1
127.0.0.1 127.0.0.1
127.0.0.1 198.51.100.1
+127.0.0.1 0
+127.0.0.1 0
diff --git a/tests/gold_tests/proxy_protocol/gold/test_case_3_stderr.gold b/tests/gold_tests/proxy_protocol/gold/test_case_3_stderr.gold
new file mode 100644
index 0000000000..f7ae430791
--- /dev/null
+++ b/tests/gold_tests/proxy_protocol/gold/test_case_3_stderr.gold
@@ -0,0 +1,10 @@
+``
+> GET /get HTTP/1.1
+> Host: localhost:``
+> User-Agent: curl/``
+``
+< HTTP/1.1 200 OK
+< Server: ATS/``
+< Date: ``
+< Age: ``
+``
diff --git a/tests/gold_tests/proxy_protocol/gold/test_case_3_stdout.gold b/tests/gold_tests/proxy_protocol/gold/test_case_3_stdout.gold
new file mode 100644
index 0000000000..ad84f31d9d
--- /dev/null
+++ b/tests/gold_tests/proxy_protocol/gold/test_case_3_stdout.gold
@@ -0,0 +1,9 @@
+{
+``
+ "headers": {
+``
+ "Forwarded": "for=127.0.0.1;by=127.0.0.1;proto=http",
+``
+ },
+``
+}
diff --git a/tests/gold_tests/proxy_protocol/gold/test_case_4_stderr.gold b/tests/gold_tests/proxy_protocol/gold/test_case_4_stderr.gold
new file mode 100644
index 0000000000..f7ae430791
--- /dev/null
+++ b/tests/gold_tests/proxy_protocol/gold/test_case_4_stderr.gold
@@ -0,0 +1,10 @@
+``
+> GET /get HTTP/1.1
+> Host: localhost:``
+> User-Agent: curl/``
+``
+< HTTP/1.1 200 OK
+< Server: ATS/``
+< Date: ``
+< Age: ``
+``
diff --git a/tests/gold_tests/proxy_protocol/gold/test_case_4_stdout.gold b/tests/gold_tests/proxy_protocol/gold/test_case_4_stdout.gold
new file mode 100644
index 0000000000..cac9e4fea4
--- /dev/null
+++ b/tests/gold_tests/proxy_protocol/gold/test_case_4_stdout.gold
@@ -0,0 +1,9 @@
+{
+``
+ "headers": {
+``
+ "Forwarded": "for=127.0.0.1;by=127.0.0.1;proto=https",
+``
+ },
+``
+}
diff --git a/tests/gold_tests/proxy_protocol/proxy_protocol.test.py b/tests/gold_tests/proxy_protocol/proxy_protocol.test.py
index ed58315f60..f0b5f1ff83 100644
--- a/tests/gold_tests/proxy_protocol/proxy_protocol.test.py
+++ b/tests/gold_tests/proxy_protocol/proxy_protocol.test.py
@@ -108,6 +108,32 @@ logging:
tr.StillRunningAfter = self.httpbin
tr.StillRunningAfter = self.ts
+ def addTestCase3(self):
+ """
+ Verify ATS with :pp: server_ports designation can handle a connection
+ without Proxy Protocol.
+ """
+ tr = Test.AddTestRun()
+ tr.Processes.Default.Command = f"curl -vs --http1.1 http://localhost:{self.ts.Variables.proxy_protocol_port}/get | {self.json_printer}"
+ tr.Processes.Default.ReturnCode = 0
+ tr.Processes.Default.Streams.stdout = "gold/test_case_3_stdout.gold"
+ tr.Processes.Default.Streams.stderr = "gold/test_case_3_stderr.gold"
+ tr.StillRunningAfter = self.httpbin
+ tr.StillRunningAfter = self.ts
+
+ def addTestCase4(self):
+ """
+ Verify ATS with :pp:ssl server_ports designation can handle a TLS
+ connection without Proxy Protocol.
+ """
+ tr = Test.AddTestRun()
+ tr.Processes.Default.Command = f"curl -vsk --http1.1 https://localhost:{self.ts.Variables.proxy_protocol_ssl_port}/get | {self.json_printer}"
+ tr.Processes.Default.ReturnCode = 0
+ tr.Processes.Default.Streams.stdout = "gold/test_case_4_stdout.gold"
+ tr.Processes.Default.Streams.stderr = "gold/test_case_4_stderr.gold"
+ tr.StillRunningAfter = self.httpbin
+ tr.StillRunningAfter = self.ts
+
def addTestCase99(self):
"""
check access log
@@ -126,6 +152,8 @@ logging:
self.addTestCase0()
self.addTestCase1()
self.addTestCase2()
+ self.addTestCase3()
+ self.addTestCase4()
self.addTestCase99()