conpot
conpot copied to clipboard
mushorg
Optional protocols
In order to make conpot more accessible, we should make all protocols optional. for example we should have a "enabled" option in the configuration. On conpot start-up we should also check if we can import the protocol libraries and skip the protocol if we fail.
I've added a few lines that should accomplish the first feature i.e making the protocols optional: https://github.com/Sp3ctr3/conpot/blob/master/bin/conpot. Is that what you intended? Modified config file: https://github.com/Sp3ctr3/conpot/blob/master/conpot/conpot.cfg It seems to be working well in my machine with the exception of enabling only the S7 server. In that case conpot crashes right after dropping privileges. Other configurations I tested seem to work.
Hey, could you send this as a pull request? Makes it much easier for me to have a look at it an compare with our code base. If you want you can also open an issue for S7 so we can investigate it.
Sure! It's done. And yeah I will open an issue after I look around some more. I think it's because other server's have to be stopped while S7 doesn't but I'm not sure.
Should I add code to check if the protocol dependencies exist? I thought installing conpot would automatically install the dependencies too.
Yes, everything gets installed: https://github.com/glastopf/conpot/blob/master/setup.py#L9 This line parses the requirements.txt and adds them as installation step to the setup.py
In my opinion if we want to do it properly you have to pass a parameter to setup.py through pip which disables unwanted protocols: --install-option <options> http://www.pip-installer.org/en/latest/usage.html#pip-install
From what I could dig up, passing parameters to setup.py doesn't sound possible. https://mail.python.org/pipermail/distutils-sig/2010-May/016147.html Any other options? Maybe reading from a setup configuration file?
From what I read in the associated commits, we have to introduce some sort of internal dependency check. Example: The HTTP interface MAY depend on SNMP (since it is able to display snmp-related values). Therefore, if SNMP is disabled, we should take care of the missing data source inside the HTTP module.
I'll look into the issue..
Not quite sure if thats the right way. I assume if you don't run an SNMP service, your HMI will look corresponding. I wouldn't go so far and mock something in the HTTP module but just let the HMI fail to connect.
@glaslos I was thinking about some sort of message on startup that notifies the user if he added "service crosstalk" (yay, let's invent tech terms) between two services and later disables one of them. Would help novice users with debugging their templates.
Hm, in my opinion this would be a bit hackish (although we sometimes like hackish).
Maybe we should make the whole protocol part a bit more modular. Here is how we load plug-ins in Glastopf: https://github.com/glastopf/glastopf/blob/master/glastopf/modules/handlers/request_handler.py#L31
Maybe we can get something similar in Conpot. So don't import everything all the way from conpot to the plugin to the protocol implementation but check to config which protocols are enabled and import only those.
Feel free to send in a pull request with your solution and we can discuss it but I'd prefer to avoid try/except imports.
I fully agree with the idea of providing some sort of plug-in mechanism. This would be a big advantage in terms of modularity..
How does this look? https://github.com/Sp3ctr3/conpot/blob/master/bin/conpot. I didn't sent it as a pull request because the build was failing.
I'm not too convinces with imports scattered over the code: https://github.com/glastopf/conpot/compare/master...Sp3ctr3:master
Opinions?
atm I don't see how this could be enhanced .. everything I come up with ends with imports being scattered one or the other way. But I fully understand your concerns. If this sets a precedent, it could turn against us in terms of debugging in future..
I thought about the problem with not having imports at the top too. But in this case the only other option would involve reading from the config file at the top of the file. Would that be preferable?