From 35280b5297deb5bbfed90b9db0d234db058be049 Mon Sep 17 00:00:00 2001 From: kartikey1306 Date: Fri, 11 Sep 2026 16:13:57 +0530 Subject: [PATCH 01/13] fix: repair master after the 09-07 batch merge Twenty PRs were merged into master in ninety minutes on 09-07, each on the base it was written against. Master has not configured, compiled, linked or passed its tests since. Every nightly and every eos simulation run (which builds eBoot master) has been red for the same reason. Configure: - tests/CMakeLists.txt registered eboot_test_fdt_loader twice. #84 added it, and #85 -- stacked on #84 -- re-added it at a different anchor when it was replayed onto a master that already had #84. Same hunk as #111. Compile (eboot_core): - #55 restored a core/sha512.c from 02b7dac that keeps its byte count in ctx->count; the eos_sha512_ctx_t master ships has bitlen[2]/buffer_len (#69/#93). Back to the pre-#55 file, blob-identical (d9aa57c). - #55's source-list "correction" dropped core/boot_log.c, core/secure_boot.c and core/fdt_loader.c, which #72/#84 had added after #55 was written. CMakeLists.txt is blob-identical to pre-#55 again (f8fe6eb). - #55 replaced the eos_boot_log_get_head() declaration with a second copy of eos_boot_log_read(); #91 had already fixed the prototype it meant to fix. Header restored (964ebb8). - #94 and #105 each repaired the Ed25519 verifier and each added an identical static scalarbase(); both merged. One copy removed. - The same pair each added k_low_order[]/messages[] to test_ed25519.c. The #105 copy is removed; #94's stays because it also carries k_non_canonical[]. Tests that stopped passing because two merged PRs disagree on behaviour: - #104 verifies the image signature at install unconditionally, before the anti-rollback check #103 added, so #103's unsigned images are refused as EOS_ERR_SIGNATURE before they can be refused as EOS_ERR_ANTI_ROLLBACK, and test_fw_transport's XMODEM install can no longer finalize. Both suites now stream genuinely signed images. eBoot has no Ed25519 signer in C, so tools/gen_fw_update_test_sigs.py signs the exact header prefixes those suites build under the RFC 8032 section 7.1 TEST 1 key and emits tests/vectors/fw_update_test_sigs.h; the suites serve that key from a simulated OTP slot 0. Negative control: one flipped signature byte fails test_write_streams_tlv_then_finalize_rejects_below_floor with EOS_ERR_SIGNATURE. - #103's step 5b reads the TLV counter through the HAL slot containing the image; test_secure_boot_policy (#82) declared no slots, so eos_secure_boot() returned EOS_SBOOT_ERR_BAD_HEADER two steps before the one under test. The fixture now places its image in slot A. Guards from #95 that later merges walked back, never run until now because the C configure step failed first: - #103 replayed the hand-written Valgrind foreach over the derived one. Restored foreach(TEST_NAME ${EBLDR_UNIT_TESTS}); eleven registered suites had no list(APPEND ...) and so no Valgrind run. - Seven suites assign tests_run = and their TEST() does not count; four suites have no TEST() macro at all. Counted, and classified. - #101 added fuzz-build after #90's gate; the gate did not wait for it. CI plumbing: - eosim-sanity.yml: the install-validate job is written in bash but ran under PowerShell on the Windows legs (no shell:), where SITE_PACKAGES=$(...) is an unknown command and `|| { exit 1 }` is an unexecuted script block. - scorecard.yml: ossf/scorecard-action@v2.4.0 pulls gcr.io, which now demands GCP billing. v2.4.3 pulls ghcr.io; eos already pins it and is green. Verified locally (macOS, clang): Release build clean, 31/31 ctest; the same under -DEBLDR_SANITIZE=ON (ASan+UBSan); 78/78 pytest. Not fixed here, reported separately: core/keystore.c's compiled-in default_dev_key is described as the RFC 8032 TEST 1 public key but differs from byte 21 on and is not a point on the curve, so nothing can verify against it on any board without OTP. With #104 that makes firmware update refuse every image on such boards. --- .github/workflows/ci.yml | 2 +- .github/workflows/eosim-sanity.yml | 8 + .github/workflows/scorecard.yml | 2 +- CMakeLists.txt | 5 +- core/ed25519_verify.c | 10 - core/sha512.c | 322 ++++++++++++++++++--------- include/eos_boot_log.h | 10 +- tests/CMakeLists.txt | 18 +- tests/unit/test_ed25519.c | 40 ---- tests/unit/test_fw_decrypt.c | 2 +- tests/unit/test_fw_transport.c | 27 ++- tests/unit/test_fw_update.c | 35 ++- tests/unit/test_fw_update_sig.c | 2 +- tests/unit/test_jump_app.c | 2 +- tests/unit/test_jump_app_bounds.c | 2 +- tests/unit/test_secure_boot_policy.c | 5 + tests/unit/test_suite_bookkeeping.py | 8 + tests/unit/test_tlv_auth.c | 1 - tests/vectors/fw_update_test_sigs.h | 43 ++++ tools/gen_fw_update_test_sigs.py | 156 +++++++++++++ 20 files changed, 515 insertions(+), 185 deletions(-) create mode 100644 tests/vectors/fw_update_test_sigs.h create mode 100644 tools/gen_fw_update_test_sigs.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 71ae0b7..24ae902 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -250,7 +250,7 @@ jobs: ci-gate: name: CI Gate runs-on: ubuntu-22.04 - needs: [test, build-arm, static-analysis] + needs: [test, build-arm, fuzz-build, static-analysis] if: always() steps: - uses: actions/checkout@v4 diff --git a/.github/workflows/eosim-sanity.yml b/.github/workflows/eosim-sanity.yml index e323860..9c6e3b4 100644 --- a/.github/workflows/eosim-sanity.yml +++ b/.github/workflows/eosim-sanity.yml @@ -23,6 +23,14 @@ jobs: install-validate: name: Install & Validate (${{ matrix.os }}, Python ${{ matrix.python-version }}) runs-on: ${{ matrix.os }} + # Every step here is written in bash: `$(...)`, `|| { ... }`, a heredoc, + # and /tmp. Without this the Windows legs ran them under PowerShell, where + # `SITE_PACKAGES=$(...)` is an unknown command and the job went red, while + # the `|| { exit 1 }` guard in "Verify installation" parsed as an unexecuted + # script block and could never fail. + defaults: + run: + shell: bash strategy: fail-fast: false matrix: diff --git a/.github/workflows/scorecard.yml b/.github/workflows/scorecard.yml index e8507e5..4a0033e 100644 --- a/.github/workflows/scorecard.yml +++ b/.github/workflows/scorecard.yml @@ -24,7 +24,7 @@ jobs: - uses: actions/checkout@v4 with: persist-credentials: false - - uses: ossf/scorecard-action@v2.4.0 + - uses: ossf/scorecard-action@v2.4.3 with: results_file: results.sarif results_format: sarif diff --git a/CMakeLists.txt b/CMakeLists.txt index 728e028..f8fe6eb 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -80,6 +80,7 @@ target_include_directories(eboot_hal PUBLIC ${EBLDR_INCLUDE_DIR}) # ---- Core boot logic ---- add_library(eboot_core STATIC core/bootctl.c + core/boot_log.c core/image_verify.c core/slot_manager.c core/boot_policy.c @@ -106,8 +107,10 @@ add_library(eboot_core STATIC core/os_adapter.c core/ed25519_verify.c core/sha512.c - core/keystore.c + core/secure_boot.c + core/fdt_loader.c core/rollback.c + core/keystore.c core/debug_lock.c core/fw_decrypt.c core/image_tlv.c diff --git a/core/ed25519_verify.c b/core/ed25519_verify.c index 7d230fd..3e9e4b1 100644 --- a/core/ed25519_verify.c +++ b/core/ed25519_verify.c @@ -300,16 +300,6 @@ static int point_is_identity(gf p[4]) return diff == 0; } -static void scalarbase(gf r[4], const uint8_t *s) -{ - gf q[4]; - fe_copy16(q[0], BX); - fe_copy16(q[1], BY); - fe_copy16(q[2], gf1); - fe_mul(q[3], BX, BY); - scalarmult(r, q, s); -} - /* Reject a public key outside the prime-order subgroup. * * Decoding a point is not enough. Ed25519 has eight points of low order, and diff --git a/core/sha512.c b/core/sha512.c index 4a1ecb5..d9aa57c 100644 --- a/core/sha512.c +++ b/core/sha512.c @@ -1,147 +1,247 @@ // SPDX-License-Identifier: MIT // Copyright (c) 2026 EoS Project -// ISO/IEC 25000 | ISO/IEC/IEEE 15288:2023 - -/** - * @file sha512.c - * @brief SHA-512 (NIST FIPS 180-4) — required by Ed25519 (RFC 8032) - * - * Ed25519 as specified in RFC 8032 derives its challenge scalar from - * SHA-512. A verifier using any other hash cannot check a signature made - * by a conforming signer, so this primitive is not optional for - * interoperability with standard tooling. - * - * Self-contained, no dynamic allocation, suitable for a bootloader. - */ #include "eos_crypto_boot.h" #include -static const uint64_t K512[80] = { - 0x428a2f98d728ae22ULL, 0x7137449123ef65cdULL, 0xb5c0fbcfec4d3b2fULL, 0xe9b5dba58189dbbcULL, - 0x3956c25bf348b538ULL, 0x59f111f1b605d019ULL, 0x923f82a4af194f9bULL, 0xab1c5ed5da6d8118ULL, - 0xd807aa98a3030242ULL, 0x12835b0145706fbeULL, 0x243185be4ee4b28cULL, 0x550c7dc3d5ffb4e2ULL, - 0x72be5d74f27b896fULL, 0x80deb1fe3b1696b1ULL, 0x9bdc06a725c71235ULL, 0xc19bf174cf692694ULL, - 0xe49b69c19ef14ad2ULL, 0xefbe4786384f25e3ULL, 0x0fc19dc68b8cd5b5ULL, 0x240ca1cc77ac9c65ULL, - 0x2de92c6f592b0275ULL, 0x4a7484aa6ea6e483ULL, 0x5cb0a9dcbd41fbd4ULL, 0x76f988da831153b5ULL, - 0x983e5152ee66dfabULL, 0xa831c66d2db43210ULL, 0xb00327c898fb213fULL, 0xbf597fc7beef0ee4ULL, - 0xc6e00bf33da88fc2ULL, 0xd5a79147930aa725ULL, 0x06ca6351e003826fULL, 0x142929670a0e6e70ULL, - 0x27b70a8546d22ffcULL, 0x2e1b21385c26c926ULL, 0x4d2c6dfc5ac42aedULL, 0x53380d139d95b3dfULL, - 0x650a73548baf63deULL, 0x766a0abb3c77b2a8ULL, 0x81c2c92e47edaee6ULL, 0x92722c851482353bULL, - 0xa2bfe8a14cf10364ULL, 0xa81a664bbc423001ULL, 0xc24b8b70d0f89791ULL, 0xc76c51a30654be30ULL, - 0xd192e819d6ef5218ULL, 0xd69906245565a910ULL, 0xf40e35855771202aULL, 0x106aa07032bbd1b8ULL, - 0x19a4c116b8d2d0c8ULL, 0x1e376c085141ab53ULL, 0x2748774cdf8eeb99ULL, 0x34b0bcb5e19b48a8ULL, - 0x391c0cb3c5c95a63ULL, 0x4ed8aa4ae3418acbULL, 0x5b9cca4f7763e373ULL, 0x682e6ff3d6b2b8a3ULL, - 0x748f82ee5defb2fcULL, 0x78a5636f43172f60ULL, 0x84c87814a1f0ab72ULL, 0x8cc702081a6439ecULL, - 0x90befffa23631e28ULL, 0xa4506cebde82bde9ULL, 0xbef9a3f7b2c67915ULL, 0xc67178f2e372532bULL, - 0xca273eceea26619cULL, 0xd186b8c721c0c207ULL, 0xeada7dd6cde0eb1eULL, 0xf57d4f7fee6ed178ULL, - 0x06f067aa72176fbaULL, 0x0a637dc5a2c898a6ULL, 0x113f9804bef90daeULL, 0x1b710b35131c471bULL, - 0x28db77f523047d84ULL, 0x32caab7b40c72493ULL, 0x3c9ebe0a15c9bebcULL, 0x431d67c49c100d4cULL, - 0x4cc5d4becb3e42b6ULL, 0x597f299cfc657e2aULL, 0x5fcb6fab3ad6faecULL, 0x6c44198c4a475817ULL, +#define ROTR64(x, n) (((x) >> (n)) | ((x) << (64 - (n)))) + +#define CH(x, y, z) (((x) & (y)) ^ (~(x) & (z))) +#define MAJ(x, y, z) (((x) & (y)) ^ ((x) & (z)) ^ ((y) & (z))) + +#define BSIG0(x) (ROTR64((x), 28) ^ ROTR64((x), 34) ^ ROTR64((x), 39)) +#define BSIG1(x) (ROTR64((x), 14) ^ ROTR64((x), 18) ^ ROTR64((x), 41)) + +#define SSIG0(x) (ROTR64((x), 1) ^ ROTR64((x), 8) ^ ((x) >> 7)) +#define SSIG1(x) (ROTR64((x), 19) ^ ROTR64((x), 61) ^ ((x) >> 6)) + +static const uint64_t K[80] = { + 0x428a2f98d728ae22ULL, + 0x7137449123ef65cdULL, + 0xb5c0fbcfec4d3b2fULL, + 0xe9b5dba58189dbbcULL, + 0x3956c25bf348b538ULL, + 0x59f111f1b605d019ULL, + 0x923f82a4af194f9bULL, + 0xab1c5ed5da6d8118ULL, + 0xd807aa98a3030242ULL, + 0x12835b0145706fbeULL, + 0x243185be4ee4b28cULL, + 0x550c7dc3d5ffb4e2ULL, + 0x72be5d74f27b896fULL, + 0x80deb1fe3b1696b1ULL, + 0x9bdc06a725c71235ULL, + 0xc19bf174cf692694ULL, + 0xe49b69c19ef14ad2ULL, + 0xefbe4786384f25e3ULL, + 0x0fc19dc68b8cd5b5ULL, + 0x240ca1cc77ac9c65ULL, + 0x2de92c6f592b0275ULL, + 0x4a7484aa6ea6e483ULL, + 0x5cb0a9dcbd41fbd4ULL, + 0x76f988da831153b5ULL, + 0x983e5152ee66dfabULL, + 0xa831c66d2db43210ULL, + 0xb00327c898fb213fULL, + 0xbf597fc7beef0ee4ULL, + 0xc6e00bf33da88fc2ULL, + 0xd5a79147930aa725ULL, + 0x06ca6351e003826fULL, + 0x142929670a0e6e70ULL, + 0x27b70a8546d22ffcULL, + 0x2e1b21385c26c926ULL, + 0x4d2c6dfc5ac42aedULL, + 0x53380d139d95b3dfULL, + 0x650a73548baf63deULL, + 0x766a0abb3c77b2a8ULL, + 0x81c2c92e47edaee6ULL, + 0x92722c851482353bULL, + 0xa2bfe8a14cf10364ULL, + 0xa81a664bbc423001ULL, + 0xc24b8b70d0f89791ULL, + 0xc76c51a30654be30ULL, + 0xd192e819d6ef5218ULL, + 0xd69906245565a910ULL, + 0xf40e35855771202aULL, + 0x106aa07032bbd1b8ULL, + 0x19a4c116b8d2d0c8ULL, + 0x1e376c085141ab53ULL, + 0x2748774cdf8eeb99ULL, + 0x34b0bcb5e19b48a8ULL, + 0x391c0cb3c5c95a63ULL, + 0x4ed8aa4ae3418acbULL, + 0x5b9cca4f7763e373ULL, + 0x682e6ff3d6b2b8a3ULL, + 0x748f82ee5defb2fcULL, + 0x78a5636f43172f60ULL, + 0x84c87814a1f0ab72ULL, + 0x8cc702081a6439ecULL, + 0x90befffa23631e28ULL, + 0xa4506cebde82bde9ULL, + 0xbef9a3f7b2c67915ULL, + 0xc67178f2e372532bULL, + 0xca273eceea26619cULL, + 0xd186b8c721c0c207ULL, + 0xeada7dd6cde0eb1eULL, + 0xf57d4f7fee6ed178ULL, + 0x06f067aa72176fbaULL, + 0x0a637dc5a2c898a6ULL, + 0x113f9804bef90daeULL, + 0x1b710b35131c471bULL, + 0x28db77f523047d84ULL, + 0x32caab7b40c72493ULL, + 0x3c9ebe0a15c9bebcULL, + 0x431d67c49c100d4cULL, + 0x4cc5d4becb3e42b6ULL, + 0x597f299cfc657e2aULL, + 0x5fcb6fab3ad6faecULL, + 0x6c44198c4a475817ULL }; -#define ROTR64(x, n) (((x) >> (n)) | ((x) << (64 - (n)))) -#define CH64(x, y, z) (((x) & (y)) ^ (~(x) & (z))) -#define MAJ64(x, y, z) (((x) & (y)) ^ ((x) & (z)) ^ ((y) & (z))) -#define EP0_64(x) (ROTR64(x, 28) ^ ROTR64(x, 34) ^ ROTR64(x, 39)) -#define EP1_64(x) (ROTR64(x, 14) ^ ROTR64(x, 18) ^ ROTR64(x, 41)) -#define SIG0_64(x) (ROTR64(x, 1) ^ ROTR64(x, 8) ^ ((x) >> 7)) -#define SIG1_64(x) (ROTR64(x, 19) ^ ROTR64(x, 61) ^ ((x) >> 6)) - -static void sha512_transform(eos_sha512_ctx_t *ctx) +static uint64_t load_be64(const uint8_t *p) +{ + return ((uint64_t)p[0] << 56) | + ((uint64_t)p[1] << 48) | + ((uint64_t)p[2] << 40) | + ((uint64_t)p[3] << 32) | + ((uint64_t)p[4] << 24) | + ((uint64_t)p[5] << 16) | + ((uint64_t)p[6] << 8) | + ((uint64_t)p[7]); +} + +static void store_be64(uint8_t *p, uint64_t x) +{ + p[0] = (uint8_t)(x >> 56); + p[1] = (uint8_t)(x >> 48); + p[2] = (uint8_t)(x >> 40); + p[3] = (uint8_t)(x >> 32); + p[4] = (uint8_t)(x >> 24); + p[5] = (uint8_t)(x >> 16); + p[6] = (uint8_t)(x >> 8); + p[7] = (uint8_t)x; +} + +static void sha512_transform(eos_sha512_ctx_t *ctx, + const uint8_t block[128]) { uint64_t w[80]; - uint64_t a, b, c, d, e, f, g, h, t1, t2; - - for (int i = 0; i < 16; i++) { - w[i] = ((uint64_t)ctx->buffer[i * 8 + 0] << 56) | - ((uint64_t)ctx->buffer[i * 8 + 1] << 48) | - ((uint64_t)ctx->buffer[i * 8 + 2] << 40) | - ((uint64_t)ctx->buffer[i * 8 + 3] << 32) | - ((uint64_t)ctx->buffer[i * 8 + 4] << 24) | - ((uint64_t)ctx->buffer[i * 8 + 5] << 16) | - ((uint64_t)ctx->buffer[i * 8 + 6] << 8) | - ((uint64_t)ctx->buffer[i * 8 + 7]); - } - for (int i = 16; i < 80; i++) { - w[i] = SIG1_64(w[i - 2]) + w[i - 7] + SIG0_64(w[i - 15]) + w[i - 16]; - } - a = ctx->state[0]; b = ctx->state[1]; c = ctx->state[2]; d = ctx->state[3]; - e = ctx->state[4]; f = ctx->state[5]; g = ctx->state[6]; h = ctx->state[7]; + for (int i = 0; i < 16; i++) + w[i] = load_be64(block + i * 8); + + for (int i = 16; i < 80; i++) + w[i] = SSIG1(w[i - 2]) + w[i - 7] + + SSIG0(w[i - 15]) + w[i - 16]; + + uint64_t a = ctx->state[0]; + uint64_t b = ctx->state[1]; + uint64_t c = ctx->state[2]; + uint64_t d = ctx->state[3]; + uint64_t e = ctx->state[4]; + uint64_t f = ctx->state[5]; + uint64_t g = ctx->state[6]; + uint64_t h = ctx->state[7]; for (int i = 0; i < 80; i++) { - t1 = h + EP1_64(e) + CH64(e, f, g) + K512[i] + w[i]; - t2 = EP0_64(a) + MAJ64(a, b, c); - h = g; g = f; f = e; e = d + t1; - d = c; c = b; b = a; a = t1 + t2; + uint64_t t1 = h + BSIG1(e) + CH(e, f, g) + K[i] + w[i]; + uint64_t t2 = BSIG0(a) + MAJ(a, b, c); + + h = g; + g = f; + f = e; + e = d + t1; + d = c; + c = b; + b = a; + a = t1 + t2; } - ctx->state[0] += a; ctx->state[1] += b; ctx->state[2] += c; ctx->state[3] += d; - ctx->state[4] += e; ctx->state[5] += f; ctx->state[6] += g; ctx->state[7] += h; + ctx->state[0] += a; + ctx->state[1] += b; + ctx->state[2] += c; + ctx->state[3] += d; + ctx->state[4] += e; + ctx->state[5] += f; + ctx->state[6] += g; + ctx->state[7] += h; } void eos_sha512_init(eos_sha512_ctx_t *ctx) { - ctx->state[0] = 0x6a09e667f3bcc908ULL; ctx->state[1] = 0xbb67ae8584caa73bULL; - ctx->state[2] = 0x3c6ef372fe94f82bULL; ctx->state[3] = 0xa54ff53a5f1d36f1ULL; - ctx->state[4] = 0x510e527fade682d1ULL; ctx->state[5] = 0x9b05688c2b3e6c1fULL; - ctx->state[6] = 0x1f83d9abfb41bd6bULL; ctx->state[7] = 0x5be0cd19137e2179ULL; - ctx->count = 0; - memset(ctx->buffer, 0, sizeof(ctx->buffer)); + ctx->state[0] = 0x6a09e667f3bcc908ULL; + ctx->state[1] = 0xbb67ae8584caa73bULL; + ctx->state[2] = 0x3c6ef372fe94f82bULL; + ctx->state[3] = 0xa54ff53a5f1d36f1ULL; + ctx->state[4] = 0x510e527fade682d1ULL; + ctx->state[5] = 0x9b05688c2b3e6c1fULL; + ctx->state[6] = 0x1f83d9abfb41bd6bULL; + ctx->state[7] = 0x5be0cd19137e2179ULL; + + ctx->bitlen[0] = 0; + ctx->bitlen[1] = 0; + ctx->buffer_len = 0; } -void eos_sha512_update(eos_sha512_ctx_t *ctx, const uint8_t *data, size_t len) +void eos_sha512_update(eos_sha512_ctx_t *ctx, + const uint8_t *data, + size_t len) { - size_t idx = (size_t)(ctx->count % EOS_SHA512_BLOCK_SIZE); + while (len > 0) { + size_t copy = 128 - ctx->buffer_len; - ctx->count += len; + if (copy > len) + copy = len; - while (len > 0) { - size_t take = EOS_SHA512_BLOCK_SIZE - idx; - if (take > len) take = len; - memcpy(ctx->buffer + idx, data, take); - idx += take; - data += take; - len -= take; - if (idx == EOS_SHA512_BLOCK_SIZE) { - sha512_transform(ctx); - idx = 0; + memcpy(ctx->buffer + ctx->buffer_len, data, copy); + + ctx->buffer_len += copy; + data += copy; + len -= copy; + + uint64_t bits = (uint64_t)copy << 3; + + uint64_t old_low = ctx->bitlen[1]; + ctx->bitlen[1] += bits; + + if (ctx->bitlen[1] < old_low) + ctx->bitlen[0]++; + + ctx->bitlen[0] += (uint64_t)copy >> 61; + + if (ctx->buffer_len == 128) { + sha512_transform(ctx, ctx->buffer); + ctx->buffer_len = 0; } } } -void eos_sha512_final(eos_sha512_ctx_t *ctx, uint8_t digest[EOS_SHA512_DIGEST_SIZE]) +void eos_sha512_final(eos_sha512_ctx_t *ctx, + uint8_t digest[EOS_SHA512_DIGEST_SIZE]) { - /* SHA-512 encodes the message length as a 128-bit big-endian bit count. - * A bootloader never hashes anywhere near 2^61 bytes, so the high 64 - * bits are always zero; they are still written so the padding block is - * byte-exact against FIPS 180-4. */ - uint64_t bits = ctx->count * 8ULL; - size_t idx = (size_t)(ctx->count % EOS_SHA512_BLOCK_SIZE); - - ctx->buffer[idx++] = 0x80; - - if (idx > 112) { - while (idx < EOS_SHA512_BLOCK_SIZE) ctx->buffer[idx++] = 0; - sha512_transform(ctx); - idx = 0; - } - while (idx < 112) ctx->buffer[idx++] = 0; + size_t i = ctx->buffer_len; - memset(ctx->buffer + 112, 0, 8); /* high 64 bits of length */ - for (int i = 0; i < 8; i++) { - ctx->buffer[120 + i] = (uint8_t)(bits >> (56 - 8 * i)); - } - sha512_transform(ctx); + ctx->buffer[i++] = 0x80; - for (int i = 0; i < 8; i++) { - for (int j = 0; j < 8; j++) { - digest[i * 8 + j] = (uint8_t)(ctx->state[i] >> (56 - 8 * j)); - } + if (i > 112) { + while (i < 128) + ctx->buffer[i++] = 0; + + sha512_transform(ctx, ctx->buffer); + i = 0; } - /* Do not leave hash state on the stack of a boot path. */ + while (i < 112) + ctx->buffer[i++] = 0; + + store_be64(ctx->buffer + 112, ctx->bitlen[0]); + store_be64(ctx->buffer + 120, ctx->bitlen[1]); + + sha512_transform(ctx, ctx->buffer); + + for (int i2 = 0; i2 < 8; i2++) + store_be64(digest + i2 * 8, ctx->state[i2]); + memset(ctx, 0, sizeof(*ctx)); } diff --git a/include/eos_boot_log.h b/include/eos_boot_log.h index 86024d7..964ebb8 100644 --- a/include/eos_boot_log.h +++ b/include/eos_boot_log.h @@ -53,12 +53,12 @@ void eos_boot_log_init(uint32_t head); void eos_boot_log_append(uint32_t event, uint32_t slot, uint32_t detail); /** - * @brief Read one boot log entry by index. - * @param index Entry index (0 to EOS_BOOT_LOG_MAX - 1). - * @param out Receives the entry at @p index. - * @return EOS_OK on success, EOS_ERR_INVALID on a bad index or null @p out. + * @brief Current ring-buffer write position. + * + * Persisted into the boot control block on handoff so the log survives a + * reset. @return Head index in [0, EOS_BOOT_LOG_MAX). */ -int eos_boot_log_read(uint32_t index, eos_boot_log_entry_t *out); +uint32_t eos_boot_log_get_head(void); /** * @brief Read one log entry by ring index. diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 11f2f3f..219c3c8 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -55,25 +55,30 @@ list(APPEND EBLDR_UNIT_TESTS test_fw_transport) add_executable(eboot_test_fw_update_sig unit/test_fw_update_sig.c) target_link_libraries(eboot_test_fw_update_sig PRIVATE eboot_core) add_test(NAME test_fw_update_sig COMMAND eboot_test_fw_update_sig) +list(APPEND EBLDR_UNIT_TESTS test_fw_update_sig) # --- test_fw_update: firmware update finalize / anti-rollback wiring --- add_executable(eboot_test_fw_update unit/test_fw_update.c) target_link_libraries(eboot_test_fw_update PRIVATE eboot_core) add_test(NAME test_fw_update COMMAND eboot_test_fw_update) +list(APPEND EBLDR_UNIT_TESTS test_fw_update) # --- test_jump_app: stage-1 jump uses the authenticated TLV counter --- add_executable(eboot_test_jump_app unit/test_jump_app.c) target_link_libraries(eboot_test_jump_app PRIVATE eboot_stage1) add_test(NAME test_jump_app COMMAND eboot_test_jump_app) +list(APPEND EBLDR_UNIT_TESTS test_jump_app) # --- test_slot_size_bounds: verify_slot() must reject image_size > slot capacity --- add_executable(eboot_test_slot_size_bounds unit/test_slot_size_bounds.c) target_link_libraries(eboot_test_slot_size_bounds PRIVATE eboot_core) add_test(NAME test_slot_size_bounds COMMAND eboot_test_slot_size_bounds) +list(APPEND EBLDR_UNIT_TESTS test_slot_size_bounds) add_executable(eboot_test_jump_app_bounds unit/test_jump_app_bounds.c) target_link_libraries(eboot_test_jump_app_bounds PRIVATE eboot_core eboot_stage1) add_test(NAME test_jump_app_bounds COMMAND eboot_test_jump_app_bounds) +list(APPEND EBLDR_UNIT_TESTS test_jump_app_bounds) # --- test_device_table: UEFI-style device table --- add_executable(eboot_test_device_table unit/test_device_table.c) @@ -105,6 +110,7 @@ target_include_directories(eboot_test_qemu_arm64_timer PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/../boards/qemu_arm64 ) add_test(NAME test_qemu_arm64_timer COMMAND eboot_test_qemu_arm64_timer) +list(APPEND EBLDR_UNIT_TESTS test_qemu_arm64_timer) # --- test_board_registry: Runtime board selection --- add_executable(eboot_test_board_registry unit/test_board_registry.c) @@ -148,6 +154,7 @@ target_link_libraries(eboot_test_ed25519_contract PRIVATE eboot_core) target_include_directories(eboot_test_ed25519_contract PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}) add_test(NAME test_ed25519_contract COMMAND eboot_test_ed25519_contract) +list(APPEND EBLDR_UNIT_TESTS test_ed25519_contract) # --- test_keystore: Key management --- add_executable(eboot_test_keystore unit/test_keystore.c) @@ -159,6 +166,7 @@ list(APPEND EBLDR_UNIT_TESTS test_keystore) add_executable(eboot_test_secure_boot_policy unit/test_secure_boot_policy.c) target_link_libraries(eboot_test_secure_boot_policy PRIVATE eboot_core) add_test(NAME test_secure_boot_policy COMMAND eboot_test_secure_boot_policy) +list(APPEND EBLDR_UNIT_TESTS test_secure_boot_policy) # --- test_rollback: Anti-rollback security counter --- add_executable(eboot_test_rollback unit/test_rollback.c) @@ -192,18 +200,21 @@ list(APPEND EBLDR_UNIT_TESTS test_ecc) add_executable(eboot_test_fw_decrypt unit/test_fw_decrypt.c) target_link_libraries(eboot_test_fw_decrypt PRIVATE eboot_core) add_test(NAME test_fw_decrypt COMMAND eboot_test_fw_decrypt) +list(APPEND EBLDR_UNIT_TESTS test_fw_decrypt) # --- test_fdt_loader: device tree parsing against malformed blobs --- # core/fdt_loader.c was in no source list, so it had never been compiled. add_executable(eboot_test_fdt_loader unit/test_fdt_loader.c) target_link_libraries(eboot_test_fdt_loader PRIVATE eboot_core) add_test(NAME test_fdt_loader COMMAND eboot_test_fdt_loader) +list(APPEND EBLDR_UNIT_TESTS test_fdt_loader) # --- test_eos_sign_boot_path: the tool's real output through the real parser --- add_executable(eboot_test_eos_sign_boot_path unit/test_eos_sign_boot_path.c) target_link_libraries(eboot_test_eos_sign_boot_path PRIVATE eboot_core) target_include_directories(eboot_test_eos_sign_boot_path PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}) add_test(NAME test_eos_sign_boot_path COMMAND eboot_test_eos_sign_boot_path) +list(APPEND EBLDR_UNIT_TESTS test_eos_sign_boot_path) # --- Valgrind test targets --- # @@ -218,12 +229,7 @@ add_test(NAME test_eos_sign_boot_path COMMAND eboot_test_eos_sign_boot_path) find_program(VALGRIND valgrind) if(VALGRIND) set(VALGRIND_OPTS --leak-check=full --error-exitcode=1 --quiet) - foreach(TEST_NAME test_bootctl test_crypto test_ed25519 test_ed25519_contract test_keystore - test_device_table test_runtime_svc test_board_config - test_multicore test_board_registry test_slot_manager - test_boot_log test_image_verify test_image_abi - test_recovery test_slot_size_bounds test_fw_transport - test_fw_update test_jump_app test_tlv_auth) + foreach(TEST_NAME ${EBLDR_UNIT_TESTS}) add_test( NAME valgrind_${TEST_NAME} COMMAND ${VALGRIND} ${VALGRIND_OPTS} $ diff --git a/tests/unit/test_ed25519.c b/tests/unit/test_ed25519.c index 6c22666..9ea837c 100644 --- a/tests/unit/test_ed25519.c +++ b/tests/unit/test_ed25519.c @@ -90,46 +90,6 @@ static const struct rfc_vector k_vectors[] = { #define N_VECTORS (sizeof(k_vectors) / sizeof(k_vectors[0])) -/* All eight compressed encodings of edwards25519's order-8 torsion subgroup - * (the cyclic group for any order-8 point G, i.e. {1G, 2G, ..., 8G=O}). - * Order 8/gcd(k,8) for kG: orders present are 1 (identity), 2, 4, 4, 8, 8, 8, 8. - * - * Not hand-transcribed: derived by decoding an order-8 generator from this - * file's own unpackneg()/point_add()/point_is_identity(), then enumerating - * 1G..8G with point_add() and re-encoding with point_pack() -- so their - * correctness rests on the same curve arithmetic this file already uses for - * real verification, not on a separately-copied constant that could carry a - * transcription error. Cross-checked: point k and point (8-k) differ only in - * the sign bit (byte 31), as required since (8-k)G = -(kG); the order-2 - * element (k=4) is its own negation, as required since 2P = O implies P = -P. - */ -static const uint8_t k_low_order[8][32] = { - {0x26,0xe8,0x95,0x8f,0xc2,0xb2,0x27,0xb0,0x45,0xc3,0xf4,0x89,0xf2,0xef,0x98,0xf0, - 0xd5,0xdf,0xac,0x05,0xd3,0xc6,0x33,0x39,0xb1,0x38,0x02,0x88,0x6d,0x53,0xfc,0x85}, - {0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00, - 0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x80}, - {0xc7,0x17,0x6a,0x70,0x3d,0x4d,0xd8,0x4f,0xba,0x3c,0x0b,0x76,0x0d,0x10,0x67,0x0f, - 0x2a,0x20,0x53,0xfa,0x2c,0x39,0xcc,0xc6,0x4e,0xc7,0xfd,0x77,0x92,0xac,0x03,0xfa}, - {0xec,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff, - 0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0x7f}, - {0xc7,0x17,0x6a,0x70,0x3d,0x4d,0xd8,0x4f,0xba,0x3c,0x0b,0x76,0x0d,0x10,0x67,0x0f, - 0x2a,0x20,0x53,0xfa,0x2c,0x39,0xcc,0xc6,0x4e,0xc7,0xfd,0x77,0x92,0xac,0x03,0x7a}, - {0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00, - 0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00}, - {0x26,0xe8,0x95,0x8f,0xc2,0xb2,0x27,0xb0,0x45,0xc3,0xf4,0x89,0xf2,0xef,0x98,0xf0, - 0xd5,0xdf,0xac,0x05,0xd3,0xc6,0x33,0x39,0xb1,0x38,0x02,0x88,0x6d,0x53,0xfc,0x05}, - {0x01,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00, - 0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00}, -}; - -/* A handful of distinct messages, so a low-order-R rejection isn't pinned - * against only one k = SHA-512(R || A || M) value. */ -static const char *messages[] = { - "", - "a", - "untrusted firmware", -}; - /* ---- positive tests: a conforming signature MUST be accepted ---- */ TEST(test_ed25519_rfc8032_vectors_accepted) diff --git a/tests/unit/test_fw_decrypt.c b/tests/unit/test_fw_decrypt.c index ca3c358..ac1b26a 100644 --- a/tests/unit/test_fw_decrypt.c +++ b/tests/unit/test_fw_decrypt.c @@ -27,6 +27,7 @@ static int tests_passed = 0; static void name(void); \ static void run_##name(void) { \ printf(" %-52s ", #name); \ + tests_run++; \ name(); \ tests_passed++; \ printf("[PASS]\n"); \ @@ -265,7 +266,6 @@ int main(void) run_test_init_rejects_bad_arguments_and_unprovisioned_keys(); run_test_update_and_final_reject_uninitialised_contexts(); - tests_run = 8; printf("\n%d/%d tests passed\n", tests_passed, tests_run); return (tests_passed == tests_run) ? 0 : 1; } diff --git a/tests/unit/test_fw_transport.c b/tests/unit/test_fw_transport.c index b5ce142..6d052e5 100644 --- a/tests/unit/test_fw_transport.c +++ b/tests/unit/test_fw_transport.c @@ -16,6 +16,7 @@ #include "eos_image.h" #include "eos_image_tlv.h" #include "eos_hal.h" +#include "../vectors/fw_update_test_sigs.h" #include #include #include @@ -102,6 +103,23 @@ static eos_reset_reason_t sim_reset_reason(void) { return EOS_RESET_POWER_ON; } static bool sim_recovery_pin(void) { return false; } static void sim_system_reset(void) {} +/* Since #104 finalize verifies the image signature unconditionally, so the + * container the XMODEM test finalizes has to be genuinely signed. The keystore + * takes its anchor from OTP slot 0 when the board has OTP at all; serve the + * public half of the key the fixture signature was made under. */ +#define OTP_KEY_OFFSET_SLOT0 0x100u + +static int sim_otp_read(uint32_t offset, void *buf, size_t len) +{ + if (!buf) return EOS_ERR_INVALID; + if (offset == OTP_KEY_OFFSET_SLOT0 && len == sizeof(eos_test_sig_pubkey)) { + memcpy(buf, eos_test_sig_pubkey, len); + return EOS_OK; + } + memset(buf, 0, len); /* slot 1 unprovisioned, nothing revoked */ + return EOS_OK; +} + static const eos_board_ops_t sim_ops = { .flash_base = 0, .flash_size = SIM_FLASH_SIZE, @@ -119,6 +137,7 @@ static const eos_board_ops_t sim_ops = { .flash_read = sim_flash_read, .flash_write = sim_flash_write, .flash_erase = sim_flash_erase, + .otp_read = sim_otp_read, .watchdog_init = sim_noop_u32, .watchdog_feed = sim_noop, @@ -354,7 +373,8 @@ static void build_container(void) hdr.load_addr = SIM_SLOT_B_ADDR; hdr.entry_addr = SIM_SLOT_B_ADDR; hdr.flags = 0; /* CRC32 integrity path */ - hdr.sig_type = EOS_SIG_NONE; + hdr.sig_type = EOS_SIG_ED25519; + hdr.sig_len = EOS_SIG_MAX_SIZE; uint32_t crc = crc32_payload(payload, CONT_PAYLOAD_LEN); memcpy(hdr.hash, &crc, sizeof(crc)); @@ -365,6 +385,10 @@ static void build_container(void) hdr.tlv_len = CONT_TLV_LEN; memcpy(hdr.tlv_hash, digest, EOS_IMG_TLV_HASH_LEN); + /* Precomputed by tools/gen_fw_update_test_sigs.py for exactly the field + * values above; change any of them and regenerate. */ + memcpy(hdr.signature, eos_test_sig_fw_transport_container, EOS_SIG_MAX_SIZE); + memcpy(container, &hdr, sizeof(hdr)); } @@ -880,7 +904,6 @@ int main(void) run_test_raw_oversized_length_is_rejected(); run_test_raw_zero_length_is_rejected(); - tests_run = 19; printf("\n%d/%d tests passed\n", tests_passed, tests_run); return (tests_passed == tests_run) ? 0 : 1; } diff --git a/tests/unit/test_fw_update.c b/tests/unit/test_fw_update.c index afe54d8..ae87c91 100644 --- a/tests/unit/test_fw_update.c +++ b/tests/unit/test_fw_update.c @@ -18,6 +18,7 @@ #include "eos_crypto_boot.h" #include "eos_bootctl.h" #include "eos_hal.h" +#include "../vectors/fw_update_test_sigs.h" #include #include #include @@ -65,6 +66,24 @@ static int sim_monotonic_read(uint32_t *value) return EOS_OK; } +/* Since #104 finalize verifies the image signature unconditionally, and it + * does so before the anti-rollback check, so an image has to be genuinely + * signed to reach the stage these tests exercise. The keystore takes its + * anchor from OTP slot 0 when the board has OTP at all; serve the public + * half of the key the fixture signatures were made under, and nothing else. */ +#define OTP_KEY_OFFSET_SLOT0 0x100u + +static int sim_otp_read(uint32_t offset, void *buf, size_t len) +{ + if (!buf) return EOS_ERR_INVALID; + if (offset == OTP_KEY_OFFSET_SLOT0 && len == sizeof(eos_test_sig_pubkey)) { + memcpy(buf, eos_test_sig_pubkey, len); + return EOS_OK; + } + memset(buf, 0, len); /* slot 1 unprovisioned, nothing revoked */ + return EOS_OK; +} + static const eos_board_ops_t sim_ops = { .flash_base = 0, .flash_size = SIM_FLASH_SIZE, @@ -79,6 +98,7 @@ static const eos_board_ops_t sim_ops = { .flash_write = sim_flash_write, .flash_erase = sim_flash_erase, .monotonic_read = sim_monotonic_read, + .otp_read = sim_otp_read, }; static int tests_run = 0; @@ -98,6 +118,7 @@ static int tests_passed = 0; sim_counter = 0; \ eos_hal_init(&sim_ops); \ printf(" %-58s ", #name); \ + tests_run++; \ name(); \ tests_passed++; \ printf("[PASS]\n"); \ @@ -135,8 +156,8 @@ static void build_image(uint8_t *out, uint32_t sec_ver) hdr.image_version = 0x00010000u; hdr.flags = EOS_IMG_FLAG_HASH_SHA256; eos_sha256(payload, PAYLOAD_SIZE, hdr.hash); - hdr.sig_type = EOS_SIG_NONE; - hdr.sig_len = 0; + hdr.sig_type = EOS_SIG_ED25519; + hdr.sig_len = EOS_SIG_MAX_SIZE; uint8_t tlv[TLV_AREA_LEN]; eos_tlv_info_t info = { EOS_TLV_INFO_MAGIC, TLV_AREA_LEN }; @@ -150,6 +171,15 @@ static void build_image(uint8_t *out, uint32_t sec_ver) hdr.tlv_len = TLV_AREA_LEN; memcpy(hdr.tlv_hash, digest, EOS_IMG_TLV_HASH_LEN); + /* Signature over the prefix, precomputed by tools/gen_fw_update_test_sigs.py + * for exactly the field values above. Every other sec_ver would need its + * own entry there, because tlv_hash is inside the signed prefix. */ + switch (sec_ver) { + case 3: memcpy(hdr.signature, eos_test_sig_fw_update_sec_ver_3, EOS_SIG_MAX_SIZE); break; + case 9: memcpy(hdr.signature, eos_test_sig_fw_update_sec_ver_9, EOS_SIG_MAX_SIZE); break; + default: printf("[FAIL] no fixture signature for sec_ver %u\n", (unsigned)sec_ver); exit(1); + } + memset(out, 0, IMAGE_BUF_LEN); memcpy(out, &hdr, sizeof(hdr)); memcpy(out + sizeof(hdr), payload, PAYLOAD_SIZE); @@ -315,7 +345,6 @@ int main(void) run_test_trailing_byte_is_rejected_the_same_across_chunk_boundaries(); run_test_finalize_accepts_tlv_counter_equal_to_floor(); - tests_run = 6; printf("\n%d/%d passed\n", tests_passed, tests_run); return tests_passed == tests_run ? 0 : 1; } diff --git a/tests/unit/test_fw_update_sig.c b/tests/unit/test_fw_update_sig.c index 42970d6..b510ede 100644 --- a/tests/unit/test_fw_update_sig.c +++ b/tests/unit/test_fw_update_sig.c @@ -129,6 +129,7 @@ static int tests_passed = 0; static void run_##name(void) { \ setup(); \ printf(" %-58s ", #name); \ + tests_run++; \ name(); \ tests_passed++; \ printf("[OK]\n"); \ @@ -249,7 +250,6 @@ int main(void) run_test_sha256_sigtype_is_still_unsigned_and_rejected(); run_test_corrupt_image_is_rejected_at_integrity_stage(); - tests_run = 3; printf("\n%d/%d tests passed\n", tests_passed, tests_run); return (tests_passed == tests_run) ? 0 : 1; } diff --git a/tests/unit/test_jump_app.c b/tests/unit/test_jump_app.c index e9a07e6..c882119 100644 --- a/tests/unit/test_jump_app.c +++ b/tests/unit/test_jump_app.c @@ -221,6 +221,7 @@ static int tests_passed = 0; eos_hal_init(&sim_ops); \ eos_rollback_clear_staged(); \ printf(" %-58s ", #name); \ + tests_run++; \ name(); \ tests_passed++; \ printf("[PASS]\n"); \ @@ -265,7 +266,6 @@ int main(void) run_test_jump_rejects_tlv_counter_below_hw_floor(); run_test_jump_stages_tlv_counter_above_floor(); - tests_run = 2; printf("\n%d/%d passed\n", tests_passed, tests_run); return tests_passed == tests_run ? 0 : 1; } diff --git a/tests/unit/test_jump_app_bounds.c b/tests/unit/test_jump_app_bounds.c index 5e08b6f..496e161 100644 --- a/tests/unit/test_jump_app_bounds.c +++ b/tests/unit/test_jump_app_bounds.c @@ -115,6 +115,7 @@ static int tests_passed = 0; payload_bytes_read = 0; \ eos_hal_init(&sim_ops); \ printf(" %-55s ", #name); \ + tests_run++; \ name(); \ tests_passed++; \ printf("[PASS]\n"); \ @@ -191,7 +192,6 @@ int main(void) run_test_oversized_image_rejected_before_reading_payload(); run_test_in_bounds_image_reaches_integrity_check(); - tests_run = 2; printf("\n%d/%d tests passed\n", tests_passed, tests_run); diff --git a/tests/unit/test_secure_boot_policy.c b/tests/unit/test_secure_boot_policy.c index be092d5..39bc2c2 100644 --- a/tests/unit/test_secure_boot_policy.c +++ b/tests/unit/test_secure_boot_policy.c @@ -128,6 +128,11 @@ static void reset_fixture(void) memset(sim_otp, 0, sizeof(sim_otp)); sim_ops.otp_read = sim_otp_read; sim_ops.flash_read = sim_flash_read; + /* Step 5b (anti-rollback, #103) reads the image's TLV counter through the + * HAL slot that contains it, and an image in no slot is a bad header + * before step 7 is ever reached. The staged image is slot A. */ + sim_ops.slot_a_addr = FLASH_BASE; + sim_ops.slot_a_size = FLASH_SIZE; if (provide_otp_write) sim_ops.otp_write = sim_otp_write; otp_write_rc = EOS_OK; otp_write_calls = 0; diff --git a/tests/unit/test_suite_bookkeeping.py b/tests/unit/test_suite_bookkeeping.py index afa59bb..699cca7 100644 --- a/tests/unit/test_suite_bookkeeping.py +++ b/tests/unit/test_suite_bookkeeping.py @@ -37,6 +37,14 @@ "test_boot_log.c": "prints its own summary and has no TEST() macro", "test_ecc.c": "single-scenario suite; no per-test harness", "test_image_abi.c": "compile-time _Static_asserts; nothing runs per test", + "test_ed25519_contract.c": "one loop over the generated vector table; " + "counts accepted/refused/wrong per vector", + "test_eos_sign_boot_path.c": "CHECK() counts failures, not tests; the " + "exit code is the failure count", + "test_fdt_loader.c": "RUN() macro with exit(1) on the first failed " + "ASSERT; tests_passed is the count", + "test_qemu_arm64_timer.c": "four inline ASSERT_EQ calls that exit(1) on " + "failure; no per-test harness", } diff --git a/tests/unit/test_tlv_auth.c b/tests/unit/test_tlv_auth.c index cd4c9c2..2046a37 100644 --- a/tests/unit/test_tlv_auth.c +++ b/tests/unit/test_tlv_auth.c @@ -362,7 +362,6 @@ int main(void) run_test_tlv_area_must_fit_in_slot(); run_test_hw_floor_uses_tlv_counter_not_image_version(); - tests_run = 9; printf("\n%d/%d passed\n", tests_passed, tests_run); return tests_passed == tests_run ? 0 : 1; } diff --git a/tests/vectors/fw_update_test_sigs.h b/tests/vectors/fw_update_test_sigs.h new file mode 100644 index 0000000..14ea295 --- /dev/null +++ b/tests/vectors/fw_update_test_sigs.h @@ -0,0 +1,43 @@ +/* Generated by tools/gen_fw_update_test_sigs.py -- do not edit. + * + * Ed25519 signatures over the 92-byte signed header prefix of the + * images tests/unit/test_fw_update.c and test_fw_transport.c build, + * under the RFC 8032 section 7.1 TEST 1 key. Regenerate with: + * + * python3 tools/gen_fw_update_test_sigs.py > tests/vectors/fw_update_test_sigs.h + */ +#ifndef EOS_FW_UPDATE_TEST_SIGS_H +#define EOS_FW_UPDATE_TEST_SIGS_H + +/* The public half. Tests serve it from simulated OTP slot 0 so the + * keystore selects it the way a provisioned board would. */ +static const unsigned char eos_test_sig_pubkey[32] = { + 0xd7,0x5a,0x98,0x01,0x82,0xb1,0x0a,0xb7,0xd5,0x4b,0xfe,0xd3,0xc9,0x64,0x07,0x3a, + 0x0e,0xe1,0x72,0xf3,0xda,0xa6,0x23,0x25,0xaf,0x02,0x1a,0x68,0xf7,0x07,0x51,0x1a, +}; + +/* test_fw_update.c build_image(out, 3) */ +static const unsigned char eos_test_sig_fw_update_sec_ver_3[64] = { + 0x67,0xf0,0x58,0x37,0x33,0x61,0x23,0x1e,0xa7,0x6e,0x04,0x59,0x35,0xc7,0x5e,0x84, + 0xfc,0xac,0xce,0x67,0x64,0x7c,0x9b,0xbf,0x17,0xed,0x0a,0xcb,0xc3,0x97,0x13,0xeb, + 0xa4,0xd9,0x1c,0x60,0xc3,0x08,0xa0,0xdf,0xd9,0x06,0x7f,0xf6,0x62,0xd4,0x98,0x3f, + 0x23,0xa6,0x54,0x32,0x48,0x1a,0xc5,0xb3,0xb6,0xe0,0xac,0xf4,0xf1,0x47,0x99,0x07, +}; + +/* test_fw_update.c build_image(out, 9) */ +static const unsigned char eos_test_sig_fw_update_sec_ver_9[64] = { + 0x33,0xcd,0x40,0x50,0x31,0x62,0x91,0x2c,0x9a,0x2a,0x22,0x09,0x0b,0x4a,0xb6,0x69, + 0x2d,0x9c,0x0c,0x20,0x49,0x91,0x9b,0xe0,0x15,0x5d,0x74,0xe1,0x94,0x7e,0x34,0xe5, + 0x41,0x96,0x5c,0x2f,0x3c,0x17,0xa8,0x1b,0x34,0xae,0x6b,0x76,0x7b,0x19,0xbf,0xc0, + 0x91,0xe7,0xc9,0xd8,0x65,0x2f,0x1f,0xa8,0x10,0x74,0x77,0xcb,0x64,0x71,0xe7,0x02, +}; + +/* test_fw_transport.c build_container() */ +static const unsigned char eos_test_sig_fw_transport_container[64] = { + 0x25,0xbb,0xa5,0x30,0x94,0xbe,0xaa,0x7b,0xe5,0xaf,0x2a,0xf6,0x27,0x09,0xa1,0xcd, + 0x42,0x4d,0x20,0xdb,0xde,0xa1,0x28,0x65,0x1b,0x03,0xf8,0xdb,0x77,0x59,0x09,0xf2, + 0x56,0x6f,0xe8,0x84,0x7f,0x2c,0xa1,0x32,0xf0,0xbc,0xd3,0x05,0x7c,0xd8,0xeb,0x84, + 0x08,0x43,0x3a,0xd9,0xf9,0x1a,0xd1,0x47,0xfd,0x5b,0x21,0x48,0x01,0x58,0xcc,0x05, +}; + +#endif /* EOS_FW_UPDATE_TEST_SIGS_H */ diff --git a/tools/gen_fw_update_test_sigs.py b/tools/gen_fw_update_test_sigs.py new file mode 100644 index 0000000..86f18d1 --- /dev/null +++ b/tools/gen_fw_update_test_sigs.py @@ -0,0 +1,156 @@ +#!/usr/bin/env python3 +# SPDX-License-Identifier: MIT +# Copyright (c) 2026 EoS Project +"""Sign the image headers that tests/unit/test_fw_update.c and +tests/unit/test_fw_transport.c stream through eos_fw_update_finalize(). + +Since #104 finalize verifies the Ed25519 signature unconditionally, and it +does so *before* the anti-rollback check, so an unsigned test image can no +longer reach the rollback stage -- it is refused as EOS_ERR_SIGNATURE first. +The images those suites build have to carry a real signature. eBoot has no +Ed25519 signer in C (only a verifier), so the signatures are computed here and +committed as tests/vectors/fw_update_test_sigs.h. + +The signing key is the RFC 8032 section 7.1 TEST 1 key. Its secret half is +printed in the RFC, so nothing here is a secret. The tests provision the +public half through their simulated OTP (slot 0), which is the path +eos_keystore_init() takes on a real provisioned board. + +core/keystore.c's compiled-in default_dev_key is described as this same key +but is not: it differs from byte 21 on and does not decode to a point on the +curve, so no signature can verify against it. That is a defect in its own +right and is not what this generator works around -- the OTP route is used +because it is the production path, not because the fallback is broken. + +Each header prefix below must be byte-identical to what the C test builds; +the field values are copied from the tests, and the layout is the one +tests/unit/test_image_header_abi.c pins. + + python3 tools/gen_fw_update_test_sigs.py > tests/vectors/fw_update_test_sigs.h +""" + +import hashlib +import struct +import sys + +from cryptography.hazmat.primitives import serialization +from cryptography.hazmat.primitives.asymmetric.ed25519 import Ed25519PrivateKey + +# RFC 8032 section 7.1, TEST 1. +RFC8032_TEST1_SECRET = bytes.fromhex( + "9d61b19deffd5a60ba844af492ec2cc44449c5697b326919703bac031cae7f60") + +EOS_IMG_MAGIC = 0x454F5349 +EOS_IMAGE_HDR_VERSION = 2 +EOS_IMG_STRUCT_SIZE = 156 +EOS_IMG_SIGNED_LEN = 92 +EOS_IMG_FLAG_HASH_SHA256 = 1 << 6 +EOS_SIG_ED25519 = 3 +EOS_SIG_MAX_SIZE = 64 +EOS_IMG_TLV_HASH_LEN = 28 +EOS_TLV_INFO_MAGIC = 0x6907 +EOS_TLV_MIN_SEC_VER = 0x50 + + +def crc32_payload(data: bytes) -> int: + """update_crc() in core/fw_update.c, as test_fw_transport.c mirrors it.""" + crc = 0xFFFFFFFF + for b in data: + crc ^= b + for _ in range(8): + crc = (crc >> 1) ^ 0xEDB88320 if crc & 1 else crc >> 1 + return (~crc) & 0xFFFFFFFF + + +def tlv_area(sec_ver: int) -> bytes: + """[tlv_info(4)][entry_hdr(4)][uint32 value] -- the shape both suites build.""" + total = 4 + 4 + 4 + return (struct.pack(" bytes: + # build_image() in tests/unit/test_fw_update.c + payload = bytes((i * 7 + 1) & 0xFF for i in range(256)) + return signed_prefix(image_size=256, load_addr=0, entry_addr=0, + version=0x00010000, flags=EOS_IMG_FLAG_HASH_SHA256, + hash32=hashlib.sha256(payload).digest(), + tlv=tlv_area(sec_ver)) + + +def fw_transport_prefix() -> bytes: + # build_container() in tests/unit/test_fw_transport.c: CRC32 integrity + # path (flags = 0), the CRC in the first four bytes of hash[]. + payload = bytes(0x5A + (i & 0x1F) for i in range(256)) + hash32 = struct.pack(" str: + lines = [] + for i in range(0, len(b), 16): + lines.append(indent + ",".join("0x%02x" % x for x in b[i:i + 16]) + ",") + return "\n".join(lines) + + +def main() -> int: + key = Ed25519PrivateKey.from_private_bytes(RFC8032_TEST1_SECRET) + pub = key.public_key().public_bytes(serialization.Encoding.Raw, + serialization.PublicFormat.Raw) + + vectors = [ + ("fw_update_sec_ver_3", fw_update_prefix(3), + "test_fw_update.c build_image(out, 3)"), + ("fw_update_sec_ver_9", fw_update_prefix(9), + "test_fw_update.c build_image(out, 9)"), + ("fw_transport_container", fw_transport_prefix(), + "test_fw_transport.c build_container()"), + ] + + out = [] + out.append("/* Generated by tools/gen_fw_update_test_sigs.py -- do not edit.") + out.append(" *") + out.append(" * Ed25519 signatures over the 92-byte signed header prefix of the") + out.append(" * images tests/unit/test_fw_update.c and test_fw_transport.c build,") + out.append(" * under the RFC 8032 section 7.1 TEST 1 key. Regenerate with:") + out.append(" *") + out.append(" * python3 tools/gen_fw_update_test_sigs.py > tests/vectors/fw_update_test_sigs.h") + out.append(" */") + out.append("#ifndef EOS_FW_UPDATE_TEST_SIGS_H") + out.append("#define EOS_FW_UPDATE_TEST_SIGS_H") + out.append("") + out.append("/* The public half. Tests serve it from simulated OTP slot 0 so the") + out.append(" * keystore selects it the way a provisioned board would. */") + out.append("static const unsigned char eos_test_sig_pubkey[32] = {") + out.append(carr(pub)) + out.append("};") + for name, prefix, origin in vectors: + sig = key.sign(prefix) + out.append("") + out.append("/* %s */" % origin) + out.append("static const unsigned char eos_test_sig_%s[64] = {" % name) + out.append(carr(sig)) + out.append("};") + out.append("") + out.append("#endif /* EOS_FW_UPDATE_TEST_SIGS_H */") + sys.stdout.write("\n".join(out) + "\n") + return 0 + + +if __name__ == "__main__": + sys.exit(main()) From 2b30fd74eaa0399bc6fac22f89203b0cf438b94e Mon Sep 17 00:00:00 2001 From: kartikey1306 Date: Fri, 11 Sep 2026 16:30:05 +0530 Subject: [PATCH 02/13] test(imgpack): pass the version as --version=X so argparse never sees -1.0.0 as an option test_out_of_range_version_is_rejected[-1.0.0] passed locally (Python 3.14) and failed in CI (ubuntu-22.04, Python 3.10) with argparse's own "expected one argument": the older negative-number matcher does not accept -1.0.0, so the token was read as an unknown option and imgpack.py's range check -- the thing under test -- never ran. The joined form is unambiguous on every interpreter and the test now reaches the tool's message. --- tests/unit/test_imgpack.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/tests/unit/test_imgpack.py b/tests/unit/test_imgpack.py index dc0fc32..7cee682 100644 --- a/tests/unit/test_imgpack.py +++ b/tests/unit/test_imgpack.py @@ -32,7 +32,13 @@ def _pack(tmp_path, version): "--input", str(tmp_path / "fw.bin"), "--output", str(tmp_path / "fw.eimg"), "--load-addr", "0x08010000", "--entry-addr", "0x08010100", - "--version", version], + # `--version=X`, not `--version X`: a value such as "-1.0.0" is a + # separate token in the second form, and whether argparse reads it + # as a value or as an unknown option depends on the interpreter -- + # 3.10 refuses it with "expected one argument" before imgpack.py + # runs, 3.13+ accepts it. The test is about imgpack's own check, + # which only the joined form reaches on every version. + f"--version={version}"], capture_output=True, text=True) From a95400f275b857cf77a60abcb00a2ad3186d60c0 Mon Sep 17 00:00:00 2001 From: kartikey1306 Date: Mon, 14 Sep 2026 19:00:03 +0530 Subject: [PATCH 03/13] test(fw_update): pin the fixture header to its generator tests/vectors/fw_update_test_sigs.h is the output of tools/gen_fw_update_test_sigs.py, committed because eBoot has no Ed25519 signer in C. Nothing checked that the two agree: a generator edit without a regeneration leaves test_fw_update and test_fw_transport verifying against stale signatures, failing with EOS_ERR_SIGNATURE and nothing to say why. tests/unit/test_fw_update_test_sigs.py runs the generator with the test's own interpreter and compares its stdout to the committed header byte for byte, so a line-ending change counts too. It follows the same dependency rule as test_eos_sign_payload_offset.py: with EOS_REQUIRE_SIGNING_TESTS set (the CI workflow sets it before the pytest step) a missing cryptography module fails the job instead of skipping. Negative control: one flipped hex byte in the header fails the test with a unified diff naming the line. build_image() and build_container() now carry a comment naming the coupling: the signed prefix is assembled both there and in the generator, and changing any field in it means changing the generator's copy and regenerating the header. --- tests/unit/test_fw_transport.c | 7 +++ tests/unit/test_fw_update.c | 7 +++ tests/unit/test_fw_update_test_sigs.py | 61 ++++++++++++++++++++++++++ 3 files changed, 75 insertions(+) create mode 100644 tests/unit/test_fw_update_test_sigs.py diff --git a/tests/unit/test_fw_transport.c b/tests/unit/test_fw_transport.c index 6d052e5..92d51d3 100644 --- a/tests/unit/test_fw_transport.c +++ b/tests/unit/test_fw_transport.c @@ -344,6 +344,13 @@ static uint32_t crc32_payload(const uint8_t *data, size_t len) return ~crc; } +/* Coupling: the signed prefix (the first EOS_IMG_SIGNED_LEN = 92 bytes of + * the header, plus the TLV area whose hash sits inside it) is built here + * AND in tools/gen_fw_update_test_sigs.py, which signs it. Changing any + * field in it means changing the generator's copy too and re-running + * python3 tools/gen_fw_update_test_sigs.py > tests/vectors/fw_update_test_sigs.h + * tests/unit/test_fw_update_test_sigs.py fails until the committed header + * is regenerated from the generator. */ static void build_container(void) { eos_image_header_t hdr; diff --git a/tests/unit/test_fw_update.c b/tests/unit/test_fw_update.c index ae87c91..f6a3129 100644 --- a/tests/unit/test_fw_update.c +++ b/tests/unit/test_fw_update.c @@ -139,6 +139,13 @@ static void fill_payload(uint8_t *payload) payload[i] = (uint8_t)(i * 7u + 1u); } +/* Coupling: the signed prefix (the first EOS_IMG_SIGNED_LEN = 92 bytes of + * the header, plus the TLV area whose hash sits inside it) is built here + * AND in tools/gen_fw_update_test_sigs.py, which signs it. Changing any + * field in it means changing the generator's copy too and re-running + * python3 tools/gen_fw_update_test_sigs.py > tests/vectors/fw_update_test_sigs.h + * tests/unit/test_fw_update_test_sigs.py fails until the committed header + * is regenerated from the generator. */ static void build_image(uint8_t *out, uint32_t sec_ver) { uint8_t payload[PAYLOAD_SIZE]; diff --git a/tests/unit/test_fw_update_test_sigs.py b/tests/unit/test_fw_update_test_sigs.py new file mode 100644 index 0000000..4db5c9d --- /dev/null +++ b/tests/unit/test_fw_update_test_sigs.py @@ -0,0 +1,61 @@ +# SPDX-License-Identifier: MIT +# Copyright (c) 2026 EoS Project + +"""tests/vectors/fw_update_test_sigs.h must be what its generator emits. + +tools/gen_fw_update_test_sigs.py signs the header prefixes that +tests/unit/test_fw_update.c build_image() and tests/unit/test_fw_transport.c +build_container() assemble, and the C suites include the committed header. +If the generator changes and the header is not regenerated, the C suites +verify against stale signatures and fail with EOS_ERR_SIGNATURE, and nothing +says why. This pins the committed header to the generator's output, byte for +byte, so line endings count too. +""" + +import difflib +import os +import subprocess +import sys +from pathlib import Path + +import pytest + +REPO_ROOT = Path(__file__).resolve().parents[2] +TOOLS = REPO_ROOT / "tools" + +# A skip is right for a developer without the signing dependency installed and +# wrong for CI, where "collected 19 tests, ran 0" is a green run that checked +# nothing -- the failure .ai/security.md names directly. EOS_REQUIRE_SIGNING_TESTS +# is set in the workflow, so there a missing dependency is a hard error; locally +# the skip still applies. +if os.environ.get("EOS_REQUIRE_SIGNING_TESTS"): + import cryptography # noqa: F401 -- ImportError here must fail the job +else: + pytest.importorskip( + "cryptography", reason="signing tools require 'cryptography'") + +GENERATOR = TOOLS / "gen_fw_update_test_sigs.py" +HEADER = REPO_ROOT / "tests" / "vectors" / "fw_update_test_sigs.h" +REGENERATE = ("python3 tools/gen_fw_update_test_sigs.py " + "> tests/vectors/fw_update_test_sigs.h") + + +def test_committed_header_is_the_generator_output(): + result = subprocess.run( + [sys.executable, str(GENERATOR)], + cwd=REPO_ROOT, capture_output=True, check=True, + ) + expected = result.stdout + actual = HEADER.read_bytes() + + if actual != expected: + diff = "".join(difflib.unified_diff( + expected.decode("ascii", "replace").splitlines(keepends=True), + actual.decode("ascii", "replace").splitlines(keepends=True), + fromfile="tools/gen_fw_update_test_sigs.py (stdout)", + tofile="tests/vectors/fw_update_test_sigs.h (committed)", + )) + pytest.fail( + "tests/vectors/fw_update_test_sigs.h differs from what " + "tools/gen_fw_update_test_sigs.py emits; regenerate it with\n" + " " + REGENERATE + "\n" + diff) From edfbaed3145d2b839a47d288a8167028525db136 Mon Sep 17 00:00:00 2001 From: kartikey1306 Date: Mon, 14 Sep 2026 19:00:03 +0530 Subject: [PATCH 04/13] docs: record the install-path check ordering as ADR-020 and log the repair #103 and #104 were both merged and disagree on whether the anti-rollback counter or the signature is checked first in eos_fw_update_finalize(). The master design orders boot as verify image, then version policy (section 8.1), but its update flow (section 15) never places the anti-rollback check, so the order the install path uses existed only in a PR body. ADR-020 records it: the signature over the signed header prefix is verified first, the TLV counter is read only after the prefix that binds it is authenticated, and an image that fails verification is refused as EOS_ERR_SIGNATURE without its counter being consulted. docs/adr/README.md is added in the shape of the eos repository's index; 020 avoids reusing 001 through 019. CHANGELOG.md gains the Unreleased entries for the repair: the configure, compile and link breakage after the 09-07 batch merge, the settled check ordering with the suites streaming signed images, the re-derived Valgrind list, fuzz-build in the CI gate, counted tests_run, the EoSim Windows legs running under bash, and the Scorecard action on its ghcr.io-hosted release. --- CHANGELOG.md | 7 +++ ...verifies-signature-before-anti-rollback.md | 58 +++++++++++++++++++ docs/adr/README.md | 22 +++++++ 3 files changed, 87 insertions(+) create mode 100644 docs/adr/ADR-020-install-path-verifies-signature-before-anti-rollback.md create mode 100644 docs/adr/README.md diff --git a/CHANGELOG.md b/CHANGELOG.md index 2fb33e3..2ff7953 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,13 @@ - **`image_verify.c`:** `eos_image_verify_integrity` rejects a zero `image_size`, and an `addr + hdr_size` that wraps `uint32_t`, instead of computing a payload address that is not the payload. ### Fixed +- **The tree did not configure, compile or link after the 09-07 batch merge.** `tests/CMakeLists.txt` registered `eboot_test_fdt_loader` twice; `core/sha512.c` had been replaced by a version predating the `bitlen[2]`/`buffer_len` context; `core/boot_log.c`, `core/secure_boot.c` and `core/fdt_loader.c` had been dropped from `eboot_core`; `scalarbase()` and `k_low_order[]` were defined twice; and the `eos_boot_log_get_head()` declaration was lost. All restored. +- **Install-path verification order settled: signature before anti-rollback.** `eos_fw_update_finalize()` verifies the Ed25519 signature over the signed header prefix first and reads the TLV security counter only after the prefix that binds it is authenticated (see `docs/adr/ADR-020`). The `fw_update` and `fw_transport` suites now stream genuinely signed images; `tools/gen_fw_update_test_sigs.py` emits their signatures as `tests/vectors/fw_update_test_sigs.h`, and `tests/unit/test_fw_update_test_sigs.py` pins the committed header to the generator's output. +- **`tests/CMakeLists.txt`:** the Valgrind list is derived from the registered suites again; a hand-written copy had replaced it and eleven suites had no Valgrind run. They are re-added. +- **`.github/workflows/ci.yml`:** `fuzz-build` is in the CI gate. It was added after the gate job and the gate never waited for it. +- **Unit suites count `tests_run`** as each test executes instead of assigning it a literal that the summary line then trusted. +- **`.github/workflows/eosim-sanity.yml`:** the install-validate job's steps are bash and now run under `shell: bash` on the Windows legs, where PowerShell rejected `SITE_PACKAGES=$(...)` and parsed `|| { exit 1 }` as an unexecuted script block. +- **`.github/workflows/scorecard.yml`:** `ossf/scorecard-action` moved to v2.4.3, the release hosted on ghcr.io; v2.4.0 pulls from gcr.io, which now requires GCP billing. - **The tree did not compile.** `include/eos_image.h` declared `eos_crc32()` as `int eos_crc32(uint32_t, size_t, uint32_t *)` while `core/image_verify.c` defined it as `uint32_t eos_crc32(uint32_t, size_t)` -- a conflicting-types error that stopped the build at the first core source file. The declaration now matches the definition and the documented behaviour. - **`ed25519_verify.c`:** `eos_ed25519_verify()` never performed the verification. Two merged copies of the challenge-hash step had been left in the function, the second referring to identifiers that do not exist (`sha512_ctx_t`, `sc_reduce`), and RFC 8032 step 4 -- the `[S]B == R + [k]A` check -- was absent entirely, leaving the function returning an undeclared `diff`. The duplicate is removed and the group-equation check restored; the function now passes the RFC 8032 test vectors and rejects tampered messages, every single-bit signature flip, wrong keys and malleated signatures. - **`recovery.c`:** `recovery_handle_write()` declared `slot_size` twice, which does not compile. The bounds check now calls `eos_recovery_write_in_range()` -- the helper the unit tests already exercise -- so the wire-input rule has one definition, and an unmapped slot (`base == 0`) is rejected too. diff --git a/docs/adr/ADR-020-install-path-verifies-signature-before-anti-rollback.md b/docs/adr/ADR-020-install-path-verifies-signature-before-anti-rollback.md new file mode 100644 index 0000000..5bf8b58 --- /dev/null +++ b/docs/adr/ADR-020-install-path-verifies-signature-before-anti-rollback.md @@ -0,0 +1,58 @@ +--- +adr: 20 +title: Install path verifies the signature before anti-rollback +status: Proposed +date: 2026-09-14 +deciders: Architecture Council, eBoot maintainers +source: EmbeddedOS Master Design v2.0 §8.1, §15; ADR-011 (eOTA firmware/update contract) +--- + +# ADR-020 — Install path verifies the signature before anti-rollback + +## Context + +Two pull requests merged on 09-07 both change `eos_fw_update_finalize()` and disagree on +the order of its checks: + +- #103 added an authenticated TLV anti-rollback counter: the image's security counter is + read from its TLV area and compared against the persistent floor at install. +- #104 made the install path verify the image signature unconditionally, no longer gated + on the header's own `sig_type`. + +Neither PR was written against a base that contained the other, so neither says which +check comes first. The master design orders BOOT as Verify Manifest → Verify Image → +Check Version Policy (§8.1). Its update flow (§15) is Download → Verify → Install and +never places the anti-rollback check. The ordering the install path uses therefore +existed only in a PR body. + +## Decision + +In the install path (`core/fw_update.c`, `eos_fw_update_finalize()`): + +1. Signature verification over the signed header prefix (`EOS_IMG_SIGNED_LEN`, the first + 92 bytes of the header) precedes anti-rollback evaluation. +2. The TLV security counter is read only after the prefix that binds it (`tlv_len`, + `tlv_hash`) has been authenticated. +3. An image that fails signature verification is refused as `EOS_ERR_SIGNATURE` and its + counter is never consulted. + +This mirrors the boot ordering of §8.1: authenticate first, then apply version policy to +what was authenticated. + +## Consequences + +- A test that wants to observe the anti-rollback stage must present a genuinely signed + image. eBoot has no Ed25519 signer in C, so `tools/gen_fw_update_test_sigs.py` signs + the header prefixes that `tests/unit/test_fw_update.c` and + `tests/unit/test_fw_transport.c` build and emits `tests/vectors/fw_update_test_sigs.h`; + `tests/unit/test_fw_update_test_sigs.py` pins the committed header to the generator's + output. +- An unsigned image cannot demonstrate a rollback regression. It is refused as + `EOS_ERR_SIGNATURE` before its counter is compared, so a rollback test built on an + unsigned image exercises the signature check, not the floor. + +## Note on numbering + +ADR-001 through ADR-011 belong to the master-design series and are not in this +repository; `eos` holds ADR-012 through ADR-019. This record is numbered 020 so that no +number is reused. It extends ADR-011, the eOTA firmware/update contract. diff --git a/docs/adr/README.md b/docs/adr/README.md new file mode 100644 index 0000000..bc72656 --- /dev/null +++ b/docs/adr/README.md @@ -0,0 +1,22 @@ +# Architecture Decision Records + +One file per decision. A record is never edited after it reaches **Accepted** — it is +superseded by a later record that names it. + +| Status | Meaning | +|---|---| +| Proposed | Written, not yet ratified by the maintainers named in `deciders`. | +| Accepted | Ratified. Binding on new code. | +| Superseded | Replaced; the replacing ADR is named in the header. | + +## Index + +| ADR | Title | Status | +|---|---|---| +| 020 | [Install path verifies the signature before anti-rollback](ADR-020-install-path-verifies-signature-before-anti-rollback.md) | Proposed | + +## Note on numbering + +ADR-001 through ADR-011 belong to the master-design series and are not in this +repository. The `eos` repository holds ADR-012 through ADR-019. This set starts at 020 so +that no number is reused; ADR-020 extends ADR-011, the eOTA firmware/update contract. From b01e9ced90c667c90cdac3ead76e65366c5967e1 Mon Sep 17 00:00:00 2001 From: kartikey1306 Date: Tue, 15 Sep 2026 00:51:09 +0530 Subject: [PATCH 05/13] docs: state the Valgrind gap and the ADR's provenance exactly The changelog said eleven suites had no Valgrind run. Eleven were missing from EBLDR_UNIT_TESTS, but four of those were named in the hand-written foreach and did run; seven had no run at all. Say which. ADR-020: its design-document citation now says where it comes from (the architecture review of #115), and the sentence about the two PRs' bases is replaced with what the history shows -- #103's commits predate #104's merge, and #104 was written without #103's check in place. --- CHANGELOG.md | 2 +- ...-install-path-verifies-signature-before-anti-rollback.md | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2ff7953..ad06fbb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,7 +15,7 @@ ### Fixed - **The tree did not configure, compile or link after the 09-07 batch merge.** `tests/CMakeLists.txt` registered `eboot_test_fdt_loader` twice; `core/sha512.c` had been replaced by a version predating the `bitlen[2]`/`buffer_len` context; `core/boot_log.c`, `core/secure_boot.c` and `core/fdt_loader.c` had been dropped from `eboot_core`; `scalarbase()` and `k_low_order[]` were defined twice; and the `eos_boot_log_get_head()` declaration was lost. All restored. - **Install-path verification order settled: signature before anti-rollback.** `eos_fw_update_finalize()` verifies the Ed25519 signature over the signed header prefix first and reads the TLV security counter only after the prefix that binds it is authenticated (see `docs/adr/ADR-020`). The `fw_update` and `fw_transport` suites now stream genuinely signed images; `tools/gen_fw_update_test_sigs.py` emits their signatures as `tests/vectors/fw_update_test_sigs.h`, and `tests/unit/test_fw_update_test_sigs.py` pins the committed header to the generator's output. -- **`tests/CMakeLists.txt`:** the Valgrind list is derived from the registered suites again; a hand-written copy had replaced it and eleven suites had no Valgrind run. They are re-added. +- **`tests/CMakeLists.txt`:** the Valgrind list is derived from the registered suites again; a hand-written copy had replaced it, eleven registered suites were missing from `EBLDR_UNIT_TESTS`, and seven of those (`test_eos_sign_boot_path`, `test_fdt_loader`, `test_fw_decrypt`, `test_fw_update_sig`, `test_jump_app_bounds`, `test_qemu_arm64_timer`, `test_secure_boot_policy`) had no Valgrind run at all; the other four were only in the hand-written list. All eleven are appended. - **`.github/workflows/ci.yml`:** `fuzz-build` is in the CI gate. It was added after the gate job and the gate never waited for it. - **Unit suites count `tests_run`** as each test executes instead of assigning it a literal that the summary line then trusted. - **`.github/workflows/eosim-sanity.yml`:** the install-validate job's steps are bash and now run under `shell: bash` on the Windows legs, where PowerShell rejected `SITE_PACKAGES=$(...)` and parsed `|| { exit 1 }` as an unexecuted script block. diff --git a/docs/adr/ADR-020-install-path-verifies-signature-before-anti-rollback.md b/docs/adr/ADR-020-install-path-verifies-signature-before-anti-rollback.md index 5bf8b58..cf501df 100644 --- a/docs/adr/ADR-020-install-path-verifies-signature-before-anti-rollback.md +++ b/docs/adr/ADR-020-install-path-verifies-signature-before-anti-rollback.md @@ -4,7 +4,7 @@ title: Install path verifies the signature before anti-rollback status: Proposed date: 2026-09-14 deciders: Architecture Council, eBoot maintainers -source: EmbeddedOS Master Design v2.0 §8.1, §15; ADR-011 (eOTA firmware/update contract) +source: EmbeddedOS Master Design v2.0 §8.1 (boot order) and §15 (update flow), as cited in the architecture review of #115; ADR-011 (eOTA firmware/update contract) --- # ADR-020 — Install path verifies the signature before anti-rollback @@ -19,8 +19,8 @@ the order of its checks: - #104 made the install path verify the image signature unconditionally, no longer gated on the header's own `sig_type`. -Neither PR was written against a base that contained the other, so neither says which -check comes first. The master design orders BOOT as Verify Manifest → Verify Image → +#103's authored commits all predate #104's merge, and #104 was written without #103's +counter check in place, so neither says which check comes first. The master design orders BOOT as Verify Manifest → Verify Image → Check Version Policy (§8.1). Its update flow (§15) is Download → Verify → Install and never places the anti-rollback check. The ordering the install path uses therefore existed only in a PR body. From e152d8edfce4a6dd9d9f4e8748e4938062926620 Mon Sep 17 00:00:00 2001 From: kartikey1306 Date: Tue, 15 Sep 2026 01:31:51 +0530 Subject: [PATCH 06/13] test(fw_update): exercise the stale-header report, not only the match The regeneration test's failure path -- the unified diff under the regenerate command -- ran only when the header was stale, so a green run never executed it and the coverage report said so. The report is now a helper the match asserts with, and a second test drives the helper with two byte strings that differ in one byte and checks the command and both sides of the changed line appear. --- tests/unit/test_fw_update_test_sigs.py | 40 +++++++++++++++++++------- 1 file changed, 29 insertions(+), 11 deletions(-) diff --git a/tests/unit/test_fw_update_test_sigs.py b/tests/unit/test_fw_update_test_sigs.py index 4db5c9d..ec7b825 100644 --- a/tests/unit/test_fw_update_test_sigs.py +++ b/tests/unit/test_fw_update_test_sigs.py @@ -40,6 +40,20 @@ "> tests/vectors/fw_update_test_sigs.h") +def mismatch_message(expected, actual): + """What a stale header is reported as: the regenerate command, then a + unified diff of generator output against the committed file.""" + diff = "".join(difflib.unified_diff( + expected.decode("ascii", "replace").splitlines(keepends=True), + actual.decode("ascii", "replace").splitlines(keepends=True), + fromfile="tools/gen_fw_update_test_sigs.py (stdout)", + tofile="tests/vectors/fw_update_test_sigs.h (committed)", + )) + return ("tests/vectors/fw_update_test_sigs.h differs from what " + "tools/gen_fw_update_test_sigs.py emits; regenerate it with\n" + " " + REGENERATE + "\n" + diff) + + def test_committed_header_is_the_generator_output(): result = subprocess.run( [sys.executable, str(GENERATOR)], @@ -48,14 +62,18 @@ def test_committed_header_is_the_generator_output(): expected = result.stdout actual = HEADER.read_bytes() - if actual != expected: - diff = "".join(difflib.unified_diff( - expected.decode("ascii", "replace").splitlines(keepends=True), - actual.decode("ascii", "replace").splitlines(keepends=True), - fromfile="tools/gen_fw_update_test_sigs.py (stdout)", - tofile="tests/vectors/fw_update_test_sigs.h (committed)", - )) - pytest.fail( - "tests/vectors/fw_update_test_sigs.h differs from what " - "tools/gen_fw_update_test_sigs.py emits; regenerate it with\n" - " " + REGENERATE + "\n" + diff) + assert actual == expected, mismatch_message(expected, actual) + + +def test_a_stale_header_is_reported_with_the_command_and_the_diff(): + """The failure path of the test above, driven directly: one changed + byte must show up as the changed line, under the command that fixes it.""" + expected = b"static const unsigned char k[4] = {\n 0x67,0xf0,\n};\n" + actual = b"static const unsigned char k[4] = {\n 0x68,0xf0,\n};\n" + + message = mismatch_message(expected, actual) + + assert REGENERATE in message + assert "- 0x67,0xf0," in message + assert "+ 0x68,0xf0," in message + assert "tests/vectors/fw_update_test_sigs.h (committed)" in message From 0c31817b17c5167e8476cebd5ffae1e40beaad71 Mon Sep 17 00:00:00 2001 From: kartikey1306 Date: Tue, 15 Sep 2026 02:59:49 +0530 Subject: [PATCH 07/13] ci(book): Eisvogel 3.5.1 for pandoc 3.6.1, and let the workflow test itself No run of "Build & Release Book PDF" in this repository's history has succeeded: every run since the step was added (ef0dc50, 2026-04-25) is a failure, and every one whose log is still readable dies with ! LaTeX Error: Command \chead already defined. (exit 43) The workflow installs pandoc 3.6.1 from the release .deb and Eisvogel 2.4.0, which predates pandoc 3.6. The 3.x line merges pandoc's default template through 3.6.1 and later (its CHANGELOG names each merge) and states pandoc 3 compatibility. The LaTeX-level cause of the clash is not established here; the run on this change is what shows whether the pair works. Three changes to the one step and its trigger: Eisvogel 2.4.0 -> 3.5.1, as EISVOGEL_VERSION next to PANDOC_VERSION. The 3.x tarball extracts into Eisvogel-/ rather than flat, so the copy names the directory -- a bare version bump would have failed at the cp. Ran the new step verbatim against the real release: template installed, 29,929 bytes; the old cp against the 3.x layout fails as predicted. No more `tar xzf ... 2>/dev/null || true`. That was fail-open on the exact step that breaks: a missing or reshaped tarball failed silently and the diagnostic landed on the cp after it. The workflow is added to its own paths: filter, for push and pull_request. It fired only on docs/book/**, so a workflow-only change could not get the run that proves it -- including this one. Verified: parsed structure asserted, not parse success -- both paths lists, push.tags ['v*'], branches, release types, workflow_dispatch and the job list are unchanged apart from the two added path entries; the template step contains no '|| true'. NOT RUN here: the PDF itself (no xelatex on this host). The evidence is this PR's own run of the job, cited in the PR once it exists -- and only if build-pdf ran rather than being skipped on has_book. Closes #134 --- .github/workflows/book-build.yml | 25 +++++++++++++++++++++---- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/.github/workflows/book-build.yml b/.github/workflows/book-build.yml index 5a1887b..76ce819 100644 --- a/.github/workflows/book-build.yml +++ b/.github/workflows/book-build.yml @@ -5,11 +5,16 @@ on: branches: [master, main] paths: - 'docs/book/**' + # The workflow runs on changes to itself, so a toolchain change here + # gets the run that proves it. Before this, the job only fired on + # docs/book/** and a workflow-only PR could not be tested at all. + - '.github/workflows/book-build.yml' tags: - 'v*' pull_request: paths: - 'docs/book/**' + - '.github/workflows/book-build.yml' release: types: [published, created] workflow_dispatch: @@ -23,6 +28,7 @@ permissions: env: PANDOC_VERSION: "3.6.1" + EISVOGEL_VERSION: "3.5.1" jobs: # ═══════════════════════════════════════════════════ @@ -95,12 +101,23 @@ jobs: echo "pandoc $(pandoc --version | head -1)" - name: Install Eisvogel template + # Eisvogel 2.4.0 with pandoc 3.6.1 fails every run with + # "LaTeX Error: Command \chead already defined" (exit 43). The 2.4.0 + # template predates pandoc 3.6; the 3.x line merges pandoc's default + # LaTeX template up to 3.6.1 and beyond (its CHANGELOG names the + # merges) and its README states pandoc 3 compatibility. The exact + # LaTeX-level cause of the clash is not established here -- the run + # on the change is what shows whether the pair works. The 3.x + # tarball extracts into a versioned directory, not flat, so the copy + # names it. + # No `|| true` on tar: a missing or reshaped tarball must fail here, + # at the step that broke, not at the copy after it. run: | mkdir -p ~/.local/share/pandoc/templates - wget -q https://github.com/Wandmalfarbe/pandoc-latex-template/releases/download/v2.4.0/Eisvogel-2.4.0.tar.gz - tar xzf Eisvogel-2.4.0.tar.gz 2>/dev/null || true - cp eisvogel.latex ~/.local/share/pandoc/templates/eisvogel.latex - echo "Eisvogel template installed" + wget -q https://github.com/Wandmalfarbe/pandoc-latex-template/releases/download/v${EISVOGEL_VERSION}/Eisvogel-${EISVOGEL_VERSION}.tar.gz + tar xzf Eisvogel-${EISVOGEL_VERSION}.tar.gz + cp Eisvogel-${EISVOGEL_VERSION}/eisvogel.latex ~/.local/share/pandoc/templates/eisvogel.latex + echo "Eisvogel ${EISVOGEL_VERSION} template installed" - name: Clean source run: | From 7dde9922c79a456371d24f5050c8f96ab4f2649d Mon Sep 17 00:00:00 2001 From: kartikey1306 Date: Tue, 15 Sep 2026 11:44:20 +0530 Subject: [PATCH 08/13] ci(book): Eisvogel 3.4.0, the last release whose fonts TeX Live 2021 ships The first run of this change got past the template clash -- no more "\chead already defined" -- and failed on the next thing: ! LaTeX Error: File `sourcesans.sty' not found. (exit 43) Eisvogel 3.5.0 migrated its default font from the `sourcesanspro` package to `sourcesans` (its CHANGELOG, 2026-06-28), and ubuntu-22.04's TeX Live 2021 has no sourcesans.sty. 3.4.0 is the last release on sourcesanspro, which texlive-fonts-extra ships (checked against the package's file list), and it already carries pandoc's default template merged through 3.9. Install step re-run verbatim against the 3.4.0 release: template installed. The PDF is still NOT RUN here; the PR's next run is the evidence. --- .github/workflows/book-build.yml | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/.github/workflows/book-build.yml b/.github/workflows/book-build.yml index 76ce819..5e59e41 100644 --- a/.github/workflows/book-build.yml +++ b/.github/workflows/book-build.yml @@ -28,7 +28,13 @@ permissions: env: PANDOC_VERSION: "3.6.1" - EISVOGEL_VERSION: "3.5.1" + # 3.4.0, not the newest: 3.5.0 (2026-06-28) migrated the default font from + # the `sourcesanspro` package to `sourcesans`, which ubuntu-22.04's TeX Live + # 2021 does not ship, so 3.5.x fails with "File `sourcesans.sty' not found" + # -- seen on the first run of this change. 3.4.0 is the last release that + # uses sourcesanspro (in texlive-fonts-extra, installed below) and already + # carries pandoc's default template merged through 3.9. + EISVOGEL_VERSION: "3.4.0" jobs: # ═══════════════════════════════════════════════════ From 8ff93b047913895b0df6abb7b0988d84bb383716 Mon Sep 17 00:00:00 2001 From: kartikey1306 Date: Tue, 15 Sep 2026 11:56:00 +0530 Subject: [PATCH 09/13] ci(book): stop loading a second header package under Eisvogel The second run of this PR, on Eisvogel 3.4.0, failed exactly as master does -- "Command \chead already defined" -- which means the first run's apparent progress was an illusion: on 3.5.1 LaTeX died earlier, on a missing font at line ~672 of the template, before it ever reached the header setup at ~890. The clash was never in the Eisvogel version. It is in this workflow. The pandoc step injected, through header-includes, \usepackage{fancyhdr} \pagestyle{fancy} \fancyhead[L]{...} ... and Eisvogel builds its own header and footer with KOMA's scrlayer-scrpage, which defines \chead. The template places $header-includes$ (line 541) before its header block (~890), so fancyhdr defined \chead first and scrlayer-scrpage's \newcommand failed. Two header packages, one command. Both arrived in ef0dc50 (2026-04-25), which is why no run of this job has ever succeeded, on any pandoc or Eisvogel version. The header and footer now go through the template's own variables -- header-left=\leftmark, header-right=, footer-center=, footer-right left at the template's default page number -- and the fancyhdr block is gone. The rest of header-includes (float, booktabs, longtable, caption, graphicx) is unchanged. Eisvogel stays at 3.4.0 for the fonts (3.5.x needs sourcesans.sty, absent from TeX Live 2021). Asserted on the parsed workflow: no fancyhdr/fancyhead/pagestyle{fancy}/ *rulewidth on any non-comment line of the step; the four variables and the remaining header-includes packages present; no comment or blank line inside the backslash-continued pandoc command (a first draft put the explanation inside it, where '#' would have swallowed the continuation); trigger paths, tags and EISVOGEL_VERSION unchanged. Still NOT RUN here: the PDF. Third run is the evidence. --- .github/workflows/book-build.yml | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/.github/workflows/book-build.yml b/.github/workflows/book-build.yml index 5e59e41..bd92bb4 100644 --- a/.github/workflows/book-build.yml +++ b/.github/workflows/book-build.yml @@ -168,6 +168,16 @@ jobs: CITE_ARGS="--citeproc --bibliography=references.bib" fi + # Header and footer go through Eisvogel's own variables (header-left, + # header-right, footer-center; footer-right is the template's default + # page number). The template builds them with KOMA's scrlayer-scrpage, + # which defines \chead. This step used to inject \usepackage{fancyhdr} + # and \fancyhead/\fancyfoot through header-includes, which the template + # places BEFORE its own header setup -- so fancyhdr defined \chead first + # and scrlayer-scrpage's \newcommand failed: "Command \chead already + # defined". Two header packages, one command. Every run since both + # arrived in ef0dc50 died there, whatever the Eisvogel or pandoc version. + pandoc \ book.md \ -o "${{ steps.meta.outputs.pdf_name }}" \ @@ -202,6 +212,9 @@ jobs: -V float-placement-figure=H \ -V caption-justification=centering \ -V table-use-row-colors=true \ + -V "header-left=\leftmark" \ + -V "header-right=${{ steps.meta.outputs.version }}" \ + -V "footer-center=EmbeddedOS Press — embeddedos-org.github.io" \ -V "header-includes=\ \usepackage{float}\ \usepackage{booktabs}\ @@ -210,14 +223,6 @@ jobs: \captionsetup{font=small,labelfont=bf,format=hang}\ \captionsetup[figure]{name=Figure}\ \captionsetup[table]{name=Table}\ - \usepackage{fancyhdr}\ - \pagestyle{fancy}\ - \fancyhead[L]{\small\leftmark}\ - \fancyhead[R]{\small ${{ steps.meta.outputs.version }}}\ - \fancyfoot[C]{\small EmbeddedOS Press — embeddedos-org.github.io}\ - \fancyfoot[R]{\thepage}\ - \renewcommand{\headrulewidth}{0.4pt}\ - \renewcommand{\footrulewidth}{0.2pt}\ \usepackage{graphicx}\ \makeatletter\def\maxwidth{\ifdim\Gin@nat@width>\linewidth\linewidth\else\Gin@nat@width\fi}\makeatother\ \setkeys{Gin}{width=\maxwidth,keepaspectratio}" \ From 358009efd04bb9ec69c59ca2219b720c2e382111 Mon Sep 17 00:00:00 2001 From: kartikey1306 Date: Tue, 15 Sep 2026 12:01:36 +0530 Subject: [PATCH 10/13] ci(book): let pandoc escape the author instead of passing it raw Run 3 got through the preamble and died on content: ! Misplaced alignment tab character &. Srikanth Patchava & EmbeddedOS Contributors The pandoc step passed -V "author=Srikanth Patchava & EmbeddedOS Contributors". A -V value is inserted into the LaTeX verbatim, so the '&' reached xelatex as an alignment tab. book.md's front matter already declares the same author, and metadata goes through pandoc's writer, which escapes it. The override is removed; the metadata is the one source. Earlier runs never reached this line -- they died in the preamble first, on \chead and on the font -- which is the same lesson as before: the first error hides every later one. Asserted on the parsed step: no -V author; every remaining literal -V value is free of unescaped LaTeX specials (& % # _ ^ ~); header-includes is exempt by design and values expanded at run time (title, version, date) come from metadata or `date`, which carry none. Still NOT RUN here. Fourth run is the evidence. --- .github/workflows/book-build.yml | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/.github/workflows/book-build.yml b/.github/workflows/book-build.yml index bd92bb4..53f4b16 100644 --- a/.github/workflows/book-build.yml +++ b/.github/workflows/book-build.yml @@ -177,6 +177,12 @@ jobs: # and scrlayer-scrpage's \newcommand failed: "Command \chead already # defined". Two header packages, one command. Every run since both # arrived in ef0dc50 died there, whatever the Eisvogel or pandoc version. + # + # No -V author=...: a -V value is inserted into the LaTeX verbatim, and + # the author string contains '&', which LaTeX reads as an alignment tab + # ("Misplaced alignment tab character &", run 3 of #135). book.md's + # metadata already carries the same author, and metadata is escaped by + # pandoc. Any -V value must be LaTeX-safe; title/subtitle/date are. pandoc \ book.md \ @@ -196,7 +202,6 @@ jobs: -V page-background-color="ffffff" \ -V "title=${{ steps.meta.outputs.title }}" \ -V "subtitle=Version ${{ steps.meta.outputs.version }}" \ - -V "author=Srikanth Patchava & EmbeddedOS Contributors" \ -V "date=$(date +'%B %Y')" \ -V toc=true \ -V toc-depth=3 \ From abf05228ff13a701c0bbc1df0348f83880d0be3a Mon Sep 17 00:00:00 2001 From: kartikey1306 Date: Tue, 15 Sep 2026 12:06:56 +0530 Subject: [PATCH 11/13] ci(book): name the runner correctly in the font comment The comment said the missing sourcesans.sty was ubuntu-22.04's TeX Live 2021. The job runs on ubuntu-latest, which the green run at 358009e shows is ubuntu-24.04 (noble). The fact that matters -- the runner's texlive-fonts-extra has no sourcesans.sty, so Eisvogel 3.5.x cannot be used -- is unchanged and is what run 1 proved; the distro and TeX Live version I attached to it were assumed, not read from the log. --- .github/workflows/book-build.yml | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/.github/workflows/book-build.yml b/.github/workflows/book-build.yml index 53f4b16..52773b1 100644 --- a/.github/workflows/book-build.yml +++ b/.github/workflows/book-build.yml @@ -29,11 +29,12 @@ permissions: env: PANDOC_VERSION: "3.6.1" # 3.4.0, not the newest: 3.5.0 (2026-06-28) migrated the default font from - # the `sourcesanspro` package to `sourcesans`, which ubuntu-22.04's TeX Live - # 2021 does not ship, so 3.5.x fails with "File `sourcesans.sty' not found" - # -- seen on the first run of this change. 3.4.0 is the last release that - # uses sourcesanspro (in texlive-fonts-extra, installed below) and already - # carries pandoc's default template merged through 3.9. + # the `sourcesanspro` package to `sourcesans`, which the runner's + # texlive-fonts-extra (ubuntu-latest, 24.04 at the time of writing) does + # not ship, so 3.5.x fails with "File `sourcesans.sty' not found" -- seen + # on the first run of this change. 3.4.0 is the last release that uses + # sourcesanspro (present in texlive-fonts-extra, installed below) and + # already carries pandoc's default template merged through 3.9. EISVOGEL_VERSION: "3.4.0" jobs: From 07ccb20c4ac2683f1bc81bb369425fc87dde5c51 Mon Sep 17 00:00:00 2001 From: kartikey1306 Date: Tue, 15 Sep 2026 02:46:41 +0530 Subject: [PATCH 12/13] build: drop EBLDR_REQUIRE_SIGNATURES and EBLDR_RECOVERY_AUTH, switches wired to nothing CMakeLists.txt offered both as options (default ON), forwarded each as a compile definition, and the README listed both under "Security options". No source file tests either macro: a configure with one of them OFF produces byte-for-byte the same objects as ON (41 of 41 non-test object files identical with the two definitions forced back in through CMAKE_C_FLAGS). Signature verification and recovery authentication are unconditional, and a switch that documents otherwise is a capability the tree does not have, presented as configuration. Remove the two options and their add_compile_definitions. A configure that still passes either as OFF now stops with a message saying there is no such build, instead of silently producing the verifying one. The README says the two are not options. docs/book/book.md's option table listed EBLDR_SECURE_BOOT, EBLDR_MULTICORE, EBLDR_RECOVERY and EBLDR_BOOT_MENU, none of which has ever existed in CMakeLists.txt; it now lists the real ones. tests/unit/test_build_options_are_read.py pins both rules: every EBLDR_ name CMakeLists.txt forwards as a compile definition is referenced by a preprocessor line in some C or header file, and every EBLDR_ name a documentation table presents as an option is declared. Against the base tree it fails on exactly the two dead options and the four phantom book entries. tests/simulate_tests.py, which nothing runs, asserted the option names were present in CMakeLists.txt; it now asserts the honest opposite, and its failure set is unchanged (9 before, 9 after, all unrelated). --- CHANGELOG.md | 1 + CMakeLists.txt | 23 ++++-- README.md | 9 ++- docs/book/book.md | 17 ++-- tests/simulate_tests.py | 9 ++- tests/unit/test_build_options_are_read.py | 94 +++++++++++++++++++++++ 6 files changed, 135 insertions(+), 18 deletions(-) create mode 100644 tests/unit/test_build_options_are_read.py diff --git a/CHANGELOG.md b/CHANGELOG.md index ad06fbb..8ffb76d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -29,6 +29,7 @@ - **`tests/unit/test_slot_manager.c`:** the file was two different test files spliced together mid-function -- stub definitions cut in half, `main()` calling twenty functions that do not exist. Rebuilt as one suite that exercises the real `core/slot_manager.c` through scriptable per-slot verification mocks. - **`tests/unit/test_recovery.c`:** local stand-ins for the boot-log API conflicted with `include/eos_boot_log.h` and duplicated symbols now linked from `core/boot_log.c`. Removed. - **`tests/CMakeLists.txt`:** `unit/test_fw_transport.c` existed but was never built or run. It is now registered. +- **`EBLDR_REQUIRE_SIGNATURES` and `EBLDR_RECOVERY_AUTH` were switches wired to nothing.** `CMakeLists.txt` offered both as options, forwarded each as a compile definition, and the README listed both as `ON` by default -- but no source file tested either macro, so a configure with one `OFF` produced byte-for-byte the same objects as `ON` (41 of 41 non-test objects identical with the definitions forced back in). Signature verification and recovery authentication are unconditional; the two options are gone, passing either as `OFF` fails the configure, the README says so, and `docs/book/book.md` no longer lists `EBLDR_SECURE_BOOT`, `EBLDR_MULTICORE`, `EBLDR_RECOVERY` and `EBLDR_BOOT_MENU`, none of which ever existed. `tests/unit/test_build_options_are_read.py` requires every forwarded `EBLDR_` definition to be read by a source file and every documented option to be declared. ### Added - **`eos_crc32_checked()`** — CRC32 over a flash region that reports read failures through its return value. `eos_crc32()` is retained for API compatibility and documented as unsuitable for verification decisions. diff --git a/CMakeLists.txt b/CMakeLists.txt index f8fe6eb..69fef52 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -21,8 +21,6 @@ stm32f4 | stm32h7 | nrf52 | rpi4 | riscv64_virt | esp32 | x86_64_efi | \ imx8m | am64x | samd51 | sifive_u | cortex_r5 | none") option(EBLDR_BUILD_TESTS "Build unit tests (native only)" OFF) -option(EBLDR_REQUIRE_SIGNATURES "Require Ed25519 signatures for boot" ON) -option(EBLDR_RECOVERY_AUTH "Require authentication for recovery commands" ON) option(EBLDR_VERIFY_STAGE1 "Verify Stage-1 hash before jump" ON) option(EBLDR_HARDENING "Enable compiler hardening flags" ON) option(EBLDR_SANITIZE "Enable ASAN/UBSAN for host builds" OFF) @@ -54,16 +52,25 @@ if(EBLDR_SANITIZE AND NOT CMAKE_CROSSCOMPILING AND CMAKE_C_COMPILER_ID MATCHES " endif() # Security compile definitions -if(EBLDR_REQUIRE_SIGNATURES) - add_compile_definitions(EBLDR_REQUIRE_SIGNATURES) -endif() -if(EBLDR_RECOVERY_AUTH) - add_compile_definitions(EBLDR_RECOVERY_AUTH) -endif() if(EBLDR_VERIFY_STAGE1) add_compile_definitions(EBLDR_VERIFY_STAGE1) endif() +# Signature verification and recovery authentication have no build-time +# switch. EBLDR_REQUIRE_SIGNATURES and EBLDR_RECOVERY_AUTH used to be offered +# as options, but no source file read the definitions they produced, so a +# build with either OFF was byte-for-byte the build with it ON. A script that +# still passes one as OFF is asking for something this bootloader does not +# do; say so instead of configuring as if it had been granted. +foreach(_dead_switch EBLDR_REQUIRE_SIGNATURES EBLDR_RECOVERY_AUTH) + if(DEFINED ${_dead_switch} AND NOT ${_dead_switch}) + message(FATAL_ERROR + "${_dead_switch}=OFF: there is no build that skips signature " + "verification or recovery authentication. The option was never " + "read by any source file and has been removed; drop the flag.") + endif() +endforeach() + # ==================================================================== # Core Libraries — platform-agnostic, always built # ==================================================================== diff --git a/README.md b/README.md index 1e0c79b..e8c850c 100644 --- a/README.md +++ b/README.md @@ -64,14 +64,19 @@ others (default `none` = native core-only build). | Option | Default | Meaning | |--------|---------|---------| -| `EBLDR_REQUIRE_SIGNATURES` | `ON` | Require Ed25519 signatures for boot | | `EBLDR_VERIFY_STAGE1` | `ON` | Verify the Stage-1 hash before jumping | -| `EBLDR_RECOVERY_AUTH` | `ON` | Require authentication for recovery commands | | `EBLDR_HARDENING` | `ON` | Compiler hardening (`-fstack-protector-strong`, `_FORTIFY_SOURCE=2`) | | `EBLDR_SANITIZE` | `OFF` | ASan/UBSan for host builds | | `EBLDR_BUILD_FUZZ` | `OFF` | Build libFuzzer targets | | `EBLDR_BUILD_TESTS` | `OFF` | Build unit tests (native only); set to `ON` to enable | +Ed25519 signature verification and recovery-command authentication are not +options: no build of this bootloader skips either. `EBLDR_REQUIRE_SIGNATURES` +and `EBLDR_RECOVERY_AUTH` used to be listed here as `ON` by default, but no +source file read the definitions they produced, so `OFF` built the same +firmware as `ON`. Both have been removed; passing either as `OFF` now fails +the configure. + ## Test ```bash diff --git a/docs/book/book.md b/docs/book/book.md index 125fe73..62b6201 100644 --- a/docs/book/book.md +++ b/docs/book/book.md @@ -1565,14 +1565,19 @@ eos_runtime_get_time(&year, &month, &day, &hour, &min, &sec); | Option | Default | Description | |---|---|---| -| EBLDR_BOARD | none | Target board name | -| EBLDR_BUILD_TESTS | OFF | Build unit tests | -| EBLDR_SECURE_BOOT | OFF | Enable secure boot | -| EBLDR_MULTICORE | OFF | Enable multicore | -| EBLDR_RECOVERY | ON | Enable recovery mode | -| EBLDR_BOOT_MENU | ON | Enable boot menu | +| EBLDR_BOARD | none | Target board name (`none` builds the native core only) | +| EBLDR_BUILD_TESTS | OFF | Build unit tests (native only) | +| EBLDR_VERIFY_STAGE1 | ON | Verify the Stage-1 hash before jumping | +| EBLDR_HARDENING | ON | Compiler hardening flags | +| EBLDR_SANITIZE | OFF | ASan/UBSan for host builds | +| EBLDR_BUILD_FUZZ | OFF | Build libFuzzer targets | | CMAKE_TOOLCHAIN_FILE | -- | Toolchain file | +Secure boot and recovery-command authentication are not options: every build +verifies signatures and authenticates recovery commands. Earlier editions of +this table listed `EBLDR_SECURE_BOOT`, `EBLDR_MULTICORE`, `EBLDR_RECOVERY` and +`EBLDR_BOOT_MENU`; none of those has ever existed in `CMakeLists.txt`. + --- ## Chapter 25: eFlash -- Unified Flashing Tool diff --git a/tests/simulate_tests.py b/tests/simulate_tests.py index 92e33e5..0b5186d 100644 --- a/tests/simulate_tests.py +++ b/tests/simulate_tests.py @@ -169,8 +169,13 @@ def test(name, condition, detail=""): with open(os.path.join(repo, "CMakeLists.txt")) as f: cmake = f.read() -test("Ed25519 signature requirement enabled by default", "EBLDR_REQUIRE_SIGNATURES" in cmake) -test("Recovery auth enabled by default", "EBLDR_RECOVERY_AUTH" in cmake) +# These two used to assert that the option names appeared in CMakeLists.txt. +# They did -- as options no source file read, so the assertion held while +# the switch did nothing. The honest claim is the opposite one. +test("No build option offers to skip signature verification", + not re.search(r"option\(\s*EBLDR_REQUIRE_SIGNATURES", cmake)) +test("No build option offers to skip recovery authentication", + not re.search(r"option\(\s*EBLDR_RECOVERY_AUTH", cmake)) test("Stage1 hash verification enabled by default", "EBLDR_VERIFY_STAGE1" in cmake) test("Stack protector hardening flag present", "fstack-protector-strong" in cmake) test("FORTIFY_SOURCE=2 present", "_FORTIFY_SOURCE=2" in cmake) diff --git a/tests/unit/test_build_options_are_read.py b/tests/unit/test_build_options_are_read.py new file mode 100644 index 0000000..1eb213a --- /dev/null +++ b/tests/unit/test_build_options_are_read.py @@ -0,0 +1,94 @@ +# SPDX-License-Identifier: MIT +# Copyright (c) 2026 EoS Project + +"""A build option that reaches the compiler must be read by something. + +CMakeLists.txt offered EBLDR_REQUIRE_SIGNATURES and EBLDR_RECOVERY_AUTH as +options, forwarded each as a compile definition, and the README listed both +as ON by default. No source file tested either macro, so a build with the +option OFF was byte-for-byte the build with it ON: the switch documented a +capability the tree did not have. docs/book/book.md went further and listed +four options that were never in CMakeLists.txt at all. + +Two source-level rules keep that from coming back: + +1. Every EBLDR_* name that CMakeLists.txt turns into a compile definition is + referenced by a preprocessor line in at least one C or header file. +2. Every EBLDR_* name a documentation table presents as a build option is + declared by CMakeLists.txt (option() or a CACHE set()). +""" + +import re +from pathlib import Path + +REPO_ROOT = Path(__file__).resolve().parents[2] +CMAKE = (REPO_ROOT / "CMakeLists.txt").read_text(encoding="utf-8") + +SOURCE_DIRS = ("core", "hal", "include", "stage0", "stage1", "boards", "tests") +DOC_TABLES = ("README.md", "docs/book/book.md") + + +def _strip_cmake_comments(text): + return "\n".join(line.split("#", 1)[0] for line in text.splitlines()) + + +def _declared_options(): + text = _strip_cmake_comments(CMAKE) + names = set(re.findall(r"\boption\(\s*(EBLDR_[A-Z0-9_]+)", text)) + names |= set(re.findall(r"\bset\(\s*(EBLDR_[A-Z0-9_]+)\b[^)]*\bCACHE\b", text)) + return names + + +def _forwarded_definitions(): + text = _strip_cmake_comments(CMAKE) + return set(re.findall(r"add_compile_definitions\(\s*(EBLDR_[A-Z0-9_]+)\s*\)", text)) + + +def _preprocessor_references(): + refs = set() + for d in SOURCE_DIRS: + for path in (REPO_ROOT / d).rglob("*"): + if path.suffix not in (".c", ".h"): + continue + for line in path.read_text(encoding="utf-8", errors="replace").splitlines(): + if line.lstrip().startswith("#"): + refs.update(re.findall(r"\bEBLDR_[A-Z0-9_]+\b", line)) + return refs + + +def _documented_options(rel): + names = set() + for line in (REPO_ROOT / rel).read_text(encoding="utf-8").splitlines(): + if line.startswith("|"): + first_cell = line.split("|")[1] + names.update(re.findall(r"\bEBLDR_[A-Z0-9_]+\b", first_cell)) + return names + + +def test_cmake_declares_the_options_this_guard_relies_on(): + declared = _declared_options() + assert {"EBLDR_BOARD", "EBLDR_BUILD_TESTS", "EBLDR_VERIFY_STAGE1"} <= declared, declared + + +def test_every_forwarded_definition_is_read_by_a_source_file(): + unread = sorted(_forwarded_definitions() - _preprocessor_references()) + assert not unread, ( + "CMakeLists.txt forwards these as compile definitions, but no C or " + "header file under %s tests them, so the option changes nothing: %s" + % (", ".join(SOURCE_DIRS), unread)) + + +def test_documented_options_exist(): + declared = _declared_options() + for rel in DOC_TABLES: + phantom = sorted(_documented_options(rel) - declared) + assert not phantom, "%s lists build options CMakeLists.txt does not declare: %s" % (rel, phantom) + + +def test_no_option_offers_to_skip_verification_or_authentication(): + # These were removed because nothing read them. Re-adding one as a real + # switch would add a way to build a bootloader that does not verify, which + # is a design change and not a configuration. + declared = _declared_options() + assert "EBLDR_REQUIRE_SIGNATURES" not in declared + assert "EBLDR_RECOVERY_AUTH" not in declared From f9fa26c4f409479d0bb06a667ae0d8f113725e0b Mon Sep 17 00:00:00 2001 From: kartikey1306 Date: Tue, 15 Sep 2026 12:03:50 +0530 Subject: [PATCH 13/13] test(build): the dead-option guard sees every way a definition is forwarded Finding 1 from the review of 88cb92e. _forwarded_definitions() matched add_compile_definitions(EBLDR_X) with the closing paren required to follow the name, so of the three ordinary ways to forward a compile definition it saw one. The reviewer showed the other two carrying a name nothing reads straight past it: add_compile_definitions(EBLDR_DEAD_A=1) -> 4 passed target_compile_definitions(eboot_core PRIVATE EBLDR_DEAD_B) -> 4 passed add_compile_definitions(EBLDR_DEAD_C) -> 1 failed The =1 form is what someone writes when they want the macro to have a value; target_compile_definitions is what they write when they want it scoped to one library. Neither is exotic, and this guard exists only for the future -- the tree forwards exactly one EBLDR_ name today and it is read. The collector now takes the whole argument list of add_ or target_compile_definitions(...), every EBLDR_ token in it, with anything after = stripped. Five shapes are pinned as collected, including a multi-line call and a mixed list, and a second test appends each shape to the real CMakeLists.txt text with a dead name and requires the guard to fail on it -- which is exactly how a dead switch would come back. All three of the reviewer's probes now fail the guard. The secondary note is taken too: test_documented_options_exist checked documented <= declared; the reverse direction is now asserted as well, with EBLDR_BOARD exempted because the README covers it in prose above the table rather than in it. Clean today, as the reviewer enumerated. Negative control: with the old one-shape collector restored and the new tests kept, 5 of 11 fail. At this head (restacked onto #115's e152d8e, itself on master 682d005): pytest tests/ 93 passed, ctest 31/31. --- tests/unit/test_build_options_are_read.py | 60 +++++++++++++++++++++-- 1 file changed, 57 insertions(+), 3 deletions(-) diff --git a/tests/unit/test_build_options_are_read.py b/tests/unit/test_build_options_are_read.py index 1eb213a..3c2bd4f 100644 --- a/tests/unit/test_build_options_are_read.py +++ b/tests/unit/test_build_options_are_read.py @@ -19,6 +19,8 @@ """ import re + +import pytest from pathlib import Path REPO_ROOT = Path(__file__).resolve().parents[2] @@ -39,9 +41,20 @@ def _declared_options(): return names -def _forwarded_definitions(): - text = _strip_cmake_comments(CMAKE) - return set(re.findall(r"add_compile_definitions\(\s*(EBLDR_[A-Z0-9_]+)\s*\)", text)) +def _forwarded_definitions(text=None): + """Every EBLDR_ name CMakeLists.txt forwards to the compiler, by any of the + three ordinary spellings: add_compile_definitions(EBLDR_X), + add_compile_definitions(EBLDR_X=1), and target_compile_definitions( + EBLDR_X). The first version of this matched only the first shape, + with the closing paren required to follow the name, so the other two + re-introduced a dead switch without the guard noticing.""" + if text is None: + text = _strip_cmake_comments(CMAKE) + names = set() + for args in re.findall(r"(?:add|target)_compile_definitions\(([^)]*)\)", text): + for tok in re.findall(r"\bEBLDR_[A-Z0-9_]+(?:=[^\s)]*)?", args): + names.add(tok.split("=", 1)[0]) + return names def _preprocessor_references(): @@ -78,6 +91,47 @@ def test_every_forwarded_definition_is_read_by_a_source_file(): % (", ".join(SOURCE_DIRS), unread)) +# The three shapes a compile definition is forwarded in. The guard once matched +# only the first; the reviewer demonstrated the other two passing it with a dead +# name in place. Each is asserted to be collected, with the =value stripped. +FORWARD_SHAPES = [ + ("add_compile_definitions(EBLDR_DEAD_A)", "EBLDR_DEAD_A"), + ("add_compile_definitions(EBLDR_DEAD_B=1)", "EBLDR_DEAD_B"), + ("target_compile_definitions(eboot_core PRIVATE EBLDR_DEAD_C)", "EBLDR_DEAD_C"), + ("target_compile_definitions(eboot_core PUBLIC EBLDR_DEAD_D=0 OTHER=1)", "EBLDR_DEAD_D"), + ("add_compile_definitions(\n EBLDR_DEAD_E=1\n EBLDR_DEAD_F\n)", "EBLDR_DEAD_E"), +] + + +@pytest.mark.parametrize("snippet, name", FORWARD_SHAPES, ids=[s[1] for s in FORWARD_SHAPES]) +def test_every_way_of_forwarding_a_definition_is_seen(snippet, name): + assert name in _forwarded_definitions(snippet) + + +def test_a_forwarded_definition_nothing_reads_is_caught_in_every_shape(): + """The guard is only worth having if each shape, with a name no source + references, fails it. Run against the real tree plus one appended line, + which is exactly how a dead switch would come back.""" + real = _strip_cmake_comments(CMAKE) + refs = _preprocessor_references() + for snippet, name in FORWARD_SHAPES: + unread = _forwarded_definitions(real + "\n" + snippet) - refs + assert name in unread, f"{snippet!r} forwarded a name nothing reads and the guard missed it" + + +def test_every_declared_option_is_documented(): + """test_documented_options_exist checks documented <= declared. This is the + other direction: an option that exists and is documented nowhere is a + switch a user cannot know about. Clean today, enumerated by the reviewer.""" + declared = _declared_options() + documented = set() + for rel in DOC_TABLES: + documented |= _documented_options(rel) + # EBLDR_BOARD is covered in README prose above the table, not the table itself. + undocumented = sorted(declared - documented - {"EBLDR_BOARD"}) + assert not undocumented, "declared in CMakeLists.txt but in no options table: %s" % undocumented + + def test_documented_options_exist(): declared = _declared_options() for rel in DOC_TABLES: