Skip to content

test: pin the .efw image header wire format (38 checks) - #67

Merged
srpatcha merged 10 commits into
masterfrom
test/pin-image-header-abi
Aug 30, 2026
Merged

test: pin the .efw image header wire format (38 checks)#67
srpatcha merged 10 commits into
masterfrom
test/pin-image-header-abi

Conversation

@srpatcha

Copy link
Copy Markdown
Member

Closes items 1 and 2 of #66.

eos_image_header_t is the format eFirmware writes and eBoot parses. eos_image.h
pins four things about it — sizeof, and the offsets of hash, sig_type,
signature.

Those catch a field that grows, and a field inserted before hash. They do not
catch two same-width fields exchanging places. I compiled the header with
load_addr and entry_addr transposed:

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.

They also say nothing about the values of EOS_IMG_MAGIC, EOS_HASH_SIZE,
EOS_SIG_MAX_SIZE, EOS_IMG_SIGNED_LEN, or the five eos_sig_type_t
enumerators. All travel inside the image; all are wire format. Renumbering
EOS_SIG_ED25519 makes eBoot check an existing image's signature under a
different 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:

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.

Not in this PR

#66 item 3, found while measuring the above: eFirmware stamps
hdr_version = 1 into every image (efw_image.c:55), eBoot's
EOS_IMAGE_HDR_VERSION is 2. image_verify.c:89 accepts 1..2;
image_verify.c:202 then verifies over EOS_IMG_SIGNED_LEN with no branch on
version 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_crc32 conflicting-types build
failure 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.

Kartikey1306 and others added 7 commits August 28, 2026 19:17
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>
srpatcha and others added 2 commits August 30, 2026 07:39
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>
@srpatcha

Copy link
Copy Markdown
Member Author

Rebased onto #71, which namespaces every test target in this directory so eos
and eBoot can be composed into one CMake project. test_image_abi is now
eboot_test_image_abi; the add_test(NAME ...) label is unchanged, so ctest
output reads the same.

Worth recording that #71's configure-time guard caught this PR's own target
during the rebase, rather than the collision showing up later in a different
repository:

CMake Error at tests/CMakeLists.txt:128 (message):
  Test target 'test_image_abi' is not namespaced.  Name it
  'eboot_test_image_abi' so it cannot collide with a target of the same name
  in eos when ebuild builds both together.

That is the guard doing the job it was added for, on its first real
opportunity.

Re-verified after the rebase: 17/17 passed, 0 build errors.

Merge order is now #58, then #71, then this.

@srpatcha
srpatcha force-pushed the test/pin-image-header-abi branch from 2364333 to 1cb0fa1 Compare August 30, 2026 19:57
@srpatcha
srpatcha merged commit ab5a4bf into master Aug 30, 2026
20 of 24 checks passed
@srpatcha
srpatcha deleted the test/pin-image-header-abi branch August 30, 2026 22:38
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants