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 2022/08/08 23:45:08 UTC
[trafficserver] branch 9.1.x updated: Add back validatation that the scheme matches the wire protocol (#9006)
This is an automated email from the ASF dual-hosted git repository.
bneradt pushed a commit to branch 9.1.x
in repository https://gitbox.apache.org/repos/asf/trafficserver.git
The following commit(s) were added to refs/heads/9.1.x by this push:
new 45d15b135 Add back validatation that the scheme matches the wire protocol (#9006)
45d15b135 is described below
commit 45d15b135e8fbaf6f06e949e6f51a8d8788d16aa
Author: Brian Neradt <br...@gmail.com>
AuthorDate: Mon Aug 8 18:45:02 2022 -0500
Add back validatation that the scheme matches the wire protocol (#9006)
This adds back in the scheme and wire protocol check (see #8465) along
with a configuration to be able to disable the check if the verification
is not desired.
---
doc/admin-guide/files/records.config.en.rst | 23 +++++
mgmt/RecordsConfig.cc | 2 +
proxy/http/HttpConfig.cc | 2 +
proxy/http/HttpConfig.h | 3 +-
proxy/http/HttpSM.cc | 14 +++
.../forward_proxy/forward_proxy.replay.yaml | 21 ++++
.../gold_tests/forward_proxy/forward_proxy.test.py | 112 +++++++++++++++++++++
7 files changed, 176 insertions(+), 1 deletion(-)
diff --git a/doc/admin-guide/files/records.config.en.rst b/doc/admin-guide/files/records.config.en.rst
index 56aba60f1..c9e0c7e2e 100644
--- a/doc/admin-guide/files/records.config.en.rst
+++ b/doc/admin-guide/files/records.config.en.rst
@@ -3743,6 +3743,29 @@ Client-Related Configuration
if sni_policy = ``verify_with_name_source``, the sni will be the host header value and the name
to check in the server certificate will be the remap header value.
+.. ts:cv:: CONFIG proxy.config.ssl.client.scheme_proto_mismatch_policy INT 2
+ :overridable:
+
+ This option controls how |TS| behaves when the client side connection
+ protocol and the client request's scheme do not match. For example, if
+ enforcement is enabled by setting this value to ``2`` and the client
+ connection is a cleartext HTTP connection but the scheme of the URL is
+ ``https://``, then |TS| will emit a warning and return an immediate 400 HTTP
+ response without proxying the request to the origin.
+
+ The default value is ``2``, meaning that |TS| will enforce that the protocol
+ matches the scheme.
+
+ ===== ======================================================================
+ Value Description
+ ===== ======================================================================
+ ``0`` Disable verification that the protocol and scheme match.
+ ``1`` Check that the protocol and scheme match, but only emit a warning if
+ they do not.
+ ``2`` Check that the protocol and scheme match and, if they do not, emit a
+ warning and return an immediate HTTP 400 response.
+ ===== ======================================================================
+
.. ts:cv:: CONFIG proxy.config.ssl.client.TLSv1 INT 0
Enables (``1``) or disables (``0``) TLSv1.0 in the ATS client context. If not specified, enabled by default
diff --git a/mgmt/RecordsConfig.cc b/mgmt/RecordsConfig.cc
index 7f62fdce5..143b1448c 100644
--- a/mgmt/RecordsConfig.cc
+++ b/mgmt/RecordsConfig.cc
@@ -1138,6 +1138,8 @@ static const RecordElement RecordsConfig[] =
,
{RECT_CONFIG, "proxy.config.ssl.client.sni_policy", RECD_STRING, "host", RECU_RESTART_TS, RR_NULL, RECC_NULL, nullptr, RECA_NULL}
,
+ {RECT_CONFIG, "proxy.config.ssl.client.scheme_proto_mismatch_policy", RECD_INT, "2", RECU_DYNAMIC, RR_NULL, RECC_NULL, nullptr, RECA_NULL}
+ ,
{RECT_CONFIG, "proxy.config.ssl.session_cache", RECD_INT, "2", RECU_RESTART_TS, RR_NULL, RECC_NULL, nullptr, RECA_NULL}
,
{RECT_CONFIG, "proxy.config.ssl.session_cache.size", RECD_INT, "102400", RECU_RESTART_TS, RR_NULL, RECC_NULL, nullptr, RECA_NULL}
diff --git a/proxy/http/HttpConfig.cc b/proxy/http/HttpConfig.cc
index 439541ac4..ee311af3b 100644
--- a/proxy/http/HttpConfig.cc
+++ b/proxy/http/HttpConfig.cc
@@ -1363,6 +1363,7 @@ HttpConfig::startup()
HttpEstablishStaticConfigByte(c.http_host_sni_policy, "proxy.config.http.host_sni_policy");
HttpEstablishStaticConfigStringAlloc(c.oride.ssl_client_sni_policy, "proxy.config.ssl.client.sni_policy");
+ HttpEstablishStaticConfigByte(c.scheme_proto_mismatch_policy, "proxy.config.ssl.client.scheme_proto_mismatch_policy");
OutboundConnTrack::config_init(&c.outbound_conntrack, &c.oride.outbound_conntrack);
@@ -1645,6 +1646,7 @@ HttpConfig::reconfigure()
params->redirect_actions_string = ats_strdup(m_master.redirect_actions_string);
params->redirect_actions_map = parse_redirect_actions(params->redirect_actions_string, params->redirect_actions_self_action);
params->http_host_sni_policy = m_master.http_host_sni_policy;
+ params->scheme_proto_mismatch_policy = m_master.scheme_proto_mismatch_policy;
params->oride.ssl_client_sni_policy = ats_strdup(m_master.oride.ssl_client_sni_policy);
diff --git a/proxy/http/HttpConfig.h b/proxy/http/HttpConfig.h
index 11ebef6d0..14e3f3bc6 100644
--- a/proxy/http/HttpConfig.h
+++ b/proxy/http/HttpConfig.h
@@ -830,7 +830,8 @@ public:
MgmtInt http_request_line_max_size = 65535;
MgmtInt http_hdr_field_max_size = 131070;
- MgmtByte http_host_sni_policy = 0;
+ MgmtByte http_host_sni_policy = 0;
+ MgmtByte scheme_proto_mismatch_policy = 2;
// noncopyable
/////////////////////////////////////
diff --git a/proxy/http/HttpSM.cc b/proxy/http/HttpSM.cc
index e67ceb5c3..de0d23392 100644
--- a/proxy/http/HttpSM.cc
+++ b/proxy/http/HttpSM.cc
@@ -863,6 +863,20 @@ HttpSM::state_read_client_request_header(int event, void *data)
break;
}
+ if (!is_internal && t_state.http_config_param->scheme_proto_mismatch_policy != 0) {
+ auto scheme = t_state.hdr_info.client_request.url_get()->scheme_get_wksidx();
+ if ((client_connection_is_ssl && (scheme == URL_WKSIDX_HTTP || scheme == URL_WKSIDX_WS)) ||
+ (!client_connection_is_ssl && (scheme == URL_WKSIDX_HTTPS || scheme == URL_WKSIDX_WSS))) {
+ Warning("scheme [%s] vs. protocol [%s] mismatch", hdrtoken_index_to_wks(scheme),
+ client_connection_is_ssl ? "tls" : "plaintext");
+ if (t_state.http_config_param->scheme_proto_mismatch_policy == 2) {
+ t_state.http_return_code = HTTP_STATUS_BAD_REQUEST;
+ call_transact_and_set_next_state(HttpTransact::BadRequest);
+ break;
+ }
+ }
+ }
+
if (_from_early_data) {
// Only allow early data for safe methods defined in RFC7231 Section 4.2.1.
// https://tools.ietf.org/html/rfc7231#section-4.2.1
diff --git a/tests/gold_tests/forward_proxy/forward_proxy.replay.yaml b/tests/gold_tests/forward_proxy/forward_proxy.replay.yaml
new file mode 100644
index 000000000..926f5c8ad
--- /dev/null
+++ b/tests/gold_tests/forward_proxy/forward_proxy.replay.yaml
@@ -0,0 +1,21 @@
+meta:
+ version: "1.0"
+
+sessions:
+- transactions:
+ - all:
+ client-request:
+ method: "GET"
+ url: "/a/path"
+ version: "1.1"
+ headers:
+ fields:
+ - [ Host, example.com ]
+ - [ uuid, 1 ]
+
+ server-response:
+ status: 200
+ reason: OK
+ headers:
+ fields:
+ - [ Content-Length, 8 ]
diff --git a/tests/gold_tests/forward_proxy/forward_proxy.test.py b/tests/gold_tests/forward_proxy/forward_proxy.test.py
new file mode 100644
index 000000000..d631b874c
--- /dev/null
+++ b/tests/gold_tests/forward_proxy/forward_proxy.test.py
@@ -0,0 +1,112 @@
+"""
+"""
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+from typing import Union
+
+Test.Summary = 'Verify ATS can function as a forward proxy'
+Test.ContinueOnFail = True
+
+
+class ForwardProxyTest:
+ _scheme_proto_mismatch_policy: Union[int, None]
+ _ts_counter: int = 0
+ _server_counter: int = 0
+
+ def __init__(self, verify_scheme_matches_protocol: Union[int, None]):
+ """Construct a ForwardProxyTest object.
+
+ :param verify_scheme_matches_protocol: The value with which to
+ configure Traffic Server's
+ proxy.config.ssl.client.scheme_proto_mismatch_policy. A value of None
+ means that no value will be explicitly set in the records.config.
+ :type verify_scheme_matches_protocol: int or None
+ """
+ self._scheme_proto_mismatch_policy = verify_scheme_matches_protocol
+ self.setupOriginServer()
+ self.setupTS()
+
+ def setupOriginServer(self):
+ """Configure the Proxy Verifier server."""
+ proc_name = f"server{ForwardProxyTest._server_counter}"
+ self.server = Test.MakeVerifierServerProcess(proc_name, "forward_proxy.replay.yaml")
+ ForwardProxyTest._server_counter += 1
+ if self._scheme_proto_mismatch_policy in (2, None):
+ self.server.Streams.All = Testers.ExcludesExpression(
+ 'Received an HTTP/1 request with key 1',
+ 'Verify that the server did not receive the request.')
+ else:
+ self.server.Streams.All = Testers.ContainsExpression(
+ 'Received an HTTP/1 request with key 1',
+ 'Verify that the server received the request.')
+
+ def setupTS(self):
+ """Configure the Traffic Server process."""
+ proc_name = f"ts{ForwardProxyTest._ts_counter}"
+ self.ts = Test.MakeATSProcess(proc_name, enable_tls=True, enable_cache=False)
+ ForwardProxyTest._ts_counter += 1
+ self.ts.addDefaultSSLFiles()
+ self.ts.Disk.ssl_multicert_config.AddLine("dest_ip=* ssl_cert_name=server.pem ssl_key_name=server.key")
+ self.ts.Disk.remap_config.AddLine(
+ f"map / http://127.0.0.1:{self.server.Variables.http_port}/")
+
+ self.ts.Disk.records_config.update({
+ 'proxy.config.ssl.server.cert.path': self.ts.Variables.SSLDir,
+ 'proxy.config.ssl.server.private_key.path': self.ts.Variables.SSLDir,
+ 'proxy.config.ssl.client.verify.server.policy': 'PERMISSIVE',
+ 'proxy.config.ssl.keylog_file': '/tmp/keylog.txt',
+
+ 'proxy.config.diags.debug.enabled': 1,
+ 'proxy.config.diags.debug.tags': "http",
+ })
+
+ if self._scheme_proto_mismatch_policy is not None:
+ self.ts.Disk.records_config.update({
+ 'proxy.config.ssl.client.scheme_proto_mismatch_policy': self._scheme_proto_mismatch_policy,
+ })
+
+ def addProxyHttpsToHttpCase(self):
+ """Test ATS as an HTTPS forward proxy behind an HTTP server."""
+ tr = Test.AddTestRun()
+ tr.Processes.Default.StartBefore(self.server)
+ tr.Processes.Default.StartBefore(self.ts)
+ tr.Processes.Default.Command = (
+ f'curl --proxy-insecure -v -H "uuid: 1" '
+ f'--proxy "https://127.0.0.1:{self.ts.Variables.ssl_port}/" '
+ f'http://example.com/')
+ tr.Processes.Default.ReturnCode = 0
+ tr.StillRunningAfter = self.server
+ tr.StillRunningAfter = self.ts
+
+ if self._scheme_proto_mismatch_policy in (2, None):
+ tr.Processes.Default.Streams.All = Testers.ContainsExpression(
+ '< HTTP/1.1 400 Invalid HTTP Request',
+ 'Verify that the request was rejected.')
+ else:
+ tr.Processes.Default.Streams.All = Testers.ContainsExpression(
+ '< HTTP/1.1 200 OK',
+ 'Verify that curl received a 200 OK response.')
+
+ def run(self):
+ """Configure the TestRun instances for this set of tests."""
+ self.addProxyHttpsToHttpCase()
+
+
+ForwardProxyTest(verify_scheme_matches_protocol=None).run()
+ForwardProxyTest(verify_scheme_matches_protocol=0).run()
+ForwardProxyTest(verify_scheme_matches_protocol=1).run()
+ForwardProxyTest(verify_scheme_matches_protocol=2).run()