This is an automated email from the ASF dual-hosted git repository.
cmcfarlen 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 b9b9109864 Deliver VCONN_CLOSE for parked TLS hooks; fix SNI queue
accounting (#13406)
b9b9109864 is described below
commit b9b9109864c3402ac61885728cffde1d32e2154f
Author: Mo Chen <[email protected]>
AuthorDate: Fri Aug 7 13:51:28 2026 -0500
Deliver VCONN_CLOSE for parked TLS hooks; fix SNI queue accounting (#13406)
* rate_limit: balance the SNI active-slot counter for queued connections
A queued SNI connection never reserves a slot, but its VCONN_CLOSE
released one unconditionally. A queued connection that closed therefore
decremented the active-slot counter without a matching increment; it
wrapped below zero and the next reserve() aborted the server on
TSReleaseAssert(_active <= _limit).
Balance the accounting: resume queued connections with reserve-then-pop
so a resumed connection owns a real slot; release a slot on close only
when the connection is no longer queued (a still-queued one never held
one) and drop it from the queue; detach an expired connection the same
way the reject path does. Removing a closing connection from the queue
also fixes a stale-pointer dereference when a parked queued connection
is reset.
Add deterministic regressions for the resume and max_age paths.
* rate_limit: add an SNI reject-teardown autest
Exercise the sync-reject path against a TLS listener: a holder reserves
the one slot and a burst of concurrent handshakes is rejected
mid-handshake (TS_EVENT_ERROR) with the allocator freelists disabled.
Asserts the reject path is reached and every rejected handshake VC is
freed without a memory-safety fault.
* rate_limit tests: annotate helpers and create the FIFO atomically
Annotate the TestRun parameters like the surrounding class-based gold
tests, and create the holder FIFO inside a fresh mktemp -d directory
instead of on an unlinked mktemp -u path, whose creation is not atomic.
* Deliver VCONN_CLOSE for connections parked in a TLS handshake hook
callHooks() moves the hook state to DONE when a connection closes, but it
kept curHook pointing into whichever handshake hook list the connection
was parked in. Each hook id owns a separate list, so advancing curHook
walked the handshake list rather than the close list: the close event was
dropped once that list ran out, and delivered to the next handshake
plugin when it did not.
A plugin that parks a connection therefore never learns that it died. In
the rate_limit SNI queue that leaves a freed TSVConn on the queue and
leaks the selector lease, and the next sweep reenables freed memory.
Restart from the head of the close hook list unless we are already
iterating it. Take the same path for TS_EVENT_VCONN_OUTBOUND_CLOSE, which
previously invoked nothing at all for a connection parked in the outbound
pre-handshake hook.
* rate_limit: address review feedback
Drop the dependency on coreutils "timeout", which is absent on macOS and
made
the gold tests fail rather than skip there, and which was relied on for
fractional deadlines. A small sleep-and-kill helper replaces it. Also drop
-verify_quiet, which is redundant with -quiet and is not accepted by every
s_client implementation.
Take the element by const reference in RateLimiter::remove(), and record
what
bounds the scan: the configured queue size, or connections_throttle when a
"queue" is given without a "size".
Correct the queue test's narration. It described the counter wrapping and
the
probe aborting the server, which is what happened before 508c1bea26 fixed
the
sweep's resume condition; the test now pins that fix rather than
reproducing it.
---
plugins/experimental/rate_limit/limiter.h | 24 ++++++
plugins/experimental/rate_limit/sni_limiter.cc | 7 +-
plugins/experimental/rate_limit/sni_selector.cc | 21 ++++-
src/iocore/net/TLSEventSupport.cc | 12 ++-
.../rate_limit/rate_limit_sni_expiry.test.py | 87 +++++++++++++++++++++
.../rate_limit/rate_limit_sni_expiry_client.sh | 77 +++++++++++++++++++
.../rate_limit/rate_limit_sni_queue.test.py | 89 ++++++++++++++++++++++
.../rate_limit/rate_limit_sni_queue_client.sh | 78 +++++++++++++++++++
.../rate_limit/rate_limit_sni_reject.test.py | 83 ++++++++++++++++++++
.../rate_limit/rate_limit_sni_reject_client.sh | 60 +++++++++++++++
.../tls_hooks/tls_hooks_close_while_parked.test.py | 88 +++++++++++++++++++++
11 files changed, 621 insertions(+), 5 deletions(-)
diff --git a/plugins/experimental/rate_limit/limiter.h
b/plugins/experimental/rate_limit/limiter.h
index 6696274faf..038c295faf 100644
--- a/plugins/experimental/rate_limit/limiter.h
+++ b/plugins/experimental/rate_limit/limiter.h
@@ -327,6 +327,30 @@ public:
return item;
}
+ // Remove a still-queued element (e.g. a connection that closed before it
was resumed).
+ // Returns true if it was found in the queue, so the caller can tell a
queued element
+ // (which never reserved a slot) from one that was already resumed.
+ //
+ // Linear in the queue depth, and only reached when an element closes while
queued, which
+ // requires the limiter to be at its limit. The depth is bounded by the
configured queue size;
+ // note that a "queue" without a "size" leaves _max_queue at UINT32_MAX, in
which case the only
+ // bound is proxy.config.net.connections_throttle.
+ bool
+ remove(const T &elem)
+ {
+ std::lock_guard<std::mutex> lock(_queue_lock);
+
+ for (auto it = _queue.begin(); it != _queue.end(); ++it) {
+ if (std::get<0>(*it) == elem) {
+ _queue.erase(it);
+ --_size;
+ return true;
+ }
+ }
+
+ return false;
+ }
+
void
incrementMetric(uint metric)
{
diff --git a/plugins/experimental/rate_limit/sni_limiter.cc
b/plugins/experimental/rate_limit/sni_limiter.cc
index 67241ad178..36bca01004 100644
--- a/plugins/experimental/rate_limit/sni_limiter.cc
+++ b/plugins/experimental/rate_limit/sni_limiter.cc
@@ -171,7 +171,12 @@ sni_limit_cont(TSCont contp, TSEvent event, void *edata)
if (limiter) {
TSUserArgSet(vc, gVCIdx, nullptr);
- limiter->free();
+ // A connection that is still queued never reserved a slot, so only
release one if it
+ // is not in the queue (either it reserved at CLIENT_HELLO or the sweep
resumed it into
+ // a reserved slot). Dropping it from the queue also avoids a stale
entry.
+ if (!limiter->remove(vc)) {
+ limiter->free();
+ }
limiter->selector()->release(); // Release the selector, such that it
can be deleted later
}
TSVConnReenable(vc);
diff --git a/plugins/experimental/rate_limit/sni_selector.cc
b/plugins/experimental/rate_limit/sni_selector.cc
index c1c2eec7ea..5d992687aa 100644
--- a/plugins/experimental/rate_limit/sni_selector.cc
+++ b/plugins/experimental/rate_limit/sni_selector.cc
@@ -219,9 +219,17 @@ sni_queue_cont(TSCont cont, TSEvent /* event ATS_UNUSED
*/, void * /* edata ATS_
QueueTime now = std::chrono::system_clock::now(); // Only do this once
per limiter
if (owner) { // Don't operate on the aliases
- // Try to enable some queued VCs (if any) if there are slots available
+ // Try to enable some queued VCs (if any) if there are slots available.
Reserving before
+ // dequeuing means a resumed VC owns the slot it was granted, so its
VCONN_CLOSE releases
+ // exactly that slot.
while (limiter->size() > 0 && limiter->reserve() ==
ReserveStatus::RESERVED) {
- auto [vc, contp, start_time] = limiter->pop();
+ auto [vc, contp, start_time] = limiter->pop();
+
+ if (nullptr == vc) { // A concurrent close emptied the queue; give the
slot back
+ limiter->free();
+ break;
+ }
+
std::chrono::milliseconds delay =
std::chrono::duration_cast<std::chrono::milliseconds>(now - start_time);
(void)contp; // Ugly, but silences some compilers.
@@ -236,11 +244,18 @@ sni_queue_cont(TSCont cont, TSEvent /* event ATS_UNUSED
*/, void * /* edata ATS_
while (limiter->size() > 0 && limiter->hasOldEntity(now)) {
// The oldest object on the queue is too old on the queue, so "kill"
it.
- auto [vc, contp, start_time] = limiter->pop();
+ auto [vc, contp, start_time] = limiter->pop();
+
+ if (nullptr == vc) { // A concurrent close emptied the queue
+ break;
+ }
+
std::chrono::milliseconds age =
std::chrono::duration_cast<std::chrono::milliseconds>(now - start_time);
(void)contp;
Dbg(dbg_ctl, "Queued VC is too old (%ldms), erroring out",
static_cast<long>(age.count()));
+ // This VC never reserved a slot; detach it (clear the arg and
release the selector
+ // lease) so its VCONN_CLOSE does not release a slot it never held.
TSUserArgSet(vc, gVCIdx, nullptr);
limiter->selector()->release();
TSVConnReenableEx(vc, TS_EVENT_ERROR);
diff --git a/src/iocore/net/TLSEventSupport.cc
b/src/iocore/net/TLSEventSupport.cc
index c1009d8c6c..a44d5c71b6 100644
--- a/src/iocore/net/TLSEventSupport.cc
+++ b/src/iocore/net/TLSEventSupport.cc
@@ -153,10 +153,20 @@ TLSEventSupport::callHooks(TSEvent eventId)
Dbg(dbg_ctl_ssl, "sslHandshakeHookState=%s eventID=%d",
get_ssl_handshake_hook_state_name(this->sslHandshakeHookState), eventId);
// Move state if it is appropriate
- if (eventId == TS_EVENT_VCONN_CLOSE) {
+ if (eventId == TS_EVENT_VCONN_CLOSE || eventId ==
TS_EVENT_VCONN_OUTBOUND_CLOSE) {
// Regardless of state, if the connection is closing, then transition to
// the DONE state. This will trigger us to call the appropriate cleanup
// routines.
+ //
+ // A connection can close while it is parked in a handshake hook, waiting
for a plugin to
+ // reenable it. curHook then still points into that handshake hook's list.
Each hook id owns
+ // a separate list, so advancing curHook below would walk the handshake
list rather than the
+ // close list: the close event is dropped when the handshake list is
exhausted, or delivered
+ // to the wrong plugin when it is not. Restart from the head of the close
list unless we are
+ // already iterating it.
+ if (this->sslHandshakeHookState !=
SSLHandshakeHookState::HANDSHAKE_HOOKS_DONE) {
+ this->curHook = nullptr;
+ }
this->sslHandshakeHookState = SSLHandshakeHookState::HANDSHAKE_HOOKS_DONE;
} else {
switch (this->sslHandshakeHookState) {
diff --git
a/tests/gold_tests/pluginTest/rate_limit/rate_limit_sni_expiry.test.py
b/tests/gold_tests/pluginTest/rate_limit/rate_limit_sni_expiry.test.py
new file mode 100644
index 0000000000..8ae995bf1c
--- /dev/null
+++ b/tests/gold_tests/pluginTest/rate_limit/rate_limit_sni_expiry.test.py
@@ -0,0 +1,87 @@
+'''
+Regression test for the max_age expiry branch of the rate_limit SNI queue
accounting. A
+queued connection never reserves a slot, so when the sweep expires it the
plugin must
+detach it rather than release a slot it never held; otherwise the expiry
underflows the
+active-slot counter and the next reserve() trips a release assertion, aborting
the server.
+ATS must survive the expiry.
+'''
+# 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.
+
+import os
+
+Test.Summary = __doc__
+
+Test.SkipUnless(Condition.PluginExists('rate_limit.so'))
+
+
+class RateLimitSniExpiryTest:
+ """Age a queued connection out via max_age and assert the active-slot
counter stays balanced."""
+
+ def __init__(self) -> None:
+ tr = Test.AddTestRun('rate_limit SNI queue max_age expiry')
+ self._configure_trafficserver()
+ self._configure_client(tr)
+
+ def _configure_trafficserver(self) -> None:
+ ts = Test.MakeATSProcess('ts', enable_tls=True, enable_cache=False)
+ self._ts = ts
+ ts.addDefaultSSLFiles()
+ for line in ['ssl_multicert:', ' - dest_ip: "*"', ' ssl_cert_name:
server.pem', ' ssl_key_name: server.key']:
+ ts.Disk.ssl_multicert_yaml.AddLine(line)
+
+ # One concurrent handshake for this SNI, a one-deep queue, and a 1s
max age so the
+ # sweep expires the queued connection. Named .config (not .yaml) so
autest treats it
+ # as a plain config file; the plugin parses it as YAML regardless.
+ ts.Disk.MakeConfigFile('rate_limit.config').AddLines(
+ [
+ 'selector:',
+ ' - sni: rate.limited.com',
+ ' limit: 1',
+ ' queue:',
+ ' size: 1',
+ ' max_age: 1',
+ ])
+ ts.Disk.plugin_config.AddLine(f'rate_limit.so
{ts.Variables.CONFIGDIR}/rate_limit.config')
+
+ # Disable the freelist / ProxyAllocator so allocation behavior is not
a confound; the
+ # abort under test is a release-assertion, and this keeps the run
representative of CI.
+ ts.Command += ' -f -F'
+
+ ts.Disk.records_config.update(
+ {
+ 'proxy.config.ssl.server.cert.path': ts.Variables.SSLDir,
+ 'proxy.config.ssl.server.private_key.path':
ts.Variables.SSLDir,
+ 'proxy.config.diags.debug.enabled': 1,
+ 'proxy.config.diags.debug.tags': 'rate_limit',
+ })
+
+ # The expiry branch is actually reached...
+ ts.Disk.traffic_out.Content = Testers.ContainsExpression('too old', 'a
queued connection was expired')
+ # ...and expiring it does not underflow the active-slot counter into
the release assertion.
+ ts.Disk.traffic_out.Content += Testers.ExcludesExpression(
+ '_active <= _limit|received signal', 'expiring a queued connection
must not underflow and abort ATS')
+
+ def _configure_client(self, tr: 'TestRun') -> None:
+ ts = self._ts
+ client = os.path.join(Test.TestDirectory,
'rate_limit_sni_expiry_client.sh')
+ tr.Processes.Default.Command = f'bash {client} 127.0.0.1
{ts.Variables.ssl_port} rate.limited.com'
+ tr.Processes.Default.ReturnCode = 0
+ tr.Processes.Default.StartBefore(ts)
+ tr.Processes.Default.Streams.stdout =
Testers.ContainsExpression('rate_limit-expiry-done', 'the client ran to
completion')
+
+
+RateLimitSniExpiryTest()
diff --git
a/tests/gold_tests/pluginTest/rate_limit/rate_limit_sni_expiry_client.sh
b/tests/gold_tests/pluginTest/rate_limit/rate_limit_sni_expiry_client.sh
new file mode 100644
index 0000000000..10742f2c8c
--- /dev/null
+++ b/tests/gold_tests/pluginTest/rate_limit/rate_limit_sni_expiry_client.sh
@@ -0,0 +1,77 @@
+#!/usr/bin/env bash
+#
+# 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.
+#
+# Exercise the max_age EXPIRY branch of the rate_limit SNI queue accounting. A
queued
+# connection never reserves a slot; when the sweep expires it (max_age), it
must be
+# detached so its close does not release a slot it never held. Otherwise the
expiry is an
+# unmatched decrement of the active-slot counter, and -- combined with the
holder's own
+# close -- the counter wraps below zero and the limiter's release assertion
aborts ATS.
+#
+# 1. holder completes its handshake and holds the single slot (counter = 1);
+# 2. one connection enqueues (slot full) and stays parked -- it is NOT
disconnected, so
+# only the sweep's max_age expiry removes it;
+# 3. after max_age the sweep errors it out -> (unfixed) unmatched decrement
-> counter 1->0;
+# 4. the holder is closed; its matched decrement lands on the understated
counter -> wrap;
+# 5. a probe connection's reserve() observes the wrapped counter and the
assertion aborts.
+#
+# args: host port sni
+set -u
+host="$1"
+port="$2"
+sni="$3"
+
+OSSL="openssl s_client -connect ${host}:${port} -servername ${sni} -quiet
-no_ign_eof"
+
+# Run a command in the background and terminate it after a deadline. coreutils
"timeout" is not
+# available everywhere (notably macOS), so do it with sleep and kill.
+run_for() {
+ deadline="$1"
+ shift
+ "$@" &
+ target=$!
+ (
+ sleep "${deadline}"
+ kill -TERM "${target}" 2>/dev/null
+ ) &
+}
+
+# 1. Holder: hold the single slot. Its stdin is a FIFO on fd 3 so we end it in
step 4.
+fifo_dir="$(mktemp -d "${TMPDIR:-/tmp}/rl_holder.XXXXXX")"
+fifo="${fifo_dir}/fifo"
+mkfifo "$fifo"
+${OSSL} <"$fifo" >/dev/null 2>&1 &
+exec 3<>"$fifo"
+rm -rf "$fifo_dir"
+sleep 3 # let the holder reserve the one slot
+
+# 2. One queued connection: enqueues and stays parked at the ClientHello hook
(not killed),
+# so the sweep's max_age expiry -- not a disconnect or a resume -- is what
removes it.
+${OSSL} </dev/null >/dev/null 2>&1 &
+queued=$!
+sleep 3 # > max_age (1s) + sweeps: the expiry path errors the queued
connection out
+
+# 4. End the holder: its matched decrement lands on the (unfixed) understated
counter.
+exec 3>&-
+sleep 2
+
+# 5. Probe: its reserve() reads the counter; if it wrapped, the release
assertion aborts.
+run_for 2 sh -c "${OSSL} </dev/null >/dev/null 2>&1"
+kill "${queued}" 2>/dev/null || true
+sleep 1
+
+echo "rate_limit-expiry-done"
diff --git
a/tests/gold_tests/pluginTest/rate_limit/rate_limit_sni_queue.test.py
b/tests/gold_tests/pluginTest/rate_limit/rate_limit_sni_queue.test.py
new file mode 100644
index 0000000000..5904275262
--- /dev/null
+++ b/tests/gold_tests/pluginTest/rate_limit/rate_limit_sni_queue.test.py
@@ -0,0 +1,89 @@
+'''
+Regression test for a queue-accounting balance bug in the rate_limit SNI
limiter: a
+queued connection never reserves a slot, but its VCONN_CLOSE unconditionally
releases
+one, so a queued connection that closes underflows the active-slot counter and
the next
+reserve() trips a release assertion, aborting the server. ATS must survive the
queue
+churn.
+'''
+# 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.
+
+import os
+
+Test.Summary = __doc__
+
+Test.SkipUnless(Condition.PluginExists('rate_limit.so'))
+
+
+class RateLimitSniQueueTest:
+ """Churn the rate_limit SNI queue and assert the active-slot counter never
underflows."""
+
+ def __init__(self) -> None:
+ tr = Test.AddTestRun('rate_limit SNI queue accounting')
+ self._configure_trafficserver()
+ self._configure_client(tr)
+
+ def _configure_trafficserver(self) -> None:
+ ts = Test.MakeATSProcess('ts', enable_tls=True, enable_cache=False)
+ self._ts = ts
+ ts.addDefaultSSLFiles()
+ for line in ['ssl_multicert:', ' - dest_ip: "*"', ' ssl_cert_name:
server.pem', ' ssl_key_name: server.key']:
+ ts.Disk.ssl_multicert_yaml.AddLine(line)
+
+ # One concurrent handshake for this SNI and a queue that admits
exactly one more.
+ # No rate and no max_age -- the sweep's resume path alone drives the
scenario, with
+ # no rate-bucket or expiry timing to confound it. Named .config (not
.yaml) so autest
+ # treats it as a plain config file; the plugin parses it as YAML
regardless.
+ ts.Disk.MakeConfigFile('rate_limit.config').AddLines(
+ [
+ 'selector:',
+ ' - sni: rate.limited.com',
+ ' limit: 1',
+ ' queue:',
+ ' size: 1',
+ ])
+ ts.Disk.plugin_config.AddLine(f'rate_limit.so
{ts.Variables.CONFIGDIR}/rate_limit.config')
+
+ # Disable the freelist / ProxyAllocator so freed objects are really
released rather
+ # than recycled, keeping allocation reuse from masking a stale access.
+ ts.Command += ' -f -F'
+
+ ts.Disk.records_config.update(
+ {
+ 'proxy.config.ssl.server.cert.path': ts.Variables.SSLDir,
+ 'proxy.config.ssl.server.private_key.path':
ts.Variables.SSLDir,
+ 'proxy.config.diags.debug.enabled': 1,
+ 'proxy.config.diags.debug.tags': 'rate_limit',
+ })
+
+ # The queue path is reached...
+ ts.Disk.traffic_out.Content = Testers.ContainsExpression('Queueing the
VC', 'a connection was queued')
+ # ...and the active-slot counter never underflows into the release
assertion. Match
+ # both the specific assertion (pins the failure to this bug) and the
generic abort.
+ ts.Disk.traffic_out.Content += Testers.ExcludesExpression(
+ '_active <= _limit|received signal', 'the active-slot counter must
not underflow and abort ATS')
+
+ def _configure_client(self, tr: 'TestRun') -> None:
+ ts = self._ts
+ client = os.path.join(Test.TestDirectory,
'rate_limit_sni_queue_client.sh')
+ tr.Processes.Default.Command = f'bash {client} 127.0.0.1
{ts.Variables.ssl_port} rate.limited.com'
+ tr.Processes.Default.ReturnCode = 0
+ tr.Processes.Default.StartBefore(ts)
+ tr.Processes.Default.Streams.stdout = Testers.ContainsExpression(
+ 'rate_limit-queue-crash-done', 'the client ran to completion')
+
+
+RateLimitSniQueueTest()
diff --git
a/tests/gold_tests/pluginTest/rate_limit/rate_limit_sni_queue_client.sh
b/tests/gold_tests/pluginTest/rate_limit/rate_limit_sni_queue_client.sh
new file mode 100644
index 0000000000..0ce7e56f3b
--- /dev/null
+++ b/tests/gold_tests/pluginTest/rate_limit/rate_limit_sni_queue_client.sh
@@ -0,0 +1,78 @@
+#!/usr/bin/env bash
+#
+# 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.
+#
+# Drive the rate_limit SNI limiter's queue-then-resume path with exactly one
queued connection,
+# and check that the active-slot counter stays balanced and the server
survives.
+#
+# 1. holder completes its handshake and holds the single slot (counter = 1);
+# 2. one connection enqueues because the slot is full, then closes while
parked;
+# 3. the sweep reserves a slot and resumes a queued connection;
+# 4. the holder is closed and releases its slot;
+# 5. a probe connection reserves the freed slot.
+#
+# Against the plugin before 508c1bea26 this aborts the server: the sweep
resumed a queued
+# connection without a reservation, whose close then decremented the counter
unmatched until it
+# wrapped and reserve() tripped TSReleaseAssert(_active <= _limit). The test
asserts the counter
+# never wraps and no signal is logged, so it pins that fix as well as this
change.
+#
+# args: host port sni
+set -u
+host="$1"
+port="$2"
+sni="$3"
+
+OSSL="openssl s_client -connect ${host}:${port} -servername ${sni} -quiet
-no_ign_eof"
+
+# Run a command in the background and terminate it after a deadline. coreutils
"timeout" is not
+# available everywhere (notably macOS), so do it with sleep and kill.
+run_for() {
+ deadline="$1"
+ shift
+ "$@" &
+ target=$!
+ (
+ sleep "${deadline}"
+ kill -TERM "${target}" 2>/dev/null
+ ) &
+}
+
+# 1. Holder: hold the single slot. Its stdin is a FIFO kept open on fd 3, so
we end the
+# holder deterministically in step 4 (closing fd 3 -> EOF -> clean TLS
close -> FIN).
+fifo_dir="$(mktemp -d "${TMPDIR:-/tmp}/rl_holder.XXXXXX")"
+fifo="${fifo_dir}/fifo"
+mkfifo "$fifo"
+${OSSL} <"$fifo" >/dev/null 2>&1 &
+exec 3<>"$fifo"
+rm -rf "$fifo_dir"
+sleep 3 # let the holder reserve the one slot
+
+# 2. One queued connection: enqueues because the slot is full, then closes
while still parked
+# at the ClientHello hook.
+run_for 0.3 sh -c "${OSSL} </dev/null >/dev/null 2>&1"
+sleep 2 # >= 2 sweep periods (300ms each), so the sweep runs while the
connection is queued
+
+# 4. End the holder, releasing its slot.
+exec 3>&- # close the FIFO write end -> holder sees EOF -> clean TLS close
(FIN)
+sleep 2
+
+# 5. Probe: reserve() must succeed against a balanced counter rather than
tripping the
+# TSReleaseAssert(_active <= _limit) that a wrapped counter causes.
+run_for 2 sh -c "${OSSL} </dev/null >/dev/null 2>&1"
+sleep 3
+
+echo "rate_limit-queue-crash-done"
diff --git
a/tests/gold_tests/pluginTest/rate_limit/rate_limit_sni_reject.test.py
b/tests/gold_tests/pluginTest/rate_limit/rate_limit_sni_reject.test.py
new file mode 100644
index 0000000000..976d320deb
--- /dev/null
+++ b/tests/gold_tests/pluginTest/rate_limit/rate_limit_sni_reject.test.py
@@ -0,0 +1,83 @@
+'''
+Exercise the rate_limit SNI limiter's reject path against a TLS listener, so
the
+consumer-driven SSLNetVConnection teardown frees every rejected handshake VC
+cleanly (no use-after-free or crash).
+'''
+# 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.
+
+import os
+
+Test.Summary = __doc__
+
+Test.SkipUnless(Condition.PluginExists('rate_limit.so'))
+
+
+class RateLimitSniRejectTest:
+ """Drive rate_limit's SNI reject path and assert ATS frees the VCs without
a fault."""
+
+ def __init__(self) -> None:
+ tr = Test.AddTestRun('rate_limit SNI reject teardown')
+ self._configure_trafficserver()
+ self._configure_client(tr)
+
+ def _configure_trafficserver(self) -> None:
+ ts = Test.MakeATSProcess('ts', enable_tls=True, enable_cache=False)
+ self._ts = ts
+ ts.addDefaultSSLFiles()
+ for line in ['ssl_multicert:', ' - dest_ip: "*"', ' ssl_cert_name:
server.pem', ' ssl_key_name: server.key']:
+ ts.Disk.ssl_multicert_yaml.AddLine(line)
+
+ # One concurrent handshake for this SNI and no queue, so every further
concurrent
+ # handshake is rejected outright (TS_EVENT_ERROR) rather than queued.
Named .config
+ # (not .yaml) so autest treats it as a plain config file; the plugin
parses it as
+ # YAML regardless (YAML::LoadFile).
+ ts.Disk.MakeConfigFile('rate_limit.config').AddLines([
+ 'selector:',
+ ' - sni: rate.limited.com',
+ ' limit: 1',
+ ])
+ ts.Disk.plugin_config.AddLine(f'rate_limit.so
{ts.Variables.CONFIGDIR}/rate_limit.config')
+
+ # Disable the freelist / ProxyAllocator so a freed SSLNetVConnection
is really
+ # free()'d rather than recycled; a stale-VC access then hits freed
memory
+ # instead of a still-valid recycled object.
+ ts.Command += ' -f -F'
+
+ ts.Disk.records_config.update(
+ {
+ 'proxy.config.ssl.server.cert.path': ts.Variables.SSLDir,
+ 'proxy.config.ssl.server.private_key.path':
ts.Variables.SSLDir,
+ 'proxy.config.diags.debug.enabled': 1,
+ 'proxy.config.diags.debug.tags': 'rate_limit',
+ })
+
+ # The reject disposition is reached...
+ ts.Disk.traffic_out.Content = Testers.ContainsExpression('Rejecting
connection', 'over-limit handshakes were rejected')
+ # ...and ATS tears every rejected handshake VC down without a
memory-safety fault.
+ ts.Disk.traffic_out.Content += Testers.ExcludesExpression(
+ 'use-after-free|attempting free|SEGV|received signal', 'ATS must
survive the reject churn')
+
+ def _configure_client(self, tr: 'TestRun') -> None:
+ ts = self._ts
+ client = os.path.join(Test.TestDirectory,
'rate_limit_sni_reject_client.sh')
+ tr.Processes.Default.Command = f'bash {client} 127.0.0.1
{ts.Variables.ssl_port} rate.limited.com'
+ tr.Processes.Default.ReturnCode = 0
+ tr.Processes.Default.StartBefore(ts)
+ tr.Processes.Default.Streams.stdout =
Testers.ContainsExpression('rate_limit-reject-done', 'the client ran to
completion')
+
+
+RateLimitSniRejectTest()
diff --git
a/tests/gold_tests/pluginTest/rate_limit/rate_limit_sni_reject_client.sh
b/tests/gold_tests/pluginTest/rate_limit/rate_limit_sni_reject_client.sh
new file mode 100644
index 0000000000..8d44b314bc
--- /dev/null
+++ b/tests/gold_tests/pluginTest/rate_limit/rate_limit_sni_reject_client.sh
@@ -0,0 +1,60 @@
+#!/usr/bin/env bash
+#
+# 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.
+#
+# Drive the rate_limit SNI limiter (limit 1, no queue) through its reject path
so the
+# consumer-driven SSLNetVConnection teardown is exercised for a rejected
handshake:
+# holder completes the handshake and HOLDS the one slot open;
+# a burst of near-simultaneous handshakes then arrives while the slot is
taken and,
+# with no queue configured, each is REJECTED with TS_EVENT_ERROR
mid-handshake.
+# ATS must free every one of these rejected handshake VCs cleanly.
+#
+# args: host port sni
+set -u
+host="$1"
+port="$2"
+sni="$3"
+
+OSSL="openssl s_client -connect ${host}:${port} -servername ${sni} -quiet
-no_ign_eof"
+
+# Run a command in the background and terminate it after a deadline. coreutils
"timeout" is not
+# available everywhere (notably macOS), so do it with sleep and kill.
+run_for() {
+ deadline="$1"
+ shift
+ "$@" &
+ target=$!
+ (
+ sleep "${deadline}"
+ kill -TERM "${target}" 2>/dev/null
+ ) &
+}
+
+# holder: complete the handshake and hold the single slot for ~5s (slow stdin
keeps it open).
+(sleep 5) | ${OSSL} >/dev/null 2>&1 &
+sleep 2 # let the holder reserve the slot
+
+# Burst of near-simultaneous handshakes against the full limiter; with no
queue every one
+# is rejected with TS_EVENT_ERROR, so its handshake VC is torn down
consumer-driven.
+for _ in $(seq 5); do
+ run_for 2 sh -c "${OSSL} </dev/null >/dev/null 2>&1"
+done
+
+# Let the burst finish and the holder release its slot cleanly.
+sleep 4
+
+echo "rate_limit-reject-done"
diff --git a/tests/gold_tests/tls_hooks/tls_hooks_close_while_parked.test.py
b/tests/gold_tests/tls_hooks/tls_hooks_close_while_parked.test.py
new file mode 100644
index 0000000000..89fe0a50a3
--- /dev/null
+++ b/tests/gold_tests/tls_hooks/tls_hooks_close_while_parked.test.py
@@ -0,0 +1,88 @@
+'''
+Verify that a plugin's VCONN_CLOSE hook runs when the connection closes while
it is
+parked in a TLS handshake hook.
+'''
+# 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.
+
+import os
+
+Test.Summary = '''
+A connection that closes while parked in a TLS handshake hook must still
deliver
+TS_VCONN_CLOSE_HOOK to the plugin.
+'''
+
+Test.SkipUnless(Condition.HasOpenSSLVersion("1.1.1"),)
+
+ts = Test.MakeATSProcess("ts", enable_tls=True)
+server = Test.MakeOriginServer("server")
+server.addResponse(
+ "sessionlog.json", {
+ "headers": "GET / HTTP/1.1\r\nHost: www.example.com\r\n\r\n",
+ "timestamp": "1469733493.993",
+ "body": ""
+ }, {
+ "headers": "HTTP/1.1 200 OK\r\nConnection: close\r\n\r\n",
+ "timestamp": "1469733493.993",
+ "body": ""
+ })
+
+ts.addDefaultSSLFiles()
+
+ts.Disk.records_config.update(
+ {
+ 'proxy.config.diags.debug.enabled': 1,
+ 'proxy.config.diags.show_location': 0,
+ 'proxy.config.diags.debug.tags': 'ssl_hook_test',
+ # Fire the handshake timeout while the plugin still has the handshake
parked (2s park).
+ 'proxy.config.ssl.handshake_timeout_in': 1,
+ 'proxy.config.ssl.server.cert.path': '{0}'.format(ts.Variables.SSLDir),
+ 'proxy.config.ssl.server.private_key.path':
'{0}'.format(ts.Variables.SSLDir),
+ })
+
+ts.Disk.ssl_multicert_yaml.AddLines(
+ """
+ssl_multicert:
+ - dest_ip: "*"
+ ssl_cert_name: server.pem
+ ssl_key_name: server.key
+""".split("\n"))
+
+ts.Disk.remap_config.AddLine(
+ 'map https://example.com:{1}
http://127.0.0.1:{0}'.format(server.Variables.Port, ts.Variables.ssl_port))
+
+# The delayed client hello callback parks the handshake for 2 seconds before
it reenables.
+Test.PrepareTestPlugin(os.path.join(Test.Variables.AtsTestPluginsDir,
'ssl_hook_test.so'), ts, '-client_hello=1 -close=1')
+
+# Give up after 1 second, which is inside the 2 second park, so the connection
closes while it
+# is still suspended in the client hello hook. curl reports operation timed
out (exit 28).
+tr = Test.AddTestRun("Client disconnects while parked in the client hello
hook")
+tr.Processes.Default.StartBefore(server)
+tr.Processes.Default.StartBefore(Test.Processes.ts)
+tr.StillRunningAfter = ts
+tr.StillRunningAfter = server
+tr.MakeCurlCommand('-k --max-time 1 -H \'host:example.com:{0}\'
https://127.0.0.1:{0}'.format(ts.Variables.ssl_port), ts=ts)
+tr.Processes.Default.ReturnCode = 28
+tr.Processes.Default.TimeOut = 15
+tr.TimeOut = 15
+
+# The handshake really was parked.
+ts.Disk.traffic_out.Content = Testers.ContainsExpression("Client Hello
callback 0", "the handshake parked in the client hello hook")
+
+# The close hook must still fire, with the correct event. Before the fix,
callHooks() advanced
+# curHook within the client hello hook list instead of the close hook list, so
this never ran.
+ts.Disk.traffic_out.Content += Testers.ContainsExpression(
+ "Close callback 0 .* - event is good", "the close hook ran for the parked
connection")
