-
Notifications
You must be signed in to change notification settings - Fork 397
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
Module wcc added some defender checks #306
Conversation
Any screenshot possible @jubeaz ? :) |
sure this is what you get visually (the screenshot is restricted to my checks) One thing that might be confusing is that when I do perform checks on defender parameters (exclusions, IOAV...) I do not check the state of defender itself. If you look at FENRIS this is a server without defender installed but as there is a GPO that apply some parameter to defender the policy is taken into account when computing parameters. For registry I do check value set on a computer only if the value is not set by policies. To mitigate that I have decided to write detailed reasons inside the DB If you prefer I can correct the reason could be KO with reason N/A if defender is not running but it will be slower. |
@jubeaz can you run Ruff against this? |
Also, why the heck is the pipeline not running sometimes |
It runs when an owner commits or we approve, I believe, otherwise we'd overuse our pipeline quota pretty fast. |
Ah you are kinda right, it blocks runs for first time contributors. If code from that contributor has been merged before, it will trigger the pipeline. |
ok I've applied the linter. Sorry I'm kind of new in development process and I did not carefully enough read the CONTRIBUTING.md hope I'm not giving you too much work |
@jubeaz don't worry about it :D we hadn't updated the PR template until after you filed this. |
@jubeaz this looks great except for the final two checks aren't logging the policy and specific reason to the log inside |
@fpreynaud take a look at this man :P |
@jubeaz sorry for the late response. That makes sense to me. If you can fix the conflicts we can get this merged. |
Hello, done. |
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.
Thanks for the PR!
@jubeaz i have integrated the checks into the existing code infrastructure, fixed a few false positives and bugs. Please retest the module to ensure i didn't break anything
ahh what can happen... lets merge |
Hello,
I've added some checks regarding Defender AV.
I've corrected a bug inside check_registry regarding the value of the
op
var.I did not want touch too much to the
check_registry
, but I think it can be merged with the function that I've createdcheck_single_registry_with_policy
.in order to allow the base tuple of
check_registry
to include the policies registry or None.regards,