Skip to content

feat: full coraza-nginx rework — config inheritance, dlopen wrapper, bug fixes#5

Merged
fzipi merged 34 commits intocorazawaf:mainfrom
ppomes:chore/update-latest-libcoraza
Feb 28, 2026
Merged

feat: full coraza-nginx rework — config inheritance, dlopen wrapper, bug fixes#5
fzipi merged 34 commits intocorazawaf:mainfrom
ppomes:chore/update-latest-libcoraza

Conversation

@ppomes
Copy link
Copy Markdown
Member

@ppomes ppomes commented Feb 20, 2026

Summary

This supersedes PR #3 (Felipe's initial "make it compile" work) and adds:

  • Config inheritance: locations without their own rules now inherit from the parent config
  • Delayed header forwarding: response headers held back until body inspection completes, so the module can return proper 403 pages instead of aborting mid-stream
  • Go runtime deadlock fix (dlopen wrapper): libcoraza is a Go shared library — when nginx forks workers, Go's runtime threads are lost and workers deadlock. Fix: load libcoraza.so via dlopen() in each worker after fork so Go's runtime starts fresh. Rules are collected as strings during config parsing, then replayed in each worker. The wrapper is transparent, no changes in request-processing files. This is a fundamental issue that will affect any forking server using libcoraza (apache too).
  • Bug fixes: memory leaks in intervention processing, double-free on cleanup, transaction ID in logs
  • Tests: 13 test files updated for compatibility

Dependencies

@fzipi

fzipi and others added 24 commits January 8, 2026 16:29
Signed-off-by: Felipe Zipitria <felipe.zipitria@owasp.org>
…ouble-free

Co-authored-by: fzipi <3012076+fzipi@users.noreply.github.com>
Co-authored-by: fzipi <3012076+fzipi@users.noreply.github.com>
Co-authored-by: fzipi <3012076+fzipi@users.noreply.github.com>
Co-authored-by: fzipi <3012076+fzipi@users.noreply.github.com>
Fix review comments: return types, null checks, and double-free vulnerability
Signed-off-by: Felipe Zipitria <felipe.zipitria@owasp.org>
feat: coraza-nginx with working tests
- Add log action to phase:1 rules so audit log H section is populated
- Fix regex to match Coraza format "Access denied" instead of ModSecurity format
- Remove TODO wrappers from now-passing tests (rules merge, auditLogParts, custom error page)
- Mark response body test as intermittent
Delay forwarding response headers to the client until the body filter
has finished Coraza phase 4 processing. This allows phase 4 rules to
return proper error pages instead of failing silently after headers
were already sent. Also handle file-backed response buffers that arrive
before the copy filter converts them to memory.
fix: delay response headers until phase 4 body inspection completes
@jptosso jptosso requested a review from Copilot February 21, 2026 00:52
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR reworks the coraza-nginx module to support config inheritance and safer phase-4 (response body) enforcement, while also addressing Go shared-library post-fork deadlocks by loading libcoraza.so via dlopen() in each worker.

Changes:

  • Collect rules during config parsing and replay/build WAF instances per worker in init_process (dlopen wrapper + deferred rule storage).
  • Delay forwarding response headers until body inspection completes to allow clean denials/custom error pages.
  • Update tests and build artifacts (Docker/config) to match the new behavior and rule semantics.

Reviewed changes

Copilot reviewed 25 out of 25 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
t/coraza.t Updates phase behavior/expectations (phase-4 now returns real status codes).
t/coraza-transaction-id.t Adjusts transaction-id tests and removes auditlog portion.
t/coraza-scoring.t Aligns scoring rules with phase-2 semantics and explicit actions.
t/coraza-response-body.t Enables mime-type handling and makes response-body rule run in phase 4.
t/coraza-request-body.t Adds request body processor ctl + updates limits/status expectations.
t/coraza-request-body-h2.t Mirrors request-body changes for HTTP/2 tests (413 on reject).
t/coraza-proxy.t Updates proxy tests for changed redirect/block behavior in phase 4.
t/coraza-proxy-h2.t Mirrors proxy test updates for HTTP/2.
t/coraza-h2.t Updates HTTP/2 base rules to explicit pass/deny/log semantics.
t/coraza-config.t Adjusts config tests for explicit pass actions.
t/coraza-config-merge.t Updates merge/inheritance test with requestBodyProcessor ctl.
t/coraza-config-debuglog.t Updates debuglog assertions to new log formatting.
t/coraza-config-custom-error-page.t Aligns audit/error-page assertions with updated logging behavior.
t/coraza-config-auditlog.t Updates auditlog parts expectations and rule actions.
src/ngx_http_coraza_utils.c Fixes ngx_str → C string conversion API to correctly return allocated output.
src/ngx_http_coraza_rewrite.c Updates call sites for new ngx_str_to_char(..., char **, ...) signature.
src/ngx_http_coraza_pre_access.c Adds early intervention checks and tracks intervention_triggered.
src/ngx_http_coraza_module.c Implements deferred rule storage, per-worker WAF build, and intervention cleanup/log changes.
src/ngx_http_coraza_header_filter.c Delays response header forwarding until body filter completes inspection.
src/ngx_http_coraza_dl.c Adds dlopen/dlsym wrapper layer for libcoraza symbols (worker-safe Go runtime init).
src/ngx_http_coraza_common.h Adds deferred rule types, ctx buffering fields, and dlopen API declarations.
src/ngx_http_coraza_body_filter.c Adds file-buffer reading, buffers output until phase-4 completes, and forwards headers after inspection.
config Switches link flags to -ldl and includes the new dlopen wrapper source.
_codeql_detected_source_root Adds CodeQL source-root marker.
Dockerfile Updates libcoraza build/install steps and tweaks nginx-tests invocation.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@fzipi
Copy link
Copy Markdown
Member

fzipi commented Feb 21, 2026

@ppomes Can you address Copilot's comments?

- Return NULL instead of NGX_CONF_ERROR in create_*_conf callbacks
- Add NULL check for ngx_array_push_n in merge_conf
- Loop on ngx_read_file to handle short reads
@ppomes
Copy link
Copy Markdown
Member Author

ppomes commented Feb 21, 2026

@ppomes Can you address Copilot's comments?

Done!

@ppomes ppomes force-pushed the chore/update-latest-libcoraza branch from 698d494 to eec1d9c Compare February 25, 2026 23:17
Copy link
Copy Markdown
Member

@fzipi fzipi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Try this. We can add a pipeline for windows builds afterwards.

@ppomes
Copy link
Copy Markdown
Member Author

ppomes commented Feb 27, 2026

Hi @fzipi ,

This PR is now using uptream libcozara.

Pierre

@fzipi
Copy link
Copy Markdown
Member

fzipi commented Feb 27, 2026

Ah, the branch renaming. Maybe you want to add the release instead? Probably as an environment variable so it's easy to update...

@ppomes
Copy link
Copy Markdown
Member Author

ppomes commented Feb 27, 2026

Ah, the branch renaming. Maybe you want to add the release instead? Probably as an environment variable so it's easy to update...

Done!

@fzipi fzipi mentioned this pull request Feb 27, 2026
@ppomes
Copy link
Copy Markdown
Member Author

ppomes commented Feb 27, 2026

CI is now ok @fzipi

@ppomes
Copy link
Copy Markdown
Member Author

ppomes commented Feb 27, 2026

CI is now ok @fzipi

Not yet... 👎

Copy link
Copy Markdown
Member

@fzipi fzipi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is good enough, and we can iterate over it if needed after merge.

Congrats on your hard work @ppomes ❤️ !

@fzipi fzipi merged commit c0f3b6c into corazawaf:main Feb 28, 2026
1 check passed
@fzipi fzipi mentioned this pull request Feb 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants