From 9ed5a7881899bbf2fb714610e26e4385959720d9 Mon Sep 17 00:00:00 2001 From: Fribur Date: Fri, 5 Jan 2024 10:13:16 -0500 Subject: [PATCH] Reverted changes to PowerLimiter, adapted DNS and mDNS handling in HttpPowerMeter For non IP address URLs, HttpPowerMeter now first tries DNS for resolution as done in WifiClient::connect, and only if that fails tries mDNS. For the latter to work mDNS needs to be enabled in settings. Log in console if mDNS is disabled. String building for Digest authorization still tries to avoid "+" for reasons outlined in https://cpp4arduino.com/2020/02/07/how-to-format-strings-without-the-string-class.html This should also be saver than just concatenating user input strings in preventing format string attacks. https://owasp.org/www-community/attacks/Format_string_attack --- src/HttpPowerMeter.cpp | 136 ++++++++++++++++++++------------------ src/PowerLimiter.cpp | 31 +++------ src/WebApi_powermeter.cpp | 10 +-- 3 files changed, 88 insertions(+), 89 deletions(-) diff --git a/src/HttpPowerMeter.cpp b/src/HttpPowerMeter.cpp index 8bd1fa14b..f58ba0cfb 100644 --- a/src/HttpPowerMeter.cpp +++ b/src/HttpPowerMeter.cpp @@ -3,7 +3,7 @@ #include "HttpPowerMeter.h" #include "MessageOutput.h" #include -#include //saves 20kB to not use FirebaseJson as ArduinoJson is used already elsewhere (e.g. in WebApi_powermeter) +#include #include #include #include @@ -59,18 +59,23 @@ bool HttpPowerMeterClass::queryPhase(int phase, const String& urlProtocol, const //first check if the urlHostname is already an IP adress if (!ipaddr.fromString(urlHostname)) { - //no it is not, so try to resolve the IP adress - const bool mdnsEnabled = Configuration.get().Mdns.Enabled; - if (!mdnsEnabled) { - snprintf_P(httpPowerMeterError, sizeof(httpPowerMeterError), PSTR("Enable mDNS in Network Settings")); - return false; - } + //urlHostname is not an IP address so try to resolve the IP adress + //first, try DNS + if(!WiFiGenericClass::hostByName(urlHostname.c_str(), ipaddr)) + { + //DNS failed, so now try mDNS + const bool mdnsEnabled = Configuration.get().Mdns.Enabled; + if (!mdnsEnabled) { + snprintf_P(httpPowerMeterError, sizeof(httpPowerMeterError), PSTR("Error resolving url %s via DNS, try to enable mDNS in Network Settings"), urlHostname); + return false; + } - ipaddr = MDNS.queryHost(urlHostname); - if (ipaddr == INADDR_NONE){ - snprintf_P(httpPowerMeterError, sizeof(httpPowerMeterError), PSTR("Error resolving url %s"), urlHostname.c_str()); - return false; - } + ipaddr = MDNS.queryHost(urlHostname); + if (ipaddr == INADDR_NONE){ + snprintf_P(httpPowerMeterError, sizeof(httpPowerMeterError), PSTR("Error resolving url %s via DNS and mDNS"), urlHostname.c_str()); + return false; + } + } } // secureWifiClient MUST be created before HTTPClient @@ -84,8 +89,10 @@ bool HttpPowerMeterClass::queryPhase(int phase, const String& urlProtocol, const } else { wifiClient = std::make_unique(); } + return httpRequest(phase, *wifiClient, urlProtocol, ipaddr.toString(), uri, authType, username, password, httpHeader, httpValue, timeout, jsonPath); } + bool HttpPowerMeterClass::httpRequest(int phase, WiFiClient &wifiClient, const String& urlProtocol, const String& urlHostname, const String& uri, Auth authType, const char* username, const char* password, const char* httpHeader, const char* httpValue, uint32_t timeout, const char* jsonPath) { @@ -134,51 +141,53 @@ bool HttpPowerMeterClass::httpRequest(int phase, WiFiClient &wifiClient, const S } String HttpPowerMeterClass::extractParam(String& authReq, const String& param, const char delimit) { - int _begin = authReq.indexOf(param); - if (_begin == -1) { return ""; } - return authReq.substring(_begin + param.length(), authReq.indexOf(delimit, _begin + param.length())); + int _begin = authReq.indexOf(param); + if (_begin == -1) { return ""; } + return authReq.substring(_begin + param.length(), authReq.indexOf(delimit, _begin + param.length())); } + void HttpPowerMeterClass::getcNonce(char* cNounce) { - static const char alphanum[] = "0123456789" + static const char alphanum[] = "0123456789" "ABCDEFGHIJKLMNOPQRSTUVWXYZ" "abcdefghijklmnopqrstuvwxyz"; - auto len=sizeof(cNounce); + auto len=sizeof(cNounce); - for (int i = 0; i < len; ++i) { cNounce[i] = alphanum[rand() % (sizeof(alphanum) - 1)]; } + for (int i = 0; i < len; ++i) { cNounce[i] = alphanum[rand() % (sizeof(alphanum) - 1)]; } } + String HttpPowerMeterClass::getDigestAuth(String& authReq, const String& username, const String& password, const String& method, const String& uri, unsigned int counter) { - // extracting required parameters for RFC 2069 simpler Digest - String realm = extractParam(authReq, "realm=\"", '"'); - String nonce = extractParam(authReq, "nonce=\"", '"'); - char cNonce[8]; - getcNonce(cNonce); - - char nc[9]; - snprintf(nc, sizeof(nc), "%08x", counter); - - // parameters for the Digest - // sha256 of the user:realm:user - char h1Prep[sizeof(username)+sizeof(realm)+sizeof(password)+2]; - snprintf(h1Prep, sizeof(h1Prep), "%s:%s:%s", username.c_str(),realm.c_str(), password.c_str()); - String ha1 = sha256(h1Prep); - - //sha256 of method:uri - char h2Prep[sizeof(method) + sizeof(uri) + 1]; - snprintf(h2Prep, sizeof(h2Prep), "%s:%s", method.c_str(),uri.c_str()); - String ha2 = sha256(h2Prep); - - //md5 of h1:nonce:nc:cNonce:auth:h2 - char responsePrep[sizeof(ha1)+sizeof(nc)+sizeof(cNonce)+4+sizeof(ha2) + 5]; - snprintf(responsePrep, sizeof(responsePrep), "%s:%s:%s:%s:auth:%s", ha1.c_str(),nonce.c_str(), nc, cNonce,ha2.c_str()); - String response = sha256(responsePrep); - - //Final authorization String; - char authorization[17 + sizeof(username) + 10 + sizeof(realm) + 10 + sizeof(nonce) + 8 + sizeof(uri) + 34 + sizeof(nc) + 10 + sizeof(cNonce) + 13 + sizeof(response)]; - snprintf(authorization, sizeof(authorization), "Digest username=\"%s\", realm=\"%s\", nonce=\"%s\", uri=\"%s\", algorithm=SHA-256, qop=auth, nc=%s, cnonce=\"%s\", response=\"%s\"", username.c_str(), realm.c_str(), nonce.c_str(), uri.c_str(), nc, cNonce, response.c_str()); - - return authorization; + // extracting required parameters for RFC 2617 Digest + String realm = extractParam(authReq, "realm=\"", '"'); + String nonce = extractParam(authReq, "nonce=\"", '"'); + char cNonce[8]; + getcNonce(cNonce); + + char nc[9]; + snprintf(nc, sizeof(nc), "%08x", counter); + + // sha256 of the user:realm:user + char h1Prep[1024];//can username+password be longer than 255 chars each? + snprintf(h1Prep, sizeof(h1Prep), "%s:%s:%s", username.c_str(),realm.c_str(), password.c_str()); + String ha1 = sha256(h1Prep); + + //sha256 of method:uri + char h2Prep[1024];//can uri be longer? + snprintf(h2Prep, sizeof(h2Prep), "%s:%s", method.c_str(),uri.c_str()); + String ha2 = sha256(h2Prep); + + //sha256 of h1:nonce:nc:cNonce:auth:h2 + char responsePrep[2048];//can nounce and cNounce be longer? + snprintf(responsePrep, sizeof(responsePrep), "%s:%s:%s:%s:auth:%s", ha1.c_str(),nonce.c_str(), nc, cNonce,ha2.c_str()); + String response = sha256(responsePrep); + + //Final authorization String; + char authorization[2048];//can username+password be longer than 255 chars each? can uri be longer? + snprintf(authorization, sizeof(authorization), "Digest username=\"%s\", realm=\"%s\", nonce=\"%s\", uri=\"%s\", algorithm=SHA-256, qop=auth, nc=%s, cnonce=\"%s\", response=\"%s\"", username.c_str(), realm.c_str(), nonce.c_str(), uri.c_str(), nc, cNonce, response.c_str()); + + return authorization; } + bool HttpPowerMeterClass::tryGetFloatValueForPhase(int phase, int httpCode, const char* jsonPath) { bool success = false; @@ -201,6 +210,7 @@ bool HttpPowerMeterClass::tryGetFloatValueForPhase(int phase, int httpCode, cons } return success; } + void HttpPowerMeterClass::extractUrlComponents(const String& url, String& protocol, String& hostname, String& uri) { // Find protocol delimiter int protocolEndIndex = url.indexOf(":"); @@ -234,23 +244,23 @@ void HttpPowerMeterClass::extractUrlComponents(const String& url, String& protoc #define HASH_SIZE 32 String HttpPowerMeterClass::sha256(const String& data) { - SHA256 sha256; - uint8_t hash[HASH_SIZE]; - - sha256.reset(); - sha256.update(data.c_str(), data.length()); - sha256.finalize(hash, HASH_SIZE); - - String hashStr = ""; - for (int i = 0; i < HASH_SIZE; i++) { - String hex = String(hash[i], HEX); - if (hex.length() == 1) { - hashStr += "0"; + SHA256 sha256; + uint8_t hash[HASH_SIZE]; + + sha256.reset(); + sha256.update(data.c_str(), data.length()); + sha256.finalize(hash, HASH_SIZE); + + String hashStr = ""; + for (int i = 0; i < HASH_SIZE; i++) { + String hex = String(hash[i], HEX); + if (hex.length() == 1) { + hashStr += "0"; + } + hashStr += hex; } - hashStr += hex; - } - return hashStr; + return hashStr; } void HttpPowerMeterClass::prepareRequest(uint32_t timeout, const char* httpHeader, const char* httpValue) { httpClient.setFollowRedirects(HTTPC_STRICT_FOLLOW_REDIRECTS); diff --git a/src/PowerLimiter.cpp b/src/PowerLimiter.cpp index 56d732570..86cb2751b 100644 --- a/src/PowerLimiter.cpp +++ b/src/PowerLimiter.cpp @@ -187,8 +187,7 @@ void PowerLimiterClass::loop() // a calculated power limit will always be limited to the reported // device's max power. that upper limit is only known after the first // DevInfoSimpleCommand succeeded. - auto maxPower = _inverter->DevInfo()->getMaxPower(); - if (maxPower <= 0) { + if (_inverter->DevInfo()->getMaxPower() <= 0) { return announceStatus(Status::InverterDevInfoPending); } @@ -199,13 +198,11 @@ void PowerLimiterClass::loop() // the normal mode of operation requires a valid // power meter reading to calculate a power limit - if (!config.PowerMeter.Enabled) { - shutdown(Status::PowerMeterDisabled); + if (!config.PowerMeter.Enabled) { + shutdown(Status::PowerMeterDisabled); return; } - //instead of shutting down on PowerMeterTimeout, how about setting alternativly to a safe "low production" mode? - //Could be usefull when PowerMeter fails but we know for sure house consumption will never fall below a certain limit (say 200W) if (millis() - PowerMeter.getLastPowerMeterUpdate() > (30 * 1000)) { shutdown(Status::PowerMeterTimeout); return; @@ -225,7 +222,6 @@ void PowerLimiterClass::loop() if (_inverter->Statistics()->getLastUpdate() <= settlingEnd) { return announceStatus(Status::InverterStatsPending); } - if (PowerMeter.getLastPowerMeterUpdate() <= settlingEnd) { return announceStatus(Status::PowerMeterPending); @@ -549,8 +545,8 @@ bool PowerLimiterClass::setNewPowerLimit(std::shared_ptr inver dcTotalChnls, dcProdChnls); effPowerLimit = round(effPowerLimit * static_cast(dcTotalChnls) / dcProdChnls); } - auto maxPower = inverter->DevInfo()->getMaxPower(); - effPowerLimit = std::min(effPowerLimit, maxPower); + + effPowerLimit = std::min(effPowerLimit, inverter->DevInfo()->getMaxPower()); // Check if the new value is within the limits of the hysteresis auto diff = std::abs(effPowerLimit - _lastRequestedPowerLimit); @@ -560,23 +556,16 @@ bool PowerLimiterClass::setNewPowerLimit(std::shared_ptr inver // staleness in case a power limit update was not received by the inverter. auto ageMillis = millis() - _lastPowerLimitMillis; - //instead pushing limit to inverter every 60 seconds no matter what, - //why not query instead the currenty configured limit...and do nothing if not needed - int currentLimit = round(inverter->SystemConfigPara()->getLimitPercent() * maxPower / 100); - auto currentDiff = std::abs(effPowerLimit - currentLimit ); - - if (diff < hysteresis && currentDiff < hysteresis ){ - //if (diff < hysteresis && ageMillis < 60 * 1000) { - //MessageOutput.printf("Keep limit: %d W, current limit %d W\r\n", effPowerLimit, currentLimit); + if (diff < hysteresis && ageMillis < 60 * 1000) { if (_verboseLogging) { - MessageOutput.printf("[DPL::setNewPowerLimit] Keep current limit. (new calculated: %d W, last limit: %d W, diff: %d W, hysteresis: %d W, age: %ld ms)\r\n", - effPowerLimit, _lastRequestedPowerLimit, diff, hysteresis, ageMillis); + MessageOutput.printf("[DPL::setNewPowerLimit] requested: %d W, last limit: %d W, diff: %d W, hysteresis: %d W, age: %ld ms\r\n", + newPowerLimit, _lastRequestedPowerLimit, diff, hysteresis, ageMillis); } return false; } - //if we end up here, it we will set new limit + if (_verboseLogging) { - MessageOutput.printf("[DPL::setNewPowerLimit] requested: %d W, sending limit: %d W\r\n", + MessageOutput.printf("[DPL::setNewPowerLimit] requested: %d W, (re-)sending limit: %d W\r\n", newPowerLimit, effPowerLimit); } diff --git a/src/WebApi_powermeter.cpp b/src/WebApi_powermeter.cpp index 83b2eda5c..d5958668f 100644 --- a/src/WebApi_powermeter.cpp +++ b/src/WebApi_powermeter.cpp @@ -203,11 +203,11 @@ void WebApiPowerMeterClass::onAdminPost(AsyncWebServerRequest* request) response->setLength(); request->send(response); - // why reboot..WebApi_powerlimiter is also not rebooting - // yield(); - // delay(1000); - // yield(); - // ESP.restart(); + // reboot requiered as per https://github.com/helgeerbe/OpenDTU-OnBattery/issues/565#issuecomment-1872552559 + yield(); + delay(1000); + yield(); + ESP.restart(); } void WebApiPowerMeterClass::onTestHttpRequest(AsyncWebServerRequest* request)