From ebb225f6c063e1946739cab7674c319a98d66784 Mon Sep 17 00:00:00 2001 From: Bernhard Kirchen Date: Mon, 30 Sep 2024 14:18:33 +0200 Subject: [PATCH 1/2] Fix: avoid deprecated setAuthentication() to fix memory exhaustion with ESPAsyncWebServer 3.3.0, the setAuthentication() method became deprecated and a replacement method was provided which acts as a shim and uses the new middleware-based approach to setup authentication. in order to eventually apply a changed "read-only access allowed" setting, the setAuthentication() method was called periodically. the shim implementation each time allocates a new AuthenticationMiddleware and adds it to the chain of middlewares, eventually exhausting the memory. we now use the new middleware-based approach ourselves and only add the respective AuthenticatonMiddleware instance once to the respective websocket server instance. a regression where enabling unauthenticated read-only access is not applied until reboot is also fixed. all the AuthenticationMiddleware instances were never removed from the chain of middlewares when calling setAuthentication("", ""). --- include/WebApi.h | 1 + include/WebApi_ws_console.h | 2 ++ include/WebApi_ws_live.h | 2 ++ src/WebApi.cpp | 6 ++++++ src/WebApi_security.cpp | 2 ++ src/WebApi_ws_console.cpp | 23 +++++++++++++++++------ src/WebApi_ws_live.cpp | 22 ++++++++++++++++------ 7 files changed, 46 insertions(+), 12 deletions(-) diff --git a/include/WebApi.h b/include/WebApi.h index b6fdbd0..2932f01 100644 --- a/include/WebApi.h +++ b/include/WebApi.h @@ -30,6 +30,7 @@ class WebApiClass { public: WebApiClass(); void init(Scheduler& scheduler); + void reload(); static bool checkCredentials(AsyncWebServerRequest* request); static bool checkCredentialsReadonly(AsyncWebServerRequest* request); diff --git a/include/WebApi_ws_console.h b/include/WebApi_ws_console.h index cf7beec..b319431 100644 --- a/include/WebApi_ws_console.h +++ b/include/WebApi_ws_console.h @@ -8,9 +8,11 @@ class WebApiWsConsoleClass { public: WebApiWsConsoleClass(); void init(AsyncWebServer& server, Scheduler& scheduler); + void reload(); private: AsyncWebSocket _ws; + AuthenticationMiddleware _simpleDigestAuth; Task _wsCleanupTask; void wsCleanupTaskCb(); diff --git a/include/WebApi_ws_live.h b/include/WebApi_ws_live.h index 05f8ab8..e16372e 100644 --- a/include/WebApi_ws_live.h +++ b/include/WebApi_ws_live.h @@ -11,6 +11,7 @@ class WebApiWsLiveClass { public: WebApiWsLiveClass(); void init(AsyncWebServer& server, Scheduler& scheduler); + void reload(); private: static void generateInverterCommonJsonResponse(JsonObject& root, std::shared_ptr inv); @@ -24,6 +25,7 @@ private: void onWebsocketEvent(AsyncWebSocket* server, AsyncWebSocketClient* client, AwsEventType type, void* arg, uint8_t* data, size_t len); AsyncWebSocket _ws; + AuthenticationMiddleware _simpleDigestAuth; uint32_t _lastPublishStats[INV_MAX_COUNT] = { 0 }; diff --git a/src/WebApi.cpp b/src/WebApi.cpp index 1a5b287..117c305 100644 --- a/src/WebApi.cpp +++ b/src/WebApi.cpp @@ -39,6 +39,12 @@ void WebApiClass::init(Scheduler& scheduler) _server.begin(); } +void WebApiClass::reload() +{ + _webApiWsConsole.reload(); + _webApiWsLive.reload(); +} + bool WebApiClass::checkCredentials(AsyncWebServerRequest* request) { CONFIG_T& config = Configuration.get(); diff --git a/src/WebApi_security.cpp b/src/WebApi_security.cpp index ddd8bb5..6be21ca 100644 --- a/src/WebApi_security.cpp +++ b/src/WebApi_security.cpp @@ -71,6 +71,8 @@ void WebApiSecurityClass::onSecurityPost(AsyncWebServerRequest* request) WebApi.writeConfig(retMsg); WebApi.sendJsonResponse(request, response, __FUNCTION__, __LINE__); + + WebApi.reload(); } void WebApiSecurityClass::onAuthenticateGet(AsyncWebServerRequest* request) diff --git a/src/WebApi_ws_console.cpp b/src/WebApi_ws_console.cpp index 1f1efcb..4dd2d69 100644 --- a/src/WebApi_ws_console.cpp +++ b/src/WebApi_ws_console.cpp @@ -21,16 +21,27 @@ void WebApiWsConsoleClass::init(AsyncWebServer& server, Scheduler& scheduler) scheduler.addTask(_wsCleanupTask); _wsCleanupTask.enable(); + + _simpleDigestAuth.setUsername(AUTH_USERNAME); + _simpleDigestAuth.setRealm("console websocket"); + + reload(); +} + +void WebApiWsConsoleClass::reload() +{ + _ws.removeMiddleware(&_simpleDigestAuth); + + auto const& config = Configuration.get(); + + if (config.Security.AllowReadonly) { return; } + + _simpleDigestAuth.setPassword(config.Security.Password); + _ws.addMiddleware(&_simpleDigestAuth); } void WebApiWsConsoleClass::wsCleanupTaskCb() { // see: https://github.com/me-no-dev/ESPAsyncWebServer#limiting-the-number-of-web-socket-clients _ws.cleanupClients(); - - if (Configuration.get().Security.AllowReadonly) { - _ws.setAuthentication("", ""); - } else { - _ws.setAuthentication(AUTH_USERNAME, Configuration.get().Security.Password); - } } diff --git a/src/WebApi_ws_live.cpp b/src/WebApi_ws_live.cpp index 4fa7983..c4b4a1f 100644 --- a/src/WebApi_ws_live.cpp +++ b/src/WebApi_ws_live.cpp @@ -36,18 +36,28 @@ void WebApiWsLiveClass::init(AsyncWebServer& server, Scheduler& scheduler) scheduler.addTask(_sendDataTask); _sendDataTask.enable(); + _simpleDigestAuth.setUsername(AUTH_USERNAME); + _simpleDigestAuth.setRealm("live websocket"); + + reload(); +} + +void WebApiWsLiveClass::reload() +{ + _ws.removeMiddleware(&_simpleDigestAuth); + + auto const& config = Configuration.get(); + + if (config.Security.AllowReadonly) { return; } + + _simpleDigestAuth.setPassword(config.Security.Password); + _ws.addMiddleware(&_simpleDigestAuth); } void WebApiWsLiveClass::wsCleanupTaskCb() { // see: https://github.com/me-no-dev/ESPAsyncWebServer#limiting-the-number-of-web-socket-clients _ws.cleanupClients(); - - if (Configuration.get().Security.AllowReadonly) { - _ws.setAuthentication("", ""); - } else { - _ws.setAuthentication(AUTH_USERNAME, Configuration.get().Security.Password); - } } void WebApiWsLiveClass::sendDataTaskCb() From d5d1a9982fa076e5ec69bd7e1dcc4a768bd5f192 Mon Sep 17 00:00:00 2001 From: Bernhard Kirchen Date: Mon, 30 Sep 2024 15:53:30 +0200 Subject: [PATCH 2/2] Fix: force websocket clients to authenticate when changing the security settings (disabling read-only access or changing the password), existing websocket connections are now closed, forcing the respective clients to authenticate (with the new password). otherwise, existing websocket clients keep connected even though the security settings now expect authentication with a (changed) password. --- src/WebApi_ws_console.cpp | 3 +++ src/WebApi_ws_live.cpp | 3 +++ 2 files changed, 6 insertions(+) diff --git a/src/WebApi_ws_console.cpp b/src/WebApi_ws_console.cpp index 4dd2d69..51035f6 100644 --- a/src/WebApi_ws_console.cpp +++ b/src/WebApi_ws_console.cpp @@ -36,8 +36,11 @@ void WebApiWsConsoleClass::reload() if (config.Security.AllowReadonly) { return; } + _ws.enable(false); _simpleDigestAuth.setPassword(config.Security.Password); _ws.addMiddleware(&_simpleDigestAuth); + _ws.closeAll(); + _ws.enable(true); } void WebApiWsConsoleClass::wsCleanupTaskCb() diff --git a/src/WebApi_ws_live.cpp b/src/WebApi_ws_live.cpp index c4b4a1f..29c204a 100644 --- a/src/WebApi_ws_live.cpp +++ b/src/WebApi_ws_live.cpp @@ -50,8 +50,11 @@ void WebApiWsLiveClass::reload() if (config.Security.AllowReadonly) { return; } + _ws.enable(false); _simpleDigestAuth.setPassword(config.Security.Password); _ws.addMiddleware(&_simpleDigestAuth); + _ws.closeAll(); + _ws.enable(true); } void WebApiWsLiveClass::wsCleanupTaskCb()