test: add valgrind memcheck/helgrind soak of the running module - #387
Open
fzipi wants to merge 1 commit into
Open
test: add valgrind memcheck/helgrind soak of the running module#387fzipi wants to merge 1 commit into
fzipi wants to merge 1 commit into
Conversation
Ports tools/soak.sh from coraza-nginx (itself adapted from the same soak added to ModSecurity-apache in owasp-modsecurity#93), rewritten for this connector's directive names and architecture: drives a real nginx (optionally under valgrind memcheck or helgrind) with concurrent benign and attack-shaped traffic across ten request shapes covering every attacker-reachable path in the connector -- request/response body inspection (including the file-backed and multi-buffer paths), header forwarding across multiple ngx_list parts, and a WAF-triggered redirect replacing an already-populated response. Adds Dockerfile + Dockerfile.fuzz (mirroring ModSecurity-apache owasp-modsecurity#93's structure): a multi-stage build image (libmodsecurity + nginx + this connector, matching the existing test_new.yml CI build steps) and a thin layer adding valgrind + curl on top, so the soak can run identically in CI and locally without rebuilding the connector from source for every run. Not a replacement for test_new.yml's existing per-PR build+functional test, which doesn't use Docker and stays as is; this is scoped to what the soak specifically needs. Adds .github/workflows/soak.yml: manual (workflow_dispatch) + weekly scheduled only, not on every PR -- a valgrind soak runs 10-50x slower than native. Adds tools/valgrind.suppress: nginx-core startup/event-loop entries (module-agnostic, apply to any nginx build) plus a libp11-kit exit-time pthread_mutex_destroy entry, generated via --gen-suppressions=all against an actual run and confirmed to be runtime/third-party noise at process teardown, not this connector. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.




Summary
Requested port of ModSecurity-apache#93's soak tooling. That PR itself adapted coraza-nginx's
tools/soak.shfor Apache; this ports it back to nginx, rewritten for this connector's directive names and architecture, plus the Docker structure ModSecurity-apache#93/#92 used.tools/soak.sh— drives a real nginx (optionally under valgrind memcheck or helgrind) with concurrent benign and attack-shaped traffic for a fixed duration across ten request shapes covering every attacker-reachable path in this connector: URI-arg/request-body/request-header attacks (phase 1/2 deny), benign GET/POST/large-response-body, a large chunked request body driving the file-backed request-body path (msc_request_body_from_file,access.c), 40 large request headers driving the multi-ngx_list-part traversal loop,RESPONSE_BODYinspection (pass and phase-4 deny,body_filter.c), and a phase:3redirect:action replacing an already-populated response (header_filter.c).Dockerfile+Dockerfile.fuzz— mirrors ModSecurity-apache#93's structure: a multi-stage build image (libmodsecurity + nginx + this connector, matching the existingtest_new.ymlCI build steps) and a thin layer adding valgrind + curl on top, so the soak runs identically in CI and locally without rebuilding from source each time. This is not a replacement fortest_new.yml's existing per-PR build+functional test (which doesn't use Docker and stays as-is) — scoped only to what the soak needs..github/workflows/soak.yml— manual (workflow_dispatch) + weekly scheduled only, not on every PR, since a valgrind soak runs 10-50x slower than native.tools/valgrind.suppress— nginx-core startup/event-loop entries (module-agnostic, apply to any nginx build) plus alibp11-kitexit-timepthread_mutex_destroyentry, generated via--gen-suppressions=allagainst an actual run and confirmed to be runtime/third-party noise at process teardown, not this connector.Notes from building this
Dockerfile/Dockerfile.fuzzon Linux/arm64 for real valgrind — valgrind isn't available natively on macOS). That process is also what surfaced two real, previously-unknown bugs in the connector itself, now fixed in separate PRs this soak specifically regression-tests: fix: fail closed on swallowed WAF return values #384 (phase-4RESPONSE_BODYdeny corrupting the response —[alert] header already sent) and fix: correctness gaps in response header/intervention handling #385 (a WAF-triggered redirect'sLocationheader silently dropped). This PR is branched offmaster, so until fix: fail closed on swallowed WAF return values #384/fix: correctness gaps in response header/intervention handling #385 merge, running this soak againstmasterwill legitimately fail those two scenarios — same as ModSecurity-apache#93 itself shipping a soak that (correctly) fails on that repo's own then-unfixed leaks.redirect:'...''s single quotes vs.modsecurity_rules '...''s own quoting — worked around viamodsecurity_rules_fileinstead), two separateset -efootguns inherited verbatim from the scripts this was adapted from (bare unguardedcurl/waitcalls that would have silently swallowed the exact failures the script exists to report), and a container-permissions issue (nginx running as root drops workers to an unprivileged user that can't traversemktemp -d's 0700 directory).Test plan
docker build -t modsec-nginx-test .thendocker build -f Dockerfile.fuzz -t modsec-nginx-soak .— both build cleanly.master).shellcheck/shfmtclean ontools/soak.sh;actionlint/zizmorclean onsoak.yml;hadolintclean on both Dockerfiles (aside fromDL3008apt-pin-versions, intentionally skipped — matches this repo's owntest_new.ymlconvention of unpinned apt packages, and literal Debian package-version pins break as soon as a version is superseded in the archive).workflow_dispatchonce fix: fail closed on swallowed WAF return values #384/fix: correctness gaps in response header/intervention handling #385 have merged, to confirm a clean run in the actual GitHub Actions environment.