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 7942543081 Fix sni.yaml ip_allow tunnel_route (#10761)
7942543081 is described below
commit 7942543081d100d14f3aebec1000ed53b3fc578c
Author: Brian Neradt <[email protected]>
AuthorDate: Fri Nov 10 16:19:26 2023 -0600
Fix sni.yaml ip_allow tunnel_route (#10761)
When combining a rejection of an incoming connection via sni.yaml
ip_allow and tunnel_route, ATS still initiated an unneccessary
connection to the origin. This constituted an unused TLS handshake with
the origin because it would immediately be closed after the client side
handshake ended. This patch fixes ATS so it doesn't do anything on the
origin side if the client side TLS handshake failed already at the
CLIENT_HELLO.
---
src/iocore/net/SSLNetVConnection.cc | 70 +++++------
.../tls/replay/ip_allow_tunnel.replay.yaml | 130 +++++++++++++++++++++
tests/gold_tests/tls/tls_sni_ip_allow.test.py | 72 +++++++++---
3 files changed, 223 insertions(+), 49 deletions(-)
diff --git a/src/iocore/net/SSLNetVConnection.cc
b/src/iocore/net/SSLNetVConnection.cc
index acc219cf00..dbff62add3 100644
--- a/src/iocore/net/SSLNetVConnection.cc
+++ b/src/iocore/net/SSLNetVConnection.cc
@@ -21,6 +21,7 @@
limitations under the License.
*/
+#include "iocore/net/NetVConnection.h"
#include "tscore/ink_config.h"
#include "tscore/EventNotify.h"
#include "tscore/Layout.h"
@@ -617,42 +618,43 @@ SSLNetVConnection::net_read_io(NetHandler *nh, EThread
*lthread)
Dbg(dbg_ctl_ssl, "Restart for allow plain");
return;
}
- // If we have flipped to blind tunnel, don't read ahead
- if (this->handShakeReader) {
- if (this->attributes == HttpProxyPort::TRANSPORT_BLIND_TUNNEL) {
- // Now in blind tunnel. Set things up to read what is in the buffer
- // Must send the READ_COMPLETE here before considering
- // forwarding on the handshake buffer, so the
- // SSLNextProtocolTrampoline has a chance to do its
- // thing before forwarding the buffers.
- this->readSignalDone(VC_EVENT_READ_COMPLETE, nh);
-
- // If the handshake isn't set yet, this means the tunnel
- // decision was make in the SNI callback. We must move
- // the client hello message back into the standard read.vio
- // so it will get forwarded onto the origin server
- if (!this->getSSLHandShakeComplete()) {
- this->sslHandshakeStatus = SSLHandshakeStatus::SSL_HANDSHAKE_DONE;
-
- // Copy over all data already read in during the SSL_accept
- // (the client hello message)
- NetState *s = &this->read;
- MIOBufferAccessor &buf = s->vio.buffer;
- int64_t r = buf.writer()->write(this->handShakeHolder);
- s->vio.nbytes += r;
- s->vio.ndone += r;
-
- // Clean up the handshake buffers
- this->free_handshake_buffers();
-
- if (r > 0) {
- // Kick things again, so the data that was copied into the
- // vio.read buffer gets processed
- this->readSignalDone(VC_EVENT_READ_COMPLETE, nh);
- }
+ // If we have flipped to blind tunnel, don't read ahead. We check for a
+ // non-error return first, though, because if TLS has already failed with
+ // the CLIENT_HELLO, then there is no need to continue toward the origin
+ // with the blind tunnel.
+ if (ret != EVENT_ERROR && this->handShakeReader && this->attributes ==
HttpProxyPort::TRANSPORT_BLIND_TUNNEL) {
+ // Now in blind tunnel. Set things up to read what is in the buffer
+ // Must send the READ_COMPLETE here before considering
+ // forwarding on the handshake buffer, so the
+ // SSLNextProtocolTrampoline has a chance to do its
+ // thing before forwarding the buffers.
+ this->readSignalDone(VC_EVENT_READ_COMPLETE, nh);
+
+ // If the handshake isn't set yet, this means the tunnel
+ // decision was make in the SNI callback. We must move
+ // the client hello message back into the standard read.vio
+ // so it will get forwarded onto the origin server
+ if (!this->getSSLHandShakeComplete()) {
+ this->sslHandshakeStatus = SSLHandshakeStatus::SSL_HANDSHAKE_DONE;
+
+ // Copy over all data already read in during the SSL_accept
+ // (the client hello message)
+ NetState *s = &this->read;
+ MIOBufferAccessor &buf = s->vio.buffer;
+ int64_t r = buf.writer()->write(this->handShakeHolder);
+ s->vio.nbytes += r;
+ s->vio.ndone += r;
+
+ // Clean up the handshake buffers
+ this->free_handshake_buffers();
+
+ if (r > 0) {
+ // Kick things again, so the data that was copied into the
+ // vio.read buffer gets processed
+ this->readSignalDone(VC_EVENT_READ_COMPLETE, nh);
}
- return; // Leave if we are tunneling
}
+ return; // Leave if we are tunneling
}
if (ret == EVENT_ERROR) {
this->read.triggered = 0;
diff --git a/tests/gold_tests/tls/replay/ip_allow_tunnel.replay.yaml
b/tests/gold_tests/tls/replay/ip_allow_tunnel.replay.yaml
new file mode 100644
index 0000000000..aefd0b636e
--- /dev/null
+++ b/tests/gold_tests/tls/replay/ip_allow_tunnel.replay.yaml
@@ -0,0 +1,130 @@
+# 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.
+
+meta:
+ version: '1.0'
+
+sessions:
+
+- protocol:
+ - name: tls
+ sni: block.me.com
+ - name: tcp
+ - name: ip
+
+ transactions:
+
+ #
+ # This GET request should be tunneled per sni, but rejected per sni.yaml
ip_allow.
+ #
+ - client-request:
+ method: GET
+ version: "1.1"
+ url: www.example.com:80
+ headers:
+ fields:
+ - [ uuid, blocked-request ]
+ - [ Host, www.example.com:80 ]
+
+ # This should not reach the origin.
+ server-response:
+ status: 500
+
+ # We expect ATS to reject this connection.
+ proxy-response:
+ status: 404
+
+ # The following should not take place, but if it does, make the client fail.
+ - client-request:
+ method: GET
+ version: "1.1"
+ url: /get/something
+ headers:
+ fields:
+ - [ uuid, blocked-tunneled-request ]
+ - [ Host, www.example.com ]
+
+ server-response:
+ status: 200
+ reason: OK
+ headers:
+ fields:
+ - [ X-Response, blocked-tunneled-response ]
+
+ proxy-response:
+ status: 200
+ headers:
+ field:
+ # Make sure the client doesn't get this response.
+ - [ X-Response, { as: absent } ]
+
+- protocol:
+ - name: tls
+ sni: allow.me.com
+ - name: tcp
+ - name: ip
+
+ transactions:
+ #
+ # This should also be configured for tunneling, but allowed per sni.yaml
ip_allow.
+ #
+ - client-request:
+ method: GET
+ version: "1.1"
+ url: www.example.com:80
+ headers:
+ fields:
+ - [ uuid, allowed-request ]
+ - [ Host, www.example.com:80 ]
+
+ # This should be tunneled to the origin.
+ server-response:
+ status: 200
+ reason: OK
+ headers:
+ fields:
+ - [ X-Response, allowed-response ]
+
+ # We expect ATS to accept this connection and reply with a 200.
+ proxy-response:
+ status: 200
+ headers:
+ field:
+ - [ X-Response, { value: allowed-response, as: equal } ]
+
+ # Once the tunnel between client and server is established, subsequent
+ # requests will reach the server (via the tunnel).
+ - client-request:
+ method: GET
+ version: "1.1"
+ url: /get/something
+ headers:
+ fields:
+ - [ uuid, allowed-tunneled-request ]
+ - [ Host, www.example.com ]
+
+ server-response:
+ status: 200
+ reason: OK
+ headers:
+ fields:
+ - [ X-Response, allowed-tunneled-response ]
+
+ proxy-response:
+ status: 200
+ headers:
+ field:
+ - [ X-Response, { value: allowed-tunneled-response, as: equal } ]
diff --git a/tests/gold_tests/tls/tls_sni_ip_allow.test.py
b/tests/gold_tests/tls/tls_sni_ip_allow.test.py
index e8b3adccb4..eb56331219 100644
--- a/tests/gold_tests/tls/tls_sni_ip_allow.test.py
+++ b/tests/gold_tests/tls/tls_sni_ip_allow.test.py
@@ -20,22 +20,37 @@ Test exercising ip_allow configuration of sni.yaml
import os
-Test.Summary = '''
-Test exercising host and SNI mismatch controls
-'''
+Test.Summary = '''Test sni.yaml ip_allow.'''
+
+
+class ConnectionType:
+ GET = 0
+ TUNNEL = 1
class TestSniIpAllow:
'''Verify ip_allow of sni.yaml.'''
+ _dns_counter: int = 0
+ _server_counter: int = 0
+ _ts_counter: int = 0
+ _client_counter: int = 0
+
+ def __init__(self, connect_type: int) -> None:
+ """Configure a test run.
+ :param connect_type: The type of connection to use.
+ """
+ tr = Test.AddTestRun("Verify ip_allow of sni.yaml")
- _replay_file: str = "replay/ip_allow.replay.yaml"
+ if connect_type == ConnectionType.GET:
+ self._replay_file: str = "replay/ip_allow.replay.yaml"
+ elif connect_type == ConnectionType.TUNNEL:
+ self._replay_file: str = "replay/ip_allow_tunnel.replay.yaml"
+ else:
+ raise ValueError(f'Invalid connect_type: {connect_type}')
- def __init__(self) -> None:
- """Configure a test run."""
- tr = Test.AddTestRun("Verify ip_allow of sni.yaml")
self._dns = self._configure_dns(tr)
self._server = self._configure_server(tr)
- self._ts = self._configure_trafficserver(tr, self._dns, self._server)
+ self._ts = self._configure_trafficserver(tr, connect_type, self._dns,
self._server)
self._configure_client(tr, self._dns, self._server, self._ts)
def _configure_dns(self, tr: 'TestRun') -> 'Process':
@@ -43,7 +58,9 @@ class TestSniIpAllow:
:param tr: The TestRun to configure with the DNS.
:return: The DNS Process.
"""
- dns = tr.MakeDNServer("dns", default='127.0.0.1')
+ name = f'dns{TestSniIpAllow._dns_counter}'
+ dns = tr.MakeDNServer(name, default='127.0.0.1')
+ TestSniIpAllow._dns_counter += 1
return dns
def _configure_server(self, tr: 'TestRun') -> 'Process':
@@ -51,37 +68,55 @@ class TestSniIpAllow:
:param tr: The TestRun to configure with the Origin Server.
:return: The Origin Server Process.
"""
- server = tr.AddVerifierServerProcess("server", self._replay_file)
+ name = f'server{TestSniIpAllow._server_counter}'
+ server = tr.AddVerifierServerProcess(name, self._replay_file)
+ TestSniIpAllow._server_counter += 1
server.Streams.All += Testers.ContainsExpression(
'allowed-request',
'The allowed request should be recieved.')
server.Streams.All += Testers.ExcludesExpression(
'blocked-request',
'The blocked request should not have been recieved.')
+ server.Streams.All += Testers.ExcludesExpression(
+ 'block.me.com',
+ 'Nothing about the block.me.com sni should have been recieved.')
return server
- def _configure_trafficserver(self, tr: 'TestRun', dns: 'Process', server:
'Process') -> 'Process':
+ def _configure_trafficserver(self, tr: 'TestRun', connect_type: int, dns:
'Process', server: 'Process') -> 'Process':
"""Configure Traffic Server for the TestRun.
:param tr: The TestRun to configure with Traffic Server.
+ :param connect_type: The type of connection to use.
:param dns: The DNS Process.
:param server: The Origin Server Process.
:return: The Traffic Server Process.
"""
- ts = tr.MakeATSProcess("ts", enable_tls=True, enable_cache=False)
+ name = f'ts{TestSniIpAllow._ts_counter}'
+ ts = tr.MakeATSProcess(name, enable_tls=True, enable_cache=False)
+ TestSniIpAllow._ts_counter += 1
ts.Disk.sni_yaml.AddLines([
'sni:',
'- fqdn: block.me.com',
' ip_allow: 192.168.10.1', # Therefore 127.0.0.1 should be
blocked.
+ ])
+ if connect_type == ConnectionType.TUNNEL:
+ ts.Disk.sni_yaml.AddLines([
+ f' tunnel_route:
backend.server.com:{server.Variables.https_port}',
+ ])
+ ts.Disk.sni_yaml.AddLines([
'- fqdn: allow.me.com',
' ip_allow: 127.0.0.1'
])
+ if connect_type == ConnectionType.TUNNEL:
+ ts.Disk.sni_yaml.AddLines([
+ f' tunnel_route:
backend.server.com:{server.Variables.https_port}',
+ ])
ts.addDefaultSSLFiles()
ts.Disk.ssl_multicert_config.AddLine(
'dest_ip=* ssl_cert_name=server.pem ssl_key_name=server.key'
)
ts.Disk.remap_config.AddLine(
- f'map / http://backend.server.com:{server.Variables.http_port}/'
+ f'map /
http://remapped.backend.server.com:{server.Variables.http_port}/'
)
ts.Disk.records_config.update({
'proxy.config.ssl.server.cert.path': ts.Variables.SSLDir,
@@ -94,6 +129,10 @@ class TestSniIpAllow:
'proxy.config.diags.debug.enabled': 1,
'proxy.config.diags.debug.tags': 'http|ssl',
})
+ if connect_type == ConnectionType.TUNNEL:
+ ts.Disk.records_config.update({
+ 'proxy.config.http.connect_ports':
f"{server.Variables.https_port}",
+ })
return ts
def _configure_client(self, tr: 'TestRun', dns: 'Process', server:
'Process', ts: 'Process') -> None:
@@ -103,11 +142,13 @@ class TestSniIpAllow:
:param server: The Origin Server Process.
:param ts: The Traffic Server Process.
"""
+ name = f'client{TestSniIpAllow._client_counter}'
p = tr.AddVerifierClientProcess(
- 'client',
+ name,
self._replay_file,
http_ports=[ts.Variables.port],
https_ports=[ts.Variables.ssl_port])
+ TestSniIpAllow._client_counter += 1
ts.StartBefore(server)
ts.StartBefore(dns)
p.StartBefore(ts)
@@ -124,4 +165,5 @@ class TestSniIpAllow:
'The response to the blocked request should not have been
recieved.')
-TestSniIpAllow()
+TestSniIpAllow(ConnectionType.GET)
+TestSniIpAllow(ConnectionType.TUNNEL)
