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 3bdfaa59b3 Fix filter_body plugin: deterministic request blocking and
response body blocking (#12876)
3bdfaa59b3 is described below
commit 3bdfaa59b3abed57c3a528dfce6beaf19f6210f7
Author: Bryan Call <[email protected]>
AuthorDate: Thu Feb 19 09:50:46 2026 -0800
Fix filter_body plugin: deterministic request blocking and response body
blocking (#12876)
* Fix filter_body request body blocking race -- transform blocks after
headers
are sent to origin, causing Content-Length mismatch and 30s hang. Set 1s
origin
no-activity timeout on block, add SEND_RESPONSE_HDR hook for
deterministic 403.
* Conditional SEND_RESPONSE_HDR hook, re-enable response blocking -- only
attach
hook when blocking a request body. Re-enable response blocking in tests.
* Add FILTER_BODY_BLOCK_CANARY to prove blocked body never reaches origin --
canary prefix in blocked body + excludes rule on verifier server output.
* Add server/client log_validation to ats_replay.test.ext -- remove
confusing
traffic_out node, use contains/excludes directly. Add client validation
support.
Fixes: #12875
---
plugins/experimental/filter_body/filter_body.cc | 75 ++++++++++++++++++++--
tests/gold_tests/autest-site/ats_replay.test.ext | 24 +++++++
.../config/filter_body_response_block.yaml | 1 -
.../pluginTest/filter_body/filter_body.replay.yaml | 37 +++++++----
4 files changed, 115 insertions(+), 22 deletions(-)
diff --git a/plugins/experimental/filter_body/filter_body.cc
b/plugins/experimental/filter_body/filter_body.cc
index 11f68a42cc..ca7471c71b 100644
--- a/plugins/experimental/filter_body/filter_body.cc
+++ b/plugins/experimental/filter_body/filter_body.cc
@@ -101,6 +101,10 @@ struct TransformData {
bool headers_added = false;
};
+// Forward declaration - send_response_handler is used in execute_actions
+// but defined later in the file.
+int send_response_handler(TSCont contp, TSEvent event, void *edata);
+
/**
* @brief Case-insensitive substring search.
*
@@ -439,10 +443,22 @@ execute_actions(TransformData *data, Rule const *rule,
std::string const *matche
if (rule->actions & ACTION_BLOCK) {
data->blocked = true;
TSHttpTxnStatusSet(data->txnp, TS_HTTP_STATUS_FORBIDDEN);
- // Set error body so client gets a proper response
char const *error_body = "Blocked by content filter";
TSHttpTxnErrorBodySet(data->txnp, TSstrdup(error_body),
strlen(error_body), TSstrdup("text/plain"));
- Dbg(dbg_ctl, "Blocking request due to rule: %s", rule->name.c_str());
+ if (data->direction == Direction::REQUEST) {
+ // Add a SEND_RESPONSE_HDR hook to override the error response to 403.
+ // Only added here (when actually blocking) to avoid hook overhead on
+ // every transaction. The transform zeros its output, creating a
+ // Content-Length mismatch at the origin. The short timeout below makes
+ // ATS close the hanging origin connection quickly instead of waiting the
+ // default 30 seconds.
+ TSCont send_resp_contp = TSContCreate(send_response_handler, nullptr);
+ TSHttpTxnHookAdd(data->txnp, TS_HTTP_SEND_RESPONSE_HDR_HOOK,
send_resp_contp);
+ TSHttpTxnHookAdd(data->txnp, TS_HTTP_TXN_CLOSE_HOOK, send_resp_contp);
+ TSHttpTxnConfigIntSet(data->txnp,
TS_CONFIG_HTTP_TRANSACTION_NO_ACTIVITY_TIMEOUT_OUT, 1);
+ }
+ Dbg(dbg_ctl, "Blocking %s body due to rule: %s", data->direction ==
Direction::REQUEST ? "request" : "response",
+ rule->name.c_str());
}
}
@@ -611,19 +627,21 @@ transform_handler(TSCont contp, TSEvent event, void
*edata ATS_UNUSED)
}
if (data->blocked) {
- // Blocking action - complete the transform with zero output
- // The 403 status we set will cause ATS to generate the error
response
+ // Zero the transform output so the blocked body is not forwarded.
+ // This creates a Content-Length mismatch at the origin (headers
+ // advertised the original body size but 0 bytes arrive). The short
+ // no-activity timeout set in execute_actions causes ATS to close
+ // the hanging origin connection quickly and generate an error
+ // response. The SEND_RESPONSE_HDR hook then overrides it to 403.
TSVIONBytesSet(data->output_vio, 0);
TSVIOReenable(data->output_vio);
- // Consume all remaining input
+ // Consume all remaining input so the client side completes cleanly
int64_t const remaining = TSIOBufferReaderAvail(reader);
if (remaining > 0) {
TSIOBufferReaderConsume(reader, remaining);
}
TSVIONDoneSet(write_vio, TSVIONBytesGet(write_vio));
-
- // Signal write complete
TSContCall(TSVIOContGet(write_vio), TS_EVENT_VCONN_WRITE_COMPLETE,
write_vio);
return 0;
}
@@ -747,6 +765,49 @@ response_handler(TSCont contp, TSEvent event, void *edata)
return 0;
}
+/**
+ * @brief Send response handler to enforce 403 for blocked requests.
+ *
+ * Called on TS_HTTP_SEND_RESPONSE_HDR_HOOK only when the request body
+ * transform has blocked a request (hook is added in execute_actions).
+ * The transform zeros its output, creating a Content-Length mismatch that
+ * causes ATS to generate an error response (typically 502). This hook
+ * overrides that error to a deterministic 403 Forbidden.
+ *
+ * @param[in] contp The continuation.
+ * @param[in] event The event type (SEND_RESPONSE_HDR or TXN_CLOSE).
+ * @param[in] edata The HTTP transaction.
+ * @return Always returns 0.
+ */
+int
+send_response_handler(TSCont contp, TSEvent event, void *edata)
+{
+ TSHttpTxn txnp = static_cast<TSHttpTxn>(edata);
+
+ if (event == TS_EVENT_HTTP_TXN_CLOSE) {
+ TSContDestroy(contp);
+ TSHttpTxnReenable(txnp, TS_EVENT_HTTP_CONTINUE);
+ return 0;
+ }
+
+ if (event == TS_EVENT_HTTP_SEND_RESPONSE_HDR) {
+ // This hook is only added when the request body was blocked, so we
+ // unconditionally override the response to 403.
+ TSMBuffer bufp;
+ TSMLoc hdr_loc;
+ if (TSHttpTxnClientRespGet(txnp, &bufp, &hdr_loc) == TS_SUCCESS) {
+ Dbg(dbg_ctl, "Overriding response to 403 for blocked request");
+ TSHttpHdrStatusSet(bufp, hdr_loc, TS_HTTP_STATUS_FORBIDDEN);
+ TSHttpHdrReasonSet(bufp, hdr_loc,
TSHttpHdrReasonLookup(TS_HTTP_STATUS_FORBIDDEN),
+
strlen(TSHttpHdrReasonLookup(TS_HTTP_STATUS_FORBIDDEN)));
+ TSHandleMLocRelease(bufp, TS_NULL_MLOC, hdr_loc);
+ }
+ }
+
+ TSHttpTxnReenable(txnp, TS_EVENT_HTTP_CONTINUE);
+ return 0;
+}
+
/**
* @brief Parse the YAML configuration file.
*
diff --git a/tests/gold_tests/autest-site/ats_replay.test.ext
b/tests/gold_tests/autest-site/ats_replay.test.ext
index bb9e19d8c8..c041c42770 100644
--- a/tests/gold_tests/autest-site/ats_replay.test.ext
+++ b/tests/gold_tests/autest-site/ats_replay.test.ext
@@ -178,6 +178,18 @@ def ATSReplayTest(obj, replay_file: str):
rc = server_config['return_code']
server.ReturnCode = Any(*rc) if isinstance(rc, list) else rc
+ # Configure server log validation if specified.
+ server_log_validation = server_config.get('log_validation', {})
+ if server_log_validation:
+ for contains_entry in server_log_validation.get('contains', []):
+ expression = contains_entry['expression']
+ description = contains_entry.get('description', f'Verify server
output contains: {expression}')
+ server.Streams.All += Testers.ContainsExpression(expression,
description)
+ for excludes_entry in server_log_validation.get('excludes', []):
+ expression = excludes_entry['expression']
+ description = excludes_entry.get('description', f'Verify server
output excludes: {expression}')
+ server.Streams.All += Testers.ExcludesExpression(expression,
description)
+
# ATS configuration.
if not 'ats' in autest_config:
raise ValueError(f"Replay file {replay_file} does not contain
'autest.ats' section")
@@ -201,6 +213,18 @@ def ATSReplayTest(obj, replay_file: str):
rc = client_config['return_code']
client.ReturnCode = Any(*rc) if isinstance(rc, list) else rc
+ # Configure client log validation if specified.
+ client_log_validation = client_config.get('log_validation', {})
+ if client_log_validation:
+ for contains_entry in client_log_validation.get('contains', []):
+ expression = contains_entry['expression']
+ description = contains_entry.get('description', f'Verify client
output contains: {expression}')
+ client.Streams.All += Testers.ContainsExpression(expression,
description)
+ for excludes_entry in client_log_validation.get('excludes', []):
+ expression = excludes_entry['expression']
+ description = excludes_entry.get('description', f'Verify client
output excludes: {expression}')
+ client.Streams.All += Testers.ExcludesExpression(expression,
description)
+
if dns:
ts.StartBefore(dns)
ts.StartBefore(server)
diff --git
a/tests/gold_tests/pluginTest/filter_body/config/filter_body_response_block.yaml
b/tests/gold_tests/pluginTest/filter_body/config/filter_body_response_block.yaml
index 2888a229f2..a853ec8efa 100644
---
a/tests/gold_tests/pluginTest/filter_body/config/filter_body_response_block.yaml
+++
b/tests/gold_tests/pluginTest/filter_body/config/filter_body_response_block.yaml
@@ -13,4 +13,3 @@ rules:
- "SSN:"
- "password:"
action: [log, block]
-
diff --git a/tests/gold_tests/pluginTest/filter_body/filter_body.replay.yaml
b/tests/gold_tests/pluginTest/filter_body/filter_body.replay.yaml
index 78f66584f4..5cb4f8bb24 100644
--- a/tests/gold_tests/pluginTest/filter_body/filter_body.replay.yaml
+++ b/tests/gold_tests/pluginTest/filter_body/filter_body.replay.yaml
@@ -26,10 +26,16 @@ autest:
server:
name: 'server'
+ process_config:
+ other_args: '--verbose diag'
+ log_validation:
+ excludes:
+ - expression: "FILTER_BODY_BLOCK_CANARY"
+ description: "Verify blocked request body never reached the origin"
client:
name: 'client'
- # There's a race condition for when the connection is droped by ATS - this
+ # There's a race condition for when the connection is dropped by ATS - this
# can result in either a 0 or 1 return code.
return_code: [0, 1]
@@ -122,8 +128,12 @@ autest:
description: "Verify response block rule matched"
traffic_out:
contains:
- - expression: "Blocking request due to rule"
+ - expression: "Blocking request body due to rule"
description: "Verify request blocking action was taken"
+ - expression: "Overriding response to 403 for blocked request"
+ description: "Verify SEND_RESPONSE_HDR hook enforced 403 status"
+ - expression: "Blocking response body due to rule"
+ description: "Verify response blocking action was taken"
- expression: "Added header X-Security-Match"
description: "Verify header was added for request"
# Adding an internal response is not supported while streaming the
body.
@@ -167,11 +177,10 @@ sessions:
#############################################################################
# Test 2: Request block - request with XXE pattern is blocked
#
- # When blocking request bodies, ATS closes the connection to the origin and
- # the client either experiences simply a closed connection or, depending upon
- # timeing, a 502 Bad Gateway response. The plugin cannot send a custom error
- # response (like 403) because the request headers have already been sent to
- # the origin by the time the body is inspected.
+ # The plugin detects the malicious pattern in the request body and blocks it.
+ # The transform aborts the origin-side VConn to prevent the body from being
+ # forwarded, and a SEND_RESPONSE_HDR hook overrides whatever error ATS
+ # generates (typically 502) to a deterministic 403.
#############################################################################
- transactions:
- client-request:
@@ -182,10 +191,10 @@ sessions:
fields:
- [Host, request-block.example.com]
- [Content-Type, "application/xml+plus_other_stuff"]
- - [Content-Length, 49]
+ - [Content-Length, 74]
- [uuid, request-block-test]
content:
- data: '<?xml version="1.0"?><!ENTITY xxe SYSTEM "file:">'
+ data: 'FILTER_BODY_BLOCK_CANARY <?xml version="1.0"?><!ENTITY xxe
SYSTEM "file:">'
server-response:
status: 200
@@ -197,7 +206,7 @@ sessions:
data: "OK"
proxy-response:
- status: 502
+ status: 403
#############################################################################
# Test 3: Request header - request passes, header added to server request
@@ -334,9 +343,10 @@ sessions:
# don't expect to see any external headers added.
#############################################################################
- # Test 7: Response block - detect pattern and attempt blocking
- # Note: Response blocking after streaming starts has limitations - the
- # response will still return 200 but the body will be blocked.
+ # Test 7: Response block - block responses with sensitive data
+ # The response body is blocked (zeroed) when a pattern is detected. Because
+ # response headers (including Content-Length) are committed before the body
+ # is inspected, the client sees a Content-Length mismatch.
#############################################################################
- transactions:
- client-request:
@@ -362,6 +372,5 @@ sessions:
content:
data: '{"name": "John", "SSN: 123-45-6789"}'
- # Note that blocking happens after the 200 response headers are sent.
proxy-response:
status: 200
