Skip to content
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

add support for |all #1060

Merged
merged 11 commits into from
Jun 1, 2023
Merged

add support for |all #1060

merged 11 commits into from
Jun 1, 2023

Conversation

kazuminn
Copy link
Collaborator

@kazuminn kazuminn commented May 28, 2023

What Changed

fix #1038

  • add |all only feature

Evidence

I done a new test.

すでに、all識別子があったので、かぶらないように、allOnly識別子を追加しています。。

計算量は、既存のor andと同じです。すべてのキーワードのデータが一つになっているメンバがあったので、それを渡すようにしています。

@kazuminn kazuminn self-assigned this May 28, 2023
@codecov
Copy link

codecov bot commented May 28, 2023

Codecov Report

Patch coverage: 97.23% and project coverage change: +0.21 🎉

Comparison is base (949c7b6) 74.00% compared to head (189d185) 74.22%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1060      +/-   ##
==========================================
+ Coverage   74.00%   74.22%   +0.21%     
==========================================
  Files          24       24              
  Lines       18186    18360     +174     
==========================================
+ Hits        13459    13628     +169     
- Misses       4727     4732       +5     
Impacted Files Coverage Δ
src/detections/rule/selectionnodes.rs 92.10% <94.64%> (+0.26%) ⬆️
src/detections/rule/matchers.rs 96.77% <98.24%> (+0.06%) ⬆️
src/detections/rule/mod.rs 94.65% <100.00%> (+0.06%) ⬆️

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@hitenkoku hitenkoku marked this pull request as draft May 28, 2023 15:03
@kazuminn kazuminn requested review from hitenkoku, itiB and YamatoSecurity and removed request for itiB May 29, 2023 14:06
@kazuminn kazuminn changed the title [WIP] add support for |all add support for |all May 29, 2023
@kazuminn kazuminn marked this pull request as ready for review May 29, 2023 14:07
@kazuminn
Copy link
Collaborator Author

レビューお願いします。

Copy link
Collaborator

@fukusuket fukusuket left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

以下3点を確認しました!LGTMです!!🚀

  1. 修正前後でcsv-timeline -d sample-hayabusa-evtx --debugの結果ファイル差分なし。メモリ使用量同等

  2. 以下ルールの検知数とsearch --keywords mimikatzの件数が一致

detection:
    selection:
        '|all':
            - mimikatz
    condition: selection
  1. 以下ルールの検知数とsearch --keywords mimikatz + cat result.csv | grep -i invoke | wc -l の件数が一致
detection:
    selection:
        '|all':
            - invoke
            - mimikatz
    condition: selection

Copy link
Collaborator

@hitenkoku hitenkoku left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

不明点についてコメントしました。ご確認ください

src/detections/rule/matchers.rs Outdated Show resolved Hide resolved
src/detections/rule/mod.rs Outdated Show resolved Hide resolved
hitenkoku

This comment was marked as duplicate.

Copy link
Collaborator

@hitenkoku hitenkoku left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

endswithとallOnlyの区別ができなくなる個所が見つかったため解決するまでマージブロックします

src/detections/rule/matchers.rs Outdated Show resolved Hide resolved
Copy link
Collaborator

@YamatoSecurity YamatoSecurity left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The implementation LGTM! Thanks so much!

@YamatoSecurity
Copy link
Collaborator

@kazuminn 対応ありがとうございます!念のために確認ですが、UserDataにも対応していますか?

@kazuminn
Copy link
Collaborator Author

@YamatoSecurity はい。対応してます。

Copy link
Collaborator

@hitenkoku hitenkoku left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@YamatoSecurity YamatoSecurity merged commit ece00c4 into main Jun 1, 2023
@hitenkoku hitenkoku deleted the 1038-new-feature-all branch August 5, 2023 12:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

New feature: add support for |all
4 participants