You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by bc...@apache.org on 2022/08/09 13:23:32 UTC
[trafficserver] branch master updated: Add back validatation that the scheme matches the wire protocol (#9005)
This is an automated email from the ASF dual-hosted git repository.
bcall 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 7ec147ec1 Add back validatation that the scheme matches the wire protocol (#9005)
7ec147ec1 is described below
commit 7ec147ec1d918c60ff0f1f92591a40441a335c44
Author: Brian Neradt <br...@gmail.com>
AuthorDate: Tue Aug 9 08:23:25 2022 -0500
Add back validatation that the scheme matches the wire protocol (#9005)
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 | 3 -
.../gold_tests/forward_proxy/forward_proxy.test.py | 68 +++++++++++++++++-----
7 files changed, 95 insertions(+), 20 deletions(-)
diff --git a/doc/admin-guide/files/records.config.en.rst b/doc/admin-guide/files/records.config.en.rst
index ed7d3b97d..c3dccaca7 100644
--- a/doc/admin-guide/files/records.config.en.rst
+++ b/doc/admin-guide/files/records.config.en.rst
@@ -3983,6 +3983,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 05c71811c..8cb8825e7 100644
--- a/mgmt/RecordsConfig.cc
+++ b/mgmt/RecordsConfig.cc
@@ -1180,6 +1180,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.origin_session_cache", RECD_INT, "1", RECU_RESTART_TS, RR_NULL, RECC_NULL, nullptr, RECA_NULL}
,
{RECT_CONFIG, "proxy.config.ssl.origin_session_cache.size", RECD_INT, "10240", RECU_RESTART_TS, RR_NULL, RECC_NULL, nullptr, RECA_NULL}
diff --git a/proxy/http/HttpConfig.cc b/proxy/http/HttpConfig.cc
index 7fe46cef7..5b97d15aa 100644
--- a/proxy/http/HttpConfig.cc
+++ b/proxy/http/HttpConfig.cc
@@ -1415,6 +1415,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.global_outbound_conntrack, &c.oride.outbound_conntrack);
@@ -1706,6 +1707,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 9f8d3138b..469b1c720 100644
--- a/proxy/http/HttpConfig.h
+++ b/proxy/http/HttpConfig.h
@@ -859,7 +859,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 baaabc4b5..7ffbf4cc6 100644
--- a/proxy/http/HttpSM.cc
+++ b/proxy/http/HttpSM.cc
@@ -853,6 +853,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
index d744fa38a..926f5c8ad 100644
--- a/tests/gold_tests/forward_proxy/forward_proxy.replay.yaml
+++ b/tests/gold_tests/forward_proxy/forward_proxy.replay.yaml
@@ -19,6 +19,3 @@ sessions:
headers:
fields:
- [ Content-Length, 8 ]
-
- proxy-response:
- status: 200
diff --git a/tests/gold_tests/forward_proxy/forward_proxy.test.py b/tests/gold_tests/forward_proxy/forward_proxy.test.py
index 2cf63e2b7..d631b874c 100644
--- a/tests/gold_tests/forward_proxy/forward_proxy.test.py
+++ b/tests/gold_tests/forward_proxy/forward_proxy.test.py
@@ -1,5 +1,5 @@
-'''
-'''
+"""
+"""
# 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
@@ -16,25 +16,49 @@
# See the License for the specific language governing permissions and
# limitations under the License.
-import sys
+from typing import Union
Test.Summary = 'Verify ATS can function as a forward proxy'
Test.ContinueOnFail = True
class ForwardProxyTest:
- def __init__(self):
+ _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):
- self.server = Test.MakeVerifierServerProcess("server", "forward_proxy.replay.yaml")
- self.server.Streams.All = Testers.ContainsExpression(
- 'Received an HTTP/1 request with key 1',
- 'Verify that the server received the request.')
+ """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):
- self.ts = Test.MakeATSProcess("ts", enable_tls=True, enable_cache=False)
+ """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(
@@ -50,10 +74,13 @@ class ForwardProxyTest:
'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.
- """
+ """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)
@@ -65,12 +92,21 @@ class ForwardProxyTest:
tr.StillRunningAfter = self.server
tr.StillRunningAfter = self.ts
- tr.Processes.Default.Streams.All = Testers.ContainsExpression(
- '< HTTP/1.1 200 OK',
- 'Verify that curl received a 200 OK response.')
+ 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().run()
+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()