diff --git a/plugins/experimental/filter_body/filter_body.cc b/plugins/experimental/filter_body/filter_body.cc index 11f68a42cc1..ca7471c71bf 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(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 bb9e19d8c8c..c041c427705 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 2888a229f20..a853ec8efa2 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 78f66584f41..5cb4f8bb24d 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: '' + data: 'FILTER_BODY_BLOCK_CANARY ' 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