fix(image): pin the whole .efw header, not four fields of it by Kartikey1306 · Pull Request #87 · embeddedos-org/eBoot

Kartikey1306 · 2026-09-01T15:18:55Z

Closes #66, parts 1 and 2.

eos_image_header_t is a wire format: eFirmware writes these bytes, eBoot reads them, and the signing tools address fields by absolute offset. Four of its fourteen fields were pinned — sizeof, hash, sig_type, signature.

A field swap passed every existing assert

Transposing two adjacent same-width fields moves neither sizeof nor any of those four offsets:

$ # load_addr and entry_addr transposed
$ cc -std=c11 -I include -c abi_tu.c
   (on master: compiles clean, 0 asserts fire)

The bootloader would then load an image at its entry point and jump to its load address. With this change:

include/eos_image.h:99:  static assertion failed: load_addr must stay at offset 12
include/eos_image.h:101: static assertion failed: entry_addr must stay at offset 16

No constant's value was pinned on this side

EOS_IMG_MAGIC, EOS_HASH_SIZE, EOS_SIG_MAX_SIZE, EOS_IMG_SIGNED_LEN, EOS_IMAGE_HDR_VERSION and the five eos_sig_type_t values all travel inside the image, so they are wire format too, and an offset assert says nothing about them.

$ # EOS_SIG_ED25519 = 3 -> 4
   master:    compiles clean, 0 asserts fire
   this PR:   static assertion failed: EOS_SIG_ED25519 is 3 on the wire

That change would make eBoot misread the signature type of every image already in the field. eFirmware pins EFW_IMAGE_MAGIC == 0x454F5349u and EFW_SIG_ED25519 == 3 on its side; these are the matching half, so the two definitions can no longer drift apart in silence.

What is added

	count
missing field offsets	10
field widths	3
constant values	11
total asserts	30 (was 4)

The three width asserts are there because an offset assert cannot see a field growing into padding that happens to keep every later offset — reserved[30] absorbs exactly that.

Header only. No struct member, constant, or line of logic changes, so every image on disk today parses exactly as it did before. The asserts are all EOS_IMG_STATIC_ASSERT, which already degrades to nothing before C11.

Validation

compiles clean, all 30 pass
load_addr/entry_addr transposed  ->  2 asserts fire   (master: 0)
EOS_SIG_ED25519 = 3 -> 4         ->  1 assert fires   (master: 0)
cmake -DEBLDR_BUILD_TESTS=ON + ctest  ->  20/20 passed

Not touched

Part 3 of #66 — eFirmware stamping hdr_version = 1 while eBoot supports only v2, so core/image_verify.c admits a v1 image at the parser and then fails it at the signature — is a security-policy call (reject at parse, branch the verifier, or change what efwtool stamps). It fails closed, so it is not an escalation, and it wants a decision rather than a patch. Deliberately left out.

Closes embeddedos-org#66 (parts 1 and 2). eos_image_header_t is a wire format — eFirmware writes these bytes, eBoot reads them, and the signing tools address fields by absolute offset. Four of its fourteen fields were pinned: sizeof, hash, sig_type, signature. Transposing two adjacent same-width fields moves neither sizeof nor any of those four offsets. With load_addr and entry_addr swapped, all four existing asserts still pass and the header compiles clean; the bootloader then loads an image at its entry point and jumps to its load address. No constant's *value* was pinned on this side either. EOS_IMG_MAGIC, EOS_HASH_SIZE, EOS_SIG_MAX_SIZE, EOS_IMG_SIGNED_LEN, EOS_IMAGE_HDR_ VERSION and the five eos_sig_type_t values all travel inside the image. Changing EOS_SIG_ED25519 from 3 to 4 compiled cleanly, passed every assert, and would make eBoot misread the signature type of every image already in the field. eFirmware pins EFW_IMAGE_MAGIC and EFW_SIG_ED25519 on its side; this is the matching half, so the two definitions can no longer drift apart in silence. Adds the ten missing offsets, three field widths (an offset assert cannot see a field growing into padding that keeps every later offset — reserved[] absorbs exactly that), and eleven constant values. Thirty asserts total. Header only. No struct member, constant, or line of logic changes, so every image on disk today parses exactly as before. Verified: compiles clean, all 30 pass load_addr/entry_addr transposed -> 2 asserts fire (master: 0) EOS_SIG_ED25519 = 3 -> 4 -> 1 assert fires (master: 0) cmake -DEBLDR_BUILD_TESTS=ON + ctest -> 20/20 passed Part 3 of embeddedos-org#66 — eFirmware stamping v1 while eBoot supports only v2 — is a security-policy call and is deliberately not touched here.

codecov-commenter · 2026-09-01T15:20:29Z

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

srpatcha

Review — eBoot#87 "fix(image): pin the whole .efw header, not four fields of it"

head: 837fed8 author: Kartikey1306 ci: pass

Verdict: Header-only, no member or logic change, and every one of the 23 new asserts is correct — I recomputed all 10 offsets against natural alignment and checked all 10 constants against their definitions. The substance is right and the "every remaining field is now pinned" claim is true. Two things are not: the arithmetic in the PR body, and the drift-protection argument the constant asserts are justified by, which does not hold for the one constant where the two repos already disagree.

Findings

#	Severity	File:line	Finding	Recommended fix
1	Medium	`include/eos_image.h`, comment above the constant asserts: "eFirmware pins the same numbers on its side … so the two definitions can no longer drift apart in silence"	The mechanism described does not exist. Each repo asserts its own literal against itself; nothing compares the two. eBoot's `_Static_assert` cannot see eFirmware's headers, and eFirmware's equivalents are runtime checks in `eFirmware/tests/test_abi.c:84,89-93`, run by a different suite in a different repo. Two independent self-consistency checks do not detect divergence — they only make each side's own value harder to change by accident, which is worth having but is a different property. And the counter-example is already in the tree: this PR adds `EOS_IMG_STATIC_ASSERT(EOS_IMAGE_HDR_VERSION == 2, …)` while `eFirmware/include/efw/efw_image.h:56` defines `EFW_IMAGE_HDR_VERSION 1u`. The two sides have already drifted on `hdr_version`, both now assert their own value, and neither assert fires. The new assert pins the disagreement rather than surfacing it.	Two parts, and only the first belongs in this PR: (a) reword the comment to what the asserts actually do — "each side pins its own values, so neither changes by accident; this is not a cross-repo check and does not detect divergence" — and drop "can no longer drift apart in silence"; (b) separately, the real check is a CI job that compiles both headers in one translation unit and static-asserts `EOS_IMG_MAGIC == EFW_IMAGE_MAGIC`, the five signature enumerators pairwise, and `offsetof` equality field by field. That is the thing that would have caught the `hdr_version` split, and it is worth its own issue.
2	Low	PR body, the counts table and "Four of its fourteen fields were pinned"; same wording in the added comment in `include/eos_image.h`	`eos_image_header_t` has 13 fields (`include/eos_image.h:25-39`: magic, hdr_version, hdr_size, image_size, load_addr, entry_addr, image_version, flags, hash, sig_type, sig_len, reserved, signature), not fourteen, and three of them had pinned offsets, not four — the fourth pre-existing assert is `sizeof`, which is not a field. The table is also off: the diff adds 10 offset asserts, 3 width asserts and 10 constant asserts, not 11, for 23 new and 27 total, not 30. The 59-line diff corroborates 23 (23 × 2 lines + 13 lines of comment). Small, but this PR's whole subject is that a stated invariant should match the artefact, and per the run brief an unsupported count is the finding.	Correct both to "three of the thirteen field offsets were pinned; all thirteen now are" and "10 + 3 + 10 = 23 new, 27 total (was 4)".
3	Low	`include/eos_image.h`, the pinned block as a whole	Byte order is not pinned and not documented, on either side of the file. eFirmware's mirror header states it — "Multi-byte fields are little-endian. That matches the in-memory struct layout on every currently supported target (ARM and x86, both LE), which is how eBoot reads the header straight out of flash" (`eFirmware/include/efw/efw_image.h:32-34`) — and eBoot, which is the side that reads the header straight out of flash, says nothing. Offsets and widths do not constrain byte order, so a big-endian port would compile with all 27 asserts passing and silently misread `magic`, `image_size`, `load_addr` and `entry_addr`. §5 lists ARM/ARM64, RISC-V, x86-64 and Xtensa as approved ports; ARM and RISC-V both have big-endian configurations. Unlike the cross-repo case, this one is a compile-time property.	Add the constraint the format actually has: `EOS_IMG_STATIC_ASSERT(__BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__, "the .efw header is little-endian on the wire")` (guarded on `__BYTE_ORDER__` being defined), and copy eFirmware's endianness paragraph into this header's doc comment so the contract is stated where it is read.
4	Low	`include/eos_image.h:66-72`, the `EOS_IMG_STATIC_ASSERT` fallback	`#else #define EOS_IMG_STATIC_ASSERT(cond, msg) /* unavailable before C11 */` — all 27 checks silently become nothing on a pre-C11 compiler, with no diagnostic. eBoot's own build is safe: `CMakeLists.txt:8-9` sets `CMAKE_C_STANDARD 11` with `CMAKE_C_STANDARD_REQUIRED ON`. But this is a file in `include/`, which `.ai/architect.md` calls "the contract other repos compile against", and the signing tools address these fields by absolute offset. A consumer building at `-std=c99` loses the entire wire-format guard and is told nothing. `.ai/security.md` puts it directly: "A verification step that cannot run must fail, not pass"; the PR body treats the degradation as a feature ("already degrades to nothing before C11"). Pre-existing macro, but this PR is what makes the whole format depend on it, taking it from 4 asserts to 27.	Make the fallback work instead of vanish: `#define EOS_IMG_STATIC_ASSERT(cond, msg) typedef char eos_img_assert_##__LINE__[(cond) ? 1 : -1]` (or an `extern` array declaration), which is C89-portable. If the no-op is kept deliberately, emit a `#warning` so a consumer at least learns the guard is off.

Architecture conformance

Conforms. §21 Tier 1 — Foundation. include/ is the correct location per .ai/architect.md's eBoot layout, and the rule that "include/ depends on nothing" holds: the file's only include is eos_types.h, unchanged, and the added asserts introduce no new dependency in any direction. No new link line or target_link_libraries entry. §8.1 "signed manifests and images" is served — pinning the offsets is what keeps EOS_IMG_SIGNED_LEN addressing the bytes the signer signed. §5.1 minimal TCB: 59 lines of compile-time assertion, zero runtime code, zero flash.

Placement of the format itself is worth stating since two repos define it. eBoot is Tier 1 and eFirmware is the producer side; the format lives in both as mirrored declarations with no shared source of truth. §21.1's split policy is about repositories, not headers, and does not speak to a duplicated wire format — the closest the design comes is §10.1's component contract, which is about manifests rather than image headers. This is the same shape as the duplicated Ed25519 verifier in #86/#89, and I am recording the design gap once, against #89, rather than three times.

The deferral of #66 part 3 is correctly described and I verified the "fails closed" claim: core/image_verify.c:89 reads if (out->hdr_version == 0 || out->hdr_version > EOS_IMAGE_HDR_VERSION), so a v1 image passes the parser (1 <= 2), and the signature is then checked over EOS_IMG_SIGNED_LEN at core/image_verify.c:202,208 — the v2 prefix — which a v1 signature over hash[] alone cannot satisfy. Rejected, not admitted. Leaving that to a policy decision is the right call.

Proposed changes

Reword the drift comment (finding 1a) and correct the counts in both the body and the comment (finding 2). Comment-only; no code moves.
Add the endianness assert and doc paragraph (finding 3).
Make the pre-C11 fallback loud or portable (finding 4).
Open a separate issue for the real cross-repo ABI check (finding 1b), which is the only thing that would catch the hdr_version split and belongs with #66 part 3 rather than here.

All four are independent. None of them argues against merging the asserts themselves.

Verified clean, for the record: all 13 field offsets recomputed from the declaration at include/eos_image.h:25-39 under natural alignment — 0, 4, 6, 8, 12, 16, 20, 24, 28, 60, 61, 62, 92, total 156, struct alignment 4, no trailing padding — every offset assert matches. All three width asserts match. All ten constant asserts match their definitions: EOS_IMG_MAGIC 0x454F5349 (include/eos_types.h:49), EOS_HASH_SIZE 32 and EOS_SIG_MAX_SIZE 64 (:150-151), the enum 0..4 (:98-102), EOS_IMAGE_HDR_VERSION 2 (include/eos_image.h:46), and EOS_IMG_SIGNED_LEN = offsetof(…, signature) = 92. After this PR every field offset in the struct is pinned, so no same-width transposition and no scalar widening or narrowing can pass unnoticed — a narrowing that would preserve later offsets is prevented by alignment, and a widening absorbed by reserved[] is caught by its width assert. The completeness claim holds.

Not checked

Nothing was compiled. The three negative probes in the body — transposing load_addr/entry_addr fires 2 asserts, EOS_SIG_ED25519 = 4 fires 1, both firing 0 on master — were not reproduced; that needs the PR head checked out, which the run brief forbids. My confirmation of the asserts is by recomputing offsets and reading the constant definitions, which is independent of the author's numbers but is not the compiler's answer. ctest 20/20 likewise not run.
Finding 1's hdr_version divergence is read from the working tree, not from this PR's base. eFirmware is at whatever commit the sync step left it; I did not check its branch or whether an unmerged PR there moves EFW_IMAGE_HDR_VERSION to 2. If one does, finding 1's example weakens — the mechanism point stands regardless.
eFirmware's own offset pinning not audited. I confirmed it pins the magic and the five enumerators (tests/test_abi.c:84,89-93) and that its struct mirrors eBoot's field for field with an identical documented offset table (include/efw/efw_image.h:15-29,84-98). Whether it pins every offset, and whether EFW_SHA256_DIGEST_LEN is 32, I did not verify.
No cross-target compile. Whether all 27 asserts hold on every §22 target — in particular the Xtensa and RISC-V ports and any big-endian configuration — is exactly what finding 3 is about and is not answered by checks.txt, whose cross legs are arm-none-eabi Cortex-M4 and STM32F4 only. Both little-endian.
22 of 23 checks pass; Create GitHub Release and assign report skipping, expected on a PR. No required check failing. mergeStateStatus: BLOCKED, mergeable: MERGEABLE; no merge attempted.

Automated architecture review of 837fed8a5e15 — scheduled, model claude-opus-5, checked against the EmbeddedOS Master Design v2.0. Advisory only: this reviewer never approves, requests changes, or merges. Reply here to discuss or push back — a wrong finding is a bug worth reporting.

…rged broken (#94) master (22d8f8b) does not compile. Two independent double-merges, both the same shape: two PRs fixing adjacent things landed on stale bases, each was green on its own branch, and the result was never rebuilt. 1. include/eos_image.h — #93 replaced reserved[30] with tlv_len (2) + tlv_hash[28], preserving every offset. #87 merged afterwards carrying asserts written against the older struct: error: no member named 'reserved' in 'eos_image_header_t' (x2) #93 already asserts tlv_len at 62 and tlv_hash at 64, so the offset assert was a duplicate; the width assert had no replacement and is restored as two asserts covering both halves of the same 30-byte span. No offset moves and the wire format is unchanged. 2. core/ed25519_verify.c — #86 and #57 both landed a subgroup guard, so the file carried two byte-identical point_is_identity() definitions: error: redefinition of 'point_is_identity' Only #57's public_key_is_valid_subgroup() is wired to the call site, so #86's key_has_prime_order() was dead. Kept the live function, folded #86's fuller rationale onto it, deleted the duplicate. 3. tests/unit/test_ed25519.c — collateral from the same merge. Two copies of test_ed25519_identity_key_forgery_rejected, main() calling it twice and two tests not at all, and test_ed25519_low_order_R_with_a_valid_key_is_not_a_forgery referencing k_low_order[] and messages[] that the merge had dropped. While restoring the corpus, corrected it (review finding on #86): the array claimed to hold "the eight low-order point encodings" and held five. Every order here was computed rather than copied — decode y, recover x, add the point to itself until it reaches the identity — giving 1, 2, 4, 4, 8, 8, 8, 8. Missing before: y=0 with the sign bit set, and both sign-flipped order-8 encodings. D9FF..FF was in the array and is not a low-order point at all — no x satisfies the curve equation for that y — so it moves to a separate k_non_canonical[], with EDFF..FF7F (y=p) and EEFF..FF7F (y=p+1). tests_run was assigned a literal (11) in main() and never incremented, which is how the duplicate call and the two unregistered tests went unnoticed. The TEST macro now increments it, so the total cannot drift. Verified: cmake -DEBLDR_BUILD_TESTS=ON on master FAILS to build, 3 errors same with this commit builds clean ctest 21/21 PASS ctest -DEBLDR_SANITIZE=ON (ASan+UBSan) 21/21 PASS pytest tests/ 24 passed, 1 skipped test_ed25519 14/14 PASS (was 11 claimed, 12 run) discrimination, with `public_key_is_valid_subgroup` disabled: test_ed25519_low_order_keys_rejected FAILS, as it must test_ed25519_non_canonical_... still PASSES — those are refused by unpackneg() on canonicality, a different mechanism, which is the reason they are held in a separate array rather than counted among the eight.

* fix: repair master — the ABI asserts and the Ed25519 verifier both merged broken master (22d8f8b) does not compile. Two independent double-merges, both the same shape: two PRs fixing adjacent things landed on stale bases, each was green on its own branch, and the result was never rebuilt. 1. include/eos_image.h — #93 replaced reserved[30] with tlv_len (2) + tlv_hash[28], preserving every offset. #87 merged afterwards carrying asserts written against the older struct: error: no member named 'reserved' in 'eos_image_header_t' (x2) #93 already asserts tlv_len at 62 and tlv_hash at 64, so the offset assert was a duplicate; the width assert had no replacement and is restored as two asserts covering both halves of the same 30-byte span. No offset moves and the wire format is unchanged. 2. core/ed25519_verify.c — #86 and #57 both landed a subgroup guard, so the file carried two byte-identical point_is_identity() definitions: error: redefinition of 'point_is_identity' Only #57's public_key_is_valid_subgroup() is wired to the call site, so #86's key_has_prime_order() was dead. Kept the live function, folded #86's fuller rationale onto it, deleted the duplicate. 3. tests/unit/test_ed25519.c — collateral from the same merge. Two copies of test_ed25519_identity_key_forgery_rejected, main() calling it twice and two tests not at all, and test_ed25519_low_order_R_with_a_valid_key_is_not_a_forgery referencing k_low_order[] and messages[] that the merge had dropped. While restoring the corpus, corrected it (review finding on #86): the array claimed to hold "the eight low-order point encodings" and held five. Every order here was computed rather than copied — decode y, recover x, add the point to itself until it reaches the identity — giving 1, 2, 4, 4, 8, 8, 8, 8. Missing before: y=0 with the sign bit set, and both sign-flipped order-8 encodings. D9FF..FF was in the array and is not a low-order point at all — no x satisfies the curve equation for that y — so it moves to a separate k_non_canonical[], with EDFF..FF7F (y=p) and EEFF..FF7F (y=p+1). tests_run was assigned a literal (11) in main() and never incremented, which is how the duplicate call and the two unregistered tests went unnoticed. The TEST macro now increments it, so the total cannot drift. Verified: cmake -DEBLDR_BUILD_TESTS=ON on master FAILS to build, 3 errors same with this commit builds clean ctest 21/21 PASS ctest -DEBLDR_SANITIZE=ON (ASan+UBSan) 21/21 PASS pytest tests/ 24 passed, 1 skipped test_ed25519 14/14 PASS (was 11 claimed, 12 run) discrimination, with `public_key_is_valid_subgroup` disabled: test_ed25519_low_order_keys_rejected FAILS, as it must test_ed25519_non_canonical_... still PASSES — those are refused by unpackneg() on canonicality, a different mechanism, which is the reason they are held in a separate array rather than counted among the eight. * fix(fuzz): point every harness at the API it claims to fuzz Four of the five fuzz harnesses did not test what they name, and the build could not say so because not one of them includes a project header. Each declares its target itself: fuzz_recovery_protocol.c extern int eos_recovery_parse_packet(...) fuzz_fw_update.c extern int eos_fw_update_init(void) extern int eos_fw_update_process_chunk(...) extern int eos_fw_update_finalize(void) fuzz_bootctl.c extern int eos_bootctl_parse(...) fuzz_image_verify.c extern int eos_image_parse_header(const void *, size_t) eos_recovery_parse_packet, eos_fw_update_init, eos_fw_update_process_chunk and eos_bootctl_parse have never existed — those names appear nowhere outside the harness that declares them, so three of the five targets have never linked. The other two are worse, because they do link: - eos_fw_update_finalize exists but takes (ctx, mode), not (void). - eos_image_parse_header takes (uint32_t addr, eos_image_header_t *), not (const void *, size_t). fuzz_image_verify has been passing a pointer where an address is expected and a length where an output struct is expected, on every input, for as long as it has run. A local extern is why none of this was caught: it tells the compiler the symbol exists with whatever shape the harness asserts, and the mismatch survives to link time or past it. All five now include the real header and drive the real entry points: fuzz_image_verify parse_header -> verify_integrity, boot-path order fuzz_bootctl bootctl_load, then trap if load() accepted a block validate() rejects fuzz_fw_update begin -> write in fuzz-chosen chunk widths -> finalize or abort fuzz_recovery_protocol write_in_range, trapping on any accepted write that leaves the slot or wraps, checked in wider types that cannot fuzz_crypto already correct; switched to the header so it stays that way Adds tests/fuzz/fuzz_sim_flash.h: the three harnesses that reach flash need board ops installed, and without them they fuzz a null op table. That the compiler now checks this is not theoretical — writing this, it rejected EOS_UPGRADE_MODE_TEST for EOS_UPGRADE_TEST immediately, which is exactly the error class the externs were hiding. Verified. libFuzzer is unavailable here (libclang_rt.fuzzer_osx.a not found), so each harness was linked against eboot_core and driven by a standalone seed generator under ASan+UBSan: all five compile -Wall -Wextra 5/5, 0 failures 20,000 inputs each 100,000 total, no reports ctest 21/21 passed Third instance of this class across these repos, after eos#50 (fuzz_devicetree naming eos_dtb_parse) and eos#110 (fuzz_ota_header naming eos_ota_parse_header). The common factor every time is a fuzz target declaring its own subject. Stacked on #94, without which include/eos_image.h does not compile. * fix(fuzz): the recovery oracle was off by one, and CI proved it The restored Fuzz Harness Build job on #101 ran these harnesses under real libFuzzer for the first time and fuzz_recovery_protocol trapped within seconds. That was my bug, not the code's. The oracle computed the one-past-the-end address: end = base + offset + len; if (end > 0xFFFFFFFF) trap; The last byte a write of len bytes touches is base + offset + len - 1. eos_recovery_write_in_range() checks exactly that: if ((uint32_t)len - 1u > UINT32_MAX - (base + offset)) reject; So an accepted write whose final byte lands exactly on 0xFFFFFFFF -- base=0xFFFFFF00, offset=0, len=256 -- is legal, the function accepts it, and the old oracle trapped on it. A harness that traps on valid input reports the opposite of the truth. Verified both directions: the boundary case, fed directly -> no trap a fail-open stub accepting all input -> trap fires (exit 133) 500,000 random inputs, ASan+UBSan -> no reports libFuzzer doing precisely what it is for -- driving inputs into the one corner a hand-written oracle got wrong -- is the strongest argument yet for #101's job. The guard could never have caught this; only execution under a coverage-driven fuzzer did.

* fix: repair master — the ABI asserts and the Ed25519 verifier both merged broken master (22d8f8b) does not compile. Two independent double-merges, both the same shape: two PRs fixing adjacent things landed on stale bases, each was green on its own branch, and the result was never rebuilt. 1. include/eos_image.h — #93 replaced reserved[30] with tlv_len (2) + tlv_hash[28], preserving every offset. #87 merged afterwards carrying asserts written against the older struct: error: no member named 'reserved' in 'eos_image_header_t' (x2) #93 already asserts tlv_len at 62 and tlv_hash at 64, so the offset assert was a duplicate; the width assert had no replacement and is restored as two asserts covering both halves of the same 30-byte span. No offset moves and the wire format is unchanged. 2. core/ed25519_verify.c — #86 and #57 both landed a subgroup guard, so the file carried two byte-identical point_is_identity() definitions: error: redefinition of 'point_is_identity' Only #57's public_key_is_valid_subgroup() is wired to the call site, so #86's key_has_prime_order() was dead. Kept the live function, folded #86's fuller rationale onto it, deleted the duplicate. 3. tests/unit/test_ed25519.c — collateral from the same merge. Two copies of test_ed25519_identity_key_forgery_rejected, main() calling it twice and two tests not at all, and test_ed25519_low_order_R_with_a_valid_key_is_not_a_forgery referencing k_low_order[] and messages[] that the merge had dropped. While restoring the corpus, corrected it (review finding on #86): the array claimed to hold "the eight low-order point encodings" and held five. Every order here was computed rather than copied — decode y, recover x, add the point to itself until it reaches the identity — giving 1, 2, 4, 4, 8, 8, 8, 8. Missing before: y=0 with the sign bit set, and both sign-flipped order-8 encodings. D9FF..FF was in the array and is not a low-order point at all — no x satisfies the curve equation for that y — so it moves to a separate k_non_canonical[], with EDFF..FF7F (y=p) and EEFF..FF7F (y=p+1). tests_run was assigned a literal (11) in main() and never incremented, which is how the duplicate call and the two unregistered tests went unnoticed. The TEST macro now increments it, so the total cannot drift. Verified: cmake -DEBLDR_BUILD_TESTS=ON on master FAILS to build, 3 errors same with this commit builds clean ctest 21/21 PASS ctest -DEBLDR_SANITIZE=ON (ASan+UBSan) 21/21 PASS pytest tests/ 24 passed, 1 skipped test_ed25519 14/14 PASS (was 11 claimed, 12 run) discrimination, with `public_key_is_valid_subgroup` disabled: test_ed25519_low_order_keys_rejected FAILS, as it must test_ed25519_non_canonical_... still PASSES — those are refused by unpackneg() on canonicality, a different mechanism, which is the reason they are held in a separate array rather than counted among the eight. * fix(fdt): bound a device tree parser that was never compiled core/fdt_loader.c is in no source list, so it has never been built. The header offers it to callers, rtos_boot.c is named as the consumer, and nothing catches what is in it -- not the compiler, not ctest, not the sanitizer job. What is in it is a parser for a blob that comes out of flash, whose every header field it uses as an offset or a length without checking any of them. eos_fdt_validate() looks at magic and version only, so it accepts a blob whose off_dt_struct points anywhere: hdr.totalsize = 40 (the header alone) hdr.off_dt_struct = 0x100000 eos_fdt_validate(blob) -> 0 eos_fdt_get_prop(...) -> AddressSanitizer: BUS, READ at fdt_loader.c:25 in fdt_read_u32 Five more, all reachable the same way: * the tag read at the top of the loop takes 4 bytes where the loop condition guarantees 1; * strlen() on a node name reads until it finds a zero, which for a name running to the end of the block is past it; * nameoff indexes the strings block unchecked, so strcmp() reads from an arbitrary address; * a property len is clamped to the caller's buffer before the memcpy, which bounds the write but not the read -- an oversized len copies whatever follows the blob out to the caller; * FDT_END_NODE decrements depth with no floor. Bound them. validate() is the gate every path goes through, so the block offsets are checked against totalsize there, and get_prop() calls it before trusting the header. Inside the loop each read is checked for the width it takes, the name and property-name scans are bounded by memchr within their blocks, and the padded advances are re-checked for overrun. Adds the file to eboot_core and tests/unit/test_fdt_loader.c to ctest: ten cases, one well-formed tree that must still parse and nine malformed ones. Against the unfixed parser the suite fails on the third; with the bounds in place the whole suite is 20/20, and 20/20 under EBLDR_SANITIZE. Not fixed here, because it is a behaviour change rather than a safety one: node paths below the root do not resolve. _get_prop derives the depth to match from a slash count, so "/chosen" looks for depth 1, which is the root -- "chosen" is at depth 2. Only "/" resolves today. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(fdt): bound the parser against the caller's length, not the blob's claim Follow-up to the review on #84. Three findings from it, all in files this branch already owns. Finding 1 (High) -- every bound in eos_fdt_validate() was expressed relative to hdr->totalsize, which is read out of the same untrusted blob. Checking the block offsets against it proves the header is internally consistent and says nothing about how many bytes are really mapped: a 40-byte buffer declaring totalsize = 0x100000 with agreeing offsets passed every check, and eos_fdt_get_prop() then walked a megabyte past the allocation. The original reproducer (an inflated *offset* against an honest totalsize) was rejected; its mirror image was not. Adds the length-carrying entry points eos_fdt_validate_sized() and eos_fdt_get_prop_sized(), which take the bytes the caller actually owns and check totalsize against that before anything else. The existing two-argument forms stay as wrappers that trust the blob's own totalsize, documented in the header as a warrant the caller has to make good; eos_fdt_load() now passes max_size, which it had all along. Finding 2 (Medium) -- a property larger than the caller's buffer was copied short and reported as success, so a clipped value was indistinguishable from a complete one. In a boot path this reads bootargs. Now returns -7 and sets *buf_len to the full length so the caller can size a retry. Finding 3 (Medium) -- adds tests/fuzz/fuzz_fdt.c alongside the five existing harnesses. .ai/security.md names device tree among the parsers that get fuzz coverage rather than unit tests alone. It drives the sized entry points and passes the real size, which is what makes the harness meaningful -- the unsized forms would let a fuzzer authorise its own out-of-bounds read. Finding 6 (Low) -- drops the unreachable `total < sizeof(fdt_header_t)` check in eos_fdt_load(); validate() already rejects that blob, and the comment above it described a bound it was not applying. Header now documents the full -1..-8 return code table. Verified: cmake -DEBLDR_BUILD_TESTS=ON, ctest 21/21 PASS same under -DEBLDR_SANITIZE=ON (ASan+UBSan) 21/21 PASS test_fdt_loader 14 tests PASS (was 10) new tests against the pre-fix parser both FAIL as they should - inflated totalsize accepted at the sized entry point - truncating call returned 0 instead of -7 grep for eos_fdt_get_prop / eos_fdt_validate no callers outside the parser and its tests, so the -7 change breaks nothing today Refs #84 * fix(build): drop the duplicate core/fdt_loader.c registration master gained core/fdt_loader.c in eboot_core while this branch was open, so after the rebase it appeared twice and tests/unit/test_cmake_core_sources.py::test_core_sources_are_registered_once failed: AssertionError: ['core/fdt_loader.c'] is not false : duplicate eboot_core sources: ['core/fdt_loader.c'] Kept master's entry, dropped the one this branch added. That guard is the same shape as the one this stack adds for toolchain specs -- a build-file check that catches the class rather than the instance -- and it did its job. Verified: ctest 22/22 PASS, pytest 24 passed 1 skipped. Refs #84 * fix(fdt): one length-carrying API, and a CI job that builds the fuzz harnesses Answers the second review on #84. Finding 2 (Medium) -- eos_fdt_validate() and eos_fdt_get_prop() kept an out-of-bounds read by design. Both dereferenced hdr->totalsize before any length was known and handed that attacker-controlled value to the _sized form as its bound, so calling either on a short buffer read past it before a single check had run. The header called that a caller warrant; a warrant is not a check, and it is the exact bug this PR exists to fix. Neither had an in-tree caller. Removed rather than documented. There is now one form of each entry point and it always takes the length: int eos_fdt_validate(const void *fdt_blob, uint32_t avail); int eos_fdt_get_prop(const void *fdt, uint32_t fdt_len, ...); An exported unsafe twin in a TCB header is a future boot-path caller reintroducing the bug with no compiler complaint, which is worth more than the convenience of a one-argument call. eos_fdt_load() already passed max_size. The test that pinned the wrapper's behaviour is gone with the wrapper, and get_prop_sized_exact() collapsed into get_prop_exact() since they became the same function. Finding 3 (Medium) -- tests/fuzz/ was built by nothing. EBLDR_BUILD_FUZZ defaults OFF and no job set it, so the harness added here joined five others that no CI job compiles. A harness that is never built cannot fail to build, which is how fuzz_devicetree came to declare a function that did not exist and sit there unnoticed (eos#50). Adds a `fuzz-build` job: configure with clang, build every harness, and run each for five seconds over its own generated inputs. That is not a campaign -- it is enough to catch a harness that no longer compiles or crashes at once, which is the failure this repo has actually had. Note it needs EBLDR_BUILD_TESTS=ON as well: tests/fuzz/ is added from tests/CMakeLists.txt, so EBLDR_BUILD_FUZZ alone configures cleanly and builds no harness at all -- the job would have passed having compiled nothing. Found that locally before writing the job, not after. NOT RUN, and this is the honest limit: this host has no libFuzzer runtime (libclang_rt.fuzzer_osx.a is absent from the Xcode toolchain), so the link step cannot be reproduced here for any harness, old or new. What I verified is that CMake configures with both flags and reports all six targets -- "Fuzz targets: fuzz_image_verify, fuzz_recovery_protocol, fuzz_fw_update, fuzz_crypto, fuzz_bootctl, fuzz_fdt" -- and that fuzz_fdt.c passes `cc -fsyntax-only`. The link and the smoke run are CI's to show, and this job is what makes them visible. Coordination note: #90 adds a `CI Gate` whose needs list is [test, build-arm, static-analysis]. Whichever of #84 and #90 lands second must add fuzz-build to that list -- and #90's own test_gate_covers_every_job_that_runs_on_a_pull_request fails loudly if it is not, which is the guard working rather than a trap. Finding 1 (High) is a PR-body correction, made there: the diff carries #94's master repair because this branch is stacked on it, and the body described only the FDT parser. Verified: ctest 22/22 PASS pytest tests/ 38 passed test_fdt_loader 13 tests PASS grep for a length-free entry point none remains in the header Refs #84 * ci: move the fuzz-harness build to its own PR The fuzz-build job added in the previous commit does its job -- it is red, on harnesses this PR did not write: undefined reference to `eos_fw_update_init' undefined reference to `eos_fw_update_process_chunk' undefined reference to `eos_recovery_parse_packet' None of those exist anywhere in the tree. tests/fuzz/fuzz_fw_update.c and fuzz_recovery_protocol.c declare an API that was never written, exactly as eos#50's fuzz_devicetree declared eos_dtb_parse(). They have compiled forever because nothing ever linked them. That is a real finding and it deserves a fix, not a red tick on an unrelated PR. The job and the repairs move to their own change; this PR keeps tests/fuzz/fuzz_fdt.c, which is the harness it is responsible for. Refs #84 * fix(fdt): read the header the way the struct block is read, and stop resynchronising on garbage Answers the third review on #84. Finding 3 (Low, and the one that matters on hardware) -- the header was read through a cast fdt_header_t* and direct member loads, while every struct-block read goes through the fdt_read_u32() memcpy helper precisely because the blob may be unaligned. Fuzz input, a buffer inside a larger message, a copy at an odd offset: nothing promises 4-byte alignment, and on a strict-alignment cross target a direct member load is the same fault class this parser exists to avoid. The unit suite could never catch it -- blob_t.bytes happens to be aligned. All header fields now go through fdt_hdr_u32() (memcpy at offsetof), one rule for the whole blob, in validate(), load() and get_prop(). FUZZ_FLAGS gains `undefined` so the fuzz job checks alignment too. Finding 4 (Low) -- `default: break;` resynchronised on unrecognised tags: the walk skipped 4 bytes and treated whatever followed as the next token, so a struct block of arbitrary bytes parsed to a clean "not found". Bounded, but a TCB parser that walks garbage to completion is accepting input it does not understand. FDT_NOP -- the one legal unknown, padding the spec allows between tokens -- is now named in the header and passes; anything else returns -6. Findings 1, 2 and the -8 line: the fuzz-build job's single home is #101 (nothing added here; the harness is inert until that job lands and then compiled by it -- said on the thread, not just here), and the header no longer documents return code -8, which nothing at this head returns. #85 adds the -8 return and re-documents it together with the FDT_MAX_PATH_DEPTH move. Verified: ctest 22/22 PASS ctest -DEBLDR_SANITIZE=ON (ASan+UBSan) 22/22 PASS pytest tests/ 38 passed test_fdt_loader 16 tests PASS (was 13) discrimination, each fix in isolation: default: break restored -> test_a_garbage_tag_is_refused_not_skipped FAILS (expects -6, gets "not found") direct member load restored -> under UBSan the new unaligned-blob test reports "load of misaligned address 0x...671 for type 'const uint32_t'" at the exact line -- and cannot fire on the fixed code, which runs the same test clean NOP counter-check: interleaved FDT_NOP tokens still resolve, so the stricter default does not reject real trees. Refs #84 --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-authored-by: Srikanth Patchava <srikanth.patchava@outlook.com>

…ul boot (#82) * fix: repair master — the ABI asserts and the Ed25519 verifier both merged broken master (22d8f8b) does not compile. Two independent double-merges, both the same shape: two PRs fixing adjacent things landed on stale bases, each was green on its own branch, and the result was never rebuilt. 1. include/eos_image.h — #93 replaced reserved[30] with tlv_len (2) + tlv_hash[28], preserving every offset. #87 merged afterwards carrying asserts written against the older struct: error: no member named 'reserved' in 'eos_image_header_t' (x2) #93 already asserts tlv_len at 62 and tlv_hash at 64, so the offset assert was a duplicate; the width assert had no replacement and is restored as two asserts covering both halves of the same 30-byte span. No offset moves and the wire format is unchanged. 2. core/ed25519_verify.c — #86 and #57 both landed a subgroup guard, so the file carried two byte-identical point_is_identity() definitions: error: redefinition of 'point_is_identity' Only #57's public_key_is_valid_subgroup() is wired to the call site, so #86's key_has_prime_order() was dead. Kept the live function, folded #86's fuller rationale onto it, deleted the duplicate. 3. tests/unit/test_ed25519.c — collateral from the same merge. Two copies of test_ed25519_identity_key_forgery_rejected, main() calling it twice and two tests not at all, and test_ed25519_low_order_R_with_a_valid_key_is_not_a_forgery referencing k_low_order[] and messages[] that the merge had dropped. While restoring the corpus, corrected it (review finding on #86): the array claimed to hold "the eight low-order point encodings" and held five. Every order here was computed rather than copied — decode y, recover x, add the point to itself until it reaches the identity — giving 1, 2, 4, 4, 8, 8, 8, 8. Missing before: y=0 with the sign bit set, and both sign-flipped order-8 encodings. D9FF..FF was in the array and is not a low-order point at all — no x satisfies the curve equation for that y — so it moves to a separate k_non_canonical[], with EDFF..FF7F (y=p) and EEFF..FF7F (y=p+1). tests_run was assigned a literal (11) in main() and never incremented, which is how the duplicate call and the two unregistered tests went unnoticed. The TEST macro now increments it, so the total cannot drift. Verified: cmake -DEBLDR_BUILD_TESTS=ON on master FAILS to build, 3 errors same with this commit builds clean ctest 21/21 PASS ctest -DEBLDR_SANITIZE=ON (ASan+UBSan) 21/21 PASS pytest tests/ 24 passed, 1 skipped test_ed25519 14/14 PASS (was 11 claimed, 12 run) discrimination, with `public_key_is_valid_subgroup` disabled: test_ed25519_low_order_keys_rejected FAILS, as it must test_ed25519_non_canonical_... still PASSES — those are refused by unpackneg() on canonicality, a different mechanism, which is the reason they are held in a separate array rather than counted among the eight. * fix(secure-boot): a debug lock that failed must not report a successful boot cfg.lock_debug asks for SWD/JTAG to be closed before the verified image runs. Step 7 of eos_secure_boot() honoured that request like this: if (cfg->lock_debug) { eos_secure_boot_lock_debug(); } /* ---- Step 8: Record successful attestation ---- */ attest_record(2, hdr.image_version, hdr.hash, NULL, EOS_SBOOT_OK); return EOS_SBOOT_OK; eos_secure_boot_lock_debug() returned void and discarded the result of the OTP write that actually blows the fuse. So when the write failed, boot continued, attestation recorded EOS_SBOOT_OK, and the device ran the image with its debug port open -- the exact condition the policy existed to prevent, reported as a clean secure boot. The interesting case is not a flaky fuse. eos_hal_otp_write() returns EOS_ERR_NOT_SUPPORTED when the board provides no otp_write hook at all, so on every such board `lock_debug: true` was silently a no-op. That is the default configuration, not an edge case. eos_secure_boot_lock_debug() now returns int, and a caller that asked for the lock and did not get it fails with EOS_SBOOT_ERR_POLICY -- a code that already existed for exactly this ("Boot policy violation") -- with the failure recorded in the attestation log rather than a success. Why this was never observable: core/secure_boot.c is not in CMakeLists.txt. The module has never been compiled, so this path could not run and could not be tested. Added the one line that builds it -- the same line #72 adds, written identically so whichever lands first leaves the other a trivial rebase. tests/unit/test_secure_boot_policy.c covers the three outcomes: the fuse written, the write failing, and a board with no otp_write. Kept in its own file so it does not collide with the test_secure_boot.c #72 introduces. Against master the new test does not compile -- `invalid operands to binary expression ('void' and 'int')` -- because there is no result to check. That is the defect stated as a compile error. Verified on this branch: build clean, ctest 20/20, pytest 30 passed. Stacked on #77 (master's test suite does not compile without it). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(secure-boot): test the policy the PR changed, not only the helper it calls Answers finding 1 (Medium) from the review on #82. All three tests called eos_secure_boot_lock_debug() directly and asserted its return value. None drove eos_secure_boot() with cfg.lock_debug = true, so the step-7 early return this PR adds -- the attest_record + return EOS_SBOOT_ERR_POLICY -- never executed under test. The file's docblock says "the debug-lock policy must be enforced, not merely attempted"; what it covered was the attempted half. Adds three end-to-end cases through eos_secure_boot(): - lock_debug: true on a board with no otp_write -> EOS_SBOOT_ERR_POLICY - the same image with lock_debug: false -> EOS_SBOOT_OK, and the recorded entry point is the header's, which is the counter-check: without it the first test would also pass if steps 1-6 were failing for an unrelated reason and never reaching step 7 - lock_debug: true with a working fuse -> EOS_SBOOT_OK, one write Reaching step 7 needs an image that clears steps 1, 2 and 5, so the fixture gains a simulated flash and stages an unsigned, unencrypted image whose SHA-256 matches. EOS_IMG_FLAG_HASH_SHA256 is load-bearing there: without it verify_integrity takes the CRC32 branch, reads a CRC out of hash[], and step 2 fails before the policy step is ever reached. Finding 4 (Low), the tests/CMakeLists.txt anchor shared with #80, is resolved on #80's side -- its block moved to the end of the registrations, so this one keeps its position and the two no longer collide. Verified: ctest 22/22 PASS test_secure_boot_policy 6/6 PASS discrimination: with step 7 reverted to `(void)eos_secure_boot_lock_debug();` test_secure_boot_refuses_when_the_debug_lock_cannot_be_taken FAILS the three original helper tests still PASS which is the gap the finding described, reproduced. Refs #82 * fix(secure-boot): step 4 must not report success for a check it never makes Answers the second review on #82. Finding 1 (High) -- step 4, "Verify signing key against OTP root-of-trust", was the same fail-open this PR removes from step 7, two steps earlier. An `eos_hal_otp_read()` failure was discarded and boot continued; and when the read succeeded and the anchor was provisioned, the body of the `if` was `/* In a full implementation, extract key hash from TLV and compare */`. So a device whose root of trust *is* provisioned booted an image signed by any key the image carried, and step 8 recorded EOS_SBOOT_OK. Implementing the TLV comparison is out of scope, as the review says. The two things that are in scope are done: a non-EOS_OK otp_read now fails EOS_SBOOT_ERR_SIGNATURE, and a provisioned anchor that nothing compares against refuses the boot rather than proceeding. An unprovisioned board (all-zero anchor) is deliberately unchanged -- there is nothing to check against, and refusing would brick every board that has not been provisioned. The comment now states plainly that the step is planned rather than implemented, which §8.1 asks for and a comment inside an `if` was not. No test for those two refusals, deliberately, and the file says why rather than leaving it to be discovered. Reaching step 4 requires passing step 3 -- a real Ed25519 signature checked against the keystore. I wrote the obvious test first and it was worthless: with require_signature = true and an unsigned fixture the boot fails at step 3 and returns the same EOS_SBOOT_ERR_SIGNATURE step 4 returns, so it passed against the unfixed code too. I confirmed that by reverting step 4 and watching it still pass. A test that cannot fail is worse than none. What is there instead is the counter-check that the change does not refuse a boot it should allow. The fixture is buildable -- the keystore ships RFC 8032 TEST 1's public key and the matching private key is in the RFC -- but that machinery is #88's (tools/gen_signed_image_fixture.py). Worth doing once #88 lands. Finding 3 (Low) -- `TEST()` did not increment `tests_run` and `main()` hardcoded `tests_run = 6`, so a test added to the file but not wired into `main()` would have been skipped with a zero exit. #94 removed exactly this from tests/unit/test_ed25519.c, where a hardcoded 11 was masking two uncalled tests. Same fix here. Finding 4 (Low) -- the Valgrind `foreach` is hand-maintained and missed 6 of 22 registered tests. Added test_secure_boot_policy, test_fdt_loader and test_fw_decrypt. The list being hand-maintained at all is the real defect and is not fixed here -- it is the same class as the hardcoded count, one level up. Finding 2 (Medium), that eos_secure_boot() has no production caller, stands and is not addressed here; finding 1 makes it sharper rather than resolving it. Finding 5 (Low) is a PR-body correction. Verified: ctest 22/22 PASS test_secure_boot_policy 7/7 PASS step 7 discrimination, still: reverting the step-7 branch fails test_secure_boot_refuses_when_the_debug_lock_cannot_be_taken Refs #82 --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

…ing them (#95) * fix: repair master — the ABI asserts and the Ed25519 verifier both merged broken master (22d8f8b) does not compile. Two independent double-merges, both the same shape: two PRs fixing adjacent things landed on stale bases, each was green on its own branch, and the result was never rebuilt. 1. include/eos_image.h — #93 replaced reserved[30] with tlv_len (2) + tlv_hash[28], preserving every offset. #87 merged afterwards carrying asserts written against the older struct: error: no member named 'reserved' in 'eos_image_header_t' (x2) #93 already asserts tlv_len at 62 and tlv_hash at 64, so the offset assert was a duplicate; the width assert had no replacement and is restored as two asserts covering both halves of the same 30-byte span. No offset moves and the wire format is unchanged. 2. core/ed25519_verify.c — #86 and #57 both landed a subgroup guard, so the file carried two byte-identical point_is_identity() definitions: error: redefinition of 'point_is_identity' Only #57's public_key_is_valid_subgroup() is wired to the call site, so #86's key_has_prime_order() was dead. Kept the live function, folded #86's fuller rationale onto it, deleted the duplicate. 3. tests/unit/test_ed25519.c — collateral from the same merge. Two copies of test_ed25519_identity_key_forgery_rejected, main() calling it twice and two tests not at all, and test_ed25519_low_order_R_with_a_valid_key_is_not_a_forgery referencing k_low_order[] and messages[] that the merge had dropped. While restoring the corpus, corrected it (review finding on #86): the array claimed to hold "the eight low-order point encodings" and held five. Every order here was computed rather than copied — decode y, recover x, add the point to itself until it reaches the identity — giving 1, 2, 4, 4, 8, 8, 8, 8. Missing before: y=0 with the sign bit set, and both sign-flipped order-8 encodings. D9FF..FF was in the array and is not a low-order point at all — no x satisfies the curve equation for that y — so it moves to a separate k_non_canonical[], with EDFF..FF7F (y=p) and EEFF..FF7F (y=p+1). tests_run was assigned a literal (11) in main() and never incremented, which is how the duplicate call and the two unregistered tests went unnoticed. The TEST macro now increments it, so the total cannot drift. Verified: cmake -DEBLDR_BUILD_TESTS=ON on master FAILS to build, 3 errors same with this commit builds clean ctest 21/21 PASS ctest -DEBLDR_SANITIZE=ON (ASan+UBSan) 21/21 PASS pytest tests/ 24 passed, 1 skipped test_ed25519 14/14 PASS (was 11 claimed, 12 run) discrimination, with `public_key_is_valid_subgroup` disabled: test_ed25519_low_order_keys_rejected FAILS, as it must test_ed25519_non_canonical_... still PASSES — those are refused by unpackneg() on canonicality, a different mechanism, which is the reason they are held in a separate array rather than counted among the eight. * test: derive the suite totals and the Valgrind list instead of restating them Two findings arrived one after another in review, on different PRs, with the same shape underneath: a number or a list that describes what the tests do, written out separately from the thing it describes, with nothing checking the two agree. Fixing them one instance at a time was going to keep producing the same finding, so this fixes the class and adds the guard. **1. Suites stated a total instead of counting one.** The `TEST()` macro incremented `tests_passed`; `main()` assigned `tests_run = <literal>`. A test defined but never wired into `main()` was skipped with a zero exit, and the suite still reported "N/N passed" for an N that was a claim. 18 of 21 suites carried it. It was not theoretical: - `test_ed25519.c` had a hardcoded 11 that masked one test called twice and two never called at all (repaired in #94, which is where this started). - `test_tlv_auth.c` assigns `tests_run` twice in one function -- 8, then 7. The stale 8 survives only because the later assignment wins. Its suite reports 7/7 today by luck. - `test_secure_boot.c` counted nothing and ended `return 0`, so a suite that ran none of its cases still reported success. The ASSERT macro exits on failure, so that return could only ever have signalled the one case it ignored. `TEST()` now increments `tests_run` where it invokes the test, every literal assignment and every `%d/<literal>` in a summary is gone, and `test_secure_boot.c` compares and returns accordingly. **2. The Valgrind list named its suites again by hand**, and had drifted to 17 of 21 -- `test_ecc`, `test_rollback`, `test_secure_boot` and `test_storage` got no memory-safety run, and nothing failed when a name was forgotten. Each `add_test()` now appends to `EBLDR_UNIT_TESTS` and the `foreach` iterates that, so a suite added without touching the block still gets a Valgrind target. Confirmed with a stub `valgrind` on PATH: 21 `valgrind_*` tests are generated, up from 17. **3. tests/unit/test_suite_bookkeeping.py** keeps both from returning. Four checks, no C toolchain needed: no suite hardcodes its own total; every `TEST()` macro counts the test it runs; every defined test is actually called; and the Valgrind list is derived rather than repeated. Three suites with no `TEST()` macro are listed in `NO_TEST_MACRO` with a reason each. Verified: ctest 21/21 PASS ctest -DEBLDR_SANITIZE=ON (ASan+UBSan) 21/21 PASS pytest tests/ 42 passed valgrind targets, with a stub on PATH 21 (was 17) every suite's reported total now equals what it ran -- e.g. test_bootctl 12/12, test_tlv_auth 7/7, test_secure_boot 4/4 Each of the four guards was probed against the defect it is written for: reintroduce `tests_run = 12` -> test_bootctl.c: tests_run = 12 remove `tests_run++` from the macro -> FAIL stop calling a defined TEST() -> FAIL drop a suite from EBLDR_UNIT_TESTS -> FAIL and all four pass again on restore. Refs #94, #80, #82 --------- Co-authored-by: Srikanth Patchava <srikanth.patchava@outlook.com>

…ped (#81) * fix: repair master — the ABI asserts and the Ed25519 verifier both merged broken master (22d8f8b) does not compile. Two independent double-merges, both the same shape: two PRs fixing adjacent things landed on stale bases, each was green on its own branch, and the result was never rebuilt. 1. include/eos_image.h — #93 replaced reserved[30] with tlv_len (2) + tlv_hash[28], preserving every offset. #87 merged afterwards carrying asserts written against the older struct: error: no member named 'reserved' in 'eos_image_header_t' (x2) #93 already asserts tlv_len at 62 and tlv_hash at 64, so the offset assert was a duplicate; the width assert had no replacement and is restored as two asserts covering both halves of the same 30-byte span. No offset moves and the wire format is unchanged. 2. core/ed25519_verify.c — #86 and #57 both landed a subgroup guard, so the file carried two byte-identical point_is_identity() definitions: error: redefinition of 'point_is_identity' Only #57's public_key_is_valid_subgroup() is wired to the call site, so #86's key_has_prime_order() was dead. Kept the live function, folded #86's fuller rationale onto it, deleted the duplicate. 3. tests/unit/test_ed25519.c — collateral from the same merge. Two copies of test_ed25519_identity_key_forgery_rejected, main() calling it twice and two tests not at all, and test_ed25519_low_order_R_with_a_valid_key_is_not_a_forgery referencing k_low_order[] and messages[] that the merge had dropped. While restoring the corpus, corrected it (review finding on #86): the array claimed to hold "the eight low-order point encodings" and held five. Every order here was computed rather than copied — decode y, recover x, add the point to itself until it reaches the identity — giving 1, 2, 4, 4, 8, 8, 8, 8. Missing before: y=0 with the sign bit set, and both sign-flipped order-8 encodings. D9FF..FF was in the array and is not a low-order point at all — no x satisfies the curve equation for that y — so it moves to a separate k_non_canonical[], with EDFF..FF7F (y=p) and EEFF..FF7F (y=p+1). tests_run was assigned a literal (11) in main() and never incremented, which is how the duplicate call and the two unregistered tests went unnoticed. The TEST macro now increments it, so the total cannot drift. Verified: cmake -DEBLDR_BUILD_TESTS=ON on master FAILS to build, 3 errors same with this commit builds clean ctest 21/21 PASS ctest -DEBLDR_SANITIZE=ON (ASan+UBSan) 21/21 PASS pytest tests/ 24 passed, 1 skipped test_ed25519 14/14 PASS (was 11 claimed, 12 run) discrimination, with `public_key_is_valid_subgroup` disabled: test_ed25519_low_order_keys_rejected FAILS, as it must test_ed25519_non_canonical_... still PASSES — those are refused by unpackneg() on canonicality, a different mechanism, which is the reason they are held in a separate array rather than counted among the eight. * ci: make the EoSim workflow able to fail for the reasons it claims to check Answers the review on #81. The install fix was right; the workflow it revives could not have failed for most of what it says it verifies. Finding 1 (Medium) -- the step named "Validate all platform configs" ran `eosim list && eosim doctor` with no platforms/ copy in that job. `eosim list` prints "Available platforms (0)" without it and exits 0, so the step passed having validated nothing -- and the same shape was in windows-sanity and macos-sanity. All three now copy platforms/ from the pinned checkout and assert a non-zero count via discover_platforms(), because the exit code is exactly what cannot be trusted here. Finding 2 (Medium) -- nested-guest-install cloned EoSim a second time with no --branch for its platforms/ copy, so the package came from v${EOSIM_VERSION} while the platform data came from whatever the default branch pointed at that morning. Now copies from the same pinned checkout; the second clone is gone. Finding 3 (Medium) -- `eosim --version` was printed and never asserted, while the tag and the package's declared version disagree upstream (v1.5.0 ships "eosim, version 2.0.0"). Pinning to a tag therefore does not pin what the name suggests, and nothing would have noticed if the tag moved. Now asserted. The mismatch itself is EoSim's bug and is raised there rather than worked around here. Finding 4 (Medium) -- sanity-gate failed only on install-validate and then printed "All EoSim sanity checks passed", which it would do with the other four jobs red. Replaced with the toJSON(needs) + jq body from ci.yml, which cannot fall out of step with `needs:`. #90 adds a test that enforces this across every gate in the repository; this gate passes it. Verified: yaml.safe_load of eosim-sanity.yml parses, 6 jobs the gate now iterates toJSON(needs), no longer branches on install-validate alone, and no longer prints an "all passed" claim #90's test_no_aggregating_gate_ignores_part_of_its_needs, run against this workflow: eosim-sanity.yml is not among its offenders pytest tests/ 38 passed ctest 21/21 PASS NOT RUN: the workflow itself. It is `on: schedule` + `workflow_dispatch` only, so none of this PR's checks execute it -- which is finding 5, and it is the one piece of evidence this PR cannot produce from a fork branch without a maintainer dispatching it. The install sequence was verified locally end to end (clone -> pip install -> eosim --version 2.0.0 -> doctor -> run am62x --headless PASSED); the assertions added here are not covered by that and remain unexecuted. Refs #81

…ing GCM (#80) * fix: repair master — the ABI asserts and the Ed25519 verifier both merged broken master (22d8f8b) does not compile. Two independent double-merges, both the same shape: two PRs fixing adjacent things landed on stale bases, each was green on its own branch, and the result was never rebuilt. 1. include/eos_image.h — #93 replaced reserved[30] with tlv_len (2) + tlv_hash[28], preserving every offset. #87 merged afterwards carrying asserts written against the older struct: error: no member named 'reserved' in 'eos_image_header_t' (x2) #93 already asserts tlv_len at 62 and tlv_hash at 64, so the offset assert was a duplicate; the width assert had no replacement and is restored as two asserts covering both halves of the same 30-byte span. No offset moves and the wire format is unchanged. 2. core/ed25519_verify.c — #86 and #57 both landed a subgroup guard, so the file carried two byte-identical point_is_identity() definitions: error: redefinition of 'point_is_identity' Only #57's public_key_is_valid_subgroup() is wired to the call site, so #86's key_has_prime_order() was dead. Kept the live function, folded #86's fuller rationale onto it, deleted the duplicate. 3. tests/unit/test_ed25519.c — collateral from the same merge. Two copies of test_ed25519_identity_key_forgery_rejected, main() calling it twice and two tests not at all, and test_ed25519_low_order_R_with_a_valid_key_is_not_a_forgery referencing k_low_order[] and messages[] that the merge had dropped. While restoring the corpus, corrected it (review finding on #86): the array claimed to hold "the eight low-order point encodings" and held five. Every order here was computed rather than copied — decode y, recover x, add the point to itself until it reaches the identity — giving 1, 2, 4, 4, 8, 8, 8, 8. Missing before: y=0 with the sign bit set, and both sign-flipped order-8 encodings. D9FF..FF was in the array and is not a low-order point at all — no x satisfies the curve equation for that y — so it moves to a separate k_non_canonical[], with EDFF..FF7F (y=p) and EEFF..FF7F (y=p+1). tests_run was assigned a literal (11) in main() and never incremented, which is how the duplicate call and the two unregistered tests went unnoticed. The TEST macro now increments it, so the total cannot drift. Verified: cmake -DEBLDR_BUILD_TESTS=ON on master FAILS to build, 3 errors same with this commit builds clean ctest 21/21 PASS ctest -DEBLDR_SANITIZE=ON (ASan+UBSan) 21/21 PASS pytest tests/ 24 passed, 1 skipped test_ed25519 14/14 PASS (was 11 claimed, 12 run) discrimination, with `public_key_is_valid_subgroup` disabled: test_ed25519_low_order_keys_rejected FAILS, as it must test_ed25519_non_canonical_... still PASSES — those are refused by unpackneg() on canonicality, a different mechanism, which is the reason they are held in a separate array rather than counted among the eight. * fix(fw_decrypt): drop a HW-crypto shortcut that cannot express streaming GCM core/fw_decrypt.c is a hand-written AES-256-GCM sitting in the secure boot path with no tests. It has an opt-in fast path: if (ops && ops->hw_aes_decrypt) { int rc = ops->hw_aes_decrypt(ctx->key, EOS_AES_KEY_SIZE, ctx->iv, data, data, len); if (rc == EOS_OK) { ctx->bytes_processed += len; return EOS_OK; } } The hook is (key, key_len, iv, in, out, len). That signature cannot carry streaming GCM, and taking the path broke decryption two ways: 1. No counter position. ctx->iv is passed unchanged on every call, so a second chunk restarts the CTR keystream at block 0 and is decrypted against the same keystream as the first. Reusing a CTR keystream across two plaintexts is the one thing the mode must never do. 2. No GHASH. The hook returns plaintext only, so ctx->ghash_acc is never fed. eos_fw_decrypt_final() computes the tag over an empty accumulator and rejects the image -- a board with an AES engine could not install a correctly encrypted firmware update at all. (2) is why this was never noticed: it fails closed, and no board in-tree implements the hook yet. (1) is why it cannot be patched by also feeding GHASH: the plaintext would still be wrong past the first chunk. Re-enabling needs a hook that takes a block offset and either exposes GHASH state or does the whole GCM operation including the tag. Removed, with that written down where the next person will look. Also adds tests/unit/test_fw_decrypt.c -- the first tests this file has had. Vectors come from an independent implementation (Python cryptography, i.e. OpenSSL) rather than from this code, so they pin behaviour rather than recording it: - whole blocks, and a 20-byte payload with a partial trailing block - the same ciphertext split [32] / [16,16] / [10,22] / [1,31] / [5,15]: GCM is a stream, so the result must not depend on how the caller sliced it - a board advertising a working AES engine must reach the same answer as one without -- this is the case that fails on master - every single-bit flip in the tag (128 of them), and in each ciphertext byte, must be rejected - unprovisioned/unreadable OTP keys and uninitialised contexts are refused Verified: 8/8 pass with this change; against the unmodified fw_decrypt.c the suite fails on test_board_with_aes_engine_still_accepts_a_genuine_image, the assertion it exists to make. The software GCM itself is correct -- I checked it against the reference vectors before changing anything, including every chunk split above. Note: the full test build on master is currently broken by test_image_verify.c (fixed in #77), so this target was built directly. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(fw_decrypt): answer the review — stale doc, orphaned hook, anchor clash Three findings from the review on #80, all in files this branch already owns. Finding 2 (Medium) -- core/fw_decrypt.c's file header still read "Falls back to HAL hw_aes_decrypt if available." The diff started at line 192, so that line survived and directly contradicted the twenty-line rationale this PR installs below it. A self-contradictory file is worse than the original. Finding 1 (Medium) -- after the removal, hw_aes_decrypt has zero call sites while eos_hal.h still advertises it under "software fallback used if NULL", which is now false in the other direction: a board author who implements the hook gets nothing, silently, with no diagnostic. That is the same class of failure this PR is fixing. Kept the member rather than deleting it -- removing it from a public struct is an ABI change for out-of-tree boards, and the hook is worth having once it can express the operation -- and documented it as reserved and currently unconsumed, with the reason and with what a streaming-capable replacement would need. Finding 3 (Low) -- this PR and #82 both inserted their add_executable/add_test triple immediately after add_test(NAME test_keystore ...), so whichever landed second would have conflicted for no reason but placement. Re-anchored this one to the end of the registrations, with a comment saying why. Also rebased: this branch was 7 commits behind and its diff against current master would have reverted the test_recovery link-line fix. It is now stacked on #94, which repairs master -- without that, every PR that builds the test suite is red on include/eos_image.h and core/ed25519_verify.c. Verified: cmake --build (EBLDR_BUILD_TESTS=ON) clean ctest 22/22 PASS test_fw_decrypt 8/8 PASS git grep hw_aes_decrypt header + this file's comment only Refs #80 --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

…ugs (#101) * fix: repair master — the ABI asserts and the Ed25519 verifier both merged broken master (22d8f8b) does not compile. Two independent double-merges, both the same shape: two PRs fixing adjacent things landed on stale bases, each was green on its own branch, and the result was never rebuilt. 1. include/eos_image.h — #93 replaced reserved[30] with tlv_len (2) + tlv_hash[28], preserving every offset. #87 merged afterwards carrying asserts written against the older struct: error: no member named 'reserved' in 'eos_image_header_t' (x2) #93 already asserts tlv_len at 62 and tlv_hash at 64, so the offset assert was a duplicate; the width assert had no replacement and is restored as two asserts covering both halves of the same 30-byte span. No offset moves and the wire format is unchanged. 2. core/ed25519_verify.c — #86 and #57 both landed a subgroup guard, so the file carried two byte-identical point_is_identity() definitions: error: redefinition of 'point_is_identity' Only #57's public_key_is_valid_subgroup() is wired to the call site, so #86's key_has_prime_order() was dead. Kept the live function, folded #86's fuller rationale onto it, deleted the duplicate. 3. tests/unit/test_ed25519.c — collateral from the same merge. Two copies of test_ed25519_identity_key_forgery_rejected, main() calling it twice and two tests not at all, and test_ed25519_low_order_R_with_a_valid_key_is_not_a_forgery referencing k_low_order[] and messages[] that the merge had dropped. While restoring the corpus, corrected it (review finding on #86): the array claimed to hold "the eight low-order point encodings" and held five. Every order here was computed rather than copied — decode y, recover x, add the point to itself until it reaches the identity — giving 1, 2, 4, 4, 8, 8, 8, 8. Missing before: y=0 with the sign bit set, and both sign-flipped order-8 encodings. D9FF..FF was in the array and is not a low-order point at all — no x satisfies the curve equation for that y — so it moves to a separate k_non_canonical[], with EDFF..FF7F (y=p) and EEFF..FF7F (y=p+1). tests_run was assigned a literal (11) in main() and never incremented, which is how the duplicate call and the two unregistered tests went unnoticed. The TEST macro now increments it, so the total cannot drift. Verified: cmake -DEBLDR_BUILD_TESTS=ON on master FAILS to build, 3 errors same with this commit builds clean ctest 21/21 PASS ctest -DEBLDR_SANITIZE=ON (ASan+UBSan) 21/21 PASS pytest tests/ 24 passed, 1 skipped test_ed25519 14/14 PASS (was 11 claimed, 12 run) discrimination, with `public_key_is_valid_subgroup` disabled: test_ed25519_low_order_keys_rejected FAILS, as it must test_ed25519_non_canonical_... still PASSES — those are refused by unpackneg() on canonicality, a different mechanism, which is the reason they are held in a separate array rather than counted among the eight. * fix(fuzz): point every harness at the API it claims to fuzz Four of the five fuzz harnesses did not test what they name, and the build could not say so because not one of them includes a project header. Each declares its target itself: fuzz_recovery_protocol.c extern int eos_recovery_parse_packet(...) fuzz_fw_update.c extern int eos_fw_update_init(void) extern int eos_fw_update_process_chunk(...) extern int eos_fw_update_finalize(void) fuzz_bootctl.c extern int eos_bootctl_parse(...) fuzz_image_verify.c extern int eos_image_parse_header(const void *, size_t) eos_recovery_parse_packet, eos_fw_update_init, eos_fw_update_process_chunk and eos_bootctl_parse have never existed — those names appear nowhere outside the harness that declares them, so three of the five targets have never linked. The other two are worse, because they do link: - eos_fw_update_finalize exists but takes (ctx, mode), not (void). - eos_image_parse_header takes (uint32_t addr, eos_image_header_t *), not (const void *, size_t). fuzz_image_verify has been passing a pointer where an address is expected and a length where an output struct is expected, on every input, for as long as it has run. A local extern is why none of this was caught: it tells the compiler the symbol exists with whatever shape the harness asserts, and the mismatch survives to link time or past it. All five now include the real header and drive the real entry points: fuzz_image_verify parse_header -> verify_integrity, boot-path order fuzz_bootctl bootctl_load, then trap if load() accepted a block validate() rejects fuzz_fw_update begin -> write in fuzz-chosen chunk widths -> finalize or abort fuzz_recovery_protocol write_in_range, trapping on any accepted write that leaves the slot or wraps, checked in wider types that cannot fuzz_crypto already correct; switched to the header so it stays that way Adds tests/fuzz/fuzz_sim_flash.h: the three harnesses that reach flash need board ops installed, and without them they fuzz a null op table. That the compiler now checks this is not theoretical — writing this, it rejected EOS_UPGRADE_MODE_TEST for EOS_UPGRADE_TEST immediately, which is exactly the error class the externs were hiding. Verified. libFuzzer is unavailable here (libclang_rt.fuzzer_osx.a not found), so each harness was linked against eboot_core and driven by a standalone seed generator under ASan+UBSan: all five compile -Wall -Wextra 5/5, 0 failures 20,000 inputs each 100,000 total, no reports ctest 21/21 passed Third instance of this class across these repos, after eos#50 (fuzz_devicetree naming eos_dtb_parse) and eos#110 (fuzz_ota_header naming eos_ota_parse_header). The common factor every time is a fuzz target declaring its own subject. Stacked on #94, without which include/eos_image.h does not compile. * fix(fuzz): the recovery oracle was off by one, and CI proved it The restored Fuzz Harness Build job on #101 ran these harnesses under real libFuzzer for the first time and fuzz_recovery_protocol trapped within seconds. That was my bug, not the code's. The oracle computed the one-past-the-end address: end = base + offset + len; if (end > 0xFFFFFFFF) trap; The last byte a write of len bytes touches is base + offset + len - 1. eos_recovery_write_in_range() checks exactly that: if ((uint32_t)len - 1u > UINT32_MAX - (base + offset)) reject; So an accepted write whose final byte lands exactly on 0xFFFFFFFF -- base=0xFFFFFF00, offset=0, len=256 -- is legal, the function accepts it, and the old oracle trapped on it. A harness that traps on valid input reports the opposite of the truth. Verified both directions: the boundary case, fed directly -> no trap a fail-open stub accepting all input -> trap fires (exit 133) 500,000 random inputs, ASan+UBSan -> no reports libFuzzer doing precisely what it is for -- driving inputs into the one corner a hand-written oracle got wrong -- is the strongest argument yet for #101's job. The guard could never have caught this; only execution under a coverage-driven fuzzer did. * ci(fuzz): compile the harnesses, and refuse the extern that hid the bugs #99 repairs the five harnesses in tests/fuzz/. Neither it nor anything else stops the same thing happening again, because two of the conditions that produced it are still in place. **Nothing compiles them.** tests/fuzz/ is behind EBLDR_BUILD_FUZZ, which defaults OFF, and no job set it. That is the whole reason three harnesses could name functions that exist nowhere -- eos_bootctl_parse, eos_recovery_parse_packet, eos_fw_update_init and friends -- and sit there for as long as they did. A harness that is not built cannot fail to build. The fuzz-build job restores that: clang, EBLDR_BUILD_FUZZ=ON (and EBLDR_BUILD_TESTS=ON, since tests/fuzz is added from tests/CMakeLists.txt, so the option alone would build nothing and the job would pass having compiled no harness at all), then a five-second smoke run of each target. It does not fuzz; a real campaign belongs in a scheduled workflow. **The `extern` is still legal.** Every one of the four defects came from a hand-written prototype standing in for an #include -- a promise the compiler is obliged to believe and has no way to check. It is what made the fourth defect invisible even to a compiler: fuzz_image_verify declared eos_image_parse_header as (const void *, size_t) when it is (uint32_t, eos_image_header_t *), so that harness linked, ran, and fuzzed nothing. tests/unit/test_fuzz_harnesses.py refuses the pattern, and also checks that every harness is built by a CMake target, links eboot_core, and defines LLVMFuzzerTestOneInput. The two halves cover different failures: the guard is static and stops the pattern being written, the job is a compiler and catches a signature that drifts under a harness that includes the right header. Neither subsumes the other. Mutation-checked: restoring one extern prototype fails test_no_harness_declares_its_own_prototypes; deleting one add_executable fails test_every_harness_is_built. Verified: pytest tests/unit 26 passed 1 skipped, ctest 21/21. The fuzz job itself could not be run here -- Apple clang ships no libclang_rt.fuzzer, so -fsanitize=fuzzer will not link on this host. Instead each of #99's five harnesses was compiled and linked against eboot_core with a stand-in driver under -fsanitize=address,undefined and run over ~20k pseudo-random inputs plus every length from 0 to 200; all five clean. That covers the symbol resolution and the harness logic, which is what the job's build step checks; it does not prove the libFuzzer link itself, which only CI can.

* fix: repair master — the ABI asserts and the Ed25519 verifier both merged broken master (22d8f8b) does not compile. Two independent double-merges, both the same shape: two PRs fixing adjacent things landed on stale bases, each was green on its own branch, and the result was never rebuilt. 1. include/eos_image.h — #93 replaced reserved[30] with tlv_len (2) + tlv_hash[28], preserving every offset. #87 merged afterwards carrying asserts written against the older struct: error: no member named 'reserved' in 'eos_image_header_t' (x2) #93 already asserts tlv_len at 62 and tlv_hash at 64, so the offset assert was a duplicate; the width assert had no replacement and is restored as two asserts covering both halves of the same 30-byte span. No offset moves and the wire format is unchanged. 2. core/ed25519_verify.c — #86 and #57 both landed a subgroup guard, so the file carried two byte-identical point_is_identity() definitions: error: redefinition of 'point_is_identity' Only #57's public_key_is_valid_subgroup() is wired to the call site, so #86's key_has_prime_order() was dead. Kept the live function, folded #86's fuller rationale onto it, deleted the duplicate. 3. tests/unit/test_ed25519.c — collateral from the same merge. Two copies of test_ed25519_identity_key_forgery_rejected, main() calling it twice and two tests not at all, and test_ed25519_low_order_R_with_a_valid_key_is_not_a_forgery referencing k_low_order[] and messages[] that the merge had dropped. While restoring the corpus, corrected it (review finding on #86): the array claimed to hold "the eight low-order point encodings" and held five. Every order here was computed rather than copied — decode y, recover x, add the point to itself until it reaches the identity — giving 1, 2, 4, 4, 8, 8, 8, 8. Missing before: y=0 with the sign bit set, and both sign-flipped order-8 encodings. D9FF..FF was in the array and is not a low-order point at all — no x satisfies the curve equation for that y — so it moves to a separate k_non_canonical[], with EDFF..FF7F (y=p) and EEFF..FF7F (y=p+1). tests_run was assigned a literal (11) in main() and never incremented, which is how the duplicate call and the two unregistered tests went unnoticed. The TEST macro now increments it, so the total cannot drift. Verified: cmake -DEBLDR_BUILD_TESTS=ON on master FAILS to build, 3 errors same with this commit builds clean ctest 21/21 PASS ctest -DEBLDR_SANITIZE=ON (ASan+UBSan) 21/21 PASS pytest tests/ 24 passed, 1 skipped test_ed25519 14/14 PASS (was 11 claimed, 12 run) discrimination, with `public_key_is_valid_subgroup` disabled: test_ed25519_low_order_keys_rejected FAILS, as it must test_ed25519_non_canonical_... still PASSES — those are refused by unpackneg() on canonicality, a different mechanism, which is the reason they are held in a separate array rather than counted among the eight. * ci: gate what the repository actually reports, and close the sibling gate that failed open Answers the review on #90. Finding 1 (Medium) -- `CI Gate` gates three jobs in ci.yml. This repository has 16 workflow files and a PR head reports 26 checks across five runs, so the maintainer action in the body -- require one name -- would have left CodeQL, every EoSim platform leg, all three Cross-Platform legs and a second host build unrequired, and `master` could still go red from any of them. The rot-guard had the same boundary: WORKFLOW was hardcoded to ci.yml, so a job added to build.yml or codeql.yml was neither covered nor noticed. Adds two tests over every workflow with a `pull_request` trigger: REQUIRED_CHECKS names what a maintainer must actually require, NO_GATE excuses the rest with a reason about the workflow itself, and a third test asserts each gated workflow really has a job displaying under the name given. Finding 2 (Medium) -- `Simulation Gate` had the exact fail-open shape this PR removes from ci.yml: `needs: [simulate, cross-platform]`, then it printed both results and branched on `simulate` alone before printing "All simulation checks passed". A red or skipped `cross-platform` -- three OS legs -- passed it. Ported the `toJSON(needs)` + `jq` body, which is dependency-list-agnostic and cannot fall out of step with `needs:`. And made it a rule rather than a one-off: test_no_aggregating_gate_ignores_part_of_its_needs walks every gate in every pull-request workflow and fails if a declared dependency is never compared. Two refinements were needed to make it mean something: - printing a result is not testing it. The first version grepped for `needs.X.result` anywhere in the script, which Simulation Gate satisfied with its echo line. Only a line that compares counts. - book-build.yml's `summary` writes a step summary and claims no verdict. It is reporting, not gating, so the check applies only to jobs that either `exit 1` or assert that everything passed. Finding 3 (Low) -- this branch had replaced master's `pip3 install -r requirements.txt pytest-cov` with a hand-maintained list to add pyyaml, which regressed the guard in tests/unit/test_requirements.py: these jobs run pytest over tests/ but never install from requirements.txt ... ['ci.yml:test'] Restored the requirements.txt install; pyyaml was already declared there, so the hand list was not needed at all. That also removes the collision with #88, which edits the same line. Verified: pytest tests/ 47 passed ctest 21/21 PASS yaml.safe_load of ci.yml, simulation-test.yml, book-build.yml all parse discrimination, both ways: - reverting Simulation Gate to its fail-open body gives simulation-test.yml:sanity-gate declares needs ['simulate', 'cross-platform'] but never tests ['cross-platform'] - with the toJSON(needs) body in place, 9 passed Refs #90 * ci: execute the gate rule in a test, instead of grepping for it Finishes finding 3 from the review on #90. Findings 1 and 2 were already addressed in 1bd6a22; this is the one left. test_gate_fails_on_any_non_success_result searched the gate's `run:` text for `!= "success"` and `exit 1`. That is a string match on an implementation, not a check of behaviour: it passes for those tokens sitting in a comment or an unreachable branch, and fails for a correct rewrite expressing the same rule differently. The only real evidence that the rule works was the fork experiment, which is a one-off nothing re-runs. The rule now lives in .github/scripts/ci-gate-check.sh, and pytest runs it against real inputs: success, failure, skipped, cancelled, a mixed set, and an empty context. Both gates -- ci.yml's `CI Gate` and simulation-test.yml's `Simulation Gate` -- call that one script, and a further test asserts they do, so the behavioural cases cover every gate rather than one. The script also refuses an empty or null `needs` context. No results is not the same as no failures, and a gate that passes when it was handed nothing is the same fail-open shape in a different place. Verified by mutation: weakening the rule to `== "failure"` fails 3 of 18, and a gate that stops calling the script fails 1 of 18. Restored, 18 pass. 42 Python tests pass (1 skipped), ctest 20/20, all 16 workflow files parse.

* fix: repair master — the ABI asserts and the Ed25519 verifier both merged broken master (22d8f8b) does not compile. Two independent double-merges, both the same shape: two PRs fixing adjacent things landed on stale bases, each was green on its own branch, and the result was never rebuilt. 1. include/eos_image.h — #93 replaced reserved[30] with tlv_len (2) + tlv_hash[28], preserving every offset. #87 merged afterwards carrying asserts written against the older struct: error: no member named 'reserved' in 'eos_image_header_t' (x2) #93 already asserts tlv_len at 62 and tlv_hash at 64, so the offset assert was a duplicate; the width assert had no replacement and is restored as two asserts covering both halves of the same 30-byte span. No offset moves and the wire format is unchanged. 2. core/ed25519_verify.c — #86 and #57 both landed a subgroup guard, so the file carried two byte-identical point_is_identity() definitions: error: redefinition of 'point_is_identity' Only #57's public_key_is_valid_subgroup() is wired to the call site, so #86's key_has_prime_order() was dead. Kept the live function, folded #86's fuller rationale onto it, deleted the duplicate. 3. tests/unit/test_ed25519.c — collateral from the same merge. Two copies of test_ed25519_identity_key_forgery_rejected, main() calling it twice and two tests not at all, and test_ed25519_low_order_R_with_a_valid_key_is_not_a_forgery referencing k_low_order[] and messages[] that the merge had dropped. While restoring the corpus, corrected it (review finding on #86): the array claimed to hold "the eight low-order point encodings" and held five. Every order here was computed rather than copied — decode y, recover x, add the point to itself until it reaches the identity — giving 1, 2, 4, 4, 8, 8, 8, 8. Missing before: y=0 with the sign bit set, and both sign-flipped order-8 encodings. D9FF..FF was in the array and is not a low-order point at all — no x satisfies the curve equation for that y — so it moves to a separate k_non_canonical[], with EDFF..FF7F (y=p) and EEFF..FF7F (y=p+1). tests_run was assigned a literal (11) in main() and never incremented, which is how the duplicate call and the two unregistered tests went unnoticed. The TEST macro now increments it, so the total cannot drift. Verified: cmake -DEBLDR_BUILD_TESTS=ON on master FAILS to build, 3 errors same with this commit builds clean ctest 21/21 PASS ctest -DEBLDR_SANITIZE=ON (ASan+UBSan) 21/21 PASS pytest tests/ 24 passed, 1 skipped test_ed25519 14/14 PASS (was 11 claimed, 12 run) discrimination, with `public_key_is_valid_subgroup` disabled: test_ed25519_low_order_keys_rejected FAILS, as it must test_ed25519_non_canonical_... still PASSES — those are refused by unpackneg() on canonicality, a different mechanism, which is the reason they are held in a separate array rather than counted among the eight. * test(ed25519): hold both repos' verifiers to one shared contract eos and eBoot each carry their own Ed25519 verifier. Different code, opposite return conventions — eos returns 1 to accept, eBoot returns EOS_OK — doing the same job on the same wire format. For a while only eos rejected low-order public keys, and nothing in either repo could notice: there is no shared build, and the implementations are far too different for a source diff to say anything. That divergence is why #73 survived in eBoot after eos had already fixed it. Fixing eBoot closes the hole; it does not stop the next crypto fix in either repo from failing to reach the other. What can be shared is the data. tests/vectors/ed25519_contract_vectors.h is 76 vectors of pure test data with a SHA-256 over them: 64 low-order keys — the eight points of order dividing 8, each over eight messages. A key of order n makes (R = identity, S = 0) verify whenever n divides SHA-512(R||A||M), about one message in n, so a test pinned to a single message passes against unfixed code. 3 RFC 8032 section 7.1 signatures that must verify 6 the same three with one bit flipped in R or in S 3 the same three with S + L, non-canonical per RFC 8032 5.1.7 The intended twin is tests/test_ed25519_contract.c in eos, compiling the byte-identical header. Each side runs its own verifier and prints the digest; a change to one copy that does not reach the other shows up as two different digests, which is the part a diff could never give. The header is generated, so the corpus is reproducible rather than transcribed: python3 tools/gen_ed25519_contract_vectors.py The three positive vectors are load-bearing and the test says so: a verifier that refuses everything satisfies all 73 negative vectors, so it also fails if the corpus ever loses its accept cases. Verified: on this branch (with the #86 fix) -> 76/76, 3 accepted, 73 refused against unfixed origin/master -> 28 of 76 wrong, exit 1 ctest -> 22/22 passed digest 1059febedae2b3e3dfaa1ef5b419fb37ed7f0d53b377a3250b53b95a546282a2 * build(tests): generate the Ed25519 contract vectors, do not commit them tests/vectors/ed25519_contract_vectors.h is 54 KB of generated data, and the identical file was checked in to BOTH eos and eBoot, along with the generator that produces it -- 61 KB duplicated verbatim across two repositories and kept in step by hand. The generator is pure stdlib and deterministic, so each repo can produce the header into its own build tree instead. tools/gen_ed25519_contract_vectors.py grows an -o/--output flag (stdout stays the default, so the regeneration command in its docstring still works), and tests/CMakeLists.txt drives it from add_custom_command. Only the 6.8 KB generator is now carried twice, and it is source rather than generated output. Verified: the generated header is byte-identical to the file it replaces; both repos still report digest 1059febedae2b3e3dfaa1ef5b419fb37ed7f0d53b377a3250b53b95a546282a2, which is the thing that actually holds the two verifiers to one contract. Suites pass (eos 35/35, eBoot 22/22). Breaking the generator fails the build rather than silently reusing a stale header. The cross-compile legs configure with tests OFF so find_package(Python3) is never reached there; confirmed by configuring under a cross toolchain. The test's include of the vector header becomes "vectors/..." rather than "../vectors/...": a path relative to the source file cannot reach the generated copy in the build tree. * test(ed25519): assert the contract digest instead of printing it Answers the review on #89. Finding 1 (High) -- the digest was printf'd and never compared to anything, so the drift guard the PR is named for did not exist. Worse, it could not: EOS_ED25519_CONTRACT_DIGEST came from the header this repo's own generator had just written, a hash taken over its own output. Editing the generator on one side produced a self-consistent corpus with a new digest, a green test, and a divergence visible only to a human reading two CI logs in two repositories. Three places asserted the guarantee -- the PR body, this file's docblock, and the generator's emitted header comment -- and none implemented it. The unused <string.h> include was the strcmp that never got written. Now pinned as a literal in committed source, with the vector count and the accept count beside it, because those are generated too: a corpus that shrank from 76 to 40, or lost two of its three RFC 8032 positives, previously passed. positives == 0 only fired when the last positive went. Finding 2 (Medium) -- tests/vectors/ed25519_contract_vectors.h is now committed rather than generated into the build tree. What the TCB's signature verifier was tested against should be visible from a tag. The reason it was generated -- two repos holding the same file and keeping it in step by hand -- is what the pinned digest now handles. That also drops find_package(Python3 ... REQUIRED), which had made a Python interpreter a hard configure-time dependency of a pure-C test suite. Finding 3 (Medium) -- #86 has merged, so this branch is rebased onto master and reduced to its own three files. It previously carried all of #84 and #85 (device tree parsing, +475 lines, unrelated to Ed25519) plus #86's verifier change, none of which was declared in the body. Verified: ctest 22/22 PASS test_ed25519_contract 76 vectors, 3 accept, 73 refuse the guard now discriminates: adding one vector to the generator gives [FAIL] corpus digest changed expected 1059febe...282a2 got e08b0632...28905 and exit 1. Before this commit the same edit printed a different digest and exited 0. cmake configure with no Python on PATH succeeds (no find_package) Refs #89 * test(ed25519): hash the vectors, not a sibling #define Answers the second review on #89. Finding 1 is a defect in my own previous commit, and it is the same class that commit was written to fix. Finding 1 (High) -- the pin did not cover the corpus it ships with. EOS_ED25519_CONTRACT_DIGEST is a #define inside the generated header, written there by the generator; EOS_ED25519_CONTRACT_EXPECTED is a #define in this file. The test compared one string literal to another. Nothing hashed the vector bytes, so editing a committed vector changed the data and left the value it was compared against untouched. The reviewer demonstrated it by changing one byte of the order-1 identity public key -- the vector that pins the Critical secure-boot bypass -- and the suite still reported "all 76 contract vectors behaved as specified". The guard fired only when someone re-ran the generator: the file it protects was the one file it could not see. Committing the corpus in the previous commit is exactly what made that gap matter. contract_digest() now recomputes SHA-256 over the vectors at run time, in the generator's serialisation -- public_key[32] || signature[64] || message[message_len] || (uint8_t)expect_accept, per vector, in order -- using eos_sha256_* from eos_crypto_boot.h, which this file already included. Two comparisons follow: the computed digest against the pinned literal, and the computed digest against the header's own claim, so a corpus and a header from different generator runs are caught too. Reproduced the reviewer's probe. With one byte of the identity key changed: before this commit: [PASS] all 76 contract vectors behaved as specified after: [FAIL] corpus digest changed expected 1059febe...282a2 got 55370deb...cae9 exit 1 Finding 2 (Low) -- three places in committed source asserted the guarantee finding 1 showed was not implemented, the generator docstring most explicitly: "if either copy is edited, that side's build fails and the digests visibly stop matching". It did not. With finding 1 fixed the claim is true, and all three are reworded to say what now actually holds -- that the digest is recomputed over the bytes, so a hand-edited vector fails as loudly as a regenerated corpus. That distinction is the point: the hand edit is the one a reviewer would not catch in a diff of 76 byte arrays. The regenerated header differs from the committed one only in that comment; the digest and every vector are byte-identical. Verified: ctest 22/22 PASS pytest tests/ 38 passed test_ed25519_contract 76 vectors, 3 accept, 73 refuse byte-flip probe FAILS, as shown above regenerate + diff data unchanged, comment only Refs #89

…85) * fix: repair master — the ABI asserts and the Ed25519 verifier both merged broken master (22d8f8b) does not compile. Two independent double-merges, both the same shape: two PRs fixing adjacent things landed on stale bases, each was green on its own branch, and the result was never rebuilt. 1. include/eos_image.h — #93 replaced reserved[30] with tlv_len (2) + tlv_hash[28], preserving every offset. #87 merged afterwards carrying asserts written against the older struct: error: no member named 'reserved' in 'eos_image_header_t' (x2) #93 already asserts tlv_len at 62 and tlv_hash at 64, so the offset assert was a duplicate; the width assert had no replacement and is restored as two asserts covering both halves of the same 30-byte span. No offset moves and the wire format is unchanged. 2. core/ed25519_verify.c — #86 and #57 both landed a subgroup guard, so the file carried two byte-identical point_is_identity() definitions: error: redefinition of 'point_is_identity' Only #57's public_key_is_valid_subgroup() is wired to the call site, so #86's key_has_prime_order() was dead. Kept the live function, folded #86's fuller rationale onto it, deleted the duplicate. 3. tests/unit/test_ed25519.c — collateral from the same merge. Two copies of test_ed25519_identity_key_forgery_rejected, main() calling it twice and two tests not at all, and test_ed25519_low_order_R_with_a_valid_key_is_not_a_forgery referencing k_low_order[] and messages[] that the merge had dropped. While restoring the corpus, corrected it (review finding on #86): the array claimed to hold "the eight low-order point encodings" and held five. Every order here was computed rather than copied — decode y, recover x, add the point to itself until it reaches the identity — giving 1, 2, 4, 4, 8, 8, 8, 8. Missing before: y=0 with the sign bit set, and both sign-flipped order-8 encodings. D9FF..FF was in the array and is not a low-order point at all — no x satisfies the curve equation for that y — so it moves to a separate k_non_canonical[], with EDFF..FF7F (y=p) and EEFF..FF7F (y=p+1). tests_run was assigned a literal (11) in main() and never incremented, which is how the duplicate call and the two unregistered tests went unnoticed. The TEST macro now increments it, so the total cannot drift. Verified: cmake -DEBLDR_BUILD_TESTS=ON on master FAILS to build, 3 errors same with this commit builds clean ctest 21/21 PASS ctest -DEBLDR_SANITIZE=ON (ASan+UBSan) 21/21 PASS pytest tests/ 24 passed, 1 skipped test_ed25519 14/14 PASS (was 11 claimed, 12 run) discrimination, with `public_key_is_valid_subgroup` disabled: test_ed25519_low_order_keys_rejected FAILS, as it must test_ed25519_non_canonical_... still PASSES — those are refused by unpackneg() on canonicality, a different mechanism, which is the reason they are held in a separate array rather than counted among the eight. * fix(fdt): bound a device tree parser that was never compiled core/fdt_loader.c is in no source list, so it has never been built. The header offers it to callers, rtos_boot.c is named as the consumer, and nothing catches what is in it -- not the compiler, not ctest, not the sanitizer job. What is in it is a parser for a blob that comes out of flash, whose every header field it uses as an offset or a length without checking any of them. eos_fdt_validate() looks at magic and version only, so it accepts a blob whose off_dt_struct points anywhere: hdr.totalsize = 40 (the header alone) hdr.off_dt_struct = 0x100000 eos_fdt_validate(blob) -> 0 eos_fdt_get_prop(...) -> AddressSanitizer: BUS, READ at fdt_loader.c:25 in fdt_read_u32 Five more, all reachable the same way: * the tag read at the top of the loop takes 4 bytes where the loop condition guarantees 1; * strlen() on a node name reads until it finds a zero, which for a name running to the end of the block is past it; * nameoff indexes the strings block unchecked, so strcmp() reads from an arbitrary address; * a property len is clamped to the caller's buffer before the memcpy, which bounds the write but not the read -- an oversized len copies whatever follows the blob out to the caller; * FDT_END_NODE decrements depth with no floor. Bound them. validate() is the gate every path goes through, so the block offsets are checked against totalsize there, and get_prop() calls it before trusting the header. Inside the loop each read is checked for the width it takes, the name and property-name scans are bounded by memchr within their blocks, and the padded advances are re-checked for overrun. Adds the file to eboot_core and tests/unit/test_fdt_loader.c to ctest: ten cases, one well-formed tree that must still parse and nine malformed ones. Against the unfixed parser the suite fails on the third; with the bounds in place the whole suite is 20/20, and 20/20 under EBLDR_SANITIZE. Not fixed here, because it is a behaviour change rather than a safety one: node paths below the root do not resolve. _get_prop derives the depth to match from a slash count, so "/chosen" looks for depth 1, which is the root -- "chosen" is at depth 2. Only "/" resolves today. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(fdt): bound the parser against the caller's length, not the blob's claim Follow-up to the review on #84. Three findings from it, all in files this branch already owns. Finding 1 (High) -- every bound in eos_fdt_validate() was expressed relative to hdr->totalsize, which is read out of the same untrusted blob. Checking the block offsets against it proves the header is internally consistent and says nothing about how many bytes are really mapped: a 40-byte buffer declaring totalsize = 0x100000 with agreeing offsets passed every check, and eos_fdt_get_prop() then walked a megabyte past the allocation. The original reproducer (an inflated *offset* against an honest totalsize) was rejected; its mirror image was not. Adds the length-carrying entry points eos_fdt_validate_sized() and eos_fdt_get_prop_sized(), which take the bytes the caller actually owns and check totalsize against that before anything else. The existing two-argument forms stay as wrappers that trust the blob's own totalsize, documented in the header as a warrant the caller has to make good; eos_fdt_load() now passes max_size, which it had all along. Finding 2 (Medium) -- a property larger than the caller's buffer was copied short and reported as success, so a clipped value was indistinguishable from a complete one. In a boot path this reads bootargs. Now returns -7 and sets *buf_len to the full length so the caller can size a retry. Finding 3 (Medium) -- adds tests/fuzz/fuzz_fdt.c alongside the five existing harnesses. .ai/security.md names device tree among the parsers that get fuzz coverage rather than unit tests alone. It drives the sized entry points and passes the real size, which is what makes the harness meaningful -- the unsized forms would let a fuzzer authorise its own out-of-bounds read. Finding 6 (Low) -- drops the unreachable `total < sizeof(fdt_header_t)` check in eos_fdt_load(); validate() already rejects that blob, and the comment above it described a bound it was not applying. Header now documents the full -1..-8 return code table. Verified: cmake -DEBLDR_BUILD_TESTS=ON, ctest 21/21 PASS same under -DEBLDR_SANITIZE=ON (ASan+UBSan) 21/21 PASS test_fdt_loader 14 tests PASS (was 10) new tests against the pre-fix parser both FAIL as they should - inflated totalsize accepted at the sized entry point - truncating call returned 0 instead of -7 grep for eos_fdt_get_prop / eos_fdt_validate no callers outside the parser and its tests, so the -7 change breaks nothing today Refs #84 * fix(build): drop the duplicate core/fdt_loader.c registration master gained core/fdt_loader.c in eboot_core while this branch was open, so after the rebase it appeared twice and tests/unit/test_cmake_core_sources.py::test_core_sources_are_registered_once failed: AssertionError: ['core/fdt_loader.c'] is not false : duplicate eboot_core sources: ['core/fdt_loader.c'] Kept master's entry, dropped the one this branch added. That guard is the same shape as the one this stack adds for toolchain specs -- a build-file check that catches the class rather than the instance -- and it did its job. Verified: ctest 22/22 PASS, pytest 24 passed 1 skipped. Refs #84 * fix(fdt): one length-carrying API, and a CI job that builds the fuzz harnesses Answers the second review on #84. Finding 2 (Medium) -- eos_fdt_validate() and eos_fdt_get_prop() kept an out-of-bounds read by design. Both dereferenced hdr->totalsize before any length was known and handed that attacker-controlled value to the _sized form as its bound, so calling either on a short buffer read past it before a single check had run. The header called that a caller warrant; a warrant is not a check, and it is the exact bug this PR exists to fix. Neither had an in-tree caller. Removed rather than documented. There is now one form of each entry point and it always takes the length: int eos_fdt_validate(const void *fdt_blob, uint32_t avail); int eos_fdt_get_prop(const void *fdt, uint32_t fdt_len, ...); An exported unsafe twin in a TCB header is a future boot-path caller reintroducing the bug with no compiler complaint, which is worth more than the convenience of a one-argument call. eos_fdt_load() already passed max_size. The test that pinned the wrapper's behaviour is gone with the wrapper, and get_prop_sized_exact() collapsed into get_prop_exact() since they became the same function. Finding 3 (Medium) -- tests/fuzz/ was built by nothing. EBLDR_BUILD_FUZZ defaults OFF and no job set it, so the harness added here joined five others that no CI job compiles. A harness that is never built cannot fail to build, which is how fuzz_devicetree came to declare a function that did not exist and sit there unnoticed (eos#50). Adds a `fuzz-build` job: configure with clang, build every harness, and run each for five seconds over its own generated inputs. That is not a campaign -- it is enough to catch a harness that no longer compiles or crashes at once, which is the failure this repo has actually had. Note it needs EBLDR_BUILD_TESTS=ON as well: tests/fuzz/ is added from tests/CMakeLists.txt, so EBLDR_BUILD_FUZZ alone configures cleanly and builds no harness at all -- the job would have passed having compiled nothing. Found that locally before writing the job, not after. NOT RUN, and this is the honest limit: this host has no libFuzzer runtime (libclang_rt.fuzzer_osx.a is absent from the Xcode toolchain), so the link step cannot be reproduced here for any harness, old or new. What I verified is that CMake configures with both flags and reports all six targets -- "Fuzz targets: fuzz_image_verify, fuzz_recovery_protocol, fuzz_fw_update, fuzz_crypto, fuzz_bootctl, fuzz_fdt" -- and that fuzz_fdt.c passes `cc -fsyntax-only`. The link and the smoke run are CI's to show, and this job is what makes them visible. Coordination note: #90 adds a `CI Gate` whose needs list is [test, build-arm, static-analysis]. Whichever of #84 and #90 lands second must add fuzz-build to that list -- and #90's own test_gate_covers_every_job_that_runs_on_a_pull_request fails loudly if it is not, which is the guard working rather than a trap. Finding 1 (High) is a PR-body correction, made there: the diff carries #94's master repair because this branch is stacked on it, and the body described only the FDT parser. Verified: ctest 22/22 PASS pytest tests/ 38 passed test_fdt_loader 13 tests PASS grep for a length-free entry point none remains in the header Refs #84 * ci: move the fuzz-harness build to its own PR The fuzz-build job added in the previous commit does its job -- it is red, on harnesses this PR did not write: undefined reference to `eos_fw_update_init' undefined reference to `eos_fw_update_process_chunk' undefined reference to `eos_recovery_parse_packet' None of those exist anywhere in the tree. tests/fuzz/fuzz_fw_update.c and fuzz_recovery_protocol.c declare an API that was never written, exactly as eos#50's fuzz_devicetree declared eos_dtb_parse(). They have compiled forever because nothing ever linked them. That is a real finding and it deserves a fix, not a red tick on an unrelated PR. The job and the repairs move to their own change; this PR keeps tests/fuzz/fuzz_fdt.c, which is the harness it is responsible for. Refs #84 * fix(fdt): read the header the way the struct block is read, and stop resynchronising on garbage Answers the third review on #84. Finding 3 (Low, and the one that matters on hardware) -- the header was read through a cast fdt_header_t* and direct member loads, while every struct-block read goes through the fdt_read_u32() memcpy helper precisely because the blob may be unaligned. Fuzz input, a buffer inside a larger message, a copy at an odd offset: nothing promises 4-byte alignment, and on a strict-alignment cross target a direct member load is the same fault class this parser exists to avoid. The unit suite could never catch it -- blob_t.bytes happens to be aligned. All header fields now go through fdt_hdr_u32() (memcpy at offsetof), one rule for the whole blob, in validate(), load() and get_prop(). FUZZ_FLAGS gains `undefined` so the fuzz job checks alignment too. Finding 4 (Low) -- `default: break;` resynchronised on unrecognised tags: the walk skipped 4 bytes and treated whatever followed as the next token, so a struct block of arbitrary bytes parsed to a clean "not found". Bounded, but a TCB parser that walks garbage to completion is accepting input it does not understand. FDT_NOP -- the one legal unknown, padding the spec allows between tokens -- is now named in the header and passes; anything else returns -6. Findings 1, 2 and the -8 line: the fuzz-build job's single home is #101 (nothing added here; the harness is inert until that job lands and then compiled by it -- said on the thread, not just here), and the header no longer documents return code -8, which nothing at this head returns. #85 adds the -8 return and re-documents it together with the FDT_MAX_PATH_DEPTH move. Verified: ctest 22/22 PASS ctest -DEBLDR_SANITIZE=ON (ASan+UBSan) 22/22 PASS pytest tests/ 38 passed test_fdt_loader 16 tests PASS (was 13) discrimination, each fix in isolation: default: break restored -> test_a_garbage_tag_is_refused_not_skipped FAILS (expects -6, gets "not found") direct member load restored -> under UBSan the new unaligned-blob test reports "load of misaligned address 0x...671 for type 'const uint32_t'" at the exact line -- and cannot fire on the fixed code, which runs the same test clean NOP counter-check: interleaved FDT_NOP tokens still resolve, so the stricter default does not reject real trees. Refs #84 * fix(fdt): resolve a node by its path, not by its depth and last name Replaces slash-counting and last-component matching with a component walk gated on ancestor matches, so /soc/uart and /decoy/uart are distinguishable. Also carries the fixes from the review on this PR: Finding 1 (High) -- the property arm tested in_target alone, and in_target is cleared only by the target's own FDT_END_NODE, so it stayed true for the entire subtree and a property found on a *child* was returned as the target's own. On this PR's own decoy tree, eos_fdt_get_prop(blob, "/soc", "reg", ...) returned 0 with "soc-uart" when /soc has no reg property at all and the correct answer is -5. Real device trees emit properties before subnodes, so a property that is present on the target is still found first -- the bug bites when the target lacks it, turning "not found" into a silently wrong value from a nested node. For an attacker-supplied blob the ordering is not enforced at all. Fixed by also requiring depth == target_depth, which is exact: depth is target_depth for the target's own properties and target_depth + 1 or deeper inside any child. Finding 2 (Low) -- test_the_parent_of_the_node_has_to_match_too asserted a second positive resolution rather than a rejection, so the suite read as though it had a negative case for wrong parents when the negative coverage actually lives in test_a_node_below_the_root_resolves. Renamed to test_each_uart_returns_its_own_value, and the load-bearing property (decoy is emitted before soc, so a last-component matcher would return "decoy-uart") is now stated on the test that depends on it. Finding 3 (Low) -- a path deeper than FDT_MAX_PATH_DEPTH returned -1, the same code as a NULL argument, so a caller could not tell a programming error from malformed input. Now -8, documented in the header's code table. Verified: ctest 21/21 PASS same under -DEBLDR_SANITIZE=ON (ASan+UBSan) 21/21 PASS test_fdt_loader 20 tests PASS (was 14) test_a_property_on_a_child_is_not_returned_as_the_parents, run against this same parser with only the depth guard reverted: [FAIL] get_prop_exact(&b, "/soc", "reg", out, &len) != 0 so it discriminates rather than restating the fix. Refs #85 * fix(fdt): match node names past the unit address, where real device trees put it Replaces slash-counting and last-component matching with a component walk gated on ancestor matches, and matches a component against the node name up to its unit address -- `/soc/uart` resolves `uart@40011000`, an explicit `@40011000` still selects exactly that node, and an address not in the tree does not fall back to a loose match. Carries the earlier review fixes (depth == target_depth on the property arm; FDT_MAX_PATH_DEPTH moved to the public header; -8 for an over-deep path, distinct from -1) and, from the overnight round: - the stray `/* Deepest node path ... */` comment left in the .c after the define moved to the header is gone (Low) - return code -8 is documented in the header's code table again, on this branch, because this is the branch where anything returns it -- #84 dropped the line for exactly that reason Rebased onto #84's alignment/NOP round; the test-file merge keeps both sides' suites (garbage-tag, NOP-padding and unaligned-blob from #84; the node-path and unit-address suites from here). Verified: ctest 22/22 PASS ctest -DEBLDR_SANITIZE=ON (ASan+UBSan) 22/22 PASS pytest tests/ 38 passed test_fdt_loader 24 tests PASS Refs #85 --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-authored-by: Srikanth Patchava <srikanth.patchava@outlook.com>

* fix: repair master — the ABI asserts and the Ed25519 verifier both merged broken master (22d8f8b) does not compile. Two independent double-merges, both the same shape: two PRs fixing adjacent things landed on stale bases, each was green on its own branch, and the result was never rebuilt. 1. include/eos_image.h — #93 replaced reserved[30] with tlv_len (2) + tlv_hash[28], preserving every offset. #87 merged afterwards carrying asserts written against the older struct: error: no member named 'reserved' in 'eos_image_header_t' (x2) #93 already asserts tlv_len at 62 and tlv_hash at 64, so the offset assert was a duplicate; the width assert had no replacement and is restored as two asserts covering both halves of the same 30-byte span. No offset moves and the wire format is unchanged. 2. core/ed25519_verify.c — #86 and #57 both landed a subgroup guard, so the file carried two byte-identical point_is_identity() definitions: error: redefinition of 'point_is_identity' Only #57's public_key_is_valid_subgroup() is wired to the call site, so #86's key_has_prime_order() was dead. Kept the live function, folded #86's fuller rationale onto it, deleted the duplicate. 3. tests/unit/test_ed25519.c — collateral from the same merge. Two copies of test_ed25519_identity_key_forgery_rejected, main() calling it twice and two tests not at all, and test_ed25519_low_order_R_with_a_valid_key_is_not_a_forgery referencing k_low_order[] and messages[] that the merge had dropped. While restoring the corpus, corrected it (review finding on #86): the array claimed to hold "the eight low-order point encodings" and held five. Every order here was computed rather than copied — decode y, recover x, add the point to itself until it reaches the identity — giving 1, 2, 4, 4, 8, 8, 8, 8. Missing before: y=0 with the sign bit set, and both sign-flipped order-8 encodings. D9FF..FF was in the array and is not a low-order point at all — no x satisfies the curve equation for that y — so it moves to a separate k_non_canonical[], with EDFF..FF7F (y=p) and EEFF..FF7F (y=p+1). tests_run was assigned a literal (11) in main() and never incremented, which is how the duplicate call and the two unregistered tests went unnoticed. The TEST macro now increments it, so the total cannot drift. Verified: cmake -DEBLDR_BUILD_TESTS=ON on master FAILS to build, 3 errors same with this commit builds clean ctest 21/21 PASS ctest -DEBLDR_SANITIZE=ON (ASan+UBSan) 21/21 PASS pytest tests/ 24 passed, 1 skipped test_ed25519 14/14 PASS (was 11 claimed, 12 run) discrimination, with `public_key_is_valid_subgroup` disabled: test_ed25519_low_order_keys_rejected FAILS, as it must test_ed25519_non_canonical_... still PASSES — those are refused by unpackneg() on canonicality, a different mechanism, which is the reason they are held in a separate array rather than counted among the eight. * fix(eos_sign): emit the layout master defines, not a new meaning for hdr_size Answers the review on #88. Both High findings had the same root, and master has since settled the question the reviewer said needed a decision. The original defect is real and unchanged: the tool emitted [header][TLV][payload] while stamping hdr_size as a fixed 156, so core/image_verify.c's `payload_addr = addr + hdr->hdr_size` landed on the TLV block and every image it produced failed integrity verification on-device. This branch fixed it by moving hdr_size to mean "offset to the payload". That was the wrong half to move. Finding 1 (High) -- eFirmware/src/efw_image.c:122 checks `hdr_size != EFW_IMAGE_HDR_SIZE` for exact equality against 156, so every TLV image would have been rejected by the other half of the format, which eFirmware's header documents as interchangeable. Finding 2 (High) -- core/rollback.c computes the TLV address as image_addr + hdr_size + image_size, i.e. it assumes the area follows the payload. Under either the old layout or this branch's, the anti-rollback EOS_TLV_MIN_SEC_VER could never be found, so the counter this repository gates downgrades on was unreachable for every image the tool produced. Master answers both, and it is the reviewer's option (b): #93 landed tlv_len and tlv_hash in the header -- inside EOS_IMG_SIGNED_LEN -- and documents that "the TLV area sits after the payload". So this PR now emits [header][payload][TLV] with hdr_size always 156. eFirmware's equality check keeps working untouched, rollback.c's arithmetic is correct as written, and no field changes meaning. That reordering also resolves an ordering problem the old shape had: the TLV carried the signature, so its digest could not be computed before the signature existed. The signature now lives in the header's signature[] field and the TLV carries metadata only, which is what makes tlv_hash signable. Finding 4 (Medium) -- the two `assert`s in cmd_sign are removed by `python -O`. This is release-signing tooling, so the signature-length check is now a `raise SystemExit`. The TLV-length assert is gone with the code that needed it. Finding 5 (Medium) -- `pytest.importorskip("cryptography")` still skipped silently if the install ever failed, leaving "collected 19, ran 0" as a green run. Now gated: EOS_REQUIRE_SIGNING_TESTS is set in ci.yml, and under it the module is imported directly so a missing dependency is a collection error. Locally, without the variable, the skip still applies. Finding 6 (Low) -- cmd_verify bounds-checks the image length before unpacking, so a short file gets a FAIL: line rather than a struct.error traceback, and the 156 is hoisted to EOS_IMG_STRUCT_SIZE rather than re-derived. Finding 3 (Medium) needs no change here: master's include/eos_image.h already documents the layout this now emits. Rebased onto #94 (the master repair) and reduced to the five files that are still this PR's own -- the branch was stale enough that its diff against master would have reverted core/rollback.c, include/eos_image.h, tests/unit/test_tlv_auth.c and the rest of #93's TLV work. Verified: end to end, with the real tool: eos_sign.py keygen / sign / verify -> VERIFIED total 744 = 156 header + 512 payload + 76 TLV hdr_size 156, tlv_len 76, TLV magic 0x6907 at 156+512 sha256(tlv)[:28] == header bytes 64..92 True ctest 22/22 PASS test_eos_sign_boot_path (real parser over real tool output): current layout [hdr][payload][tlv]: verify_integrity -> 0 PASS old layout [hdr][tlv][payload]: verify_integrity -> -3 PASS TLV magic at addr + hdr_size + image_size: 0x6907 PASS that third line is finding 2's regression test -- it fails on the old layout, where the address lands past the end of the image. pytest tests/ (EOS_REQUIRE_SIGNING_TESTS=1) 44 passed the CI gate itself: with the variable set and the module absent, the import raises ImportError -> collection error -> job fails, rather than skipping. Refs #88, #93

Kartikey1306 requested review from hshanmug12 and srpatcha as code owners September 1, 2026 15:18

Kartikey1306 mentioned this pull request Sep 1, 2026

ci: add one job branch protection can require #90

Merged

srpatcha approved these changes Sep 3, 2026

View reviewed changes

srpatcha merged commit bbd997a into embeddedos-org:master Sep 3, 2026
27 checks passed

srpatcha reviewed Sep 3, 2026

View reviewed changes

This was referenced Sep 3, 2026

fix(eos_sign): put the payload where the bootloader looks for it #88

Merged

fix: run the signing suite in CI and keep the boot log head consistent on write failure #91

Merged

Kartikey1306 mentioned this pull request Sep 3, 2026

fix: repair master — the ABI asserts and the Ed25519 verifier both merged broken #94

Merged

srpatcha mentioned this pull request Sep 3, 2026

ci: install EoSim from a tag that exists, not a wheel that never shipped #81

Merged

Kartikey1306 mentioned this pull request Sep 3, 2026

fix: repair master — #86, #87 and #57 merged clean and do not compile #97

Closed

KhajaRaheelAhmedMohiuddin mentioned this pull request Sep 3, 2026

fix: repair merge artifacts that left master uncompilable #98

Closed

Navigation Menu

Repository navigation

fix(image): pin the whole .efw header, not four fields of it - #87

fix(image): pin the whole .efw header, not four fields of it#87
srpatcha merged 1 commit into
embeddedos-org:masterfrom
Kartikey1306:fix/pin-efw-wire-format

Kartikey1306 commented Sep 1, 2026 •

edited

Loading

Uh oh!

codecov-commenter commented Sep 1, 2026

Uh oh!

Uh oh!

srpatcha left a comment

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

Kartikey1306 commented Sep 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

A field swap passed every existing assert

No constant's value was pinned on this side

What is added

Validation

Not touched

Uh oh!

codecov-commenter commented Sep 1, 2026

Codecov Report

Uh oh!

Uh oh!

srpatcha left a comment

Choose a reason for hiding this comment

Review — eBoot#87 "fix(image): pin the whole .efw header, not four fields of it"

Findings

Architecture conformance

Proposed changes

Not checked

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Kartikey1306 commented Sep 1, 2026 •

edited

Loading