ftw icon indicating copy to clipboard operation
ftw copied to clipboard

Add `annotation` parameter that stores meta data for programs which extend FTW

Open nullpo-head opened this issue 2 years ago • 2 comments

Hi FTW team. Thanks so much for your great product! It's helping us to improve the security of our application.

In this PR, I'd like to propose to add a new parameter, annotation in the YAML file. I would like to ask your opinion, or ask you to merge the PR if it looks good.

Short description of what annotation is

As the updated YAMLFormat.md describes, annotation stores arbitrary data so that external programs which extend FTW, such as CRS_Tets.py of coreruleset, can embed their own data in the format they define.

Why annotation is useful

This allows external programs that extend FTW to control the test flow based on their program-specific settings such as skipping tests. Example usage is as follows.

Example: skip tests based on the runtime in the regression test of the coreruleset. In this example, runtime is the concept of the CRS test, not of FTW. So, skipping a test case based on it can be handled only by the CRS side.

test_title: 920100-14
desc: Invalid HTTP Request Line (920100) - Test 3 from old modsec regressions
annotation:
  skip:
    runtime:
      is: modsec3-nginx
      reason: The invalid HTTP method is rejected by Nginx without WAF

Then, the test runner of the CRS can implement its own skip mechanism like this

def test_crs(ruleset, test, logchecker_obj, config):  # note that `config` is the concept of the CRS regression test, not of FTW.
    runner = testrunner.TestRunner()
    if test.annotation and 'skip' in test.annotation:
        if skips_test(test.annotation['skip'], config):
            pytest.skip('Skipped due to the skip condition.')
    for stage in test.stages:
        runner.run_stage(stage, logchecker_obj)

Another possible use case.

test_title: 920100-14
desc: Invalid HTTP Request Line (920100) - Test 3 from old modsec regressions
annotation:
  test-scenario: ["apache2-tests"] 

Motivation

FTW can be extended by external programs which use FTW as a library, just as the regression test of the coreruleset does. Such program often introduces a their own config system. CRS introduces a config to switch the ModSecurity implementations.

I guess it's common demand to control the test cases based on their own configuration. In my case as an example, I wanted to skip some test cases on Nginx and Envoy, on which I implemented my own WAF.

Design Discussion

Q: Why do you expect external applications to control the test flow, instead of implementing actual use cases such as skipping tests in FTW?
A: Because a user will want to fine-grained control based on the config or additional CLI parameters the external application provides. FTW side doesn't know any application-side concepts, such as whether the CLI parameter for the CRS test is modsec2-apache or modsec3-nginx. Thus, annotation can be used to delegate the responsibility to external applications that extend the FTW, just as the FTW delegates the logging method to them. As a side note, it is also possible to introduce new concepts into FTW, such as "configchecker_obj" to reference application-specific settings, but annotation is a simpler change and helps to keep FTW as a general framework.

Possible future work

This PR adds annotation parameter only to Test and RuleSet. However, if you add annotation to Output as well, a program which extends FTW can implement the conditional output test discussed in #19

output:
  status: [200, 400]
  annotation:
    conditional:
    - log_contains: id "1234"
      cond:
        runtime:
          is: apache2-modsec

Reference

Here are two commits from my fork of CRS that implement a feature to skip tests on modsec3-nginx or modsec2-apache.

  • Skip test: https://github.com/nullpo-head/coreruleset/commit/fc048f54c69fe5af97d02e7d6dda94147c89abed
  • Example yaml: https://github.com/nullpo-head/coreruleset/commit/c92a7f6e5c11bdda8d80e4aafd23a66ef8b66d20

nullpo-head avatar Jul 29 '21 08:07 nullpo-head

This is a very interesting addition, @nullpo-head. Thank you very much.

We are currently phasing out ftw and replace it with go-ftw. https://github.com/fzipi/go-ftw

You may want to check that out - the performance is outstanding when compared to ftw and the YAML is almost 100% compatible (go-ftw is a bit more YAML compliant). Consequently, you're unlikely to see any further development here.

@fzipi : Is the PR proposed above something you would be interested in for go-ftw?

dune73 avatar Aug 04 '21 08:08 dune73

Sorry for the delay.

In go-ftw I just did override test results based on configuration. If you are testing modsec2-apache, you can go will all tests, but skip the ones that fail in modsec3-nginx. See https://github.com/fzipi/go-ftw/#overriding-test-results.

I agree we need to extend the YAML Test format, but maybe we need to think on an overhaul of the tests and add a version to the test file itself. That way the engine can see if it is compatible or not, or choose to execute tests depending on settings.

We can also interact with Coraza's developers and see if we foresee an extension on the yaml format to get broader options (I've always wanted to test results based on returned headers, for example, for WAFs that you can't read logs but can return X-FTW-Test: YYYYYYY).

So if you think this is a good idea, I will probably create a new repository with the actual YAML test format, and then we can iterate over there to see what is best for everyone.

fzipi avatar Nov 11 '21 11:11 fzipi