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.

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

fix - kill network capture process if any container is killed #361

Merged
merged 2 commits into from
Sep 17, 2022

Conversation

spiderxm
Copy link
Collaborator

@spiderxm spiderxm commented Aug 2, 2022

Description

Fix network thread issue

Issue is network thread is not terminating after an error
to reproduce the issue, run a module, and then kill the container with docker kill container_name. OHP is expected to exit from working and throw an error, but the network thread stays open and it's not terminating.

Steps to Reproduce

# run the command below
$ sudo python3 ohp.py -m ftp/weak_password,ftp/strong_password --store-pcap
# wait until network capture process starts
# on new terminal run the command below
$ docker kill ohp_ftpserver_strong_password
# on the terminal where you started the capture process it will not end but it should.

Related Issue

#359

Screenshots (if appropriate):

Screenshot 2022-08-02 at 10 18 45 AM

After killing the container the network capture process terminates successfully.

Screenshot 2022-08-02 at 10 18 52 AM

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Other

Checklist

  • I have followed the Contributor Guidelines.
  • The code has been thoroughly tested in my local development environment with flake8 and pylint.
  • The code is Python 3 compatible.
  • The code follows the PEP8 styling guidelines with 4 spaces indentation.
  • This Pull Request relates to only one issue or only one feature
  • I have referenced the corresponding issue number in my commit message
  • I have added the relevant documentation.
  • My branch is up-to-date with the Upstream master branch.

@spiderxm spiderxm requested a review from dhirensr August 2, 2022 04:54
core/load.py Outdated
@@ -362,13 +362,16 @@ def wait_until_interrupt(virtual_machine_container_reset_factory_time_seconds, c
# start containers based on selected modules
start_containers(configuration)
if not new_network_events_thread.is_alive():
return error(messages["interrupt_application"])
error(messages["interrupt_application"])
return True
Copy link
Collaborator

Choose a reason for hiding this comment

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

return in a while loop?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Return error("")
Is returning None

So we need to return True in failing cases

We can break the while loop and at the end return True
What say @dhirensr ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

we can't use break there. sounds like a use case for break?

core/load.py Outdated
@@ -884,6 +887,9 @@ def load_honeypot_engine():
)
# killed the network traffic capture process by ctrl + c... waiting to end.
info(messages["killing_capture_process"])
if exit_flag:
# Terminate the network capture process
network_traffic_capture_process.terminate()
Copy link
Collaborator

Choose a reason for hiding this comment

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

this should do the job right without the above changes?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

nopes, we need a exit flag value which denotes this has exited

@spiderxm
Copy link
Collaborator Author

@dhirensr check now

@spiderxm spiderxm mentioned this pull request Sep 13, 2022
@spiderxm spiderxm merged commit 4bf399f into OWASP:development Sep 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants