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

[EsadCetiner]

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.

[marcstern]

Macro expansion should probably be allowed everywhere and consistent between all SecLang implementations.
This should be coordinated between all projects.

[airween]

This is the list of operators (in mod_security2) which support macros:

@rsub
@validateHash
@rx
@within
@contains
@containsWord
@streq
@beginsWith
@endsWith
@eq
@gt
@lt
@ge
@le

Adding the macro expansion to each operator (one by one) is not difficult, but I'm not sure all operators need that feature.

Perhaps we should decide what other operators need that, and implement that in one PR.

Add macro expansion in general seems very difficult and I'm afraid we should re-organize the code structure - I'm not sure it's worth it.

List of operators which do not support macro expansion:

@unconditionalMatch
@ipMatch
@ipmatchFromFile
@pm
@gsbLookup
@detectSQLi
@detectXSS
@strmatch
@validateDTD
@validateSchema
@verifyCC
@verifyCPF
@verifySSN
@geoLookup
@rbl
@fuzzyHash
@inspectFile
@validateByteRange
@valudateUrlEncoding
@validateUtf8Encoding

Please make a comment which operators need the macro expansion feature.

[RedXanadu]

For the operators:

• @ipMatch*
• @pm*
• @strmatch

Is it correct that these operators all pre-compile search trees and similar data structures?

Does that mean that adding macro expansion to these operators would involve a big performance penalty? It feels similar to macro expansion with @rx which forces repeated re-compilation of the regex.

[airween]

Hi @RedXanadu,

thanks for the answer!

Is it correct that these operators all pre-compile search trees and similar data structures?

Sorry, what trees and data structures you think about?

Does that mean that adding macro expansion to these operators would involve a big performance penalty? It feels similar to macro expansion with @rx which forces repeated re-compilation of the regex.

Yes, using macros causes a small performance degradation. But if the operator does not use macros, then the expansion code won't run - I mean if we add the macro expansion, then that will be an option/chance to use that, the expansion flow will evaluated only if the operator's argument begins with %. In this case the expand_macro() function finds the macro name and makes a lookup in the table.

I think your mentioned case (@rx op) is a very unique. Yes, obviously in that case the engine must re-compile the regex, because the administrator decided it will be a "dynamic" regex. But in other cases (eg. with your mentioned operators), we don't use regex therefore we don't need re-compile.

Hope this helped to clarify the behavior.

Just one question: why did you mark the first two operators with an *?

[Xhoenix]

airween, I think the * is for @ipMatchFromFile and @pmFromFile.

[airween]

I think the * is for @ipMatchFromFile and @pmFromFile.

But (I'm sure you guys know that) @ipMatchFromFile and @pmFromFile operators handle their arguments as files.

What's the use case here? You pass a macro as argument, then operator evaluates it at every transaction: expand the macro, look up the value (which must be a file...), open it and process the content? Or put macro's into files?