-
Notifications
You must be signed in to change notification settings - Fork 795
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
Modify ExploiterResultData #1728
Modify ExploiterResultData #1728
Conversation
import pwd | ||
|
||
try: # can't import on Windows | ||
import pwd |
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.
This is solved by Vakaris in parsing credential telemetry. Check this commit
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.
I'm going to leave this in since this'll most likely get merged before that.
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.
If we move this import to line 37, then we don't need to try/except the import, since _get_home_dirs()
only runs if the check on line 24 passes.
( | ||
exploitation_result, | ||
propagation_result, | ||
os, | ||
info, | ||
attempts, | ||
error_message, | ||
) = self._puppet.exploit_host("PowerShellExploiter", "10.0.0.1", {}, None) |
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.
Instead of unpacking this, let's modify ExploitTelem
to accept an ExploiterResultsData
object.
We may have missed this in planning, but do we need any changes on the Island side to process the propagated and exploited booleans differently? |
I think we need to change it ( |
Let's do it here. These changes effectively change the interface between the Island and the Agent, so we need the Island to properly handle the new cases. |
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.
LGTM, good job!
:param result: The result from the 'exploit_host' method | ||
:param info: Information about the exploiter | ||
:param attempts: Information about the exploiter's attempts | ||
:param result: Data about the exploitation attempt (success status, info, attempts, etc) |
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.
We're using the word "attempt" in this comment to mean two different things.
What does this PR do?
Fixes a part of #1605.
PR Checklist
Is the TravisCI build passing?Was the CHANGELOG.md updated to reflect the changes?Was the documentation framework updated to reflect the changes?Testing Checklist
Added relevant unit tests?Have you successfully tested your changes locally? Elaborate:If applicable, add screenshots or log transcripts of the feature working