From f78c6332f0eeabf683d496ae3471c85a0b591765 Mon Sep 17 00:00:00 2001 From: Edd Inglis Date: Mon, 11 Apr 2022 12:14:26 +0100 Subject: [PATCH] Permit using the Updater _hash function, even if we don't have a signature appended to the image (#8507) The _hash and _verify functionality of the Updater class are pretty much entwined. But it might be useful to calculate the hash, even without a signature present in the image itself. This change permits that by allowing a zero-length signature. For reference, one possible use case is where the expected hash is provided separately to the uploaded image. Another is to provide generic post-upload validation of the image: the hash function permits a convenient way of inspecting the complete image against arbitrary conditions; this is particularly useful after HTTP client OTA updates. (It's fair that reading the whole image back out of flash is not very efficient, but that's not the concern of this PR.) --- cores/esp8266/Updater.cpp | 46 +++++++++++++++++++++++++-------------- 1 file changed, 30 insertions(+), 16 deletions(-) diff --git a/cores/esp8266/Updater.cpp b/cores/esp8266/Updater.cpp index c1584cf98..c194c1d23 100644 --- a/cores/esp8266/Updater.cpp +++ b/cores/esp8266/Updater.cpp @@ -223,19 +223,29 @@ bool UpdaterClass::end(bool evenIfRemaining){ _size = progress(); } - uint32_t sigLen = 0; if (_verify) { - ESP.flashRead(_startAddress + _size - sizeof(uint32_t), &sigLen, sizeof(uint32_t)); + const uint32_t expectedSigLen = _verify->length(); + // If expectedSigLen is non-zero, we expect the last four bytes of the buffer to + // contain a matching length field, preceded by the bytes of the signature itself. + // But if expectedSigLen is zero, we expect neither a signature nor a length field; + uint32_t sigLen = 0; + + if (expectedSigLen > 0) { + ESP.flashRead(_startAddress + _size - sizeof(uint32_t), &sigLen, sizeof(uint32_t)); + } #ifdef DEBUG_UPDATER DEBUG_UPDATER.printf_P(PSTR("[Updater] sigLen: %d\n"), sigLen); #endif - if (sigLen != _verify->length()) { + if (sigLen != expectedSigLen) { _setError(UPDATE_ERROR_SIGN); _reset(); return false; } - int binSize = _size - sigLen - sizeof(uint32_t) /* The siglen word */; + int binSize = _size; + if (expectedSigLen > 0) { + _size -= (sigLen + sizeof(uint32_t) /* The siglen word */); + } _hash->begin(); #ifdef DEBUG_UPDATER DEBUG_UPDATER.printf_P(PSTR("[Updater] Adjusted binsize: %d\n"), binSize); @@ -254,20 +264,24 @@ bool UpdaterClass::end(bool evenIfRemaining){ for (int i=0; i<_hash->len(); i++) DEBUG_UPDATER.printf(" %02x", ret[i]); DEBUG_UPDATER.printf("\n"); #endif - uint8_t *sig = (uint8_t*)malloc(sigLen); - if (!sig) { - _setError(UPDATE_ERROR_SIGN); - _reset(); - return false; - } - ESP.flashRead(_startAddress + binSize, sig, sigLen); + + uint8_t *sig = nullptr; // Safe to free if we don't actually malloc + if (expectedSigLen > 0) { + sig = (uint8_t*)malloc(sigLen); + if (!sig) { + _setError(UPDATE_ERROR_SIGN); + _reset(); + return false; + } + ESP.flashRead(_startAddress + binSize, sig, sigLen); #ifdef DEBUG_UPDATER - DEBUG_UPDATER.printf_P(PSTR("[Updater] Received Signature:")); - for (size_t i=0; iverify(_hash, (void *)sig, sigLen)) { free(sig); _setError(UPDATE_ERROR_SIGN);