Change MQTT library to PubSubClient for ESP8266 to support TLS connections using WifiSecureClient

[hermlon]

What does this implement/fix?

On esp8266 devices, the current MQTT implementation using AsyncMqttClient only supports a ssl_fingerprints method for checking TLS certificates. As this is not robust against TLS certificate renewals, I implemented the esp8266 backend using the PubSubClient library, which is the MQTT library also used by Tasmota.

This results in the same configuration working for esp8266 that is also working for esp32, namely certificate_authority and skip_cert_cn_check. (Although it is possible in theory, I did not implement support for TLS client certificates using client_certificate and client_certificate_key, as my esp8266 ran out of memory when including these large strings in the configuration.)

One thing I definitely require feedback on is this pointer storing the current object, which can then be called from the on_mqtt_message_wrapper_ callback. I think this would fail when initializing multiple instances of the MQTT component, but I don't know how this could be solved differently.

Types of changes

• [ ] Bugfix (non-breaking change which fixes an issue)
• [ ] New feature (non-breaking change which adds functionality)
• [x] Breaking change (fix or feature that would cause existing functionality to not work as expected)
• [ ] Other

Related issue or feature (if applicable): fixes https://github.com/esphome/issues/issues/2660

Pull request in esphome-docs with documentation (if applicable): I would update the documentation as soon as someone confirmed the project actually wants to merge this contribution and that the ssl_fingerprints configuration for esp8266 can be dropped.

Test Environment

• [ ] ESP32
• [ ] ESP32 IDF
• [x] ESP8266
• [ ] RP2040
• [ ] BK72xx
• [ ] RTL87xx

Example entry for config.yaml:

# Example config.yaml
wifi:
  ssid: MySSID
  password: mypassword

# has to be included so the TLS certificate validity time span can be verified
time:
  - platform: sntp

mqtt:
  client_id: "test-client-id"
  broker: broker.hivemq.com
  port: 8883
  certificate_authority: |
    -----BEGIN CERTIFICATE-----
    MIIEdTCCA12gAwIBAgIJAKcOSkw0grd/MA0GCSqGSIb3DQEBCwUAMGgxCzAJBgNV
    BAYTAlVTMSUwIwYDVQQKExxTdGFyZmllbGQgVGVjaG5vbG9naWVzLCBJbmMuMTIw
    MAYDVQQLEylTdGFyZmllbGQgQ2xhc3MgMiBDZXJ0aWZpY2F0aW9uIEF1dGhvcml0
    eTAeFw0wOTA5MDIwMDAwMDBaFw0zNDA2MjgxNzM5MTZaMIGYMQswCQYDVQQGEwJV
    UzEQMA4GA1UECBMHQXJpem9uYTETMBEGA1UEBxMKU2NvdHRzZGFsZTElMCMGA1UE
    ChMcU3RhcmZpZWxkIFRlY2hub2xvZ2llcywgSW5jLjE7MDkGA1UEAxMyU3RhcmZp
    ZWxkIFNlcnZpY2VzIFJvb3QgQ2VydGlmaWNhdGUgQXV0aG9yaXR5IC0gRzIwggEi
    MA0GCSqGSIb3DQEBAQUAA4IBDwAwggEKAoIBAQDVDDrEKvlO4vW+GZdfjohTsR8/
    y8+fIBNtKTrID30892t2OGPZNmCom15cAICyL1l/9of5JUOG52kbUpqQ4XHj2C0N
    Tm/2yEnZtvMaVq4rtnQU68/7JuMauh2WLmo7WJSJR1b/JaCTcFOD2oR0FMNnngRo
    Ot+OQFodSk7PQ5E751bWAHDLUu57fa4657wx+UX2wmDPE1kCK4DMNEffud6QZW0C
    zyyRpqbn3oUYSXxmTqM6bam17jQuug0DuDPfR+uxa40l2ZvOgdFFRjKWcIfeAg5J
    Q4W2bHO7ZOphQazJ1FTfhy/HIrImzJ9ZVGif/L4qL8RVHHVAYBeFAlU5i38FAgMB
    AAGjgfAwge0wDwYDVR0TAQH/BAUwAwEB/zAOBgNVHQ8BAf8EBAMCAYYwHQYDVR0O
    BBYEFJxfAN+qAdcwKziIorhtSpzyEZGDMB8GA1UdIwQYMBaAFL9ft9HO3R+G9FtV
    rNzXEMIOqYjnME8GCCsGAQUFBwEBBEMwQTAcBggrBgEFBQcwAYYQaHR0cDovL28u
    c3MyLnVzLzAhBggrBgEFBQcwAoYVaHR0cDovL3guc3MyLnVzL3guY2VyMCYGA1Ud
    HwQfMB0wG6AZoBeGFWh0dHA6Ly9zLnNzMi51cy9yLmNybDARBgNVHSAECjAIMAYG
    BFUdIAAwDQYJKoZIhvcNAQELBQADggEBACMd44pXyn3pF3lM8R5V/cxTbj5HD9/G
    VfKyBDbtgB9TxF00KGu+x1X8Z+rLP3+QsjPNG1gQggL4+C/1E2DUBc7xgQjB3ad1
    l08YuW3e95ORCLp+QCztweq7dp4zBncdDQh/U90bZKuCJ/Fp1U1ervShw3WnWEQt
    8jxwmKy6abaVd38PMV4s/KCHOkdp8Hlf9BRUpJVeEXgSYCfOn8J3/yNTd126/+pZ
    59vPr5KW7ySaNRB6nJHGDn2Z9j8Z3/VyVOEVqQdZe4O/Ui5GjLIAZHYcSNPYeehu
    VsyuLAOQ1xk4meTKCRlb/weWsKh/NEnfVqn3sF/tM+2MR7cwA130A4w=
    -----END CERTIFICATE-----
  skip_cert_cn_check: false

