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

feat: show alert level for stacking command #113

Merged
merged 16 commits into from
Feb 20, 2024
Merged

Conversation

fukusuket
Copy link
Collaborator

@fukusuket fukusuket commented Feb 17, 2024

What Changed

Standard output specifications

  • Output Count, level, RuleTitle in table layout
  • Sort by level(cirt -> high -> med -> low)
  • If the number of alerts is the same, sort in alphabetical order

I tried creating it with the above specifications, but what do you think? Please let me know if there is anything that should be changed :)

Test

In the integration test below, I confirmed that the altert level is output to stdout.

I would appreciate it if you could check it out when you have time🙏

@fukusuket fukusuket added the enhancement New feature or request label Feb 17, 2024
@fukusuket fukusuket added this to the v2.4.0 milestone Feb 17, 2024
@fukusuket fukusuket self-assigned this Feb 17, 2024
@YamatoSecurity
Copy link
Collaborator

@fukusuket Thanks so much! Sorry, I don't think my explanation was good. I want to combine all of these into one big table.

Screenshot 2024-02-18 at 08 02 39

So the columns would be Count, Cmdline, NumOfAlerts, Levels, Alerts. Both when outputting to terminal and saving the CSV. I like how you sort by alert level first, and then how many alerts there were. Let's keep this order.
Sometimes command lines are very long, so we probably need to specify how many characters in a single line and do word wrapping so that the table does not overflow the screen.

@fukusuket fukusuket changed the title feat: show alert level for staking command feat: show alert level for stacking command Feb 18, 2024
@fukusuket
Copy link
Collaborator Author

fukusuket commented Feb 18, 2024

@YamatoSecurity
Thank you so much for checking :) I have changed it as follows
https://github.com/Yamato-Security/takajo/actions/runs/7949301433/job/21700287941

Also, depending on the command,
there seemed to be a lot of noise when the info level was output..., so I tried adding the --level option.

  • --level informational(default): stack-dns/tasks/services
  • --level low(default): stack-processes/cmdlines

what do you think?

@YamatoSecurity
Copy link
Collaborator

@fukusuket Thanks for adding the --level option! stack-services was using sysmon instead of system so I fixed it.
Sorry, just a few more things. Can you add an Event column after the Count that displays where the event came from. For example Sec-4688, Sysmon-1, System-7050, etc...
Also, combining the results with -> is harder to read than I first thought. Is it possible to separate in new columns instead of in the same column separated by -> ?

@fukusuket
Copy link
Collaborator Author

fukusuket commented Feb 19, 2024

@YamatoSecurity
Thank you so much for checking and fixing :) I implemented #113 (comment) ! what do you think?
https://github.com/Yamato-Security/takajo/actions/runs/7961384382/job/21732479063

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.

@fukusuket Looks great! Thanks so much. I changed the , separator to | to make it easier to read and added some messages about what log events are being used. If my commits look ok, please merge this.

@fukusuket
Copy link
Collaborator Author

fukusuket commented Feb 20, 2024

@YamatoSecurity
Thank you so much! | is easier to read :) I was able to confirm that it works properly!

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 851353f into main Feb 20, 2024
3 checks passed
@YamatoSecurity YamatoSecurity deleted the 112-show-alert-level branch February 20, 2024 22:50
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.

show alert levels and rule names for stacking commands
3 participants