Feat/classtype unknown/v17 #4285
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Issue warning instead of erroring and invalidating the rule. It's not a very serious issue, so don't error out.
Introduce Signature init_flag to indicate priority has been set. This will be needed in a follow-up classtype update. Detect duplicate priority instances in a keyword, and use the highest priority in the rule. Do issue a warning in this case.
Detect duplicate instances and use the one with the highest priority. Use new priority flag to make the logic around explicit priority sets easier to follow. Minor code cleanups. Also clean up unittests.
Reduce memory use by making sure SCClassConfClasstype has a more optimal memory layout.
Switch from u8 to u16 to allow for more classtypes. Rename Signature::class to Signature::class_id to make it clear it is an id.
Effect of classification on Suricata's working is minimal. Impact of adding undefined classtypes is large: rules will fail to load completely. This also leads multiple lines of log output per rule, which in a large ruleset can lead to excessive output. This patch changes the classtype keyword behavior. Instead of erroring and invalidating a rule, we will merely warn. The undefined classtype is then defined with a default priority, so other rules using the classtype will not also warn. This way there will be just a single warning per missing classtype.
Still initialize the classtype hash table so that the classtypes rules use can be added to it. The file missing now reports a warning instead of error, as we will continue to work.
References are currently not used in Suricata, so erroring out on rules using a undefined reference is too harsh. Just issue a warning once per unique missing reference.
Add --strict-rule-keywords commandline option to enable strict rule parsing. It can be used without options or with a comma separated list: --strict-rule-keywords --strict-rule-keywords=all --strict-rule-keywords=classtype,reference Parsing implementations can use SigMatchStrictEnabled to check if strict parsing is enabled for them and act accordingly. SQUASH strict option
A sigmatches 'Setup' function may indicate it intends to fail silently after the first error. It will return -2 instead of -1 in this case. This is tracked in the DetectEngineCtx object, so errors will be shown again at rule reloads.
Use 'silent error' logic for any other rules using ja3 as well.
|
With respect to the commit "detect/ja3: print error for one rule only", I'm still seeing multiple errors. After install and rule update I changed ja_fingerprints to no, and I still get this in my output: Plus many more. |
|
Weird, it appears using |
This was referenced Oct 9, 2019
Closed
|
Merged as part of #4288 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Replaces #4282.
Fix compile warning.
Support all ja3 keywords.
PRScript output (if applicable):
https://buildbot.openinfosecfoundation.org/builders/victorjulien/builds/326
https://buildbot.openinfosecfoundation.org/builders/victorjulien-pcap/builds/325