kbr / fritzconnection

Python-Tool to communicate with the AVM Fritz!Box by the TR-064 protocol and the AHA-HTTP-Interface
MIT License
304 stars 59 forks source link

added TLS support using a shared requests session; #25

Closed andreas-seiderer closed 4 years ago

andreas-seiderer commented 4 years ago

Hello,

Thank you very much for sharing this Python module which makes the communication with FritzBox devices much easier.

I added TLS support to the module to prevent passwords being sent without encryption through the network which might not always be safe. Since the FritzBox currently creates just self-signed certificates I decided to use certificate pinning where you first download the certificate from the FritzBox (e.g. by using the browser) and check against this certificate to prevent man-in-the-middle attacks. Nevertheless, it is also possible to disable certificate verification at all while still using TLS. Usually the FritzBox doesn't create a certificate for its IP address in the LAN so hostname verification is disabled (compare https://github.com/urllib3/urllib3/issues/517). A requests session of a FritzConnection instance is shared to speed up the execution time especially of the TLS connections. The other classes are adapted to be able to pass the parameters.

I tried to do the changes in a way that the current function calls have not to be changed. I also didn't change the CLI tools, which should still be working. Nevertheless, some tests still have to be done but at least in my tests with my FritzBox it still worked like before.

TLS with certificate verification (certificate can be downloaded e.g. with Firefox by accessing the TLS encrypted web interface of the FritzBox)

fc = FritzConnection(address='192.168.178.1', password='PASSWORD', protocol='https', certificate="certificate.pem")

TLS without certificate verification (will cause warnings)

fc = FritzConnection(address='192.168.178.1', password='PASSWORD', protocol='https', certificate=False)

regular behavior

fc = FritzConnection(address='192.168.178.1', password='PASSWORD')

Best regards,

Andreas

kbr commented 4 years ago

Thank you for the pull request. Unfortunately it's a very large one with a lot of changes to a lot of files. Too much than I would like to apply in a single merge. However TLS communication has been at the todo-list for a while and by introducing TLS I would like to have an api like:

fc = FritzConnection(address='192.168.178.1', password='xx', use_tls=True)

because I find it hard for the end-user to manually fetch certificates, storing them somewhere and managing the paths as parameters for verification.

So I took up your idea of TLS use without verification for the next release (1.2). Some commits are already in the master branch (therefore the reported conflicts). Reviews and comments are always welcome.

andreas-seiderer commented 4 years ago

Thank you for your response.

I mainly wanted to share the code with you since I already switched it to TLS. I currently use it regularly as a replacement for the (blocking) Perl implementation in FHEM (which already used TLS). I hope it can still be useful for you.

Several adaptions were caused since it was necessary to pass the certificate through all functions to do the certificate pinning. If set to "False" this would not be required but the many (understandable) security warnings of the requests module should be caught. Another option would be to automatically download the certificate at the first call from the FritzBox device and store it. This would provide some extra security since adapted certificates e.g. caused by man-in-the-middle attacks could be detected in future calls. Nevertheless, such features are probably just required if the FritzBox interface is accessed directly over the internet. Perhaps the certificate pinning could still be included as an option, like with my adaptations.

Since I already tested the performance I would recommend that you also share a requests session for at least the TLS connections to speed them (massively) up.

So I'm looking forward to your 1.2 release and close this pull request :) .