wasilukm / hoymiles-mqtt

Send data from Hoymiles PV plant to Home Assistant without cloud
MIT License
46 stars 9 forks source link

Support for MQTT TLS connection #23

Closed majherek closed 7 months ago

majherek commented 7 months ago

Two parameters have been added to enable an encrypted connection to the MQTT server.

We can enable TLS support via --mqtt-tls.

In case the certificate is signed by an unknown CA, we can enable insecure TLS connection via --mqtt-tls-insecure.

wasilukm commented 7 months ago

Thank you for the pull request.

If there is no option for the end user to specify paths to certificates and keys does it make sense to use --mqtt-tls without --mqtt-tls-insecure? If so do we need two options? If we want to support real encryption and authentication there shall be additional parameters to specify certs and keys.

majherek commented 7 months ago

Thank you for the pull request.

If there is no option for the end user to specify paths to certificates and keys does it make sense to use --mqtt-tls without --mqtt-tls-insecure? If so do we need two options? If we want to support real encryption and authentication there shall be additional parameters to specify certs and keys.

The addition of the --mqtt-tls and --mqtt-tls-insecure options to the project is designed to enhance security and flexibility when connecting to MQTT servers over SSL/TLS. Here's a breakdown of the rationale and functionality behind each option:

--mqtt-tls Option: This option enables TLS encryption for the MQTT connection, ensuring that the data transmitted between the client and the server is encrypted. When using trusted certificates issued by recognized Certificate Authorities (CAs), such as those from Let's Encrypt, this option alone is sufficient. The underlying system or library typically has a list of trusted CAs, and if the server's certificate is signed by one of these CAs, the connection can be established securely without additional configuration.

--mqtt-tls-insecure Option: This option is specifically designed for scenarios where the MQTT server uses a self-signed certificate or a certificate not issued by a recognized CA. In such cases, the default certificate verification process would fail because the CA is not trusted. By using the --mqtt-tls-insecure option, you can bypass the CA verification step, allowing the connection to proceed. This is particularly useful for internal, development, or testing environments where the strict verification of certificates by a recognized CA is not necessary or feasible.

The combination of these options provides flexibility in how SSL/TLS connections are established. It accommodates both secure environments with properly signed certificates and less secure, or internal environments where self-signed certificates might be in use.

However, to address the original question, for true encryption and authentication, especially in production environments, it is indeed beneficial to have the option to specify paths to custom certificates and keys. This would allow users to use certificates (whether self-signed or CA-issued) securely by explicitly trusting them and to provide client certificates for mutual authentication if required by the server.

In summary, while the --mqtt-tls and --mqtt-tls-insecure options cover basic use cases, incorporating additional parameters for specifying custom certificate and key paths would significantly enhance security and flexibility for all users, catering to a broader range of deployment scenarios.

codecov[bot] commented 7 months ago

Codecov Report

Attention: 9 lines in your changes are missing coverage. Please review.

Comparison is base (0c5d55a) 53.27% compared to head (a5ec6c9) 51.35%.

Files Patch % Lines
hoymiles_mqtt/mqtt.py 0.00% 7 Missing :warning:
hoymiles_mqtt/__main__.py 0.00% 2 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #23 +/- ## ========================================== - Coverage 53.27% 51.35% -1.92% ========================================== Files 5 5 Lines 214 222 +8 Branches 34 36 +2 ========================================== Hits 114 114 - Misses 92 100 +8 Partials 8 8 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

wasilukm commented 7 months ago

Thank you for the explanation. Documentation for paho-mqtt says "ca_certs is required, all other parameters are optional and will default to None if not provided", so it doesn't look like it can use system certificates. However, after reading the documentation for the underlying SSL library, you may be correct. Okay, let's add more options when it will be requested.