TLS support (also in CLI utils)

[calestyo]

Hey.

First of all,... reaaaally nice project, thanks a lot for your efforts :-)

Second, from https://fritzconnection.readthedocs.io/en/1.14.0/sources/getting_started.html#tls-encryption:

Since the router uses a self-signed certificate, currently certificate-verification is disabled.

1. Would it be possible to add support for proper verification, i.e., if enabled, using the system certificate store per default, but also allowing to specify a CA-cert file/path manually?
   
   Yes, most people will use it via WiFi, but WPA (including WPA3) isn't really the most secure thing in the world.
   And yes, certs are per default self-signed, but one can upload proper ones (like from a self signed CA) or one could simply directly use that self signed cert and store that somehow locally and use it for verification.

2. And also to export that via the CLI tool?
   
   Like via adding a --tls-verify option (or better, a --no-tls-verify-option and let it have the verification done per default)?
   
   And by some --tls-ca-pathname option or so (which if not given, defaults to system certs)?

3. Perhaps consider whether TLS should be default and people have to explicitly disable it and/or use some no_verify option to use TLS but don't check the cert? Again, simply for security reasons.

Thanks,
Chris.

[kbr]

The library is used by a lot of home automation installations with users probably not knowing anything about TLS (and may be other users too). So I don't want to introduce changes, that require actions. But making encryption default or adding an option for other certificates with the corresponding verification could be an idea.

[calestyo]

But making encryption default

You mean in the CLI tools then?

[calestyo]

What just came to my mind and what one could think about (though perhaps this should then go better into it's own issue) is additionally to proper verification, offering a TOFU (trust on first use) mode.

Like the first time the lib connects to a given fritzbox (not sure how to identify them, perhaps simply by IP and/or BSSID or something like that?), it would fetch the cert it uses (which would then of course not really be authenticated/trustworthy) store it and henceforth expect that certificate (unless it expires; there also taking the expiry times from that very first fetch).

That would be at least somewhat more secure than not doing TLS at all, respectively just trusting every time the current certificate.

[calestyo]

Just wondered whether that's somewhere one the horizon? :-)

I haven't really looked into the code, so not sure how easy it would be to add TLS support... it seems you're using requests, in which case I'd guess it should be rather easy (at least on the library part)... one probably just need to allow the (API) user to specify the various TLS settings and your code needs to pass it through to requests.

[calestyo]

Looking at the current code’s imports, I think the following might be candidates for requiring changes with respect to TLS:

• requests
  

  fritzconnection/fritzconnection/core/fritzconnection.py

  Lines 21 to 22 in 896894c

  	import requests
  	from requests.auth import HTTPDigestAuth

  fritzconnection/fritzconnection/core/soaper.py

  Lines 15 to 16 in 896894c

  	import requests
  	from requests.auth import HTTPDigestAuth

  fritzconnection/fritzconnection/core/utils.py

  Line 9 in 896894c

  import requests

• urllib3
  

  fritzconnection/fritzconnection/core/fritzconnection.py

  Line 41 in 896894c

  import urllib3

  Only urllib3.disable_warnings() seems to be ever used from that, so I guess no changes are needed here.

• http
  

  fritzconnection/fritzconnection/core/fritzhttp.py

  Line 13 in 896894c

  from http import HTTPStatus

  Just the HTTP status codes... no changes should be needed here for TLS:

• http.client
  

  fritzconnection/fritzconnection/core/fritzhttp.py

  Line 14 in 896894c

  from http.client import HTTP_PORT

Only HTTP_PORT is ever used. Perhaps we'd need something that uses HTTPS_PORT in the right cases, but I guess even if, it would not compromise security otherwise.

So I'd guess it that all actual communication with a Fritz!Box is done using requests, right? And thus is should be enough, that wherever a requests session is made respectively a non-session request is made, we set the necessary TLS parameters as described upstream.

From a simple grep I'd say that requests is used in three files only:

• fritzconnection/core/fritzconnection.py
• fritzconnection/core/soaper.py
• fritzconnection/core/utils.py
  but there might be shenanigans where you use the objects from these files in others or so.

If not, it would be only these places:
in FritzConnection.__init__():

• fritzconnection/fritzconnection/core/fritzconnection.py

  Lines 273 to 274 in 896894c

  	session = requests.Session()
  	session.verify = False

  => verify would need to be made configurable via the API.

• fritzconnection/fritzconnection/core/fritzconnection.py

  Lines 277 to 278 in 896894c

  	adapter = requests.adapters.HTTPAdapter(
  	pool_connections=pool_connections, pool_maxsize=pool_maxsize)

  AFAIU, this one would imply use the TLS from session, if configured there. So nothing needed here.

in handle_response():

fritzconnection/fritzconnection/core/soaper.py

Lines 310 to 316 in 896894c

	response = requests.post(
	url,
	data=envelope,
	headers=headers,
	auth=auth,
	timeout=self.timeout,
	verify=False,

=> Same here, set verify via the API.

in do_request():

fritzconnection/fritzconnection/core/utils.py

Line 52 in 896894c

response = requests.get(url, timeout=timeout, verify=False)

=> Here, too.

You already have use_tls, which I think controls whether plain HTTP is used or (currently unverified) HTTPS, right?

I'd leave that as is and allow the use to set the value of verify, which - in a first step - could still default to False - but a user could then either give True or a specific pathname to CAs (as described here).

The only downside of that is, that at some point might want to have a 4 method (the TOFU one I've described above)... so for that we'd then either need to use None or some other special type.