mushorg / conpot

ICS/SCADA honeypot
GNU General Public License v2.0
1.22k stars 413 forks source link

Added support for plc-stop signal. #447

Closed rohangireeshnair closed 5 years ago

rohangireeshnair commented 5 years ago

Added the support for plc stop signal in conpot.protocols.s7comm.s7 and conpot.protocols.s7comm.s7_server to support plc stop signal (0x29).

xandfury commented 5 years ago

@rohangireeshnair can you add a supporting test for this as well? :slightly_smiling_face:

coveralls commented 5 years ago

Pull Request Test Coverage Report for Build 1143


Changes Missing Coverage Covered Lines Changed/Added Lines %
conpot/protocols/s7comm/s7_server.py 0 1 0.0%
<!-- Total: 8 9 88.89% -->
Files with Coverage Reduction New Missed Lines %
conpot/protocols/ipmi/ipmi_server.py 1 68.8%
conpot/protocols/http/command_responder.py 4 62.14%
conpot/protocols/ftp/ftp_handler.py 13 82.58%
conpot/protocols/ftp/ftp_base_handler.py 17 81.04%
<!-- Total: 35 -->
Totals Coverage Status
Change from base Build 1134: 0.08%
Covered Lines: 5445
Relevant Lines: 7573

💛 - Coveralls
rohangireeshnair commented 5 years ago

@xandfury I used snap7 python library to send a plc-stop signal to the conpot which it logged successfully. Previously it was throwing NoneType Iteration Exception.

rohangireeshnair commented 5 years ago

@xandfury Here's a python script to test it. Check the requirements commented on the script before running it

#First install python-snap7 using pip install python-snap7
#Also Before running the script install SNAP7 library which is required by python-snap7. Refer https://python-snap7.readthedocs.io/en/latest/installation.html#snap7
#Run the script with first argument as conpot ip and second the port on which s7comm is running
#Example - python3 script.py 127.0.0.1 10201

import snap7
import sys

if __name__ == '__main__':
    server = sys.argv[1]
    port = sys.argv[2]
    plc = snap7.client.Client()
    plc.connect(server,0,0, int(port))
    plc.plc_stop()
creolis commented 5 years ago

@xandfury Here's a python script to test it. Check the requirements commented on the script before running it

I think you got him wrong :) We're not going to do any tests. This is the job of a friend called Travis.

Everytime you push your code, travis will run your tests and tells us if your code is fine. So you have to make sure, Travis feels happy ;)

rohangireeshnair commented 5 years ago

@creolis Oh :) I see that the travis check had passed, also saw his comment asking me to add supporting test.🙂

creolis commented 5 years ago

Hm .. let me try again :) Travis CI tests your code not in terms of code quality, but for proper operation. But how the heck does Travis know what is proper behaviour and what is broken?

Introducing: Unit tests!

The Travis check has passed because Travis does not know how to test your changes yet. The heck, it doesn't even know that you actually added a feature. Therefore, you need to tell Travis, that there's a new kid in town and that he's gotta keep an eye in him.

Take a look here: https://github.com/mushorg/conpot/tree/master/conpot/tests

Most stuff in here has been written or massaged by @xandfury - he's the caretaker, which makes the directory a graveyard where unit tests are going when they want to die :)

What @xandfury asked you was to extend the existing test suite (https://github.com/mushorg/conpot/blob/master/conpot/tests/test_s7_server.py?) in order to also support the STOP signal. I don't think that introducing a whole new funky library like snap7 is needed to achieve that, as you can start extending https://github.com/mushorg/conpot/blob/master/conpot/tests/helpers/s7comm_client.py to suite your needs :)

Hope that helps,

Daniel

rohangireeshnair commented 5 years ago

@creolis Oh. Sure I can do that 😄.And Thanks for the info, I’ll get it done soon.

rohangireeshnair commented 5 years ago

@creolis @xandfury Sorry for the delay guys. I've added the tests. :)

xandfury commented 5 years ago

Thanks for the PR :+1: