Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Current status of MQTT SSL #1465

Open
ghost opened this issue Jan 2, 2019 · 22 comments
Open

Current status of MQTT SSL #1465

ghost opened this issue Jan 2, 2019 · 22 comments
Assignees
Labels
enhancement New feature or request question

Comments

@ghost
Copy link

ghost commented Jan 2, 2019

Running latest master branch as of today, I'm not sure anymore if the SSL support still conflicts with the web UI regardless of the platform used (I'm building with 180).

It would be nice to have a current document on SSL on its own page, as well as tweaks to enable HTTP access for the UI but MQTT over SSL, to minimize the performance requirements.

@ghost ghost added the question label Jan 2, 2019
@mcspr mcspr self-assigned this Jan 7, 2019
@mcspr
Copy link
Collaborator

mcspr commented Jan 31, 2019

Some notes after testing:

  • Core 2.4.2 / async-mqtt-client
    is ok'ish with both mqtt and web. can save settings, can view debug log. once crashed on page reload
  • Core 2.4.2 / lwip2 (low/high) / pubsubclient / modified mqtt module to support bearssl wificlient
    works ok with basic payloads, but crashes with mere 260 bytes of json. upping pubsubclient mqtt packet buffer size does not help, neither is tuning in/out fragment length of the wificlient. web ok
  • Core 2.5.0 beta (platform staging) / lwip2 low / same pubsubclient - works ok for both basic and json. could not get web to crash.

All tests done with local server, because I could not get cloudmqtt.com to work. It just crashes the board (or fails to connect at the best of times). Tried async/axtls builds, new sync/bearssl, even tried different firmware from mongoose os - nothing works with them :/

Pubsubclient + bearssl combo can also use CA cert instead of fingerprinting, either as hardcoded string or spiffs file. Also, fingerprinting api is different and there is no need to manually do it (client.connect, check fp, mqtt.setClient, mqtt.connect repeat)

@stale

This comment has been minimized.

@stale

This comment has been minimized.

@stale stale bot closed this as completed Apr 8, 2019
@Niek
Copy link
Contributor

Niek commented Apr 8, 2019

My experience with core 2.4.2:

  • aync-mqtt-client works, but disconnects randomly when sending or receiving messages. I thought it was related to setWill() cause mqtt could not connect with SSL enable marvinroger/async-mqtt-client#107, but disabling setWill() doesn't help
  • pubsubclient doesn't work at all for me, it keeps disconnecting at connection stage - maybe interfering with web? (edit: pretty sure this is due to conflicts with ESPAsyncTCP)
  • pubsubclient + bearssl is no option because the binary size grows with 100KB+ - maybe using BearSSL::WiFiClientSecure results in both SSL variants to be compiled in?

I tried testing with core 2.5.0 as well, but unfortunately the binary grows too large to do OTA on a 1MB board - no matter which MQTT library I use.

@mcspr mcspr changed the title Document current status as of Jan 2019 of MQTT SSL Current status of MQTT SSL Apr 9, 2019
@mcspr mcspr reopened this Apr 9, 2019
@stale stale bot removed the stale label Apr 9, 2019
@mcspr
Copy link
Collaborator

mcspr commented Apr 9, 2019

pretty sure this is due to conflicts with ESPAsyncTCP

Like, what conflicts? I've seen the comment, but I still do not quite understand what conflict is there.

In general with BearSSL:

@Niek
Copy link
Contributor

Niek commented Apr 10, 2019

MFLN is interesting - if you compile your broker against OpenSSL 1.1 it should work out of the box. It should be quite easy to do a couple of quick probeMaxFragmentLength() calls in the MQTT module before connecting.

As for conflicts: whenever I try to use PubSubClient (MQTT_USE_ASYNC=0) I cannot connect to any MQTT broker - the _mqtt_client_secure.connect() call always fails. Since it's suggested in the header files, I assumed the WifiClientSecure conflicts with WifiClient used in ESPAsyncTCP.

@Niek
Copy link
Contributor

Niek commented Apr 16, 2019

Just an update after lots of experimentation:
PubSubClient SSL (MQTT_USE_ASYNC=0) does not work in any of my cases. When I compile with -DDEBUG_ESP_SSL -DDEBUG_ESP_TLS_MEM I see the following errors:

