From 006c177abdb7ffbe006ceb0ad1aa7d2346406318 Mon Sep 17 00:00:00 2001 From: Shay Nehmad Date: Wed, 2 Oct 2019 13:04:58 +0300 Subject: [PATCH 1/6] Added lock on report generation and improved the get_completed_steps method --- monkey/monkey_island/cc/resources/root.py | 47 ++++++++++++++++------- 1 file changed, 33 insertions(+), 14 deletions(-) diff --git a/monkey/monkey_island/cc/resources/root.py b/monkey/monkey_island/cc/resources/root.py index e3b3e9854..f1914b3a2 100644 --- a/monkey/monkey_island/cc/resources/root.py +++ b/monkey/monkey_island/cc/resources/root.py @@ -1,5 +1,6 @@ from datetime import datetime import logging +import threading import flask_restful from flask import request, make_response, jsonify @@ -18,13 +19,15 @@ logger = logging.getLogger(__name__) class Root(flask_restful.Resource): + def __init__(self): + self.report_generating_lock = threading.Event() def get(self, action=None): if not action: action = request.args.get('action') if not action: - return Root.get_server_info() + return self.get_server_info() elif action == "reset": return jwt_required()(Database.reset_db)() elif action == "killall": @@ -34,11 +37,10 @@ class Root(flask_restful.Resource): else: return make_response(400, {'error': 'unknown action'}) - @staticmethod @jwt_required() - def get_server_info(): + def get_server_info(self): return jsonify(ip_addresses=local_ip_addresses(), mongo=str(mongo.db), - completed_steps=Root.get_completed_steps()) + completed_steps=self.get_completed_steps()) @staticmethod @jwt_required() @@ -49,17 +51,34 @@ class Root(flask_restful.Resource): logger.info('Kill all monkeys was called') return jsonify(status='OK') - @staticmethod @jwt_required() - def get_completed_steps(): + def get_completed_steps(self): is_any_exists = NodeService.is_any_monkey_exists() infection_done = NodeService.is_monkey_finished_running() - if not infection_done: - report_done = False - else: - if is_any_exists: - ReportService.get_report() - AttackReportService.get_latest_report() + + if infection_done: + if self.should_generate_report(): + self.generate_report() report_done = ReportService.is_report_generated() - return dict(run_server=True, run_monkey=is_any_exists, infection_done=infection_done, - report_done=report_done) + else: # Infection is not done + report_done = False + + return dict( + run_server=True, + run_monkey=is_any_exists, + infection_done=infection_done, + report_done=report_done) + + def generate_report(self): + # Set the event - enter the critical section + self.report_generating_lock.set() + # Not using the return value, as the get_report function also saves the report in the DB for later. + _ = ReportService.get_report() + _ = AttackReportService.get_latest_report() + # Clear the event - exit the critical section + self.report_generating_lock.clear() + + def should_generate_report(self): + # If the lock is not set, that means no one is generating a report right now. + is_any_thread_generating_a_report_right_now = not self.report_generating_lock.is_set() + return is_any_thread_generating_a_report_right_now and not ReportService.is_latest_report_exists() From 19dcf689fea7f076054ac678398d4346deb6ed86 Mon Sep 17 00:00:00 2001 From: Shay Nehmad Date: Wed, 2 Oct 2019 13:12:48 +0300 Subject: [PATCH 2/6] Added documentation --- monkey/monkey_island/cc/resources/root.py | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/monkey/monkey_island/cc/resources/root.py b/monkey/monkey_island/cc/resources/root.py index f1914b3a2..f2978f1ee 100644 --- a/monkey/monkey_island/cc/resources/root.py +++ b/monkey/monkey_island/cc/resources/root.py @@ -20,6 +20,9 @@ logger = logging.getLogger(__name__) class Root(flask_restful.Resource): def __init__(self): + # This lock will allow only one thread to generate a report at a time. Report generation can be quite + # slow if there is a lot of data, and the UI queries the Root service often; without the lock, these requests + # would accumulate, overload the server, eventually causing it to crash. self.report_generating_lock = threading.Event() def get(self, action=None): @@ -39,8 +42,10 @@ class Root(flask_restful.Resource): @jwt_required() def get_server_info(self): - return jsonify(ip_addresses=local_ip_addresses(), mongo=str(mongo.db), - completed_steps=self.get_completed_steps()) + return jsonify( + ip_addresses=local_ip_addresses(), + mongo=str(mongo.db), + completed_steps=self.get_completed_steps()) @staticmethod @jwt_required() @@ -70,15 +75,16 @@ class Root(flask_restful.Resource): report_done=report_done) def generate_report(self): - # Set the event - enter the critical section + # Set the event when entering the critical section self.report_generating_lock.set() # Not using the return value, as the get_report function also saves the report in the DB for later. _ = ReportService.get_report() _ = AttackReportService.get_latest_report() - # Clear the event - exit the critical section + # Clear the event when leaving the critical section self.report_generating_lock.clear() def should_generate_report(self): # If the lock is not set, that means no one is generating a report right now. is_any_thread_generating_a_report_right_now = not self.report_generating_lock.is_set() - return is_any_thread_generating_a_report_right_now and not ReportService.is_latest_report_exists() + is_there_a_need_for_a_new_report = not ReportService.is_latest_report_exists() + return is_any_thread_generating_a_report_right_now and is_there_a_need_for_a_new_report From f5d78508025ce10d8b18d4d938ab590c5a626278 Mon Sep 17 00:00:00 2001 From: Shay Nehmad Date: Wed, 2 Oct 2019 13:21:55 +0300 Subject: [PATCH 3/6] Bad import -_- --- monkey/infection_monkey/network/network_scanner.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/monkey/infection_monkey/network/network_scanner.py b/monkey/infection_monkey/network/network_scanner.py index 9452a3fb8..f0145e843 100644 --- a/monkey/infection_monkey/network/network_scanner.py +++ b/monkey/infection_monkey/network/network_scanner.py @@ -6,7 +6,7 @@ from infection_monkey.config import WormConfiguration from infection_monkey.model.victim_host_generator import VictimHostGenerator from infection_monkey.network.info import local_ips, get_interfaces_ranges from infection_monkey.network import TcpScanner, PingScanner -from infection_monkey.utils import is_windows_os +from infection_monkey.utils.environment import is_windows_os if is_windows_os(): from multiprocessing.dummy import Pool From 35befae6e07fc331c941f27001044dc4c5dae46c Mon Sep 17 00:00:00 2001 From: Shay Nehmad Date: Wed, 2 Oct 2019 14:24:01 +0300 Subject: [PATCH 4/6] Revert network_scanner fix --- monkey/infection_monkey/network/network_scanner.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/monkey/infection_monkey/network/network_scanner.py b/monkey/infection_monkey/network/network_scanner.py index f0145e843..9452a3fb8 100644 --- a/monkey/infection_monkey/network/network_scanner.py +++ b/monkey/infection_monkey/network/network_scanner.py @@ -6,7 +6,7 @@ from infection_monkey.config import WormConfiguration from infection_monkey.model.victim_host_generator import VictimHostGenerator from infection_monkey.network.info import local_ips, get_interfaces_ranges from infection_monkey.network import TcpScanner, PingScanner -from infection_monkey.utils.environment import is_windows_os +from infection_monkey.utils import is_windows_os if is_windows_os(): from multiprocessing.dummy import Pool From 61a81c2da47044c5e10b87c07a39dcceb479cd51 Mon Sep 17 00:00:00 2001 From: Shay Nehmad Date: Wed, 2 Oct 2019 16:31:31 +0300 Subject: [PATCH 5/6] Created the report generation sync module and now using it exclusivly to create reports. Almost all debug logs should probably be deleted once testing is done --- monkey/monkey_island/cc/resources/root.py | 25 ++------ .../cc/services/attack/attack_report.py | 4 +- .../cc/services/reporting/report.py | 17 ++--- .../report_generation_synchronisation.py | 63 +++++++++++++++++++ 4 files changed, 80 insertions(+), 29 deletions(-) create mode 100644 monkey/monkey_island/cc/services/reporting/report_generation_synchronisation.py diff --git a/monkey/monkey_island/cc/resources/root.py b/monkey/monkey_island/cc/resources/root.py index f2978f1ee..d7cae8bd7 100644 --- a/monkey/monkey_island/cc/resources/root.py +++ b/monkey/monkey_island/cc/resources/root.py @@ -10,6 +10,7 @@ from monkey_island.cc.database import mongo from monkey_island.cc.services.node import NodeService from monkey_island.cc.services.reporting.report import ReportService from monkey_island.cc.services.attack.attack_report import AttackReportService +from monkey_island.cc.services.reporting.report_generation_synchronisation import is_report_being_generated, safe_generate_reports from monkey_island.cc.utils import local_ip_addresses from monkey_island.cc.services.database import Database @@ -20,9 +21,6 @@ logger = logging.getLogger(__name__) class Root(flask_restful.Resource): def __init__(self): - # This lock will allow only one thread to generate a report at a time. Report generation can be quite - # slow if there is a lot of data, and the UI queries the Root service often; without the lock, these requests - # would accumulate, overload the server, eventually causing it to crash. self.report_generating_lock = threading.Event() def get(self, action=None): @@ -62,8 +60,10 @@ class Root(flask_restful.Resource): infection_done = NodeService.is_monkey_finished_running() if infection_done: - if self.should_generate_report(): - self.generate_report() + # Checking is_report_being_generated here, because we don't want to wait to generate a report; rather, + # we want to skip and reply. + if not is_report_being_generated() and not ReportService.is_latest_report_exists(): + safe_generate_reports() report_done = ReportService.is_report_generated() else: # Infection is not done report_done = False @@ -73,18 +73,3 @@ class Root(flask_restful.Resource): run_monkey=is_any_exists, infection_done=infection_done, report_done=report_done) - - def generate_report(self): - # Set the event when entering the critical section - self.report_generating_lock.set() - # Not using the return value, as the get_report function also saves the report in the DB for later. - _ = ReportService.get_report() - _ = AttackReportService.get_latest_report() - # Clear the event when leaving the critical section - self.report_generating_lock.clear() - - def should_generate_report(self): - # If the lock is not set, that means no one is generating a report right now. - is_any_thread_generating_a_report_right_now = not self.report_generating_lock.is_set() - is_there_a_need_for_a_new_report = not ReportService.is_latest_report_exists() - return is_any_thread_generating_a_report_right_now and is_there_a_need_for_a_new_report diff --git a/monkey/monkey_island/cc/services/attack/attack_report.py b/monkey/monkey_island/cc/services/attack/attack_report.py index c04e6870f..c7457c2f6 100644 --- a/monkey/monkey_island/cc/services/attack/attack_report.py +++ b/monkey/monkey_island/cc/services/attack/attack_report.py @@ -6,6 +6,7 @@ from monkey_island.cc.services.attack.technique_reports import T1145, T1105, T10 from monkey_island.cc.services.attack.technique_reports import T1090, T1041, T1222, T1005, T1018, T1016, T1021, T1064 from monkey_island.cc.services.attack.attack_config import AttackConfig from monkey_island.cc.database import mongo +from monkey_island.cc.services.reporting.report_generation_synchronisation import safe_generate_attack_report __author__ = "VakarisZ" @@ -88,7 +89,8 @@ class AttackReportService: report_modifytime = latest_report['meta']['latest_monkey_modifytime'] if monkey_modifytime and report_modifytime and monkey_modifytime == report_modifytime: return latest_report - return AttackReportService.generate_new_report() + + return safe_generate_attack_report() @staticmethod def is_report_generated(): diff --git a/monkey/monkey_island/cc/services/reporting/report.py b/monkey/monkey_island/cc/services/reporting/report.py index f00fbc22c..1221200c4 100644 --- a/monkey/monkey_island/cc/services/reporting/report.py +++ b/monkey/monkey_island/cc/services/reporting/report.py @@ -1,25 +1,25 @@ -import itertools import functools +import itertools +import logging +import time import ipaddress -import logging - from bson import json_util from enum import Enum - from six import text_type +from common.network.network_range import NetworkRange from common.network.segmentation_utils import get_ip_in_src_and_not_in_dst from monkey_island.cc.database import mongo from monkey_island.cc.models import Monkey -from monkey_island.cc.services.reporting.report_exporter_manager import ReportExporterManager from monkey_island.cc.services.config import ConfigService from monkey_island.cc.services.configuration.utils import get_config_network_segments_as_subnet_groups from monkey_island.cc.services.edge import EdgeService from monkey_island.cc.services.node import NodeService -from monkey_island.cc.utils import local_ip_addresses, get_subnets from monkey_island.cc.services.reporting.pth_report import PTHReportService -from common.network.network_range import NetworkRange +from monkey_island.cc.services.reporting.report_exporter_manager import ReportExporterManager +from monkey_island.cc.services.reporting.report_generation_synchronisation import safe_generate_regular_report +from monkey_island.cc.utils import local_ip_addresses, get_subnets __author__ = "itay.mizeretz" @@ -692,6 +692,7 @@ class ReportService: @staticmethod def generate_report(): + time.sleep(40) domain_issues = ReportService.get_domain_issues() issues = ReportService.get_issues() config_users = ReportService.get_config_users() @@ -780,7 +781,7 @@ class ReportService: def get_report(): if ReportService.is_latest_report_exists(): return ReportService.decode_dot_char_before_mongo_insert(mongo.db.report.find_one()) - return ReportService.generate_report() + return safe_generate_regular_report() @staticmethod def did_exploit_type_succeed(exploit_type): diff --git a/monkey/monkey_island/cc/services/reporting/report_generation_synchronisation.py b/monkey/monkey_island/cc/services/reporting/report_generation_synchronisation.py new file mode 100644 index 000000000..1fe4d8bb8 --- /dev/null +++ b/monkey/monkey_island/cc/services/reporting/report_generation_synchronisation.py @@ -0,0 +1,63 @@ +import logging +import threading + +logger = logging.getLogger(__name__) + +# These are pseudo-singletons - global Locks. These locks will allow only one thread to generate a report at a time. +# Report generation can be quite slow if there is a lot of data, and the UI queries the Root service often; without +# the locks, these requests would accumulate, overload the server, eventually causing it to crash. +logger.debug("Initializing report generation lock.") +report_generating_lock = threading.Semaphore() +__attack_report_generating_lock = threading.Semaphore() +__regular_report_generating_lock = threading.Semaphore() + + +def safe_generate_reports(): + # Wait until report generation is available. + logger.debug("Waiting for report generation...") + # Entering the critical section. + report_generating_lock.acquire() + logger.debug("Report generation locked.") + report = safe_generate_regular_report() + attack_report = safe_generate_attack_report() + # Leaving the critical section. + report_generating_lock.release() + logger.debug("Report generation released.") + return report, attack_report + + +def safe_generate_regular_report(): + # Local import to avoid circular imports + from monkey_island.cc.services.reporting.report import ReportService + logger.debug("Waiting for regular report generation...") + __regular_report_generating_lock.acquire() + logger.debug("Regular report generation locked.") + report = ReportService.generate_report() + __regular_report_generating_lock.release() + logger.debug("Regular report generation released.") + return report + + +def safe_generate_attack_report(): + # Local import to avoid circular imports + from monkey_island.cc.services.attack.attack_report import AttackReportService + logger.debug("Waiting for attack report generation...") + __attack_report_generating_lock.acquire() + logger.debug("Attack report generation locked.") + attack_report = AttackReportService.generate_new_report() + __attack_report_generating_lock.release() + logger.debug("Attack report generation released.") + return attack_report + + +def is_report_being_generated(): + # From https://docs.python.org/2/library/threading.html#threading.Semaphore.acquire: + # When invoked with blocking set to false, do not block. + # If a call without an argument would block, return false immediately; + # otherwise, do the same thing as when called without arguments, and return true. + is_report_being_generated_right_now = not report_generating_lock.acquire(blocking=False) + logger.debug("is_report_being_generated_right_now == " + str(is_report_being_generated_right_now)) + if not is_report_being_generated_right_now: + # We're not using the critical resource; we just checked its state. + report_generating_lock.release() + return is_report_being_generated_right_now From b14fd4687c3cdb73cdb49bfca86dfdb18d3b0831 Mon Sep 17 00:00:00 2001 From: Shay Nehmad Date: Wed, 2 Oct 2019 16:39:59 +0300 Subject: [PATCH 6/6] Removed debug logs and made all locks private to the module --- .../cc/services/reporting/report.py | 2 -- .../report_generation_synchronisation.py | 25 ++++++------------- 2 files changed, 7 insertions(+), 20 deletions(-) diff --git a/monkey/monkey_island/cc/services/reporting/report.py b/monkey/monkey_island/cc/services/reporting/report.py index 1221200c4..03d0f5011 100644 --- a/monkey/monkey_island/cc/services/reporting/report.py +++ b/monkey/monkey_island/cc/services/reporting/report.py @@ -1,7 +1,6 @@ import functools import itertools import logging -import time import ipaddress from bson import json_util @@ -692,7 +691,6 @@ class ReportService: @staticmethod def generate_report(): - time.sleep(40) domain_issues = ReportService.get_domain_issues() issues = ReportService.get_issues() config_users = ReportService.get_config_users() diff --git a/monkey/monkey_island/cc/services/reporting/report_generation_synchronisation.py b/monkey/monkey_island/cc/services/reporting/report_generation_synchronisation.py index 1fe4d8bb8..9025ff68f 100644 --- a/monkey/monkey_island/cc/services/reporting/report_generation_synchronisation.py +++ b/monkey/monkey_island/cc/services/reporting/report_generation_synchronisation.py @@ -6,47 +6,37 @@ logger = logging.getLogger(__name__) # These are pseudo-singletons - global Locks. These locks will allow only one thread to generate a report at a time. # Report generation can be quite slow if there is a lot of data, and the UI queries the Root service often; without # the locks, these requests would accumulate, overload the server, eventually causing it to crash. -logger.debug("Initializing report generation lock.") -report_generating_lock = threading.Semaphore() +logger.debug("Initializing report generation locks.") +__report_generating_lock = threading.Semaphore() __attack_report_generating_lock = threading.Semaphore() __regular_report_generating_lock = threading.Semaphore() def safe_generate_reports(): - # Wait until report generation is available. - logger.debug("Waiting for report generation...") - # Entering the critical section. - report_generating_lock.acquire() - logger.debug("Report generation locked.") + # Entering the critical section; Wait until report generation is available. + __report_generating_lock.acquire() report = safe_generate_regular_report() attack_report = safe_generate_attack_report() # Leaving the critical section. - report_generating_lock.release() - logger.debug("Report generation released.") + __report_generating_lock.release() return report, attack_report def safe_generate_regular_report(): # Local import to avoid circular imports from monkey_island.cc.services.reporting.report import ReportService - logger.debug("Waiting for regular report generation...") __regular_report_generating_lock.acquire() - logger.debug("Regular report generation locked.") report = ReportService.generate_report() __regular_report_generating_lock.release() - logger.debug("Regular report generation released.") return report def safe_generate_attack_report(): # Local import to avoid circular imports from monkey_island.cc.services.attack.attack_report import AttackReportService - logger.debug("Waiting for attack report generation...") __attack_report_generating_lock.acquire() - logger.debug("Attack report generation locked.") attack_report = AttackReportService.generate_new_report() __attack_report_generating_lock.release() - logger.debug("Attack report generation released.") return attack_report @@ -55,9 +45,8 @@ def is_report_being_generated(): # When invoked with blocking set to false, do not block. # If a call without an argument would block, return false immediately; # otherwise, do the same thing as when called without arguments, and return true. - is_report_being_generated_right_now = not report_generating_lock.acquire(blocking=False) - logger.debug("is_report_being_generated_right_now == " + str(is_report_being_generated_right_now)) + is_report_being_generated_right_now = not __report_generating_lock.acquire(blocking=False) if not is_report_being_generated_right_now: # We're not using the critical resource; we just checked its state. - report_generating_lock.release() + __report_generating_lock.release() return is_report_being_generated_right_now