-
-
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
Draft
Copilot
wants to merge
17
commits into
main
Choose a base branch
from
copilot/fix-4929
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Changes from all commits
Commits
Show all changes
17 commits
Select commit
Hold shift + click to select a range
8225a2a
Initial plan
Copilot 54746c9
Implement OTA release compatibility checking system
Copilot e920d2e
Complete OTA release compatibility system with comprehensive testing
Copilot caf3d90
Fix OTA release checking to use flexible string search instead of har…
Copilot fb077ec
Improve OTA release checking with scoring-based candidate selection
Copilot 2d8edfc
Remove build artifacts from repository
Copilot 42ff73f
Implement metadata-based OTA release checking system
Copilot 7d550ba
Replace metadata header approach with ESP-IDF custom description section
Copilot 691c058
Fix runtime release name replacement - move to build-time
Copilot 5c0c84e
Add ESP8266 support to OTA release compatibility system using .ver_nu…
Copilot 18dfc70
Address review feedback: unify structures, remove code duplication, i…
Copilot f706c6c
Rename ignoreRelease to skipValidation in OTA validation system
Copilot 70e8be6
Address review feedback: unify structures, fix C++11 compatibility, i…
Copilot 71ad0d9
Address final review feedback: improve error handling, remove debug d…
Copilot 6676705
Address final review feedback: revert unnecessary files, use F-string…
Copilot 6149842
Fix file reverts and implement efficient metadata validation without …
Copilot 196f579
Implement correct metadata validation logic as requested in review co…
Copilot File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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; | ||
} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,90 @@ | ||
#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 | ||
|
||
// Platform-specific metadata offset in binary file | ||
#ifdef ESP32 | ||
#define METADATA_OFFSET 0 // ESP32: metadata appears at beginning | ||
#elif defined(ESP8266) | ||
#define METADATA_OFFSET 0x1000 // ESP8266: metadata appears at 4KB offset | ||
#endif | ||
|
||
/** | ||
* 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 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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; | ||
} |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.