fix(920100): drop HTTP/0.9 GET support from request line validation#4621
Conversation
remove the GET-without-protocol alternative from the regex assembly. HTTP/0.9 is obsolete per RFC 9110 and the dedicated alternative was the only one that did not require a protocol suffix in the request line. GET requests with proper HTTP/1.x or HTTP/2 protocol versions remain covered by the generic method alternative.
|
📊 Quantitative test results for language: |
There was a problem hiding this comment.
Pull request overview
This PR removes HTTP/0.9-style GET request-line support from rule 920100 to align request-line validation with RFC 9110 and existing protocol-version enforcement behavior in the 920xxx ruleset.
Changes:
- Removes the HTTP/0.9
GET /...(no protocol suffix) alternative fromregex-assembly/920100.ra. - Regenerates/simplifies the compiled 920100 request-line regex in
REQUEST-920-PROTOCOL-ENFORCEMENT.conf. - Updates
crs-setup.conf.exampleto remove the HTTP/0.9 “legacy clients” example and document that HTTP/0.9 is no longer supported.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| rules/REQUEST-920-PROTOCOL-ENFORCEMENT.conf | Updates the compiled REQUEST_LINE validation regex for rule 920100 after removing HTTP/0.9 support. |
| regex-assembly/920100.ra | Drops the standalone HTTP/0.9 GET request-line pattern from the regex assembly source. |
| crs-setup.conf.example | Removes the HTTP/0.9 example for tx.allowed_http_versions and adds a note clarifying current support. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Max Leske <[email protected]>
|
Now, there is the suggestion from @airween to backport this change to LTS v4.25.x. For those who think this is a good idea, please add your 👍 here. |
|
Of course HTTP/0.9 it must be dropped, in the security perspective |
* fix: false positive with parameter name `.history` (#4614) (cherry picked from commit 0885958) * fix(942200): reduce false positives on payloads with comments (#4608) * fix(942200): reduce false positives on payloads with comments * add known fp test case * improve test comments * fix: wrong test * add known fp test case * fix typo * fix typo (cherry picked from commit 2c64b25) * fix(unix): exclude `pg` command from pl-1 (#4613) (cherry picked from commit 82195a4) * fix(920100): drop HTTP/0.9 GET support from request line validation (#4621) * fix(920100): drop HTTP/0.9 GET support from request line validation remove the GET-without-protocol alternative from the regex assembly. HTTP/0.9 is obsolete per RFC 9110 and the dedicated alternative was the only one that did not require a protocol suffix in the request line. GET requests with proper HTTP/1.x or HTTP/2 protocol versions remain covered by the generic method alternative. * Apply suggestions from code review Co-authored-by: Max Leske <[email protected]> --------- Co-authored-by: Max Leske <[email protected]> (cherry picked from commit f8402a9) * fix(920240, 920400): don't rely on content-type header (#4639) (cherry picked from commit 2f1afb7) * fix: remove exponential backtracking in 933160/933161 comment suffix (#4666) * fix: remove exponential backtracking in 933160/933161 comment suffix The suffix that skips whitespace and PHP comments between a function name and the opening parenthesis used an ambiguous alternation where the whitespace branch and the comment bodies both matched the same characters under a `*` quantifier. On a backtracking engine a payload that never reaches `(` backtracks exponentially. De-ambiguate the alternation while keeping RE2 compatibility (no atomic groups): - block comments use the unrolled form instead of `/\*.*\*/` - line comments (`#`, `//`) must terminate at a line break, because a line comment before `(` is only reachable across a newline; otherwise the `(` is inside the comment and there is no function call Add positive tests for the comment/whitespace evasions (block, line with newline, multi-line block, vertical tab, mixed) and negatives to 933161. * docs: explain unrolled comment form in 933160/933161 .ra * Update regex-assembly/933161.ra Co-authored-by: Max Leske <[email protected]> --------- Co-authored-by: Max Leske <[email protected]> (cherry picked from commit c1df902) * fix(941140): remove exponential backtracking in CSS url(javascript) detection (#4670) * fix(941140): bound CSS declaration value to remove exponential backtracking Move the inline pattern to regex-assembly/941140.ra and replace the free '.+' in the declaration list with '[^;]+', removing the ambiguity that caused exponential backtracking on backtracking engines (PCRE2/RE2 were unaffected). Detection is unchanged (differential-tested). * docs(941140): clarify whitespace handling and regression-test intent Address review feedback on #4670: - note that t:removeWhitespace strips whitespace before the regex runs, so the character classes intentionally do not account for it - reword test_id 13 desc: it is a no-match canary for the unbounded-value shape (PCRE2 tames both old and new patterns), not a latency assertion (cherry picked from commit fe59387) * fix(933180): remove exponential backtracking in variable-function noise suffix (#4669) * fix(933180): remove exponential backtracking in variable-function noise suffix * fix(933180): require non-empty array/brace bodies in noise suffix Address review on #4669: use + instead of * for the array-access and curly-brace noise branches so empty [] / {} are not matched. This restores the non-empty semantics of the original .+ patterns; the * form had accidentally broadened them. Differential vs the original rule shows zero true-positive loss; REQUEST-933 416/416 pass. * test(933180): verify empty array/brace bodies are not matched (cherry picked from commit 3828246) * feat(crs-setup): add default actions for phases 3-5 (#4675) (cherry picked from commit 09c1a11) * fix: 4RI-250413 (#4672) * fix: 4RI-250413 * fix: tests * chore(formatting): auto fixes from pre-commit hooks for more information, see https://pre-commit.ci * fix: typo * fix: tests * apply code review suggestions * apply code review suggestions * chore(formatting): auto fixes from pre-commit hooks for more information, see https://pre-commit.ci * chore(942360): update regex with better format Signed-off-by: Felipe Zipitria <[email protected]> * fix(942362): add json. prefix support for JSON ARGS_NAMES Rule 942362 uses a ^[\W\d]+ anchor that silently fails on libModSecurity3 because JSON payloads prepend json. (starting with j, a letter) to ARGS_NAMES. Apply the same fix used in 942360/942361: ^(?:json\.)?[\W\d]+. * Apply suggestions from code review Co-authored-by: Max Leske <[email protected]> * apply code review suggestions * Update regex-assembly/942360.ra Co-authored-by: Max Leske <[email protected]> --------- Signed-off-by: Felipe Zipitria <[email protected]> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Felipe Zipitria <[email protected]> Co-authored-by: Max Leske <[email protected]> (cherry picked from commit 6e9ff3c) * chore: release v4.25.1 * fix: correct indentation and action order in commented 900510 example * docs: note libmodsecurity v3.0.16 requirement in CHANGES * chore(tests): bump modsecurity-crs nginx image to libmodsecurity v3.0.16 * fix(901180): split XML attribute inspection gate to fix unused TX variable lint error * fix: renumber duplicated test Signed-off-by: Felipe Zipitria <[email protected]> * fix: restore dropped json. prefix fixes and fix stale nginx CONNECT override Cherry-picking PR #4672 (4RI-250413) took the LTS side wholesale for rules/REQUEST-932-APPLICATION-ATTACK-RCE.conf and rules/REQUEST-942-APPLICATION-ATTACK-SQLI.conf, then only regenerated the rule IDs backed by a regex-assembly (.ra) file. Two hand-written inline regexes with no .ra file (932171, 942361) were silently dropped in that process and never got the `(?:json\.)?` ARGS_NAMES prefix fix, so 942361-9 failed to detect the SQLi payload on nginx/libmodsecurity3. Also updates the nginx-overrides.yaml entry for 920100 test 4: the libmodsecurity v3.0.16 image bump also picked up nginx 1.30.3 (up from 1.28.2), which now rejects CONNECT with 405 instead of 400. * docs: note ModSecurity v2 limitation for XML attribute opt-out in CHANGES --------- Signed-off-by: Felipe Zipitria <[email protected]> Co-authored-by: Esad Cetiner <[email protected]> Co-authored-by: Max Leske <[email protected]> Co-authored-by: Prateek saini <[email protected]> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
what
regex-assembly/920100.rarules/REQUEST-920-PROTOCOL-ENFORCEMENT.conf(shorter, simpler)crs-setup.conf.exampleand add a note that HTTP/0.9 is no longer supportedwhy
tx.allowed_http_versionsby default, but rule 920100 accepted HTTP/0.9 request-line syntaxcrs-setup.conf.examplelegacy hint would have led users to a half-working configuration (HTTP/0.9 allowed at the protocol level, but blocked at the request-line level)refs
ai disclosure
.raedit, regenerating the compiled regex viacrs-toolchain regex update 920100, drafting thecrs-setup.conf.examplecomment update, drafting this PR descriptioncrs-toolchain regex compare/generate, inspected diff); confirmed all 16 existing regression tests for 920100 still pass; verified the removed HTTP/0.9 behavior manually by sendingGET /\r\n\r\nviancand confirming rule 920100 now fires (log entry present); cross-checked rule 920430's defaulttx.allowed_http_versionsto confirm consistency; read RFC 9110 to verify HTTP/0.9 status