-
-
Notifications
You must be signed in to change notification settings - Fork 3.8k
Implement Metadata-Based OTA Release Compatibility Checking System #4930
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 15 commits
8225a2a
54746c9
e920d2e
caf3d90
fb077ec
2d8edfc
42ff73f
7d550ba
691c058
5c0c84e
18dfc70
f706c6c
70e8be6
71ad0d9
6676705
6149842
196f579
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please remove unnecessary changes to this file. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Reverted unnecessary changes to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This was not done correctly - I mean before all your modifications, not just the last commit. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Correctly reverted |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,111 @@ | ||
#include "ota_release_check.h" | ||
#include "wled.h" | ||
|
||
#ifdef ESP32 | ||
#include <esp_app_format.h> | ||
#include <esp_ota_ops.h> | ||
#endif | ||
|
||
bool extractWledCustomDesc(const uint8_t* binaryData, size_t dataSize, wled_custom_desc_t* extractedDesc) { | ||
if (!binaryData || !extractedDesc || dataSize < 64) { | ||
return false; | ||
} | ||
|
||
// Search in first 8KB only. This range was chosen because: | ||
// - ESP32 .rodata.wled_desc sections appear early in the binary (typically within first 2-4KB) | ||
// - ESP8266 .ver_number sections also appear early (typically within first 1-2KB) | ||
// - 8KB provides ample coverage for metadata discovery while minimizing processing time | ||
// - Larger firmware files (>1MB) would take significantly longer to process with full search | ||
// - Analysis of typical WLED binary layouts shows metadata appears well within this range | ||
const size_t search_limit = min(dataSize, (size_t)8192); | ||
|
||
for (size_t offset = 0; offset <= search_limit - sizeof(wled_custom_desc_t); offset++) { | ||
const wled_custom_desc_t* custom_desc = (const wled_custom_desc_t*)(binaryData + offset); | ||
|
||
// Check for magic number | ||
if (custom_desc->magic == WLED_CUSTOM_DESC_MAGIC) { | ||
// Found potential match, validate version | ||
if (custom_desc->version != WLED_CUSTOM_DESC_VERSION) { | ||
DEBUG_PRINTF_P(PSTR("Found WLED structure at offset %u but version mismatch: %u\n"), | ||
offset, custom_desc->version); | ||
continue; | ||
} | ||
|
||
// Validate hash using runtime function | ||
uint32_t expected_hash = djb2_hash_runtime(custom_desc->release_name); | ||
if (custom_desc->crc32 != expected_hash) { | ||
DEBUG_PRINTF_P(PSTR("Found WLED structure at offset %u but hash mismatch\n"), offset); | ||
continue; | ||
} | ||
|
||
// Valid structure found - copy entire structure | ||
memcpy(extractedDesc, custom_desc, sizeof(wled_custom_desc_t)); | ||
|
||
DEBUG_PRINTF_P(PSTR("Extracted WLED structure at offset %u: '%s'\n"), | ||
offset, extractedDesc->release_name); | ||
return true; | ||
} | ||
} | ||
|
||
DEBUG_PRINTLN(F("No WLED custom description found in binary")); | ||
return false; | ||
} | ||
|
||
bool validateReleaseCompatibility(const char* extractedRelease) { | ||
if (!extractedRelease) { | ||
return false; | ||
} | ||
|
||
// Ensure extractedRelease is properly null terminated (guard against fixed-length buffer issues) | ||
char safeRelease[WLED_RELEASE_NAME_MAX_LEN]; | ||
strncpy(safeRelease, extractedRelease, WLED_RELEASE_NAME_MAX_LEN - 1); | ||
safeRelease[WLED_RELEASE_NAME_MAX_LEN - 1] = '\0'; | ||
|
||
if (strlen(safeRelease) == 0) { | ||
return false; | ||
} | ||
|
||
// Simple string comparison - releases must match exactly | ||
bool match = strcmp(releaseString, safeRelease) == 0; | ||
|
||
DEBUG_PRINTF_P(PSTR("Release compatibility check: current='%s', uploaded='%s', match=%s\n"), | ||
releaseString, safeRelease, match ? "YES" : "NO"); | ||
|
||
return match; | ||
} | ||
|
||
bool shouldAllowOTA(const uint8_t* binaryData, size_t dataSize, char* errorMessage, size_t errorMessageLen) { | ||
// Clear error message | ||
if (errorMessage && errorMessageLen > 0) { | ||
errorMessage[0] = '\0'; | ||
} | ||
|
||
// Ensure our custom description structure is referenced (prevents optimization) | ||
const wled_custom_desc_t* local_desc = getWledCustomDesc(); | ||
(void)local_desc; // Suppress unused variable warning | ||
|
||
// Try to extract WLED structure directly from binary data | ||
wled_custom_desc_t extractedDesc; | ||
bool hasCustomDesc = extractWledCustomDesc(binaryData, dataSize, &extractedDesc); | ||
|
||
if (!hasCustomDesc) { | ||
// No custom description - this could be a legacy binary | ||
if (errorMessage && errorMessageLen > 0) { | ||
strncpy_P(errorMessage, PSTR("This firmware file is missing compatibility metadata. Enable 'Ignore firmware validation' to proceed anyway."), errorMessageLen - 1); | ||
errorMessage[errorMessageLen - 1] = '\0'; | ||
} | ||
return false; | ||
} | ||
|
||
// Validate compatibility using extracted release name | ||
if (!validateReleaseCompatibility(extractedDesc.release_name)) { | ||
if (errorMessage && errorMessageLen > 0) { | ||
snprintf_P(errorMessage, errorMessageLen, PSTR("Firmware compatibility mismatch: current='%s', uploaded='%s'. Enable 'Ignore firmware validation' to proceed anyway."), | ||
releaseString, extractedDesc.release_name); | ||
errorMessage[errorMessageLen - 1] = '\0'; // Ensure null termination | ||
} | ||
return false; | ||
} | ||
|
||
return true; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,83 @@ | ||
#ifndef WLED_OTA_RELEASE_CHECK_H | ||
#define WLED_OTA_RELEASE_CHECK_H | ||
|
||
/* | ||
* OTA Release Compatibility Checking using ESP-IDF Custom Description Section | ||
* Functions to extract and validate release names from uploaded binary files using embedded metadata | ||
*/ | ||
|
||
#include <Arduino.h> | ||
|
||
#ifdef ESP32 | ||
#include <esp_app_format.h> | ||
#endif | ||
|
||
#define WLED_CUSTOM_DESC_MAGIC 0x57535453 // "WSTS" (WLED System Tag Structure) | ||
#define WLED_CUSTOM_DESC_VERSION 1 | ||
#define WLED_RELEASE_NAME_MAX_LEN 48 | ||
willmmiles marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
/** | ||
* DJB2 hash function (C++11 compatible constexpr) | ||
* Used for compile-time hash computation of release names | ||
*/ | ||
constexpr uint32_t djb2_hash_constexpr(const char* str, uint32_t hash = 5381) { | ||
return (*str == '\0') ? hash : djb2_hash_constexpr(str + 1, ((hash << 5) + hash) + *str); | ||
} | ||
|
||
/** | ||
* Runtime DJB2 hash function for validation | ||
*/ | ||
inline uint32_t djb2_hash_runtime(const char* str) { | ||
uint32_t hash = 5381; | ||
while (*str) { | ||
hash = ((hash << 5) + hash) + *str++; | ||
} | ||
return hash; | ||
} | ||
|
||
/** | ||
* WLED Custom Description Structure | ||
* This structure is embedded in platform-specific sections at a fixed offset | ||
* in ESP32/ESP8266 binaries, allowing extraction without modifying the binary format | ||
*/ | ||
typedef struct { | ||
uint32_t magic; // Magic number to identify WLED custom description | ||
uint32_t version; // Structure version for future compatibility | ||
char release_name[WLED_RELEASE_NAME_MAX_LEN]; // Release name (null-terminated) | ||
uint32_t crc32; // CRC32 of the above fields for integrity check | ||
} __attribute__((packed)) wled_custom_desc_t; | ||
|
||
/** | ||
* Extract WLED custom description structure from binary | ||
* @param binaryData Pointer to binary file data | ||
* @param dataSize Size of binary data in bytes | ||
* @param extractedDesc Buffer to store extracted custom description structure | ||
* @return true if structure was found and extracted, false otherwise | ||
*/ | ||
bool extractWledCustomDesc(const uint8_t* binaryData, size_t dataSize, wled_custom_desc_t* extractedDesc); | ||
|
||
/** | ||
* Validate if extracted release name matches current release | ||
* @param extractedRelease Release name from uploaded binary | ||
* @return true if releases match (OTA should proceed), false if they don't match | ||
*/ | ||
bool validateReleaseCompatibility(const char* extractedRelease); | ||
|
||
/** | ||
* Check if OTA should be allowed based on release compatibility using custom description | ||
* @param binaryData Pointer to binary file data (not modified) | ||
* @param dataSize Size of binary data in bytes | ||
* @param errorMessage Buffer to store error message if validation fails | ||
* @param errorMessageLen Maximum length of error message buffer | ||
* @return true if OTA should proceed, false if it should be blocked | ||
*/ | ||
bool shouldAllowOTA(const uint8_t* binaryData, size_t dataSize, char* errorMessage, size_t errorMessageLen); | ||
|
||
/** | ||
* Get pointer to the embedded custom description structure | ||
* This ensures the structure is referenced and not optimized out | ||
* @return pointer to the custom description structure | ||
*/ | ||
const wled_custom_desc_t* getWledCustomDesc(); | ||
|
||
#endif // WLED_OTA_RELEASE_CHECK_H |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,29 @@ | ||
#include "ota_release_check.h" | ||
#include "wled.h" | ||
|
||
// Platform-specific section definition | ||
#ifdef ESP32 | ||
#define WLED_CUSTOM_DESC_SECTION ".rodata.wled_desc" | ||
#elif defined(ESP8266) | ||
#define WLED_CUSTOM_DESC_SECTION ".ver_number" | ||
#endif | ||
|
||
// Single structure definition for both platforms | ||
const wled_custom_desc_t __attribute__((section(WLED_CUSTOM_DESC_SECTION))) wled_custom_description = { | ||
WLED_CUSTOM_DESC_MAGIC, // magic | ||
WLED_CUSTOM_DESC_VERSION, // version | ||
WLED_RELEASE_NAME, // release_name | ||
djb2_hash_constexpr(WLED_RELEASE_NAME) // crc32 - computed at compile time | ||
}; | ||
|
||
// Compile-time validation that release name doesn't exceed maximum length | ||
static_assert(sizeof(WLED_RELEASE_NAME) <= WLED_RELEASE_NAME_MAX_LEN, | ||
"WLED_RELEASE_NAME exceeds maximum length of WLED_RELEASE_NAME_MAX_LEN characters"); | ||
|
||
// Single reference to ensure it's not optimized away | ||
const wled_custom_desc_t* __attribute__((used)) wled_custom_desc_ref = &wled_custom_description; | ||
|
||
// Function to ensure the structure is referenced by code | ||
const wled_custom_desc_t* getWledCustomDesc() { | ||
return &wled_custom_description; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remove unnecessary changes to this file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reverted unnecessary changes to
pio-scripts/build_ui.py
- file restored to its state before my modifications. (6676705)Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was not done correctly. I do not mean "unnecessary changes from your last commit", I mean "unnecessary changes from main". There should be no changes to this file at all in this PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correctly reverted
pio-scripts/build_ui.py
to its original state before ALL modifications. File now has no changes from main branch. (6149842)