From 7e273879b578694fe8b3d78e8b18554e96da38ab Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 23 Sep 2025 09:45:02 -0500 Subject: [PATCH 1/6] reduce magic numbers --- .../components/esphome/ota/ota_esphome.cpp | 19 ++++++++++--------- esphome/components/esphome/ota/ota_esphome.h | 4 ++-- esphome/espota2.py | 2 +- 3 files changed, 13 insertions(+), 12 deletions(-) diff --git a/esphome/components/esphome/ota/ota_esphome.cpp b/esphome/components/esphome/ota/ota_esphome.cpp index 7fd16ce3d0..6df7144064 100644 --- a/esphome/components/esphome/ota/ota_esphome.cpp +++ b/esphome/components/esphome/ota/ota_esphome.cpp @@ -268,14 +268,14 @@ void ESPHomeOTAComponent::handle_data_() { // TODO: Remove this entire ifdef block in 2026.1.0 if (client_supports_sha256) { sha256::SHA256 sha_hasher; - auth_success = this->perform_hash_auth_(&sha_hasher, this->password_, 16, ota::OTA_RESPONSE_REQUEST_SHA256_AUTH, + auth_success = this->perform_hash_auth_(&sha_hasher, this->password_, ota::OTA_RESPONSE_REQUEST_SHA256_AUTH, LOG_STR("SHA256"), sbuf); } else { #ifdef USE_OTA_MD5 ESP_LOGW(TAG, "Using MD5 auth for compatibility (deprecated)"); md5::MD5Digest md5_hasher; - auth_success = this->perform_hash_auth_(&md5_hasher, this->password_, 8, ota::OTA_RESPONSE_REQUEST_AUTH, - LOG_STR("MD5"), sbuf); + auth_success = + this->perform_hash_auth_(&md5_hasher, this->password_, ota::OTA_RESPONSE_REQUEST_AUTH, LOG_STR("MD5"), sbuf); #endif // USE_OTA_MD5 } #else @@ -286,7 +286,7 @@ void ESPHomeOTAComponent::handle_data_() { goto error; // NOLINT(cppcoreguidelines-avoid-goto) } sha256::SHA256 sha_hasher; - auth_success = this->perform_hash_auth_(&sha_hasher, this->password_, 16, ota::OTA_RESPONSE_REQUEST_SHA256_AUTH, + auth_success = this->perform_hash_auth_(&sha_hasher, this->password_, ota::OTA_RESPONSE_REQUEST_SHA256_AUTH, LOG_STR("SHA256"), sbuf); #endif // ALLOW_OTA_DOWNGRADE_MD5 #else @@ -295,7 +295,7 @@ void ESPHomeOTAComponent::handle_data_() { #ifdef USE_OTA_MD5 md5::MD5Digest md5_hasher; auth_success = - this->perform_hash_auth_(&md5_hasher, this->password_, 8, ota::OTA_RESPONSE_REQUEST_AUTH, LOG_STR("MD5"), sbuf); + this->perform_hash_auth_(&md5_hasher, this->password_, ota::OTA_RESPONSE_REQUEST_AUTH, LOG_STR("MD5"), sbuf); #endif // USE_OTA_MD5 #endif // USE_OTA_SHA256 @@ -529,10 +529,11 @@ void ESPHomeOTAComponent::log_auth_warning_(const LogString *action, const LogSt } // Non-template function definition to reduce binary size -bool ESPHomeOTAComponent::perform_hash_auth_(HashBase *hasher, const std::string &password, size_t nonce_size, - uint8_t auth_request, const LogString *name, char *buf) { +bool ESPHomeOTAComponent::perform_hash_auth_(HashBase *hasher, const std::string &password, uint8_t auth_request, + const LogString *name, char *buf) { // Get sizes from the hasher - const size_t hex_size = hasher->get_size() * 2; // Hex is twice the byte size + const size_t hex_size = hasher->get_size() * 2; // Hex is twice the byte size + const size_t nonce_hex_len = hasher->get_size() / 2; // Nonce hex length is 1/4 of full hex size // Use the provided buffer for all hex operations @@ -552,7 +553,7 @@ bool ESPHomeOTAComponent::perform_hash_auth_(HashBase *hasher, const std::string nonce_bytes[2] = (r1 >> 8) & 0xFF; nonce_bytes[3] = r1 & 0xFF; - if (nonce_size == 8) { + if (nonce_hex_len == 8) { // MD5: 8 chars = "%08x" format = 4 bytes from one random uint32 hasher->add(nonce_bytes, 4); } else { diff --git a/esphome/components/esphome/ota/ota_esphome.h b/esphome/components/esphome/ota/ota_esphome.h index 39f2f878de..5bacb60706 100644 --- a/esphome/components/esphome/ota/ota_esphome.h +++ b/esphome/components/esphome/ota/ota_esphome.h @@ -32,8 +32,8 @@ class ESPHomeOTAComponent : public ota::OTAComponent { void handle_handshake_(); void handle_data_(); #ifdef USE_OTA_PASSWORD - bool perform_hash_auth_(HashBase *hasher, const std::string &password, size_t nonce_size, uint8_t auth_request, - const LogString *name, char *buf); + bool perform_hash_auth_(HashBase *hasher, const std::string &password, uint8_t auth_request, const LogString *name, + char *buf); void log_auth_warning_(const LogString *action, const LogString *hash_name); #endif // USE_OTA_PASSWORD bool readall_(uint8_t *buf, size_t len); diff --git a/esphome/espota2.py b/esphome/espota2.py index 2a4d21dc3e..2712d00127 100644 --- a/esphome/espota2.py +++ b/esphome/espota2.py @@ -166,7 +166,7 @@ def check_error(data: list[int] | bytes, expect: int | list[int] | None) -> None raise OTAError("Error: Authentication invalid. Is the password correct?") if dat == RESPONSE_ERROR_WRITING_FLASH: raise OTAError( - "Error: Wring OTA data to flash memory failed. See USB logs for more " + "Error: Writing OTA data to flash memory failed. See USB logs for more " "information." ) if dat == RESPONSE_ERROR_UPDATE_END: From 307ad1c18bba241f5f1f9fc1a5c65d63bf5c21fb Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 23 Sep 2025 09:46:58 -0500 Subject: [PATCH 2/6] reduce magic numbers --- esphome/components/esphome/ota/ota_esphome.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/esphome/components/esphome/ota/ota_esphome.cpp b/esphome/components/esphome/ota/ota_esphome.cpp index 6df7144064..e56f2a7ab6 100644 --- a/esphome/components/esphome/ota/ota_esphome.cpp +++ b/esphome/components/esphome/ota/ota_esphome.cpp @@ -532,8 +532,8 @@ void ESPHomeOTAComponent::log_auth_warning_(const LogString *action, const LogSt bool ESPHomeOTAComponent::perform_hash_auth_(HashBase *hasher, const std::string &password, uint8_t auth_request, const LogString *name, char *buf) { // Get sizes from the hasher - const size_t hex_size = hasher->get_size() * 2; // Hex is twice the byte size - const size_t nonce_hex_len = hasher->get_size() / 2; // Nonce hex length is 1/4 of full hex size + const size_t hex_size = hasher->get_size() * 2; // Hex is twice the byte size + const size_t nonce_len = hasher->get_size() / 4; // Nonce is 1/4 of hash size in bytes // Use the provided buffer for all hex operations @@ -553,11 +553,11 @@ bool ESPHomeOTAComponent::perform_hash_auth_(HashBase *hasher, const std::string nonce_bytes[2] = (r1 >> 8) & 0xFF; nonce_bytes[3] = r1 & 0xFF; - if (nonce_hex_len == 8) { - // MD5: 8 chars = "%08x" format = 4 bytes from one random uint32 + if (nonce_len == 4) { + // MD5: 4 bytes from one random uint32 hasher->add(nonce_bytes, 4); } else { - // SHA256: 16 chars = "%08x%08x" format = 8 bytes from two random uint32s + // SHA256: 8 bytes from two random uint32s uint32_t r2 = random_uint32(); nonce_bytes[4] = (r2 >> 24) & 0xFF; nonce_bytes[5] = (r2 >> 16) & 0xFF; From 106f8e6804064be1782738fcd7c1e2b827c43cf7 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 23 Sep 2025 09:48:36 -0500 Subject: [PATCH 3/6] dry --- .../components/esphome/ota/ota_esphome.cpp | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/esphome/components/esphome/ota/ota_esphome.cpp b/esphome/components/esphome/ota/ota_esphome.cpp index e56f2a7ab6..039950bee1 100644 --- a/esphome/components/esphome/ota/ota_esphome.cpp +++ b/esphome/components/esphome/ota/ota_esphome.cpp @@ -528,6 +528,14 @@ void ESPHomeOTAComponent::log_auth_warning_(const LogString *action, const LogSt ESP_LOGW(TAG, "Auth: %s %s failed", LOG_STR_ARG(action), LOG_STR_ARG(hash_name)); } +// Helper to convert uint32 to big-endian bytes +static inline void uint32_to_bytes(uint32_t value, uint8_t *bytes) { + bytes[0] = (value >> 24) & 0xFF; + bytes[1] = (value >> 16) & 0xFF; + bytes[2] = (value >> 8) & 0xFF; + bytes[3] = value & 0xFF; +} + // Non-template function definition to reduce binary size bool ESPHomeOTAComponent::perform_hash_auth_(HashBase *hasher, const std::string &password, uint8_t auth_request, const LogString *name, char *buf) { @@ -547,11 +555,7 @@ bool ESPHomeOTAComponent::perform_hash_auth_(HashBase *hasher, const std::string // Generate nonce seed bytes uint32_t r1 = random_uint32(); - // Convert first uint32 to bytes (always needed for MD5) - nonce_bytes[0] = (r1 >> 24) & 0xFF; - nonce_bytes[1] = (r1 >> 16) & 0xFF; - nonce_bytes[2] = (r1 >> 8) & 0xFF; - nonce_bytes[3] = r1 & 0xFF; + uint32_to_bytes(r1, nonce_bytes); if (nonce_len == 4) { // MD5: 4 bytes from one random uint32 @@ -559,10 +563,7 @@ bool ESPHomeOTAComponent::perform_hash_auth_(HashBase *hasher, const std::string } else { // SHA256: 8 bytes from two random uint32s uint32_t r2 = random_uint32(); - nonce_bytes[4] = (r2 >> 24) & 0xFF; - nonce_bytes[5] = (r2 >> 16) & 0xFF; - nonce_bytes[6] = (r2 >> 8) & 0xFF; - nonce_bytes[7] = r2 & 0xFF; + uint32_to_bytes(r2, nonce_bytes + 4); hasher->add(nonce_bytes, 8); } hasher->calculate(); From 7ac0f1c9a2aea7982257b338c1fdf1cc417731f0 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 23 Sep 2025 09:49:03 -0500 Subject: [PATCH 4/6] dry --- esphome/components/esphome/ota/ota_esphome.cpp | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/esphome/components/esphome/ota/ota_esphome.cpp b/esphome/components/esphome/ota/ota_esphome.cpp index 039950bee1..eaffbb0705 100644 --- a/esphome/components/esphome/ota/ota_esphome.cpp +++ b/esphome/components/esphome/ota/ota_esphome.cpp @@ -554,16 +554,14 @@ bool ESPHomeOTAComponent::perform_hash_auth_(HashBase *hasher, const std::string hasher->init(); // Generate nonce seed bytes - uint32_t r1 = random_uint32(); - uint32_to_bytes(r1, nonce_bytes); + uint32_to_bytes(random_uint32(), nonce_bytes); if (nonce_len == 4) { // MD5: 4 bytes from one random uint32 hasher->add(nonce_bytes, 4); } else { // SHA256: 8 bytes from two random uint32s - uint32_t r2 = random_uint32(); - uint32_to_bytes(r2, nonce_bytes + 4); + uint32_to_bytes(random_uint32(), nonce_bytes + 4); hasher->add(nonce_bytes, 8); } hasher->calculate(); From 174cdac5e11158d0ba547954dcc55b7a25ae2420 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 23 Sep 2025 09:49:42 -0500 Subject: [PATCH 5/6] dry --- esphome/components/esphome/ota/ota_esphome.cpp | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/esphome/components/esphome/ota/ota_esphome.cpp b/esphome/components/esphome/ota/ota_esphome.cpp index eaffbb0705..405633b990 100644 --- a/esphome/components/esphome/ota/ota_esphome.cpp +++ b/esphome/components/esphome/ota/ota_esphome.cpp @@ -555,15 +555,10 @@ bool ESPHomeOTAComponent::perform_hash_auth_(HashBase *hasher, const std::string // Generate nonce seed bytes uint32_to_bytes(random_uint32(), nonce_bytes); - - if (nonce_len == 4) { - // MD5: 4 bytes from one random uint32 - hasher->add(nonce_bytes, 4); - } else { - // SHA256: 8 bytes from two random uint32s + if (nonce_len > 4) { uint32_to_bytes(random_uint32(), nonce_bytes + 4); - hasher->add(nonce_bytes, 8); } + hasher->add(nonce_bytes, nonce_len); hasher->calculate(); // Generate and send nonce From f42b523fd918fa679ba18917a7c1163e5b2ccd5f Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 23 Sep 2025 09:54:39 -0500 Subject: [PATCH 6/6] dry --- tests/unit_tests/test_espota2.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit_tests/test_espota2.py b/tests/unit_tests/test_espota2.py index c036a5de8e..bd1a6bde81 100644 --- a/tests/unit_tests/test_espota2.py +++ b/tests/unit_tests/test_espota2.py @@ -149,7 +149,7 @@ def test_receive_exactly_socket_error(mock_socket: Mock) -> None: (espota2.RESPONSE_ERROR_AUTH_INVALID, "Error: Authentication invalid"), ( espota2.RESPONSE_ERROR_WRITING_FLASH, - "Error: Wring OTA data to flash memory failed", + "Error: Writing OTA data to flash memory failed", ), (espota2.RESPONSE_ERROR_UPDATE_END, "Error: Finishing update failed"), (