From 35280b5297deb5bbfed90b9db0d234db058be049 Mon Sep 17 00:00:00 2001 From: kartikey1306 Date: Fri, 11 Sep 2026 16:13:57 +0530 Subject: [PATCH 01/23] fix: repair master after the 09-07 batch merge Twenty PRs were merged into master in ninety minutes on 09-07, each on the base it was written against. Master has not configured, compiled, linked or passed its tests since. Every nightly and every eos simulation run (which builds eBoot master) has been red for the same reason. Configure: - tests/CMakeLists.txt registered eboot_test_fdt_loader twice. #84 added it, and #85 -- stacked on #84 -- re-added it at a different anchor when it was replayed onto a master that already had #84. Same hunk as #111. Compile (eboot_core): - #55 restored a core/sha512.c from 02b7dac that keeps its byte count in ctx->count; the eos_sha512_ctx_t master ships has bitlen[2]/buffer_len (#69/#93). Back to the pre-#55 file, blob-identical (d9aa57c). - #55's source-list "correction" dropped core/boot_log.c, core/secure_boot.c and core/fdt_loader.c, which #72/#84 had added after #55 was written. CMakeLists.txt is blob-identical to pre-#55 again (f8fe6eb). - #55 replaced the eos_boot_log_get_head() declaration with a second copy of eos_boot_log_read(); #91 had already fixed the prototype it meant to fix. Header restored (964ebb8). - #94 and #105 each repaired the Ed25519 verifier and each added an identical static scalarbase(); both merged. One copy removed. - The same pair each added k_low_order[]/messages[] to test_ed25519.c. The #105 copy is removed; #94's stays because it also carries k_non_canonical[]. Tests that stopped passing because two merged PRs disagree on behaviour: - #104 verifies the image signature at install unconditionally, before the anti-rollback check #103 added, so #103's unsigned images are refused as EOS_ERR_SIGNATURE before they can be refused as EOS_ERR_ANTI_ROLLBACK, and test_fw_transport's XMODEM install can no longer finalize. Both suites now stream genuinely signed images. eBoot has no Ed25519 signer in C, so tools/gen_fw_update_test_sigs.py signs the exact header prefixes those suites build under the RFC 8032 section 7.1 TEST 1 key and emits tests/vectors/fw_update_test_sigs.h; the suites serve that key from a simulated OTP slot 0. Negative control: one flipped signature byte fails test_write_streams_tlv_then_finalize_rejects_below_floor with EOS_ERR_SIGNATURE. - #103's step 5b reads the TLV counter through the HAL slot containing the image; test_secure_boot_policy (#82) declared no slots, so eos_secure_boot() returned EOS_SBOOT_ERR_BAD_HEADER two steps before the one under test. The fixture now places its image in slot A. Guards from #95 that later merges walked back, never run until now because the C configure step failed first: - #103 replayed the hand-written Valgrind foreach over the derived one. Restored foreach(TEST_NAME ${EBLDR_UNIT_TESTS}); eleven registered suites had no list(APPEND ...) and so no Valgrind run. - Seven suites assign tests_run = and their TEST() does not count; four suites have no TEST() macro at all. Counted, and classified. - #101 added fuzz-build after #90's gate; the gate did not wait for it. CI plumbing: - eosim-sanity.yml: the install-validate job is written in bash but ran under PowerShell on the Windows legs (no shell:), where SITE_PACKAGES=$(...) is an unknown command and `|| { exit 1 }` is an unexecuted script block. - scorecard.yml: ossf/scorecard-action@v2.4.0 pulls gcr.io, which now demands GCP billing. v2.4.3 pulls ghcr.io; eos already pins it and is green. Verified locally (macOS, clang): Release build clean, 31/31 ctest; the same under -DEBLDR_SANITIZE=ON (ASan+UBSan); 78/78 pytest. Not fixed here, reported separately: core/keystore.c's compiled-in default_dev_key is described as the RFC 8032 TEST 1 public key but differs from byte 21 on and is not a point on the curve, so nothing can verify against it on any board without OTP. With #104 that makes firmware update refuse every image on such boards. --- .github/workflows/ci.yml | 2 +- .github/workflows/eosim-sanity.yml | 8 + .github/workflows/scorecard.yml | 2 +- CMakeLists.txt | 5 +- core/ed25519_verify.c | 10 - core/sha512.c | 322 ++++++++++++++++++--------- include/eos_boot_log.h | 10 +- tests/CMakeLists.txt | 18 +- tests/unit/test_ed25519.c | 40 ---- tests/unit/test_fw_decrypt.c | 2 +- tests/unit/test_fw_transport.c | 27 ++- tests/unit/test_fw_update.c | 35 ++- tests/unit/test_fw_update_sig.c | 2 +- tests/unit/test_jump_app.c | 2 +- tests/unit/test_jump_app_bounds.c | 2 +- tests/unit/test_secure_boot_policy.c | 5 + tests/unit/test_suite_bookkeeping.py | 8 + tests/unit/test_tlv_auth.c | 1 - tests/vectors/fw_update_test_sigs.h | 43 ++++ tools/gen_fw_update_test_sigs.py | 156 +++++++++++++ 20 files changed, 515 insertions(+), 185 deletions(-) create mode 100644 tests/vectors/fw_update_test_sigs.h create mode 100644 tools/gen_fw_update_test_sigs.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 71ae0b7..24ae902 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -250,7 +250,7 @@ jobs: ci-gate: name: CI Gate runs-on: ubuntu-22.04 - needs: [test, build-arm, static-analysis] + needs: [test, build-arm, fuzz-build, static-analysis] if: always() steps: - uses: actions/checkout@v4 diff --git a/.github/workflows/eosim-sanity.yml b/.github/workflows/eosim-sanity.yml index e323860..9c6e3b4 100644 --- a/.github/workflows/eosim-sanity.yml +++ b/.github/workflows/eosim-sanity.yml @@ -23,6 +23,14 @@ jobs: install-validate: name: Install & Validate (${{ matrix.os }}, Python ${{ matrix.python-version }}) runs-on: ${{ matrix.os }} + # Every step here is written in bash: `$(...)`, `|| { ... }`, a heredoc, + # and /tmp. Without this the Windows legs ran them under PowerShell, where + # `SITE_PACKAGES=$(...)` is an unknown command and the job went red, while + # the `|| { exit 1 }` guard in "Verify installation" parsed as an unexecuted + # script block and could never fail. + defaults: + run: + shell: bash strategy: fail-fast: false matrix: diff --git a/.github/workflows/scorecard.yml b/.github/workflows/scorecard.yml index e8507e5..4a0033e 100644 --- a/.github/workflows/scorecard.yml +++ b/.github/workflows/scorecard.yml @@ -24,7 +24,7 @@ jobs: - uses: actions/checkout@v4 with: persist-credentials: false - - uses: ossf/scorecard-action@v2.4.0 + - uses: ossf/scorecard-action@v2.4.3 with: results_file: results.sarif results_format: sarif diff --git a/CMakeLists.txt b/CMakeLists.txt index 728e028..f8fe6eb 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -80,6 +80,7 @@ target_include_directories(eboot_hal PUBLIC ${EBLDR_INCLUDE_DIR}) # ---- Core boot logic ---- add_library(eboot_core STATIC core/bootctl.c + core/boot_log.c core/image_verify.c core/slot_manager.c core/boot_policy.c @@ -106,8 +107,10 @@ add_library(eboot_core STATIC core/os_adapter.c core/ed25519_verify.c core/sha512.c - core/keystore.c + core/secure_boot.c + core/fdt_loader.c core/rollback.c + core/keystore.c core/debug_lock.c core/fw_decrypt.c core/image_tlv.c diff --git a/core/ed25519_verify.c b/core/ed25519_verify.c index 7d230fd..3e9e4b1 100644 --- a/core/ed25519_verify.c +++ b/core/ed25519_verify.c @@ -300,16 +300,6 @@ static int point_is_identity(gf p[4]) return diff == 0; } -static void scalarbase(gf r[4], const uint8_t *s) -{ - gf q[4]; - fe_copy16(q[0], BX); - fe_copy16(q[1], BY); - fe_copy16(q[2], gf1); - fe_mul(q[3], BX, BY); - scalarmult(r, q, s); -} - /* Reject a public key outside the prime-order subgroup. * * Decoding a point is not enough. Ed25519 has eight points of low order, and diff --git a/core/sha512.c b/core/sha512.c index 4a1ecb5..d9aa57c 100644 --- a/core/sha512.c +++ b/core/sha512.c @@ -1,147 +1,247 @@ // SPDX-License-Identifier: MIT // Copyright (c) 2026 EoS Project -// ISO/IEC 25000 | ISO/IEC/IEEE 15288:2023 - -/** - * @file sha512.c - * @brief SHA-512 (NIST FIPS 180-4) — required by Ed25519 (RFC 8032) - * - * Ed25519 as specified in RFC 8032 derives its challenge scalar from - * SHA-512. A verifier using any other hash cannot check a signature made - * by a conforming signer, so this primitive is not optional for - * interoperability with standard tooling. - * - * Self-contained, no dynamic allocation, suitable for a bootloader. - */ #include "eos_crypto_boot.h" #include -static const uint64_t K512[80] = { - 0x428a2f98d728ae22ULL, 0x7137449123ef65cdULL, 0xb5c0fbcfec4d3b2fULL, 0xe9b5dba58189dbbcULL, - 0x3956c25bf348b538ULL, 0x59f111f1b605d019ULL, 0x923f82a4af194f9bULL, 0xab1c5ed5da6d8118ULL, - 0xd807aa98a3030242ULL, 0x12835b0145706fbeULL, 0x243185be4ee4b28cULL, 0x550c7dc3d5ffb4e2ULL, - 0x72be5d74f27b896fULL, 0x80deb1fe3b1696b1ULL, 0x9bdc06a725c71235ULL, 0xc19bf174cf692694ULL, - 0xe49b69c19ef14ad2ULL, 0xefbe4786384f25e3ULL, 0x0fc19dc68b8cd5b5ULL, 0x240ca1cc77ac9c65ULL, - 0x2de92c6f592b0275ULL, 0x4a7484aa6ea6e483ULL, 0x5cb0a9dcbd41fbd4ULL, 0x76f988da831153b5ULL, - 0x983e5152ee66dfabULL, 0xa831c66d2db43210ULL, 0xb00327c898fb213fULL, 0xbf597fc7beef0ee4ULL, - 0xc6e00bf33da88fc2ULL, 0xd5a79147930aa725ULL, 0x06ca6351e003826fULL, 0x142929670a0e6e70ULL, - 0x27b70a8546d22ffcULL, 0x2e1b21385c26c926ULL, 0x4d2c6dfc5ac42aedULL, 0x53380d139d95b3dfULL, - 0x650a73548baf63deULL, 0x766a0abb3c77b2a8ULL, 0x81c2c92e47edaee6ULL, 0x92722c851482353bULL, - 0xa2bfe8a14cf10364ULL, 0xa81a664bbc423001ULL, 0xc24b8b70d0f89791ULL, 0xc76c51a30654be30ULL, - 0xd192e819d6ef5218ULL, 0xd69906245565a910ULL, 0xf40e35855771202aULL, 0x106aa07032bbd1b8ULL, - 0x19a4c116b8d2d0c8ULL, 0x1e376c085141ab53ULL, 0x2748774cdf8eeb99ULL, 0x34b0bcb5e19b48a8ULL, - 0x391c0cb3c5c95a63ULL, 0x4ed8aa4ae3418acbULL, 0x5b9cca4f7763e373ULL, 0x682e6ff3d6b2b8a3ULL, - 0x748f82ee5defb2fcULL, 0x78a5636f43172f60ULL, 0x84c87814a1f0ab72ULL, 0x8cc702081a6439ecULL, - 0x90befffa23631e28ULL, 0xa4506cebde82bde9ULL, 0xbef9a3f7b2c67915ULL, 0xc67178f2e372532bULL, - 0xca273eceea26619cULL, 0xd186b8c721c0c207ULL, 0xeada7dd6cde0eb1eULL, 0xf57d4f7fee6ed178ULL, - 0x06f067aa72176fbaULL, 0x0a637dc5a2c898a6ULL, 0x113f9804bef90daeULL, 0x1b710b35131c471bULL, - 0x28db77f523047d84ULL, 0x32caab7b40c72493ULL, 0x3c9ebe0a15c9bebcULL, 0x431d67c49c100d4cULL, - 0x4cc5d4becb3e42b6ULL, 0x597f299cfc657e2aULL, 0x5fcb6fab3ad6faecULL, 0x6c44198c4a475817ULL, +#define ROTR64(x, n) (((x) >> (n)) | ((x) << (64 - (n)))) + +#define CH(x, y, z) (((x) & (y)) ^ (~(x) & (z))) +#define MAJ(x, y, z) (((x) & (y)) ^ ((x) & (z)) ^ ((y) & (z))) + +#define BSIG0(x) (ROTR64((x), 28) ^ ROTR64((x), 34) ^ ROTR64((x), 39)) +#define BSIG1(x) (ROTR64((x), 14) ^ ROTR64((x), 18) ^ ROTR64((x), 41)) + +#define SSIG0(x) (ROTR64((x), 1) ^ ROTR64((x), 8) ^ ((x) >> 7)) +#define SSIG1(x) (ROTR64((x), 19) ^ ROTR64((x), 61) ^ ((x) >> 6)) + +static const uint64_t K[80] = { + 0x428a2f98d728ae22ULL, + 0x7137449123ef65cdULL, + 0xb5c0fbcfec4d3b2fULL, + 0xe9b5dba58189dbbcULL, + 0x3956c25bf348b538ULL, + 0x59f111f1b605d019ULL, + 0x923f82a4af194f9bULL, + 0xab1c5ed5da6d8118ULL, + 0xd807aa98a3030242ULL, + 0x12835b0145706fbeULL, + 0x243185be4ee4b28cULL, + 0x550c7dc3d5ffb4e2ULL, + 0x72be5d74f27b896fULL, + 0x80deb1fe3b1696b1ULL, + 0x9bdc06a725c71235ULL, + 0xc19bf174cf692694ULL, + 0xe49b69c19ef14ad2ULL, + 0xefbe4786384f25e3ULL, + 0x0fc19dc68b8cd5b5ULL, + 0x240ca1cc77ac9c65ULL, + 0x2de92c6f592b0275ULL, + 0x4a7484aa6ea6e483ULL, + 0x5cb0a9dcbd41fbd4ULL, + 0x76f988da831153b5ULL, + 0x983e5152ee66dfabULL, + 0xa831c66d2db43210ULL, + 0xb00327c898fb213fULL, + 0xbf597fc7beef0ee4ULL, + 0xc6e00bf33da88fc2ULL, + 0xd5a79147930aa725ULL, + 0x06ca6351e003826fULL, + 0x142929670a0e6e70ULL, + 0x27b70a8546d22ffcULL, + 0x2e1b21385c26c926ULL, + 0x4d2c6dfc5ac42aedULL, + 0x53380d139d95b3dfULL, + 0x650a73548baf63deULL, + 0x766a0abb3c77b2a8ULL, + 0x81c2c92e47edaee6ULL, + 0x92722c851482353bULL, + 0xa2bfe8a14cf10364ULL, + 0xa81a664bbc423001ULL, + 0xc24b8b70d0f89791ULL, + 0xc76c51a30654be30ULL, + 0xd192e819d6ef5218ULL, + 0xd69906245565a910ULL, + 0xf40e35855771202aULL, + 0x106aa07032bbd1b8ULL, + 0x19a4c116b8d2d0c8ULL, + 0x1e376c085141ab53ULL, + 0x2748774cdf8eeb99ULL, + 0x34b0bcb5e19b48a8ULL, + 0x391c0cb3c5c95a63ULL, + 0x4ed8aa4ae3418acbULL, + 0x5b9cca4f7763e373ULL, + 0x682e6ff3d6b2b8a3ULL, + 0x748f82ee5defb2fcULL, + 0x78a5636f43172f60ULL, + 0x84c87814a1f0ab72ULL, + 0x8cc702081a6439ecULL, + 0x90befffa23631e28ULL, + 0xa4506cebde82bde9ULL, + 0xbef9a3f7b2c67915ULL, + 0xc67178f2e372532bULL, + 0xca273eceea26619cULL, + 0xd186b8c721c0c207ULL, + 0xeada7dd6cde0eb1eULL, + 0xf57d4f7fee6ed178ULL, + 0x06f067aa72176fbaULL, + 0x0a637dc5a2c898a6ULL, + 0x113f9804bef90daeULL, + 0x1b710b35131c471bULL, + 0x28db77f523047d84ULL, + 0x32caab7b40c72493ULL, + 0x3c9ebe0a15c9bebcULL, + 0x431d67c49c100d4cULL, + 0x4cc5d4becb3e42b6ULL, + 0x597f299cfc657e2aULL, + 0x5fcb6fab3ad6faecULL, + 0x6c44198c4a475817ULL }; -#define ROTR64(x, n) (((x) >> (n)) | ((x) << (64 - (n)))) -#define CH64(x, y, z) (((x) & (y)) ^ (~(x) & (z))) -#define MAJ64(x, y, z) (((x) & (y)) ^ ((x) & (z)) ^ ((y) & (z))) -#define EP0_64(x) (ROTR64(x, 28) ^ ROTR64(x, 34) ^ ROTR64(x, 39)) -#define EP1_64(x) (ROTR64(x, 14) ^ ROTR64(x, 18) ^ ROTR64(x, 41)) -#define SIG0_64(x) (ROTR64(x, 1) ^ ROTR64(x, 8) ^ ((x) >> 7)) -#define SIG1_64(x) (ROTR64(x, 19) ^ ROTR64(x, 61) ^ ((x) >> 6)) - -static void sha512_transform(eos_sha512_ctx_t *ctx) +static uint64_t load_be64(const uint8_t *p) +{ + return ((uint64_t)p[0] << 56) | + ((uint64_t)p[1] << 48) | + ((uint64_t)p[2] << 40) | + ((uint64_t)p[3] << 32) | + ((uint64_t)p[4] << 24) | + ((uint64_t)p[5] << 16) | + ((uint64_t)p[6] << 8) | + ((uint64_t)p[7]); +} + +static void store_be64(uint8_t *p, uint64_t x) +{ + p[0] = (uint8_t)(x >> 56); + p[1] = (uint8_t)(x >> 48); + p[2] = (uint8_t)(x >> 40); + p[3] = (uint8_t)(x >> 32); + p[4] = (uint8_t)(x >> 24); + p[5] = (uint8_t)(x >> 16); + p[6] = (uint8_t)(x >> 8); + p[7] = (uint8_t)x; +} + +static void sha512_transform(eos_sha512_ctx_t *ctx, + const uint8_t block[128]) { uint64_t w[80]; - uint64_t a, b, c, d, e, f, g, h, t1, t2; - - for (int i = 0; i < 16; i++) { - w[i] = ((uint64_t)ctx->buffer[i * 8 + 0] << 56) | - ((uint64_t)ctx->buffer[i * 8 + 1] << 48) | - ((uint64_t)ctx->buffer[i * 8 + 2] << 40) | - ((uint64_t)ctx->buffer[i * 8 + 3] << 32) | - ((uint64_t)ctx->buffer[i * 8 + 4] << 24) | - ((uint64_t)ctx->buffer[i * 8 + 5] << 16) | - ((uint64_t)ctx->buffer[i * 8 + 6] << 8) | - ((uint64_t)ctx->buffer[i * 8 + 7]); - } - for (int i = 16; i < 80; i++) { - w[i] = SIG1_64(w[i - 2]) + w[i - 7] + SIG0_64(w[i - 15]) + w[i - 16]; - } - a = ctx->state[0]; b = ctx->state[1]; c = ctx->state[2]; d = ctx->state[3]; - e = ctx->state[4]; f = ctx->state[5]; g = ctx->state[6]; h = ctx->state[7]; + for (int i = 0; i < 16; i++) + w[i] = load_be64(block + i * 8); + + for (int i = 16; i < 80; i++) + w[i] = SSIG1(w[i - 2]) + w[i - 7] + + SSIG0(w[i - 15]) + w[i - 16]; + + uint64_t a = ctx->state[0]; + uint64_t b = ctx->state[1]; + uint64_t c = ctx->state[2]; + uint64_t d = ctx->state[3]; + uint64_t e = ctx->state[4]; + uint64_t f = ctx->state[5]; + uint64_t g = ctx->state[6]; + uint64_t h = ctx->state[7]; for (int i = 0; i < 80; i++) { - t1 = h + EP1_64(e) + CH64(e, f, g) + K512[i] + w[i]; - t2 = EP0_64(a) + MAJ64(a, b, c); - h = g; g = f; f = e; e = d + t1; - d = c; c = b; b = a; a = t1 + t2; + uint64_t t1 = h + BSIG1(e) + CH(e, f, g) + K[i] + w[i]; + uint64_t t2 = BSIG0(a) + MAJ(a, b, c); + + h = g; + g = f; + f = e; + e = d + t1; + d = c; + c = b; + b = a; + a = t1 + t2; } - ctx->state[0] += a; ctx->state[1] += b; ctx->state[2] += c; ctx->state[3] += d; - ctx->state[4] += e; ctx->state[5] += f; ctx->state[6] += g; ctx->state[7] += h; + ctx->state[0] += a; + ctx->state[1] += b; + ctx->state[2] += c; + ctx->state[3] += d; + ctx->state[4] += e; + ctx->state[5] += f; + ctx->state[6] += g; + ctx->state[7] += h; } void eos_sha512_init(eos_sha512_ctx_t *ctx) { - ctx->state[0] = 0x6a09e667f3bcc908ULL; ctx->state[1] = 0xbb67ae8584caa73bULL; - ctx->state[2] = 0x3c6ef372fe94f82bULL; ctx->state[3] = 0xa54ff53a5f1d36f1ULL; - ctx->state[4] = 0x510e527fade682d1ULL; ctx->state[5] = 0x9b05688c2b3e6c1fULL; - ctx->state[6] = 0x1f83d9abfb41bd6bULL; ctx->state[7] = 0x5be0cd19137e2179ULL; - ctx->count = 0; - memset(ctx->buffer, 0, sizeof(ctx->buffer)); + ctx->state[0] = 0x6a09e667f3bcc908ULL; + ctx->state[1] = 0xbb67ae8584caa73bULL; + ctx->state[2] = 0x3c6ef372fe94f82bULL; + ctx->state[3] = 0xa54ff53a5f1d36f1ULL; + ctx->state[4] = 0x510e527fade682d1ULL; + ctx->state[5] = 0x9b05688c2b3e6c1fULL; + ctx->state[6] = 0x1f83d9abfb41bd6bULL; + ctx->state[7] = 0x5be0cd19137e2179ULL; + + ctx->bitlen[0] = 0; + ctx->bitlen[1] = 0; + ctx->buffer_len = 0; } -void eos_sha512_update(eos_sha512_ctx_t *ctx, const uint8_t *data, size_t len) +void eos_sha512_update(eos_sha512_ctx_t *ctx, + const uint8_t *data, + size_t len) { - size_t idx = (size_t)(ctx->count % EOS_SHA512_BLOCK_SIZE); + while (len > 0) { + size_t copy = 128 - ctx->buffer_len; - ctx->count += len; + if (copy > len) + copy = len; - while (len > 0) { - size_t take = EOS_SHA512_BLOCK_SIZE - idx; - if (take > len) take = len; - memcpy(ctx->buffer + idx, data, take); - idx += take; - data += take; - len -= take; - if (idx == EOS_SHA512_BLOCK_SIZE) { - sha512_transform(ctx); - idx = 0; + memcpy(ctx->buffer + ctx->buffer_len, data, copy); + + ctx->buffer_len += copy; + data += copy; + len -= copy; + + uint64_t bits = (uint64_t)copy << 3; + + uint64_t old_low = ctx->bitlen[1]; + ctx->bitlen[1] += bits; + + if (ctx->bitlen[1] < old_low) + ctx->bitlen[0]++; + + ctx->bitlen[0] += (uint64_t)copy >> 61; + + if (ctx->buffer_len == 128) { + sha512_transform(ctx, ctx->buffer); + ctx->buffer_len = 0; } } } -void eos_sha512_final(eos_sha512_ctx_t *ctx, uint8_t digest[EOS_SHA512_DIGEST_SIZE]) +void eos_sha512_final(eos_sha512_ctx_t *ctx, + uint8_t digest[EOS_SHA512_DIGEST_SIZE]) { - /* SHA-512 encodes the message length as a 128-bit big-endian bit count. - * A bootloader never hashes anywhere near 2^61 bytes, so the high 64 - * bits are always zero; they are still written so the padding block is - * byte-exact against FIPS 180-4. */ - uint64_t bits = ctx->count * 8ULL; - size_t idx = (size_t)(ctx->count % EOS_SHA512_BLOCK_SIZE); - - ctx->buffer[idx++] = 0x80; - - if (idx > 112) { - while (idx < EOS_SHA512_BLOCK_SIZE) ctx->buffer[idx++] = 0; - sha512_transform(ctx); - idx = 0; - } - while (idx < 112) ctx->buffer[idx++] = 0; + size_t i = ctx->buffer_len; - memset(ctx->buffer + 112, 0, 8); /* high 64 bits of length */ - for (int i = 0; i < 8; i++) { - ctx->buffer[120 + i] = (uint8_t)(bits >> (56 - 8 * i)); - } - sha512_transform(ctx); + ctx->buffer[i++] = 0x80; - for (int i = 0; i < 8; i++) { - for (int j = 0; j < 8; j++) { - digest[i * 8 + j] = (uint8_t)(ctx->state[i] >> (56 - 8 * j)); - } + if (i > 112) { + while (i < 128) + ctx->buffer[i++] = 0; + + sha512_transform(ctx, ctx->buffer); + i = 0; } - /* Do not leave hash state on the stack of a boot path. */ + while (i < 112) + ctx->buffer[i++] = 0; + + store_be64(ctx->buffer + 112, ctx->bitlen[0]); + store_be64(ctx->buffer + 120, ctx->bitlen[1]); + + sha512_transform(ctx, ctx->buffer); + + for (int i2 = 0; i2 < 8; i2++) + store_be64(digest + i2 * 8, ctx->state[i2]); + memset(ctx, 0, sizeof(*ctx)); } diff --git a/include/eos_boot_log.h b/include/eos_boot_log.h index 86024d7..964ebb8 100644 --- a/include/eos_boot_log.h +++ b/include/eos_boot_log.h @@ -53,12 +53,12 @@ void eos_boot_log_init(uint32_t head); void eos_boot_log_append(uint32_t event, uint32_t slot, uint32_t detail); /** - * @brief Read one boot log entry by index. - * @param index Entry index (0 to EOS_BOOT_LOG_MAX - 1). - * @param out Receives the entry at @p index. - * @return EOS_OK on success, EOS_ERR_INVALID on a bad index or null @p out. + * @brief Current ring-buffer write position. + * + * Persisted into the boot control block on handoff so the log survives a + * reset. @return Head index in [0, EOS_BOOT_LOG_MAX). */ -int eos_boot_log_read(uint32_t index, eos_boot_log_entry_t *out); +uint32_t eos_boot_log_get_head(void); /** * @brief Read one log entry by ring index. diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 11f2f3f..219c3c8 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -55,25 +55,30 @@ list(APPEND EBLDR_UNIT_TESTS test_fw_transport) add_executable(eboot_test_fw_update_sig unit/test_fw_update_sig.c) target_link_libraries(eboot_test_fw_update_sig PRIVATE eboot_core) add_test(NAME test_fw_update_sig COMMAND eboot_test_fw_update_sig) +list(APPEND EBLDR_UNIT_TESTS test_fw_update_sig) # --- test_fw_update: firmware update finalize / anti-rollback wiring --- add_executable(eboot_test_fw_update unit/test_fw_update.c) target_link_libraries(eboot_test_fw_update PRIVATE eboot_core) add_test(NAME test_fw_update COMMAND eboot_test_fw_update) +list(APPEND EBLDR_UNIT_TESTS test_fw_update) # --- test_jump_app: stage-1 jump uses the authenticated TLV counter --- add_executable(eboot_test_jump_app unit/test_jump_app.c) target_link_libraries(eboot_test_jump_app PRIVATE eboot_stage1) add_test(NAME test_jump_app COMMAND eboot_test_jump_app) +list(APPEND EBLDR_UNIT_TESTS test_jump_app) # --- test_slot_size_bounds: verify_slot() must reject image_size > slot capacity --- add_executable(eboot_test_slot_size_bounds unit/test_slot_size_bounds.c) target_link_libraries(eboot_test_slot_size_bounds PRIVATE eboot_core) add_test(NAME test_slot_size_bounds COMMAND eboot_test_slot_size_bounds) +list(APPEND EBLDR_UNIT_TESTS test_slot_size_bounds) add_executable(eboot_test_jump_app_bounds unit/test_jump_app_bounds.c) target_link_libraries(eboot_test_jump_app_bounds PRIVATE eboot_core eboot_stage1) add_test(NAME test_jump_app_bounds COMMAND eboot_test_jump_app_bounds) +list(APPEND EBLDR_UNIT_TESTS test_jump_app_bounds) # --- test_device_table: UEFI-style device table --- add_executable(eboot_test_device_table unit/test_device_table.c) @@ -105,6 +110,7 @@ target_include_directories(eboot_test_qemu_arm64_timer PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/../boards/qemu_arm64 ) add_test(NAME test_qemu_arm64_timer COMMAND eboot_test_qemu_arm64_timer) +list(APPEND EBLDR_UNIT_TESTS test_qemu_arm64_timer) # --- test_board_registry: Runtime board selection --- add_executable(eboot_test_board_registry unit/test_board_registry.c) @@ -148,6 +154,7 @@ target_link_libraries(eboot_test_ed25519_contract PRIVATE eboot_core) target_include_directories(eboot_test_ed25519_contract PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}) add_test(NAME test_ed25519_contract COMMAND eboot_test_ed25519_contract) +list(APPEND EBLDR_UNIT_TESTS test_ed25519_contract) # --- test_keystore: Key management --- add_executable(eboot_test_keystore unit/test_keystore.c) @@ -159,6 +166,7 @@ list(APPEND EBLDR_UNIT_TESTS test_keystore) add_executable(eboot_test_secure_boot_policy unit/test_secure_boot_policy.c) target_link_libraries(eboot_test_secure_boot_policy PRIVATE eboot_core) add_test(NAME test_secure_boot_policy COMMAND eboot_test_secure_boot_policy) +list(APPEND EBLDR_UNIT_TESTS test_secure_boot_policy) # --- test_rollback: Anti-rollback security counter --- add_executable(eboot_test_rollback unit/test_rollback.c) @@ -192,18 +200,21 @@ list(APPEND EBLDR_UNIT_TESTS test_ecc) add_executable(eboot_test_fw_decrypt unit/test_fw_decrypt.c) target_link_libraries(eboot_test_fw_decrypt PRIVATE eboot_core) add_test(NAME test_fw_decrypt COMMAND eboot_test_fw_decrypt) +list(APPEND EBLDR_UNIT_TESTS test_fw_decrypt) # --- test_fdt_loader: device tree parsing against malformed blobs --- # core/fdt_loader.c was in no source list, so it had never been compiled. add_executable(eboot_test_fdt_loader unit/test_fdt_loader.c) target_link_libraries(eboot_test_fdt_loader PRIVATE eboot_core) add_test(NAME test_fdt_loader COMMAND eboot_test_fdt_loader) +list(APPEND EBLDR_UNIT_TESTS test_fdt_loader) # --- test_eos_sign_boot_path: the tool's real output through the real parser --- add_executable(eboot_test_eos_sign_boot_path unit/test_eos_sign_boot_path.c) target_link_libraries(eboot_test_eos_sign_boot_path PRIVATE eboot_core) target_include_directories(eboot_test_eos_sign_boot_path PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}) add_test(NAME test_eos_sign_boot_path COMMAND eboot_test_eos_sign_boot_path) +list(APPEND EBLDR_UNIT_TESTS test_eos_sign_boot_path) # --- Valgrind test targets --- # @@ -218,12 +229,7 @@ add_test(NAME test_eos_sign_boot_path COMMAND eboot_test_eos_sign_boot_path) find_program(VALGRIND valgrind) if(VALGRIND) set(VALGRIND_OPTS --leak-check=full --error-exitcode=1 --quiet) - foreach(TEST_NAME test_bootctl test_crypto test_ed25519 test_ed25519_contract test_keystore - test_device_table test_runtime_svc test_board_config - test_multicore test_board_registry test_slot_manager - test_boot_log test_image_verify test_image_abi - test_recovery test_slot_size_bounds test_fw_transport - test_fw_update test_jump_app test_tlv_auth) + foreach(TEST_NAME ${EBLDR_UNIT_TESTS}) add_test( NAME valgrind_${TEST_NAME} COMMAND ${VALGRIND} ${VALGRIND_OPTS} $ diff --git a/tests/unit/test_ed25519.c b/tests/unit/test_ed25519.c index 6c22666..9ea837c 100644 --- a/tests/unit/test_ed25519.c +++ b/tests/unit/test_ed25519.c @@ -90,46 +90,6 @@ static const struct rfc_vector k_vectors[] = { #define N_VECTORS (sizeof(k_vectors) / sizeof(k_vectors[0])) -/* All eight compressed encodings of edwards25519's order-8 torsion subgroup - * (the cyclic group for any order-8 point G, i.e. {1G, 2G, ..., 8G=O}). - * Order 8/gcd(k,8) for kG: orders present are 1 (identity), 2, 4, 4, 8, 8, 8, 8. - * - * Not hand-transcribed: derived by decoding an order-8 generator from this - * file's own unpackneg()/point_add()/point_is_identity(), then enumerating - * 1G..8G with point_add() and re-encoding with point_pack() -- so their - * correctness rests on the same curve arithmetic this file already uses for - * real verification, not on a separately-copied constant that could carry a - * transcription error. Cross-checked: point k and point (8-k) differ only in - * the sign bit (byte 31), as required since (8-k)G = -(kG); the order-2 - * element (k=4) is its own negation, as required since 2P = O implies P = -P. - */ -static const uint8_t k_low_order[8][32] = { - {0x26,0xe8,0x95,0x8f,0xc2,0xb2,0x27,0xb0,0x45,0xc3,0xf4,0x89,0xf2,0xef,0x98,0xf0, - 0xd5,0xdf,0xac,0x05,0xd3,0xc6,0x33,0x39,0xb1,0x38,0x02,0x88,0x6d,0x53,0xfc,0x85}, - {0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00, - 0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x80}, - {0xc7,0x17,0x6a,0x70,0x3d,0x4d,0xd8,0x4f,0xba,0x3c,0x0b,0x76,0x0d,0x10,0x67,0x0f, - 0x2a,0x20,0x53,0xfa,0x2c,0x39,0xcc,0xc6,0x4e,0xc7,0xfd,0x77,0x92,0xac,0x03,0xfa}, - {0xec,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff, - 0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0x7f}, - {0xc7,0x17,0x6a,0x70,0x3d,0x4d,0xd8,0x4f,0xba,0x3c,0x0b,0x76,0x0d,0x10,0x67,0x0f, - 0x2a,0x20,0x53,0xfa,0x2c,0x39,0xcc,0xc6,0x4e,0xc7,0xfd,0x77,0x92,0xac,0x03,0x7a}, - {0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00, - 0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00}, - {0x26,0xe8,0x95,0x8f,0xc2,0xb2,0x27,0xb0,0x45,0xc3,0xf4,0x89,0xf2,0xef,0x98,0xf0, - 0xd5,0xdf,0xac,0x05,0xd3,0xc6,0x33,0x39,0xb1,0x38,0x02,0x88,0x6d,0x53,0xfc,0x05}, - {0x01,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00, - 0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00}, -}; - -/* A handful of distinct messages, so a low-order-R rejection isn't pinned - * against only one k = SHA-512(R || A || M) value. */ -static const char *messages[] = { - "", - "a", - "untrusted firmware", -}; - /* ---- positive tests: a conforming signature MUST be accepted ---- */ TEST(test_ed25519_rfc8032_vectors_accepted) diff --git a/tests/unit/test_fw_decrypt.c b/tests/unit/test_fw_decrypt.c index ca3c358..ac1b26a 100644 --- a/tests/unit/test_fw_decrypt.c +++ b/tests/unit/test_fw_decrypt.c @@ -27,6 +27,7 @@ static int tests_passed = 0; static void name(void); \ static void run_##name(void) { \ printf(" %-52s ", #name); \ + tests_run++; \ name(); \ tests_passed++; \ printf("[PASS]\n"); \ @@ -265,7 +266,6 @@ int main(void) run_test_init_rejects_bad_arguments_and_unprovisioned_keys(); run_test_update_and_final_reject_uninitialised_contexts(); - tests_run = 8; printf("\n%d/%d tests passed\n", tests_passed, tests_run); return (tests_passed == tests_run) ? 0 : 1; } diff --git a/tests/unit/test_fw_transport.c b/tests/unit/test_fw_transport.c index b5ce142..6d052e5 100644 --- a/tests/unit/test_fw_transport.c +++ b/tests/unit/test_fw_transport.c @@ -16,6 +16,7 @@ #include "eos_image.h" #include "eos_image_tlv.h" #include "eos_hal.h" +#include "../vectors/fw_update_test_sigs.h" #include #include #include @@ -102,6 +103,23 @@ static eos_reset_reason_t sim_reset_reason(void) { return EOS_RESET_POWER_ON; } static bool sim_recovery_pin(void) { return false; } static void sim_system_reset(void) {} +/* Since #104 finalize verifies the image signature unconditionally, so the + * container the XMODEM test finalizes has to be genuinely signed. The keystore + * takes its anchor from OTP slot 0 when the board has OTP at all; serve the + * public half of the key the fixture signature was made under. */ +#define OTP_KEY_OFFSET_SLOT0 0x100u + +static int sim_otp_read(uint32_t offset, void *buf, size_t len) +{ + if (!buf) return EOS_ERR_INVALID; + if (offset == OTP_KEY_OFFSET_SLOT0 && len == sizeof(eos_test_sig_pubkey)) { + memcpy(buf, eos_test_sig_pubkey, len); + return EOS_OK; + } + memset(buf, 0, len); /* slot 1 unprovisioned, nothing revoked */ + return EOS_OK; +} + static const eos_board_ops_t sim_ops = { .flash_base = 0, .flash_size = SIM_FLASH_SIZE, @@ -119,6 +137,7 @@ static const eos_board_ops_t sim_ops = { .flash_read = sim_flash_read, .flash_write = sim_flash_write, .flash_erase = sim_flash_erase, + .otp_read = sim_otp_read, .watchdog_init = sim_noop_u32, .watchdog_feed = sim_noop, @@ -354,7 +373,8 @@ static void build_container(void) hdr.load_addr = SIM_SLOT_B_ADDR; hdr.entry_addr = SIM_SLOT_B_ADDR; hdr.flags = 0; /* CRC32 integrity path */ - hdr.sig_type = EOS_SIG_NONE; + hdr.sig_type = EOS_SIG_ED25519; + hdr.sig_len = EOS_SIG_MAX_SIZE; uint32_t crc = crc32_payload(payload, CONT_PAYLOAD_LEN); memcpy(hdr.hash, &crc, sizeof(crc)); @@ -365,6 +385,10 @@ static void build_container(void) hdr.tlv_len = CONT_TLV_LEN; memcpy(hdr.tlv_hash, digest, EOS_IMG_TLV_HASH_LEN); + /* Precomputed by tools/gen_fw_update_test_sigs.py for exactly the field + * values above; change any of them and regenerate. */ + memcpy(hdr.signature, eos_test_sig_fw_transport_container, EOS_SIG_MAX_SIZE); + memcpy(container, &hdr, sizeof(hdr)); } @@ -880,7 +904,6 @@ int main(void) run_test_raw_oversized_length_is_rejected(); run_test_raw_zero_length_is_rejected(); - tests_run = 19; printf("\n%d/%d tests passed\n", tests_passed, tests_run); return (tests_passed == tests_run) ? 0 : 1; } diff --git a/tests/unit/test_fw_update.c b/tests/unit/test_fw_update.c index afe54d8..ae87c91 100644 --- a/tests/unit/test_fw_update.c +++ b/tests/unit/test_fw_update.c @@ -18,6 +18,7 @@ #include "eos_crypto_boot.h" #include "eos_bootctl.h" #include "eos_hal.h" +#include "../vectors/fw_update_test_sigs.h" #include #include #include @@ -65,6 +66,24 @@ static int sim_monotonic_read(uint32_t *value) return EOS_OK; } +/* Since #104 finalize verifies the image signature unconditionally, and it + * does so before the anti-rollback check, so an image has to be genuinely + * signed to reach the stage these tests exercise. The keystore takes its + * anchor from OTP slot 0 when the board has OTP at all; serve the public + * half of the key the fixture signatures were made under, and nothing else. */ +#define OTP_KEY_OFFSET_SLOT0 0x100u + +static int sim_otp_read(uint32_t offset, void *buf, size_t len) +{ + if (!buf) return EOS_ERR_INVALID; + if (offset == OTP_KEY_OFFSET_SLOT0 && len == sizeof(eos_test_sig_pubkey)) { + memcpy(buf, eos_test_sig_pubkey, len); + return EOS_OK; + } + memset(buf, 0, len); /* slot 1 unprovisioned, nothing revoked */ + return EOS_OK; +} + static const eos_board_ops_t sim_ops = { .flash_base = 0, .flash_size = SIM_FLASH_SIZE, @@ -79,6 +98,7 @@ static const eos_board_ops_t sim_ops = { .flash_write = sim_flash_write, .flash_erase = sim_flash_erase, .monotonic_read = sim_monotonic_read, + .otp_read = sim_otp_read, }; static int tests_run = 0; @@ -98,6 +118,7 @@ static int tests_passed = 0; sim_counter = 0; \ eos_hal_init(&sim_ops); \ printf(" %-58s ", #name); \ + tests_run++; \ name(); \ tests_passed++; \ printf("[PASS]\n"); \ @@ -135,8 +156,8 @@ static void build_image(uint8_t *out, uint32_t sec_ver) hdr.image_version = 0x00010000u; hdr.flags = EOS_IMG_FLAG_HASH_SHA256; eos_sha256(payload, PAYLOAD_SIZE, hdr.hash); - hdr.sig_type = EOS_SIG_NONE; - hdr.sig_len = 0; + hdr.sig_type = EOS_SIG_ED25519; + hdr.sig_len = EOS_SIG_MAX_SIZE; uint8_t tlv[TLV_AREA_LEN]; eos_tlv_info_t info = { EOS_TLV_INFO_MAGIC, TLV_AREA_LEN }; @@ -150,6 +171,15 @@ static void build_image(uint8_t *out, uint32_t sec_ver) hdr.tlv_len = TLV_AREA_LEN; memcpy(hdr.tlv_hash, digest, EOS_IMG_TLV_HASH_LEN); + /* Signature over the prefix, precomputed by tools/gen_fw_update_test_sigs.py + * for exactly the field values above. Every other sec_ver would need its + * own entry there, because tlv_hash is inside the signed prefix. */ + switch (sec_ver) { + case 3: memcpy(hdr.signature, eos_test_sig_fw_update_sec_ver_3, EOS_SIG_MAX_SIZE); break; + case 9: memcpy(hdr.signature, eos_test_sig_fw_update_sec_ver_9, EOS_SIG_MAX_SIZE); break; + default: printf("[FAIL] no fixture signature for sec_ver %u\n", (unsigned)sec_ver); exit(1); + } + memset(out, 0, IMAGE_BUF_LEN); memcpy(out, &hdr, sizeof(hdr)); memcpy(out + sizeof(hdr), payload, PAYLOAD_SIZE); @@ -315,7 +345,6 @@ int main(void) run_test_trailing_byte_is_rejected_the_same_across_chunk_boundaries(); run_test_finalize_accepts_tlv_counter_equal_to_floor(); - tests_run = 6; printf("\n%d/%d passed\n", tests_passed, tests_run); return tests_passed == tests_run ? 0 : 1; } diff --git a/tests/unit/test_fw_update_sig.c b/tests/unit/test_fw_update_sig.c index 42970d6..b510ede 100644 --- a/tests/unit/test_fw_update_sig.c +++ b/tests/unit/test_fw_update_sig.c @@ -129,6 +129,7 @@ static int tests_passed = 0; static void run_##name(void) { \ setup(); \ printf(" %-58s ", #name); \ + tests_run++; \ name(); \ tests_passed++; \ printf("[OK]\n"); \ @@ -249,7 +250,6 @@ int main(void) run_test_sha256_sigtype_is_still_unsigned_and_rejected(); run_test_corrupt_image_is_rejected_at_integrity_stage(); - tests_run = 3; printf("\n%d/%d tests passed\n", tests_passed, tests_run); return (tests_passed == tests_run) ? 0 : 1; } diff --git a/tests/unit/test_jump_app.c b/tests/unit/test_jump_app.c index e9a07e6..c882119 100644 --- a/tests/unit/test_jump_app.c +++ b/tests/unit/test_jump_app.c @@ -221,6 +221,7 @@ static int tests_passed = 0; eos_hal_init(&sim_ops); \ eos_rollback_clear_staged(); \ printf(" %-58s ", #name); \ + tests_run++; \ name(); \ tests_passed++; \ printf("[PASS]\n"); \ @@ -265,7 +266,6 @@ int main(void) run_test_jump_rejects_tlv_counter_below_hw_floor(); run_test_jump_stages_tlv_counter_above_floor(); - tests_run = 2; printf("\n%d/%d passed\n", tests_passed, tests_run); return tests_passed == tests_run ? 0 : 1; } diff --git a/tests/unit/test_jump_app_bounds.c b/tests/unit/test_jump_app_bounds.c index 5e08b6f..496e161 100644 --- a/tests/unit/test_jump_app_bounds.c +++ b/tests/unit/test_jump_app_bounds.c @@ -115,6 +115,7 @@ static int tests_passed = 0; payload_bytes_read = 0; \ eos_hal_init(&sim_ops); \ printf(" %-55s ", #name); \ + tests_run++; \ name(); \ tests_passed++; \ printf("[PASS]\n"); \ @@ -191,7 +192,6 @@ int main(void) run_test_oversized_image_rejected_before_reading_payload(); run_test_in_bounds_image_reaches_integrity_check(); - tests_run = 2; printf("\n%d/%d tests passed\n", tests_passed, tests_run); diff --git a/tests/unit/test_secure_boot_policy.c b/tests/unit/test_secure_boot_policy.c index be092d5..39bc2c2 100644 --- a/tests/unit/test_secure_boot_policy.c +++ b/tests/unit/test_secure_boot_policy.c @@ -128,6 +128,11 @@ static void reset_fixture(void) memset(sim_otp, 0, sizeof(sim_otp)); sim_ops.otp_read = sim_otp_read; sim_ops.flash_read = sim_flash_read; + /* Step 5b (anti-rollback, #103) reads the image's TLV counter through the + * HAL slot that contains it, and an image in no slot is a bad header + * before step 7 is ever reached. The staged image is slot A. */ + sim_ops.slot_a_addr = FLASH_BASE; + sim_ops.slot_a_size = FLASH_SIZE; if (provide_otp_write) sim_ops.otp_write = sim_otp_write; otp_write_rc = EOS_OK; otp_write_calls = 0; diff --git a/tests/unit/test_suite_bookkeeping.py b/tests/unit/test_suite_bookkeeping.py index afa59bb..699cca7 100644 --- a/tests/unit/test_suite_bookkeeping.py +++ b/tests/unit/test_suite_bookkeeping.py @@ -37,6 +37,14 @@ "test_boot_log.c": "prints its own summary and has no TEST() macro", "test_ecc.c": "single-scenario suite; no per-test harness", "test_image_abi.c": "compile-time _Static_asserts; nothing runs per test", + "test_ed25519_contract.c": "one loop over the generated vector table; " + "counts accepted/refused/wrong per vector", + "test_eos_sign_boot_path.c": "CHECK() counts failures, not tests; the " + "exit code is the failure count", + "test_fdt_loader.c": "RUN() macro with exit(1) on the first failed " + "ASSERT; tests_passed is the count", + "test_qemu_arm64_timer.c": "four inline ASSERT_EQ calls that exit(1) on " + "failure; no per-test harness", } diff --git a/tests/unit/test_tlv_auth.c b/tests/unit/test_tlv_auth.c index cd4c9c2..2046a37 100644 --- a/tests/unit/test_tlv_auth.c +++ b/tests/unit/test_tlv_auth.c @@ -362,7 +362,6 @@ int main(void) run_test_tlv_area_must_fit_in_slot(); run_test_hw_floor_uses_tlv_counter_not_image_version(); - tests_run = 9; printf("\n%d/%d passed\n", tests_passed, tests_run); return tests_passed == tests_run ? 0 : 1; } diff --git a/tests/vectors/fw_update_test_sigs.h b/tests/vectors/fw_update_test_sigs.h new file mode 100644 index 0000000..14ea295 --- /dev/null +++ b/tests/vectors/fw_update_test_sigs.h @@ -0,0 +1,43 @@ +/* Generated by tools/gen_fw_update_test_sigs.py -- do not edit. + * + * Ed25519 signatures over the 92-byte signed header prefix of the + * images tests/unit/test_fw_update.c and test_fw_transport.c build, + * under the RFC 8032 section 7.1 TEST 1 key. Regenerate with: + * + * python3 tools/gen_fw_update_test_sigs.py > tests/vectors/fw_update_test_sigs.h + */ +#ifndef EOS_FW_UPDATE_TEST_SIGS_H +#define EOS_FW_UPDATE_TEST_SIGS_H + +/* The public half. Tests serve it from simulated OTP slot 0 so the + * keystore selects it the way a provisioned board would. */ +static const unsigned char eos_test_sig_pubkey[32] = { + 0xd7,0x5a,0x98,0x01,0x82,0xb1,0x0a,0xb7,0xd5,0x4b,0xfe,0xd3,0xc9,0x64,0x07,0x3a, + 0x0e,0xe1,0x72,0xf3,0xda,0xa6,0x23,0x25,0xaf,0x02,0x1a,0x68,0xf7,0x07,0x51,0x1a, +}; + +/* test_fw_update.c build_image(out, 3) */ +static const unsigned char eos_test_sig_fw_update_sec_ver_3[64] = { + 0x67,0xf0,0x58,0x37,0x33,0x61,0x23,0x1e,0xa7,0x6e,0x04,0x59,0x35,0xc7,0x5e,0x84, + 0xfc,0xac,0xce,0x67,0x64,0x7c,0x9b,0xbf,0x17,0xed,0x0a,0xcb,0xc3,0x97,0x13,0xeb, + 0xa4,0xd9,0x1c,0x60,0xc3,0x08,0xa0,0xdf,0xd9,0x06,0x7f,0xf6,0x62,0xd4,0x98,0x3f, + 0x23,0xa6,0x54,0x32,0x48,0x1a,0xc5,0xb3,0xb6,0xe0,0xac,0xf4,0xf1,0x47,0x99,0x07, +}; + +/* test_fw_update.c build_image(out, 9) */ +static const unsigned char eos_test_sig_fw_update_sec_ver_9[64] = { + 0x33,0xcd,0x40,0x50,0x31,0x62,0x91,0x2c,0x9a,0x2a,0x22,0x09,0x0b,0x4a,0xb6,0x69, + 0x2d,0x9c,0x0c,0x20,0x49,0x91,0x9b,0xe0,0x15,0x5d,0x74,0xe1,0x94,0x7e,0x34,0xe5, + 0x41,0x96,0x5c,0x2f,0x3c,0x17,0xa8,0x1b,0x34,0xae,0x6b,0x76,0x7b,0x19,0xbf,0xc0, + 0x91,0xe7,0xc9,0xd8,0x65,0x2f,0x1f,0xa8,0x10,0x74,0x77,0xcb,0x64,0x71,0xe7,0x02, +}; + +/* test_fw_transport.c build_container() */ +static const unsigned char eos_test_sig_fw_transport_container[64] = { + 0x25,0xbb,0xa5,0x30,0x94,0xbe,0xaa,0x7b,0xe5,0xaf,0x2a,0xf6,0x27,0x09,0xa1,0xcd, + 0x42,0x4d,0x20,0xdb,0xde,0xa1,0x28,0x65,0x1b,0x03,0xf8,0xdb,0x77,0x59,0x09,0xf2, + 0x56,0x6f,0xe8,0x84,0x7f,0x2c,0xa1,0x32,0xf0,0xbc,0xd3,0x05,0x7c,0xd8,0xeb,0x84, + 0x08,0x43,0x3a,0xd9,0xf9,0x1a,0xd1,0x47,0xfd,0x5b,0x21,0x48,0x01,0x58,0xcc,0x05, +}; + +#endif /* EOS_FW_UPDATE_TEST_SIGS_H */ diff --git a/tools/gen_fw_update_test_sigs.py b/tools/gen_fw_update_test_sigs.py new file mode 100644 index 0000000..86f18d1 --- /dev/null +++ b/tools/gen_fw_update_test_sigs.py @@ -0,0 +1,156 @@ +#!/usr/bin/env python3 +# SPDX-License-Identifier: MIT +# Copyright (c) 2026 EoS Project +"""Sign the image headers that tests/unit/test_fw_update.c and +tests/unit/test_fw_transport.c stream through eos_fw_update_finalize(). + +Since #104 finalize verifies the Ed25519 signature unconditionally, and it +does so *before* the anti-rollback check, so an unsigned test image can no +longer reach the rollback stage -- it is refused as EOS_ERR_SIGNATURE first. +The images those suites build have to carry a real signature. eBoot has no +Ed25519 signer in C (only a verifier), so the signatures are computed here and +committed as tests/vectors/fw_update_test_sigs.h. + +The signing key is the RFC 8032 section 7.1 TEST 1 key. Its secret half is +printed in the RFC, so nothing here is a secret. The tests provision the +public half through their simulated OTP (slot 0), which is the path +eos_keystore_init() takes on a real provisioned board. + +core/keystore.c's compiled-in default_dev_key is described as this same key +but is not: it differs from byte 21 on and does not decode to a point on the +curve, so no signature can verify against it. That is a defect in its own +right and is not what this generator works around -- the OTP route is used +because it is the production path, not because the fallback is broken. + +Each header prefix below must be byte-identical to what the C test builds; +the field values are copied from the tests, and the layout is the one +tests/unit/test_image_header_abi.c pins. + + python3 tools/gen_fw_update_test_sigs.py > tests/vectors/fw_update_test_sigs.h +""" + +import hashlib +import struct +import sys + +from cryptography.hazmat.primitives import serialization +from cryptography.hazmat.primitives.asymmetric.ed25519 import Ed25519PrivateKey + +# RFC 8032 section 7.1, TEST 1. +RFC8032_TEST1_SECRET = bytes.fromhex( + "9d61b19deffd5a60ba844af492ec2cc44449c5697b326919703bac031cae7f60") + +EOS_IMG_MAGIC = 0x454F5349 +EOS_IMAGE_HDR_VERSION = 2 +EOS_IMG_STRUCT_SIZE = 156 +EOS_IMG_SIGNED_LEN = 92 +EOS_IMG_FLAG_HASH_SHA256 = 1 << 6 +EOS_SIG_ED25519 = 3 +EOS_SIG_MAX_SIZE = 64 +EOS_IMG_TLV_HASH_LEN = 28 +EOS_TLV_INFO_MAGIC = 0x6907 +EOS_TLV_MIN_SEC_VER = 0x50 + + +def crc32_payload(data: bytes) -> int: + """update_crc() in core/fw_update.c, as test_fw_transport.c mirrors it.""" + crc = 0xFFFFFFFF + for b in data: + crc ^= b + for _ in range(8): + crc = (crc >> 1) ^ 0xEDB88320 if crc & 1 else crc >> 1 + return (~crc) & 0xFFFFFFFF + + +def tlv_area(sec_ver: int) -> bytes: + """[tlv_info(4)][entry_hdr(4)][uint32 value] -- the shape both suites build.""" + total = 4 + 4 + 4 + return (struct.pack(" bytes: + # build_image() in tests/unit/test_fw_update.c + payload = bytes((i * 7 + 1) & 0xFF for i in range(256)) + return signed_prefix(image_size=256, load_addr=0, entry_addr=0, + version=0x00010000, flags=EOS_IMG_FLAG_HASH_SHA256, + hash32=hashlib.sha256(payload).digest(), + tlv=tlv_area(sec_ver)) + + +def fw_transport_prefix() -> bytes: + # build_container() in tests/unit/test_fw_transport.c: CRC32 integrity + # path (flags = 0), the CRC in the first four bytes of hash[]. + payload = bytes(0x5A + (i & 0x1F) for i in range(256)) + hash32 = struct.pack(" str: + lines = [] + for i in range(0, len(b), 16): + lines.append(indent + ",".join("0x%02x" % x for x in b[i:i + 16]) + ",") + return "\n".join(lines) + + +def main() -> int: + key = Ed25519PrivateKey.from_private_bytes(RFC8032_TEST1_SECRET) + pub = key.public_key().public_bytes(serialization.Encoding.Raw, + serialization.PublicFormat.Raw) + + vectors = [ + ("fw_update_sec_ver_3", fw_update_prefix(3), + "test_fw_update.c build_image(out, 3)"), + ("fw_update_sec_ver_9", fw_update_prefix(9), + "test_fw_update.c build_image(out, 9)"), + ("fw_transport_container", fw_transport_prefix(), + "test_fw_transport.c build_container()"), + ] + + out = [] + out.append("/* Generated by tools/gen_fw_update_test_sigs.py -- do not edit.") + out.append(" *") + out.append(" * Ed25519 signatures over the 92-byte signed header prefix of the") + out.append(" * images tests/unit/test_fw_update.c and test_fw_transport.c build,") + out.append(" * under the RFC 8032 section 7.1 TEST 1 key. Regenerate with:") + out.append(" *") + out.append(" * python3 tools/gen_fw_update_test_sigs.py > tests/vectors/fw_update_test_sigs.h") + out.append(" */") + out.append("#ifndef EOS_FW_UPDATE_TEST_SIGS_H") + out.append("#define EOS_FW_UPDATE_TEST_SIGS_H") + out.append("") + out.append("/* The public half. Tests serve it from simulated OTP slot 0 so the") + out.append(" * keystore selects it the way a provisioned board would. */") + out.append("static const unsigned char eos_test_sig_pubkey[32] = {") + out.append(carr(pub)) + out.append("};") + for name, prefix, origin in vectors: + sig = key.sign(prefix) + out.append("") + out.append("/* %s */" % origin) + out.append("static const unsigned char eos_test_sig_%s[64] = {" % name) + out.append(carr(sig)) + out.append("};") + out.append("") + out.append("#endif /* EOS_FW_UPDATE_TEST_SIGS_H */") + sys.stdout.write("\n".join(out) + "\n") + return 0 + + +if __name__ == "__main__": + sys.exit(main()) From 2b30fd74eaa0399bc6fac22f89203b0cf438b94e Mon Sep 17 00:00:00 2001 From: kartikey1306 Date: Fri, 11 Sep 2026 16:30:05 +0530 Subject: [PATCH 02/23] test(imgpack): pass the version as --version=X so argparse never sees -1.0.0 as an option test_out_of_range_version_is_rejected[-1.0.0] passed locally (Python 3.14) and failed in CI (ubuntu-22.04, Python 3.10) with argparse's own "expected one argument": the older negative-number matcher does not accept -1.0.0, so the token was read as an unknown option and imgpack.py's range check -- the thing under test -- never ran. The joined form is unambiguous on every interpreter and the test now reaches the tool's message. --- tests/unit/test_imgpack.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/tests/unit/test_imgpack.py b/tests/unit/test_imgpack.py index dc0fc32..7cee682 100644 --- a/tests/unit/test_imgpack.py +++ b/tests/unit/test_imgpack.py @@ -32,7 +32,13 @@ def _pack(tmp_path, version): "--input", str(tmp_path / "fw.bin"), "--output", str(tmp_path / "fw.eimg"), "--load-addr", "0x08010000", "--entry-addr", "0x08010100", - "--version", version], + # `--version=X`, not `--version X`: a value such as "-1.0.0" is a + # separate token in the second form, and whether argparse reads it + # as a value or as an unknown option depends on the interpreter -- + # 3.10 refuses it with "expected one argument" before imgpack.py + # runs, 3.13+ accepts it. The test is about imgpack's own check, + # which only the joined form reaches on every version. + f"--version={version}"], capture_output=True, text=True) From a95400f275b857cf77a60abcb00a2ad3186d60c0 Mon Sep 17 00:00:00 2001 From: kartikey1306 Date: Mon, 14 Sep 2026 19:00:03 +0530 Subject: [PATCH 03/23] test(fw_update): pin the fixture header to its generator tests/vectors/fw_update_test_sigs.h is the output of tools/gen_fw_update_test_sigs.py, committed because eBoot has no Ed25519 signer in C. Nothing checked that the two agree: a generator edit without a regeneration leaves test_fw_update and test_fw_transport verifying against stale signatures, failing with EOS_ERR_SIGNATURE and nothing to say why. tests/unit/test_fw_update_test_sigs.py runs the generator with the test's own interpreter and compares its stdout to the committed header byte for byte, so a line-ending change counts too. It follows the same dependency rule as test_eos_sign_payload_offset.py: with EOS_REQUIRE_SIGNING_TESTS set (the CI workflow sets it before the pytest step) a missing cryptography module fails the job instead of skipping. Negative control: one flipped hex byte in the header fails the test with a unified diff naming the line. build_image() and build_container() now carry a comment naming the coupling: the signed prefix is assembled both there and in the generator, and changing any field in it means changing the generator's copy and regenerating the header. --- tests/unit/test_fw_transport.c | 7 +++ tests/unit/test_fw_update.c | 7 +++ tests/unit/test_fw_update_test_sigs.py | 61 ++++++++++++++++++++++++++ 3 files changed, 75 insertions(+) create mode 100644 tests/unit/test_fw_update_test_sigs.py diff --git a/tests/unit/test_fw_transport.c b/tests/unit/test_fw_transport.c index 6d052e5..92d51d3 100644 --- a/tests/unit/test_fw_transport.c +++ b/tests/unit/test_fw_transport.c @@ -344,6 +344,13 @@ static uint32_t crc32_payload(const uint8_t *data, size_t len) return ~crc; } +/* Coupling: the signed prefix (the first EOS_IMG_SIGNED_LEN = 92 bytes of + * the header, plus the TLV area whose hash sits inside it) is built here + * AND in tools/gen_fw_update_test_sigs.py, which signs it. Changing any + * field in it means changing the generator's copy too and re-running + * python3 tools/gen_fw_update_test_sigs.py > tests/vectors/fw_update_test_sigs.h + * tests/unit/test_fw_update_test_sigs.py fails until the committed header + * is regenerated from the generator. */ static void build_container(void) { eos_image_header_t hdr; diff --git a/tests/unit/test_fw_update.c b/tests/unit/test_fw_update.c index ae87c91..f6a3129 100644 --- a/tests/unit/test_fw_update.c +++ b/tests/unit/test_fw_update.c @@ -139,6 +139,13 @@ static void fill_payload(uint8_t *payload) payload[i] = (uint8_t)(i * 7u + 1u); } +/* Coupling: the signed prefix (the first EOS_IMG_SIGNED_LEN = 92 bytes of + * the header, plus the TLV area whose hash sits inside it) is built here + * AND in tools/gen_fw_update_test_sigs.py, which signs it. Changing any + * field in it means changing the generator's copy too and re-running + * python3 tools/gen_fw_update_test_sigs.py > tests/vectors/fw_update_test_sigs.h + * tests/unit/test_fw_update_test_sigs.py fails until the committed header + * is regenerated from the generator. */ static void build_image(uint8_t *out, uint32_t sec_ver) { uint8_t payload[PAYLOAD_SIZE]; diff --git a/tests/unit/test_fw_update_test_sigs.py b/tests/unit/test_fw_update_test_sigs.py new file mode 100644 index 0000000..4db5c9d --- /dev/null +++ b/tests/unit/test_fw_update_test_sigs.py @@ -0,0 +1,61 @@ +# SPDX-License-Identifier: MIT +# Copyright (c) 2026 EoS Project + +"""tests/vectors/fw_update_test_sigs.h must be what its generator emits. + +tools/gen_fw_update_test_sigs.py signs the header prefixes that +tests/unit/test_fw_update.c build_image() and tests/unit/test_fw_transport.c +build_container() assemble, and the C suites include the committed header. +If the generator changes and the header is not regenerated, the C suites +verify against stale signatures and fail with EOS_ERR_SIGNATURE, and nothing +says why. This pins the committed header to the generator's output, byte for +byte, so line endings count too. +""" + +import difflib +import os +import subprocess +import sys +from pathlib import Path + +import pytest + +REPO_ROOT = Path(__file__).resolve().parents[2] +TOOLS = REPO_ROOT / "tools" + +# A skip is right for a developer without the signing dependency installed and +# wrong for CI, where "collected 19 tests, ran 0" is a green run that checked +# nothing -- the failure .ai/security.md names directly. EOS_REQUIRE_SIGNING_TESTS +# is set in the workflow, so there a missing dependency is a hard error; locally +# the skip still applies. +if os.environ.get("EOS_REQUIRE_SIGNING_TESTS"): + import cryptography # noqa: F401 -- ImportError here must fail the job +else: + pytest.importorskip( + "cryptography", reason="signing tools require 'cryptography'") + +GENERATOR = TOOLS / "gen_fw_update_test_sigs.py" +HEADER = REPO_ROOT / "tests" / "vectors" / "fw_update_test_sigs.h" +REGENERATE = ("python3 tools/gen_fw_update_test_sigs.py " + "> tests/vectors/fw_update_test_sigs.h") + + +def test_committed_header_is_the_generator_output(): + result = subprocess.run( + [sys.executable, str(GENERATOR)], + cwd=REPO_ROOT, capture_output=True, check=True, + ) + expected = result.stdout + actual = HEADER.read_bytes() + + if actual != expected: + diff = "".join(difflib.unified_diff( + expected.decode("ascii", "replace").splitlines(keepends=True), + actual.decode("ascii", "replace").splitlines(keepends=True), + fromfile="tools/gen_fw_update_test_sigs.py (stdout)", + tofile="tests/vectors/fw_update_test_sigs.h (committed)", + )) + pytest.fail( + "tests/vectors/fw_update_test_sigs.h differs from what " + "tools/gen_fw_update_test_sigs.py emits; regenerate it with\n" + " " + REGENERATE + "\n" + diff) From edfbaed3145d2b839a47d288a8167028525db136 Mon Sep 17 00:00:00 2001 From: kartikey1306 Date: Mon, 14 Sep 2026 19:00:03 +0530 Subject: [PATCH 04/23] docs: record the install-path check ordering as ADR-020 and log the repair #103 and #104 were both merged and disagree on whether the anti-rollback counter or the signature is checked first in eos_fw_update_finalize(). The master design orders boot as verify image, then version policy (section 8.1), but its update flow (section 15) never places the anti-rollback check, so the order the install path uses existed only in a PR body. ADR-020 records it: the signature over the signed header prefix is verified first, the TLV counter is read only after the prefix that binds it is authenticated, and an image that fails verification is refused as EOS_ERR_SIGNATURE without its counter being consulted. docs/adr/README.md is added in the shape of the eos repository's index; 020 avoids reusing 001 through 019. CHANGELOG.md gains the Unreleased entries for the repair: the configure, compile and link breakage after the 09-07 batch merge, the settled check ordering with the suites streaming signed images, the re-derived Valgrind list, fuzz-build in the CI gate, counted tests_run, the EoSim Windows legs running under bash, and the Scorecard action on its ghcr.io-hosted release. --- CHANGELOG.md | 7 +++ ...verifies-signature-before-anti-rollback.md | 58 +++++++++++++++++++ docs/adr/README.md | 22 +++++++ 3 files changed, 87 insertions(+) create mode 100644 docs/adr/ADR-020-install-path-verifies-signature-before-anti-rollback.md create mode 100644 docs/adr/README.md diff --git a/CHANGELOG.md b/CHANGELOG.md index 2fb33e3..2ff7953 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,13 @@ - **`image_verify.c`:** `eos_image_verify_integrity` rejects a zero `image_size`, and an `addr + hdr_size` that wraps `uint32_t`, instead of computing a payload address that is not the payload. ### Fixed +- **The tree did not configure, compile or link after the 09-07 batch merge.** `tests/CMakeLists.txt` registered `eboot_test_fdt_loader` twice; `core/sha512.c` had been replaced by a version predating the `bitlen[2]`/`buffer_len` context; `core/boot_log.c`, `core/secure_boot.c` and `core/fdt_loader.c` had been dropped from `eboot_core`; `scalarbase()` and `k_low_order[]` were defined twice; and the `eos_boot_log_get_head()` declaration was lost. All restored. +- **Install-path verification order settled: signature before anti-rollback.** `eos_fw_update_finalize()` verifies the Ed25519 signature over the signed header prefix first and reads the TLV security counter only after the prefix that binds it is authenticated (see `docs/adr/ADR-020`). The `fw_update` and `fw_transport` suites now stream genuinely signed images; `tools/gen_fw_update_test_sigs.py` emits their signatures as `tests/vectors/fw_update_test_sigs.h`, and `tests/unit/test_fw_update_test_sigs.py` pins the committed header to the generator's output. +- **`tests/CMakeLists.txt`:** the Valgrind list is derived from the registered suites again; a hand-written copy had replaced it and eleven suites had no Valgrind run. They are re-added. +- **`.github/workflows/ci.yml`:** `fuzz-build` is in the CI gate. It was added after the gate job and the gate never waited for it. +- **Unit suites count `tests_run`** as each test executes instead of assigning it a literal that the summary line then trusted. +- **`.github/workflows/eosim-sanity.yml`:** the install-validate job's steps are bash and now run under `shell: bash` on the Windows legs, where PowerShell rejected `SITE_PACKAGES=$(...)` and parsed `|| { exit 1 }` as an unexecuted script block. +- **`.github/workflows/scorecard.yml`:** `ossf/scorecard-action` moved to v2.4.3, the release hosted on ghcr.io; v2.4.0 pulls from gcr.io, which now requires GCP billing. - **The tree did not compile.** `include/eos_image.h` declared `eos_crc32()` as `int eos_crc32(uint32_t, size_t, uint32_t *)` while `core/image_verify.c` defined it as `uint32_t eos_crc32(uint32_t, size_t)` -- a conflicting-types error that stopped the build at the first core source file. The declaration now matches the definition and the documented behaviour. - **`ed25519_verify.c`:** `eos_ed25519_verify()` never performed the verification. Two merged copies of the challenge-hash step had been left in the function, the second referring to identifiers that do not exist (`sha512_ctx_t`, `sc_reduce`), and RFC 8032 step 4 -- the `[S]B == R + [k]A` check -- was absent entirely, leaving the function returning an undeclared `diff`. The duplicate is removed and the group-equation check restored; the function now passes the RFC 8032 test vectors and rejects tampered messages, every single-bit signature flip, wrong keys and malleated signatures. - **`recovery.c`:** `recovery_handle_write()` declared `slot_size` twice, which does not compile. The bounds check now calls `eos_recovery_write_in_range()` -- the helper the unit tests already exercise -- so the wire-input rule has one definition, and an unmapped slot (`base == 0`) is rejected too. diff --git a/docs/adr/ADR-020-install-path-verifies-signature-before-anti-rollback.md b/docs/adr/ADR-020-install-path-verifies-signature-before-anti-rollback.md new file mode 100644 index 0000000..5bf8b58 --- /dev/null +++ b/docs/adr/ADR-020-install-path-verifies-signature-before-anti-rollback.md @@ -0,0 +1,58 @@ +--- +adr: 20 +title: Install path verifies the signature before anti-rollback +status: Proposed +date: 2026-09-14 +deciders: Architecture Council, eBoot maintainers +source: EmbeddedOS Master Design v2.0 §8.1, §15; ADR-011 (eOTA firmware/update contract) +--- + +# ADR-020 — Install path verifies the signature before anti-rollback + +## Context + +Two pull requests merged on 09-07 both change `eos_fw_update_finalize()` and disagree on +the order of its checks: + +- #103 added an authenticated TLV anti-rollback counter: the image's security counter is + read from its TLV area and compared against the persistent floor at install. +- #104 made the install path verify the image signature unconditionally, no longer gated + on the header's own `sig_type`. + +Neither PR was written against a base that contained the other, so neither says which +check comes first. The master design orders BOOT as Verify Manifest → Verify Image → +Check Version Policy (§8.1). Its update flow (§15) is Download → Verify → Install and +never places the anti-rollback check. The ordering the install path uses therefore +existed only in a PR body. + +## Decision + +In the install path (`core/fw_update.c`, `eos_fw_update_finalize()`): + +1. Signature verification over the signed header prefix (`EOS_IMG_SIGNED_LEN`, the first + 92 bytes of the header) precedes anti-rollback evaluation. +2. The TLV security counter is read only after the prefix that binds it (`tlv_len`, + `tlv_hash`) has been authenticated. +3. An image that fails signature verification is refused as `EOS_ERR_SIGNATURE` and its + counter is never consulted. + +This mirrors the boot ordering of §8.1: authenticate first, then apply version policy to +what was authenticated. + +## Consequences + +- A test that wants to observe the anti-rollback stage must present a genuinely signed + image. eBoot has no Ed25519 signer in C, so `tools/gen_fw_update_test_sigs.py` signs + the header prefixes that `tests/unit/test_fw_update.c` and + `tests/unit/test_fw_transport.c` build and emits `tests/vectors/fw_update_test_sigs.h`; + `tests/unit/test_fw_update_test_sigs.py` pins the committed header to the generator's + output. +- An unsigned image cannot demonstrate a rollback regression. It is refused as + `EOS_ERR_SIGNATURE` before its counter is compared, so a rollback test built on an + unsigned image exercises the signature check, not the floor. + +## Note on numbering + +ADR-001 through ADR-011 belong to the master-design series and are not in this +repository; `eos` holds ADR-012 through ADR-019. This record is numbered 020 so that no +number is reused. It extends ADR-011, the eOTA firmware/update contract. diff --git a/docs/adr/README.md b/docs/adr/README.md new file mode 100644 index 0000000..bc72656 --- /dev/null +++ b/docs/adr/README.md @@ -0,0 +1,22 @@ +# Architecture Decision Records + +One file per decision. A record is never edited after it reaches **Accepted** — it is +superseded by a later record that names it. + +| Status | Meaning | +|---|---| +| Proposed | Written, not yet ratified by the maintainers named in `deciders`. | +| Accepted | Ratified. Binding on new code. | +| Superseded | Replaced; the replacing ADR is named in the header. | + +## Index + +| ADR | Title | Status | +|---|---|---| +| 020 | [Install path verifies the signature before anti-rollback](ADR-020-install-path-verifies-signature-before-anti-rollback.md) | Proposed | + +## Note on numbering + +ADR-001 through ADR-011 belong to the master-design series and are not in this +repository. The `eos` repository holds ADR-012 through ADR-019. This set starts at 020 so +that no number is reused; ADR-020 extends ADR-011, the eOTA firmware/update contract. From b01e9ced90c667c90cdac3ead76e65366c5967e1 Mon Sep 17 00:00:00 2001 From: kartikey1306 Date: Tue, 15 Sep 2026 00:51:09 +0530 Subject: [PATCH 05/23] docs: state the Valgrind gap and the ADR's provenance exactly The changelog said eleven suites had no Valgrind run. Eleven were missing from EBLDR_UNIT_TESTS, but four of those were named in the hand-written foreach and did run; seven had no run at all. Say which. ADR-020: its design-document citation now says where it comes from (the architecture review of #115), and the sentence about the two PRs' bases is replaced with what the history shows -- #103's commits predate #104's merge, and #104 was written without #103's check in place. --- CHANGELOG.md | 2 +- ...-install-path-verifies-signature-before-anti-rollback.md | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2ff7953..ad06fbb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,7 +15,7 @@ ### Fixed - **The tree did not configure, compile or link after the 09-07 batch merge.** `tests/CMakeLists.txt` registered `eboot_test_fdt_loader` twice; `core/sha512.c` had been replaced by a version predating the `bitlen[2]`/`buffer_len` context; `core/boot_log.c`, `core/secure_boot.c` and `core/fdt_loader.c` had been dropped from `eboot_core`; `scalarbase()` and `k_low_order[]` were defined twice; and the `eos_boot_log_get_head()` declaration was lost. All restored. - **Install-path verification order settled: signature before anti-rollback.** `eos_fw_update_finalize()` verifies the Ed25519 signature over the signed header prefix first and reads the TLV security counter only after the prefix that binds it is authenticated (see `docs/adr/ADR-020`). The `fw_update` and `fw_transport` suites now stream genuinely signed images; `tools/gen_fw_update_test_sigs.py` emits their signatures as `tests/vectors/fw_update_test_sigs.h`, and `tests/unit/test_fw_update_test_sigs.py` pins the committed header to the generator's output. -- **`tests/CMakeLists.txt`:** the Valgrind list is derived from the registered suites again; a hand-written copy had replaced it and eleven suites had no Valgrind run. They are re-added. +- **`tests/CMakeLists.txt`:** the Valgrind list is derived from the registered suites again; a hand-written copy had replaced it, eleven registered suites were missing from `EBLDR_UNIT_TESTS`, and seven of those (`test_eos_sign_boot_path`, `test_fdt_loader`, `test_fw_decrypt`, `test_fw_update_sig`, `test_jump_app_bounds`, `test_qemu_arm64_timer`, `test_secure_boot_policy`) had no Valgrind run at all; the other four were only in the hand-written list. All eleven are appended. - **`.github/workflows/ci.yml`:** `fuzz-build` is in the CI gate. It was added after the gate job and the gate never waited for it. - **Unit suites count `tests_run`** as each test executes instead of assigning it a literal that the summary line then trusted. - **`.github/workflows/eosim-sanity.yml`:** the install-validate job's steps are bash and now run under `shell: bash` on the Windows legs, where PowerShell rejected `SITE_PACKAGES=$(...)` and parsed `|| { exit 1 }` as an unexecuted script block. diff --git a/docs/adr/ADR-020-install-path-verifies-signature-before-anti-rollback.md b/docs/adr/ADR-020-install-path-verifies-signature-before-anti-rollback.md index 5bf8b58..cf501df 100644 --- a/docs/adr/ADR-020-install-path-verifies-signature-before-anti-rollback.md +++ b/docs/adr/ADR-020-install-path-verifies-signature-before-anti-rollback.md @@ -4,7 +4,7 @@ title: Install path verifies the signature before anti-rollback status: Proposed date: 2026-09-14 deciders: Architecture Council, eBoot maintainers -source: EmbeddedOS Master Design v2.0 §8.1, §15; ADR-011 (eOTA firmware/update contract) +source: EmbeddedOS Master Design v2.0 §8.1 (boot order) and §15 (update flow), as cited in the architecture review of #115; ADR-011 (eOTA firmware/update contract) --- # ADR-020 — Install path verifies the signature before anti-rollback @@ -19,8 +19,8 @@ the order of its checks: - #104 made the install path verify the image signature unconditionally, no longer gated on the header's own `sig_type`. -Neither PR was written against a base that contained the other, so neither says which -check comes first. The master design orders BOOT as Verify Manifest → Verify Image → +#103's authored commits all predate #104's merge, and #104 was written without #103's +counter check in place, so neither says which check comes first. The master design orders BOOT as Verify Manifest → Verify Image → Check Version Policy (§8.1). Its update flow (§15) is Download → Verify → Install and never places the anti-rollback check. The ordering the install path uses therefore existed only in a PR body. From e152d8edfce4a6dd9d9f4e8748e4938062926620 Mon Sep 17 00:00:00 2001 From: kartikey1306 Date: Tue, 15 Sep 2026 01:31:51 +0530 Subject: [PATCH 06/23] 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 547ab9faeb201ce49122051d1fc672bea8e7dc24 Mon Sep 17 00:00:00 2001 From: kartikey1306 Date: Fri, 11 Sep 2026 16:26:08 +0530 Subject: [PATCH 07/23] fix(keystore): the compiled-in dev trust anchor was not a point on the curve core/keystore.c's default_dev_key claims, in a #warning and in comments, to be the RFC 8032 section 7.1 TEST 1 public key. From v0.1.0 it agreed with the RFC for 21 bytes and then did not, and the bytes it held do not decode to a point on edwards25519. No signature could ever verify against it, on any board that fell back to it; after #104 made signature verification unconditional at install, that meant firmware update refused every image on every board without OTP. Nothing noticed because no test ever asked the fallback key to verify anything. The array is now the RFC's 32 bytes. test_keystore.c asks the compiled-in anchor to verify the RFC's own TEST 1 signature over the empty message, and then shows the accept discriminates (wrong message, flipped bit). On the old bytes the first verify returns EOS_ERR_SIGNATURE. Also corrects the note in test_secure_boot_policy.c that repeated the claim. --- core/keystore.c | 9 +++-- tests/unit/test_keystore.c | 53 ++++++++++++++++++++++++++++ tests/unit/test_secure_boot_policy.c | 5 ++- 3 files changed, 64 insertions(+), 3 deletions(-) diff --git a/core/keystore.c b/core/keystore.c index a86654d..6e16600 100644 --- a/core/keystore.c +++ b/core/keystore.c @@ -27,11 +27,16 @@ * build that embeds it says so on every compile. */ #ifndef EBLDR_PRODUCTION_KEY #warning "eBoot: building with the RFC 8032 test-vector public key as the secure-boot trust anchor; define EBLDR_PRODUCTION_KEY for any real device" +/* RFC 8032 section 7.1, TEST 1, PUBLIC KEY -- all 32 bytes of it. The array + * shipped from v0.1.0 to here agreed with the RFC for 21 bytes and then did + * not, and the result was not a point on the curve: no signature could ever + * verify against it, on any board that fell back to it. See + * tests/unit/test_keystore.c::test_compiled_in_anchor_verifies_its_own_rfc_vector. */ static const uint8_t default_dev_key[EOS_ED25519_PUB_KEY_SIZE] = { 0xd7, 0x5a, 0x98, 0x01, 0x82, 0xb1, 0x0a, 0xb7, 0xd5, 0x4b, 0xfe, 0xd3, 0xc9, 0x64, 0x07, 0x3a, - 0x0e, 0xe1, 0x72, 0xf3, 0xda, 0xa3, 0xf4, 0xa1, - 0x8c, 0x42, 0xc4, 0x76, 0x84, 0x37, 0x77, 0x25, + 0x0e, 0xe1, 0x72, 0xf3, 0xda, 0xa6, 0x23, 0x25, + 0xaf, 0x02, 0x1a, 0x68, 0xf7, 0x07, 0x51, 0x1a, }; #endif diff --git a/tests/unit/test_keystore.c b/tests/unit/test_keystore.c index 1e1960d..fcc64d5 100644 --- a/tests/unit/test_keystore.c +++ b/tests/unit/test_keystore.c @@ -8,6 +8,7 @@ */ #include "eos_keystore.h" +#include "eos_crypto_boot.h" #include "eos_hal.h" #include #include @@ -263,6 +264,57 @@ TEST(test_revoke_reports_a_failed_persist) otp_detach(); } +/* The compiled-in anchor claims, in a #warning and in comments, to be the + * RFC 8032 section 7.1 TEST 1 public key. That claim is what makes it + * usable for development at all: the matching secret is printed in the RFC, + * so anyone can sign a test image for a board that falls back to it. + * + * From v0.1.0 the array agreed with the RFC for 21 bytes and then diverged, + * and the bytes it held did not decode to a point on the curve. Every + * signature check against the fallback failed, and after #104 made signature + * verification unconditional at install, firmware update refused every + * image on every board without OTP. Nothing noticed because no test ever + * asked the fallback key to verify anything. + * + * This asks. The vector is RFC 8032 TEST 1 itself: empty message, and the + * signature the RFC prints for it. On the old bytes eos_ed25519_verify() + * returns EOS_ERR_SIGNATURE; the two negative checks after it show the + * accept is discriminating, not a verifier that says yes to everything. */ +TEST(test_compiled_in_anchor_verifies_its_own_rfc_vector) +{ + static const uint8_t rfc8032_test1_sig[64] = { + 0xe5,0x56,0x43,0x00,0xc3,0x60,0xac,0x72,0x90,0x86,0xe2,0xcc,0x80,0x6e,0x82,0x8a, + 0x84,0x87,0x7f,0x1e,0xb8,0xe5,0xd9,0x74,0xd8,0x73,0xe0,0x65,0x22,0x49,0x01,0x55, + 0x5f,0xb8,0x82,0x15,0x90,0xa3,0x3b,0xac,0xc6,0x1e,0x39,0x70,0x1c,0xf9,0xb4,0x6b, + 0xd2,0x5b,0xf5,0xf0,0x59,0x5b,0xbe,0x24,0x65,0x51,0x41,0x43,0x8e,0x7a,0x10,0x0b, + }; + static const uint8_t rfc8032_test1_pub[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, + }; + const uint8_t *key = NULL; + size_t key_len = 0; + eos_keystore_t ks; + + otp_detach(); /* no OTP at all: the compiled-in path */ + ASSERT(eos_keystore_init(&ks) == EOS_OK); + ASSERT(eos_keystore_get_active_key(&ks, &key, &key_len) == EOS_OK); + ASSERT(key_len == 32); + + /* The bytes are the RFC's bytes, and they verify the RFC's signature. */ + ASSERT(memcmp(key, rfc8032_test1_pub, 32) == 0); + ASSERT(eos_ed25519_verify(rfc8032_test1_sig, key, NULL, 0) == EOS_OK); + + /* Discrimination: the same signature must not verify a different message, + * and a bit-flipped signature must not verify the empty one. */ + const uint8_t other[1] = { 'x' }; + ASSERT(eos_ed25519_verify(rfc8032_test1_sig, key, other, 1) != EOS_OK); + uint8_t flipped[64]; + memcpy(flipped, rfc8032_test1_sig, 64); + flipped[0] ^= 0x01; + ASSERT(eos_ed25519_verify(flipped, key, NULL, 0) != EOS_OK); +} + int main(void) { printf("=== eBootloader: Keystore Unit Tests ===\n\n"); @@ -276,6 +328,7 @@ int main(void) run_test_failed_otp_read_does_not_fall_back_to_the_compiled_key(); run_test_revocation_is_persisted_without_clobbering_other_slots(); run_test_revoke_reports_a_failed_persist(); + run_test_compiled_in_anchor_verifies_its_own_rfc_vector(); printf("\n%d/%d tests passed\n", tests_passed, tests_run); return (tests_passed == tests_run) ? 0 : 1; diff --git a/tests/unit/test_secure_boot_policy.c b/tests/unit/test_secure_boot_policy.c index 39bc2c2..877a603 100644 --- a/tests/unit/test_secure_boot_policy.c +++ b/tests/unit/test_secure_boot_policy.c @@ -242,7 +242,10 @@ TEST(test_secure_boot_proceeds_when_the_debug_lock_succeeds) * over the header prefix checked against the keystore anchor. A fixture for * that is buildable -- the keystore ships RFC 8032 TEST 1's public key and * the matching private key is in the RFC -- but the machinery for it belongs - * to #88 (tools/gen_signed_image_fixture.py), not here. + * to #88 (tools/gen_signed_image_fixture.py), not here. (When this note was + * first written the shipped key was *not* that key -- it was off the curve + * from byte 21 on, so no such fixture could have verified; that is fixed in + * core/keystore.c and pinned by test_keystore.c.) * * I wrote the obvious test first and it was worthless: with * require_signature = true and an unsigned fixture the boot fails at step 3, From ef5b70d0f702b65ddaec343383b2b0ed91a2ec3a Mon Sep 17 00:00:00 2001 From: kartikey1306 Date: Mon, 14 Sep 2026 19:15:56 +0530 Subject: [PATCH 08/23] test(keystore): compare the anchor against the generated fixture key, and record the behaviour change The unit test that checks the compiled-in development trust anchor carried its own hand-typed copy of the RFC 8032 TEST 1 public key, the third such copy in the tree. tests/vectors/fw_update_test_sigs.h already holds that key as eos_test_sig_pubkey: it is derived from the RFC secret by tools/gen_fw_update_test_sigs.py and pinned to the generator by tests/unit/test_fw_update_test_sigs.py. The test now includes the fixture and compares against it, so the anchor is checked against a second, independently derived copy of the key instead of bytes that could have been mistyped the same way. The signature constant and the four assertions are unchanged; on the old anchor bytes the memcmp still fails. The CHANGELOG gains a Security entry for the anchor fix and states the consequence plainly: a board with no OTP and no EBLDR_PRODUCTION_KEY moves from refusing every image, as it has since #104, to accepting images signed with the public RFC test key. That is what the #warning in core/keystore.c has always said a development build does, and the key must never reach a device; #120 tracks making that structural. --- CHANGELOG.md | 1 + tests/unit/test_keystore.c | 16 ++++++++++------ 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ad06fbb..f3b12a6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ ## [Unreleased] ### Security +- **The compiled-in development trust anchor was not a point on the curve.** `core/keystore.c` `default_dev_key` was described as the RFC 8032 section 7.1 TEST 1 public key, but it diverged from that key at byte 21 and the bytes it held did not decode to a point on the curve, so no signature could ever verify against it. It is now the RFC key, and `tests/unit/test_keystore.c` checks that the anchor verifies the RFC's own signature. Behaviour change: a board with no OTP and no `EBLDR_PRODUCTION_KEY` goes from refusing every image (since #104 made signature verification unconditional at install) to accepting images signed with the public RFC test key -- the documented development intent of the `#warning` in `core/keystore.c`, and a key that must never reach a device. #120 tracks making that structural. - **Image header is now authenticated (header format v2).** `eos_image_verify_signature()` signed `hdr->hash` only — 32 of the header's 156 bytes. Everything else (`image_size`, `load_addr`, `entry_addr`, `flags`, `sig_type`, `image_version`) sat outside the signature, so an attacker holding a legitimately signed image could relocate it, move its entry point, or clear `EOS_IMG_FLAG_HASH_SHA256` to downgrade integrity checking from SHA-256 to forgeable CRC32 — all while keeping the signature valid. The signature now covers `EOS_IMG_SIGNED_LEN` (92) bytes: the whole header except `signature[]` itself. **Existing signed images must be re-signed.** - **`eos_image_parse_header`:** validates `hdr_version`, rejecting 0 and anything newer than this build understands. - **`tools/eos_sign.py`:** `SIG_TYPE_ED25519` was `1` — that is `EOS_SIG_CRC32` in `eos_types.h`, which `eos_image_verify_signature()` rejects outright — and `IMG_FLAG_SIGNED` was `1 << 2`, which is `EOS_IMG_FLAG_DEBUG`. It also never set `EOS_IMG_FLAG_HASH_SHA256`, so the bootloader read the stored SHA-256 as a CRC32. Constants now match `include/eos_types.h`. diff --git a/tests/unit/test_keystore.c b/tests/unit/test_keystore.c index fcc64d5..7d06aec 100644 --- a/tests/unit/test_keystore.c +++ b/tests/unit/test_keystore.c @@ -10,6 +10,7 @@ #include "eos_keystore.h" #include "eos_crypto_boot.h" #include "eos_hal.h" +#include "../vectors/fw_update_test_sigs.h" #include #include #include @@ -288,10 +289,6 @@ TEST(test_compiled_in_anchor_verifies_its_own_rfc_vector) 0x5f,0xb8,0x82,0x15,0x90,0xa3,0x3b,0xac,0xc6,0x1e,0x39,0x70,0x1c,0xf9,0xb4,0x6b, 0xd2,0x5b,0xf5,0xf0,0x59,0x5b,0xbe,0x24,0x65,0x51,0x41,0x43,0x8e,0x7a,0x10,0x0b, }; - static const uint8_t rfc8032_test1_pub[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, - }; const uint8_t *key = NULL; size_t key_len = 0; eos_keystore_t ks; @@ -301,8 +298,15 @@ TEST(test_compiled_in_anchor_verifies_its_own_rfc_vector) ASSERT(eos_keystore_get_active_key(&ks, &key, &key_len) == EOS_OK); ASSERT(key_len == 32); - /* The bytes are the RFC's bytes, and they verify the RFC's signature. */ - ASSERT(memcmp(key, rfc8032_test1_pub, 32) == 0); + /* The bytes are the RFC's bytes, and they verify the RFC's signature. + * + * The expected key is eos_test_sig_pubkey from the generated fixture + * header, not a third hand-typed copy. That public key is derived from + * the RFC 8032 secret by tools/gen_fw_update_test_sigs.py and pinned to + * that script by tests/unit/test_fw_update_test_sigs.py, so this compares + * the compiled-in anchor against a second, independently derived copy of + * the same key. */ + ASSERT(memcmp(key, eos_test_sig_pubkey, 32) == 0); ASSERT(eos_ed25519_verify(rfc8032_test1_sig, key, NULL, 0) == EOS_OK); /* Discrimination: the same signature must not verify a different message, From f8afc12b01641a23a8713321dc02f841b7dd588e Mon Sep 17 00:00:00 2001 From: kartikey1306 Date: Tue, 15 Sep 2026 01:10:53 +0530 Subject: [PATCH 09/23] build: a Release build of a real board refuses to configure without a production trust anchor core/keystore.c falls back to a compiled-in public key when the board has no OTP -- and no board under boards/ implements otp_read, so on every shipped board the compiled-in key is the trust anchor. EBLDR_PRODUCTION_KEY had no CMake option, nothing set it, and its #else branch declared an extern that nothing defined: following the #warning's own instruction produced a link error. Every artifact ever built therefore carried the RFC 8032 test key, whose secret is published. Now -DEBLDR_PRODUCTION_KEY=<64 hex characters> is checked (exactly 64 hex, and not the development key), turned into build/generated/production_key.c defining ebldr_production_key[] (declared in include/eos_production_key.h), compiled into eboot_core, and selects the production branch of keystore.c. A Release build of a real board that sets no key refuses to configure and says how to proceed: give a key, or pass -DEBLDR_ALLOW_DEV_KEY=ON for a bring-up or CI build that will never reach a device. Host builds and Debug builds are not gated, so development is unchanged apart from the existing #warning. Verified on this host: Release + stm32f4 with no key -> FATAL_ERROR with the gate message as the first error; the same with EBLDR_ALLOW_DEV_KEY=ON -> no gate message; the development key, and abc, as the production key -> refused; the RFC 8032 TEST 2 public key -> eboot_core builds without the keystore #warning, nm shows ebldr_production_key defined and default_dev_key absent. --- CMakeLists.txt | 39 +++++++++++++++++++ cmake/ProductionKey.cmake | 74 ++++++++++++++++++++++++++++++++++++ core/keystore.c | 33 +++++++++++----- include/eos_production_key.h | 30 +++++++++++++++ 4 files changed, 167 insertions(+), 9 deletions(-) create mode 100644 cmake/ProductionKey.cmake create mode 100644 include/eos_production_key.h diff --git a/CMakeLists.txt b/CMakeLists.txt index f8fe6eb..1fcfbad 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -27,9 +27,42 @@ option(EBLDR_VERIFY_STAGE1 "Verify Stage-1 hash before jump" ON) option(EBLDR_HARDENING "Enable compiler hardening flags" ON) option(EBLDR_SANITIZE "Enable ASAN/UBSAN for host builds" OFF) option(EBLDR_BUILD_FUZZ "Build libFuzzer fuzz targets" OFF) +set(EBLDR_PRODUCTION_KEY "" CACHE STRING + "Ed25519 public key, 64 hex characters, compiled in as the trust anchor for \ +a board without OTP. A Release build of a real board refuses to configure without it.") +option(EBLDR_ALLOW_DEV_KEY + "Let a Release build of a real board fall back to the RFC 8032 test key \ +(bring-up and CI cross-compiles only; never a device)" OFF) set(EBLDR_INCLUDE_DIR ${CMAKE_CURRENT_SOURCE_DIR}/include) +# ==================================================================== +# Trust anchor for boards without OTP +# ==================================================================== +# core/keystore.c falls back to a compiled-in public key when the board has +# no OTP -- which today is every board under boards/, none of which +# implements otp_read. Without a production key that fallback is the RFC 8032 +# test key, whose secret is published, and an artifact built that way accepts +# anyone's firmware. So a Release build of a real board refuses to configure +# until it is given a key, or told in so many words that it is not a release. +include(cmake/ProductionKey.cmake) +if(NOT EBLDR_PRODUCTION_KEY STREQUAL "") + ebldr_write_production_key_source("${EBLDR_PRODUCTION_KEY}" + "${CMAKE_BINARY_DIR}/generated/production_key.c") + set(EBLDR_PRODUCTION_KEY_SOURCE "${CMAKE_BINARY_DIR}/generated/production_key.c") + message(STATUS " Trust anchor: production key from EBLDR_PRODUCTION_KEY") +elseif(CMAKE_BUILD_TYPE STREQUAL "Release" AND NOT EBLDR_BOARD STREQUAL "none" + AND NOT EBLDR_ALLOW_DEV_KEY) + message(FATAL_ERROR + "EBLDR_PRODUCTION_KEY is not set: a Release build of board '${EBLDR_BOARD}' " + "would compile in the RFC 8032 test key as its trust anchor, and anyone can " + "sign for that key. Pass -DEBLDR_PRODUCTION_KEY=<64 hex characters> (the raw " + "Ed25519 public key), or, for a bring-up or CI build that will never reach a " + "device, -DEBLDR_ALLOW_DEV_KEY=ON.") +else() + message(STATUS " Trust anchor: RFC 8032 test key (development only; see core/keystore.c)") +endif() + # ==================================================================== # Compiler flags # ==================================================================== @@ -117,6 +150,12 @@ add_library(eboot_core STATIC ) target_include_directories(eboot_core PUBLIC ${EBLDR_INCLUDE_DIR}) target_link_libraries(eboot_core PUBLIC eboot_hal) +if(EBLDR_PRODUCTION_KEY_SOURCE) + # The generated ebldr_production_key[] and the switch that makes + # core/keystore.c use it instead of the development key. + target_sources(eboot_core PRIVATE ${EBLDR_PRODUCTION_KEY_SOURCE}) + target_compile_definitions(eboot_core PRIVATE EBLDR_PRODUCTION_KEY=1) +endif() # ---- Stage-1 boot manager ---- add_library(eboot_stage1 STATIC diff --git a/cmake/ProductionKey.cmake b/cmake/ProductionKey.cmake new file mode 100644 index 0000000..e5188d0 --- /dev/null +++ b/cmake/ProductionKey.cmake @@ -0,0 +1,74 @@ +# SPDX-License-Identifier: MIT +# Copyright (c) 2026 EoS Project +# +# The compiled-in trust anchor. +# +# core/keystore.c falls back to a compiled-in public key when the board has no +# OTP to read one from. With EBLDR_PRODUCTION_KEY unset that key is the RFC 8032 +# section 7.1 TEST 1 public key, whose secret is printed in the RFC, so a device +# built that way accepts firmware from anyone. These two functions are how a +# real key gets in: the top-level CMakeLists.txt calls them for the value of +# EBLDR_PRODUCTION_KEY, and tests/CMakeLists.txt calls them for a fixture key +# so the production branch of keystore.c is compiled and exercised on every +# host build. + +# The development key, lower-case hex. A production key that equals it is +# refused: it is not a secret, and the whole point of the option is to keep it +# out of an artifact. +set(EBLDR_DEV_KEY_HEX + "d75a980182b10ab7d54bfed3c964073a0ee172f3daa62325af021a68f707511a") + +# Fail the configure unless `hex` is a raw Ed25519 public key that is not the +# development key. +function(ebldr_check_production_key_hex hex) + string(LENGTH "${hex}" _len) + if(NOT _len EQUAL 64 OR NOT hex MATCHES "^[0-9a-fA-F]+$") + message(FATAL_ERROR + "EBLDR_PRODUCTION_KEY must be a raw Ed25519 public key as exactly 64 " + "hexadecimal characters; got ${_len} character(s).") + endif() + string(TOLOWER "${hex}" _lower) + if(_lower STREQUAL EBLDR_DEV_KEY_HEX) + message(FATAL_ERROR + "EBLDR_PRODUCTION_KEY is the RFC 8032 section 7.1 TEST 1 public key -- " + "the development key, whose secret is published. It cannot be a " + "production trust anchor.") + endif() +endfunction() + +# Write a C translation unit defining ebldr_production_key[] from `hex` to +# `out`. The file is only rewritten when its content changes, so an unchanged +# key does not rebuild the keystore on every configure. +function(ebldr_write_production_key_source hex out) + ebldr_check_production_key_hex("${hex}") + string(TOLOWER "${hex}" _lower) + string(REGEX MATCHALL "[0-9a-f][0-9a-f]" _bytes "${_lower}") + set(_body "") + set(_i 0) + foreach(_b IN LISTS _bytes) + math(EXPR _col "${_i} % 8") + if(_col EQUAL 0) + string(APPEND _body " ") + endif() + string(APPEND _body "0x${_b},") + if(_col EQUAL 7) + string(APPEND _body "\n") + else() + string(APPEND _body " ") + endif() + math(EXPR _i "${_i} + 1") + endforeach() + set(_content +"/* SPDX-License-Identifier: MIT */ +/* Generated by cmake/ProductionKey.cmake from a 64-hex-character Ed25519 + * public key. Do not edit; change the key the build was configured with. */ +#include \"eos_production_key.h\" + +const uint8_t ebldr_production_key[EOS_ED25519_PUB_KEY_SIZE] = { +${_body}}; +") + get_filename_component(_dir "${out}" DIRECTORY) + file(MAKE_DIRECTORY "${_dir}") + file(WRITE "${out}.in" "${_content}") + configure_file("${out}.in" "${out}" COPYONLY) +endfunction() diff --git a/core/keystore.c b/core/keystore.c index 6e16600..aebd0d2 100644 --- a/core/keystore.c +++ b/core/keystore.c @@ -14,17 +14,33 @@ #include "eos_keystore.h" #include "eos_hal.h" #include +#ifdef EBLDR_PRODUCTION_KEY +#include "eos_production_key.h" +#endif -/* Default development key — REPLACE with production key before deployment. +/* The compiled-in trust anchor, used when the board has no OTP to read a + * key from -- which today is every board under boards/. + * + * Without EBLDR_PRODUCTION_KEY it is the public half of TEST 1 in RFC 8032 + * section 7.1. The matching private key is printed in the RFC, so anyone at + * all can produce a signature that a bootloader trusting this key will + * accept. It is a usable default for bring-up and for the unit tests, and it + * must never reach a device. The #warning below is deliberate: this key going + * out silently is the failure mode, so a build that embeds it says so on + * every compile. + * + * A production key is supplied at configure time: * - * This is the public half of TEST 1 in RFC 8032 section 7.1. The matching - * private key is printed in the RFC, so anyone at all can produce a signature - * that a bootloader trusting this key will accept. It is a usable default for - * bring-up and for the unit tests, and it must never reach a device. + * cmake -B build -DEBLDR_BOARD= -DCMAKE_BUILD_TYPE=Release \ + * -DEBLDR_PRODUCTION_KEY=<64 hex characters, the raw Ed25519 public key> * - * Production builds set EBLDR_PRODUCTION_KEY, which replaces it. The #warning - * below is deliberate: this key going out silently is the failure mode, so a - * build that embeds it says so on every compile. */ + * cmake/ProductionKey.cmake turns that into a generated translation unit + * defining ebldr_production_key[] (declared in eos_production_key.h), + * compiles it into eboot_core and defines EBLDR_PRODUCTION_KEY, which selects + * the #else branch below. A Release build of a real board that sets no key + * refuses to configure unless it also sets EBLDR_ALLOW_DEV_KEY=ON, and + * .github/workflows/release.yml refuses any artifact that still contains the + * development key's bytes. */ #ifndef EBLDR_PRODUCTION_KEY #warning "eBoot: building with the RFC 8032 test-vector public key as the secure-boot trust anchor; define EBLDR_PRODUCTION_KEY for any real device" /* RFC 8032 section 7.1, TEST 1, PUBLIC KEY -- all 32 bytes of it. The array @@ -122,7 +138,6 @@ int eos_keystore_init(eos_keystore_t *ks) #ifndef EBLDR_PRODUCTION_KEY memcpy(ks->slots[0].key, default_dev_key, EOS_ED25519_PUB_KEY_SIZE); #else - extern const uint8_t ebldr_production_key[EOS_ED25519_PUB_KEY_SIZE]; memcpy(ks->slots[0].key, ebldr_production_key, EOS_ED25519_PUB_KEY_SIZE); #endif ks->slots[0].valid = true; diff --git a/include/eos_production_key.h b/include/eos_production_key.h new file mode 100644 index 0000000..40f906a --- /dev/null +++ b/include/eos_production_key.h @@ -0,0 +1,30 @@ +// SPDX-License-Identifier: MIT +// Copyright (c) 2026 EoS Project + +/** + * @file eos_production_key.h + * @brief The compiled-in production trust anchor. + * + * Defined by a translation unit that cmake/ProductionKey.cmake generates from + * EBLDR_PRODUCTION_KEY, and compiled into eboot_core only when that option is + * set. core/keystore.c uses it in place of the RFC 8032 development key for a + * board that has no OTP to read a key from. + */ + +#ifndef EOS_PRODUCTION_KEY_H +#define EOS_PRODUCTION_KEY_H + +#include +#include "eos_keystore.h" + +#ifdef __cplusplus +extern "C" { +#endif + +extern const uint8_t ebldr_production_key[EOS_ED25519_PUB_KEY_SIZE]; + +#ifdef __cplusplus +} +#endif + +#endif /* EOS_PRODUCTION_KEY_H */ From 2096bc313fd4db5a793363b930387a63eb81920c Mon Sep 17 00:00:00 2001 From: kartikey1306 Date: Tue, 15 Sep 2026 01:10:53 +0530 Subject: [PATCH 10/23] ci(release): pass the production anchor, and refuse an artifact that embeds the development key Every board configure in release.yml -- the stm32f4/stm32h7/nrf52 matrix, rpi4, riscv64_virt, both esp32 and both esp32c3 lines, x86_64_efi -- now passes -DEBLDR_PRODUCTION_KEY="${{ secrets.EBLDR_PRODUCTION_KEY_HEX }}". With the secret unset the value is empty and CMake refuses to configure, so a release cannot be cut on the development key until a maintainer provides the anchor. That is the intended shape. After each firmware build a step scans every .elf, .bin, .a and .o under build/ for the development key's 32 bytes and fails the job on a hit, printing the file. Verified the snippet against a file containing the bytes (hit, exit 1) and a clean one (exit 0). ci.yml's ARM cross-compile is a Release build of stm32f4 and would now trip the gate; it passes -DEBLDR_ALLOW_DEV_KEY=ON with a comment saying why: it checks that the tree cross-compiles and nothing it produces reaches a device. --- .github/workflows/ci.yml | 6 +- .github/workflows/release.yml | 125 +++++++++++++++++++++++++++++++--- 2 files changed, 122 insertions(+), 9 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 24ae902..4dbaac1 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -103,11 +103,15 @@ jobs: - name: Configure (ARM) run: | + # A Release build of a real board refuses to configure without a + # production trust anchor. This build checks that the tree + # cross-compiles; nothing it produces reaches a device, so it says so. cmake -B build/arm -G Ninja \ -DCMAKE_BUILD_TYPE=$BUILD_TYPE \ -DCMAKE_TOOLCHAIN_FILE=toolchains/arm-none-eabi.cmake \ -DEBLDR_BOARD=stm32f4 \ - -DEBLDR_BUILD_TESTS=OFF + -DEBLDR_BUILD_TESTS=OFF \ + -DEBLDR_ALLOW_DEV_KEY=ON - name: Build (ARM) run: cmake --build build/arm --parallel $(nproc) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 0980421..eba7b90 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -47,9 +47,33 @@ jobs: id: version run: echo "version=${GITHUB_REF#refs/tags/v}" >> $GITHUB_OUTPUT - name: Build firmware + # EBLDR_PRODUCTION_KEY_HEX is the raw Ed25519 public key, 64 hex + # characters, that boards without OTP compile in as their trust + # anchor. With the secret unset the value is empty and CMake refuses + # to configure a Release build of a real board -- so a release cannot + # be cut with the RFC 8032 test key until a maintainer provides the + # anchor. That is the intended shape; do not work around it with + # EBLDR_ALLOW_DEV_KEY here. run: | - cmake -B build -DEBLDR_BOARD=${{ matrix.board }} -DCMAKE_BUILD_TYPE=Release + cmake -B build -DEBLDR_BOARD=${{ matrix.board }} -DEBLDR_PRODUCTION_KEY="${{ secrets.EBLDR_PRODUCTION_KEY_HEX }}" -DCMAKE_BUILD_TYPE=Release cmake --build build --parallel + - name: Refuse an artifact that embeds the development anchor + run: | + python3 - <<'PY' + import pathlib, sys + # RFC 8032 section 7.1 TEST 1 public key: what core/keystore.c falls + # back to when EBLDR_PRODUCTION_KEY is unset. Its secret is published. + dev = bytes.fromhex( + "d75a980182b10ab7d54bfed3c964073a0ee172f3daa62325af021a68f707511a") + hits = sorted(str(p) for p in pathlib.Path("build").rglob("*") + if p.is_file() and p.suffix in (".elf", ".bin", ".a", ".o") + and dev in p.read_bytes()) + for h in hits: + print(f"::error file={h}::embeds the development trust anchor") + if hits: + sys.exit(1) + print("no artifact under build/ contains the development key") + PY - name: Collect artifacts run: | mkdir -p fw @@ -77,10 +101,27 @@ jobs: run: echo "version=${GITHUB_REF#refs/tags/v}" >> $GITHUB_OUTPUT - name: Build run: | - cmake -B build -DEBLDR_BOARD=rpi4 \ + cmake -B build -DEBLDR_BOARD=rpi4 -DEBLDR_PRODUCTION_KEY="${{ secrets.EBLDR_PRODUCTION_KEY_HEX }}" \ -DCMAKE_C_COMPILER=aarch64-linux-gnu-gcc \ -DCMAKE_BUILD_TYPE=Release cmake --build build --parallel + - name: Refuse an artifact that embeds the development anchor + run: | + python3 - <<'PY' + import pathlib, sys + # RFC 8032 section 7.1 TEST 1 public key: what core/keystore.c falls + # back to when EBLDR_PRODUCTION_KEY is unset. Its secret is published. + dev = bytes.fromhex( + "d75a980182b10ab7d54bfed3c964073a0ee172f3daa62325af021a68f707511a") + hits = sorted(str(p) for p in pathlib.Path("build").rglob("*") + if p.is_file() and p.suffix in (".elf", ".bin", ".a", ".o") + and dev in p.read_bytes()) + for h in hits: + print(f"::error file={h}::embeds the development trust anchor") + if hits: + sys.exit(1) + print("no artifact under build/ contains the development key") + PY - name: Collect artifacts run: | mkdir -p fw @@ -107,10 +148,27 @@ jobs: run: echo "version=${GITHUB_REF#refs/tags/v}" >> $GITHUB_OUTPUT - name: Build run: | - cmake -B build -DEBLDR_BOARD=riscv64_virt \ + cmake -B build -DEBLDR_BOARD=riscv64_virt -DEBLDR_PRODUCTION_KEY="${{ secrets.EBLDR_PRODUCTION_KEY_HEX }}" \ -DCMAKE_C_COMPILER=riscv64-linux-gnu-gcc \ -DCMAKE_BUILD_TYPE=Release cmake --build build --parallel + - name: Refuse an artifact that embeds the development anchor + run: | + python3 - <<'PY' + import pathlib, sys + # RFC 8032 section 7.1 TEST 1 public key: what core/keystore.c falls + # back to when EBLDR_PRODUCTION_KEY is unset. Its secret is published. + dev = bytes.fromhex( + "d75a980182b10ab7d54bfed3c964073a0ee172f3daa62325af021a68f707511a") + hits = sorted(str(p) for p in pathlib.Path("build").rglob("*") + if p.is_file() and p.suffix in (".elf", ".bin", ".a", ".o") + and dev in p.read_bytes()) + for h in hits: + print(f"::error file={h}::embeds the development trust anchor") + if hits: + sys.exit(1) + print("no artifact under build/ contains the development key") + PY - name: Collect artifacts run: | mkdir -p fw @@ -152,10 +210,27 @@ jobs: run: | source ~/esp-idf/export.sh # Use ESP-IDF's idf.py if a top-level CMakeLists has IDF setup, else direct cmake - cmake -B build -DEBLDR_BOARD=esp32 -DCMAKE_BUILD_TYPE=Release \ + cmake -B build -DEBLDR_BOARD=esp32 -DEBLDR_PRODUCTION_KEY="${{ secrets.EBLDR_PRODUCTION_KEY_HEX }}" -DCMAKE_BUILD_TYPE=Release \ -DCMAKE_TOOLCHAIN_FILE=$IDF_PATH/tools/cmake/toolchain-esp32.cmake || \ - cmake -B build -DEBLDR_BOARD=esp32 -DCMAKE_BUILD_TYPE=Release + cmake -B build -DEBLDR_BOARD=esp32 -DEBLDR_PRODUCTION_KEY="${{ secrets.EBLDR_PRODUCTION_KEY_HEX }}" -DCMAKE_BUILD_TYPE=Release cmake --build build --parallel + - name: Refuse an artifact that embeds the development anchor + run: | + python3 - <<'PY' + import pathlib, sys + # RFC 8032 section 7.1 TEST 1 public key: what core/keystore.c falls + # back to when EBLDR_PRODUCTION_KEY is unset. Its secret is published. + dev = bytes.fromhex( + "d75a980182b10ab7d54bfed3c964073a0ee172f3daa62325af021a68f707511a") + hits = sorted(str(p) for p in pathlib.Path("build").rglob("*") + if p.is_file() and p.suffix in (".elf", ".bin", ".a", ".o") + and dev in p.read_bytes()) + for h in hits: + print(f"::error file={h}::embeds the development trust anchor") + if hits: + sys.exit(1) + print("no artifact under build/ contains the development key") + PY - name: Collect artifacts run: | mkdir -p fw @@ -195,10 +270,27 @@ jobs: - name: Build firmware (RISC-V) run: | source ~/esp-idf/export.sh - cmake -B build -DEBLDR_BOARD=esp32c3 -DCMAKE_BUILD_TYPE=Release \ + cmake -B build -DEBLDR_BOARD=esp32c3 -DEBLDR_PRODUCTION_KEY="${{ secrets.EBLDR_PRODUCTION_KEY_HEX }}" -DCMAKE_BUILD_TYPE=Release \ -DCMAKE_TOOLCHAIN_FILE=$IDF_PATH/tools/cmake/toolchain-esp32c3.cmake || \ - cmake -B build -DEBLDR_BOARD=esp32c3 -DCMAKE_BUILD_TYPE=Release + cmake -B build -DEBLDR_BOARD=esp32c3 -DEBLDR_PRODUCTION_KEY="${{ secrets.EBLDR_PRODUCTION_KEY_HEX }}" -DCMAKE_BUILD_TYPE=Release cmake --build build --parallel + - name: Refuse an artifact that embeds the development anchor + run: | + python3 - <<'PY' + import pathlib, sys + # RFC 8032 section 7.1 TEST 1 public key: what core/keystore.c falls + # back to when EBLDR_PRODUCTION_KEY is unset. Its secret is published. + dev = bytes.fromhex( + "d75a980182b10ab7d54bfed3c964073a0ee172f3daa62325af021a68f707511a") + hits = sorted(str(p) for p in pathlib.Path("build").rglob("*") + if p.is_file() and p.suffix in (".elf", ".bin", ".a", ".o") + and dev in p.read_bytes()) + for h in hits: + print(f"::error file={h}::embeds the development trust anchor") + if hits: + sys.exit(1) + print("no artifact under build/ contains the development key") + PY - name: Collect artifacts run: | mkdir -p fw @@ -224,8 +316,25 @@ jobs: run: echo "version=${GITHUB_REF#refs/tags/v}" >> $GITHUB_OUTPUT - name: Build EFI firmware run: | - cmake -B build -DEBLDR_BOARD=x86_64_efi -DCMAKE_BUILD_TYPE=Release + cmake -B build -DEBLDR_BOARD=x86_64_efi -DEBLDR_PRODUCTION_KEY="${{ secrets.EBLDR_PRODUCTION_KEY_HEX }}" -DCMAKE_BUILD_TYPE=Release cmake --build build --parallel + - name: Refuse an artifact that embeds the development anchor + run: | + python3 - <<'PY' + import pathlib, sys + # RFC 8032 section 7.1 TEST 1 public key: what core/keystore.c falls + # back to when EBLDR_PRODUCTION_KEY is unset. Its secret is published. + dev = bytes.fromhex( + "d75a980182b10ab7d54bfed3c964073a0ee172f3daa62325af021a68f707511a") + hits = sorted(str(p) for p in pathlib.Path("build").rglob("*") + if p.is_file() and p.suffix in (".elf", ".bin", ".a", ".o") + and dev in p.read_bytes()) + for h in hits: + print(f"::error file={h}::embeds the development trust anchor") + if hits: + sys.exit(1) + print("no artifact under build/ contains the development key") + PY - name: Collect artifacts run: | mkdir -p fw From 58f8216f698fa33fcf3f0c000c2c97f81be50aa4 Mon Sep 17 00:00:00 2001 From: kartikey1306 Date: Tue, 15 Sep 2026 01:10:53 +0530 Subject: [PATCH 11/23] test: pin the production-anchor gate, the release workflow, and the production keystore tests/unit/test_keystore_production.c compiles core/keystore.c with EBLDR_PRODUCTION_KEY and a fixture key (RFC 8032 TEST 2, generated by the same cmake/ProductionKey.cmake function the real build uses), so the production branch -- the one a release is built from, which had never been compiled -- is built and run on every host build. It asserts the generated symbol carries the configured bytes, that a board without OTP gets that key as its active anchor from the compiled-in source, and that the key is not the development key. Flipping one fixture byte fails it; configuring the fixture as the development key is refused by CMake. tests/unit/test_production_key_gate.py runs real configures: Release + board with no key fails with the gate message as the first error; the opt-out clears it; Debug and host builds are not gated; the development key (either case) and four malformed values are refused; a real key generates the anchor source with exactly those bytes. tests/unit/test_release_workflow_production_key.py parses the workflows: all 8 board configures in release.yml carry the secret and none the opt-out; all 6 firmware jobs scan after building and before collecting; the bytes the scan names are the bytes core/keystore.c and cmake/ProductionKey.cmake name; and no workflow anywhere cross-compiles a Release board without a key or the opt-out. Each property was checked by mutation: dropping the flag from one esp32c3 line, changing one scan hex digit, and removing ci.yml's opt-out each fail the suite. --- tests/CMakeLists.txt | 20 +++ tests/unit/test_keystore_production.c | 112 +++++++++++++ tests/unit/test_production_key_gate.py | 110 +++++++++++++ .../test_release_workflow_production_key.py | 148 ++++++++++++++++++ 4 files changed, 390 insertions(+) create mode 100644 tests/unit/test_keystore_production.c create mode 100644 tests/unit/test_production_key_gate.py create mode 100644 tests/unit/test_release_workflow_production_key.py diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 219c3c8..4150393 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -216,6 +216,26 @@ target_include_directories(eboot_test_eos_sign_boot_path PRIVATE ${CMAKE_CURRENT 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) +# --- test_keystore_production: core/keystore.c with EBLDR_PRODUCTION_KEY --- +# eboot_core carries the development-key keystore, so this suite compiles +# core/keystore.c itself, with the definition a Release board build gets and +# a fixture key generated the same way (cmake/ProductionKey.cmake). It still +# links eboot_core for eos_crypto_hash(): the suite's own keystore.o already +# defines every keystore symbol, so the archive's copy is never pulled in and +# the object under test is the one compiled here. The key is RFC 8032 +# section 7.1 TEST 2's public key. +ebldr_write_production_key_source( + "3d4017c3e843895a92b70aa74d1b7ebc9c982ccf2ec4968cc0cd55f12af4660c" + "${CMAKE_CURRENT_BINARY_DIR}/generated/test_production_key.c") +add_executable(eboot_test_keystore_production + unit/test_keystore_production.c + ../core/keystore.c + ${CMAKE_CURRENT_BINARY_DIR}/generated/test_production_key.c) +target_compile_definitions(eboot_test_keystore_production PRIVATE EBLDR_PRODUCTION_KEY=1) +target_link_libraries(eboot_test_keystore_production PRIVATE eboot_core) +add_test(NAME test_keystore_production COMMAND eboot_test_keystore_production) +list(APPEND EBLDR_UNIT_TESTS test_keystore_production) + # --- Valgrind test targets --- # # The list is derived from the suites registered above, not written out again. diff --git a/tests/unit/test_keystore_production.c b/tests/unit/test_keystore_production.c new file mode 100644 index 0000000..af7101b --- /dev/null +++ b/tests/unit/test_keystore_production.c @@ -0,0 +1,112 @@ +// SPDX-License-Identifier: MIT +// Copyright (c) 2026 EoS Project + +/** + * @file test_keystore_production.c + * @brief core/keystore.c compiled with EBLDR_PRODUCTION_KEY. + * + * The production branch of the keystore -- the one a release artifact is + * built from -- had never been compiled: it declared an extern that nothing + * defined, so following the #warning's own instruction produced a link + * error. This suite builds core/keystore.c the way a Release board build + * does (EBLDR_PRODUCTION_KEY defined, ebldr_production_key[] generated by + * cmake/ProductionKey.cmake from a fixture key) and asks the no-OTP path for + * its anchor. The suite's own keystore.o defines every keystore symbol, so + * the copy inside eboot_core is never pulled in: the object under test is + * the one compiled here, not the development-key one. + * + * The fixture key is RFC 8032 section 7.1 TEST 2's public key: a real curve + * point that is not the development key, so the assertions can tell the two + * apart. + */ + +#include "eos_keystore.h" +#include "eos_production_key.h" +#include "eos_hal.h" +#include "../vectors/fw_update_test_sigs.h" +#include +#include +#include + +/* RFC 8032 section 7.1, TEST 2, PUBLIC KEY: what tests/CMakeLists.txt hands + * to ebldr_write_production_key_source() for this suite. */ +static const uint8_t k_fixture_key[EOS_ED25519_PUB_KEY_SIZE] = { + 0x3d,0x40,0x17,0xc3,0xe8,0x43,0x89,0x5a,0x92,0xb7,0x0a,0xa7,0x4d,0x1b,0x7e,0xbc, + 0x9c,0x98,0x2c,0xcf,0x2e,0xc4,0x96,0x8c,0xc0,0xcd,0x55,0xf1,0x2a,0xf4,0x66,0x0c, +}; + +/* A board with no OTP at all: eos_hal_otp_read() reports EOS_ERR_NOT_SUPPORTED + * and the keystore takes the compiled-in path. */ +static const eos_board_ops_t no_otp_board = { 0 }; + +static int tests_run = 0; +static int tests_passed = 0; + +#define TEST(name) \ + static void name(void); \ + static void run_##name(void) { \ + printf(" %-62s ", #name); \ + tests_run++; \ + name(); \ + tests_passed++; \ + printf("[PASS]\n"); \ + } \ + static void name(void) + +#define ASSERT(cond) do { \ + if (!(cond)) { \ + printf("[FAIL] %s:%d: %s\n", __FILE__, __LINE__, #cond); \ + exit(1); \ + } \ +} while(0) + +/* The generated translation unit defines the symbol keystore.c consumes, and + * with the bytes the build was configured with. */ +TEST(test_generated_symbol_carries_the_configured_key) +{ + ASSERT(memcmp(ebldr_production_key, k_fixture_key, EOS_ED25519_PUB_KEY_SIZE) == 0); +} + +/* On a board without OTP the active key is the production key, from the + * compiled-in source. */ +TEST(test_no_otp_board_gets_the_production_key) +{ + eos_keystore_t ks; + const uint8_t *key = NULL; + size_t key_len = 0; + + eos_hal_init(&no_otp_board); + ASSERT(eos_keystore_init(&ks) == EOS_OK); + ASSERT(ks.source == EOS_KEY_SOURCE_COMPILED); + ASSERT(eos_keystore_get_active_key(&ks, &key, &key_len) == EOS_OK); + ASSERT(key_len == EOS_ED25519_PUB_KEY_SIZE); + ASSERT(memcmp(key, k_fixture_key, EOS_ED25519_PUB_KEY_SIZE) == 0); +} + +/* And it is not the development key. eos_test_sig_pubkey is the RFC 8032 + * TEST 1 public key, derived from the RFC's secret by + * tools/gen_fw_update_test_sigs.py -- the key the #ifndef branch compiles in + * and the one a release must never carry. */ +TEST(test_no_otp_board_does_not_get_the_development_key) +{ + eos_keystore_t ks; + const uint8_t *key = NULL; + size_t key_len = 0; + + eos_hal_init(&no_otp_board); + ASSERT(eos_keystore_init(&ks) == EOS_OK); + ASSERT(eos_keystore_get_active_key(&ks, &key, &key_len) == EOS_OK); + ASSERT(memcmp(key, eos_test_sig_pubkey, EOS_ED25519_PUB_KEY_SIZE) != 0); +} + +int main(void) +{ + printf("=== eBootloader: Keystore with EBLDR_PRODUCTION_KEY ===\n\n"); + + run_test_generated_symbol_carries_the_configured_key(); + run_test_no_otp_board_gets_the_production_key(); + run_test_no_otp_board_does_not_get_the_development_key(); + + printf("\n%d/%d tests passed\n", tests_passed, tests_run); + return (tests_passed == tests_run) ? 0 : 1; +} diff --git a/tests/unit/test_production_key_gate.py b/tests/unit/test_production_key_gate.py new file mode 100644 index 0000000..04c2c65 --- /dev/null +++ b/tests/unit/test_production_key_gate.py @@ -0,0 +1,110 @@ +# SPDX-License-Identifier: MIT +# Copyright (c) 2026 EoS Project + +"""The configure-time gate on the compiled-in trust anchor. + +core/keystore.c falls back to a compiled-in public key on a board without +OTP -- every board under boards/ today. Without EBLDR_PRODUCTION_KEY that key +is the RFC 8032 test key, whose secret is published. These tests run real +CMake configures against the repository and pin what the gate does: + + * a Release build of a real board refuses to configure without a key, + and says why, before anything else goes wrong; + * EBLDR_ALLOW_DEV_KEY=ON is the one way past that, and it is explicit; + * the development key and a malformed value are refused as production keys; + * a real key produces the generated translation unit with those bytes. + +Configure only; nothing is compiled. +""" + +import re +import shutil +import subprocess +import tempfile +from pathlib import Path + +import pytest + +REPO_ROOT = Path(__file__).resolve().parents[2] + +# RFC 8032 section 7.1 TEST 2 public key: a real curve point that is not the +# development key. +GOOD_KEY = "3d4017c3e843895a92b70aa74d1b7ebc9c982ccf2ec4968cc0cd55f12af4660c" +# RFC 8032 section 7.1 TEST 1 public key: core/keystore.c's fallback. +DEV_KEY = "d75a980182b10ab7d54bfed3c964073a0ee172f3daa62325af021a68f707511a" + +GATE_MESSAGE = "EBLDR_PRODUCTION_KEY is not set" + +pytestmark = pytest.mark.skipif( + shutil.which("cmake") is None, reason="cmake is not on PATH") + + +def configure(*defs): + build = Path(tempfile.mkdtemp(prefix="eboot-gate-")) + try: + result = subprocess.run( + ["cmake", "-S", str(REPO_ROOT), "-B", str(build), *defs], + capture_output=True, text=True) + generated = build / "generated" / "production_key.c" + text = generated.read_text() if generated.exists() else None + return result, text + finally: + shutil.rmtree(build, ignore_errors=True) + + +def first_error(stderr): + """The first 'CMake Error' block, so ordering can be asserted.""" + blocks = re.split(r"(?=CMake Error)", stderr) + return next((b for b in blocks if b.startswith("CMake Error")), "") + + +def test_release_board_build_refuses_without_a_key(): + result, _ = configure("-DCMAKE_BUILD_TYPE=Release", "-DEBLDR_BOARD=stm32f4") + assert result.returncode != 0 + assert GATE_MESSAGE in result.stderr + # The gate, not a missing toolchain or a board port, is what stops it. + assert GATE_MESSAGE in first_error(result.stderr), result.stderr + + +def test_release_board_build_can_say_it_is_not_a_release(): + result, _ = configure("-DCMAKE_BUILD_TYPE=Release", "-DEBLDR_BOARD=stm32f4", + "-DEBLDR_ALLOW_DEV_KEY=ON") + assert GATE_MESSAGE not in result.stderr + + +def test_debug_board_build_and_host_build_are_not_gated(): + # Neither is release-shaped: no key, no opt-out, no gate message. + result, _ = configure("-DCMAKE_BUILD_TYPE=Debug", "-DEBLDR_BOARD=stm32f4") + assert GATE_MESSAGE not in result.stderr + result, generated = configure("-DCMAKE_BUILD_TYPE=Release") + assert result.returncode == 0, result.stderr + assert GATE_MESSAGE not in result.stderr + assert generated is None, "no key was given, so nothing should be generated" + + +def test_development_key_is_refused_as_a_production_key(): + for spelling in (DEV_KEY, DEV_KEY.upper()): + result, generated = configure("-DEBLDR_PRODUCTION_KEY=" + spelling) + assert result.returncode != 0 + assert "development key" in result.stderr, result.stderr + assert generated is None + + +@pytest.mark.parametrize("bad", ["abc", GOOD_KEY[:-2], GOOD_KEY + "00", + "zz" + GOOD_KEY[2:]]) +def test_malformed_key_is_refused(bad): + result, generated = configure("-DEBLDR_PRODUCTION_KEY=" + bad) + assert result.returncode != 0 + assert "exactly 64" in result.stderr, result.stderr + assert generated is None + + +def test_real_key_generates_the_anchor_source(): + result, generated = configure("-DCMAKE_BUILD_TYPE=Release", + "-DEBLDR_BOARD=stm32f4", + "-DEBLDR_PRODUCTION_KEY=" + GOOD_KEY.upper()) + assert GATE_MESSAGE not in result.stderr + assert generated is not None, result.stderr + assert "ebldr_production_key[EOS_ED25519_PUB_KEY_SIZE]" in generated + emitted = "".join(re.findall(r"0x([0-9a-f]{2})", generated)) + assert emitted == GOOD_KEY, "the generated bytes must be the configured key" diff --git a/tests/unit/test_release_workflow_production_key.py b/tests/unit/test_release_workflow_production_key.py new file mode 100644 index 0000000..75449bb --- /dev/null +++ b/tests/unit/test_release_workflow_production_key.py @@ -0,0 +1,148 @@ +# SPDX-License-Identifier: MIT +# Copyright (c) 2026 EoS Project + +"""Every release-shaped CMake configure in the workflows carries a trust +anchor, and the release scans what it built for the development key. + +The gate in CMakeLists.txt is what makes a Release board build refuse the +development key. This pins the other half: that release.yml actually passes +EBLDR_PRODUCTION_KEY on every board configure (so an unset secret fails +closed instead of being worked around), never passes EBLDR_ALLOW_DEV_KEY, and +scans every firmware build for the development key's bytes -- and that the +bytes it scans for are the ones core/keystore.c and cmake/ProductionKey.cmake +name. +""" + +import re +from pathlib import Path + +import yaml + +REPO_ROOT = Path(__file__).resolve().parents[2] +WORKFLOWS = REPO_ROOT / ".github" / "workflows" +SCAN_STEP = "Refuse an artifact that embeds the development anchor" + + +def load(name): + return yaml.safe_load((WORKFLOWS / name).read_text(encoding="utf-8")) + + +def run_steps(job): + return [s for s in job.get("steps", []) if isinstance(s.get("run"), str)] + + +def configure_lines(run): + """Each `cmake -B` configure in a run block, with its backslash + continuation lines. A continuation that itself starts another + `cmake -B` (the esp32 jobs chain two with `||`) is its own configure.""" + lines = run.splitlines() + blocks, i = [], 0 + while i < len(lines): + if "cmake -B" in lines[i]: + block = lines[i] + while block.rstrip().endswith("\\") and i + 1 < len(lines): + i += 1 + block += "\n" + lines[i] + blocks.append(block) + i += 1 + out = [] + for block in blocks: + out.extend(part for part in re.split(r"(?=cmake -B)", block) if "cmake -B" in part) + return out + + +def board_of(block): + m = re.search(r"-DEBLDR_BOARD=(\S+)", block) + return m.group(1) if m else None + + +def is_release_shaped(block): + return "-DCMAKE_BUILD_TYPE=Release" in block or "$BUILD_TYPE" in block + + +def dev_key_from_keystore(): + src = (REPO_ROOT / "core" / "keystore.c").read_text(encoding="utf-8") + m = re.search(r"default_dev_key\[EOS_ED25519_PUB_KEY_SIZE\]\s*=\s*\{([^}]*)\}", src) + assert m, "default_dev_key[] not found in core/keystore.c" + return "".join(f"{int(x, 16):02x}" for x in re.findall(r"0x([0-9a-fA-F]{2})", m.group(1))) + + +def dev_key_from_cmake(): + src = (REPO_ROOT / "cmake" / "ProductionKey.cmake").read_text(encoding="utf-8") + m = re.search(r'set\(EBLDR_DEV_KEY_HEX\s*"([0-9a-f]{64})"\)', src) + assert m, "EBLDR_DEV_KEY_HEX not found in cmake/ProductionKey.cmake" + return m.group(1) + + +def test_every_release_board_configure_passes_the_production_key(): + doc = load("release.yml") + seen = 0 + for job_id, job in doc["jobs"].items(): + for step in run_steps(job): + for block in configure_lines(step["run"]): + if board_of(block) in (None, "none"): + continue + seen += 1 + assert "-DEBLDR_PRODUCTION_KEY=" in block, ( + f"release.yml job {job_id!r}: a board configure without " + f"EBLDR_PRODUCTION_KEY would ship the development key:\n{block}") + assert "secrets.EBLDR_PRODUCTION_KEY_HEX" in block, ( + f"release.yml job {job_id!r}: the key must come from the " + f"EBLDR_PRODUCTION_KEY_HEX secret, not a literal:\n{block}") + assert "EBLDR_ALLOW_DEV_KEY" not in block, ( + f"release.yml job {job_id!r} opts into the development key") + assert seen == 8, f"expected the 8 board configures release.yml had, found {seen}" + + +def test_every_release_firmware_job_scans_for_the_development_key(): + doc = load("release.yml") + firmware_jobs = [ + (job_id, job) for job_id, job in doc["jobs"].items() + if any(board_of(b) not in (None, "none") + for s in run_steps(job) for b in configure_lines(s["run"]))] + assert len(firmware_jobs) == 6, [j for j, _ in firmware_jobs] + for job_id, job in firmware_jobs: + names = [s.get("name") for s in job["steps"]] + assert SCAN_STEP in names, f"release.yml job {job_id!r} has no scan step" + build = next(i for i, s in enumerate(job["steps"]) + if isinstance(s.get("run"), str) and "cmake --build" in s["run"]) + scan = names.index(SCAN_STEP) + collect = names.index("Collect artifacts") + assert build < scan < collect, ( + f"release.yml job {job_id!r}: the scan must run after the build and " + f"before artifacts are collected") + + +def test_the_scan_looks_for_the_key_keystore_actually_compiles_in(): + doc = load("release.yml") + scans = [s["run"] for job in doc["jobs"].values() for s in run_steps(job) + if s.get("name") == SCAN_STEP] + assert scans + keystore = dev_key_from_keystore() + cmake = dev_key_from_cmake() + assert keystore == cmake, "core/keystore.c and cmake/ProductionKey.cmake disagree on the development key" + for run in scans: + assert keystore in run.replace("\n", "").replace(" ", "").replace('"', ""), ( + "the scan step does not name the development key core/keystore.c compiles in") + + +def test_no_ci_workflow_cross_compiles_a_release_board_without_saying_so(): + """A Release build of a real board must either carry a key or opt into + the development key explicitly; otherwise its configure fails, and a CI + job that hits the gate is a job that stops testing the tree.""" + for path in sorted(WORKFLOWS.glob("*.yml")): + doc = yaml.safe_load(path.read_text(encoding="utf-8")) + if not isinstance(doc, dict) or "jobs" not in doc: + continue + for job_id, job in doc["jobs"].items(): + if not isinstance(job, dict): + continue + for step in run_steps(job): + for block in configure_lines(step["run"]): + if board_of(block) in (None, "none") or not is_release_shaped(block): + continue + assert ("-DEBLDR_PRODUCTION_KEY=" in block + or "-DEBLDR_ALLOW_DEV_KEY=ON" in block), ( + f"{path.name} job {job_id!r}: a Release build of " + f"{board_of(block)} with no key and no opt-out cannot " + f"configure:\n{block}") From 700da34b50076f5d48bb6729316497685d545550 Mon Sep 17 00:00:00 2001 From: kartikey1306 Date: Tue, 15 Sep 2026 01:10:53 +0530 Subject: [PATCH 12/23] docs: key lifecycle and changelog for the production anchor docs/key_lifecycle.md described a header scheme (eos_signing_key.h, EOS_BUILD_PRODUCTION/STAGING) that does not exist in the tree and a development key that was not the one compiled in. Sections 3.1, 7.2 and 7.4 now describe what core/keystore.c, cmake/ProductionKey.cmake and release.yml actually do, name the RFC 8032 TEST 1 pair as the development key, and quote the gate's message. --- CHANGELOG.md | 1 + docs/key_lifecycle.md | 123 +++++++++++++++++++++++------------------- 2 files changed, 68 insertions(+), 56 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f3b12a6..2e26920 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ ## [Unreleased] ### Security +- **A release can no longer be built on the development trust anchor.** `EBLDR_PRODUCTION_KEY` had no CMake option, nothing set it, and its `#else` branch declared an `extern` nothing defined -- so every firmware job in `release.yml` compiled the `#ifndef` branch and shipped the RFC 8032 test key as the anchor for every board without OTP, which is every board under `boards/`. The only guard was a `#warning`. Now: `-DEBLDR_PRODUCTION_KEY=<64 hex characters>` generates `ebldr_production_key[]` (`cmake/ProductionKey.cmake`, declared in `include/eos_production_key.h`) and selects the production branch of `core/keystore.c`; a Release build of a real board refuses to configure without it unless it says `-DEBLDR_ALLOW_DEV_KEY=ON`; the development key and malformed values are refused as production keys; `release.yml` passes the `EBLDR_PRODUCTION_KEY_HEX` secret on every board configure (empty until a maintainer provides it, so a release fails closed) and scans every built `.elf`/`.bin`/`.a`/`.o` for the development key's bytes. `tests/unit/test_keystore_production.c` compiles the production branch with a fixture key on every host build; `tests/unit/test_production_key_gate.py` and `test_release_workflow_production_key.py` pin the gate and the workflow. - **The compiled-in development trust anchor was not a point on the curve.** `core/keystore.c` `default_dev_key` was described as the RFC 8032 section 7.1 TEST 1 public key, but it diverged from that key at byte 21 and the bytes it held did not decode to a point on the curve, so no signature could ever verify against it. It is now the RFC key, and `tests/unit/test_keystore.c` checks that the anchor verifies the RFC's own signature. Behaviour change: a board with no OTP and no `EBLDR_PRODUCTION_KEY` goes from refusing every image (since #104 made signature verification unconditional at install) to accepting images signed with the public RFC test key -- the documented development intent of the `#warning` in `core/keystore.c`, and a key that must never reach a device. #120 tracks making that structural. - **Image header is now authenticated (header format v2).** `eos_image_verify_signature()` signed `hdr->hash` only — 32 of the header's 156 bytes. Everything else (`image_size`, `load_addr`, `entry_addr`, `flags`, `sig_type`, `image_version`) sat outside the signature, so an attacker holding a legitimately signed image could relocate it, move its entry point, or clear `EOS_IMG_FLAG_HASH_SHA256` to downgrade integrity checking from SHA-256 to forgeable CRC32 — all while keeping the signature valid. The signature now covers `EOS_IMG_SIGNED_LEN` (92) bytes: the whole header except `signature[]` itself. **Existing signed images must be re-signed.** - **`eos_image_parse_header`:** validates `hdr_version`, rejecting 0 and anything newer than this build understands. diff --git a/docs/key_lifecycle.md b/docs/key_lifecycle.md index a9400db..94bbe9f 100644 --- a/docs/key_lifecycle.md +++ b/docs/key_lifecycle.md @@ -94,52 +94,52 @@ python3 tools/sign_image.py --genkey \ ### 3.1 Compiled-In Key (Default) -The public key is compiled directly into the stage-1 bootloader binary. This is the default for devices without OTP/eFuse capability. +The public key is compiled into the bootloader. `eos_keystore_init()` +(`core/keystore.c`) uses it only when the board has no OTP at all — when +`eos_hal_otp_read()` returns `EOS_ERR_NOT_SUPPORTED` because the board port +provides no `otp_read` hook. Today that is every board under `boards/`, so +on every shipped board the compiled-in key *is* the trust anchor. -**Generated header (`include/eos_signing_key.h`):** +**Which key gets compiled in is a configure-time decision:** -```c -// SPDX-License-Identifier: MIT -// Auto-generated by sign_image.py — do not edit manually - -#ifndef EOS_SIGNING_KEY_H -#define EOS_SIGNING_KEY_H - -#include - -/* Primary signing key (slot 0) */ -static const uint8_t eos_signing_pubkey_0[32] = { - 0x3b, 0x6a, 0x27, 0xbc, /* ... 28 more bytes ... */ -}; +```bash +# A real key: the raw 32-byte Ed25519 public key as 64 hex characters, +# e.g. the `.pub.raw` file from §2.2 step 3, hex-encoded. +cmake -B build -DEBLDR_BOARD=stm32f4 -DCMAKE_BUILD_TYPE=Release \ + -DEBLDR_PRODUCTION_KEY=3d4017c3e843895a92b70aa74d1b7ebc9c982ccf2ec4968cc0cd55f12af4660c +``` -/* Backup signing key (slot 1) */ -static const uint8_t eos_signing_pubkey_1[32] = { - 0x9d, 0x61, 0xb1, 0x9d, /* ... 28 more bytes ... */ -}; +`cmake/ProductionKey.cmake` checks the value (exactly 64 hex characters, and +not the development key), generates `build/generated/production_key.c` +defining `ebldr_production_key[]` (declared in `include/eos_production_key.h`), +compiles it into `eboot_core`, and defines `EBLDR_PRODUCTION_KEY` so +`core/keystore.c` uses that symbol in place of its development key. -#define EOS_SIGNING_KEY_COUNT 2 +**Without `EBLDR_PRODUCTION_KEY` the compiled-in key is the development key** +(§7.4), and the build says so with a `#warning` on every compile of +`core/keystore.c`. A **Release build of a real board refuses to configure** +in that state: -#endif /* EOS_SIGNING_KEY_H */ ``` - -**Verification logic:** - -```c -int eos_image_verify_signature(const eos_image_header_t *hdr) { - for (int i = 0; i < EOS_SIGNING_KEY_COUNT; i++) { - const uint8_t *pubkey = (i == 0) - ? eos_signing_pubkey_0 - : eos_signing_pubkey_1; - int rc = eos_crypto_verify_signature( - hdr->hash, EOS_SHA256_DIGEST_SIZE, - hdr->signature, hdr->sig_len, - pubkey, 32); - if (rc == EOS_OK) return EOS_OK; - } - return EOS_ERR_SIGNATURE; -} +CMake Error at CMakeLists.txt:56 (message): + EBLDR_PRODUCTION_KEY is not set: a Release build of board 'stm32f4' would + compile in the RFC 8032 test key as its trust anchor, and anyone can sign + for that key. Pass -DEBLDR_PRODUCTION_KEY=<64 hex characters> (the raw + Ed25519 public key), or, for a bring-up or CI build that will never reach a + device, -DEBLDR_ALLOW_DEV_KEY=ON. ``` +`-DEBLDR_ALLOW_DEV_KEY=ON` is the one way past the gate. It exists for +bring-up on a bench and for CI cross-compiles that only check the tree +builds; it is never passed by `.github/workflows/release.yml`, and +`tests/unit/test_release_workflow_production_key.py` fails if it ever is. +`tests/unit/test_production_key_gate.py` runs real configures against the +tree and pins each of the behaviours above. + +**Verification** uses whatever the keystore selected: `eos_image_verify_signature()` +takes the active key from `eos_keystore_get_active_key()` and checks the +Ed25519 signature over the signed header prefix (`EOS_IMG_SIGNED_LEN`). + ### 3.2 OTP/eFuse Key Storage For devices with one-time-programmable memory: @@ -319,41 +319,52 @@ Device OTP: security_version = 5 → ACCEPT (6 ≥ 5) ### 7.2 Enforcement Mechanism -```c -#if defined(EOS_BUILD_PRODUCTION) - /* Production keys — compiled from HSM-exported header */ - #include "eos_signing_key_production.h" -#elif defined(EOS_BUILD_STAGING) - #include "eos_signing_key_staging.h" -#else - /* Development key — well-known test key */ - #include "eos_signing_key_dev.h" -#endif -``` +There is one switch, and it is structural rather than advisory: + +- `core/keystore.c` compiles the development key under `#ifndef + EBLDR_PRODUCTION_KEY` and the generated `ebldr_production_key[]` under + `#else`. The two never coexist in one object. +- `CMakeLists.txt` defines `EBLDR_PRODUCTION_KEY` only when a key was given, + and refuses a Release build of a real board that gives none (§3.1). +- `.github/workflows/release.yml` passes the `EBLDR_PRODUCTION_KEY_HEX` + repository secret on every board configure; with the secret unset the value + is empty and the configure fails closed. After every firmware build it scans + each `.elf`, `.bin`, `.a` and `.o` under `build/` for the development key's + 32 bytes and fails the job on a hit. + +Staging keys are not modelled: a staging build is a production-shaped build +configured with a staging public key. ### 7.3 Development Key Policy | Rule | Rationale | |---|---| | Development private key is **committed** to the repository | Enables any developer to build and test signed images locally | -| Development key is **never** used in production | Production builds fail if development key header is detected | +| Development key is **never** used in production | A Release build of a real board refuses to configure without `EBLDR_PRODUCTION_KEY`, and the release workflow scans every artifact for the development key's bytes (§3.1, §7.2) | | Production key **never** appears in source control | Only the public key is embedded; private key stays in HSM | | CI pipeline uses **staging** key for integration tests | Tests signature verification without exposing production key | ### 7.4 Well-Known Development Key -The development key is intentionally public and must never be used for production: +The development key is the RFC 8032 section 7.1 TEST 1 key pair. Its secret +scalar is printed in the RFC, so images signed with it are not authenticated +by anyone in particular: ``` -Development Private Key (Ed25519, PEM): - MC4CAQAwBQYDK2VwBCIEIJ+DYvh6SEqVTm50DFtMDoQikQ2Ig35R6DIxQ/BV1Cxz - -Development Public Key SHA-256: - 9f836af87a484a954e6e74c5b4c0e842291d8883be51e832314bf055d42c73... +Public key (compiled in by core/keystore.c without EBLDR_PRODUCTION_KEY): + d75a980182b10ab7d54bfed3c964073a0ee172f3daa62325af021a68f707511a +Secret (RFC 8032 §7.1 TEST 1): + 9d61b19deffd5a60ba844af492ec2cc44449c5697b326919703bac031cae7f60 ⚠️ THIS KEY IS PUBLIC. Images signed with this key are NOT authenticated. ``` +`tools/gen_fw_update_test_sigs.py` signs the unit-test fixtures with it, and +`tests/unit/test_keystore.c` checks that the compiled-in bytes are exactly +this key (an earlier revision of the array was off the curve, so nothing +could verify against it at all). `cmake/ProductionKey.cmake` refuses it as a +value for `EBLDR_PRODUCTION_KEY`. + --- ## 8. Emergency Key Compromise Response From b26c45a6a30c62d2fa0c07f0d148b695d362673e Mon Sep 17 00:00:00 2001 From: kartikey1306 Date: Tue, 15 Sep 2026 01:32:32 +0530 Subject: [PATCH 13/23] test: drive the workflow guard with a synthetic document so its branches run on a green tree The check that no workflow cross-compiles a Release board without a key or the opt-out only ever took its skip branches on the real workflows, so a green run never executed the branch that reports a violation and the coverage report said so. The check is now a helper returning what it would flag, and a second test feeds it a synthetic workflow: one Release + real-board job with neither flag is reported, and the keyed, opted-out, Debug, host, no-board and not-a-job cases are not. That is the mutation check from the review, kept as a test. --- .../test_release_workflow_production_key.py | 83 +++++++++++++++---- 1 file changed, 66 insertions(+), 17 deletions(-) diff --git a/tests/unit/test_release_workflow_production_key.py b/tests/unit/test_release_workflow_production_key.py index 75449bb..9ed7d08 100644 --- a/tests/unit/test_release_workflow_production_key.py +++ b/tests/unit/test_release_workflow_production_key.py @@ -126,23 +126,72 @@ def test_the_scan_looks_for_the_key_keystore_actually_compiles_in(): "the scan step does not name the development key core/keystore.c compiles in") +def unguarded_release_board_configures(doc): + """Every `cmake -B` configure in `doc` that is a Release build of a real + board and passes neither a production key nor the explicit opt-out. + Such a configure fails under CMakeLists.txt's gate, and a CI job that + hits the gate is a job that has stopped testing the tree.""" + found = [] + if not isinstance(doc, dict) or not isinstance(doc.get("jobs"), dict): + return found + for job_id, job in doc["jobs"].items(): + if not isinstance(job, dict): + continue + for step in run_steps(job): + for block in configure_lines(step["run"]): + if board_of(block) in (None, "none") or not is_release_shaped(block): + continue + if ("-DEBLDR_PRODUCTION_KEY=" in block + or "-DEBLDR_ALLOW_DEV_KEY=ON" in block): + continue + found.append((job_id, board_of(block), block)) + return found + + def test_no_ci_workflow_cross_compiles_a_release_board_without_saying_so(): - """A Release build of a real board must either carry a key or opt into - the development key explicitly; otherwise its configure fails, and a CI - job that hits the gate is a job that stops testing the tree.""" for path in sorted(WORKFLOWS.glob("*.yml")): doc = yaml.safe_load(path.read_text(encoding="utf-8")) - if not isinstance(doc, dict) or "jobs" not in doc: - continue - for job_id, job in doc["jobs"].items(): - if not isinstance(job, dict): - continue - for step in run_steps(job): - for block in configure_lines(step["run"]): - if board_of(block) in (None, "none") or not is_release_shaped(block): - continue - assert ("-DEBLDR_PRODUCTION_KEY=" in block - or "-DEBLDR_ALLOW_DEV_KEY=ON" in block), ( - f"{path.name} job {job_id!r}: a Release build of " - f"{board_of(block)} with no key and no opt-out cannot " - f"configure:\n{block}") + for job_id, board, block in unguarded_release_board_configures(doc): + raise AssertionError( + f"{path.name} job {job_id!r}: a Release build of {board} with " + f"no key and no opt-out cannot configure:\n{block}") + + +def test_the_guard_flags_exactly_the_unguarded_release_board_configure(): + """The check above, driven with a synthetic workflow so its every branch + runs on a green tree: the one job that is Release + real board with + neither flag is reported, and nothing else is.""" + doc = yaml.safe_load(""" +jobs: + bad: + steps: + - run: | + cmake -B build -DEBLDR_BOARD=stm32f4 -DCMAKE_BUILD_TYPE=Release + cmake --build build + keyed: + steps: + - run: cmake -B build -DEBLDR_BOARD=stm32f4 -DEBLDR_PRODUCTION_KEY="x" -DCMAKE_BUILD_TYPE=Release + opted-out: + steps: + - run: | + cmake -B build -DEBLDR_BOARD=stm32f4 \\ + -DCMAKE_BUILD_TYPE=$BUILD_TYPE \\ + -DEBLDR_ALLOW_DEV_KEY=ON + debug: + steps: + - run: cmake -B build -DEBLDR_BOARD=stm32f4 -DCMAKE_BUILD_TYPE=Debug + host: + steps: + - run: cmake -B build -DEBLDR_BOARD=none -DCMAKE_BUILD_TYPE=Release + no-board: + steps: + - run: cmake -B build -DCMAKE_BUILD_TYPE=Release + not-a-job: 42 +""") + flagged = unguarded_release_board_configures(doc) + assert [(job, board) for job, board, _ in flagged] == [("bad", "stm32f4")] + + # Documents without a jobs mapping are not workflows and are ignored. + assert unguarded_release_board_configures(None) == [] + assert unguarded_release_board_configures({"name": "x"}) == [] + assert unguarded_release_board_configures({"jobs": "not a mapping"}) == [] From a7d35004abf7451880220624e56aef4e19e81ca7 Mon Sep 17 00:00:00 2001 From: kartikey1306 Date: Tue, 15 Sep 2026 02:16:13 +0530 Subject: [PATCH 14/23] build: gate every release-shaped build type, and refuse a key the verifier would Two findings from the review at 6a1d21f, both in the gate rather than the mechanism, both closed before this merges because this PR is what makes the anchor load-bearing. The gate matched the literal string "Release" and nothing else. Real configures of stm32f4 with RelWithDebInfo, MinSizeRel, "release", "RELEASE" and no CMAKE_BUILD_TYPE at all every one configured successfully and printed the development anchor as a STATUS line. MinSizeRel is the ordinary build type for a bootloader. Only Debug is not release-shaped -- every other value is optimised, an unset value on a cross build still gets -Os from this file, and a multi-config generator has no build type at configure time -- so the gate now asks "is this Debug?" and refuses everything else. The error names the build type it saw, or says there was none. EBLDR_PRODUCTION_KEY was checked for length and hex-ness and for not being the development key, and never for being a point on the curve -- the exact defect the development key had until #116, now reachable through one mistyped hex digit in a release secret: green build, green artifact scan, a status line saying "production key", and a fleet whose bootloader refuses every image it is ever offered. tools/check_production_key.py applies core/ed25519_verify.c's own acceptance rule in pure Python with no dependencies: decode per RFC 8032 5.1.3, [L]P == identity, P != identity, not the development key. cmake/ProductionKey.cmake runs it at configure when python3 is found and warns, naming what was not checked, when it is not. release.yml runs it on the secret in the validate job every firmware job needs, so a bad secret fails the release before a board is configured. docs/key_lifecycle.md no longer says "never"; it says what the gate covers and what it cannot see (a Debug build flashed to a device, an explicit EBLDR_ALLOW_DEV_KEY=ON, a fork that removes the gate). Verified by execution: configure, real board, no key, each of Release / RelWithDebInfo / MinSizeRel / release / RELEASE / unset -> refused by the gate Debug, debug -> configures MinSizeRel + EBLDR_ALLOW_DEV_KEY=ON -> configures host build, no board -> configures, no gate key = the pre-#116 off-curve bytes -> refused, "no point" key = the order-2 point ec ff..7f -> refused, "low order" key = RFC 8032 TEST 2 -> accepted tools/check_production_key.py against 15 vectors, each refusal checked for its stated reason -- two of my first vectors were 66 characters and were being refused for length, which the reason check caught. negative controls: gate reverted to the literal "Release" -> 3 tests fail; the curve check's result ignored -> 3 tests fail. Restored. pytest 121 passed; host build rc=0; ctest 32/32. Not done here, and stated: the six copies of the artifact scan in release.yml (review finding 3, Low) are unchanged -- lifting them into one tool is a follow-up, not a merge blocker. --- .github/workflows/release.yml | 10 ++ CHANGELOG.md | 1 + CMakeLists.txt | 30 ++++-- cmake/ProductionKey.cmake | 38 ++++++- docs/key_lifecycle.md | 2 +- tests/unit/test_check_production_key.py | 57 ++++++++++ tests/unit/test_production_key_gate.py | 57 ++++++++-- tools/check_production_key.py | 135 ++++++++++++++++++++++++ 8 files changed, 315 insertions(+), 15 deletions(-) create mode 100644 tests/unit/test_check_production_key.py create mode 100755 tools/check_production_key.py diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index eba7b90..4bbdc15 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -20,6 +20,16 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@v4 + - name: Refuse a production key the bootloader could not use + # Every firmware job below needs this job, so a bad secret fails the + # release here -- before a single board is configured -- rather than + # shipping a fleet whose bootloader refuses every image it is offered. + # cmake/ProductionKey.cmake repeats the check per board when python3 + # is present; this is the one place it is guaranteed to run, on the + # exact bytes the secret holds. A missing or empty secret fails too. + env: + KEY: ${{ secrets.EBLDR_PRODUCTION_KEY_HEX }} + run: python3 tools/check_production_key.py "$KEY" - name: Build & Test run: | cmake -B build -DEBLDR_BUILD_TESTS=ON -DEBLDR_HARDENING=ON diff --git a/CHANGELOG.md b/CHANGELOG.md index 2e26920..970199d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ ## [Unreleased] ### Security +- **The production-key gate covers every release-shaped build, and the key it accepts is checked against the curve.** The gate matched the literal string `Release` and nothing else, so `MinSizeRel`, `RelWithDebInfo`, `release`, `RELEASE` and an unset build type all configured a real board on the development anchor. It now exempts only `Debug`. Separately, `EBLDR_PRODUCTION_KEY` was checked for length and hex-ness but never for being a point on edwards25519 — the defect the development key itself had — so one mistyped hex digit in the release secret would have shipped a fleet that refuses every image, with a green build. `tools/check_production_key.py` applies the verifier's own rule (`[L]P == identity`, `P != identity`, not the development key) in pure Python; `cmake/ProductionKey.cmake` runs it at configure when python3 is present and warns in so many words when it is not, and `release.yml` runs it on the secret before any board is configured. - **A release can no longer be built on the development trust anchor.** `EBLDR_PRODUCTION_KEY` had no CMake option, nothing set it, and its `#else` branch declared an `extern` nothing defined -- so every firmware job in `release.yml` compiled the `#ifndef` branch and shipped the RFC 8032 test key as the anchor for every board without OTP, which is every board under `boards/`. The only guard was a `#warning`. Now: `-DEBLDR_PRODUCTION_KEY=<64 hex characters>` generates `ebldr_production_key[]` (`cmake/ProductionKey.cmake`, declared in `include/eos_production_key.h`) and selects the production branch of `core/keystore.c`; a Release build of a real board refuses to configure without it unless it says `-DEBLDR_ALLOW_DEV_KEY=ON`; the development key and malformed values are refused as production keys; `release.yml` passes the `EBLDR_PRODUCTION_KEY_HEX` secret on every board configure (empty until a maintainer provides it, so a release fails closed) and scans every built `.elf`/`.bin`/`.a`/`.o` for the development key's bytes. `tests/unit/test_keystore_production.c` compiles the production branch with a fixture key on every host build; `tests/unit/test_production_key_gate.py` and `test_release_workflow_production_key.py` pin the gate and the workflow. - **The compiled-in development trust anchor was not a point on the curve.** `core/keystore.c` `default_dev_key` was described as the RFC 8032 section 7.1 TEST 1 public key, but it diverged from that key at byte 21 and the bytes it held did not decode to a point on the curve, so no signature could ever verify against it. It is now the RFC key, and `tests/unit/test_keystore.c` checks that the anchor verifies the RFC's own signature. Behaviour change: a board with no OTP and no `EBLDR_PRODUCTION_KEY` goes from refusing every image (since #104 made signature verification unconditional at install) to accepting images signed with the public RFC test key -- the documented development intent of the `#warning` in `core/keystore.c`, and a key that must never reach a device. #120 tracks making that structural. - **Image header is now authenticated (header format v2).** `eos_image_verify_signature()` signed `hdr->hash` only — 32 of the header's 156 bytes. Everything else (`image_size`, `load_addr`, `entry_addr`, `flags`, `sig_type`, `image_version`) sat outside the signature, so an attacker holding a legitimately signed image could relocate it, move its entry point, or clear `EOS_IMG_FLAG_HASH_SHA256` to downgrade integrity checking from SHA-256 to forgeable CRC32 — all while keeping the signature valid. The signature now covers `EOS_IMG_SIGNED_LEN` (92) bytes: the whole header except `signature[]` itself. **Existing signed images must be re-signed.** diff --git a/CMakeLists.txt b/CMakeLists.txt index 1fcfbad..97b74fe 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -46,19 +46,35 @@ set(EBLDR_INCLUDE_DIR ${CMAKE_CURRENT_SOURCE_DIR}/include) # anyone's firmware. So a Release build of a real board refuses to configure # until it is given a key, or told in so many words that it is not a release. include(cmake/ProductionKey.cmake) +# +# What counts as a release. The gate used to match the literal string +# "Release" and nothing else, so MinSizeRel -- the ordinary build type for a +# bootloader -- RelWithDebInfo, "release", "RELEASE" and an unset build type +# all configured a real board around it. Only Debug is not release-shaped: +# every other value is optimised, an unset value on a cross build still gets +# -Os from this file, and a multi-config generator has no build type at all at +# configure time. So the gate asks "is this Debug?" and refuses everything +# else, rather than asking "is this spelled Release?". +string(TOUPPER "${CMAKE_BUILD_TYPE}" _ebldr_build_type) if(NOT EBLDR_PRODUCTION_KEY STREQUAL "") ebldr_write_production_key_source("${EBLDR_PRODUCTION_KEY}" "${CMAKE_BINARY_DIR}/generated/production_key.c") set(EBLDR_PRODUCTION_KEY_SOURCE "${CMAKE_BINARY_DIR}/generated/production_key.c") message(STATUS " Trust anchor: production key from EBLDR_PRODUCTION_KEY") -elseif(CMAKE_BUILD_TYPE STREQUAL "Release" AND NOT EBLDR_BOARD STREQUAL "none" - AND NOT EBLDR_ALLOW_DEV_KEY) +elseif(NOT EBLDR_BOARD STREQUAL "none" AND NOT EBLDR_ALLOW_DEV_KEY + AND NOT _ebldr_build_type STREQUAL "DEBUG") + if(CMAKE_BUILD_TYPE STREQUAL "") + set(_ebldr_build_desc "build with no CMAKE_BUILD_TYPE") + else() + set(_ebldr_build_desc "${CMAKE_BUILD_TYPE} build") + endif() message(FATAL_ERROR - "EBLDR_PRODUCTION_KEY is not set: a Release build of board '${EBLDR_BOARD}' " - "would compile in the RFC 8032 test key as its trust anchor, and anyone can " - "sign for that key. Pass -DEBLDR_PRODUCTION_KEY=<64 hex characters> (the raw " - "Ed25519 public key), or, for a bring-up or CI build that will never reach a " - "device, -DEBLDR_ALLOW_DEV_KEY=ON.") + "EBLDR_PRODUCTION_KEY is not set: a ${_ebldr_build_desc} of board " + "'${EBLDR_BOARD}' would compile in the RFC 8032 test key as its trust " + "anchor, and anyone can sign for that key. Only a Debug build is exempt. " + "Pass -DEBLDR_PRODUCTION_KEY=<64 hex characters> (the raw Ed25519 public " + "key), or, for a bring-up or CI build that will never reach a device, " + "-DEBLDR_ALLOW_DEV_KEY=ON.") else() message(STATUS " Trust anchor: RFC 8032 test key (development only; see core/keystore.c)") endif() diff --git a/cmake/ProductionKey.cmake b/cmake/ProductionKey.cmake index e5188d0..f7fc498 100644 --- a/cmake/ProductionKey.cmake +++ b/cmake/ProductionKey.cmake @@ -18,8 +18,23 @@ set(EBLDR_DEV_KEY_HEX "d75a980182b10ab7d54bfed3c964073a0ee172f3daa62325af021a68f707511a") +# Where this module lives, captured at include time: inside a function, +# CMAKE_CURRENT_LIST_DIR is the caller's directory, not this file's. +set(_EBLDR_PRODUCTION_KEY_MODULE_DIR "${CMAKE_CURRENT_LIST_DIR}") + # Fail the configure unless `hex` is a raw Ed25519 public key that is not the -# development key. +# development key -- and, when python3 is available, one the verifier would +# accept: a point on edwards25519, in the prime-order subgroup. +# +# The length and hex checks say nothing about whether the bytes are a key at +# all. The development key shipped for months decoding to no point on the +# curve, and one mistyped hex digit in a release secret reproduces that: the +# build is green, the artifact scan is green, the status line says +# "production key", and the device refuses every image it is ever offered. +# tools/check_production_key.py applies core/ed25519_verify.c's own rule, +# [L]P == identity and P != identity, in pure Python. CMake cannot do the +# field arithmetic itself, so the check needs python3; without it the +# configure warns, in so many words, about what was not checked. function(ebldr_check_production_key_hex hex) string(LENGTH "${hex}" _len) if(NOT _len EQUAL 64 OR NOT hex MATCHES "^[0-9a-fA-F]+$") @@ -34,6 +49,27 @@ function(ebldr_check_production_key_hex hex) "the development key, whose secret is published. It cannot be a " "production trust anchor.") endif() + find_package(Python3 COMPONENTS Interpreter QUIET) + if(Python3_Interpreter_FOUND) + execute_process( + COMMAND "${Python3_EXECUTABLE}" + "${_EBLDR_PRODUCTION_KEY_MODULE_DIR}/../tools/check_production_key.py" + "${hex}" + RESULT_VARIABLE _rc + OUTPUT_VARIABLE _out + ERROR_VARIABLE _err) + if(NOT _rc EQUAL 0) + message(FATAL_ERROR + "EBLDR_PRODUCTION_KEY is not a usable Ed25519 public key: ${_err}" + "A device built with it would refuse every firmware image.") + endif() + else() + message(WARNING + "python3 was not found, so EBLDR_PRODUCTION_KEY was NOT checked for " + "being a point in the prime-order subgroup of edwards25519. A key that " + "is not one ships a device that refuses every image. Run " + "tools/check_production_key.py by hand before trusting this build.") + endif() endfunction() # Write a C translation unit defining ebldr_production_key[] from `hex` to diff --git a/docs/key_lifecycle.md b/docs/key_lifecycle.md index 94bbe9f..42ffa1f 100644 --- a/docs/key_lifecycle.md +++ b/docs/key_lifecycle.md @@ -340,7 +340,7 @@ configured with a staging public key. | Rule | Rationale | |---|---| | Development private key is **committed** to the repository | Enables any developer to build and test signed images locally | -| Development key is **never** used in production | A Release build of a real board refuses to configure without `EBLDR_PRODUCTION_KEY`, and the release workflow scans every artifact for the development key's bytes (§3.1, §7.2) | +| Development key does not reach a production artifact **built through the gate** | Any build of a real board that is not `CMAKE_BUILD_TYPE=Debug` -- `Release`, `MinSizeRel`, `RelWithDebInfo`, any spelling, or no build type at all -- refuses to configure without `EBLDR_PRODUCTION_KEY` unless `EBLDR_ALLOW_DEV_KEY=ON` is passed explicitly; the key it accepts is checked to be a point in the prime-order subgroup (`tools/check_production_key.py`); and the release workflow validates the secret before any board is configured and scans every artifact for the development key's bytes (§3.1, §7.2). What the gate cannot see: a Debug build flashed to a device, a build that passes `EBLDR_ALLOW_DEV_KEY=ON`, or a fork that removes the gate. Those are policy, not mechanism. | | Production key **never** appears in source control | Only the public key is embedded; private key stays in HSM | | CI pipeline uses **staging** key for integration tests | Tests signature verification without exposing production key | diff --git a/tests/unit/test_check_production_key.py b/tests/unit/test_check_production_key.py new file mode 100644 index 0000000..6ed3b73 --- /dev/null +++ b/tests/unit/test_check_production_key.py @@ -0,0 +1,57 @@ +# SPDX-License-Identifier: MIT +# Copyright (c) 2026 EoS Project + +"""tools/check_production_key.py applies the verifier's acceptance rule to a +candidate production key before it is compiled into anything. These vectors +pin each branch of that rule; every refusal is checked for its *reason*, so a +key refused for the wrong reason (a mistyped vector, say) is a failure here +rather than a pass. +""" + +import sys +from pathlib import Path + +import pytest + +sys.path.insert(0, str(Path(__file__).resolve().parents[2] / "tools")) +from check_production_key import check_production_key_hex, main # noqa: E402 + +# RFC 8032 section 7.1 TEST 2 and TEST 3 public keys: real curve points. +TEST2 = "3d4017c3e843895a92b70aa74d1b7ebc9c982ccf2ec4968cc0cd55f12af4660c" +TEST3 = "fc51cd8e6218a1a38da47ed00230f0580816ed13ba3303ac5deb911548908025" +DEV = "d75a980182b10ab7d54bfed3c964073a0ee172f3daa62325af021a68f707511a" +# What core/keystore.c compiled in before eBoot#116: decodes to no point. +OFF_CURVE = "d75a980182b10ab7d54bfed3c964073a0ee172f3daa3f4a18c42c47684377725" + + +@pytest.mark.parametrize("key", [TEST2, TEST3, TEST2.upper()]) +def test_a_real_key_is_accepted(key): + check_production_key_hex(key) + + +@pytest.mark.parametrize("key,reason", [ + (DEV, "development key"), + (DEV.upper(), "development key"), + (OFF_CURVE, "no point on edwards25519"), + ("01" + "00" * 31, "identity"), # the identity point + ("ec" + "ff" * 30 + "7f", "low order"), # order 2 + ("00" * 31 + "80", "low order"), # order 4 + ("c7176a703d4dd84fba3c0b760d10670f2a2053fa2c39ccc64ec7fd7792ac037a", + "low order"), # order 8 + ("ff" * 31 + "7f", "not below p"), # y = 2^255 - 1 + ("ed" + "ff" * 30 + "7f", "not below p"), # y = p exactly + (TEST2[:-2], "exactly 64"), + (TEST2 + "00", "exactly 64"), + ("zz" + TEST2[2:], "non-hexadecimal"), +]) +def test_an_unusable_key_is_refused_for_the_stated_reason(key, reason): + with pytest.raises(ValueError) as exc: + check_production_key_hex(key) + assert reason in str(exc.value), str(exc.value) + + +def test_cli_exit_codes(capsys): + assert main(["check", TEST2]) == 0 + assert main(["check", OFF_CURVE]) == 1 + assert "refused" in capsys.readouterr().err + assert main(["check"]) == 2 diff --git a/tests/unit/test_production_key_gate.py b/tests/unit/test_production_key_gate.py index 04c2c65..badc4f9 100644 --- a/tests/unit/test_production_key_gate.py +++ b/tests/unit/test_production_key_gate.py @@ -58,12 +58,36 @@ def first_error(stderr): return next((b for b in blocks if b.startswith("CMake Error")), "") -def test_release_board_build_refuses_without_a_key(): - result, _ = configure("-DCMAKE_BUILD_TYPE=Release", "-DEBLDR_BOARD=stm32f4") +# Every build type that is not Debug is release-shaped: the optimised types +# under every spelling CMake accepts, and no build type at all -- a cross build +# still gets -Os from CMakeLists.txt, and a multi-config generator has no type +# at configure time. The gate used to match the literal "Release" only, and +# the other five configured a real board around it. +RELEASE_SHAPED = ["Release", "RelWithDebInfo", "MinSizeRel", + "release", "RELEASE", None] + + +@pytest.mark.parametrize("build_type", RELEASE_SHAPED, + ids=[t or "unset" for t in RELEASE_SHAPED]) +def test_release_shaped_board_build_refuses_without_a_key(build_type): + defs = ["-DEBLDR_BOARD=stm32f4"] + if build_type is not None: + defs.append("-DCMAKE_BUILD_TYPE=" + build_type) + result, generated = configure(*defs) assert result.returncode != 0 assert GATE_MESSAGE in result.stderr # The gate, not a missing toolchain or a board port, is what stops it. assert GATE_MESSAGE in first_error(result.stderr), result.stderr + assert "Only a Debug build is exempt" in result.stderr + assert generated is None + + +@pytest.mark.parametrize("build_type", ["Debug", "debug", "DEBUG"]) +def test_debug_board_build_is_the_one_exemption(build_type): + result, _ = configure("-DCMAKE_BUILD_TYPE=" + build_type, + "-DEBLDR_BOARD=stm32f4") + assert result.returncode == 0, result.stderr + assert GATE_MESSAGE not in result.stderr def test_release_board_build_can_say_it_is_not_a_release(): @@ -72,10 +96,8 @@ def test_release_board_build_can_say_it_is_not_a_release(): assert GATE_MESSAGE not in result.stderr -def test_debug_board_build_and_host_build_are_not_gated(): - # Neither is release-shaped: no key, no opt-out, no gate message. - result, _ = configure("-DCMAKE_BUILD_TYPE=Debug", "-DEBLDR_BOARD=stm32f4") - assert GATE_MESSAGE not in result.stderr +def test_host_build_is_not_gated(): + # No board, so nothing reaches a device: no key, no opt-out, no gate message. result, generated = configure("-DCMAKE_BUILD_TYPE=Release") assert result.returncode == 0, result.stderr assert GATE_MESSAGE not in result.stderr @@ -99,6 +121,29 @@ def test_malformed_key_is_refused(bad): assert generated is None +# The bytes core/keystore.c shipped before eBoot#116: 64 hex characters, not +# the development key, and no point on edwards25519. The length and dev-key +# checks accept them; only the curve check refuses them. +OFF_CURVE_KEY = "d75a980182b10ab7d54bfed3c964073a0ee172f3daa3f4a18c42c47684377725" +# The order-2 point. On the curve, refused by the verifier's subgroup check. +LOW_ORDER_KEY = "ec" + "ff" * 30 + "7f" + + +@pytest.mark.parametrize("key,reason", [ + (OFF_CURVE_KEY, "no point on edwards25519"), + (LOW_ORDER_KEY, "low order"), + ("01" + "00" * 31, "identity"), +]) +def test_key_the_verifier_would_refuse_is_refused_at_configure(key, reason): + result, generated = configure("-DCMAKE_BUILD_TYPE=Release", + "-DEBLDR_BOARD=stm32f4", + "-DEBLDR_PRODUCTION_KEY=" + key) + assert result.returncode != 0 + assert "not a usable Ed25519 public key" in result.stderr, result.stderr + assert reason in result.stderr, result.stderr + assert generated is None, "an unusable key must not be compiled in" + + def test_real_key_generates_the_anchor_source(): result, generated = configure("-DCMAKE_BUILD_TYPE=Release", "-DEBLDR_BOARD=stm32f4", diff --git a/tools/check_production_key.py b/tools/check_production_key.py new file mode 100755 index 0000000..8eb0cef --- /dev/null +++ b/tools/check_production_key.py @@ -0,0 +1,135 @@ +#!/usr/bin/env python3 +# SPDX-License-Identifier: MIT +# Copyright (c) 2026 EoS Project +"""Refuse a production trust anchor that the bootloader could not use. + +cmake/ProductionKey.cmake checks that EBLDR_PRODUCTION_KEY is 64 hexadecimal +characters and is not the development key. Neither check asks whether the bytes +are a public key at all. They can fail to be one in two ways that matter: + + * they decode to no point on edwards25519 -- the exact defect the shipped + development key had until it was fixed, and one mistyped hex digit in a + release secret reproduces it; + * they decode to a point of low order, which the verifier refuses + (core/ed25519_verify.c, public_key_is_valid_subgroup). + +A device built with such an anchor refuses every firmware image it is ever +offered, with a green build and a status line saying the production key is in +place. That fails closed, so it is not a compromise; it is still unrecoverable +in the field. This tool applies the verifier's own acceptance rule before the +key is compiled into anything: the encoding decodes to a point, [L]P is the +identity and P is not. Pure Python, no dependencies, so it can run anywhere a +release is cut. + +Usage: check_production_key.py <64 hex chars> exit 0 if acceptable +""" + +import sys + +P = 2**255 - 19 +D = (-121665 * pow(121666, P - 2, P)) % P +# The order of the prime-order subgroup: 2^252 + 27742317777372353535851937790883648493. +L = 2**252 + 27742317777372353535851937790883648493 +# sqrt(-1) mod p, used when recovering x. +SQRT_M1 = pow(2, (P - 1) // 4, P) + +# RFC 8032 section 7.1 TEST 1 public key: what core/keystore.c compiles in when +# no production key is given. Its secret is printed in the RFC. +DEV_KEY_HEX = "d75a980182b10ab7d54bfed3c964073a0ee172f3daa62325af021a68f707511a" + +IDENTITY = (0, 1, 1, 0) # extended coordinates (X, Y, Z, T) + + +def _decode(key: bytes): + """RFC 8032 section 5.1.3. Returns (x, y) or raises ValueError.""" + if len(key) != 32: + raise ValueError("a public key is exactly 32 bytes") + y = int.from_bytes(key, "little") + sign = y >> 255 + y &= (1 << 255) - 1 + if y >= P: + raise ValueError("y coordinate is not below p (non-canonical encoding)") + u = (y * y - 1) % P + v = (D * y * y + 1) % P + x = (u * pow(v, 3, P) * pow(u * pow(v, 7, P), (P - 5) // 8, P)) % P + vx2 = (v * x * x) % P + if vx2 == u: + pass + elif vx2 == (-u) % P: + x = (x * SQRT_M1) % P + else: + raise ValueError("the bytes decode to no point on edwards25519") + if x == 0 and sign == 1: + raise ValueError("x is zero but the sign bit is set (invalid encoding)") + if (x & 1) != sign: + x = P - x + return x, y + + +def _add(p, q): + x1, y1, z1, t1 = p + x2, y2, z2, t2 = q + a = (y1 - x1) * (y2 - x2) % P + b = (y1 + x1) * (y2 + x2) % P + c = 2 * t1 * t2 * D % P + d = 2 * z1 * z2 % P + e, f, g, h = b - a, d - c, d + c, b + a + return (e * f % P, g * h % P, f * g % P, e * h % P) + + +def _mul(point, n): + result, addend = IDENTITY, point + while n: + if n & 1: + result = _add(result, addend) + addend = _add(addend, addend) + n >>= 1 + return result + + +def _is_identity(p): + x, y, z, _ = p + return x % P == 0 and (y - z) % P == 0 + + +def check_production_key_hex(hex_key: str) -> None: + """Raise ValueError with a one-line reason if hex_key is not acceptable.""" + hex_key = hex_key.strip() + if len(hex_key) != 64: + raise ValueError( + f"EBLDR_PRODUCTION_KEY must be exactly 64 hexadecimal characters; " + f"got {len(hex_key)}") + try: + key = bytes.fromhex(hex_key) + except ValueError: + raise ValueError("EBLDR_PRODUCTION_KEY contains a non-hexadecimal character") + if hex_key.lower() == DEV_KEY_HEX: + raise ValueError( + "EBLDR_PRODUCTION_KEY is the RFC 8032 section 7.1 TEST 1 public key -- " + "the development key, whose secret is published") + x, y = _decode(key) + point = (x, y, 1, x * y % P) + if _is_identity(point): + raise ValueError("the key is the identity point; any signature verifies " + "against it") + if not _is_identity(_mul(point, L)): + raise ValueError("the key is a point of low order; the verifier refuses " + "it and every image would be rejected") + + +def main(argv) -> int: + if len(argv) != 2: + print(__doc__.strip().splitlines()[-1], file=sys.stderr) + return 2 + try: + check_production_key_hex(argv[1]) + except ValueError as e: + print(f"production key refused: {e}", file=sys.stderr) + return 1 + print("production key accepted: a point in the prime-order subgroup of " + "edwards25519, and not the development key") + return 0 + + +if __name__ == "__main__": + sys.exit(main(sys.argv)) From 17709bd8107b6d60a2f0037ff0878c00e8c30f8f Mon Sep 17 00:00:00 2001 From: kartikey1306 Date: Tue, 15 Sep 2026 02:27:25 +0530 Subject: [PATCH 15/23] ci(build): the STM32F4 compile check opts into the development key The previous commit made the trust-anchor gate refuse every build of a real board that is not Debug, including one with no CMAKE_BUILD_TYPE. build.yml's "Cross-compile STM32F4" job is exactly that: -DEBLDR_BOARD=stm32f4 with no build type and no opt-out, so the gate refused it and the job went red on this PR. The gate was right and the reply on this PR was wrong -- it said no CI leg was affected, having looked for lines that set CMAKE_BUILD_TYPE and not for jobs that set none. The job is a compile check whose output reaches no device, the same shape as ci.yml's ARM leg, which already passes -DEBLDR_ALLOW_DEV_KEY=ON. It now does too, with the comment that travels with every copy of that flag: never use it in a workflow that publishes an artifact. Verified: configure of stm32f4 with no build type and the opt-out passes the gate and reports the development anchor; the gate and release-workflow test modules still pass. Every other board-configuring job was audited by grep for EBLDR_BOARD= and carries Debug, the opt-out, or a production key. --- .github/workflows/build.yml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index e3ea60c..1ed0d20 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -43,10 +43,16 @@ jobs: - name: Configure run: | + # EBLDR_ALLOW_DEV_KEY: this job sets no CMAKE_BUILD_TYPE, which the + # trust-anchor gate treats as release-shaped (the cross branch adds + # -Os regardless), so it refuses to configure a real board on the + # RFC 8032 test key. This is a compile check whose output reaches no + # device. Never use this flag in a workflow that publishes an artifact. cmake -B build-arm \ -DCMAKE_SYSTEM_NAME=Generic \ -DCMAKE_C_COMPILER=arm-none-eabi-gcc \ -DEBLDR_BOARD=stm32f4 \ + -DEBLDR_ALLOW_DEV_KEY=ON \ -DCMAKE_C_FLAGS="-mcpu=cortex-m4 -mthumb -mfloat-abi=hard -mfpu=fpv4-sp-d16 -specs=nosys.specs" - name: Build From a17d27210db2674bd232d7c7bda884ed2d2dcde4 Mon Sep 17 00:00:00 2001 From: kartikey1306 Date: Tue, 15 Sep 2026 02:32:09 +0530 Subject: [PATCH 16/23] ci(release): one artifact scanner that decodes what it scans, in place of six copies Finding 3 from the review of 6a1d21f, the one f011dc5 deferred. The scan for the development trust anchor was six inline copies in release.yml, each reading .elf .bin .a .o, while the Collect artifacts step in the same jobs shipped .hex .uf2 and .efi too. Two lists kept by hand beside each other, only one of them pinned by a test. It was more than a list drift. Intel HEX is ASCII, and a UF2 file is 512-byte blocks with headers, so grepping either for the key's raw bytes finds nothing even when the key is in the image. Extending the suffix list without decoding would have made the scan open those files and report them clean, which is worse than not opening them. tools/check_no_dev_anchor.py is the one copy. It decodes Intel HEX (record types 00/01/02/04; anything else is an error, not a skip) and UF2 (magic, payload size, address per block) to the image they encode, merges records and blocks into contiguous runs so a key straddling two of them is one search, and reads the raw formats directly. A file with a scanned suffix that cannot be decoded is reported as a failure, because a file the scanner could not read is a file it did not check. release.yml's six scan steps each call it. is_release_shaped() in the workflow test, which still matched the literal "Release", now mirrors the gate as 302fc5f left it: any optimised type, case-insensitive, or none named at all. tests/unit/test_release_workflow_production_key.py: - every scan step is exactly one call to the tool - the tool names the key core/keystore.c compiles in - the tool's suffix set covers every -name glob in every Collect artifacts step, parsed from the YAML, so the two cannot drift apart - a key placed to straddle a 16-byte HEX record and a 256-byte UF2 block is found in .hex and .uf2 -- and the same test first asserts a raw search of those files returns False, so the decoding is demonstrated to be load-bearing rather than assumed - a clean tree passes; an undecodable .hex fails Negative controls, reverting one piece and keeping the tests: scanner grepping .hex/.uf2 raw instead of decoding -> 2 of 9 fail scanner suffix set missing .hex -> 3 of 9 fail one inline scan restored in release.yml -> 1 of 9 fail Measured at this head: pytest tests/ 126 passed; ctest 32/32; all 17 workflows parse and release.yml's six scan steps and every firmware job's needs: are asserted from the parsed structure. --- .github/workflows/release.yml | 102 +----------- .../test_release_workflow_production_key.py | 114 ++++++++++++- tools/check_no_dev_anchor.py | 155 ++++++++++++++++++ 3 files changed, 268 insertions(+), 103 deletions(-) create mode 100755 tools/check_no_dev_anchor.py diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 4bbdc15..b66c223 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -68,22 +68,7 @@ jobs: cmake -B build -DEBLDR_BOARD=${{ matrix.board }} -DEBLDR_PRODUCTION_KEY="${{ secrets.EBLDR_PRODUCTION_KEY_HEX }}" -DCMAKE_BUILD_TYPE=Release cmake --build build --parallel - name: Refuse an artifact that embeds the development anchor - run: | - python3 - <<'PY' - import pathlib, sys - # RFC 8032 section 7.1 TEST 1 public key: what core/keystore.c falls - # back to when EBLDR_PRODUCTION_KEY is unset. Its secret is published. - dev = bytes.fromhex( - "d75a980182b10ab7d54bfed3c964073a0ee172f3daa62325af021a68f707511a") - hits = sorted(str(p) for p in pathlib.Path("build").rglob("*") - if p.is_file() and p.suffix in (".elf", ".bin", ".a", ".o") - and dev in p.read_bytes()) - for h in hits: - print(f"::error file={h}::embeds the development trust anchor") - if hits: - sys.exit(1) - print("no artifact under build/ contains the development key") - PY + run: python3 tools/check_no_dev_anchor.py build - name: Collect artifacts run: | mkdir -p fw @@ -116,22 +101,7 @@ jobs: -DCMAKE_BUILD_TYPE=Release cmake --build build --parallel - name: Refuse an artifact that embeds the development anchor - run: | - python3 - <<'PY' - import pathlib, sys - # RFC 8032 section 7.1 TEST 1 public key: what core/keystore.c falls - # back to when EBLDR_PRODUCTION_KEY is unset. Its secret is published. - dev = bytes.fromhex( - "d75a980182b10ab7d54bfed3c964073a0ee172f3daa62325af021a68f707511a") - hits = sorted(str(p) for p in pathlib.Path("build").rglob("*") - if p.is_file() and p.suffix in (".elf", ".bin", ".a", ".o") - and dev in p.read_bytes()) - for h in hits: - print(f"::error file={h}::embeds the development trust anchor") - if hits: - sys.exit(1) - print("no artifact under build/ contains the development key") - PY + run: python3 tools/check_no_dev_anchor.py build - name: Collect artifacts run: | mkdir -p fw @@ -163,22 +133,7 @@ jobs: -DCMAKE_BUILD_TYPE=Release cmake --build build --parallel - name: Refuse an artifact that embeds the development anchor - run: | - python3 - <<'PY' - import pathlib, sys - # RFC 8032 section 7.1 TEST 1 public key: what core/keystore.c falls - # back to when EBLDR_PRODUCTION_KEY is unset. Its secret is published. - dev = bytes.fromhex( - "d75a980182b10ab7d54bfed3c964073a0ee172f3daa62325af021a68f707511a") - hits = sorted(str(p) for p in pathlib.Path("build").rglob("*") - if p.is_file() and p.suffix in (".elf", ".bin", ".a", ".o") - and dev in p.read_bytes()) - for h in hits: - print(f"::error file={h}::embeds the development trust anchor") - if hits: - sys.exit(1) - print("no artifact under build/ contains the development key") - PY + run: python3 tools/check_no_dev_anchor.py build - name: Collect artifacts run: | mkdir -p fw @@ -225,22 +180,7 @@ jobs: cmake -B build -DEBLDR_BOARD=esp32 -DEBLDR_PRODUCTION_KEY="${{ secrets.EBLDR_PRODUCTION_KEY_HEX }}" -DCMAKE_BUILD_TYPE=Release cmake --build build --parallel - name: Refuse an artifact that embeds the development anchor - run: | - python3 - <<'PY' - import pathlib, sys - # RFC 8032 section 7.1 TEST 1 public key: what core/keystore.c falls - # back to when EBLDR_PRODUCTION_KEY is unset. Its secret is published. - dev = bytes.fromhex( - "d75a980182b10ab7d54bfed3c964073a0ee172f3daa62325af021a68f707511a") - hits = sorted(str(p) for p in pathlib.Path("build").rglob("*") - if p.is_file() and p.suffix in (".elf", ".bin", ".a", ".o") - and dev in p.read_bytes()) - for h in hits: - print(f"::error file={h}::embeds the development trust anchor") - if hits: - sys.exit(1) - print("no artifact under build/ contains the development key") - PY + run: python3 tools/check_no_dev_anchor.py build - name: Collect artifacts run: | mkdir -p fw @@ -285,22 +225,7 @@ jobs: cmake -B build -DEBLDR_BOARD=esp32c3 -DEBLDR_PRODUCTION_KEY="${{ secrets.EBLDR_PRODUCTION_KEY_HEX }}" -DCMAKE_BUILD_TYPE=Release cmake --build build --parallel - name: Refuse an artifact that embeds the development anchor - run: | - python3 - <<'PY' - import pathlib, sys - # RFC 8032 section 7.1 TEST 1 public key: what core/keystore.c falls - # back to when EBLDR_PRODUCTION_KEY is unset. Its secret is published. - dev = bytes.fromhex( - "d75a980182b10ab7d54bfed3c964073a0ee172f3daa62325af021a68f707511a") - hits = sorted(str(p) for p in pathlib.Path("build").rglob("*") - if p.is_file() and p.suffix in (".elf", ".bin", ".a", ".o") - and dev in p.read_bytes()) - for h in hits: - print(f"::error file={h}::embeds the development trust anchor") - if hits: - sys.exit(1) - print("no artifact under build/ contains the development key") - PY + run: python3 tools/check_no_dev_anchor.py build - name: Collect artifacts run: | mkdir -p fw @@ -329,22 +254,7 @@ jobs: cmake -B build -DEBLDR_BOARD=x86_64_efi -DEBLDR_PRODUCTION_KEY="${{ secrets.EBLDR_PRODUCTION_KEY_HEX }}" -DCMAKE_BUILD_TYPE=Release cmake --build build --parallel - name: Refuse an artifact that embeds the development anchor - run: | - python3 - <<'PY' - import pathlib, sys - # RFC 8032 section 7.1 TEST 1 public key: what core/keystore.c falls - # back to when EBLDR_PRODUCTION_KEY is unset. Its secret is published. - dev = bytes.fromhex( - "d75a980182b10ab7d54bfed3c964073a0ee172f3daa62325af021a68f707511a") - hits = sorted(str(p) for p in pathlib.Path("build").rglob("*") - if p.is_file() and p.suffix in (".elf", ".bin", ".a", ".o") - and dev in p.read_bytes()) - for h in hits: - print(f"::error file={h}::embeds the development trust anchor") - if hits: - sys.exit(1) - print("no artifact under build/ contains the development key") - PY + run: python3 tools/check_no_dev_anchor.py build - name: Collect artifacts run: | mkdir -p fw diff --git a/tests/unit/test_release_workflow_production_key.py b/tests/unit/test_release_workflow_production_key.py index 9ed7d08..13a60d1 100644 --- a/tests/unit/test_release_workflow_production_key.py +++ b/tests/unit/test_release_workflow_production_key.py @@ -56,8 +56,16 @@ def board_of(block): return m.group(1) if m else None +# Mirrors the gate in CMakeLists.txt: any optimised build type, case-insensitive, +# or none named at all. The gate test parametrises the same spellings, so if +# the two drift it fails there rather than going quiet here. +RELEASE_SHAPED = re.compile( + r"-DCMAKE_BUILD_TYPE=(Release|RelWithDebInfo|MinSizeRel)\b", re.IGNORECASE) + def is_release_shaped(block): - return "-DCMAKE_BUILD_TYPE=Release" in block or "$BUILD_TYPE" in block + if "$BUILD_TYPE" in block or RELEASE_SHAPED.search(block): + return True + return "-DCMAKE_BUILD_TYPE=" not in block def dev_key_from_keystore(): @@ -113,17 +121,109 @@ def test_every_release_firmware_job_scans_for_the_development_key(): f"before artifacts are collected") -def test_the_scan_looks_for_the_key_keystore_actually_compiles_in(): +SCAN_TOOL = "tools/check_no_dev_anchor.py" + + +def scan_tool(): + import importlib.util + spec = importlib.util.spec_from_file_location("check_no_dev_anchor", REPO_ROOT / SCAN_TOOL) + mod = importlib.util.module_from_spec(spec) + spec.loader.exec_module(mod) + return mod + + +def test_every_scan_step_calls_the_one_scanner(): + """Six jobs used to carry the scan inline, each a copy, and the copies + drifted from the artifact list. One tool, called six times, cannot.""" doc = load("release.yml") - scans = [s["run"] for job in doc["jobs"].values() for s in run_steps(job) + scans = [s["run"].strip() for job in doc["jobs"].values() for s in run_steps(job) if s.get("name") == SCAN_STEP] - assert scans + assert len(scans) == 6, scans + for run in scans: + assert run == f"python3 {SCAN_TOOL} build", run + + +def test_the_scanner_looks_for_the_key_keystore_actually_compiles_in(): keystore = dev_key_from_keystore() cmake = dev_key_from_cmake() assert keystore == cmake, "core/keystore.c and cmake/ProductionKey.cmake disagree on the development key" - for run in scans: - assert keystore in run.replace("\n", "").replace(" ", "").replace('"', ""), ( - "the scan step does not name the development key core/keystore.c compiles in") + assert scan_tool().DEV_KEY.hex() == keystore, ( + "the scanner does not name the development key core/keystore.c compiles in") + + +def collected_suffixes(doc): + """Every `-name \"*.ext\"` glob in every Collect artifacts step.""" + found = set() + for job in doc["jobs"].values(): + for s in run_steps(job): + if s.get("name") == "Collect artifacts": + found |= set(re.findall(r'-name "\*(\.[a-z0-9]+)"', s["run"])) + return found + + +def test_the_scanner_covers_every_suffix_the_workflow_ships(): + """The scan list and the artifact list were maintained by hand beside each + other and disagreed: the scan read .elf .bin .a .o while Collect shipped + .hex .uf2 .efi too. This pins the one against the other.""" + shipped = collected_suffixes(load("release.yml")) + assert shipped >= {".elf", ".bin", ".a", ".hex", ".uf2", ".efi"}, shipped + missing = shipped - scan_tool().SUFFIXES + assert not missing, f"release.yml ships {sorted(missing)} but the scanner never opens them" + + +def _intel_hex(data, base=0x08000000): + def rec(t, addr, payload): + body = bytes([len(payload), (addr >> 8) & 0xFF, addr & 0xFF, t]) + payload + return ":" + (body + bytes([(-sum(body)) & 0xFF])).hex().upper() + lines = [rec(0x04, 0, (base >> 16).to_bytes(2, "big"))] + lines += [rec(0x00, (base & 0xFFFF) + off, data[off:off + 16]) for off in range(0, len(data), 16)] + return "\n".join(lines + [rec(0x01, 0, b"")]) + "\n" + + +def _uf2(data, base=0x2000): + import struct + blocks = [data[i:i + 256] for i in range(0, len(data), 256)] + return b"".join( + struct.pack(" 476: + raise ValueError(f"UF2 payload size {size} exceeds the block") + chunks[addr] = blob[off + 32:off + 32 + size] + return _runs(chunks) + + +def _runs(chunks): + """Merge address->bytes chunks into contiguous runs, so a key that + straddles two records or two blocks is still one search.""" + runs, cur, cur_addr = [], bytearray(), None + for addr in sorted(chunks): + data = chunks[addr] + if cur_addr is not None and addr == cur_addr + len(cur): + cur += data + else: + if cur: + runs.append(bytes(cur)) + cur, cur_addr = bytearray(data), addr + if cur: + runs.append(bytes(cur)) + return runs + + +def images_of(path): + """The byte images to search for a given artifact.""" + if path.suffix == ".hex": + return decode_intel_hex(path.read_text(encoding="ascii", errors="strict")) + if path.suffix == ".uf2": + return decode_uf2(path.read_bytes()) + return [path.read_bytes()] + + +def scan(roots): + hits, undecodable = [], [] + for root in roots: + for p in sorted(pathlib.Path(root).rglob("*")): + if not p.is_file() or p.suffix not in SUFFIXES: + continue + try: + if any(DEV_KEY in img for img in images_of(p)): + hits.append(str(p)) + except (ValueError, UnicodeDecodeError) as e: + undecodable.append((str(p), str(e))) + return hits, undecodable + + +def main(argv): + roots = argv[1:] or ["build"] + hits, undecodable = scan(roots) + for h in hits: + print(f"::error file={h}::embeds the development trust anchor") + for path, why in undecodable: + # A file the scan could not read is a file the scan did not check. + print(f"::error file={path}::could not be decoded for the anchor scan: {why}") + if hits or undecodable: + return 1 + print(f"no artifact under {', '.join(roots)} contains the development key " + f"(suffixes searched: {' '.join(sorted(SUFFIXES))})") + return 0 + + +if __name__ == "__main__": + sys.exit(main(sys.argv)) From 723a6314de041b93d51d271769e01b0b69430879 Mon Sep 17 00:00:00 2001 From: kartikey1306 Date: Tue, 15 Sep 2026 02:32:09 +0530 Subject: [PATCH 17/23] build: a production key that cannot be checked is refused, not warned about 302fc5f wires tools/check_production_key.py into the configure-time gate through execute_process, and when python3 is not found it emits a WARNING and continues: the key is compiled in unchecked. The review's finding 2 names why that is the wrong branch -- the gate is "the only control for anyone building a device image outside this workflow: a vendor, a downstream fork, a board bring-up that becomes a product" -- and a warning scrolls past. .ai/security.md: a verification step that cannot run must fail, not pass. Now FATAL_ERROR, saying what to install. Development builds pass no EBLDR_PRODUCTION_KEY and never reach this branch, confirmed below. production key, -DCMAKE_DISABLE_FIND_PACKAGE_Python3=TRUE -> rc 1, "python3 was not found", no CMake Warning, nothing generated production key, python3 findable -> rc 0 Debug board build, python3 unfindable -> rc 0 Negative control: with the WARNING branch restored, the new test fails (1 failed). pytest tests/ 126 passed at this head. --- cmake/ProductionKey.cmake | 16 +++++++++++----- tests/unit/test_production_key_gate.py | 19 +++++++++++++++++++ 2 files changed, 30 insertions(+), 5 deletions(-) diff --git a/cmake/ProductionKey.cmake b/cmake/ProductionKey.cmake index f7fc498..a867c8c 100644 --- a/cmake/ProductionKey.cmake +++ b/cmake/ProductionKey.cmake @@ -64,11 +64,17 @@ function(ebldr_check_production_key_hex hex) "A device built with it would refuse every firmware image.") endif() else() - message(WARNING - "python3 was not found, so EBLDR_PRODUCTION_KEY was NOT checked for " - "being a point in the prime-order subgroup of edwards25519. A key that " - "is not one ships a device that refuses every image. Run " - "tools/check_production_key.py by hand before trusting this build.") + # Not a warning. This gate is the only control for anyone building a + # device image outside release.yml, and a warning scrolls past. A + # production-key configure that cannot validate its key does not + # compile an unchecked key in; it stops, and says what to install. + # Development builds (no EBLDR_PRODUCTION_KEY) never reach this. + message(FATAL_ERROR + "EBLDR_PRODUCTION_KEY was given but python3 was not found, so it cannot " + "be checked for being a point in the prime-order subgroup of " + "edwards25519. A key that is not one ships a device that refuses every " + "image, so the key is not compiled in unchecked. Install python3, or " + "configure on a machine that has it.") endif() endfunction() diff --git a/tests/unit/test_production_key_gate.py b/tests/unit/test_production_key_gate.py index badc4f9..5f01a26 100644 --- a/tests/unit/test_production_key_gate.py +++ b/tests/unit/test_production_key_gate.py @@ -144,6 +144,25 @@ def test_key_the_verifier_would_refuse_is_refused_at_configure(key, reason): assert generated is None, "an unusable key must not be compiled in" +def test_production_key_without_python_refuses_rather_than_skipping_the_check(): + """The point check runs in Python. A production-key configure on a machine + with no python3 must stop, not warn and compile an unchecked key in: this + gate is the only control for a build made outside release.yml, and a + warning scrolls past. CMAKE_DISABLE_FIND_PACKAGE_Python3 is how CMake + itself simulates the interpreter being absent.""" + result, generated = configure("-DCMAKE_BUILD_TYPE=Release", "-DEBLDR_BOARD=stm32f4", + "-DEBLDR_PRODUCTION_KEY=" + GOOD_KEY, + "-DCMAKE_DISABLE_FIND_PACKAGE_Python3=TRUE") + assert result.returncode != 0, "a production key was compiled in unchecked" + assert "python3 was not found" in result.stderr, result.stderr + assert "CMake Warning" not in result.stderr, "it must refuse, not warn" + assert generated is None + # A development build never runs the check, so it is unaffected. + result, _ = configure("-DCMAKE_BUILD_TYPE=Debug", "-DEBLDR_BOARD=stm32f4", + "-DCMAKE_DISABLE_FIND_PACKAGE_Python3=TRUE") + assert result.returncode == 0, result.stderr + + def test_real_key_generates_the_anchor_source(): result, generated = configure("-DCMAKE_BUILD_TYPE=Release", "-DEBLDR_BOARD=stm32f4", From 7ddc735d0a16dd62a28dc2513ed9650dbc19b0af Mon Sep 17 00:00:00 2001 From: kartikey1306 Date: Tue, 15 Sep 2026 02:50:09 +0530 Subject: [PATCH 18/23] build: cross-compile the production-key branch before a tag, and say which way a bad key fails The three findings from the review of 8d09efb, none blocking, all taken. 1. (Medium) No pre-merge job compiled the production-key branch on any target. After f011dc5, EBLDR_PRODUCTION_KEY appeared only in release.yml; both jobs that configure a real board pre-merge passed -DEBLDR_ALLOW_DEV_KEY=ON, so build/generated/production_key.c and the #ifdef EBLDR_PRODUCTION_KEY half of core/keystore.c were first cross-compiled when a tag was pushed, inside the workflow that publishes. A link or section-placement fault in the generated TU would have surfaced during a release. build.yml's Cross-compile STM32F4 leg now passes the RFC 8032 TEST 2 public key with -DCMAKE_BUILD_TYPE=Release instead of the opt-out -- a genuine curve point the configure-time check accepts, the same fixture the host tests use, and one whose secret is published, which is fine for a job whose output reaches no device. ci.yml's ARM leg still compiles the dev-key branch, so both halves of keystore.c are cross-compiled on every PR. A new step proves it rather than assuming it: generated/production_key.c exists, its object was built, and the object carries the fixture key and not the dev key. Locally, with the host compiler standing in for arm-none-eabi: configure rc 0, generated TU present, eboot_core builds with 0 errors, production_key.c.o: fixture key present, dev key absent. 2. (Low) tools/check_no_dev_anchor.py indexed rec[0..3] before checking the record had them, so a `:00` line escaped as an IndexError traceback -- still exit 1, so it failed closed, but by accident, and the file-annotated "could not be decoded" line the tool promises was lost. Now a length check before unpacking, and the record's own byte count is checked against its actual length; IndexError and struct.error are caught with the rest. Separately, p.suffix was compared case-sensitively, so a .BIN with the dev key in it was skipped and the tree reported clean. Case-folded. 3. (Low) tools/check_production_key.py said "a point of low order" for every point outside the prime-order subgroup. Two very different things land there. A genuine low-order point (order 2, 4, 8) makes every signature verify -- fails open, an attack vector. A point of order 2L, 4L or 8L is where a mistyped hex digit lands about half the time; the verifier refuses it and every image is rejected -- fails closed, a typo. The message pointed a release engineer at the first when the cause was the second, and no test covered the mixed case, which is why the wording was never challenged. [8]P == identity now tells them apart and each gets its own sentence. Reviewer's 0300...00 and the dev key with its last digit changed both now report "not in the prime-order subgroup -- a mistyped hex digit usually lands here"; the order-2/4/8 vectors still report low order. Negative controls, one piece reverted at a time, tests kept: scanner at 8d09efb -> 2 of 11 fail (truncated record, .BIN) checker at 8d09efb -> 3 of 19 fail (0300..00, mistyped dev key, class-distinction test) At this head: pytest tests/ 131 passed; ctest 32/32; 17 workflows parse; build.yml's configure asserted from non-comment lines of the parsed step (the opt-out flag is named in the comment explaining why it is gone, which is exactly the kind of grep false-match to check for). NOT RUN: the ARM cross-compile itself (no arm-none-eabi-gcc here). The step that proves the branch was compiled is the one CI runs; what was run locally is the same configure and build with the host compiler. --- .github/workflows/build.yml | 34 +++++++++++++++---- tests/unit/test_check_production_key.py | 22 ++++++++++++ .../test_release_workflow_production_key.py | 27 +++++++++++++++ tools/check_no_dev_anchor.py | 20 ++++++++--- tools/check_production_key.py | 18 ++++++++-- 5 files changed, 107 insertions(+), 14 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 1ed0d20..0eb54e5 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -43,20 +43,42 @@ jobs: - name: Configure run: | - # EBLDR_ALLOW_DEV_KEY: this job sets no CMAKE_BUILD_TYPE, which the - # trust-anchor gate treats as release-shaped (the cross branch adds - # -Os regardless), so it refuses to configure a real board on the - # RFC 8032 test key. This is a compile check whose output reaches no - # device. Never use this flag in a workflow that publishes an artifact. + # A production key, not the dev-key opt-out. With the trust-anchor gate + # in place, every other pre-merge job that configures a real board + # passes -DEBLDR_ALLOW_DEV_KEY=ON, so the branch a real device takes -- + # build/generated/production_key.c and the #ifdef EBLDR_PRODUCTION_KEY + # half of core/keystore.c -- was first cross-compiled inside release.yml, + # when a tag was pushed. A link or section-placement fault in the + # generated TU would have surfaced during a release, not a review. + # The key is RFC 8032 section 7.1 TEST 2's public key: a genuine curve + # point (the configure-time check runs on it), the same fixture the host + # tests use, and one whose secret is published -- this job's output + # reaches no device. ci.yml's ARM leg still compiles the dev-key branch. cmake -B build-arm \ -DCMAKE_SYSTEM_NAME=Generic \ -DCMAKE_C_COMPILER=arm-none-eabi-gcc \ + -DCMAKE_BUILD_TYPE=Release \ -DEBLDR_BOARD=stm32f4 \ - -DEBLDR_ALLOW_DEV_KEY=ON \ + -DEBLDR_PRODUCTION_KEY=3d4017c3e843895a92b70aa74d1b7ebc9c982ccf2ec4968cc0cd55f12af4660c \ -DCMAKE_C_FLAGS="-mcpu=cortex-m4 -mthumb -mfloat-abi=hard -mfpu=fpv4-sp-d16 -specs=nosys.specs" - name: Build run: cmake --build build-arm --parallel + - name: The production-key branch was compiled, not the dev key + run: | + test -f build-arm/generated/production_key.c + obj=$(find build-arm -name 'production_key.c.o' | head -1) + test -n "$obj" || { echo '::error::generated/production_key.c was not compiled'; exit 1; } + # the object must carry the fixture key and must not carry the dev key + python3 - "$obj" <<'PY' + import sys + o = open(sys.argv[1], 'rb').read() + fixture = bytes.fromhex('3d4017c3e843895a92b70aa74d1b7ebc9c982ccf2ec4968cc0cd55f12af4660c') + dev = bytes.fromhex('d75a980182b10ab7d54bfed3c964073a0ee172f3daa62325af021a68f707511a') + assert fixture in o, 'the configured production key is not in the object' + assert dev not in o, 'the development key is in the production-key object' + print('production_key.c.o carries the fixture key and not the dev key') + PY - name: Report size run: arm-none-eabi-size build-arm/*.elf 2>/dev/null || true diff --git a/tests/unit/test_check_production_key.py b/tests/unit/test_check_production_key.py index 6ed3b73..56133aa 100644 --- a/tests/unit/test_check_production_key.py +++ b/tests/unit/test_check_production_key.py @@ -29,6 +29,22 @@ def test_a_real_key_is_accepted(key): check_production_key_hex(key) +def test_mixed_order_is_not_reported_as_low_order(): + """Both classes are outside the prime-order subgroup and both are refused, + but they fail in opposite directions: a genuine low-order point makes every + signature verify, a mixed-order one makes the verifier refuse every image. + The message must say which, because the operator's next action differs.""" + with pytest.raises(ValueError) as low: + check_production_key_hex("ec" + "ff" * 30 + "7f") # order 2 + with pytest.raises(ValueError) as mixed: + check_production_key_hex(DEV[:-1] + "b") # order 8L + assert "low order" in str(low.value) + assert "every signature would verify" in str(low.value) + assert "low order" not in str(mixed.value) + assert "prime-order subgroup" in str(mixed.value) + assert "mistyped" in str(mixed.value) + + @pytest.mark.parametrize("key,reason", [ (DEV, "development key"), (DEV.upper(), "development key"), @@ -38,6 +54,12 @@ def test_a_real_key_is_accepted(key): ("00" * 31 + "80", "low order"), # order 4 ("c7176a703d4dd84fba3c0b760d10670f2a2053fa2c39ccc64ec7fd7792ac037a", "low order"), # order 8 + # On the curve, outside the prime-order subgroup, NOT low order: order 8L. + # This is where a mistyped hex digit lands about half the time, and it is + # the case an operator will actually hit, so it must not be reported as + # "low order" -- that names an attack vector when the cause is a typo. + ("03" + "00" * 31, "prime-order subgroup"), + (DEV[:-1] + "b", "prime-order subgroup"), # dev key, last digit mistyped ("ff" * 31 + "7f", "not below p"), # y = 2^255 - 1 ("ed" + "ff" * 30 + "7f", "not below p"), # y = p exactly (TEST2[:-2], "exactly 64"), diff --git a/tests/unit/test_release_workflow_production_key.py b/tests/unit/test_release_workflow_production_key.py index 13a60d1..4b4068a 100644 --- a/tests/unit/test_release_workflow_production_key.py +++ b/tests/unit/test_release_workflow_production_key.py @@ -212,6 +212,33 @@ def test_the_scanner_decodes_hex_and_uf2_rather_than_grepping_them(tmp_path): assert sorted(Path(h).name for h in hits) == ["fw.hex", "fw.uf2"], hits +def test_a_truncated_hex_record_is_reported_not_raised(tmp_path): + """`:00` is a record with a byte count and nothing else. Indexing it before + checking its length escaped as an IndexError traceback -- still exit 1, so + it failed closed, but by accident: the operator lost the file-annotated + "could not be decoded" line the tool promises. Now a ValueError, caught, + reported against the file.""" + tool = scan_tool() + build = tmp_path / "build"; build.mkdir() + (build / "short.hex").write_text(":00\n") + hits, undecodable = tool.scan([build]) # must not raise + assert hits == [] + assert len(undecodable) == 1 and undecodable[0][0].endswith("short.hex") + assert "truncated" in undecodable[0][1] + + +def test_suffix_match_is_case_insensitive(tmp_path): + """A .BIN is the same artifact as a .bin. Skipping it on case would report + it clean without reading it, which is the one thing this scan must not do.""" + tool = scan_tool() + build = tmp_path / "build"; build.mkdir() + (build / "FW.BIN").write_bytes(bytes(64) + tool.DEV_KEY) + (build / "FW.HEX").write_text(_intel_hex(bytes(64) + tool.DEV_KEY)) + hits, undecodable = tool.scan([build]) + assert undecodable == [] + assert sorted(Path(h).name for h in hits) == ["FW.BIN", "FW.HEX"], hits + + def test_the_scanner_passes_a_clean_tree_and_fails_an_unreadable_file(tmp_path): tool = scan_tool() build = tmp_path / "build"; build.mkdir() diff --git a/tools/check_no_dev_anchor.py b/tools/check_no_dev_anchor.py index 9d82bee..52de66a 100755 --- a/tools/check_no_dev_anchor.py +++ b/tools/check_no_dev_anchor.py @@ -64,7 +64,14 @@ def decode_intel_hex(text): if not line.startswith(":"): raise ValueError(f"not an Intel HEX record: {line[:20]!r}") rec = bytes.fromhex(line[1:]) + # Byte count, two address bytes, type, checksum: five bytes minimum. + # Indexing before checking turned a truncated record into an + # IndexError traceback instead of the ::error line this tool promises. + if len(rec) < 5: + raise ValueError(f"Intel HEX record is truncated: {line[:20]!r}") length, addr, rtype = rec[0], (rec[1] << 8) | rec[2], rec[3] + if len(rec) != 5 + length: + raise ValueError(f"Intel HEX record length field disagrees with the record: {line[:20]!r}") data = rec[4:4 + length] if (sum(rec) & 0xFF) != 0: raise ValueError("Intel HEX checksum mismatch") @@ -115,9 +122,10 @@ def _runs(chunks): def images_of(path): """The byte images to search for a given artifact.""" - if path.suffix == ".hex": + suffix = path.suffix.lower() + if suffix == ".hex": return decode_intel_hex(path.read_text(encoding="ascii", errors="strict")) - if path.suffix == ".uf2": + if suffix == ".uf2": return decode_uf2(path.read_bytes()) return [path.read_bytes()] @@ -126,13 +134,15 @@ def scan(roots): hits, undecodable = [], [] for root in roots: for p in sorted(pathlib.Path(root).rglob("*")): - if not p.is_file() or p.suffix not in SUFFIXES: + # Case-folded: a .BIN or .HEX artifact is the same artifact, and + # skipping it would report it clean without having read it. + if not p.is_file() or p.suffix.lower() not in SUFFIXES: continue try: if any(DEV_KEY in img for img in images_of(p)): hits.append(str(p)) - except (ValueError, UnicodeDecodeError) as e: - undecodable.append((str(p), str(e))) + except (ValueError, UnicodeDecodeError, IndexError, struct.error) as e: + undecodable.append((str(p), f"{type(e).__name__}: {e}")) return hits, undecodable diff --git a/tools/check_production_key.py b/tools/check_production_key.py index 8eb0cef..cc6dba0 100755 --- a/tools/check_production_key.py +++ b/tools/check_production_key.py @@ -10,7 +10,8 @@ * they decode to no point on edwards25519 -- the exact defect the shipped development key had until it was fixed, and one mistyped hex digit in a release secret reproduces it; - * they decode to a point of low order, which the verifier refuses + * they decode to a point of low order (every signature verifies) or one + outside the prime-order subgroup (the verifier refuses every image) (core/ed25519_verify.c, public_key_is_valid_subgroup). A device built with such an anchor refuses every firmware image it is ever @@ -113,8 +114,19 @@ def check_production_key_hex(hex_key: str) -> None: raise ValueError("the key is the identity point; any signature verifies " "against it") if not _is_identity(_mul(point, L)): - raise ValueError("the key is a point of low order; the verifier refuses " - "it and every image would be rejected") + # Outside the prime-order subgroup. Two very different things land + # here and an operator needs to be told which: + # [8]P == identity: P has order 2, 4 or 8. Every signature verifies + # against it -- fails open. Somebody handed over an attack vector. + # otherwise: P has order 2L, 4L or 8L. That is where a mistyped hex + # digit lands about half the time. The verifier refuses it and every + # image is rejected -- fails closed. Somebody typed the secret wrong. + if _is_identity(_mul(point, 8)): + raise ValueError("the key is a point of low order (order 2, 4 or 8); " + "every signature would verify against it") + raise ValueError("the key is on the curve but not in the prime-order " + "subgroup -- a mistyped hex digit usually lands here; " + "the verifier refuses it and every image would be rejected") def main(argv) -> int: From e5dbbb42be3d4aa521159cfef02c3704bb53edfd Mon Sep 17 00:00:00 2001 From: kartikey1306 Date: Tue, 15 Sep 2026 02:52:11 +0530 Subject: [PATCH 19/23] ci(build): the production-key proof step looks for .obj too The step added in 4a98951 to prove the production-key branch was cross-compiled failed on its first run: ::error::generated/production_key.c was not compiled The build log two steps above it says otherwise: [ 70%] Building C object CMakeFiles/eboot_core.dir/generated/production_key.c.obj CMake names objects .obj, not .o, when CMAKE_SYSTEM_NAME=Generic, and the step searched for .o. So the finding-1 concern is in fact answered -- the TU is compiled into eboot_core and linked into eboot_firmware.elf on the cross toolchain -- and the step that was meant to show it had a wrong glob. It failed closed on its own bug rather than passing vacuously, which is the right failure, but a red check that is wrong about why it is red helps nobody. Matches both namings now. The find was run against a fixture tree with each extension in turn and returns the object for both. build.yml parses; the workflow test suite (11) passes at this head. Local reproduction of the .obj naming is NOT possible here (the host generator produces .o); the evidence is the CI log line above. --- .github/workflows/build.yml | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 0eb54e5..c157307 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -67,7 +67,10 @@ jobs: - name: The production-key branch was compiled, not the dev key run: | test -f build-arm/generated/production_key.c - obj=$(find build-arm -name 'production_key.c.o' | head -1) + # CMake names objects .obj, not .o, when CMAKE_SYSTEM_NAME=Generic; + # the first run of this step looked for .o and reported the TU + # uncompiled when the build log showed it compiling. Match both. + obj=$(find build-arm \( -name 'production_key.c.o' -o -name 'production_key.c.obj' \) | head -1) test -n "$obj" || { echo '::error::generated/production_key.c was not compiled'; exit 1; } # the object must carry the fixture key and must not carry the dev key python3 - "$obj" <<'PY' From a50df2809bf0e6156fd3bfa723feba0140d6fad6 Mon Sep 17 00:00:00 2001 From: kartikey1306 Date: Tue, 15 Sep 2026 11:48:48 +0530 Subject: [PATCH 20/23] ci(release): scan .obj as well as .o, and say what the key check cannot catch The one finding from the review of 4d4e702, plus the docs line agreed on the thread. RAW_SUFFIXES in tools/check_no_dev_anchor.py listed .o and not .obj. 4d4e702 had just fixed the proof step in build.yml for exactly this -- CMake names objects .obj under CMAKE_SYSTEM_NAME=Generic, which is every cross target -- and the scanner's own list did not get the same correction. So on the builds whose objects matter most, the entry that exists to scan object files matched nothing. Reproduced: a .obj holding the dev key under build/ was reported clean. Added .obj, with the reason in the comment above the set, and a test that puts the key in both a .o and a .obj and requires both to be found. docs/key_lifecycle.md gains a provisioning row for the thing the checks cannot do. Of all 960 single-hex-digit typos of a key, 495 go off the curve and 413 land outside the prime-order subgroup -- both refused -- but 52 (about one in twenty) land on a different valid key inside the subgroup, indistinguishable from the real one by construction. A device built on it refuses every image, with a green build. The only control is comparing the secret to the .pub the generator wrote before it enters the secret store, and the mixed-order refusal message now ends with that instruction. Negative control: with the scanner at 4d4e702 and the new test kept, 1 of 12 fails (the .obj case). At this head: pytest tests/ 132 passed; ctest 32/32; line endings unchanged in every edited file. --- docs/key_lifecycle.md | 1 + .../unit/test_release_workflow_production_key.py | 16 ++++++++++++++++ tools/check_no_dev_anchor.py | 5 ++++- tools/check_production_key.py | 3 ++- 4 files changed, 23 insertions(+), 2 deletions(-) diff --git a/docs/key_lifecycle.md b/docs/key_lifecycle.md index 42ffa1f..d329f0f 100644 --- a/docs/key_lifecycle.md +++ b/docs/key_lifecycle.md @@ -342,6 +342,7 @@ configured with a staging public key. | Development private key is **committed** to the repository | Enables any developer to build and test signed images locally | | Development key does not reach a production artifact **built through the gate** | Any build of a real board that is not `CMAKE_BUILD_TYPE=Debug` -- `Release`, `MinSizeRel`, `RelWithDebInfo`, any spelling, or no build type at all -- refuses to configure without `EBLDR_PRODUCTION_KEY` unless `EBLDR_ALLOW_DEV_KEY=ON` is passed explicitly; the key it accepts is checked to be a point in the prime-order subgroup (`tools/check_production_key.py`); and the release workflow validates the secret before any board is configured and scans every artifact for the development key's bytes (§3.1, §7.2). What the gate cannot see: a Debug build flashed to a device, a build that passes `EBLDR_ALLOW_DEV_KEY=ON`, or a fork that removes the gate. Those are policy, not mechanism. | | Production key **never** appears in source control | Only the public key is embedded; private key stays in HSM | +| The stored `EBLDR_PRODUCTION_KEY_HEX` is checked against the generated `.pub` **before** it is stored | The configure-time and release-time checks refuse a key that is off the curve, of low order, or outside the prime-order subgroup -- but of all single-hex-digit typos of a key, about one in twenty (52 of 960, measured) lands on a *different valid key* inside the subgroup, which no check on the key alone can distinguish from the real one. A device built on it refuses every image, with a green build. The only control for that case is comparing the secret to the `.pub` the key generator wrote, by eye or by `cmp`, before it enters the secret store. | | CI pipeline uses **staging** key for integration tests | Tests signature verification without exposing production key | ### 7.4 Well-Known Development Key diff --git a/tests/unit/test_release_workflow_production_key.py b/tests/unit/test_release_workflow_production_key.py index 4b4068a..a7f9f90 100644 --- a/tests/unit/test_release_workflow_production_key.py +++ b/tests/unit/test_release_workflow_production_key.py @@ -227,6 +227,22 @@ def test_a_truncated_hex_record_is_reported_not_raised(tmp_path): assert "truncated" in undecodable[0][1] +def test_cross_target_object_files_are_scanned(tmp_path): + """CMake names objects .obj under CMAKE_SYSTEM_NAME=Generic -- every cross + target. The .o entry existed to scan object files, which is where the key + actually lives after keystore.c compiles, and on those builds it matched + nothing. The proof step in build.yml learned this first; the scanner + had not.""" + tool = scan_tool() + assert ".obj" in tool.SUFFIXES and ".o" in tool.SUFFIXES + build = tmp_path / "build"; build.mkdir() + (build / "keystore.c.obj").write_bytes(bytes(32) + tool.DEV_KEY) + (build / "keystore.c.o").write_bytes(bytes(32) + tool.DEV_KEY) + hits, undecodable = tool.scan([build]) + assert undecodable == [] + assert sorted(Path(h).name for h in hits) == ["keystore.c.o", "keystore.c.obj"], hits + + def test_suffix_match_is_case_insensitive(tmp_path): """A .BIN is the same artifact as a .bin. Skipping it on case would report it clean without reading it, which is the one thing this scan must not do.""" diff --git a/tools/check_no_dev_anchor.py b/tools/check_no_dev_anchor.py index 52de66a..6befe3f 100755 --- a/tools/check_no_dev_anchor.py +++ b/tools/check_no_dev_anchor.py @@ -38,7 +38,10 @@ # Everything the release workflow collects, plus the intermediates the key # actually lives in. Keep RAW and DECODED disjoint and complete: the test # checks their union against release.yml's `find ... -name` globs. -RAW_SUFFIXES = {".elf", ".bin", ".a", ".o", ".efi"} +# Both .o and .obj: CMake names objects .obj under CMAKE_SYSTEM_NAME=Generic, +# which is every cross target, so an .o-only entry scanned nothing on exactly +# the builds whose objects matter most. +RAW_SUFFIXES = {".elf", ".bin", ".a", ".o", ".obj", ".efi"} DECODED_SUFFIXES = {".hex", ".uf2"} SUFFIXES = RAW_SUFFIXES | DECODED_SUFFIXES diff --git a/tools/check_production_key.py b/tools/check_production_key.py index cc6dba0..5f4ff57 100755 --- a/tools/check_production_key.py +++ b/tools/check_production_key.py @@ -126,7 +126,8 @@ def check_production_key_hex(hex_key: str) -> None: "every signature would verify against it") raise ValueError("the key is on the curve but not in the prime-order " "subgroup -- a mistyped hex digit usually lands here; " - "the verifier refuses it and every image would be rejected") + "the verifier refuses it and every image would be rejected. " + "Check the secret against the key that was generated") def main(argv) -> int: From 18fab203e658a65401c8da743788702fd6b6ca58 Mon Sep 17 00:00:00 2001 From: kartikey1306 Date: Tue, 15 Sep 2026 12:21:55 +0530 Subject: [PATCH 21/23] docs(key_lifecycle): state the typo-collision rate as the derivable one, not one key's count The provisioning row added in 1cd1265 said "about one in twenty (52 of 960, measured)" for the share of single-hex-digit typos that land on a different valid key. The review of a50df28 is right that this is one key's count stated as the general rate, and that the general rate is worse: a mutated encoding is a curve point with probability about 1/2 and in the prime-order subgroup with probability 1/8, so about 1/16 for any key, by construction. Re-measured at this head with the branch's own checker: 52/960 for the development key, 64/960 for RFC 8032 TEST 2, 75/960 for TEST 3 -- the spread around 60 that 1/16 predicts. The row now gives the derivable rate and cites all three as measurements of specific keys, which is what a number in a security document needs to be: reproducible without the key it was measured on. One sentence in one Markdown file; nothing else changes. --- docs/key_lifecycle.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/key_lifecycle.md b/docs/key_lifecycle.md index d329f0f..fa56231 100644 --- a/docs/key_lifecycle.md +++ b/docs/key_lifecycle.md @@ -342,7 +342,7 @@ configured with a staging public key. | Development private key is **committed** to the repository | Enables any developer to build and test signed images locally | | Development key does not reach a production artifact **built through the gate** | Any build of a real board that is not `CMAKE_BUILD_TYPE=Debug` -- `Release`, `MinSizeRel`, `RelWithDebInfo`, any spelling, or no build type at all -- refuses to configure without `EBLDR_PRODUCTION_KEY` unless `EBLDR_ALLOW_DEV_KEY=ON` is passed explicitly; the key it accepts is checked to be a point in the prime-order subgroup (`tools/check_production_key.py`); and the release workflow validates the secret before any board is configured and scans every artifact for the development key's bytes (§3.1, §7.2). What the gate cannot see: a Debug build flashed to a device, a build that passes `EBLDR_ALLOW_DEV_KEY=ON`, or a fork that removes the gate. Those are policy, not mechanism. | | Production key **never** appears in source control | Only the public key is embedded; private key stays in HSM | -| The stored `EBLDR_PRODUCTION_KEY_HEX` is checked against the generated `.pub` **before** it is stored | The configure-time and release-time checks refuse a key that is off the curve, of low order, or outside the prime-order subgroup -- but of all single-hex-digit typos of a key, about one in twenty (52 of 960, measured) lands on a *different valid key* inside the subgroup, which no check on the key alone can distinguish from the real one. A device built on it refuses every image, with a green build. The only control for that case is comparing the secret to the `.pub` the key generator wrote, by eye or by `cmp`, before it enters the secret store. | +| The stored `EBLDR_PRODUCTION_KEY_HEX` is checked against the generated `.pub` **before** it is stored | The configure-time and release-time checks refuse a key that is off the curve, of low order, or outside the prime-order subgroup -- but about **one in sixteen** single-hex-digit typos of a key lands on a *different valid key* inside the subgroup, which no check on the key alone can distinguish from the real one. That rate is derivable, not measured: a mutated encoding decodes to a curve point with probability about 1/2 and lands in the prime-order subgroup with probability 1/8. Measured counts for specific keys vary around it -- 52/960 for the development key, 64/960 for the RFC 8032 TEST 2 key, 75/960 for TEST 3. A device built on it refuses every image, with a green build. The only control for that case is comparing the secret to the `.pub` the key generator wrote, by eye or by `cmp`, before it enters the secret store. | | CI pipeline uses **staging** key for integration tests | Tests signature verification without exposing production key | ### 7.4 Well-Known Development Key From 0c7c8f59914d40bd345f924c0aae89bc5b62b9ce Mon Sep 17 00:00:00 2001 From: kartikey1306 Date: Tue, 15 Sep 2026 12:25:39 +0530 Subject: [PATCH 22/23] fix(sign_image): --genkey writes the key the build reads, not a header nothing includes Finding 2 (High, P1) from the review of #116 at ef5b70d. Closes #141. docs/quickstart.md told a developer that `sign_image.py --genkey` writes keys/public_key.h. It did: a header defining ebldr_default_pubkey[32]. That symbol has zero consumers -- core/image_verify.c's own comment records that it "was never defined anywhere, so this did not link", and the verifier reads the anchor from the keystore instead, whose production slot is ebldr_production_key[], generated at configure time from -DEBLDR_PRODUCTION_KEY=<64 hex>. So the documented procedure generated a key, wrote a header, built without error, and shipped firmware whose trust anchor was still the RFC 8032 test key. The review checked that #122 does not close this, and it does not: after the stack landed the repo would have documented two key paths, one of which did nothing. Both emitters -- --genkey and --extract-pubkey -- now write the one artefact the build consumes: public_key.hex, 64 lowercase hex characters, the exact value EBLDR_PRODUCTION_KEY takes, and print the cmake invocation that uses it with a pointer to docs/key_lifecycle.md and the instruction to compare the value against the .pub before storing it. The header is not written under any name: a second mechanism for the same thing is how this drifted in the first place. docs/quickstart.md's signing section now shows the configure step with -DEBLDR_PRODUCTION_KEY=$(cat keys/public_key.hex), says what happens without it, and names the lifecycle document. Tests, tests/unit/test_sign_image.py: - --genkey writes no public_key.h; writes public_key.hex; 64 hex chars; equal to the raw key in public.pem; stdout names the flag - the written value passes tools/check_production_key.py, the same check the configure-time gate runs, so the documented path cannot produce a key the build then refuses - --extract-pubkey writes the same hex, and no dead symbol - quickstart names public_key.hex and the flag, and not the header Negative controls, one file reverted at a time, tests kept: tools/sign_image.py at 18fab20 -> 3 of 20 fail docs/quickstart.md at 18fab20 -> 1 of 1 fails End to end, the documented procedure at this head: --genkey, then cmake -DEBLDR_BOARD=stm32f4 -DCMAKE_BUILD_TYPE=Release -DEBLDR_PRODUCTION_KEY=$(cat keys/public_key.hex): configure rc 0, and build/generated/production_key.c carries exactly the generated key. At this head, on #122 at 18fab20: pytest tests/ 138 passed. --- docs/quickstart.md | 10 +++++- tests/unit/test_sign_image.py | 61 +++++++++++++++++++++++++++++++ tools/sign_image.py | 68 +++++++++++++---------------------- 3 files changed, 95 insertions(+), 44 deletions(-) diff --git a/docs/quickstart.md b/docs/quickstart.md index 528d90d..638b448 100644 --- a/docs/quickstart.md +++ b/docs/quickstart.md @@ -83,9 +83,17 @@ This produces: cd EoS/eboot/tools # Generate an Ed25519 keypair (first time only). -# Writes keys/private.pem, keys/public.pem and keys/public_key.h. +# Writes keys/private.pem, keys/public.pem and keys/public_key.hex -- the +# 64-hex-character value the bootloader build takes as its trust anchor. python3 sign_image.py --genkey --output keys/ +# Build the bootloader with THAT key as its trust anchor. Without +# EBLDR_PRODUCTION_KEY a board build keeps the RFC 8032 test key, whose +# private half is public -- and a release-shaped build refuses to configure +# rather than let that happen silently. Full lifecycle: docs/key_lifecycle.md. +cmake -B build -DEBLDR_BOARD= -DCMAKE_BUILD_TYPE=Release \ + -DEBLDR_PRODUCTION_KEY=$(cat keys/public_key.hex) + # Pack a raw firmware binary into an .eimg container. python3 imgpack.py --input firmware.bin --output firmware.eimg \ --load-addr 0x08010000 --entry-addr 0x08010100 --version 1.0.0 diff --git a/tests/unit/test_sign_image.py b/tests/unit/test_sign_image.py index d00b287..bb21396 100644 --- a/tests/unit/test_sign_image.py +++ b/tests/unit/test_sign_image.py @@ -235,3 +235,64 @@ def test_verify_requires_a_signature_when_a_key_is_given(tmp_path): r = _verify(tmp_path / "u.eimg", keys / "public.pem") assert r.returncode != 0 assert "expected an Ed25519 signature" in r.stderr + + +# --- --genkey emits the value the build reads, not a header nothing includes --- + +def _raw_pubkey_from_pem(pem_path): + from cryptography.hazmat.primitives import serialization + pub = serialization.load_pem_public_key(pem_path.read_bytes()) + return pub.public_bytes(serialization.Encoding.Raw, serialization.PublicFormat.Raw) + + +def test_genkey_writes_the_hex_the_build_consumes_and_no_dead_header(tmp_path): + """--genkey used to write keys/public_key.h defining ebldr_default_pubkey[], + a symbol nothing in the tree includes or links, so the documented + procedure left the RFC test key as the trust anchor with no error. The + build's actual input is -DEBLDR_PRODUCTION_KEY=<64 hex>; that is what + must be written, and it must be the key in public.pem.""" + keys = tmp_path / "keys" + r = _run(TOOLS / "sign_image.py", "--genkey", "--output", keys) + assert r.returncode == 0, r.stderr + + assert not (keys / "public_key.h").exists(), "the dead header is back" + hex_path = keys / "public_key.hex" + assert hex_path.exists(), "public_key.hex was not written" + hex_key = hex_path.read_text().strip() + assert len(hex_key) == 64 and all(c in "0123456789abcdef" for c in hex_key) + assert bytes.fromhex(hex_key) == _raw_pubkey_from_pem(keys / "public.pem") + # the tool tells the developer exactly how to use it + assert "-DEBLDR_PRODUCTION_KEY=" in r.stdout + assert "key_lifecycle.md" in r.stdout + + +def test_genkey_output_passes_the_configure_time_key_check(tmp_path): + """The gate runs tools/check_production_key.py on EBLDR_PRODUCTION_KEY. A + freshly generated key is a real curve point in the prime-order subgroup, + so the value --genkey writes must pass that check as-is -- otherwise the + documented path produces a key the build refuses.""" + keys = tmp_path / "keys" + assert _run(TOOLS / "sign_image.py", "--genkey", "--output", keys).returncode == 0 + hex_key = (keys / "public_key.hex").read_text().strip() + r = _run(TOOLS / "check_production_key.py", hex_key) + assert r.returncode == 0, r.stderr + + +def test_extract_pubkey_writes_hex_too(tmp_path): + keys = tmp_path / "keys" + assert _run(TOOLS / "sign_image.py", "--genkey", "--output", keys).returncode == 0 + out = tmp_path / "anchor.hex" + r = _run(TOOLS / "sign_image.py", "--extract-pubkey", keys / "private.pem", "--output", out) + assert r.returncode == 0, r.stderr + assert out.read_text().strip() == (keys / "public_key.hex").read_text().strip() + assert "ebldr_default_pubkey" not in out.read_text() + + +def test_quickstart_documents_the_key_path_the_build_reads(): + """docs/quickstart.md told developers about public_key.h. It must now name + the file --genkey writes and the flag that consumes it, and not the header.""" + doc = (REPO_ROOT / "docs" / "quickstart.md").read_text(encoding="utf-8") + assert "public_key.hex" in doc + assert "-DEBLDR_PRODUCTION_KEY=" in doc + assert "key_lifecycle.md" in doc + assert "public_key.h\n" not in doc and "public_key.h." not in doc and "public_key.h " not in doc diff --git a/tools/sign_image.py b/tools/sign_image.py index 84e8e52..4beca25 100644 --- a/tools/sign_image.py +++ b/tools/sign_image.py @@ -18,7 +18,7 @@ Key Generation: python sign_image.py --genkey --output keys/ Creates keys/private.pem and keys/public.pem (Ed25519 keypair) - Also generates keys/public_key.h for embedding in bootloader. + Also writes keys/public_key.hex, the value for -DEBLDR_PRODUCTION_KEY. """ import argparse @@ -327,28 +327,28 @@ def generate_keypair(output_dir: Path): format=serialization.PublicFormat.Raw ) - # Generate C header for embedding - hdr_path = output_dir / "public_key.h" - hex_bytes = ', '.join(f'0x{b:02x}' for b in pub_raw) - c_header = f"""\ -// SPDX-License-Identifier: MIT -// Auto-generated by sign_image.py --genkey -// DO NOT EDIT — regenerate with: python sign_image.py --genkey - -#ifndef EBLDR_PUBLIC_KEY_H -#define EBLDR_PUBLIC_KEY_H - -#include - -static const uint8_t ebldr_default_pubkey[32] = {{ - {hex_bytes} -}}; - -#endif /* EBLDR_PUBLIC_KEY_H */ -""" - hdr_path.write_text(c_header) - print(f"C header saved: {hdr_path}") - print(f"Public key (hex): {pub_raw.hex()}") + # The one artefact the build consumes. This used to write public_key.h + # defining ebldr_default_pubkey[] -- a symbol nothing includes or links, + # so a developer who followed the quickstart built firmware whose trust + # anchor was still the RFC 8032 test key, with no error to say so. The + # bootloader takes its anchor from -DEBLDR_PRODUCTION_KEY=<64 hex> at + # configure time (cmake/ProductionKey.cmake, docs/key_lifecycle.md), and + # that is what is written here. + write_public_key_hex(pub_raw, output_dir / "public_key.hex") + + +def write_public_key_hex(pub_raw: bytes, path: Path): + """Write the raw Ed25519 public key as 64 lowercase hex characters -- the + exact value EBLDR_PRODUCTION_KEY takes -- and print the configure flag.""" + hex_key = pub_raw.hex() + assert len(hex_key) == 64, len(hex_key) + path.write_text(hex_key + "\n") + print(f"Public key (hex) saved: {path}") + print(f"Public key (hex): {hex_key}") + print("Build the bootloader with this key as its trust anchor:") + print(" cmake -B build -DEBLDR_BOARD= -DCMAKE_BUILD_TYPE=Release \\") + print(f" -DEBLDR_PRODUCTION_KEY=$(cat {path})") + print("See docs/key_lifecycle.md; compare this value against the .pub before storing it.") def extract_pubkey(key_path: Path, output_path: Path): @@ -368,25 +368,7 @@ def extract_pubkey(key_path: Path, output_path: Path): format=serialization.PublicFormat.Raw ) - hex_bytes = ', '.join(f'0x{b:02x}' for b in pub_raw) - c_header = f"""\ -// SPDX-License-Identifier: MIT -// Auto-generated by sign_image.py --extract-pubkey - -#ifndef EBLDR_PUBLIC_KEY_H -#define EBLDR_PUBLIC_KEY_H - -#include - -static const uint8_t ebldr_default_pubkey[32] = {{ - {hex_bytes} -}}; - -#endif /* EBLDR_PUBLIC_KEY_H */ -""" - output_path.write_text(c_header) - print(f"Public key header saved: {output_path}") - print(f"Public key (hex): {pub_raw.hex()}") + write_public_key_hex(pub_raw, output_path) def main(): @@ -410,7 +392,7 @@ def main(): if args.extract_pubkey: key_path = Path(args.extract_pubkey) - output_path = Path(args.output) if args.output else Path('public_key.h') + output_path = Path(args.output) if args.output else Path('public_key.hex') extract_pubkey(key_path, output_path) return From 180a74f7e1d6e86c45e5df46f3d29687ea2ccc1d Mon Sep 17 00:00:00 2001 From: kartikey1306 Date: Tue, 15 Sep 2026 12:30:53 +0530 Subject: [PATCH 23/23] docs(key_lifecycle): show the flags sign_image.py has, not two it never had Same class as the quickstart defect this branch fixes, in the second document. docs/key_lifecycle.md section 2.2 "Alternative -- using eBootloader tooling" showed: python3 tools/sign_image.py --genkey \ --key-out keys/production_key.pem \ --pub-header include/eos_signing_key.h sign_image.py has never defined --key-out or --pub-header; its options are --genkey, --extract-pubkey and --output. A reader following the lifecycle document -- the one written for production provisioning -- got an argparse error, or, reading the intent, went looking for a header to embed, which is the dead path 0c7c8f5 removes. Replaced with the real invocation and the configure flag that consumes its output. The doc test now covers both documents, and a second test reads every sign_image.py invocation shown in quickstart.md, key_lifecycle.md and README.md -- continuation lines included -- and requires each --flag to be one the tool defines, so a documented flag that does not exist fails the suite the same way a documented header nothing reads does. The first version of that guard could not fail: its pattern's [^\n]* swallowed the line-ending backslash, so it never read a continuation line and saw only --genkey. Caught because the negative control reported 1 failure where 2 were claimed. Fixed to consume backslash-newline pairs explicitly; the same control now reports 2. Negative control: key_lifecycle.md at 0c7c8f5 with the tests kept -> 2 of 24 fail (the phantom flags, and the header-free assertion). pytest tests/ 142 passed at this head. Found by a sibling session working the same review finding in parallel; its patch was not applied, this is the one piece of it #142 lacked. --- docs/key_lifecycle.md | 11 +++++---- tests/unit/test_sign_image.py | 45 ++++++++++++++++++++++++++++------- 2 files changed, 44 insertions(+), 12 deletions(-) diff --git a/docs/key_lifecycle.md b/docs/key_lifecycle.md index fa56231..98dbfd2 100644 --- a/docs/key_lifecycle.md +++ b/docs/key_lifecycle.md @@ -74,10 +74,13 @@ echo "test" | openssl pkeyutl -sign -inkey eboot_signing_key.pem | \ **Alternative — using eBootloader tooling:** ```bash -# Generate keypair and C header in one step -python3 tools/sign_image.py --genkey \ - --key-out keys/production_key.pem \ - --pub-header include/eos_signing_key.h +# Generate the keypair and the value the bootloader build takes as its +# trust anchor. Writes keys/private.pem, keys/public.pem and +# keys/public_key.hex (64 hex characters). There is no header to embed: +# the anchor is compiled in at configure time from the flag below. +python3 tools/sign_image.py --genkey --output keys/ +cmake -B build -DEBLDR_BOARD= -DCMAKE_BUILD_TYPE=Release \ + -DEBLDR_PRODUCTION_KEY=$(cat keys/public_key.hex) ``` ### 2.3 Key Storage After Generation diff --git a/tests/unit/test_sign_image.py b/tests/unit/test_sign_image.py index bb21396..05010d8 100644 --- a/tests/unit/test_sign_image.py +++ b/tests/unit/test_sign_image.py @@ -288,11 +288,40 @@ def test_extract_pubkey_writes_hex_too(tmp_path): assert "ebldr_default_pubkey" not in out.read_text() -def test_quickstart_documents_the_key_path_the_build_reads(): - """docs/quickstart.md told developers about public_key.h. It must now name - the file --genkey writes and the flag that consumes it, and not the header.""" - doc = (REPO_ROOT / "docs" / "quickstart.md").read_text(encoding="utf-8") - assert "public_key.hex" in doc - assert "-DEBLDR_PRODUCTION_KEY=" in doc - assert "key_lifecycle.md" in doc - assert "public_key.h\n" not in doc and "public_key.h." not in doc and "public_key.h " not in doc +def _sign_image_flags(): + """Every long option sign_image.py actually defines.""" + import re + src = (TOOLS / "sign_image.py").read_text(encoding="utf-8", errors="replace") + return set(re.findall(r"add_argument\(\s*'(--[a-z-]+)'", src)) + + +@pytest.mark.parametrize("rel", ["docs/quickstart.md", "docs/key_lifecycle.md"]) +def test_docs_name_the_key_path_the_build_reads(rel): + """Both documents told developers about a header. quickstart named + public_key.h; key_lifecycle showed --key-out and --pub-header, two flags + sign_image.py has never defined. Each must now name the file --genkey + writes and the flag that consumes it, and no header.""" + doc = (REPO_ROOT / rel).read_text(encoding="utf-8") + assert "public_key.hex" in doc, rel + assert "-DEBLDR_PRODUCTION_KEY=" in doc, rel + assert "public_key.h\n" not in doc and "public_key.h." not in doc and "public_key.h " not in doc, rel + + +@pytest.mark.parametrize("rel", ["docs/quickstart.md", "docs/key_lifecycle.md", "README.md"]) +def test_docs_only_show_sign_image_flags_that_exist(rel): + """A documented flag the tool does not have is the same defect as a + documented header nothing reads: the reader follows it and gets an + argparse error at best, or a silently-wrong build at worst.""" + import re + path = REPO_ROOT / rel + if not path.exists(): + pytest.skip(f"{rel} absent") + doc = path.read_text(encoding="utf-8") + shown = set() + # Lines ending in a backslash continue the invocation. [^\n]* alone + # swallows that backslash, so the first version of this never read a + # continuation line and could not fail on the flags it was written for. + for block in re.findall(r"sign_image\.py(?:[^\n]*\\\n)*[^\n]*", doc): + shown |= set(re.findall(r"(--[a-z-]+)", block)) + phantom = sorted(shown - _sign_image_flags()) + assert not phantom, f"{rel} documents sign_image.py flags that do not exist: {phantom}"