From 55a0abf1e15cc45a6b7de98c7708face7cb9e1c4 Mon Sep 17 00:00:00 2001 From: Max Prokhorov Date: Thu, 28 Nov 2019 04:28:34 +0300 Subject: [PATCH] Secure client / server cleanup (#2016) * web: revert changed from secure_client patch * ota/httpupdate: use SecureClient * ota/httpupdate: use scheduled function on mqtt event * config: finish up 590282e changes + add warnings * typo fix * use bind instead of c++14 magic, use debug text from secureclient * bump * actually use the locking --- README.md | 2 +- code/espurna/config/dependencies.h | 29 +++--- code/espurna/config/deprecated.h | 5 + code/espurna/ota_httpupdate.ino | 145 +++++++++++++++-------------- code/espurna/thinkspeak.ino | 2 +- code/espurna/web.ino | 12 +-- code/platformio.ini | 13 +-- 7 files changed, 111 insertions(+), 97 deletions(-) diff --git a/README.md b/README.md index b34aac5c..71f2ac5e 100644 --- a/README.md +++ b/README.md @@ -268,7 +268,7 @@ Here is the list of supported hardware. For more information please refer to the |![Itead Sonoff 4CH](images/devices/itead-sonoff-4ch.jpg)|![Itead Sonoff 4CH Pro](images/devices/itead-sonoff-4ch-pro.jpg)|| |**Itead Sonoff 4CH**|**Itead Sonoff 4CH Pro**|| |![Allterco Shelly 1 / 1PM](images/devices/allterco-shelly1.jpg)|![Allterco Shelly 2 / 2.5](images/devices/allterco-shelly2.jpg)|![Jan Goedeke Wifi Relay (NO/NC)](images/devices/jangoe-wifi-relay.jpg)| -|**Alterco Shelly1**|**Alterco Shelly2**|**Jan Goedeke Wifi Relay (NO/NC)**| +|**Alterco Shelly 1 / 1PM**|**Alterco Shelly 2 / 2.5**|**Jan Goedeke Wifi Relay (NO/NC)**| |![EXS Wifi Relay v3.1](images/devices/exs-wifi-relay-v31.jpg)|![EXS Wifi Relay v5.0](images/devices/exs-wifi-relay-v50.jpg)|![Jorge García Wifi + Relays Board Kit](images/devices/jorgegarcia-wifi-relays.jpg)| |**EXS Wifi Relay v3.1**|**EXS Wifi Relay v5.0**|**Jorge García Wifi + Relays Board Kit**| |![Allnet ESP8266-UP-Relay](images/devices/allnet-esp8266-up-relay.jpg)|![Bruno Horta's OnOfre](images/devices/bh-onofre.jpg)|![Luani HVIO](images/devices/luani-hvio.jpg)| diff --git a/code/espurna/config/dependencies.h b/code/espurna/config/dependencies.h index 31935ef7..eb2aa810 100644 --- a/code/espurna/config/dependencies.h +++ b/code/espurna/config/dependencies.h @@ -5,11 +5,6 @@ // Configuration settings are in the general.h file //------------------------------------------------------------------------------ -#if defined(ASYNC_TCP_SSL_ENABLED) && SECURE_CLIENT == SECURE_CLIENT_NONE -#undef SECURE_CLIENT -#define SECURE_CLIENT SECURE_CLIENT_AXTLS -#endif - #if DEBUG_TELNET_SUPPORT #undef TELNET_SUPPORT #define TELNET_SUPPORT 1 @@ -70,13 +65,6 @@ #define MQTT_SUPPORT 1 // If Home Assistant enabled enable MQTT #endif -#if SECURE_CLIENT != SECURE_CLIENT_AXTLS -#if THINGSPEAK_USE_SSL && THINGSPEAK_USE_ASYNC -#undef THINGSPEAK_SUPPORT -#define THINGSPEAK_SUPPORT 0 // Thingspeak in ASYNC mode requires SECURE_CLIENT_AXTLS -#endif -#endif - #if THINGSPEAK_SUPPORT #undef BROKER_SUPPORT #define BROKER_SUPPORT 1 // If Thingspeak enabled enable BROKER @@ -124,3 +112,20 @@ #define BROKER_SUPPORT 1 // Broker is required to process relay & lights events #endif +// TODO: clean-up SSL_ENABLED and USE_SSL settings for 1.15.0 +#if ASYNC_TCP_SSL_ENABLED && SECURE_CLIENT == SECURE_CLIENT_NONE +#undef SECURE_CLIENT +#define SECURE_CLIENT SECURE_CLIENT_AXTLS +#endif + +#if THINGSPEAK_USE_SSL && THINGSPEAK_USE_ASYNC && (!ASYNC_TCP_SSL_ENABLED) +#warning "Thingspeak in ASYNC mode requires a globally defined ASYNC_TCP_SSL_ENABLED=1" +#undef THINGSPEAK_SUPPORT +#define THINGSPEAK_SUPPORT 0 // Thingspeak in ASYNC mode requires ASYNC_TCP_SSL_ENABLED +#endif + +#if WEB_SUPPORT && WEB_SSL_ENABLED && (!ASYNC_TCP_SSL_ENABLED) +#warning "WEB_SUPPORT with SSL requires a globally defined ASYNC_TCP_SSL_ENABLED=1" +#undef WEB_SSL_ENABLED +#define WEB_SSL_ENABLED 0 // WEB_SUPPORT mode th SSL requires ASYNC_TCP_SSL_ENABLED +#endif diff --git a/code/espurna/config/deprecated.h b/code/espurna/config/deprecated.h index 1bdef4ae..c5817822 100644 --- a/code/espurna/config/deprecated.h +++ b/code/espurna/config/deprecated.h @@ -42,3 +42,8 @@ #ifdef HOMEASSISTANT_PAYLOAD_NOT_AVAILABLE #warning HOMEASSISTANT_PAYLOAD_NOT_AVAILABLE is deprecated! Global MQTT_STATUS_OFFLINE is used instead #endif + +// 1.14.0 adds SecureClient +#if MQTT_SUPPORT && MQTT_LIBRARY == MQTT_LIBRARY_ASYNCMQTT_CLIENT && ASYNC_TCP_SSL_ENABLED +#warning "Current implementation of AsyncMqttClient with axTLS is no longer supported. Consider switching to the SECURE_CLIENT configuration with MQTT_LIBRARY_ARDUINOMQTT or MQTT_LIBRARY_PUBSUBCLIENT. See: https://github.com/xoseperez/espurna/issues/1465" +#endif diff --git a/code/espurna/ota_httpupdate.ino b/code/espurna/ota_httpupdate.ino index 95671952..b80113c0 100644 --- a/code/espurna/ota_httpupdate.ino +++ b/code/espurna/ota_httpupdate.ino @@ -16,9 +16,11 @@ Copyright (C) 2019 by Maxim Prokhorov #include #include +#include #include "libs/URL.h" #include "libs/TypeChecks.h" +#include "libs/SecureClientHelpers.h" #if SECURE_CLIENT != SECURE_CLIENT_NONE @@ -31,6 +33,10 @@ Copyright (C) 2019 by Maxim Prokhorov #endif // SECURE_CLIENT != SECURE_CLIENT_NONE +// ----------------------------------------------------------------------------- +// Configuration templates +// ----------------------------------------------------------------------------- + template void _otaFollowRedirects(const std::true_type&, T& instance) { instance.followRedirects(true); @@ -64,6 +70,49 @@ namespace ota { using has_WiFiClient_argument = is_detected; } +// ----------------------------------------------------------------------------- +// Settings helper +// ----------------------------------------------------------------------------- + +#if SECURE_CLIENT == SECURE_CLIENT_AXTLS +SecureClientConfig _ota_sc_config { + "OTA", + []() -> String { + return String(); // NOTE: unused + }, + []() -> int { + return getSetting("otaScCheck", OTA_SECURE_CLIENT_CHECK).toInt(); + }, + []() -> String { + return getSetting("otaFP", OTA_FINGERPRINT); + }, + true +}; +#endif + +#if SECURE_CLIENT == SECURE_CLIENT_BEARSSL +SecureClientConfig _ota_sc_config { + "OTA", + []() -> int { + return getSetting("otaScCheck", OTA_SECURE_CLIENT_CHECK).toInt(); + }, + []() -> PGM_P { + return _ota_client_trusted_root_ca; + }, + []() -> String { + return getSetting("otaFP", OTA_FINGERPRINT); + }, + []() -> uint16_t { + return getSetting("otaScMFLN", OTA_SECURE_CLIENT_MFLN).toInt(); + }, + true +}; +#endif + +// ----------------------------------------------------------------------------- +// Generic update methods +// ----------------------------------------------------------------------------- + void _otaClientRunUpdater(WiFiClient* client, const String& url, const String& fp = "") { UNUSED(client); @@ -76,8 +125,6 @@ void _otaClientRunUpdater(WiFiClient* client, const String& url, const String& f // TODO: support currentVersion (string arg after 'url') // NOTE: ESPhttpUpdate.update(..., fp) will **always** fail with empty fingerprint - // NOTE: It is possible to support BearSSL with 2.4.2 by using uint8_t[20] instead of String for fingerprint argument - _otaFollowRedirects(ota::has_followRedirects{}, ESPhttpUpdate); ESPhttpUpdate.rebootOnUpdate(false); @@ -91,6 +138,8 @@ void _otaClientRunUpdater(WiFiClient* client, const String& url, const String& f result = ESPhttpUpdate.update(url); } #else + // TODO: implement through callbacks? + // see https://github.com/esp8266/Arduino/pull/6796 result = _otaClientUpdate(ota::has_WiFiClient_argument{}, ESPhttpUpdate, client, url); #endif @@ -123,65 +172,21 @@ void _otaClientFromHttp(const String& url) { void _otaClientFromHttps(const String& url) { - int check = getSetting("otaScCheck", OTA_SECURE_CLIENT_CHECK).toInt(); - bool settime = (check == SECURE_CLIENT_CHECK_CA); - - if (!ntpSynced() && settime) { - DEBUG_MSG_P(PSTR("[OTA] Time not synced!\n")); + // Check for NTP early to avoid constructing SecureClient prematurely + const int check = _ota_sc_config.on_check(); + if (!ntpSynced() && (check == SECURE_CLIENT_CHECK_CA)) { + DEBUG_MSG_P(PSTR("[OTA] Time not synced! Cannot use CA validation\n")); return; } // unique_ptr self-destructs after exiting function scope - // create WiFiClient on heap to use less stack space - auto client = std::make_unique(); - - if (check == SECURE_CLIENT_CHECK_NONE) { - DEBUG_MSG_P(PSTR("[OTA] !!! Connection will not be validated !!!\n")); - client->setInsecure(); - } - - if (check == SECURE_CLIENT_CHECK_FINGERPRINT) { - String fp_string = getSetting("otaFP", OTA_FINGERPRINT); - if (!fp_string.length()) { - DEBUG_MSG_P(PSTR("[OTA] Requested fingerprint auth, but 'otaFP' is not set\n")); - return; - } - - uint8_t fp_bytes[20] = {0}; - sslFingerPrintArray(fp_string.c_str(), fp_bytes); - - client->setFingerprint(fp_bytes); - } - - BearSSL::X509List *ca = nullptr; - if (check == SECURE_CLIENT_CHECK_CA) { - ca = new BearSSL::X509List(_ota_client_trusted_root_ca); - // because we do not support libc methods of getting time, force client to use ntpclientlib's current time - // XXX: local2utc method use is detrimental when DST happening. now() should be utc - client->setX509Time(ntpLocal2UTC(now())); - client->setTrustAnchors(ca); - } - - // TODO: RX and TX buffer sizes must be equal? - const uint16_t requested_mfln = getSetting("otaScMFLN", OTA_SECURE_CLIENT_MFLN).toInt(); - switch (requested_mfln) { - // default, do nothing - case 0: - break; - // match valid sizes only - case 512: - case 1024: - case 2048: - case 4096: - { - client->setBufferSizes(requested_mfln, requested_mfln); - break; - } - default: - DEBUG_MSG_P(PSTR("[OTA] Warning: MFLN buffer size must be one of 512, 1024, 2048 or 4096\n")); + // create the client on heap to use less stack space + auto client = std::make_unique(_ota_sc_config); + if (!client->beforeConnected()) { + return; } - _otaClientRunUpdater(client.get(), url); + _otaClientRunUpdater(&client->get(), url); } @@ -192,11 +197,12 @@ void _otaClientFromHttps(const String& url) { void _otaClientFromHttps(const String& url) { - const int check = getSetting("otaScCheck", OTA_SECURE_CLIENT_CHECK).toInt(); + // Note: this being the legacy option, only supporting legacy methods on ESPHttpUpdate itself + // no way to know when it is connected, so no afterConnected + const int check = _ota_sc_config.on_check(); + const String fp_string = _ota_sc_config.on_fingerprint(); - String fp_string; if (check == SECURE_CLIENT_CHECK_FINGERPRINT) { - fp_string = getSetting("otaFP", OTA_FINGERPRINT); if (!fp_string.length() || !sslCheckFingerPrint(fp_string.c_str())) { DEBUG_MSG_P(PSTR("[OTA] Wrong fingerprint\n")); return; @@ -247,15 +253,6 @@ void _otaClientInitCommands() { #if (MQTT_SUPPORT && OTA_MQTT_SUPPORT) bool _ota_do_update = false; -String _ota_url; - -void _otaClientLoop() { - if (_ota_do_update) { - _otaClientFrom(_ota_url); - _ota_do_update = false; - _ota_url = ""; - } -} void _otaClientMqttCallback(unsigned int type, const char * topic, const char * payload) { @@ -264,11 +261,18 @@ void _otaClientMqttCallback(unsigned int type, const char * topic, const char * } if (type == MQTT_MESSAGE_EVENT) { - String t = mqttMagnitude((char *) topic); - if (t.equals(MQTT_TOPIC_OTA)) { + const String t = mqttMagnitude((char *) topic); + if (!_ota_do_update && t.equals(MQTT_TOPIC_OTA)) { DEBUG_MSG_P(PSTR("[OTA] Queuing from URL: %s\n"), payload); + // TODO: c++14 support is required for `[_payload = String(payload)]() { ... }` + // c++11 also supports basic `std::bind(func, arg)`, but we need to reset the lock _ota_do_update = true; - _ota_url = payload; + + const String _payload(payload); + schedule_function([_payload]() { + _otaClientFrom(_payload); + _ota_do_update = false; + }); } } @@ -289,7 +293,6 @@ void otaClientSetup() { #if (MQTT_SUPPORT && OTA_MQTT_SUPPORT) mqttRegister(_otaClientMqttCallback); - espurnaRegisterLoop(_otaClientLoop); #endif } diff --git a/code/espurna/thinkspeak.ino b/code/espurna/thinkspeak.ino index fe3c9ee7..3e706056 100644 --- a/code/espurna/thinkspeak.ino +++ b/code/espurna/thinkspeak.ino @@ -227,7 +227,7 @@ void _tspkPost() { _tspk_client_ts = millis(); - #if SECURE_CLIENT == SECURE_CLIENT_AXTLS + #if THINGSPEAK_USE_SSL bool connected = _tspk_client->connect(THINGSPEAK_HOST, THINGSPEAK_PORT, THINGSPEAK_USE_SSL); #else bool connected = _tspk_client->connect(THINGSPEAK_HOST, THINGSPEAK_PORT); diff --git a/code/espurna/web.ino b/code/espurna/web.ino index 3620a721..4fbd0ccc 100644 --- a/code/espurna/web.ino +++ b/code/espurna/web.ino @@ -39,10 +39,10 @@ Copyright (C) 2016-2019 by Xose Pérez #endif // WEB_EMBEDDED -#if SECURE_CLIENT == SECURE_CLIENT_AXTLS & WEB_SSL_ENABLED +#if WEB_SSL_ENABLED #include "static/server.cer.h" #include "static/server.key.h" -#endif // SECURE_CLIENT == SECURE_CLIENT_AXTLS & WEB_SSL_ENABLED +#endif // WEB_SSL_ENABLED // ----------------------------------------------------------------------------- @@ -194,7 +194,7 @@ void _onHome(AsyncWebServerRequest *request) { } else { - #if SECURE_CLIENT == SECURE_CLIENT_AXTLS + #if WEB_SSL_ENABLED // Chunked response, we calculate the chunks based on free heap (in multiples of 32) // This is necessary when a TLS connection is open since it sucks too much memory @@ -235,7 +235,7 @@ void _onHome(AsyncWebServerRequest *request) { } #endif -#if SECURE_CLIENT == SECURE_CLIENT_AXTLS & WEB_SSL_ENABLED +#if WEB_SSL_ENABLED int _onCertificate(void * arg, const char *filename, uint8_t **buf) { @@ -446,7 +446,7 @@ void webRequestRegister(web_request_callback_f callback) { } unsigned int webPort() { - #if SECURE_CLIENT == SECURE_CLIENT_AXTLS & WEB_SSL_ENABLED + #if WEB_SSL_ENABLED return 443; #else return getSetting("webPort", WEB_PORT).toInt(); @@ -497,7 +497,7 @@ void webSetup() { _server->onNotFound(_onRequest); // Run server - #if SECURE_CLIENT == SECURE_CLIENT_AXTLS & WEB_SSL_ENABLED + #if WEB_SSL_ENABLED _server->onSslFileRequest(_onCertificate, NULL); _server->beginSecure("server.cer", "server.key", NULL); #else diff --git a/code/platformio.ini b/code/platformio.ini index e0cb1a93..a1ad196b 100644 --- a/code/platformio.ini +++ b/code/platformio.ini @@ -10,12 +10,13 @@ data_dir = espurna/data # We use Arduino Core 2.3.0 (platformIO 1.5.0) as default # # arduino core 2.3.0 = platformIO 1.5.0 -# arduino core 2.4.0 = platformIO 1.6.0 -# arduino core 2.4.1 = platformIO 1.7.3 -# arduino core 2.4.2 = platformIO 1.8.0 -# arduino core 2.5.0 = platformIO 2.0.4 -# arduino core 2.5.1 = platformIO 2.1.1 -# arduino core 2.5.2 = platformIO 2.2.2 +# arduino core 2.4.0 = platformIO 1.6.0 (unsupported) +# arduino core 2.4.1 = platformIO 1.7.3 (unsupported) +# arduino core 2.4.2 = platformIO 1.8.0 (unsupported) +# arduino core 2.5.0 = platformIO 2.0.4 (unsupported) +# arduino core 2.5.1 = platformIO 2.1.1 (unsupported) +# arduino core 2.5.2 = platformIO 2.2.3 (unsupported) +# arduino core 2.6.1 = platformIO 2.3.0 # ------------------------------------------------------------------------------ arduino_core_2_3_0 = espressif8266@1.5.0 arduino_core_2_4_0 = espressif8266@1.6.0