Feature #7559
openAllow rule comment at end of rule line using #
Description
Currently, comments are not allowed behind rules (see https://redmine.openinfosecfoundation.org/issues/2856). A previous user experienced issues with this, thinking it should be supported but later closed the issue when he found out it was by design.
I am not sure whether there are good reasons to have this by design, but I would like to present a use-case for comments behind rules that might be compelling enough to permit this by design.
Currently, I am working on a linter for Suricata rules and one common feature shared by many linters is that they allow you to suppress issues on a per-line (or per-rule) basis. To do this, I intended to use comments behind rules (similar to how PyRight suppresses issues), an example of which is given below and which is further detailed on https://suricata-check.teuwen.net/ignore.html
alert ip any any -> any any (msg:"Test"; sid:1;) # suricata-check: ignore C.*'The current behavior (Suricata v7) is the rule fails to be parsed, whereas it is not considered invalid without the comment.
Error: detect-parse: no terminating ";" found
Error: detect: error parsing signature "alert ip any any -> any any (msg:"Test"; sid:1;) # suricata-check: ignore C.*" from file /tmp/6297954521506998625.rules at line 1
Error: suricata: Loading signatures failed.I do not foresee complications caused by adding this feature, except that care has to be taken when parsing rules that include the comment character \# inside the rule (e.g. msg field). I managed to work around this issue for my linter, so I imagine the same would be possible within the Suricata codebase.
Since I'm not familiar with the codebase of Suricata itself, I cannot estimate how much effort it is to implement, but I imagine it is relatively simple and could possibly be implemented by stripping the comment of lines.
Updated by Koen Teuwen 6 days ago
There is an additional note I would like to make. I did consider several alternatives for implementing a feature for the linter to suppress issues but they all have their own drawbacks.
- Suppress via separate file: This obscures which rules have suppressed issues for users and also makes it more difficult to integrate linters with IDEs such as Visual Studio Code.
- Suppress using comment at line above or below: This has the benefit that the suppression is closer to the rule being suppressed, but it makes it unclear to which rule the suppression belongs since the suppression comment may have a rule directly above and below.
Considering the drawbacks of these alternatives, I think it would be great to have this feature in Suricata.
Updated by Victor Julien 6 days ago
Most rule management tools don't keep comments, so I wonder if it makes sense to make it part of the rule itself. You could use the `metadata` keyword, where you could add something like
alert ip any any -> any any (msg:"Test"; sid:1; metadata:suricata-check "ignore C.*")
Metadata is a comma separated list of a key value pairs.
Updated by Koen Teuwen 5 days ago
Victor Julien wrote in #note-2:
Most rule management tools don't keep comments, so I wonder if it makes sense to make it part of the rule itself. You could use the `metadata` keyword, where you could add something like
[...]
Metadata is a comma separated list of a key value pairs.
That is a valid point, and an interesting suggestion. The removal of comments by management tools doesn't seem a problem when comments are removed during the transfer to production since these linting comments are irrelevant to security analysts.
I wonder if there are management tools out there that remove comments and simultaneously offer some sort of development environment to engineer rules. If that's the case, it would indeed be a very good suggestion to incorporate this into the rule itself.