-
Notifications
You must be signed in to change notification settings - Fork 54
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 computer SID for Windows systems #824
Conversation
It returns the machine SID found in the SAM hive.
This reverts commit 9c214a1.
- The machine SID from the SAM hive - The domain SID from the security policy
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #824 +/- ##
==========================================
- Coverage 77.02% 77.02% -0.01%
==========================================
Files 322 322
Lines 27566 27582 +16
==========================================
+ Hits 21232 21244 +12
- Misses 6334 6338 +4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
@fox-evv I've suggested some changes and also added some test with this commit. One thing I could not verify myself is whether there can be multiple Machine / Domain SIDs. The initial implementation kind of seemed to suggest this. Do you encounter this scenario yourself? I left it out of this implementation because it seemed illogical to me. |
"""Return the machine- and optional domain SID of the system.""" | ||
|
||
try: | ||
key = self.target.registry.key("HKLM\\SAM\\SAM\\Domains\\Account") |
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.
Can there be multiple Machine- and Domain SIDS associated with one system?
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.
@Horofic can you fix these notes or create a ticket for them?
currentversion_key_name = "Software\\Microsoft\\Windows NT\\CurrentVersion" | ||
currentversion_key = VirtualKey(hive_hklm, currentversion_key_name) | ||
currentversion_key.add_value("InstallDate", VirtualValue(hive_hklm, "InstallDate", 0)) | ||
hive_hklm.map_key(currentversion_key_name, currentversion_key) | ||
target_win_users.add_plugin(GenericPlugin) | ||
|
||
assert target_win_users.install_date == from_unix(0) | ||
|
||
|
||
def test_windows_generic_sid(target_win: Target, hive_hklm: VirtualHive): |
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.
Missing return type hint
|
||
|
||
def test_windows_generic_install_date(target_win_users, fs_win, hive_hklm): | ||
def test_windows_generic_install_date(target_win_users: Target, hive_hklm: VirtualHive): |
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.
Missing return type hint
"windows/sid/computer", | ||
[ | ||
("datetime", "ts"), | ||
("string", "sidtype"), |
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.
Should be type
and not sidtype
.
This addition to the generic Windows plugin exports the machine and domain SIDs
of the target. If the system is not joined to a domain, it uses only the/
SAM hive, but if the system is domain joined, it will all so use
the Security Policy.