From 35280b5297deb5bbfed90b9db0d234db058be049 Mon Sep 17 00:00:00 2001 From: kartikey1306 Date: Fri, 11 Sep 2026 16:13:57 +0530 Subject: [PATCH 1/9] 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 2/9] 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 3/9] 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 4/9] 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 5/9] 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 6/9] 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 1de05d6f5dc6f522b30e123e82e395c0c7c6f1c5 Mon Sep 17 00:00:00 2001 From: kartikey1306 Date: Tue, 15 Sep 2026 02:25:28 +0530 Subject: [PATCH 7/9] fix(stage0): refuse a stage-1 image the flash could not read The stage-1 hash loop in stage0/jump_stage1.c called eos_hal_flash_read() and hashed the buffer whatever the call returned. A read that failed left the buffer holding the previous chunk, or whatever the stack held, and that was hashed as if it were stage-1. The mismatch that followed sent the device to recovery, so the outcome was fail-closed -- by accident, and logged as 0xBAD1, a hash mismatch, which is not what happened. core/crypto_boot.c's eos_crypto_verify_image() does the same job and refuses a failed read; stage-0 now does too, entering recovery with its own reason (0xBAD2) and returning so the loop can never fall through to the jump. stage0/ is only compiled by a cross build, so the test is a source-level guard in the style of test_stage0_reset_entry.py: every call to a HAL read or write in stage0/*.c must assign or test its result, and the stage-1 loop in particular must test the read, enter recovery on failure with the 0xBAD2 reason, and return. Against the unfixed file both tests fail. --- CHANGELOG.md | 1 + stage0/jump_stage1.c | 12 +++- tests/unit/test_stage0_hal_results.py | 79 +++++++++++++++++++++++++++ 3 files changed, 91 insertions(+), 1 deletion(-) create mode 100644 tests/unit/test_stage0_hal_results.py diff --git a/CHANGELOG.md b/CHANGELOG.md index ad06fbb..2af88fe 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ - **`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 +- **Stage-0 refuses a stage-1 image it could not read.** The stage-1 hash loop in `stage0/jump_stage1.c` discarded `eos_hal_flash_read()`'s result, so a failed read hashed the previous chunk (or the stack) as if it were stage-1; the mismatch that followed reached recovery with the wrong reason (`0xBAD1`, hash mismatch), and only by luck. A failed read now enters recovery with its own reason (`0xBAD2`) and never falls through to the jump, matching what `eos_crypto_verify_image()` in `core/` already does. `tests/unit/test_stage0_hal_results.py` is a source-level guard that every HAL read or write in `stage0/` examines its result. - **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, 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. diff --git a/stage0/jump_stage1.c b/stage0/jump_stage1.c index dcbe645..8e66bad 100644 --- a/stage0/jump_stage1.c +++ b/stage0/jump_stage1.c @@ -76,7 +76,17 @@ void ebldr_stage0_main(void) while (off < stage1_expected_size) { uint32_t chunk = stage1_expected_size - off; if (chunk > sizeof(buf)) chunk = sizeof(buf); - eos_hal_flash_read(stage1_addr + off, buf, chunk); + /* A read that fails leaves buf holding the previous chunk, or + * whatever the stack held, and the result was hashed as if it + * were stage-1. That happened to mismatch, so the failure mode + * was recovery with the wrong reason; a read failure is its + * own reason and is refused as such, like eos_crypto_verify_image() + * refuses it in core. */ + if (eos_hal_flash_read(stage1_addr + off, buf, chunk) != EOS_OK) { + eos_boot_log_append(EOS_LOG_BOOT_FAIL, EOS_SLOT_NONE, 0xBAD2); + eos_recovery_enter(&bctl); + return; + } eos_sha256_update(&sha_ctx, buf, chunk); off += chunk; } diff --git a/tests/unit/test_stage0_hal_results.py b/tests/unit/test_stage0_hal_results.py new file mode 100644 index 0000000..f796af1 --- /dev/null +++ b/tests/unit/test_stage0_hal_results.py @@ -0,0 +1,79 @@ +# SPDX-License-Identifier: MIT +# Copyright (c) 2026 EoS Project + +"""Every HAL read in stage-0 checks its result. + +stage0/jump_stage1.c hashed stage-1 with a loop that discarded the return +value of eos_hal_flash_read(): a read that failed left the buffer holding +the previous chunk, or whatever the stack held, and that was hashed as if +it were stage-1. The mismatch that followed sent the device to recovery +with the wrong reason (0xBAD1, "hash mismatch") -- and only by luck; the +loop in core/crypto_boot.c that does the same job refuses a failed read. + +stage0/ is only compiled by a cross build, so this is a source-level guard +in the style of test_stage0_reset_entry.py: every call to a HAL read or +write function in stage0/*.c must have its result examined -- assigned, or +tested in the same statement. +""" + +import re +from pathlib import Path + +REPO_ROOT = Path(__file__).resolve().parents[2] +STAGE0 = sorted((REPO_ROOT / "stage0").glob("*.c")) + +# HAL calls whose failure means the bytes are not what they claim to be. +CHECKED_CALLS = ("eos_hal_flash_read", "eos_hal_flash_write", "eos_hal_flash_erase", + "eos_hal_otp_read", "eos_hal_otp_write", "eos_hal_monotonic_read") + + +def _strip_comments(text): + text = re.sub(r"/\*.*?\*/", " ", text, flags=re.DOTALL) + return re.sub(r"//[^\n]*", " ", text) + + +def _statements_calling(text, name): + """Each statement (up to its ';') that contains a call to *name*.""" + out = [] + for m in re.finditer(re.escape(name) + r"\s*\(", text): + start = max(text.rfind(";", 0, m.start()), text.rfind("{", 0, m.start())) + end = text.find(";", m.end()) + out.append(text[start + 1:end].strip()) + return out + + +def _result_is_examined(statement, name): + """True when the call's value is assigned or tested, not discarded.""" + head = statement[:statement.index(name)] + return "=" in head or "if" in head or "return" in head or "while" in head + + +def test_stage0_files_exist(): + assert STAGE0, "no stage0/*.c found" + assert any(p.name == "jump_stage1.c" for p in STAGE0) + + +def test_every_hal_read_and_write_in_stage0_examines_its_result(): + discarded = [] + for path in STAGE0: + text = _strip_comments(path.read_text(encoding="utf-8")) + for name in CHECKED_CALLS: + for stmt in _statements_calling(text, name): + if not _result_is_examined(stmt, name): + discarded.append(f"{path.name}: {stmt}") + assert not discarded, ( + "these stage-0 statements discard a HAL result, so a failed read or " + "write is treated as data:\n " + "\n ".join(discarded)) + + +def test_the_stage1_hash_loop_refuses_a_failed_read(): + """The specific regression: a failed read in the stage-1 hash loop must + leave the loop for recovery, with a reason distinct from a bad hash.""" + text = _strip_comments((REPO_ROOT / "stage0" / "jump_stage1.c").read_text(encoding="utf-8")) + loop = text[text.index("while (off < stage1_expected_size)"):] + loop = loop[:loop.index("eos_sha256_final")] + assert re.search(r"if\s*\(\s*eos_hal_flash_read\([^;]*\)\s*!=\s*EOS_OK\s*\)", loop), \ + "the stage-1 hash loop does not test eos_hal_flash_read()'s result" + assert "eos_recovery_enter" in loop, "a failed read must enter recovery" + assert "0xBAD2" in loop, "a failed read must be logged with its own reason, not the hash-mismatch one" + assert "return;" in loop, "after recovery the loop must not fall through to the jump" From d68d5d1f379a4fbf60d8c2efa7210ad162224a69 Mon Sep 17 00:00:00 2001 From: kartikey1306 Date: Tue, 15 Sep 2026 03:16:11 +0530 Subject: [PATCH 8/9] fix(stage0): a failed stage-1 verification returns, and the detail codes have names The hash-mismatch path in ebldr_stage0_main() entered recovery and fell through: no return, so control reached the EOS_LOG_IMAGE_VALID record and ops->jump(). The only thing holding it back was eos_recovery_enter() never returning, which its own `int` return type does not promise. The recovery-trigger path eight lines earlier had the same shape. Both return now, and the positive record is written only when the hash matched. 0xBAD1 and 0xBAD2 were bare magic values defined in no header, so a boot log read in the field gave `event=BOOT_FAIL detail=0xBAD2` and no way to learn what that meant. EBLDR_FAIL_STAGE1_HASH and EBLDR_FAIL_STAGE1_READ sit beside the EOS_LOG_* codes in include/eos_types.h and jump_stage1.c uses them by name. tests/unit/test_stage0_hal_results.py had two demonstrated false negatives: `int rd = read(...); (void)rd;` passed because a bare `=` in the statement head counted as examined, and a discarded call placed after a `#ifdef` line passed because "#ifdef" contains "if". Preprocessor lines are now stripped with the comments, and a result counts as examined only when the call sits in an if/while condition or a return, or is assigned to a name that a later if/while/return in the same block reads. A new test asserts every eos_recovery_enter() in ebldr_stage0_main() is followed by a return (a closing brace counts only when it closes the function), that the mismatch block returns and carries no IMAGE_VALID, and that both bypass probes are refused while the two legitimate shapes are accepted. Against 4b42626 the fall-through test fails on the mismatch call. --- CHANGELOG.md | 2 +- include/eos_types.h | 5 ++ stage0/jump_stage1.c | 13 +++- tests/unit/test_stage0_hal_results.py | 105 +++++++++++++++++++++++--- 4 files changed, 111 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2af88fe..54d6f01 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,7 +13,7 @@ - **`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 -- **Stage-0 refuses a stage-1 image it could not read.** The stage-1 hash loop in `stage0/jump_stage1.c` discarded `eos_hal_flash_read()`'s result, so a failed read hashed the previous chunk (or the stack) as if it were stage-1; the mismatch that followed reached recovery with the wrong reason (`0xBAD1`, hash mismatch), and only by luck. A failed read now enters recovery with its own reason (`0xBAD2`) and never falls through to the jump, matching what `eos_crypto_verify_image()` in `core/` already does. `tests/unit/test_stage0_hal_results.py` is a source-level guard that every HAL read or write in `stage0/` examines its result. +- **Stage-0 refuses a stage-1 image it could not read.** The stage-1 hash loop in `stage0/jump_stage1.c` discarded `eos_hal_flash_read()`'s result, so a failed read hashed the previous chunk (or the stack) as if it were stage-1; the mismatch that followed reached recovery with the wrong reason (`0xBAD1`, hash mismatch), and only by luck. A failed read now enters recovery with its own reason (`EBLDR_FAIL_STAGE1_READ`, `0xBAD2`) and never falls through to the jump, matching what `eos_crypto_verify_image()` in `core/` already does. The hash-mismatch path (`EBLDR_FAIL_STAGE1_HASH`, `0xBAD1`) and the recovery-trigger path had the same fall-through -- `eos_recovery_enter()` with nothing after it, so a failed verification reached the `IMAGE_VALID` record and the jump, held back only by recovery never returning -- and both now return. The two detail codes are named in `include/eos_types.h` beside the `EOS_LOG_*` events. `tests/unit/test_stage0_hal_results.py` is a source-level guard that every HAL read or write in `stage0/` has its result tested (not merely assigned; preprocessor lines are ignored), and that every entry into recovery in `ebldr_stage0_main()` is followed by a return. - **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, 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. diff --git a/include/eos_types.h b/include/eos_types.h index a36a580..4b2f00f 100644 --- a/include/eos_types.h +++ b/include/eos_types.h @@ -150,6 +150,11 @@ typedef struct { #define EOS_HASH_SIZE 32 #define EOS_SIG_MAX_SIZE 64 +/* `detail` values stage-0 attaches to EOS_LOG_BOOT_FAIL when it refuses to + * jump to stage-1. Named here so a boot log can be read without the source. */ +#define EBLDR_FAIL_STAGE1_HASH 0xBAD1 /* stage-1 hash does not match the build-time value */ +#define EBLDR_FAIL_STAGE1_READ 0xBAD2 /* the flash read of stage-1 failed */ + /* ---------------- RTOS Boot Types (Phase 2) ---------------- */ typedef enum { diff --git a/stage0/jump_stage1.c b/stage0/jump_stage1.c index 8e66bad..fdb5aaa 100644 --- a/stage0/jump_stage1.c +++ b/stage0/jump_stage1.c @@ -52,7 +52,9 @@ void ebldr_stage0_main(void) /* Check for recovery triggers */ if (ebldr_recovery_triggered(&bctl)) { eos_recovery_enter(&bctl); - /* Does not return unless recovery instructs a reboot */ + /* Recovery does not return today; if it ever does, a requested + * recovery session must not fall through into a normal boot. */ + return; } ebldr_watchdog_feed(); @@ -83,7 +85,7 @@ void ebldr_stage0_main(void) * own reason and is refused as such, like eos_crypto_verify_image() * refuses it in core. */ if (eos_hal_flash_read(stage1_addr + off, buf, chunk) != EOS_OK) { - eos_boot_log_append(EOS_LOG_BOOT_FAIL, EOS_SLOT_NONE, 0xBAD2); + eos_boot_log_append(EOS_LOG_BOOT_FAIL, EOS_SLOT_NONE, EBLDR_FAIL_STAGE1_READ); eos_recovery_enter(&bctl); return; } @@ -101,9 +103,14 @@ void ebldr_stage0_main(void) if (computed[i] != stage1_expected_hash[i]) match2 = 1; } + /* A mismatch enters recovery and stops here. The only thing that + * used to keep it from the jump below was eos_recovery_enter() + * never returning -- an invariant its own `int` return type does + * not promise -- and IMAGE_VALID was appended on the way past. */ if (match1 || match2) { - eos_boot_log_append(EOS_LOG_BOOT_FAIL, EOS_SLOT_NONE, 0xBAD1); + eos_boot_log_append(EOS_LOG_BOOT_FAIL, EOS_SLOT_NONE, EBLDR_FAIL_STAGE1_HASH); eos_recovery_enter(&bctl); + return; } eos_boot_log_append(EOS_LOG_IMAGE_VALID, EOS_SLOT_NONE, 0); #endif diff --git a/tests/unit/test_stage0_hal_results.py b/tests/unit/test_stage0_hal_results.py index f796af1..239e07d 100644 --- a/tests/unit/test_stage0_hal_results.py +++ b/tests/unit/test_stage0_hal_results.py @@ -28,24 +28,57 @@ def _strip_comments(text): + """Drop comments and preprocessor lines, so neither can supply a token + the checks below look for (a `#ifdef` used to read as an `if`).""" text = re.sub(r"/\*.*?\*/", " ", text, flags=re.DOTALL) - return re.sub(r"//[^\n]*", " ", text) + text = re.sub(r"//[^\n]*", " ", text) + return re.sub(r"(?m)^\s*#[^\n]*", " ", text) def _statements_calling(text, name): - """Each statement (up to its ';') that contains a call to *name*.""" + """Each statement (up to its ';') that contains a call to *name*, with + the offset where that statement ends, for the look-ahead below.""" out = [] for m in re.finditer(re.escape(name) + r"\s*\(", text): - start = max(text.rfind(";", 0, m.start()), text.rfind("{", 0, m.start())) + start = max(text.rfind(";", 0, m.start()), text.rfind("{", 0, m.start()), + text.rfind("}", 0, m.start())) end = text.find(";", m.end()) - out.append(text[start + 1:end].strip()) + out.append((text[start + 1:end].strip(), end + 1)) return out -def _result_is_examined(statement, name): - """True when the call's value is assigned or tested, not discarded.""" +def _rest_of_enclosing_block(text, pos): + """Text from *pos* to the `}` that closes the block *pos* is in.""" + depth = 0 + for i in range(pos, len(text)): + if text[i] == "{": + depth += 1 + elif text[i] == "}": + if depth == 0: + return text[pos:i] + depth -= 1 + return text[pos:] + + +def _result_is_examined(statement, name, block_after): + """True when the call's value is tested, not merely received. + + Tested means one of: the call sits inside an `if`/`while` condition or + a `return`; or it is assigned to a name that a later `if`/`while` or + `return` in the same block reads. A bare assignment is not enough: an + `int rd = read(...); (void)rd;` discards the result as surely as no + assignment at all. + """ head = statement[:statement.index(name)] - return "=" in head or "if" in head or "return" in head or "while" in head + if re.match(r"(if|while)\s*\(", head) or re.match(r"return\b", head): + return True + assigned = re.match(r"(?:[A-Za-z_][\w\s\*]*\s)?([A-Za-z_]\w*)\s*=\s*$", head) + if not assigned: + return False + var = assigned.group(1) + tested = re.compile( + r"(?:\b(?:if|while)\s*\([^;{]*\b%s\b[^;{]*\))|(?:\breturn\b[^;]*\b%s\b)" % (var, var)) + return bool(tested.search(block_after)) def test_stage0_files_exist(): @@ -58,8 +91,8 @@ def test_every_hal_read_and_write_in_stage0_examines_its_result(): for path in STAGE0: text = _strip_comments(path.read_text(encoding="utf-8")) for name in CHECKED_CALLS: - for stmt in _statements_calling(text, name): - if not _result_is_examined(stmt, name): + for stmt, end in _statements_calling(text, name): + if not _result_is_examined(stmt, name, _rest_of_enclosing_block(text, end)): discarded.append(f"{path.name}: {stmt}") assert not discarded, ( "these stage-0 statements discard a HAL result, so a failed read or " @@ -75,5 +108,57 @@ def test_the_stage1_hash_loop_refuses_a_failed_read(): assert re.search(r"if\s*\(\s*eos_hal_flash_read\([^;]*\)\s*!=\s*EOS_OK\s*\)", loop), \ "the stage-1 hash loop does not test eos_hal_flash_read()'s result" assert "eos_recovery_enter" in loop, "a failed read must enter recovery" - assert "0xBAD2" in loop, "a failed read must be logged with its own reason, not the hash-mismatch one" + assert "EBLDR_FAIL_STAGE1_READ" in loop, "a failed read must be logged with its own reason, not the hash-mismatch one" assert "return;" in loop, "after recovery the loop must not fall through to the jump" + + +def test_every_recovery_entry_in_stage0_is_followed_by_return(): + """The mismatch path had the same fall-through: eos_recovery_enter() + with no return after it, so a hash that failed reached the IMAGE_VALID + record and the jump, held back only by recovery never returning -- an + invariant its `int` return type does not promise. Every entry into + recovery inside a function that goes on to jump must be followed by a + return, and a positive record must not sit on the failure path.""" + text = _strip_comments((REPO_ROOT / "stage0" / "jump_stage1.c").read_text(encoding="utf-8")) + body = text[text.index("void ebldr_stage0_main"):] + calls = [m.end() for m in re.finditer(r"eos_recovery_enter\s*\([^;]*\)\s*;", body)] + assert calls, "no eos_recovery_enter() call found in ebldr_stage0_main" + unreturned = [] + for end in calls: + following = body[end:].lstrip() + depth = body.count("{", 0, end) - body.count("}", 0, end) + closes_function = following.startswith("}") and depth == 1 + if not (following.startswith("return") or closes_function): + unreturned.append(body[end - 40:end + 40].strip()) + assert not unreturned, "eos_recovery_enter() falls through to what follows it:\n " + "\n ".join(unreturned) + + mismatch = body[body.index("if (match1 || match2)"):] + mismatch = mismatch[:mismatch.index("}") + 1] + assert "return;" in mismatch + assert "EOS_LOG_IMAGE_VALID" not in mismatch + assert "EBLDR_FAIL_STAGE1_HASH" in mismatch + + +def test_the_guard_rejects_an_assigned_but_untested_result(tmp_path): + """Two bypasses the review demonstrated against the earlier guard: a + result assigned and then cast to void, and a discarded call placed + right after a `#ifdef` line. Neither may pass.""" + assigned = "void f(void) {\n int rd = eos_hal_flash_read(0, 0, 0);\n (void)rd;\n}\n" + text = _strip_comments(assigned) + (stmt, end), = _statements_calling(text, "eos_hal_flash_read") + assert not _result_is_examined(stmt, "eos_hal_flash_read", _rest_of_enclosing_block(text, end)) + + ifdef = "void f(void) {\n#ifdef X\n eos_hal_flash_read(0, 0, 0);\n#endif\n}\n" + text = _strip_comments(ifdef) + (stmt, end), = _statements_calling(text, "eos_hal_flash_read") + assert not _result_is_examined(stmt, "eos_hal_flash_read", _rest_of_enclosing_block(text, end)) + + tested = "void f(void) {\n int rc = eos_hal_flash_read(0, 0, 0);\n if (rc != EOS_OK) return;\n}\n" + text = _strip_comments(tested) + (stmt, end), = _statements_calling(text, "eos_hal_flash_read") + assert _result_is_examined(stmt, "eos_hal_flash_read", _rest_of_enclosing_block(text, end)) + + inline = "void f(void) {\n if (eos_hal_flash_read(0, 0, 0) != EOS_OK) return;\n}\n" + text = _strip_comments(inline) + (stmt, end), = _statements_calling(text, "eos_hal_flash_read") + assert _result_is_examined(stmt, "eos_hal_flash_read", _rest_of_enclosing_block(text, end)) From e0184746fb9165c7f09d26ca0e0ec5fa1c4bdd6a Mon Sep 17 00:00:00 2001 From: kartikey1306 Date: Tue, 15 Sep 2026 12:30:34 +0530 Subject: [PATCH 9/9] test(stage0): a call as a one-line if/while body is not inside the condition The guard counted a statement as examined when its head started with "if (" or "while (". The head of `if (need) eos_hal_flash_read(...)` does, and that call is the body, its result discarded -- reproduced in review against the module's own helpers. The call is inside the condition only when the head has an unclosed parenthesis; that is the whole fix, and the two bypasses join the probe test beside the two from the previous round, with `if (ok && read(...) != EOS_OK)` as the control that a call deeper in a condition still counts. --- tests/unit/test_stage0_hal_results.py | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/tests/unit/test_stage0_hal_results.py b/tests/unit/test_stage0_hal_results.py index 239e07d..9d7fb93 100644 --- a/tests/unit/test_stage0_hal_results.py +++ b/tests/unit/test_stage0_hal_results.py @@ -70,7 +70,12 @@ def _result_is_examined(statement, name, block_after): assignment at all. """ head = statement[:statement.index(name)] - if re.match(r"(if|while)\s*\(", head) or re.match(r"return\b", head): + # Inside the condition's parentheses, not merely after an `if`: the head of + # `if (need) read(...)` also starts with "if (", and that call's result is + # discarded in the body. An unclosed "(" in the head is what says the call + # sits within the condition. + in_condition = re.match(r"(if|while)\s*\(", head) and head.count("(") > head.count(")") + if in_condition or re.match(r"return\b", head): return True assigned = re.match(r"(?:[A-Za-z_][\w\s\*]*\s)?([A-Za-z_]\w*)\s*=\s*$", head) if not assigned: @@ -153,6 +158,20 @@ def test_the_guard_rejects_an_assigned_but_untested_result(tmp_path): (stmt, end), = _statements_calling(text, "eos_hal_flash_read") assert not _result_is_examined(stmt, "eos_hal_flash_read", _rest_of_enclosing_block(text, end)) + # A call as a one-line if/while body: the statement starts with "if (" + # but the call is outside the condition and its result is discarded. + for body in ("void f(int need) {\n if (need) eos_hal_flash_read(0, 0, 0);\n}\n", + "void f(int n) {\n while (n--) eos_hal_flash_read(0, 0, 0);\n}\n"): + text = _strip_comments(body) + (stmt, end), = _statements_calling(text, "eos_hal_flash_read") + assert not _result_is_examined(stmt, "eos_hal_flash_read", _rest_of_enclosing_block(text, end)), stmt + + # A call deeper inside a condition is still inside it. + nested = "void f(int ok) {\n if (ok && eos_hal_flash_read(0, 0, 0) != EOS_OK) return;\n}\n" + text = _strip_comments(nested) + (stmt, end), = _statements_calling(text, "eos_hal_flash_read") + assert _result_is_examined(stmt, "eos_hal_flash_read", _rest_of_enclosing_block(text, end)) + tested = "void f(void) {\n int rc = eos_hal_flash_read(0, 0, 0);\n if (rc != EOS_OK) return;\n}\n" text = _strip_comments(tested) (stmt, end), = _statements_calling(text, "eos_hal_flash_read")