Skip to content
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.

Sign up for GitHub

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

Schedule jobs bugfix #861

Merged
merged 3 commits into from
Dec 8, 2020
Merged

Schedule jobs bugfix #861

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1cbcb69
Fixed schedule_jobs bug, where scheduled job is never deleted and mon…
VakarisZ Oct 12, 2020
7abafb7
Fixed bad exception handling in version_update.py
VakarisZ Oct 9, 2020
a777431
Refactored exception name to a more specific one
VakarisZ Oct 15, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions monkey/common/utils/exceptions.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,3 +20,7 @@ class CredentialsNotRequiredError(RegistrationNotNeededError):

class AlreadyRegisteredError(RegistrationNotNeededError):
""" Raise to indicate the reason why registration is not required """


class NoInternetError(Exception):
""" Raise to indicate problems caused when no internet connection is present"""
2 changes: 1 addition & 1 deletion monkey/infection_monkey/control.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ def find_server(default_tunnel=None):
if ControlClient.proxies:
debug_message += " through proxies: %s" % ControlClient.proxies
LOG.debug(debug_message)
requests.get("https://%s/api?action=is-up" % (server,), # noqa: DUO123
requests.get(f"https://{server}/api?action=is-up", # noqa: DUO123
verify=False,
proxies=ControlClient.proxies,
timeout=TIMEOUT_IN_SECONDS)
Expand Down
4 changes: 3 additions & 1 deletion monkey/infection_monkey/post_breach/actions/schedule_jobs.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,5 +15,7 @@ def __init__(self):
super(ScheduleJobs, self).__init__(name=POST_BREACH_JOB_SCHEDULING,
linux_cmd=' '.join(linux_cmds),
windows_cmd=windows_cmds)


def run(self):
super(ScheduleJobs, self).run()
remove_scheduled_jobs()
2 changes: 1 addition & 1 deletion monkey/infection_monkey/post_breach/job_scheduling/windows_job_scheduling.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@


def get_windows_commands_to_schedule_jobs():
return f'schtasks /Create /SC monthly /TN {SCHEDULED_TASK_NAME} /TR {SCHEDULED_TASK_COMMAND}'
return f'schtasks /Create /SC monthly /F /TN {SCHEDULED_TASK_NAME} /TR {SCHEDULED_TASK_COMMAND}'


def get_windows_commands_to_remove_scheduled_jobs():
Expand Down
7 changes: 4 additions & 3 deletions monkey/infection_monkey/post_breach/post_breach_handler.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,9 @@ def execute_all_configured(self):
"""
Executes all post breach actions.
"""
pool = Pool(5)
pool.map(self.run_pba, self.pba_list)
LOG.info("All PBAs executed. Total {} executed.".format(len(self.pba_list)))
with Pool(5) as pool:
pool.map(self.run_pba, self.pba_list)
LOG.info("All PBAs executed. Total {} executed.".format(len(self.pba_list)))

@staticmethod
def config_to_pba_list() -> Sequence[PBA]:
Expand All @@ -40,5 +40,6 @@ def run_pba(self, pba):
try:
LOG.debug("Executing PBA: '{}'".format(pba.name))
pba.run()
LOG.debug(f"Execution of {pba.name} finished")
except Exception as e:
LOG.error("PBA {} failed. Error info: {}".format(pba.name, e))
11 changes: 8 additions & 3 deletions monkey/monkey_island/cc/services/version_update.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import requests

import monkey_island.cc.environment.environment_singleton as env_singleton
from common.utils.exceptions import NoInternetError
from common.version import get_version

__author__ = "itay.mizeretz"
Expand All @@ -29,8 +30,8 @@ def get_newer_version():
if VersionUpdateService.newer_version is None:
try:
VersionUpdateService.newer_version = VersionUpdateService._check_new_version()
except Exception:
logger.exception('Failed updating version number')
except NoInternetError:
logger.info('Failed updating version number')

return VersionUpdateService.newer_version

Expand All @@ -42,7 +43,11 @@ def _check_new_version():
"""
url = VersionUpdateService.VERSION_SERVER_CHECK_NEW_URL % (env_singleton.env.get_deployment(), get_version())

reply = requests.get(url, timeout=15)
try:
reply = requests.get(url, timeout=7)
except requests.exceptions.RequestException:
logger.info("Can't get latest monkey version, probably no connection to the internet.")
raise NoInternetError
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This might result from lack of internet connection, but also if our update server fails to respond for some reason.
I would change the exception to something like UpdateServerUnreachableException

I think the term "update server" better describes the server than "version server", but it's more important to stay consistent. So either change the rest of the variable names here to "update server" or call the exception VersionServerUnreachableException


res = reply.json().get('newer_version', None)

Expand Down