-
Notifications
You must be signed in to change notification settings - Fork 200
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Improving speed by changing EventID
matching process from regular expression
to exact string matching
#882
Improving speed by changing EventID
matching process from regular expression
to exact string matching
#882
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your pull request.
LGTM
I'm sorry. I checked the test section and had some questions, so I had to comment.
I would appreciate your response to my question.
EventID
matching processing from regular expression
to exact string matching
EventID
matching process from regular expression
to exact string matching
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm sorry. I checked the test section and had some questions, so I had to comment.
I would appreciate your response to my question.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@fukusuket Great find! I think it is better to do exact string matches by default and fall back to regular expression support if needed like in your PR. (Although it is probably not needed we might as well support it just in case.)
Codecov ReportBase: 69.49% // Head: 69.59% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## main #882 +/- ##
==========================================
+ Coverage 69.49% 69.59% +0.10%
==========================================
Files 23 23
Lines 13674 13719 +45
==========================================
+ Hits 9503 9548 +45
Misses 4171 4171
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
@fukusuket Sorry, i initially just want to change JQ to jq in the readme but then after checking changed a few things more in the JQ doc. as I want to fix it soon. I'll merge this PR if it is ready now? |
There are currently no rules in the Sigma repository with the following conditions:
and it seems that unlikely to be created(at this time). Therefore, in this PR, performance is prioritized and the following specifications are used.
|
Thank you for your review🙇 Yes, it's okay to merge this PR :) |
Currently all rule string matching processes use
regular expression match
.However, I think that
exact string match
is sufficient for matching process ofEventID
, except for rare cases like*
or?
🤔 (Just to be sure, I checked that there were no rules in the Sigma repository with non-numeric valueEventID
fields)What Changed
EventID
matching process has been changed to useexact string matching
in this PR.EventID
field can't be converted to a number.EventID
number conversion fails, use theregular expression match
, as before.Evidence
Environment
Benchmark
I ran a benchmark using this procedure(6.1GB evtx) and the results were as follows.
Console output
main
This PR
I would appreciate it if you could review🙏