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

[Rule Tuning] User Added to Privileged Group #3763

Merged
merged 5 commits into from
Jun 7, 2024
Merged

[Rule Tuning] User Added to Privileged Group #3763

merged 5 commits into from
Jun 7, 2024

Conversation

w0rk3r
Copy link
Contributor

@w0rk3r w0rk3r commented Jun 7, 2024

Issues

Related to #3005

Summary

Expands the rule to detect modifications to these groups on non-English AD envs.

@w0rk3r w0rk3r added Rule: Tuning tweaking or tuning an existing rule OS: Windows windows related rules Domain: Endpoint backport: auto labels Jun 7, 2024
@w0rk3r w0rk3r self-assigned this Jun 7, 2024
@w0rk3r w0rk3r changed the title [New Rule] User Added to Privileged Group [Rule Tuning] User Added to Privileged Group Jun 7, 2024
Comment on lines 90 to 99
group.id : (
"S-1-5-32-544",
"S-1-5-32-544",
"S-1-5-21-*-512",
"S-1-5-21-*-519",
"S-1-5-21-*-551",
"S-1-5-21-*-518",
"S-1-5-21-*-1101",
"S-1-5-21-*-1102"
)
Copy link
Contributor

Choose a reason for hiding this comment

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

better use endswith for performance when using multi wildcards checks, also not sure if 1101 and 1102 are std group ids ?

Suggested change
group.id : (
"S-1-5-32-544",
"S-1-5-32-544",
"S-1-5-21-*-512",
"S-1-5-21-*-519",
"S-1-5-21-*-551",
"S-1-5-21-*-518",
"S-1-5-21-*-1101",
"S-1-5-21-*-1102"
)
endswith(group.id, "S-1-5-32-544", "-512", "-519", "-551", "-518")
)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

1101 and 1102

Not sure either, but these are commonly related to the DNS Admins group, but it seems that there is no "official" RID for it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

better use endswith for performance when using multi wildcards checks

Isn't that the same as using the wildcard? I think this was the conclusion

Copy link
Contributor

@terrancedejesus terrancedejesus left a comment

Choose a reason for hiding this comment

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

Left a couple ideas and suggestions. Up to you, thanks for tuning!

@w0rk3r w0rk3r merged commit 087e8a6 into main Jun 7, 2024
9 checks passed
@w0rk3r w0rk3r deleted the rt_group_sid branch June 7, 2024 16:43
protectionsmachine pushed a commit that referenced this pull request Jun 7, 2024
* [New Rule] User Added to Privileged Group

* add more groups

* Update rules/windows/persistence_user_account_added_to_privileged_group_ad.toml

Co-authored-by: Terrance DeJesus <[email protected]>

* Update persistence_user_account_added_to_privileged_group_ad.toml

---------

Co-authored-by: Terrance DeJesus <[email protected]>

(cherry picked from commit 087e8a6)
protectionsmachine pushed a commit that referenced this pull request Jun 7, 2024
* [New Rule] User Added to Privileged Group

* add more groups

* Update rules/windows/persistence_user_account_added_to_privileged_group_ad.toml

Co-authored-by: Terrance DeJesus <[email protected]>

* Update persistence_user_account_added_to_privileged_group_ad.toml

---------

Co-authored-by: Terrance DeJesus <[email protected]>

(cherry picked from commit 087e8a6)
protectionsmachine pushed a commit that referenced this pull request Jun 7, 2024
* [New Rule] User Added to Privileged Group

* add more groups

* Update rules/windows/persistence_user_account_added_to_privileged_group_ad.toml

Co-authored-by: Terrance DeJesus <[email protected]>

* Update persistence_user_account_added_to_privileged_group_ad.toml

---------

Co-authored-by: Terrance DeJesus <[email protected]>

(cherry picked from commit 087e8a6)
protectionsmachine pushed a commit that referenced this pull request Jun 7, 2024
* [New Rule] User Added to Privileged Group

* add more groups

* Update rules/windows/persistence_user_account_added_to_privileged_group_ad.toml

Co-authored-by: Terrance DeJesus <[email protected]>

* Update persistence_user_account_added_to_privileged_group_ad.toml

---------

Co-authored-by: Terrance DeJesus <[email protected]>

(cherry picked from commit 087e8a6)
protectionsmachine pushed a commit that referenced this pull request Jun 7, 2024
* [New Rule] User Added to Privileged Group

* add more groups

* Update rules/windows/persistence_user_account_added_to_privileged_group_ad.toml

Co-authored-by: Terrance DeJesus <[email protected]>

* Update persistence_user_account_added_to_privileged_group_ad.toml

---------

Co-authored-by: Terrance DeJesus <[email protected]>

(cherry picked from commit 087e8a6)
protectionsmachine pushed a commit that referenced this pull request Jun 7, 2024
* [New Rule] User Added to Privileged Group

* add more groups

* Update rules/windows/persistence_user_account_added_to_privileged_group_ad.toml

Co-authored-by: Terrance DeJesus <[email protected]>

* Update persistence_user_account_added_to_privileged_group_ad.toml

---------

Co-authored-by: Terrance DeJesus <[email protected]>

(cherry picked from commit 087e8a6)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport: auto Domain: Endpoint OS: Windows windows related rules Rule: Tuning tweaking or tuning an existing rule
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants