-
Notifications
You must be signed in to change notification settings - Fork 775
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
Old machine bootloader #527
Old machine bootloader #527
Conversation
…there still is a timeout
Some notes on the design. For building the bootloader, I suggest doing the following Regarding libcurl and openssl. If we are connecting to the Monkey, then we probably only need libcurl. I don't think we need our version of openssl. I think the open port should not be hard coded to be 5001, but rather +1 on the current server port. This is a simple change and will also help in the Monkey Island side. We will also need to update documentation on the Island. Regarding data collected. I suggest the following fields
Regarding saving the bootloader message. I suggest turning it into a telemetry message then it will be saved in that collection automatically. This doesn't require us to really include JSON parsing in the bootloader, since it's a pretty hard coded message. |
|
1 - Ok. |
Regarding pyinstaller env, I think the best for the dev enviornment is just building from git source rather than Pypi. Pip supports that. We should explicitly list our repo and then we don't need to copy/override things. |
Regarding basic Island http server. Is this also a tornado/flask server or something else? |
ATM it's vanilla python BaseHTTPServer |
…ng and icon placeholders
… windows bootloaders
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.
Haven't finished my review yet, but some comments are here already. Will continue tomorrow
monkey/infection_monkey/monkey.py
Outdated
|
||
#TODO change back | ||
time.sleep(WormConfiguration.keep_tunnel_open_time) | ||
|
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.
TODO left in the code
@@ -108,7 +110,20 @@ def log_message(self, format_string, *args): | |||
class HTTPConnectProxyHandler(http.server.BaseHTTPRequestHandler): | |||
timeout = 30 # timeout with clients, set to None not to make persistent connection | |||
proxy_via = None # pseudonym of the proxy in Via header, set to None not to modify original Via header | |||
protocol_version = "HTTP/1.1" | |||
#protocol_version = "HTTP/1.1" |
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.
Don't leave commented out code
self.send_response(200) | ||
self.end_headers() | ||
self.wfile.write(r.content) | ||
self.connection.close() |
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 we wrap this function with try except finally
and then close the connection on the finally
?
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.
Not a necessity, but we can
monkey/monkey_island/cc/main.py
Outdated
logger.info("Starting bootloader server") | ||
mongo_url = os.environ.get('MONGO_URL', env.get_mongo_url()) | ||
bootloader_server_thread = Thread(target=BootloaderHttpServer(mongo_url).serve_forever, daemon=True) | ||
# island_server_thread = Thread(target=start_island_server) |
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.
Commented out code
monkey/monkey_island/cc/main.py
Outdated
bootloader_server_thread = Thread(target=BootloaderHttpServer(mongo_url).serve_forever, daemon=True) | ||
# island_server_thread = Thread(target=start_island_server) | ||
|
||
bootloader_server_thread.start() | ||
#island_server_thread.start() | ||
start_island_server() | ||
bootloader_server_thread.join() |
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.
Extract all this to function
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.
What do you mean? It's already the only lines lines in this function.
@staticmethod | ||
def parse_bootloader_request_linux(request_data: bytes) -> Dict[str, str]: | ||
parsed_data = json.loads(request_data.decode().replace("\n", "") | ||
.replace("NAME=\"", "") | ||
.replace("\"\"", "\"") | ||
.replace("\":\",", "\":\"\",")) | ||
return parsed_data | ||
|
||
@staticmethod | ||
def parse_bootloader_request_windows(request_data: bytes) -> Dict[str, str]: | ||
return json.loads(request_data.decode("utf-16", "ignore")) |
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.
Cover this with UTs - the tests will be good documentation as well
else: | ||
return make_response({"status": "OS_NOT_FOUND"}, 404) | ||
|
||
resp = BootloaderService.parse_bootloader_telem(data) |
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.
rename resp
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.
Added more comments
info = this.props.item.group.includes('monkey', 'manual') ? this.infectedAssetInfo(this.props.item) : | ||
this.props.item.group !== 'island' ? this.assetInfo(this.props.item) : this.islandAssetInfo(); |
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 we re-write this conditinal to be more explicit and clearer? it's very hard to understand even upon second reading
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'll rewrite it, even though this is not mine.
// This list must correspond to the one on back end in cc/services/utils/node_groups.py | ||
const groupNames = ['clean_unknown', 'clean_linux', 'clean_windows', 'exploited_linux', 'exploited_windows', 'island', | ||
'island_monkey_linux', 'island_monkey_linux_running', 'island_monkey_windows', 'island_monkey_windows_running', | ||
'manual_linux', 'manual_linux_running', 'manual_windows', 'manual_windows_running', 'monkey_linux', | ||
'monkey_linux_running', 'monkey_windows', 'monkey_windows_running']; | ||
'island_monkey_linux', 'island_monkey_linux_running', 'island_monkey_linux_starting', 'island_monkey_windows', | ||
'island_monkey_windows_running', 'island_monkey_windows_starting', 'manual_linux', 'manual_linux_running', | ||
'manual_windows', 'manual_windows_running', 'monkey_linux', 'monkey_linux_running', 'monkey_windows', | ||
'monkey_windows_running', 'monkey_windows_starting', 'monkey_linux_starting', 'monkey_windows_old', | ||
'monkey_linux_old' ]; |
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.
Perhaps a joined JSON file in common
somewhere is a better solution?
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 exported it to a file, although it would be better to just send it from backend.
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.
On a second thought lets refactor
tst1 = NodeGroups.get_group_by_keywords(['island']) == NodeGroups.ISLAND | ||
tst2 = NodeGroups.get_group_by_keywords(['running', 'linux', 'monkey']) == NodeGroups.MONKEY_LINUX_RUNNING | ||
tst3 = NodeGroups.get_group_by_keywords(['monkey', 'linux', 'running']) == NodeGroups.MONKEY_LINUX_RUNNING | ||
tst4 = False | ||
try: | ||
NodeGroups.get_group_by_keywords(['bogus', 'values', 'from', 'long', 'list', 'should', 'fail']) | ||
except NoGroupsFoundException: | ||
tst4 = True | ||
self.assertTrue(tst1 and tst2 and tst3 and tst4) |
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.
Cool test :)
It would be better to assert each statement separately I think
@VakarisZ I think you need to update the pip requirement to aim at our pyinstaller fork |
|
||
class TestNodeGroups(TestCase): | ||
|
||
def test_get_group_by_keywords(self): |
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.
Use self.assertEqual(NodeStates.get_by_keywords(['island']), NodeStates.ISLAND)
instead of assertTrue for tests 1,2 and 3
Use self.assertRaises(ExpectedException, afunction)
for test 4
…ng machine used in tests
…loader # Conflicts: # monkey/monkey_island/cc/main.py
Codecov Report
@@ Coverage Diff @@
## develop #527 +/- ##
===========================================
+ Coverage 56.15% 56.72% +0.56%
===========================================
Files 105 112 +7
Lines 3588 3820 +232
===========================================
+ Hits 2015 2167 +152
- Misses 1573 1653 +80
Continue to review full report at Codecov.
|
As per #527, this code was added for the bootloader. Now that we're removing the bootloader, this is no longer needed.
As per #527, this code was added for the bootloader. Now that we're removing the bootloader, this is no longer needed.
As per #527, this code was added for the bootloader. Now that we're removing the bootloader, this is no longer needed.
What is this?
Fixes #479
Add any further explanations here.
TODO
TDD
Prerequisites
Old machine bootloader program is incorporated into pyinstaller bootloader. Pyinstaller bootloader is the first thing that starts when a built monkey is launched, thus our code is launched before monkey and has access to monkey flags. This allows us to write code that would inspect the current machine and send requests to island directly or via tunnel and decide to continue with launching monkey code or to quit.
This is a multilevel problem so TDD is separated into these sections that represent corresponding layers:
Bootloader program
How to setup bootloader development env.
We have pyinstaller forks, one for linux and one for windows. Pull them and get relevant bootloader dev env.
To update pyinstaller we'll have to pull from pyinstaller git, merge and rebuild pyinstaller bootloader. Then do a release and add built pyinstaller bootloader.
To update bootloader we'll have to push to our forks and rebuild pyinstaller bootloader. Then do a release and add built pyinstaller bootloader.
Monkey build process
Building the monkey will require custom pyinstaller bootloader binary. Depending on OS this binary should be downloaded from our pyinstaller for release and placed into a pyinstaller install folder. Maybe this can be automated via deployment scripts.
Bootloader program workflow
Connections and tunneling
Monkey
Tunneling on monkey is expanded to tunnel http traffic.
Island
Island starts a basic http server on a separate thread. It connects to the mongodb and gets port to serve on. It's either island's port +1 or a dedicated config value. When this HTTP proxy server gets a request it passes it to the
/api/bootloader
endpoint on the main server and also forwards back the response.Island server and data gathered
Island receives data on an
api/bootloader
endpoint and stores it on a separate collection. Each bootloader telemetry has the following fields: OS, IP list, Hostname and if monkey will be run. Upon receiving bootloader telemetry island finds and updatesbootloader_state
variable on corresponding node.bootloader_state
can be one of three values:monkey_will_run
value is True in bootloader telem )monkey_will_run
value is False in bootloader telem )Data representation
Map
States of node will be the following:
exploited: False
bootloader_state: not_started
on node)exploited: True
bootloader_state: not_started
on node)exploited: True
bootloader_state: monkey_won't_run
on node)OR
exploited: True
bootloader_state: monkey_will_run
on node)To recap 2 states with corresponding UI will be added: Machine too old to run monkey and Monkey will run.
Report
When generating report island will query mongodb to find if there are any entries of bootloader telemetry with
bootloader_state: monkey_won't_run
. If so, outdated machine issue is generated.Testing
Bootloader binary
Manual tests on some old windows and linux machines. Also, on some new ones.
Tunneling
Windows machine will be added to tunneling env in monkeyzoo. New blackbox test will run tunneling test as usual, but will also verify that each bootloader communicated with island.
Data gathering and UI
Tested manually
Dev. env. deployment
This feature changes development environment in the following way:
So deployment scripts will have to be altered to do: