From 2f853c1311235db4532222c3a69a5d7598472c65 Mon Sep 17 00:00:00 2001 From: N Vignesh Reddy Date: Wed, 2 Sep 2026 10:04:37 +0000 Subject: [PATCH 1/3] fix(rollback): bind the TLV area to the signed image header eos_rollback_read_image_counter() takes the image's anti-rollback security counter from the EOS_TLV_MIN_SEC_VER entry in the TLV area, which sits at image_addr + hdr_size + image_size -- immediately after the payload. Nothing covered those bytes. eos_image_verify_signature() signs header[0, EOS_IMG_SIGNED_LEN) and eos_image_verify_integrity() hashes [hdr_size, hdr_size + image_size); the TLV area is disjoint from both. An attacker able to write flash could take a genuinely signed *old* image, rewrite four bytes of TLV to raise its declared counter above the device floor, and boot it -- the exact downgrade anti-rollback exists to prevent -- without disturbing a byte the signature or the payload hash covers. Carve the 30 formerly reserved header bytes (offsets 62..91, inside the signed prefix) into tlv_len and a 224-bit truncated SHA-256 of the TLV area. Every other field keeps the offset the signing tools already address and the header stays 156 bytes on the wire. rollback.c now hashes the declared area and refuses to read a counter from one that does not match; an image declaring tlv_len == 0 reports counter 0, which can only fail against the device floor, never raise it. Also: eos_image_tlv.h documented the layout as [header][TLV][payload], which contradicts both the parser and the loader; correct it to match the code. Drive-by, both needed to run the suite that proves this: - tests/production_test_suite.py hard-coded /home/ubuntu/eBoot and could not run anywhere else; derive the repo root from __file__. - core/sha512.c was missing its SPDX header (SA-6.6). Verified: ctest 38/38 (17 valgrind-clean), pytest 33/33, production_test_suite.py down to one pre-existing failure (SA-6.9, CI sanitizer job -- out of scope here). Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01MSSnU9cmWFkxan4G57J4VV --- core/rollback.c | 69 ++++++++ core/sha512.c | 3 + include/eos_image.h | 34 +++- include/eos_image_tlv.h | 16 +- tests/CMakeLists.txt | 8 +- tests/production_test_suite.py | 6 +- tests/unit/test_image_abi.c | 3 +- tests/unit/test_image_verify.c | 3 +- tests/unit/test_slot_manager.c | 8 +- tests/unit/test_tlv_auth.c | 309 +++++++++++++++++++++++++++++++++ tools/eos_sign.py | 2 +- tools/imgpack.py | 2 +- 12 files changed, 444 insertions(+), 19 deletions(-) create mode 100644 tests/unit/test_tlv_auth.c diff --git a/core/rollback.c b/core/rollback.c index 35810ef..39405e7 100644 --- a/core/rollback.c +++ b/core/rollback.c @@ -10,12 +10,52 @@ #include "eos_rollback.h" #include "eos_image.h" #include "eos_image_tlv.h" +#include "eos_crypto_boot.h" #include "eos_hal.h" #include static uint32_t g_staged_counter; static bool g_staged_valid; +/** + * Check that the TLV area at @p tlv_addr is the one the signed header vouches + * for, by hashing hdr->tlv_len bytes of it and comparing against hdr->tlv_hash. + * + * Streams the area in small chunks rather than buffering it, so the stack cost + * does not scale with EOS_TLV_MAX_SIZE. + */ +static int tlv_area_matches_header(const eos_image_header_t *hdr, + uint32_t tlv_addr) +{ + eos_sha256_ctx_t sha; + uint8_t chunk[64]; + uint8_t digest[EOS_SHA256_DIGEST_SIZE]; + uint32_t remaining = hdr->tlv_len; + uint32_t addr = tlv_addr; + + eos_sha256_init(&sha); + + while (remaining > 0) { + uint32_t n = (remaining > sizeof(chunk)) ? (uint32_t)sizeof(chunk) + : remaining; + if (eos_hal_flash_read(addr, chunk, n) != EOS_OK) + return EOS_ERR_FLASH; + eos_sha256_update(&sha, chunk, n); + addr += n; + remaining -= n; + } + + eos_sha256_final(&sha, digest); + + /* Constant-time: this compares attacker-influenced bytes, and a bootloader + * offers unlimited retries to anyone with a logic analyser. */ + volatile uint8_t diff = 0; + for (uint32_t i = 0; i < EOS_IMG_TLV_HASH_LEN; i++) + diff |= (uint8_t)(digest[i] ^ hdr->tlv_hash[i]); + + return (diff == 0) ? EOS_OK : EOS_ERR_INVALID; +} + int eos_rollback_read_image_counter(uint32_t image_addr, uint32_t *counter_out) { if (!counter_out) return EOS_ERR_INVALID; @@ -33,6 +73,35 @@ int eos_rollback_read_image_counter(uint32_t image_addr, uint32_t *counter_out) if (tlv_addr + hdr.image_size < tlv_addr) return EOS_ERR_INVALID; tlv_addr += hdr.image_size; + /* Nothing else in the image covers those bytes: the signature stops at + * EOS_IMG_SIGNED_LEN and hash[] stops after image_size payload bytes. An + * attacker able to write flash could otherwise take a genuinely signed old + * image, raise its declared counter, and walk it straight past + * eos_rollback_verify() — the downgrade anti-rollback exists to prevent. + * + * hdr.tlv_len and hdr.tlv_hash are inside the signed prefix, so they are + * the only place a claim about this area can be trusted from. + * + * tlv_len == 0 means the image makes no such claim. The area is then + * unauthenticated and is not read: reporting 0 is the conservative + * reading, since a counter of 0 can only fail against the device floor, + * never raise it. */ + if (hdr.tlv_len == 0) + return EOS_OK; + + if (hdr.tlv_len < sizeof(eos_tlv_info_t) || hdr.tlv_len > EOS_TLV_MAX_SIZE) + return EOS_ERR_INVALID; + + if (tlv_addr + hdr.tlv_len < tlv_addr) + return EOS_ERR_INVALID; + + /* The header claims an authenticated area and the bytes do not match it. + * That is tampering, not absence — fail closed rather than degrading to 0, + * so the condition is reported instead of passing silently. */ + rc = tlv_area_matches_header(&hdr, tlv_addr); + if (rc != EOS_OK) + return rc; + eos_tlv_ctx_t ctx; rc = eos_tlv_parse(&ctx, tlv_addr); if (rc == EOS_ERR_NOT_FOUND) return EOS_OK; diff --git a/core/sha512.c b/core/sha512.c index 5510e47..d9aa57c 100644 --- a/core/sha512.c +++ b/core/sha512.c @@ -1,3 +1,6 @@ +// SPDX-License-Identifier: MIT +// Copyright (c) 2026 EoS Project + #include "eos_crypto_boot.h" #include diff --git a/include/eos_image.h b/include/eos_image.h index a2a9d18..c7763bc 100644 --- a/include/eos_image.h +++ b/include/eos_image.h @@ -22,6 +22,22 @@ extern "C" { /* ---------------- Image Header ---------------- */ +/** + * @brief Bytes of the TLV area's SHA-256 digest carried in the header. + * + * The TLV area sits after the payload, so neither the signature (which covers + * the header prefix) nor hash[] (which covers exactly image_size payload bytes) + * reaches it. tlv_len and tlv_hash do, and they are inside the signed prefix — + * that is what makes a TLV-declared value such as EOS_TLV_MIN_SEC_VER + * trustworthy enough to gate anti-rollback on. + * + * The digest is truncated to 224 bits so the pair fits the 30 bytes previously + * reserved between sig_len and signature[], leaving every other field at the + * offset the signing tools already address. Second-preimage resistance at 224 + * bits is far beyond what an attacker rewriting a ~64-byte TLV blob can reach. + */ +#define EOS_IMG_TLV_HASH_LEN 28 + typedef struct { uint32_t magic; /* EOS_IMG_MAGIC */ uint16_t hdr_version; /* Header format version */ @@ -34,7 +50,8 @@ typedef struct { uint8_t hash[EOS_HASH_SIZE]; /* SHA-256 hash of payload */ uint8_t sig_type; /* eos_sig_type_t */ uint8_t sig_len; /* Actual signature length */ - uint8_t reserved[30]; /* Reserved for future use */ + uint16_t tlv_len; /* Bytes of TLV area following the payload; 0 = none */ + uint8_t tlv_hash[EOS_IMG_TLV_HASH_LEN]; /* Truncated SHA-256 of that area */ uint8_t signature[EOS_SIG_MAX_SIZE]; /* Digital signature */ } eos_image_header_t; @@ -51,7 +68,7 @@ typedef struct { * * Everything except the signature field itself: magic, hdr_version, hdr_size, * image_size, load_addr, entry_addr, image_version, flags, hash, sig_type, - * sig_len and reserved. + * sig_len, tlv_len and tlv_hash. * * Signing hash[] alone leaves every other field unauthenticated. An attacker * could keep a legitimately signed image's signature and still change the load @@ -77,8 +94,17 @@ EOS_IMG_STATIC_ASSERT(offsetof(eos_image_header_t, hash) == 28, "hash[] must stay at offset 28"); EOS_IMG_STATIC_ASSERT(offsetof(eos_image_header_t, sig_type) == 60, "sig_type must stay at offset 60"); +EOS_IMG_STATIC_ASSERT(offsetof(eos_image_header_t, tlv_len) == 62, + "tlv_len must occupy the first 2 of the 30 formerly " + "reserved bytes"); +EOS_IMG_STATIC_ASSERT(offsetof(eos_image_header_t, tlv_hash) == 64, + "tlv_hash must occupy the remaining 28 reserved bytes"); EOS_IMG_STATIC_ASSERT(offsetof(eos_image_header_t, signature) == 92, "signature[] must stay at offset 92"); +EOS_IMG_STATIC_ASSERT(offsetof(eos_image_header_t, tlv_hash) + + EOS_IMG_TLV_HASH_LEN == + offsetof(eos_image_header_t, signature), + "the TLV binding must be inside the signed prefix"); /* ---------------- Image Validation API ---------------- */ @@ -93,8 +119,8 @@ int eos_image_parse_header(uint32_t addr, eos_image_header_t *out); /** * @brief Verify image integrity using CRC32 or hash. * @param hdr Parsed image header. - * @param addr Flash address of the image (header base). Payload starts after - * hdr_size and an optional TLV area, matching eos_sign.py. + * @param addr Flash address of the image (header base). The payload starts at + * addr + hdr_size; any TLV area follows it, not precedes it. * @return EOS_OK if integrity check passes, EOS_ERR_CRC on failure. */ int eos_image_verify_integrity(const eos_image_header_t *hdr, uint32_t addr); diff --git a/include/eos_image_tlv.h b/include/eos_image_tlv.h index 2bc858b..cf8bd1e 100644 --- a/include/eos_image_tlv.h +++ b/include/eos_image_tlv.h @@ -6,12 +6,22 @@ * @file eos_image_tlv.h * @brief TLV (Type-Length-Value) metadata for firmware images * - * mcuboot-inspired TLV area appended after the fixed image header. - * TLVs are covered by the image signature for tamper protection. + * mcuboot-inspired TLV area carrying extensible image metadata. * * Layout: - * [image_header][TLV area][payload] + * [image_header][payload][TLV area] * TLV area = [tlv_info][tlv_entry_0][tlv_entry_1]... + * + * The TLV area follows the payload: image_verify.c reads the payload at + * hdr_size and rollback.c looks for the TLV info header at + * hdr_size + image_size. (This block used to document the opposite order, + * which matched neither.) + * + * These bytes are covered by NEITHER the signature (which stops at + * EOS_IMG_SIGNED_LEN) NOR hash[] (which covers exactly image_size payload + * bytes). An image that needs its TLVs trusted -- EOS_TLV_MIN_SEC_VER gates + * anti-rollback -- must bind them through eos_image_header_t::tlv_len and + * ::tlv_hash, which do sit inside the signed prefix. */ #ifndef EOS_IMAGE_TLV_H diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index f221ba2..2e0284d 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -91,6 +91,11 @@ add_executable(eboot_test_rollback unit/test_rollback.c) target_link_libraries(eboot_test_rollback PRIVATE eboot_core) add_test(NAME test_rollback COMMAND eboot_test_rollback) +# --- test_tlv_auth: TLV area must be bound to the signed header --- +add_executable(eboot_test_tlv_auth unit/test_tlv_auth.c) +target_link_libraries(eboot_test_tlv_auth PRIVATE eboot_core) +add_test(NAME test_tlv_auth COMMAND eboot_test_tlv_auth) + # --- test_storage: Unified storage abstraction --- add_executable(eboot_test_storage unit/test_storage.c) target_link_libraries(eboot_test_storage PRIVATE eboot_core) @@ -109,7 +114,8 @@ if(VALGRIND) test_device_table test_runtime_svc test_board_config test_multicore test_board_registry test_slot_manager test_boot_log test_image_verify test_image_abi - test_recovery test_slot_size_bounds test_fw_transport) + test_recovery test_slot_size_bounds test_fw_transport + test_tlv_auth) add_test( NAME valgrind_${TEST_NAME} COMMAND ${VALGRIND} ${VALGRIND_OPTS} $ diff --git a/tests/production_test_suite.py b/tests/production_test_suite.py index 8082ac8..01e535e 100644 --- a/tests/production_test_suite.py +++ b/tests/production_test_suite.py @@ -147,7 +147,7 @@ def make_image_header( hdr += hash_bytes[:EOS_HASH_SIZE] # 32 hdr += struct.pack('= 83 (actual: {board_count})", board_count >= 83, f"got {board_count}") diff --git a/tests/unit/test_image_abi.c b/tests/unit/test_image_abi.c index 375878b..c9726ad 100644 --- a/tests/unit/test_image_abi.c +++ b/tests/unit/test_image_abi.c @@ -71,7 +71,8 @@ int main(void) CHECK_FIELD(hash, 28, 32); CHECK_FIELD(sig_type, 60, 1); CHECK_FIELD(sig_len, 61, 1); - CHECK_FIELD(reserved, 62, 30); + CHECK_FIELD(tlv_len, 62, 2); + CHECK_FIELD(tlv_hash, 64, 28); CHECK_FIELD(signature, 92, 64); /* No padding anywhere, and none on the end. The struct is written to flash diff --git a/tests/unit/test_image_verify.c b/tests/unit/test_image_verify.c index 9e44af9..c275a4f 100644 --- a/tests/unit/test_image_verify.c +++ b/tests/unit/test_image_verify.c @@ -328,7 +328,8 @@ TEST(test_signed_region_covers_all_metadata) COVERED(hash); COVERED(sig_type); COVERED(sig_len); - COVERED(reserved); + COVERED(tlv_len); + COVERED(tlv_hash); #undef COVERED /* The signature itself is the only thing outside it. */ diff --git a/tests/unit/test_slot_manager.c b/tests/unit/test_slot_manager.c index ec12af6..5bca305 100644 --- a/tests/unit/test_slot_manager.c +++ b/tests/unit/test_slot_manager.c @@ -121,7 +121,7 @@ static int slot_index(uint32_t addr) /* ---- Image verification mocks (override eboot_core's real ones) ---- * * verify_slot() passes the parsed header straight to the integrity and - * signature checks, so the mocks stash the slot index in reserved[0] on + * signature checks, so the mocks stash the slot index in tlv_hash[0] on * parse and read it back to decide which scripted result to return. */ int eos_image_parse_header(uint32_t addr, eos_image_header_t *out) @@ -133,7 +133,7 @@ int eos_image_parse_header(uint32_t addr, eos_image_header_t *out) memset(out, 0, sizeof(*out)); out->magic = EOS_IMG_MAGIC; out->image_version = slot_version[slot]; - out->reserved[0] = (uint8_t)slot; + out->tlv_hash[0] = (uint8_t)slot; return EOS_OK; } @@ -146,8 +146,8 @@ int eos_image_verify_integrity(const eos_image_header_t *hdr, uint32_t addr) int eos_image_verify_signature(const eos_image_header_t *hdr) { - if (!hdr || hdr->reserved[0] > EOS_SLOT_B) return EOS_ERR_INVALID; - return signature_result[hdr->reserved[0]]; + if (!hdr || hdr->tlv_hash[0] > EOS_SLOT_B) return EOS_ERR_INVALID; + return signature_result[hdr->tlv_hash[0]]; } /* ---- Test harness ---- */ diff --git a/tests/unit/test_tlv_auth.c b/tests/unit/test_tlv_auth.c new file mode 100644 index 0000000..28c0e41 --- /dev/null +++ b/tests/unit/test_tlv_auth.c @@ -0,0 +1,309 @@ +// SPDX-License-Identifier: MIT +// Copyright (c) 2026 EoS Project +// ISO/IEC 25000 | ISO/IEC/IEEE 15288:2023 + +/** + * @file test_tlv_auth.c + * @brief Regression test: the TLV area feeding anti-rollback must be + * authenticated by the signed image header. + * + * eos_rollback_read_image_counter() takes the image's security counter from + * the EOS_TLV_MIN_SEC_VER entry in the TLV area, which lives at + * image_addr + hdr_size + image_size + * i.e. immediately after the payload. + * + * Nothing used to cover those bytes: + * - eos_image_verify_signature() signs header[0, EOS_IMG_SIGNED_LEN) = [0,92) + * - eos_image_verify_integrity() hashes [hdr_size, hdr_size + image_size) + * + * The TLV area is disjoint from both. An attacker able to write flash could + * therefore take a genuinely signed *old* image, raise its declared counter, + * and walk it past eos_rollback_verify() — defeating the anti-rollback gate + * that exists precisely to stop that downgrade — without disturbing a single + * byte the signature or the payload hash covers. + * + * The fix binds the TLV area to the signed header via hdr.tlv_len and + * hdr.tlv_hash (both inside the signed prefix). This test pins that: + * 1. an authenticated TLV area is still read normally; + * 2. tampering with it is now detected and fails closed; + * 3. an image that declares no authenticated TLV area reports counter 0 + * rather than trusting whatever bytes happen to follow the payload; + * 4. the tampering in (2) really is invisible to signature/integrity + * coverage, so the header binding is the only thing that catches it. + */ + +#include "eos_image.h" +#include "eos_image_tlv.h" +#include "eos_rollback.h" +#include "eos_crypto_boot.h" +#include "eos_hal.h" +#include +#include +#include +#include + +#define SIM_FLASH_SIZE (64 * 1024) +#define SLOT_A_ADDR 0x1000u +#define SLOT_A_SIZE 0x8000u +#define PAYLOAD_SIZE 256u + +static uint8_t sim_flash[SIM_FLASH_SIZE]; + +static int sim_flash_read(uint32_t addr, void *buf, size_t len) +{ + if (addr + len > SIM_FLASH_SIZE) return EOS_ERR_FLASH; + memcpy(buf, &sim_flash[addr], len); + return EOS_OK; +} + +static int sim_flash_write(uint32_t addr, const void *buf, size_t len) +{ + if (addr + len > SIM_FLASH_SIZE) return EOS_ERR_FLASH; + memcpy(&sim_flash[addr], buf, len); + return EOS_OK; +} + +static int sim_flash_erase(uint32_t addr, size_t len) +{ + if (addr + len > SIM_FLASH_SIZE) return EOS_ERR_FLASH; + memset(&sim_flash[addr], 0xFF, len); + return EOS_OK; +} + +static uint32_t sim_counter = 0; +static int sim_monotonic_read(uint32_t *value) +{ + if (!value) return EOS_ERR_INVALID; + *value = sim_counter; + return EOS_OK; +} + +static const eos_board_ops_t sim_ops = { + .flash_base = 0, + .flash_size = SIM_FLASH_SIZE, + .slot_a_addr = SLOT_A_ADDR, + .slot_a_size = SLOT_A_SIZE, + .flash_read = sim_flash_read, + .flash_write = sim_flash_write, + .flash_erase = sim_flash_erase, + .monotonic_read = sim_monotonic_read, +}; + +static int tests_run = 0; +static int tests_passed = 0; + +#define TEST(name) \ + static void name(void); \ + static void run_##name(void) { \ + memset(sim_flash, 0xFF, sizeof(sim_flash)); \ + sim_counter = 0; \ + eos_hal_init(&sim_ops); \ + printf(" %-58s ", #name); \ + name(); \ + tests_passed++; \ + printf("[PASS]\n"); \ + } \ + static void name(void) + +#define ASSERT(cond) do { \ + if (!(cond)) { \ + printf("[FAIL] %s:%d: %s\n", __FILE__, __LINE__, #cond); \ + exit(1); \ + } \ +} while(0) + +/* ------------------------------------------------------------------ + * Image construction + * ------------------------------------------------------------------ */ + +/* Byte offset of the TLV area, and of the MIN_SEC_VER value inside it. */ +static uint32_t tlv_offset(void) +{ + return SLOT_A_ADDR + sizeof(eos_image_header_t) + PAYLOAD_SIZE; +} + +/* [tlv_info(4)][entry_hdr(4)][uint32 value] */ +#define TLV_AREA_LEN (uint16_t)(sizeof(eos_tlv_info_t) + \ + sizeof(eos_tlv_entry_hdr_t) + sizeof(uint32_t)) +#define TLV_VALUE_OFF (uint32_t)(sizeof(eos_tlv_info_t) + sizeof(eos_tlv_entry_hdr_t)) + +/** + * Lay a well-formed image into the simulated flash. + * + * @param sec_ver value written into the MIN_SEC_VER TLV + * @param emit_tlv write a TLV area after the payload at all + * @param bind_tlv record tlv_len/tlv_hash in the header, i.e. sign it + */ +static void build_image(uint32_t sec_ver, bool emit_tlv, bool bind_tlv) +{ + uint8_t payload[PAYLOAD_SIZE]; + for (uint32_t i = 0; i < PAYLOAD_SIZE; i++) + payload[i] = (uint8_t)(i * 7u + 1u); + + eos_image_header_t hdr; + memset(&hdr, 0, sizeof(hdr)); + hdr.magic = EOS_IMG_MAGIC; + hdr.hdr_version = EOS_IMAGE_HDR_VERSION; + hdr.hdr_size = (uint16_t)sizeof(eos_image_header_t); + hdr.image_size = PAYLOAD_SIZE; + hdr.load_addr = 0; /* skips the entry_addr range check */ + hdr.entry_addr = 0; + hdr.image_version = 0x00010000u; + hdr.flags = EOS_IMG_FLAG_HASH_SHA256; + eos_sha256(payload, PAYLOAD_SIZE, hdr.hash); + hdr.sig_type = EOS_SIG_ED25519; + hdr.sig_len = EOS_SIG_MAX_SIZE; + + uint8_t tlv[TLV_AREA_LEN]; + if (emit_tlv) { + eos_tlv_info_t info = { EOS_TLV_INFO_MAGIC, TLV_AREA_LEN }; + eos_tlv_entry_hdr_t ent = { EOS_TLV_MIN_SEC_VER, sizeof(uint32_t) }; + memcpy(tlv, &info, sizeof(info)); + memcpy(tlv + sizeof(info), &ent, sizeof(ent)); + memcpy(tlv + TLV_VALUE_OFF, &sec_ver, sizeof(sec_ver)); + + if (bind_tlv) { + uint8_t digest[EOS_SHA256_DIGEST_SIZE]; + eos_sha256(tlv, TLV_AREA_LEN, digest); + hdr.tlv_len = TLV_AREA_LEN; + memcpy(hdr.tlv_hash, digest, EOS_IMG_TLV_HASH_LEN); + } + } + + memcpy(&sim_flash[SLOT_A_ADDR], &hdr, sizeof(hdr)); + memcpy(&sim_flash[SLOT_A_ADDR + sizeof(hdr)], payload, PAYLOAD_SIZE); + if (emit_tlv) + memcpy(&sim_flash[tlv_offset()], tlv, TLV_AREA_LEN); +} + +/* Rewrite the MIN_SEC_VER value in place, exactly as an attacker with flash + * write access would: no header byte and no payload byte is touched. */ +static void tamper_sec_ver(uint32_t new_value) +{ + memcpy(&sim_flash[tlv_offset() + TLV_VALUE_OFF], &new_value, sizeof(new_value)); +} + +/* ------------------------------------------------------------------ + * Tests + * ------------------------------------------------------------------ */ + +TEST(test_authenticated_tlv_counter_is_read) +{ + build_image(7, true, true); + + uint32_t counter = 0xDEADBEEF; + ASSERT(eos_rollback_read_image_counter(SLOT_A_ADDR, &counter) == EOS_OK); + ASSERT(counter == 7); +} + +TEST(test_tampered_tlv_counter_is_rejected) +{ + build_image(3, true, true); + + /* Sanity: it reads as signed before the tamper. */ + uint32_t counter = 0; + ASSERT(eos_rollback_read_image_counter(SLOT_A_ADDR, &counter) == EOS_OK); + ASSERT(counter == 3); + + /* The downgrade attack: raise the declared counter past the device floor + * so an old, genuinely signed image sails through eos_rollback_verify(). */ + sim_counter = 9; + ASSERT(eos_rollback_verify(3) == EOS_ERR_ANTI_ROLLBACK); + + tamper_sec_ver(9); + + counter = 0; + int rc = eos_rollback_read_image_counter(SLOT_A_ADDR, &counter); + + /* Fail closed. Before the header binding this returned EOS_OK with + * counter == 9, and eos_rollback_verify(9) then accepted the image. */ + ASSERT(rc == EOS_ERR_INVALID); +} + +TEST(test_tamper_is_invisible_to_signature_and_integrity) +{ + build_image(3, true, true); + + eos_image_header_t hdr; + ASSERT(eos_image_parse_header(SLOT_A_ADDR, &hdr) == EOS_OK); + ASSERT(eos_image_verify_integrity(&hdr, SLOT_A_ADDR) == EOS_OK); + + uint8_t hdr_before[sizeof(eos_image_header_t)]; + memcpy(hdr_before, &sim_flash[SLOT_A_ADDR], sizeof(hdr_before)); + + tamper_sec_ver(9); + + /* The payload hash still matches and every byte of the signed prefix is + * unchanged: neither of the bootloader's two cryptographic checks can see + * this edit. That is why the counter needs its own binding. */ + ASSERT(eos_image_verify_integrity(&hdr, SLOT_A_ADDR) == EOS_OK); + ASSERT(memcmp(hdr_before, &sim_flash[SLOT_A_ADDR], EOS_IMG_SIGNED_LEN) == 0); +} + +TEST(test_unbound_tlv_area_is_not_trusted) +{ + /* A TLV area sitting after the payload that the header does not vouch for + * is attacker-supplied as far as the bootloader can tell. It must not be + * able to raise the counter; 0 is the conservative reading. */ + build_image(9, true, false); + + uint32_t counter = 0xDEADBEEF; + ASSERT(eos_rollback_read_image_counter(SLOT_A_ADDR, &counter) == EOS_OK); + ASSERT(counter == 0); + + sim_counter = 5; + ASSERT(eos_rollback_verify(counter) == EOS_ERR_ANTI_ROLLBACK); +} + +TEST(test_image_without_tlv_area_still_reports_zero) +{ + /* Unchanged behaviour for images from tools/sign_image.py, which emits + * [header][payload] and no TLV area at all. */ + build_image(0, false, false); + + uint32_t counter = 0xDEADBEEF; + ASSERT(eos_rollback_read_image_counter(SLOT_A_ADDR, &counter) == EOS_OK); + ASSERT(counter == 0); +} + +TEST(test_tlv_binding_fields_are_inside_the_signed_prefix) +{ + /* The binding is only worth anything if the signature covers it. */ + ASSERT(offsetof(eos_image_header_t, tlv_len) + + sizeof(((eos_image_header_t *)0)->tlv_len) <= EOS_IMG_SIGNED_LEN); + ASSERT(offsetof(eos_image_header_t, tlv_hash) + + sizeof(((eos_image_header_t *)0)->tlv_hash) <= EOS_IMG_SIGNED_LEN); +} + +TEST(test_oversized_tlv_len_is_rejected) +{ + build_image(7, true, true); + + /* A header claiming a TLV area larger than the parser will ever accept + * must be refused outright, not hashed over an unbounded flash range. */ + eos_image_header_t hdr; + memcpy(&hdr, &sim_flash[SLOT_A_ADDR], sizeof(hdr)); + hdr.tlv_len = (uint16_t)(EOS_TLV_MAX_SIZE + 1); + memcpy(&sim_flash[SLOT_A_ADDR], &hdr, sizeof(hdr)); + + uint32_t counter = 0; + ASSERT(eos_rollback_read_image_counter(SLOT_A_ADDR, &counter) == EOS_ERR_INVALID); +} + +int main(void) +{ + printf("TLV authentication (anti-rollback counter)\n\n"); + + tests_run = 8; + run_test_authenticated_tlv_counter_is_read(); + run_test_tampered_tlv_counter_is_rejected(); + run_test_tamper_is_invisible_to_signature_and_integrity(); + run_test_unbound_tlv_area_is_not_trusted(); + run_test_image_without_tlv_area_still_reports_zero(); + run_test_tlv_binding_fields_are_inside_the_signed_prefix(); + run_test_oversized_tlv_len_is_rejected(); + + tests_run = 7; + printf("\n%d/%d passed\n", tests_passed, tests_run); + return tests_passed == tests_run ? 0 : 1; +} diff --git a/tools/eos_sign.py b/tools/eos_sign.py index 786be9d..59a55f7 100644 --- a/tools/eos_sign.py +++ b/tools/eos_sign.py @@ -102,7 +102,7 @@ def build_header(payload: bytes, entry_addr: int, load_addr: int, # wait for the signature to exist. hdr += struct.pack('B', EOS_SIG_MAX_SIZE if sig_type == SIG_TYPE_ED25519 else (len(sig) if sig else 0)) - hdr += b'\x00' * 30 # reserved + hdr += b'\x00' * 30 # tlv_len(2) + tlv_hash(28); 0 = no authenticated TLV area # Signature field (padded to 64 bytes) sig_padded = (sig or b'').ljust(EOS_SIG_MAX_SIZE, b'\x00') diff --git a/tools/imgpack.py b/tools/imgpack.py index 11affc8..aae6ed0 100644 --- a/tools/imgpack.py +++ b/tools/imgpack.py @@ -71,7 +71,7 @@ def build_header( header += hash_field header += struct.pack(' Date: Wed, 2 Sep 2026 10:09:29 +0000 Subject: [PATCH 2/3] docs(poc): reproduce the TLV anti-rollback downgrade against upstream A host program that links libeboot_core against a simulated flash and shows the four-byte rewrite passing every check the bootloader performs. Kept out of tests/ deliberately: it demonstrates the pre-fix behaviour at 13a7a02, while tests/unit/test_tlv_auth.c pins the post-fix behaviour. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01MSSnU9cmWFkxan4G57J4VV --- poc/README.md | 45 ++++++++++++++++++++++++ poc/tlv_downgrade_poc.c | 77 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 122 insertions(+) create mode 100644 poc/README.md create mode 100644 poc/tlv_downgrade_poc.c diff --git a/poc/README.md b/poc/README.md new file mode 100644 index 0000000..5484ab7 --- /dev/null +++ b/poc/README.md @@ -0,0 +1,45 @@ +# Proof of concept — unauthenticated anti-rollback counter + +`tlv_downgrade_poc.c` demonstrates, against **unmodified** eBoot at `13a7a02`, +that the security counter gating `eos_rollback_verify()` can be raised by +rewriting four bytes that neither the image signature nor the payload hash +covers. + +It is a host program, not a test: it links `libeboot_core` and a simulated +flash, lays down a well-formed image declaring counter 3, sets the device +anti-rollback floor to 9, and then rewrites the `EOS_TLV_MIN_SEC_VER` value in +the TLV area that sits after the payload. + +## Run it against upstream + +```bash +git checkout 13a7a02 +cmake -B build -DCMAKE_BUILD_TYPE=Debug && cmake --build build --parallel +gcc -I include poc/tlv_downgrade_poc.c -o /tmp/poc \ + build/libeboot_core.a build/libeboot_hal.a -lm +/tmp/poc +``` + +Expected output before the fix: + +``` +device anti-rollback floor : 9 +read_image_counter (untampered) : rc=0 counter=3 +rollback_verify(3) : -14 (rejected, as it should be) + +after rewriting 4 TLV bytes: + signed header prefix changed? : NO + parse_header : 0 + verify_integrity (SHA-256) : 0 (still PASSES) + read_image_counter : rc=0 counter=9 + rollback_verify(9) : 0 (ACCEPTED -- downgrade succeeded) +``` + +After the fix on this branch the same binary reports `counter=0` in both +cases and `rollback_verify` returns `-14` (`EOS_ERR_ANTI_ROLLBACK`) both +times: the image no longer declares an authenticated TLV area, so nothing in +it can raise the floor. + +The behaviour is pinned as a real regression test in +`tests/unit/test_tlv_auth.c`; this directory exists so the original finding +can be reproduced against upstream without applying the patch. diff --git a/poc/tlv_downgrade_poc.c b/poc/tlv_downgrade_poc.c new file mode 100644 index 0000000..6b72865 --- /dev/null +++ b/poc/tlv_downgrade_poc.c @@ -0,0 +1,77 @@ +/* PoC against unmodified eBoot @ 13a7a02: the anti-rollback counter can be + * raised without touching any byte the signature or payload hash covers. */ +#include "eos_image.h" +#include "eos_image_tlv.h" +#include "eos_rollback.h" +#include "eos_crypto_boot.h" +#include "eos_hal.h" +#include +#include + +#define FLASH_SZ (64*1024) +#define SLOT 0x1000u +#define PAY 256u +static uint8_t f[FLASH_SZ]; +static uint32_t hw_counter = 9; /* device floor: only >= 9 may boot */ + +static int fr(uint32_t a, void *b, size_t l){ if(a+l>FLASH_SZ) return EOS_ERR_FLASH; memcpy(b,&f[a],l); return EOS_OK; } +static int fw_(uint32_t a, const void *b, size_t l){ if(a+l>FLASH_SZ) return EOS_ERR_FLASH; memcpy(&f[a],b,l); return EOS_OK; } +static int fe(uint32_t a, size_t l){ if(a+l>FLASH_SZ) return EOS_ERR_FLASH; memset(&f[a],0xFF,l); return EOS_OK; } +static int mr(uint32_t *v){ *v = hw_counter; return EOS_OK; } +static const eos_board_ops_t ops = { .flash_size=FLASH_SZ, .slot_a_addr=SLOT, .slot_a_size=0x8000, + .flash_read=fr, .flash_write=fw_, .flash_erase=fe, .monotonic_read=mr }; + +int main(void) +{ + memset(f, 0xFF, sizeof f); + eos_hal_init(&ops); + + uint8_t pay[PAY]; + for (unsigned i=0;i Date: Wed, 2 Sep 2026 10:14:43 +0000 Subject: [PATCH 3/3] docs: add PROPOSAL.md and RUN.md for the assessment submission Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01MSSnU9cmWFkxan4G57J4VV --- PROPOSAL.md | 260 ++++++++++++++++++++++++++++++++++++++++++++++++++++ RUN.md | 179 ++++++++++++++++++++++++++++++++++++ 2 files changed, 439 insertions(+) create mode 100644 PROPOSAL.md create mode 100644 RUN.md diff --git a/PROPOSAL.md b/PROPOSAL.md new file mode 100644 index 0000000..b4c9b0c --- /dev/null +++ b/PROPOSAL.md @@ -0,0 +1,260 @@ +# eBoot — Technical Assessment Submission + +**N Vignesh Reddy** · nvigneshreddy26@gmail.com · github.com/vignesh917 +**Project reviewed:** eBoot (`embeddedos-org/eBoot`), commit `13a7a02` +**Finding:** the anti-rollback security counter is read from unauthenticated flash + +--- + +## 1. The issue + +eBoot's anti-rollback gate reads the image's security counter from the +`EOS_TLV_MIN_SEC_VER` entry in the image's TLV area +(`core/rollback.c::eos_rollback_read_image_counter`), and `eos_secure_boot()` +uses that value in step 5b to decide whether an image is new enough to boot. + +The TLV area is not covered by anything the bootloader verifies: + +| Check | Region it covers | +|---|---| +| `eos_image_verify_signature()` | `header[0 .. EOS_IMG_SIGNED_LEN)` = bytes 0–91 | +| `eos_image_verify_integrity()` | `[hdr_size, hdr_size + image_size)` — the payload | +| TLV area | starts at `hdr_size + image_size` — **after** both | + +So the one input that decides whether a downgrade is allowed sits in the only +part of the image nothing signs or hashes. + +An attacker who can write flash — physical access, an exposed debug port, or a +compromised recovery agent, since `recovery_handle_write()` accepts any offset +inside the slot — can take a **genuinely signed old image**, rewrite four bytes +of TLV so its declared counter clears the device floor, and boot it. That is +exactly the downgrade anti-rollback exists to stop. This is not a bypass of +signature checking: the image is authentic, it is just old, and the mechanism +meant to reject it can be told otherwise. + +A second consequence: `eos_secure_boot()` calls `eos_rollback_stage(img_counter)` +with the attacker's value, and `eos_bootctl_confirm()` later commits it to the +monotonic counter. `EOS_ROLLBACK_MAX_STEP` caps a single advance at 16, but a +repeatable +16 per confirmed boot is a path to burning the fuse counter past +every legitimate image — a permanent brick. + +I also found the codebase disagreed with itself about where the TLV area is. +`include/eos_image_tlv.h` documented `[image_header][TLV area][payload]` and +claimed "TLVs are covered by the image signature for tamper protection"; +`core/image_verify.c` and `core/rollback.c` both implement +`[image_header][payload][TLV area]`, and the signature covers no part of it. +`tools/eos_sign.py` follows the header's layout and carries a `KNOWN LIMITATION` +note saying the images it produces do not boot as a result. + +### Demonstration against unmodified `13a7a02` + +A short host program against `libeboot_core` (`poc/tlv_downgrade_poc.c`), with +the device floor at 9 and a signed image declaring counter 3: + +``` +device anti-rollback floor : 9 +read_image_counter (untampered) : rc=0 counter=3 +rollback_verify(3) : -14 (rejected, as it should be) + +after rewriting 4 TLV bytes: + signed header prefix changed? : NO + parse_header : 0 + verify_integrity (SHA-256) : 0 (still PASSES) + read_image_counter : rc=0 counter=9 + rollback_verify(9) : 0 (ACCEPTED -- downgrade succeeded) +``` + +Same binary against the patched tree: the counter reads 0 both before and +after, and `rollback_verify` returns `EOS_ERR_ANTI_ROLLBACK` in both cases. + +--- + +## 2. Proposed solution + +Bind the TLV area to the signed image header, the way mcuboot's *protected* +TLVs are covered by the signature, rather than trusting bytes that sit outside +every check. + +The header has 30 reserved bytes at offsets 62–91, and — usefully — +`EOS_IMG_SIGNED_LEN` is 92, so those bytes are already inside the signed +prefix. I split them into: + +```c +uint16_t tlv_len; /* offset 62, 2 bytes */ +uint8_t tlv_hash[EOS_IMG_TLV_HASH_LEN]; /* offset 64, 28 bytes */ +``` + +`tlv_hash` is SHA-256 of the TLV area truncated to 224 bits. Every other field +keeps the byte offset the signing tools address, the header stays 156 bytes on +the wire, and the static asserts in `eos_image.h` (plus `test_image_abi.c`) pin +that. + +`eos_rollback_read_image_counter()` then: + +- `tlv_len == 0` → the image makes no authenticated claim about a TLV area, so + the area is not read and the counter reports **0**; +- `tlv_len` outside `[sizeof(eos_tlv_info_t), EOS_TLV_MAX_SIZE]`, or the address + range wrapping → `EOS_ERR_INVALID`; +- otherwise, hash `tlv_len` bytes at `hdr_size + image_size` and compare against + `tlv_hash`. Mismatch → `EOS_ERR_INVALID`; match → parse and read the counter + as before. + +--- + +## 3. Technical approach and why these choices + +**Why 224 bits and not a full 32-byte digest.** A full digest plus a length +needs 36 bytes and there are 30. Widening the header would move `signature[]` +off offset 92 and break the on-disk format that `sign_image.py`, `imgpack.py`, +`eos_sign.py` and `test_image_abi.c` all address by absolute offset. Truncated +SHA-256 is standard practice (SHA-256/224 is a FIPS 180-4 variant), and what +matters here is second-preimage resistance on a ~64-byte blob — 224 bits is far +past anything reachable. Trading 32 bits of margin for a format that does not +move felt like the right side of that trade; I have noted the alternative in +§5. + +**Why absence reads as 0 rather than failing.** A counter of 0 can only fail +against the device floor, never clear it. So "no authenticated TLV area" is +already the conservative answer and needs no special case. This is also what +keeps the change backward compatible: every image the project's tools currently +produce has those bytes zeroed, so `tlv_len` reads 0 and behaviour is identical +to today. `tools/sign_image.py` — the one the CI path uses — emits +`[header][payload]` with no TLV area at all, so nothing regresses there. + +**Why a claimed-but-mismatched area fails closed instead of falling back to 0.** +Falling back would let tampering degrade the check silently. If the header +vouches for an area and the bytes disagree, something rewrote flash; that +should be reported, and `eos_secure_boot()` already turns a non-`EOS_OK` return +from this function into a refusal to boot. + +**Why the digest is streamed.** The area is hashed in 64-byte chunks through +`eos_sha256_update()` rather than buffered, so stack use is fixed (~64 B + the +SHA-256 context) instead of scaling with `EOS_TLV_MAX_SIZE`. Stage-1 stacks on +the smaller targets in `boards/` are tight. + +**Why the comparison is constant-time.** It runs over attacker-influenced +bytes, and a bootloader gives an attacker unlimited retries with a logic +analyser. It matches the `secure_compare()` pattern already used in +`keystore.c` and `secure_boot.c`. + +**Documentation.** `eos_image_tlv.h` now states the layout the code actually +implements, and says plainly that the area is covered by neither the signature +nor `hash[]` and must be bound through `tlv_len`/`tlv_hash` to be trusted. + +--- + +## 4. Testing and validation + +**New regression test** — `tests/unit/test_tlv_auth.c`, registered in +`tests/CMakeLists.txt` for both ctest and the valgrind sweep. Seven cases: + +| Test | What it pins | +|---|---| +| `authenticated_tlv_counter_is_read` | a correctly bound area still reads normally | +| `tampered_tlv_counter_is_rejected` | the downgrade now returns `EOS_ERR_INVALID` | +| `tamper_is_invisible_to_signature_and_integrity` | the tampered image still passes SHA-256 and leaves the signed prefix byte-identical — proving the binding is the only thing that can catch it | +| `unbound_tlv_area_is_not_trusted` | an area the header does not vouch for reports 0 | +| `image_without_tlv_area_still_reports_zero` | no behaviour change for `sign_image.py` images | +| `tlv_binding_fields_are_inside_the_signed_prefix` | `tlv_len`/`tlv_hash` stay under `EOS_IMG_SIGNED_LEN` | +| `oversized_tlv_len_is_rejected` | a bogus `tlv_len` is refused before any hashing | + +The third case is the one I would point a reviewer at: it fails only if someone +later moves the TLV area back under an existing check, and it documents why the +new field exists. + +**Results** (Linux x86-64, GCC, `-DEBLDR_BUILD_TESTS=ON`, Debug): + +- `ctest` — **38/38 passed**, including 17 valgrind targets (`--leak-check=full + --error-exitcode=1`), up from 36 before. +- `python run_all_tests.py` — **33/33 passed**. +- `python tests/production_test_suite.py` — 1 failure remaining, `SA-6.9: CI + workflow has sanitizer job`, pre-existing and out of scope here. + +**Not covered by my testing, and I want to be clear about it:** I only built +and ran the native host configuration. I did not cross-compile for any board in +`boards/`, and I have no hardware, so nothing here is validated on a real +target. + +### Two small things fixed on the way + +Both were in the way of running the suite that proves the main change: + +- `tests/production_test_suite.py` hard-coded `/home/ubuntu/eBoot`, so it + crashed with a `FileNotFoundError` for anyone else. Now derived from + `__file__`. Running it revealed two pre-existing failures that the crash had + been hiding — the SPDX one below, and `SA-6.9`. +- `core/sha512.c` was missing its SPDX header (`SA-6.6`). Added. + +--- + +## 5. Limitations and things I would want a maintainer's call on + +1. **The signing tools are not updated to emit the binding.** Nothing regresses + — every current tool writes zeros into those bytes, which now means "no + authenticated TLV area" — but until `sign_image.py` learns to append a TLV + area and fill `tlv_len`/`tlv_hash`, a TLV-declared security counter is + effectively unusable rather than merely untrusted. That is a deliberate + fail-closed default, not a finished feature. I left it out because the + format question in §5.2 should be settled first. + +2. **This is a header-format decision, not just a bug fix.** Consuming the + reserved bytes is cheap and offset-stable, but it spends the header's only + remaining space and settles on a truncated digest. The alternatives — + bumping `hdr_version` to 3 and widening the header, or moving the TLV area + in front of the payload so `hash[]` covers it (which is what + `eos_image_tlv.h` used to claim and `eos_sign.py` still assumes) — are both + defensible and both break existing images. I picked the option that changes + nothing for images already in the field; a maintainer may reasonably prefer + one of the others. + +3. **The `eos_sign.py` / `sign_image.py` split is still unresolved.** + `eos_sign.py` emits `[header][TLV][payload]`, which no part of the + bootloader reads. My change makes the intended layout explicit in the + headers, but does not reconcile the two tools. + +4. **Threat model.** This matters only against an attacker who can write flash. + That is deliberately the anti-rollback threat model — an attacker who cannot + write flash has no way to install the old image in the first place — but it + is worth stating that this is not a remote-network finding. + +5. **The TLV read is still not bounded against slot capacity.** `verify_slot()` + bounds `hdr_size + image_size` against `eos_hal_slot_size()`, but nothing + bounds the TLV area that follows, so hashing `tlv_len` bytes can read up to + 512 bytes past the end of a slot that an image exactly fills. This is not a + regression — `eos_tlv_parse()` already read the same region, bounded the + same way — and every HAL `flash_read` I looked at rejects an out-of-range + address. It is still a loose end, and the natural place to close it is the + slot-capacity check in `slot_manager.c`, alongside the one already there. + +6. **Not addressed, though I noticed them.** `eos_secure_boot()` step 4 reads + the OTP root-of-trust key hash and then does nothing with it (two `TODO`-ish + comments where the TLV keyhash comparison should be). `eos_storage_dump()` + is the only function in `storage.c` that does not null-check `dev`. Both + looked like separate changes rather than things to fold into this one. + +--- + +## Files changed + +``` + core/rollback.c | 69 +++++++++ the fix + include/eos_image.h | 34 ++++- tlv_len / tlv_hash + static asserts + include/eos_image_tlv.h | 16 ++- corrected layout documentation + tests/unit/test_tlv_auth.c | 309 +++++++++ new regression test + tests/CMakeLists.txt | 8 +- register it (ctest + valgrind) + tests/unit/test_image_abi.c | 3 +- ABI pins for the new fields + tests/unit/test_image_verify.c | 3 +- signed-prefix coverage assertions + tests/unit/test_slot_manager.c | 8 +- mock used reserved[0] + tests/production_test_suite.py | 6 +- hard-coded path + core/sha512.c | 3 + missing SPDX header + tools/eos_sign.py | 2 +- comment: name the new fields + tools/imgpack.py | 2 +- same +``` + +Per the screening instructions I have not opened a pull request against +`embeddedos-org/eBoot`. The branch and the proof-of-concept are on my fork and +I am happy to open one if the team would like it. + +I used AI assistance while working through this, and I have read and can walk +through every line of it — including why 224 bits, why absence reads as zero, +and why the mismatch case fails closed rather than degrading. diff --git a/RUN.md b/RUN.md new file mode 100644 index 0000000..471b00c --- /dev/null +++ b/RUN.md @@ -0,0 +1,179 @@ +# eBoot — TLV anti-rollback fix: build, test, and reproduce + +Everything below runs on a plain Linux host (or macOS). No board, no +cross-compiler, no hardware needed — the native build compiles the +platform-agnostic core libraries and the whole unit-test suite. + +The finding and the reasoning are in `PROPOSAL.md`. + +--- + +## 0. Prerequisites + +```bash +# Debian / Ubuntu +sudo apt update +sudo apt install -y build-essential cmake git python3 python3-pip valgrind + +# macOS (Homebrew) +brew install cmake python git +``` + +Python bits used by the script-driven suites: + +```bash +pip3 install -r requirements.txt +pip3 install pytest +``` + +> On newer Debian/Ubuntu, `pip3` may refuse to touch system packages. Either +> use a virtualenv, or append `--break-system-packages` to the two commands. + +`valgrind` is optional. Without it, CMake skips the 17 valgrind targets and +`ctest` runs 21 tests instead of 38; everything else is identical. + +--- + +## 1. Build + +```bash +cd eBoot +cmake -B build -DEBLDR_BUILD_TESTS=ON -DCMAKE_BUILD_TYPE=Debug +cmake --build build --parallel +``` + +One `#warning` during the build is expected and intentional — it fires because +no production signing key is configured, so the RFC 8032 test-vector key is +being compiled in as the trust anchor. + +--- + +## 2. Run the tests + +```bash +ctest --test-dir build --output-on-failure +``` + +Expected: **`100% tests passed, 0 tests failed out of 38`** (21 without +valgrind). The new one is `test_tlv_auth`, which you can run on its own: + +```bash +./build/tests/eboot_test_tlv_auth +``` + +``` +TLV authentication (anti-rollback counter) + + test_authenticated_tlv_counter_is_read [PASS] + test_tampered_tlv_counter_is_rejected [PASS] + test_tamper_is_invisible_to_signature_and_integrity [PASS] + test_unbound_tlv_area_is_not_trusted [PASS] + test_image_without_tlv_area_still_reports_zero [PASS] + test_tlv_binding_fields_are_inside_the_signed_prefix [PASS] + test_oversized_tlv_len_is_rejected [PASS] + +7/7 passed +``` + +The script-driven suites: + +```bash +python3 run_all_tests.py # expect 33 passed +python3 tests/production_test_suite.py # expect 1 failure: SA-6.9 (pre-existing) +``` + +`SA-6.9` wants a sanitizer job in the CI workflow. It is unrelated to this +change and deliberately left alone. + +--- + +## 3. Reproduce the original vulnerability + +This is the part worth doing yourself. It builds **upstream, unpatched** eBoot +in a throwaway worktree and runs the proof of concept against it. + +```bash +export EBOOT="$PWD" # run this from the root of this archive +git worktree add /tmp/eboot-upstream 13a7a02 +cd /tmp/eboot-upstream +cmake -B build -DCMAKE_BUILD_TYPE=Debug +cmake --build build --parallel +gcc -I include "$EBOOT/poc/tlv_downgrade_poc.c" -o /tmp/poc \ + build/libeboot_core.a build/libeboot_hal.a -lm +/tmp/poc +``` + +**Before the fix — the downgrade succeeds:** + +``` +device anti-rollback floor : 9 +read_image_counter (untampered) : rc=0 counter=3 +rollback_verify(3) : -14 (rejected, as it should be) + +after rewriting 4 TLV bytes: + signed header prefix changed? : NO + parse_header : 0 + verify_integrity (SHA-256) : 0 (still PASSES) + read_image_counter : rc=0 counter=9 + rollback_verify(9) : 0 (ACCEPTED -- downgrade succeeded) +``` + +Four bytes were rewritten. The SHA-256 payload check still passes and not one +byte of the signed header prefix changed — because the TLV area sits outside +both. + +**After the fix — same binary, patched library:** + +```bash +cd "$EBOOT" +gcc -I include poc/tlv_downgrade_poc.c -o /tmp/poc_after \ + build/libeboot_core.a build/libeboot_hal.a -lm +/tmp/poc_after +``` + +``` +read_image_counter (untampered) : rc=0 counter=0 +rollback_verify(0) : -14 (rejected, as it should be) +... + read_image_counter : rc=0 counter=0 + rollback_verify(0) : -14 (rejected) +``` + +The image declares no authenticated TLV area, so nothing in it can raise the +anti-rollback floor. + +Clean up the worktree when done: + +```bash +git worktree remove --force /tmp/eboot-upstream +``` + +--- + +## 4. Read the change + +```bash +git log --oneline 13a7a02..HEAD +git diff 13a7a02 -- core/rollback.c include/eos_image.h +git show --stat HEAD~1 +``` + +The substance is in three files: + +| File | What changed | +|---|---| +| `include/eos_image.h` | `reserved[30]` → `tlv_len` (2 B) + `tlv_hash` (28 B), both inside the signed prefix, plus static asserts pinning the offsets | +| `core/rollback.c` | hash the declared TLV area and refuse to read a counter from one that does not match | +| `tests/unit/test_tlv_auth.c` | the regression test, 7 cases | + +--- + +## 5. Optional: cross-compile check + +Not required, and not something I validated. If you have the toolchain: + +```bash +cmake -B build-arm -DEBLDR_BOARD=stm32f4 \ + -DCMAKE_TOOLCHAIN_FILE=toolchains/arm-none-eabi.cmake +cmake --build build-arm --parallel +```