Skip to content

fix(920100): drop HTTP/0.9 GET support from request line validation#4621

Merged
fzipi merged 2 commits into
mainfrom
fix/920100-drop-http09
Apr 22, 2026
Merged

fix(920100): drop HTTP/0.9 GET support from request line validation#4621
fzipi merged 2 commits into
mainfrom
fix/920100-drop-http09

Conversation

@fzipi

@fzipi fzipi commented Apr 20, 2026

Copy link
Copy Markdown
Member

what

  • remove the GET-without-protocol alternative from regex-assembly/920100.ra
  • regenerate the compiled regex in rules/REQUEST-920-PROTOCOL-ENFORCEMENT.conf (shorter, simpler)
  • remove the misleading "HTTP/0.9 for legacy clients" example from crs-setup.conf.example and add a note that HTTP/0.9 is no longer supported

why

  • HTTP/0.9 is obsolete per RFC 9110
  • the GET HTTP/0.9 branch was the only alternative that did not require a protocol suffix in the request line, leaving an inconsistency: rule 920430 already excludes HTTP/0.9 from tx.allowed_http_versions by default, but rule 920100 accepted HTTP/0.9 request-line syntax
  • the crs-setup.conf.example legacy 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)
  • GET requests with proper HTTP/1.x, HTTP/2, or HTTP/3 protocols remain covered by the generic method alternative

refs

  • RFC 9110 (HTTP Semantics)

ai disclosure

  • tools used: Claude (Opus 4.7)
  • assisted with: analysis of the existing regex alternatives, identification of the HTTP/0.9 branch as removable given RFC 9110, drafting the .ra edit, regenerating the compiled regex via crs-toolchain regex update 920100, drafting the crs-setup.conf.example comment update, drafting this PR description
  • review performed: verified the regex change manually (ran crs-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 sending GET /\r\n\r\n via nc and confirming rule 920100 now fires (log entry present); cross-checked rule 920430's default tx.allowed_http_versions to confirm consistency; read RFC 9110 to verify HTTP/0.9 status

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.
@github-actions

github-actions Bot commented Apr 20, 2026

Copy link
Copy Markdown
Contributor

📊 Quantitative test results for language: eng, year: 2023, size: 10K, paranoia level: 1:
🚀 Quantitative testing did not detect new false positives

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 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 from regex-assembly/920100.ra.
  • Regenerates/simplifies the compiled 920100 request-line regex in REQUEST-920-PROTOCOL-ENFORCEMENT.conf.
  • Updates crs-setup.conf.example to 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.

Comment thread rules/REQUEST-920-PROTOCOL-ENFORCEMENT.conf
Comment thread rules/REQUEST-920-PROTOCOL-ENFORCEMENT.conf
@fzipi
fzipi requested a review from a team April 20, 2026 21:53
Comment thread rules/REQUEST-920-PROTOCOL-ENFORCEMENT.conf Outdated
Comment thread rules/REQUEST-920-PROTOCOL-ENFORCEMENT.conf Outdated
airween
airween previously approved these changes Apr 21, 2026
@fzipi

fzipi commented Apr 21, 2026

Copy link
Copy Markdown
Member Author

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.

@airween airween added the backport:lts-4.25.1 PR that must be backported to LTS release 4.25.1 label Apr 21, 2026
@HackingRepo

Copy link
Copy Markdown
Contributor

Of course HTTP/0.9 it must be dropped, in the security perspective

@fzipi
fzipi added this pull request to the merge queue Apr 22, 2026
Merged via the queue into main with commit f8402a9 Apr 22, 2026
8 checks passed
@fzipi
fzipi deleted the fix/920100-drop-http09 branch April 22, 2026 12:07
@fzipi fzipi mentioned this pull request Jul 1, 2026
fzipi added a commit that referenced this pull request Jul 2, 2026
* 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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport:lts-4.25.1 PR that must be backported to LTS release 4.25.1 release:fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants