Linter should check that all rules have the correct CRS tag and version

[theseion]

The linter currently doesn't check that all rules have the OWASP_CRS and the version action.

As discussed in #3571, maybe not all rules need the tag (or should have it), but detection rules definitely should.

@airween?

[airween]

This is a valid request, I can add more step to check the rule has a tag OWASP_CRS - but please help me, how can we formally describe the rule, which SecRule needs this tag? I mean rules with id:NNNN11, id:NNNN12... don't need this action (even so they mustn't have!).

Eg. SecRule's which have actions like block and deny?

[dune73]

Let's discuss this on Monday. It's already in the agenda.

[fzipi]

Did we add something to the linter then? Can this one be closed?

[airween]

Did we add something to the linter then?

No, as I remember we haven't made any decision. My question above is still relevant.

[dune73]

As far as I remember there was no hard opposition against tagging and versioning everything and instead of creating a complicated set of checks and exclusions, I suggest we tag and version everything.

We can look into plugins separately if it need be.

[theseion]

I agree with @dune73: tag all rules. I don't see the benefit of not tagging a subset of rules.

[airween]

The new functions which check the existence of tag:'OWASP_CRS' and ver actions are done - but I need some help.

I need a reference value which describe the current ver value. Not it's 4.3.0-dev in main branch.

An additional note: the script in our workflow has a method which removes the comment signs before the SecRule and SecAction lines. This is necessary, because if the current file is during the check is crs-setup.conf.example, then we can check the necessary things on that file too.

So I can grab the current version from the first rule from there, namely 900000, or from the specified rule, eg. 900990.

What do you think about, which would be the preferred way? Store the version in a specific file (eg. VERSION in the root dir) and read it from there?

Or is there a GITHUB variable? But then we have to modify that too when the version is changed.

[theseion]

You can get the version by looking at the latest tag, e.g., git describe --tags, that will give you something like v4.2.0-75-g6a3df157 if the current commit is ahead of the tag or v4.2.0, if the current commit is the tagged commit. In the first case, you know that the ver mast be v4.3.0-dev, in the second case it must be v4.2.0.

Or you can read it from the setup file, that works too.

[airween]

You can get the version by looking at the latest tag, e.g., git describe --tags, that will give you something like v4.2.0-75-g6a3df157 if the current commit is ahead of the tag or v4.2.0, if the current commit is the tagged commit.

I was thinking about git describe, but - may be something is wrong in my local repository - I got

$ git tag
2.2.7
2.2.8
2.2.9
nightly
test
...
v4.1.0
v4.2.0

and

$ git describe
2.2.7-4950-g742309b8

so I skipped that.

In the first case, you know that the ver mast be v4.3.0-dev, in the second case it must be v4.2.0.

This is the other problem: I don't like these implicit things. What if we use a patch level in a release (eg. v4.2.1), then what will the next version?

Or you can read it from the setup file, that works too.

Okay, then - if there is no idea - I'm going to introduce a new file with name VERSION in the source root.

[theseion]

git describe is not the same as git describe --tags. You need --tags because we don't use annotated tags.

Please don't add a a file to hold the version, we've worked hard to base versions on Git tags.

This is the other problem: I don't like these implicit things. What if we use a patch level in a release (eg. v4.2.1), then what will the next version?

The next version will always be <minor> + 1.

[airween]

git describe is not the same as git describe --tags. You need --tags because we don't use annotated tags.

Ah, this makes sense - thanks. This works for me as you described.

$ git describe --tags
v4.2.0-92-g742309b8

Please don't add a a file to hold the version, we've worked hard to base versions on Git tags.

Right,

This is the other problem: I don't like these implicit things. What if we use a patch level in a release (eg. v4.2.1), then what will the next version?

The next version will always be <minor> + 1.

So I can hard code this scheme:

'4' '.' $MINOR+1 '.' '0'

right?

[theseion]

Yes, unless @fzipi or @dune73 have other opinions. Since we never really know what the next version will be, we simply use the next minor at the moment.

[fzipi]

Here we have two scenarios:

• the release is an LTS? Then it will be a patch probably, instead or minor. We should probably add -lts to the release 🤔
• any other case, next minor is good.

[theseion]

I think we could just skip running the check for LTS. Such commits will usually be backports anyway.

[dune73]

I am not 100% sure there is agreement and if yes, I have not understood it yet. It may take a few more words to make it clear for everybody.