logger:
  level: DEBUG

esphome:
  name: "mqtt-esp8266"

esp8266:
  board: esp12e

This config above can actually be run, in contrast to the one I provided in tests/components/mqtt/esp8266-tls.yaml. I tried to follow the existing tests so I did not include all the configuration there. It would be nice if it could be tested whether my provided test actually works.

Checklist:

• [x] The code change is tested and works locally.
• [x] Tests have been added to verify that the new code works (under tests/ folder).

If user exposed functionality or configuration variables are added/changed:

• [ ] Documentation added/updated in esphome-docs.

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 50.00000% with 3 lines in your changes missing coverage. Please review.

Project coverage is 53.96%. Comparing base (4d8b5ed) to head (70b54ab). Report is 1314 commits behind head on dev.

Files with missing lines	Patch %	Lines
esphome/loader.py	50.00%	1 Missing and 1 partial :warning:
esphome/mqtt.py	0.00%	1 Missing :warning:

Additional details and impacted files

@@            Coverage Diff             @@
##              dev    #7369      +/-   ##
==========================================
+ Coverage   53.70%   53.96%   +0.25%     
==========================================
  Files          50       50              
  Lines        9408     9691     +283     
  Branches     1654     1712      +58     
==========================================
+ Hits         5053     5230     +177     
- Misses       4056     4129      +73     
- Partials      299      332      +33     

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

[hermlon]

I'm going to rewrite this using https://github.com/256dpi/arduino-mqtt and look into LibreTiny support as well, so there's no review necessary at that point.

[cociweb]

Um, well, great job! But...

as my esp8266 ran out of memory when including these large strings in the configuration.

That's why a fingerprint is a crucial alternative. It occupies far less space than storing an extensive string for certificate authority. (Fingerprint is also configurable in Tasmota, by the way.) As I observe, in your pull request, the support for fingerprint has been completely removed.I have doubts about whether it is a good concept.

[hermlon]

Thank you, that's a really good point, I haven't thought of it in that way. I'll put the fingerprint back in, so people can choose what to use.

[cociweb]

Thank you, that's a really good point, I haven't thought of it in that way. I'll put the fingerprint back in, so people can choose what to use.

Well in that case, it won't resolve the the oiginal problem of https://github.com/esphome/issues/issues/2660.

[cociweb]

I noticed that you reverted the CONF_SSL_FINGERPRINT. Was it intentional to use CONF_SSL_FINGERPRINT rather than CONF_SSL_FINGERPRINTS?

[hermlon]

yes, unfortunately WiFiClientSecure's setFingerprint only supports one certificate hash at a time. Therefore I thought this could be changed to only support one fingerprint, instead of a list. Is there a use case for having multiple fingerprints to validate against?

[cociweb]

yes, unfortunately WiFiClientSecure's setFingerprint only supports one certificate hash at a time. Therefore I thought this could be changed to only support one fingerprint, instead of a list. Is there a use case for having multiple fingerprints to validate against?

Well, in this case (because of the 'S') this is a 'breaking change'...of an already broken feature. :D https://esphome.io/components/mqtt.html#ssl-fingerprints

[github-actions[bot]]

There hasn't been any activity on this pull request recently. This pull request has been automatically marked as stale because of that and will be closed if no further activity occurs within 7 days. If you are the author of this PR, please leave a comment if you want to keep it open. Also, please rebase your PR onto the latest dev branch to ensure that it's up to date with the latest changes. Thank you for your contribution!