[MQTT] Connecting as user X
please start sntp first !
State:	sending Client Hello (1)
Error: connection lost

When using AsyncMqttClient, SSL works but only a few packets. It disconnects and then reconnects in a loop every x packets.
I tested with Arduino core 2.4.2 and 2.5.0 (axTLS only, BearSSL is too heavy) and LWIP 1.4 and 2.0 - no difference.

@mcspr
Copy link
Collaborator

mcspr commented Apr 16, 2019

iit i've already mentioned the problem - you need to manually supply NTP time to the WiFiClient instance. "please start sntp first !" error is coming from us not using LWIP implementation of NTP client, it expects us to call configTime() somewhere (example).

With NtpClientLib flow should be

  1. MQTT module waits for ntpSynced() before starting connection routine
  2. mqtt_client.setX509Time(now()). setInsecure() / set trust anchors
  3. try connecting

I'll try to update pubsubclient support for this while keeping fingerprinting / support old Core versions.

@Niek
Copy link
Contributor

Niek commented Apr 16, 2019

Hmm, but this all with axTLS - which doesn't even have setX509Time(). Also manually calling configTime() doesn't seem to resolve anything - but maybe I'm overseeing something.

@mcspr
Copy link
Collaborator

mcspr commented Apr 16, 2019

Ah. Time error would only come with bearssl, when it calls time() internally
searching time(... in https://github.com/earlephilhower/bearssl-esp8266
which comes from this override https://github.com/esp8266/Arduino/blob/3b9db65ea33890e9fa5e911ea4170e4b31fd03be/cores/esp8266/time.cpp#L83
and the origin of the error https://github.com/esp8266/Arduino/blob/3b9db65ea33890e9fa5e911ea4170e4b31fd03be/tools/sdk/lwip/src/core/sntp.c#L625
(that is lwip1.4, but lwip2 provides the same function)

What server config / could mqtt provider do you use for testing? I don't remember any reconnects with local mosquitto + async mqtt

@Niek
Copy link
Contributor

Niek commented Apr 17, 2019

I see slightly different output in LWIP2 vs LWIP 1.4:

Core 2.4.2 / 2.5.0, LWIP 2 with axTLS:

:close
:ur 1
:del
:ref 1
:ref 2
State:	sending Client Hello (1)
Error: connection lost
:ur 2

Core 2.4.2 / 2.5.0, LWIP 1.4 with axTLS:

:close
:ur 1
:del
:ref 1
:ref 2
please start sntp first !
State:	sending Client Hello (1)
Error: connection lost
:ur 2

This is just calling connect() on the axTLS::WiFiClientSecure object. It doesn't matter which host I use, I tried a lot of them and all connect() calls fail. If I compile a sample like https://github.com/esp8266/Arduino/blob/master/libraries/ESP8266WiFi/examples/HTTPSRequestAxTLS/HTTPSRequestAxTLS.ino it works fine, so I think some library used in Espurna is interfering with WifiClientSecure like is suggested in the config.

Edit: Finally got it working. I can confirm that it's really ESPAsyncTCP that is messing with WifiClientSecure. The following config works for me:

#define TELNET_SUPPORT              0
#define WEB_SUPPORT                 0
#define TERMINAL_SUPPORT            0
#define OTA_MQTT_SUPPORT            0
#define DEBUG_TELNET_SUPPORT        0
#define ALEXA_SUPPORT               0
#define INFLUXDB_SUPPORT            0
#define THINGSPEAK_SUPPORT          0

@mcspr
Copy link
Collaborator

mcspr commented Apr 18, 2019

🤷‍♂️ Opposite results. BearSSL works, axTLS variant does not. I would not put async as a whole as the reason. We are using more RAM than basic example, and do a lot more inside loop() too. If you are not connected via async library, web or telnet, how would it conflict?

Using 2.5.0.
axTLS enabled via -DUSING_AXTLS, manually including WiFiClientSecureAxTLS.h, using namespace axTLS;

BearSSL test that worked:

  • drop manual probing that checks fingerprint, just do _mqtt.setClient(_mqtt_client_secure);
  • _mqtt_client_secure.setInsecure() somewhere before connect call (since we only want to test if it works at all)

There's also a small bug with sync client - _mqtt_last_connection = millis(); needs to be set if we can't connect. It messes up timing really bad, forcing reconnection every loop after some time, if server is not responding.

@Niek
Copy link
Contributor

Niek commented Apr 18, 2019

Strange. What makes it complicated is that it depends on a lot of factors, like if the broker supports one of the few ciphers that axTLS supports (e.g. AES128-SHA or AES265-SHA).
I have a WIP branch @ https://github.com/Niek/espurna/tree/mqtt-fixes with some fixes. I added the https://github.com/256dpi/arduino-mqtt library as well, since it's much better maintained than AsyncMqttClient and PubSubClient. Next step would be to add full support for BearSSL and a configurable setting for validation (non/insecure, fingerprint, chain, etc).

@mcspr
Copy link
Collaborator

mcspr commented Apr 18, 2019

Huh.

Tests done on Fedora 29
mosquitto-1.5.8-1.fc29.x86_64
openssl-1.1.1b-3.fc29.x86_64

Looking at debug log, connection fails on Hello step. Packet capture shows that server answers:

Cipher Suite: TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (0xc030)

Which I don't see here: https://github.com/igrr/axtls-8266/blob/master/README.md

But!, It is in the cipher list inside client's Hello:
Cipher Suites (45 suites)
    Cipher Suite: TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256 (0xcca9)
    Cipher Suite: TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256 (0xcca8)
    Cipher Suite: TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256 (0xc02b)
    Cipher Suite: TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 (0xc02f)
    Cipher Suite: TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384 (0xc02c)
    Cipher Suite: TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (0xc030) <--------
    Cipher Suite: TLS_ECDHE_ECDSA_WITH_AES_128_CCM (0xc0ac)
    Cipher Suite: TLS_ECDHE_ECDSA_WITH_AES_256_CCM (0xc0ad)
    Cipher Suite: TLS_ECDHE_ECDSA_WITH_AES_128_CCM_8 (0xc0ae)
    Cipher Suite: TLS_ECDHE_ECDSA_WITH_AES_256_CCM_8 (0xc0af)
    Cipher Suite: TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256 (0xc023)
    Cipher Suite: TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256 (0xc027)
    Cipher Suite: TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA384 (0xc024)
    Cipher Suite: TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384 (0xc028)
    Cipher Suite: TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA (0xc009)
    Cipher Suite: TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA (0xc013)
    Cipher Suite: TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA (0xc00a)
    Cipher Suite: TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA (0xc014)
    Cipher Suite: TLS_ECDH_ECDSA_WITH_AES_128_GCM_SHA256 (0xc02d)
    Cipher Suite: TLS_ECDH_RSA_WITH_AES_128_GCM_SHA256 (0xc031)
    Cipher Suite: TLS_ECDH_ECDSA_WITH_AES_256_GCM_SHA384 (0xc02e)
    Cipher Suite: TLS_ECDH_RSA_WITH_AES_256_GCM_SHA384 (0xc032)
    Cipher Suite: TLS_ECDH_ECDSA_WITH_AES_128_CBC_SHA256 (0xc025)
    Cipher Suite: TLS_ECDH_RSA_WITH_AES_128_CBC_SHA256 (0xc029)
    Cipher Suite: TLS_ECDH_ECDSA_WITH_AES_256_CBC_SHA384 (0xc026)
    Cipher Suite: TLS_ECDH_RSA_WITH_AES_256_CBC_SHA384 (0xc02a)
    Cipher Suite: TLS_ECDH_ECDSA_WITH_AES_128_CBC_SHA (0xc004)
    Cipher Suite: TLS_ECDH_RSA_WITH_AES_128_CBC_SHA (0xc00e)
    Cipher Suite: TLS_ECDH_ECDSA_WITH_AES_256_CBC_SHA (0xc005)
    Cipher Suite: TLS_ECDH_RSA_WITH_AES_256_CBC_SHA (0xc00f)
    Cipher Suite: TLS_RSA_WITH_AES_128_GCM_SHA256 (0x009c)
    Cipher Suite: TLS_RSA_WITH_AES_256_GCM_SHA384 (0x009d)
    Cipher Suite: TLS_RSA_WITH_AES_128_CCM (0xc09c)
    Cipher Suite: TLS_RSA_WITH_AES_256_CCM (0xc09d)
    Cipher Suite: TLS_RSA_WITH_AES_128_CCM_8 (0xc0a0)
    Cipher Suite: TLS_RSA_WITH_AES_256_CCM_8 (0xc0a1)
    Cipher Suite: TLS_RSA_WITH_AES_128_CBC_SHA256 (0x003c)
    Cipher Suite: TLS_RSA_WITH_AES_256_CBC_SHA256 (0x003d)
    Cipher Suite: TLS_RSA_WITH_AES_128_CBC_SHA (0x002f)
    Cipher Suite: TLS_RSA_WITH_AES_256_CBC_SHA (0x0035)
    Cipher Suite: TLS_ECDHE_ECDSA_WITH_3DES_EDE_CBC_SHA (0xc008)
    Cipher Suite: TLS_ECDHE_RSA_WITH_3DES_EDE_CBC_SHA (0xc012)
    Cipher Suite: TLS_ECDH_ECDSA_WITH_3DES_EDE_CBC_SHA (0xc003)
    Cipher Suite: TLS_ECDH_RSA_WITH_3DES_EDE_CBC_SHA (0xc00d)
    Cipher Suite: TLS_RSA_WITH_3DES_EDE_CBC_SHA (0x000a)

Most basic mosquitto config. Changing ciphers does nothing, client disconnects right after server Hello is done.

port 8883
cafile ca.crt
certfile server.crt
keyfile server.key
#this does not help either:
#ciphers AES256-SHA256:AES-SHA:AES128-SHA256:AES128-SHA

edit: Which is probably something broken with 2.5.0 + axtls client, because older Cores work fine.

@Niek
Copy link
Contributor

Niek commented Apr 19, 2019

I just tested both core 2.4.2 (axTLS) and 2.5.0 (axTLS and BearSSL), and both work fine for me (BearSSL needs the setInsecure() call though), axTLS on 2.5.0 works only with this explicit declaration:

#define USING_AXTLS // do not use BearSSL
#include "WiFiClientSecureAxTLS.h"
axTLS::WiFiClientSecure _mqtt_client_secure;

I pushed the changes to https://github.com/Niek/espurna/tree/mqtt-fixes

@mcspr
Copy link
Collaborator

mcspr commented Apr 19, 2019

Ok. BTW, I had changed MQTT settings bit in #1701. I'd like to merge that to fix reloading, if you can review for any issues it would be appreciated.
If the mqtt-fixes tree can work with all Cores, it would be great

Brief look ArduinoMQTT, I think it still applies. It copies some of params, and we can't know which were set. e.g. hostname and will topic are private members with no public getters. Otherwise, I am a big fan of WiFiClient interface. I wish both async clients were a bit more pluggable, so it would not be a must to change async-mqtt-client code :(

sidenote: In #1693 I tried a different approach at splitting implementations. I am a bit scared of too much nested ifdefs, but it might be a personal preference

@ghost
Copy link
Author

ghost commented Apr 21, 2019

If this gains traction, especially @Niek 's changes, it could be a positive development to merge. Because the telnet interface is incomplete, and insecure, not being able to operate the web interface over SSL is an issue, especially since MQTT with SSL is probably a definite must at this point. Relying on the security of the physical transport link (wireless) to safeguard MQTT credentials is a dumb idea.

I haven't had time to test any of these yet, but I'm keeping a watchful eye on this thread and hope @xoseperez does too.

@mcspr
Copy link
Collaborator

mcspr commented May 17, 2019

Regarding secure WiFiClient in general, there are two annoying problems with old Cores:
esp8266/Arduino#5786

Verifier struct(s) not freed before client is destroyed (only needed when connecting, not after)
Client might hang under certain conditions (see second PR)

@Niek
Copy link
Contributor

Niek commented May 22, 2019

Regarding secure WiFiClient in general, there are two annoying problems with old Cores:
esp8266/Arduino#5786

Verifier struct(s) not freed before client is destroyed (only needed when connecting, not after)
Client might hang under certain conditions (see second PR)

That seems to be fixed in core 2.5.2 / PIO 2.2.0 - I'm adding a PR now to add support for the last 2 releases. I will also soon post a PR with the MQTT changes.

@Niek
Copy link
Contributor

Niek commented Jul 17, 2019

Linking this PR in here: #1829

@mcspr
Copy link
Collaborator

mcspr commented Aug 23, 2019

Followup on the #1829 (comment), not to mix PR comments and general info.

Tasmota implementation includes bearssl & wificlientsecure into the tree (which reminds me of the ESPAsyncTCP fork...), and ignores X509List class so that cert chain is read directly from the progmem pointer:
https://github.com/arendst/Sonoff-Tasmota/blob/5cb863d35b8f7cc67497d937ab50c299b301e290/sonoff/xdrv_02_mqtt.ino#L217
https://github.com/arendst/Sonoff-Tasmota/blob/46d2cb8d0b58a68ac91a531f355730bca5a75309/sonoff/WiFiClientSecureLightBearSSL.cpp#L237
https://github.com/arendst/Sonoff-Tasmota/blob/46d2cb8d0b58a68ac91a531f355730bca5a75309/sonoff/WiFiClientSecureLightBearSSL.cpp#L836

original method accepts helper class X509List, which is a really simple wrapper for the C x509 struct
https://github.com/esp8266/Arduino/blob/8f45a0fb91621de2c8bedb5723f2eb3b527706c4/libraries/ESP8266WiFi/src/WiFiClientSecureBearSSL.h#L94
https://github.com/esp8266/Arduino/blob/55539ae941bcf5a84ba6c53f1ea8bce145882424/libraries/ESP8266WiFi/src/WiFiClientSecureBearSSL.cpp#L1019
https://github.com/esp8266/Arduino/blob/55539ae941bcf5a84ba6c53f1ea8bce145882424/libraries/ESP8266WiFi/src/BearSSLHelpers.h#L113
And current bearssl in the Core should be working ok?
earlephilhower/bearssl-esp8266#15 I am sort of assuming that the version they are using is from the core, can't find any pre-built binaries in the tree there. Nontheless, "t_bearssl.h" is used instead of "bearssl.h" for bearssl C library APIs

This approach is only useful if cert is stored inside of flash sector, skipping costly RAM copy. Because the other option is to use FS (SPIFFS / LittleFS) and a single certs.ar container, where CertStore helper comes into play. While full chain is stored in the file, we still seem to have to load the required CA cert into the RAM for validation (i think it is like that? https://github.com/esp8266/Arduino/blob/55539ae941bcf5a84ba6c53f1ea8bce145882424/libraries/ESP8266WiFi/src/CertStoreBearSSL.cpp#L182)

Sources:
https://github.com/esp8266/Arduino/blob/master/libraries/ESP8266WiFi/src/WiFiClientSecureBearSSL.cpp
https://github.com/esp8266/Arduino/blob/master/libraries/ESP8266WiFi/src/CertStoreBearSSL.cpp
https://github.com/arendst/Sonoff-Tasmota/blob/development/sonoff/WiFiClientSecureLightBearSSL.cpp
https://github.com/arendst/Sonoff-Tasmota/tree/development/lib/bearssl-esp8266

@Niek
Copy link
Contributor

Niek commented Aug 27, 2019

The Tasmota way of PEM->DER->base64->config sounds overly complicated to me, in my opinion it would make much more sense to upload the certificate files through the web UI and store it somewhere on the FS (sample code: https://github.com/esp8266/Arduino/blob/0937b076c8ac31d3dbfe7ed4ccc3a2efd7378396/libraries/ESP8266WiFi/examples/BearSSL_CertStore/BearSSL_CertStore.ino#L141-L150). Or maybe even offer the option to auto-fetch the server certificate once and store it for future connects.

@mcspr mcspr added the enhancement New feature or request label Aug 30, 2019
mcspr added a commit that referenced this issue Oct 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request question
Projects
None yet
Development

No branches or pull requests

2 participants