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.

(cherry picked from commit 7ec147e)

Author: bneradt

doc/admin-guide/files/records.config.en.rst

@@ -3938,6 +3938,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

mgmt/RecordsConfig.cc

@@ -1176,6 +1176,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}

proxy/http/HttpConfig.cc

@@ -1411,6 +1411,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);
 
@@ -1702,6 +1703,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);
 

proxy/http/HttpConfig.h

@@ -856,7 +856,8 @@ struct HttpConfigParams : public ConfigInfo {
   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
   /////////////////////////////////////

proxy/http/HttpSM.cc

@@ -849,6 +849,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

tests/gold_tests/forward_proxy/forward_proxy.replay.yaml

@@ -19,6 +19,3 @@ sessions:
       headers:
         fields:
         - [ Content-Length, 8 ]
-
-    proxy-response:
-      status: 200

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 @@ def setupTS(self):
             '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 @@ def addProxyHttpsToHttpCase(self):
         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()