feat: chaitin waf response logging - #13763
Conversation
membphis
left a comment
There was a problem hiding this comment.
Two P2 issues need to be addressed before merge:
-
Response reporting can fail silently, and the tests do not verify the core behavior.
ngx.timer.atdoes not consume callback return values, so connect/send/receive failures returned bysocket.do_socketare neither logged nor measured. Tests 18-21 only verify configuration acceptance and the client response; they do not prove that the mock WAF receives the response status, headers, and truncated body, or that ignored content types are skipped. Please add observable error handling and an end-to-end assertion with bounded polling for the asynchronous report. -
The required exact-head CI is failing. The
t/plugin/[a-k]*.tjob is red inkafka-logger.tTEST 18 because the expectedpartition_id: 1andpartition_id: 2log entries are missing. This appears outside the files changed by this PR, but the required check still needs to pass before merge. Please rerun it and investigate if the failure is reproducible: failed job.
lua-resty-t1k 1.2.0 can report the response back to the SafeLine WAF detection service in addition to the request. Bump the dependency and expose it through three new `config` options: - `log_resp` enables response reporting, off by default so existing configurations keep their current behavior - `resp_body_size` caps how much of the response body is buffered and reported, in KB, defaulting to 4 - `extra_ignored_content_types` skips further response content types on top of the built-in ignored list Wire up the `body_filter` and `log` phases to the corresponding library entry points. The report is sent from an ngx.timer during the log phase, so it does not delay the response, and the detection result is only visible in the SafeLine console: a response is never blocked or modified based on it. `get_conf` applied the metadata and route level config blocks with two copies of the same field-by-field assignment, which three more options would have made harder to keep in sync. Fold both into a local helper, keeping the existing precedence of route level config over metadata. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Add schema validation cases for the `resp_body_size` minimum and the `extra_ignored_content_types` type, plus round-trip cases asserting that a route configured with response logging still serves its response unchanged, and that the default configuration behaves the same way. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The previous tests only checked that the plugin accepted the new options and that the response reached the client unchanged. They never showed that anything was reported, so response logging could have been broken outright and they would still have passed. Assert instead on what lua-resty-t1k logs while it reports: that a report was sent, how much of the response body it buffered, and that an ignored content type or a disabled log_resp skips the report entirely. The truncation case is the sharpest one, since the client receives 4096 bytes while only 1024 are buffered for the report. Reporting happens in a timer after the response has been handed back, so a failure to deliver it cannot surface in the response. Cover that too, now that the library logs it. pass_keepalive() is needed because the library pools the connection and sends the response report over the same one, which pass() closes as soon as it has answered the request report. Bump lua-resty-t1k to 1.2.1-0 for the log lines these tests match.
dea2e30 to
73b9035
Compare
|
Description
lua-resty-t1k1.2.0 can report the response to the SafeLine WAF detection service in addition to the request. This PR bumps the dependency and exposes the capability through thechaitin-wafplugin.Until now the plugin only reported requests, so anything the WAF could have detected on the way out — data leaks in a response body, a successful exploit's output, an unexpected status code — was invisible to SafeLine. Enabling response reporting closes that gap without changing how requests are handled.
Three new
configoptions, available both at plugin level and in the plugin metadata:log_respfalseresp_body_size>= 0)40reports only the status line and headers.extra_ignored_content_typesContent-Typevalues to skip, on top of the built-in list.How it works:
body_filterandlogphases, delegating tot1k.do_body_filter()andt1k.do_log().body_filterbuffers up toresp_body_sizeKB of the response body; the report itself is sent from anngx.timerduring the log phase, after the response has been handed back to the client. It therefore adds no latency to the response.Content-Typeis in the ignored list (audio, video, font, image and other binary media types out of the box).One refactor was needed along the way:
get_confapplied the metadata and plugin levelconfigblocks with two verbatim copies of the same field-by-field assignment. Adding three more options would have meant keeping three copies in sync, so both are folded into a local helper. Precedence is unchanged — plugin level config still overrides metadata.Docs updated in both
enandzh, including a new "Response Logging" section that covers the async reporting model, the skip conditions, and the memory cost of buffering response bodies on routes serving large responses.Which issue(s) this PR fixes:
N/A — no existing issue; this adds a new capability to the plugin.
Checklist