1597 implement exploitation #1657
Conversation
The AutomatedMaster will need access to the monkey's tunnel, IP addresses, and default server in order to properly configure the victim host. The VictimHostFactory can abstract these dependencies away and handle these details on behalf of the AutomatedMaster.
mssalvatore
requested review from
ilija-lazoroski,
VakarisZ and
shreyamalviya
| def build_victim_host(self, ip: str): | ||
| victim_host = VictimHost(ip) | ||
|
|
||
| # TODO: Reimplement the below logic from the old monkey.py |
There was a problem hiding this comment.
This will get done when we integrate the automated master with monkey.py
Co-authored-by: Shreya Malviya <shreya.malviya@gmail.com>
| if stop.is_set(): | ||
| break |
There was a problem hiding this comment.
Same consideration as before: each layer of logic contains the same
if stop.is_set():
return
This logic is not specific to the Exploiter, it's an exception logic. Maybe there's a way to raise an exception when stop.is_set() instead. Maybe this can be solved with an interface/re-usable decorator
There was a problem hiding this comment.
A decorator won't work because the Event needs to be available on import, but we could potentially add a utility function that handles this. We're going to need it for the plugins as well.
There was a problem hiding this comment.
For the moment I'm going to leave it. Each loop is using it in a slightly different way, or has slightly different requirements. As we accumulate more, similar loops, we can find the common patterns and make them reusable. At the moment I think it may be too early to do so.
| self, exploiter_name: str, victim_host: VictimHost, stop: Event | ||
| ) -> ExploiterResultData: | ||
| logger.debug(f"Attempting to use {exploiter_name} on {victim_host}") | ||
| return self._puppet.exploit_host(exploiter_name, victim_host.ip_addr, {}, stop) |
There was a problem hiding this comment.
Maybe it's worth considering creating the stop event in the agent. Then, puppet and master will be created with it. This way we wouldn't need to pass stop through the parameters everywhere. This would de-couple stopping logic from execution logic a bit, for better or worse.
There was a problem hiding this comment.
That would leak too much information into the agent, give the agent the ability to stop parts of the master that it shouldn't, and limit how the master is able to control functionality.
The stop signal is something that the master can control per-run of the Propagator. The master controls the Propagator, the Propagator controls the scanner and exploiter. Giving the higher-level abstractions the ability to skip layers and control the lower-level abstractions increases complexity and coupling.
What does this PR do?
Calls the exploiters from the AutomatedMaster for issue #1597
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
If applicable, add screenshots or log transcripts of the feature working