owasp-modsecurity / owasp-modsecurity/ModSecurity

Feature Request (v2): Add support for macro expansion for ipMatch operator

Open
#3,332 6 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

2.x
Dominant language
C++
Stars
9.8k
Forks
1.8k
Avg merge
2h 46m
Merged PRs (30d)
1

Description

Feature

libModSecurity3 supports the use of transaction collection variables as an value when using the ipMatch operator, this isn't the case for ModSecurity2.
For example. these rules work on libModSecurity3 but not on ModSecurity2:

SecAction \
    "id:1,\
    phase:1,\
    pass,\
    t:none,\
    nolog,\
    setvar:'tx.allowed-ips=1.1.1.1'"

SecRule REMOTE_ADDR "@ipMatch %{tx.allowed-ips}" \
    "id:9508032,\
    phase:1,\
    pass,\
    t:none,\
    nolog,\
    ctl:ruleRemoveById=920300,\
    ctl:ruleRemoveById=920320,\
    ctl:ruleRemoveById=920330,\
    ctl:ruleRemoveTargetById=920120;FILES_NAMES,\
    ctl:ruleRemoveTargetById=920121;FILES_NAMES,\
    ctl:ruleRemoveTargetById=922130;MULTIPART_PART_HEADERS"

Another issue I encountered was you can't use semicolons within an setvar action, this issue happens on both engines and makes referencing IPv6 addresses via a variable unusable. I'm not sure if I'm doing something wrong here but I feel like this should work.

SecAction \
    "id:1,\
    phase:1,\
    pass,\
    t:none,\
    nolog,\
    setvar:'tx.allowed-ips=1.1.1.1,::1'"

SecRule REMOTE_ADDR "@ipMatch %{tx.allowed-ips}" \
    "id:9508032,\
    phase:1,\
    pass,\
    t:none,\
    nolog,\
    ctl:ruleRemoveById=920300,\
    ctl:ruleRemoveById=920320,\
    ctl:ruleRemoveById=920330,\
    ctl:ruleRemoveTargetById=920120;FILES_NAMES,\
    ctl:ruleRemoveTargetById=920121;FILES_NAMES,\
    ctl:ruleRemoveTargetById=922130;MULTIPART_PART_HEADERS"

Use Case:

This is yet another issue with the CRS Nextcloud plugin, this feature in particular is needed to disable certain rules for the server itself. There's no way to know what the server's IP might be in advance, so this needs to be an configuration option. For now this is documented in the readme but it does complicate the installation process and requires end users to manually update the rule if changes are ever made to it.

Maybe there's a better way to do what I'm trying to do, but this feels like the easiest for the end user as all they'd need to change is a single variable in the plugin config file.

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

No source files or tests are named. Start by tracing the ipMatch operator's variable expansion and the setvar parsing path, then inspect existing tests for collection variables and IPv6 values. Done means transaction variables work as ipMatch values and semicolon-containing IPv6 addresses can be stored and referenced, with regression coverage for both examples.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
security
Issue type
Feature
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.