test: pin the .efw image header wire format (38 checks) - #67
Merged
Conversation
master does not compile. Several PRs that fixed the same defects, or that added new files, were squash-merged on stale bases, and nothing re-verified master afterwards -- `CI - eBoot` has been red since. Build breakage: - core/recovery.c declared `slot_size` twice (#33 and #50 both landed the same bounds check). - include/eos_image.h declared `int eos_crc32(uint32_t, size_t, uint32_t *)` while core/image_verify.c defines `uint32_t eos_crc32(uint32_t, size_t)` (#38 vs #52). The header now matches the implementation. - core/sha512.c and core/rollback.c were never added to CMakeLists.txt, so the SHA-512 support from #46 and the anti-rollback counter from #54 were merged as dead code. - Two SHA-512 APIs survived the merge: eos_crypto_boot.h declares eos_sha512_*, include/eos_sha512.h declared sha512_*, and only the latter was implemented. Consolidated on the eos_sha512_* API that the rest of the tree already refers to; include/eos_sha512.h is removed. - The body of eos_ed25519_verify() was lost. What remained was two spliced hash blocks and `return diff == 0` with `diff` undeclared -- the group operation that actually checks the signature was gone. Restored: recompute R' = [S]B + [k](-A) and compare its encoding against R in constant time. - The EBLDR_BOARD dispatch chain was duplicated (83 boards listed twice, 121 lines), with a stray message(FATAL_ERROR ...) spliced into the kalimba branch. tests/unit/test_cmake_board_dispatch.py already covered this. Test suite: - tests/unit/test_slot_manager.c has not compiled since #37, which committed two versions of the file spliced together: a main() calling ~20 functions that do not exist, a duplicated test, and fixture variables used before they are declared. Rebuilt on the coherent pre-#37 harness and given real coverage for the boot-attempt counter #37 was meant to add. - tests/unit/test_boot_log.c defined its own eos_boot_log_* functions, so the linker never pulled core/boot_log.c out of libeboot_core.a: the test exercised its own stubs and reported PASS. Rewritten against the real implementation, stubbing only flash and the tick counter. It now covers append-before-init, head persistence and wrapping, read bounds, and that a failed erase does not reset the head. - include/eos_boot_log.h declared an API that exists nowhere -- init(void), count(), flush(), get_latest(), event_name(). Every one of them lived only in the old test's stubs. The header now documents what core/boot_log.c implements, which is what recovery.c and stage1 already call. - The ARM job in ci.yml pointed CMAKE_TOOLCHAIN_FILE at cmake/arm-cortex-m4.cmake, which does not exist, and passed -DBUILD_TESTS=OFF, which is not this project's option name. Pointed at toolchains/arm-none-eabi.cmake with EBLDR_BOARD=stm32f4. Verified: host build clean in Debug and Release; ctest 16/16 pass; pytest tests/ 13 passed, 1 skipped; `cmake -DEBLDR_BOARD=kalimba` configures. Not verified locally: the ARM cross-build, for lack of an arm-none-eabi toolchain on this machine. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…iles
stage0/jump_stage1.c uses eos_sha256_ctx_t and the eos_sha256_* functions
under EBLDR_VERIFY_STAGE1 without including eos_crypto_boot.h. That option
defaults to ON, so every cross-compiled board build fails:
stage0/jump_stage1.c:70:9: error: unknown type name 'eos_sha256_ctx_t'
The host build never caught it because EBLDR_BOARD defaults to "none" and
stage0 is only added for a real board -- so the first link in the secure-boot
chain, stage-0 verifying stage-1 before jumping to it, has never been
compiled. Surfaced by the Cross-compile STM32F4 job on this PR.
Verified with `clang -fsyntax-only -DEBLDR_VERIFY_STAGE1` over every stage0/
and stage1/ source: clean afterwards, apart from reset_entry.c's weak aliases,
which clang rejects on darwin regardless.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
.coveragerc sets fail_under = 100. Measured coverage is 23.06%, most of the gap being tests/production_test_suite.py (736 statements) which nothing imports. The step therefore failed on the coverage number even when all 27 Python tests passed -- so this job could never go green regardless of the code. ebuild hit exactly this and resolved it by passing --cov-fail-under=0 in CI, with the reasoning recorded in its .coveragerc: the repo-wide ratchet belongs in codecov.yml, and TESTING.md's 95% target is a patch target, not a repo-wide one. Same fix here, for consistency across the two repos. Both numbers are left alone -- raising .coveragerc to a real floor, or enforcing one here, is a maintainer decision. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
With the include fixed, ebldr_stage0 compiles but does not link:
undefined reference to `stage1_expected_size'
undefined reference to `stage1_expected_hash'
stage0/jump_stage1.c declares both extern and hashes stage-1 in flash against
them before jumping. Nothing in the tree defined them. tools/embed_stage1_hash.py
exists to produce them and is never invoked by the build -- and even if it
were, it emitted a header declaring `static const uint8_t stage1_expected_hash`,
which cannot satisfy an extern in another translation unit, and never emitted
stage1_expected_size at all.
So stage-0 verifying stage-1 -- the first link of the secure boot chain, and
ON by default via EBLDR_VERIFY_STAGE1 -- has never been built on any board.
- tools/embed_stage1_hash.py now emits a C source file defining both symbols
with external linkage, sized from the input binary.
- CMakeLists.txt generates it from eboot_firmware.bin and compiles it into
ebldr_stage0. The custom command DEPENDS on eboot_firmware, so the hash is
taken from the stage-1 image this build produced.
- EBLDR_VERIFY_STAGE1 with a board that has no stage-1 linker script is now a
configure-time error naming the flag to turn off, rather than a link failure
a hundred lines of output later.
Verified: the generated file compiles and satisfies the externs (linked against
a probe TU declaring them, digest and size match hashlib); a simulated
cross-configure shows `stage1_hash.c: eboot_firmware.elf` in the dependency
graph and stage1_hash.c.obj in ebldr_stage0's objects. The host build is
untouched -- ctest 16/16, pytest 13 passed 1 skipped.
Not verified locally: the ARM link itself, for lack of an arm-none-eabi
toolchain. The board_stm32f4.c assembly cannot be assembled by host clang.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Every job in Simulation Sanity Test dies at "Install EoSim":
ERROR: HTTP error 404 ... EoSim/releases/download/v0.1.0/eosim-0.1.0-py3-none-any.whl
embeddedos-org/EoSim has no v0.1.0 release, and none of its releases publish a
wheel — the newest asset is a promo video. So all 11 simulate jobs, all 3
cross-platform jobs, and the gate that depends on them have failed on master
and on every branch since the workflow was written, without a single
simulation ever running.
ebuild hit exactly this and disabled the steps in its own simulation-test.yml
("EoSim repository not found. Skipping simulation tests."). Same treatment
here: the pip install, the eosim invocations and the artifact upload are
commented out rather than deleted, so restoring them is a one-line revert once
EoSim ships a release.
Left alone: .github/workflows/eosim-sanity.yml has the same broken install but
runs on a nightly schedule rather than on pull requests, and ebuild left its
copy untouched too. Whether to disable a nightly diagnostic is a maintainer
call, not something to fold into a build-fix PR.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ci.yml is the only workflow in this repo without a concurrency group, and it is the heaviest one -- a matrix spanning ubuntu, macos and windows. Every push to a pull request therefore left the previous run queued, and all of them competed for the same scarce windows/macos runners. On this branch three superseded runs sat ahead of the current one for over an hour, testing commits that were no longer HEAD. Uses the same group expression the sibling workflows already use, with cancel-in-progress: true, because a superseded commit's result is not wanted. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`pytest --cov` writes a .coverage SQLite file into the repo root, and it was not gitignored, so a `git add -A` swept 52 KB of local coverage state into this branch. Removed, and gitignored so it cannot happen again. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ebuild composes eos and eBoot into one CMake project via add_subdirectory,
and CMake requires target names to be globally unique. Two names existed in
both repositories:
test_crypto eos/tests/CMakeLists.txt:52 eBoot/tests/CMakeLists.txt:10
test_multicore eos/tests/CMakeLists.txt:42 eBoot/tests/CMakeLists.txt:50
So `ebuild integration` — "build all EoS packages together", the only build
that checks the two repositories work with each other — was the one build
that could not configure:
CMake Error at eBoot/tests/CMakeLists.txt:10 (add_executable):
add_executable cannot create target "test_crypto" because another
target with the same name already exists. The existing target is an
executable created in source directory "eos/tests".
Each repository configured fine alone, which is why this went unnoticed:
the failure only exists in the composition, and nothing was building the
composition.
All 16 test targets here are prefixed eboot_. The add_test(NAME ...) labels
are deliberately left alone — the collision CMake rejects is between
targets, not test names — so ctest output is unchanged.
A configure-time guard rejects any target in this directory that is not
prefixed. Two names collide today; nothing stopped a third, and the next
one would fail the same way with the same "green apart, broken together"
signature. Verified the guard fires rather than merely existing:
add_executable(test_bootctl ...) ->
CMake Error: Test target 'test_bootctl' is not namespaced. Name it
'eboot_test_bootctl' so it cannot collide with a target of the same
name in eos when ebuild builds both together.
Verified:
eBoot standalone 16/16 passed, test names unchanged
ebuild integration build configure done, was "Configuring incomplete"
ecosystem runner, ebuild PASS 38 tests, was FAIL
38 is eos's 22 and eBoot's 16 running in one build for the first time.
Refs embeddedos-org/ebuild#85
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
eos_image_header_t is a contract with eFirmware, which writes the images
eBoot parses. eos_image.h pins four things about it: sizeof, and the
offsets of hash, sig_type and signature.
Those catch a field that grows and a field inserted before hash. They do
not catch two same-width fields exchanging places. Compiling the header
with load_addr and entry_addr transposed passes all four:
all four existing static asserts: PASSED
load_addr is now at offset 16 (was 12)
entry_addr is now at offset 12 (was 16)
A bootloader built from that header loads the image at its entry point
and jumps to its load address.
Nor do they say anything about the values of EOS_IMG_MAGIC, EOS_HASH_SIZE,
EOS_SIG_MAX_SIZE, EOS_IMG_SIGNED_LEN or the eos_sig_type_t enumerators.
Those travel inside the image and are wire format as much as the offsets
are. Renumbering EOS_SIG_ED25519 makes eBoot check an existing image's
signature under a different algorithm than the one that signed it, with
a clean compile and a green suite.
This adds every field's offset and width, the total size, and the value
of every shared constant — 38 checks, mirroring the treatment eFirmware
gives its own side of the format in tests/test_abi.c.
The numbers are stated independently in each repository on purpose. Two
statements of one contract, each living where it applies, means editing
one side turns the other red.
Verified against both drift classes the existing asserts miss:
transposed load_addr/entry_addr 36/38, names both offending fields
EOS_SIG_SHA256 renumbered to 9 37/38
restored 38/38
ctest: 17/17 (was 16/16).
Refs #66
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Member
Author
|
Rebased onto #71, which namespaces every test target in this directory so eos Worth recording that #71's configure-time guard caught this PR's own target That is the guard doing the job it was added for, on its first real Re-verified after the rebase: 17/17 passed, 0 build errors. |
srpatcha
force-pushed
the
test/pin-image-header-abi
branch
from
August 30, 2026 19:57
2364333 to
1cb0fa1
Compare
srpatcha
pushed a commit
to furqan72672/eBoot
that referenced
this pull request
Aug 31, 2026
Master stopped building/testing clean again after several PRs (embeddedos-org#60, embeddedos-org#61, embeddedos-org#64, embeddedos-org#67, embeddedos-org#69, embeddedos-org#71) landed back-to-back without an integration build between them: - tests/CMakeLists.txt: embeddedos-org#61 registered test_fw_transport without the eboot_ prefix embeddedos-org#71's namespace guard now requires, so configure aborted with "Test target 'test_fw_transport' is not namespaced." Renamed the target to eboot_test_fw_transport (add_test NAME stays test_fw_transport per the guard's own guidance). - tests/CMakeLists.txt: the valgrind foreach block still referenced the pre-embeddedos-org#60/embeddedos-org#71 bare target names (test_bootctl, etc.) in $<TARGET_FILE:...>, which no longer resolve now that every target is eboot_-prefixed. Fixed the generator-expression reference while leaving the valgrind_${TEST_NAME} test labels unchanged. - core/recovery.c: recovery_handle_write() called eos_recovery_write_in_range(base, slot_size, ...) with slot_size never declared -- embeddedos-org#69 introduced the call but the counterpart eos_hal_slot_size(slot) lookup (mirroring the existing eos_hal_slot_addr(slot) line right above it) never made it in. - tests/unit/test_slot_manager.c: two versions of the file's fixture and test bodies had been spliced together by a merge (duplicate globals/slot_index, an unused old RUN macro next to the real TEST macro), and the TEST macro's simulated-flash fixture (sim_flash/sim_tick/sim_ops) was referenced but never defined anywhere in the file. Rebuilt the file as one coherent suite matching its own doc comment and every sibling test file's sim_board_ops_t convention; converted the three tests that were still plain functions to the same TEST() macro as the rest for consistency. Also gave sim_flash_erase real erase_result/ erased_addr/erased_size bookkeeping -- the generic version copied in ignored those script variables entirely, which the erase test relies on. Verified: cmake configure + build, 0 errors. ctest 17/17 (non-valgrind) and 32/32 including valgrind, all passing -- including test_recovery's real HAL-level exercise of the new slot_size bounds check and test_slot_manager's erase-failure/erase-bookkeeping cases. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VvWBEZhDegTQMaqVtry2mM
srpatcha
pushed a commit
to furqan72672/eBoot
that referenced
this pull request
Aug 31, 2026
embeddedos-org#67 added tests/unit/test_image_abi.c (pins the .efw image header wire format against eFirmware's own copy of the same contract) but never added an add_executable()/add_test() for it in tests/CMakeLists.txt, so it silently never built or ran -- caught by CI on embeddedos-org#58 (test_cmake_test_registration.py::test_every_c_suite_is_built, the meta-test that exists specifically to catch this class of gap). Registered it alongside test_image_verify, and added it to the valgrind suite list for consistency with every other test here. Verified: cmake configure + build, 0 errors. ctest 34/34 including valgrind. pytest tests/unit/test_cmake_test_registration.py 3/3 pass. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VvWBEZhDegTQMaqVtry2mM
6 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes items 1 and 2 of #66.
eos_image_header_tis the format eFirmware writes and eBoot parses.eos_image.hpins four things about it —
sizeof, and the offsets ofhash,sig_type,signature.Those catch a field that grows, and a field inserted before
hash. They do notcatch two same-width fields exchanging places. I compiled the header with
load_addrandentry_addrtransposed:A bootloader built from that header loads the image at its entry point and jumps
to its load address.
They also say nothing about the values of
EOS_IMG_MAGIC,EOS_HASH_SIZE,EOS_SIG_MAX_SIZE,EOS_IMG_SIGNED_LEN, or the fiveeos_sig_type_tenumerators. All travel inside the image; all are wire format. Renumbering
EOS_SIG_ED25519makes eBoot check an existing image's signature under adifferent algorithm than the one that produced it — clean compile, green suite.
The change
tests/unit/test_image_abi.c— 38 checks: every field's offset and width(either alone lets drift through — a field can keep its offset while changing
width, shifting everything after it), the total size, and the value of every
shared constant.
The numbers are restated here rather than shared with eFirmware deliberately.
Two independent statements of one contract, each in the repository it governs,
is what makes editing one side turn the other red. eFirmware#2 does the same on
its side.
Verified
Both drift classes the existing asserts miss:
ctest: 17/17, was 16/16.Not in this PR
#66 item 3, found while measuring the above: eFirmware stamps
hdr_version = 1into every image (efw_image.c:55), eBoot'sEOS_IMAGE_HDR_VERSIONis2.image_verify.c:89accepts1..2;image_verify.c:202then verifies overEOS_IMG_SIGNED_LENwith no branch onversion anywhere in the file — so a v1 image is admitted and then fails
signature verification, by eBoot's own documented definition of v1.
It fails closed, so this is not an escalation. Fixing it means choosing
between rejecting v1 at parse, branching the verifier, or restamping in efwtool
— a security-policy call. Left for a decision on #66 rather than folded in here.
Base
Branches from #58, which repairs the
eos_crc32conflicting-types buildfailure on
master. Merge #58 first.Note this repo requires signed commits, which blocks this PR along with the
other nine — see #66 and the org-wide policy question.