Feature/str exact match#79

[itiB]

StartsWithとEndsWith、Containsの実装を行い正規表現以外の一致ルールを作成する。

close #79

[hach1yon]

ちょっと、気になる部分があって、parse_selection_recursively()という関数に引数を追加している部分ですね。
できれば、ルールを追加するときに変更するのはSelectionNodeトレイトをimplementしたクラスか、LeafMatcherトレイトをimplementしたクラスだけにしたいです。
ルールを追加する度にparse_selection_recursively()に引数を増やしていると、引数がすごく多くなってしまうので、できれば避けたいなという気持ちがあります。

[hach1yon]

あと、テストケースを書いてほしいです。
メインブランチにrule.rsのテストケースを追加したものをマージしているので、その辺を参考にしていただければと思います。

[hach1yon]

@itiB
プルリクありがとうございます。
今仕事が立て込んでいるので、すぐ確認できないのですが、なるべく次のミーティング迄にレビューできるようにしようと思います。

[hach1yon]

@itiB
正常系のパターンは正しく動いていて、良さそうです。さすがです。テストケースも良さそうです。

ちょっと、気になったのは以下の部分です。

ハイプに間違ったキーワードを指定した場合のエラー出力をしたい。

• 例えば、今の実装だと、"contains"と書くべきところを"contain"と書いた場合、書き間違いに気づけないという問題があるかなと思います。
• SelectionNodeトレイトを実装した構造体にはinit()という関数があって、この戻り値にResult::Errを返すと、detection.rsのparse_rule_filesでルールファイルのパースエラーを出力するような感じになっています。
• パイプ(|)で指定できるのは、startsWithとendsWithとcontainsの3つだけだったと思うので、それ以外の文字列がハイプに指定されていた場合、init関数でエラー出力されるようにしてほしいです。

new()関数に実装している内容をinit()関数に移動したい。

• LeafSelectionNode::new()関数で、StartsWithMatcher等の生成をしていますが、LeafSelectionNode::new()関数は前のままの内容にしておいて、init()関数にStartsWithMatcher等の生成を移動したいです。
• SelectionNodeトレイトの設計方針として、ルールファイルのパースやパースエラーはinit()関数で実装するようになっていて、new()関数だとエラーは返せません。ハイプに間違ったキーワードを指定した場合のエラー出力する処理はinit()関数に書くので、StartsWithMatcher等の生成処理もinit関数にまとめた方が書きやすいような気がします。

[hitenkoku]

@ichiichi11 さんのレビューコメントを追加しておきました。とりあえずCommentで出しておきます

[hitenkoku]

@ichiichi11 さんの記載に関連するところをコードと紐づけてレビューに入れておきました。(レビュー内容とコメントでのやり取りを容易にするため)

ハイプに間違ったキーワードを指定した場合のエラー出力をしたい。
例えば、今の実装だと、"contains"と書くべきところを"contain"と書いた場合、書き間違いに気づけないという問題があるかなと思います。
SelectionNodeトレイトを実装した構造体にはinit()という関数があって、この戻り値にResult::Errを返すと、detection.rsのparse_rule_filesでルールファイルのパースエラーを出力するような感じになっています。
パイプ(|)で指定できるのは、startsWithとendsWithとcontainsの3つだけだったと思うので、それ以外の文字列がハイプに指定されていた場合、init関数でエラー出力されるようにしてほしいです。あと、現状パイプが複数あることもないと思うので、その場合もエラーにしてほしいです。

[hitenkoku]

@itiB 私の方でも確認しました。問題ないと思います。 @ichiichi11 さんが記載いただいているとところとして

こちらですが、現状のSIGMAルールの内容を確認したところ、パイプ(|)の後につながるものとしてallとかがあります(参照)。今回の作成範囲ではないので切り分けとして、init関数で想定していないルールのパイプが出てきたらエラー出力させるようにするのは賛成です。

https://github.com/SigmaHQ/sigma/blob/fb07b204b4f0284ebf6e684d5e55513c26dc932d/rules/windows/builtin/win_cobaltstrike_service_installs.yml

[itiB]

パイプでつなげられたものを含めてエラーが出るように変更しました、
また下のほうにエラーが出ることを確認するテストケースを追加しました。

こうやって new() と init() を使い分けるのですね....いままであんまり考えずに書いていました😥
指摘ありがとうございます！

[hach1yon]

@itiB
いい感じなので、マージしました。

new() と init() を使い分けについては、今回は自分がそういう意図で設計していたので、指摘させてもらいました。こういうファイルをパースするような時の実装方法・設計方針は色々あると思うので、必ずしもこの方法が良いという感じではないです。

最初にSelectionNodeとかLeafMatcherとかのトレイト作ったのは私で、自分以外の人に設計意図が伝わらないソースを書いてしまったという私の問題かなーと思いました。もうちょっと、コメントを書くようにします。

[hitenkoku]

@itiB さん 私の方でも確認致しました。動作として問題ないと思います。 @ichiichi11 さんのコメントはやり取りをConversationで管理したほうが良いと思いますので私のレビューのほうに入れておきました。