Refactor greenlet handling and fix related bugs

[srenfo]

This PR refactors all greenlet handling, especially in test code, and fixes all bugs that were exposed in the process.

I'm sorry that this is such a big PR but it's next to impossible to chop it into independent pieces. It may be best to view this commit by commit or as a side-by-side diff rather than a unified diff.

Highlights:

• Refactor the debugging __main__s in the protocol servers into a single script in bin/start_protocol.py. Those mains would otherwise have been a major pain to keep up to date with these changes. The script should be able to start all protocol individually. It keeps the previous port choices where there were any, though that may be more confusing than necessary.
• Refactor starting and stopping of servers and greenlets in protocol tests. This also includes databus init, paths to XML files, etc.
• The test suite is much faster (by almost exactly 50% on my machine). Execution time of the TFTP tests alone went down from 12228ms to 119ms on my machine. That is not a typo. (I'll have to eat those words if it turns out the new code is broken.)
• bin/conpot now waits for greenlets to shut down.
• The startup delay of five seconds is gone.

There are new tests for the refactored utility code.

Bug fixes:

• conpot_protocol tried to override some class properties but failed
• Modbus and SNMP servers were never properly shut down
• Modbus and IEC104 would not block in their start() method, contrary to all other protocols

Tests for these bugs are included. (All those tests would fail on the current master.)

Caveats:

• There's a single TODO in the PR in greenlets.py. The assumption that the protocol's start() is fairly simple does not hold true for the HTTP server, so it is special cased there. That said, the new tests would/should catch such cases (they did for HTTP). It's hard to be certain.
• Testing of the changes consisted of running the test suite and checking that bin/conpot still started up with the default template. No other functional testing was done. That's because the changes are focussed on test code.

[srenfo]

Let me know if you want me to remove parts of this or split it up in a certain way.

[xandfury]

Thanks for your contribution :slightly_smiling_face: Would need some time to review this. I hope that's okay.

[srenfo]

If this proves too unwieldy I can split it into multiple PRs that build on each other.

[xandfury]

Good job @srenfo :tada: I'll be happy to merge your changes. I can live without logger comments. But I strongly believe that we should keep the __main__ sections where ever they were added.

[srenfo]

Sorry, I'm rather busy at work right now. It may take another week or so but I'll come back to this. Ping me otherwise.

[srenfo]

I appreciate the kind words @glaslos and @xandfury, thank you!

How did you end up working on Conpot?

Primarily, it's convenient if you want to quickly spin up a server of an ICS protocol. For example, we're currently doing research work on/with IEC104 and sometimes you just want a random IEC104 server to talk to. :smile: For a previous research project we actually had it deployed as a honeypot, though not much came from that unfortunately.

So after using it for some time it's only appropriate to give back here and there.

We have contributed a few small PRs before, we just haven't tagged ourselves as a group.

[glaslos]

@xandfury any remaining concerns?

[xandfury]

Hey @srenfo. The changes look good to me. Can you fix the conflicts so that we can merge?

[glaslos]

@srenfo you probably want to run black over your branch before merging. Should be easier than fixing the merge conflicts.

[srenfo]

@srenfo you probably want to run black over your branch before merging. Should be easier than fixing the merge conflicts.

Are you sure this is a good idea with master currently failing? (I haven't looked into why it's failing exactly.) I can hold off until that's fixed.

Alternatively you could revert the broken commits (or actually remove them from master with a force push), reopen https://github.com/mushorg/conpot/pull/501 and do the fixes on the branch there. That would require eventually rebasing that branch of course if you wanted to merge this branch here in the meantime.

[glaslos]

@srenfo if you give me permissions to change the PR I can help with resolving the conflicts

[srenfo]

@srenfo if you give me permissions to change the PR I can help with resolving the conflicts

The conflicts should be resolved.

There's a chance Travis will complain since master has changed since this was first opened. Fingers crossed. :crossed_fingers:

[glaslos]

hm, maybe merge master to get the git action to run?

[srenfo]

Trying again... it's rebased on the current master, so a merge does nothing.

What's weird is that I've used this workflow before and it had always triggered the build (AFAICT).

I've also (re-)enabled "Allow edits and access to secrets by maintainers", though I believe it was the same before. Do you need me to do anything else?

[srenfo]

I've even added an empty commit to try to force the build to trigger, but apparently it's just slow.

As of this writing, the CI build is queued: https://travis-ci.org/github/mushorg/conpot/pull_requests

You can see that the black check succeeded in my fork (i. e. the source of this PR): https://github.com/srenfo/conpot/runs/1343712255

[coveralls]

Pull Request Test Coverage Report for Build 1236

• 71 of 72 (98.61%) changed or added relevant lines in 10 files are covered.
• 85 unchanged lines in 8 files lost coverage.
• Overall coverage increased (+0.3%) to 73.619%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
conpot/utils/greenlet.py	40	41	97.56%
<!--	Total:	71	72	98.61%	-->

Files with Coverage Reduction	New Missed Lines	%
conpot/core/protocol_wrapper.py	1	95.45%
conpot/protocols/s7comm/s7_server.py	2	64.66%
conpot/protocols/http/command_responder.py	3	60.82%
conpot/protocols/IEC104/IEC104_server.py	5	86.02%
conpot/protocols/kamstrup/management_protocol/kamstrup_management_server.py	8	83.64%
conpot/protocols/enip/enip_server.py	20	65.14%
conpot/protocols/ftp/ftp_base_handler.py	20	83.08%
conpot/protocols/ftp/ftp_handler.py	26	81.23%
<!--	Total:	85		-->

Totals
Change from base Build 1233:	0.3%
Covered Lines:	5545
Relevant Lines:	7532

---

💛 - Coveralls

[glaslos]

Maybe the new pricing policies are having an effect? https://blog.travis-ci.com/2020-11-02-travis-ci-new-billing

[glaslos]

Alright, let's get this merged. Travis seems a bit confused due to the force-push.