Skip to content

HTTP: lower proxy CRLF rejection log level - #1

Open
louzt wants to merge 1 commit into
DemiMarie:no-crlf-injectionfrom
louzt:pr/demi-proxy-crlf-log-level
Open

HTTP: lower proxy CRLF rejection log level#1
louzt wants to merge 1 commit into
DemiMarie:no-crlf-injectionfrom
louzt:pr/demi-proxy-crlf-log-level

Conversation

@louzt

@louzt louzt commented May 31, 2026

Copy link
Copy Markdown

Summary

This is a small follow-up to your no-crlf-injection branch.

It changes the runtime-expanded proxy header name/value rejection logs from NGX_LOG_ALERT to NGX_LOG_ERR in:

  • ngx_http_proxy_module.c
  • ngx_http_proxy_v2_module.c

There are no control-flow changes.

Why

The rejected input here is user-triggered bad runtime expansion in a vulnerable configuration. That is useful operator-facing error logging, but it should not trip the nginx-tests automatic no alerts check.

This matches Vadim's review distinction: user-triggered invalid runtime values should be error, while internal consistency failures can remain alert.

Validation

Built this branch with --with-debug and ran the proxy-only nginx-tests draft PR against it:

TEST_NGINX_BINARY=/path/to/objs/nginx prove -v proxy_crlf_injection.t

Result:

1..8
ok 1 - proxy_pass uri rejects CRLF
ok 2 - proxy_pass uri does not inject backend header
ok 3 - proxy_set_header rejects CRLF value
ok 4 - proxy_set_header does not inject backend header
ok 5 - proxy_set_header rejects LF value
ok 6 - proxy_set_header rejects CR value
ok 7 - no alerts
ok 8 - no sanitizer errors

Related test PR: nginx/nginx-tests#55
Related upstream discussion in nginx#590.

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.

1 participant