From 71cd49b5809cecc5cf94d08820905ef90cb868f2 Mon Sep 17 00:00:00 2001 From: Greeva Patel Date: Sat, 31 Oct 2026 16:11:24 +0530 Subject: [PATCH] fix(secure_boot): write the rollback counter to its own OTP field eos_secure_boot_update_rollback() wrote the 4-byte anti-rollback counter at OTP_ROLLBACK_OFFSET = 0x180. That is where core/recovery.c reads the 32-byte recovery shared secret, so every counter update overwrote the first four bytes of the secret. OTP bits only move one way: the secret was permanently corrupted and authenticated recovery locked out. The OTP map in docs/key_lifecycle.md 3.2 (base 0x100, which matches the keystore's 0x100/0x120 key slots and 0x140 revoke mask) puts the 4-byte security_version counter at base + 0x44. OTP_ROLLBACK_OFFSET is now 0x144, which overlaps neither the revoke mask, the recovery secret (0x180-0x19F), the debug lock (0x1F0) nor the firmware key (0x200). test_secure_boot_policy: update_rollback() writes the counter bytes at 0x144 and leaves every other OTP byte, the recovery secret included, untouched. Co-Authored-By: Claude Opus 5.5 --- core/secure_boot.c | 8 ++++-- tests/unit/test_secure_boot_policy.c | 37 ++++++++++++++++++++++++++++ 2 files changed, 43 insertions(+), 2 deletions(-) diff --git a/core/secure_boot.c b/core/secure_boot.c index 4d80cd3..4207b97 100644 --- a/core/secure_boot.c +++ b/core/secure_boot.c @@ -16,10 +16,14 @@ #include "eos_hal.h" #include -/* OTP offsets for root-of-trust */ +/* OTP offsets for root-of-trust. The map is docs/key_lifecycle.md 3.2 + * with OTP base 0x100: key slots at 0x100/0x120 and the revoke mask at + * 0x140 (core/keystore.c), the security_version counter at 0x144. 0x180 + * holds the recovery shared secret (core/recovery.c), so the counter must + * never be written there. */ #define OTP_KEY_HASH_OFFSET 0x100 #define OTP_KEY_HASH_SIZE 32 -#define OTP_ROLLBACK_OFFSET 0x180 +#define OTP_ROLLBACK_OFFSET 0x144 #define OTP_DEBUG_LOCK_OFFSET 0x1F0 /* Global attestation log — persisted in RAM for kernel to read */ diff --git a/tests/unit/test_secure_boot_policy.c b/tests/unit/test_secure_boot_policy.c index 877a603..c0eaca8 100644 --- a/tests/unit/test_secure_boot_policy.c +++ b/tests/unit/test_secure_boot_policy.c @@ -274,6 +274,42 @@ TEST(test_the_ordinary_boot_path_is_unaffected_by_the_step_4_change) ASSERT(entry == 0x20000001U); } +/* ---- Anti-rollback counter placement ---- + * + * eos_secure_boot_update_rollback() wrote its 4 bytes at OTP 0x180, which is + * where core/recovery.c reads the recovery shared secret. OTP bits only move + * one way, so every counter update permanently corrupted the secret and + * locked authenticated recovery out. It must write only the security_version + * field: OTP base 0x100 + 0x44 per docs/key_lifecycle.md 3.2. */ +#define OTP_ROLLBACK_OFFSET 0x144 +#define OTP_RECOVERY_SECRET_OFFSET 0x180 +#define OTP_RECOVERY_SECRET_SIZE 32 + +TEST(test_update_rollback_writes_only_the_counter_field) +{ + provide_otp_write = 1; + reset_fixture(); + memset(sim_otp, 0xA5, sizeof(sim_otp)); + + ASSERT(eos_secure_boot_update_rollback(0x01020304U) == EOS_OK); + ASSERT(otp_write_calls == 1); + + /* The counter, big-endian, in its own field. */ + ASSERT(sim_otp[OTP_ROLLBACK_OFFSET + 0] == 0x01); + ASSERT(sim_otp[OTP_ROLLBACK_OFFSET + 1] == 0x02); + ASSERT(sim_otp[OTP_ROLLBACK_OFFSET + 2] == 0x03); + ASSERT(sim_otp[OTP_ROLLBACK_OFFSET + 3] == 0x04); + + /* Nothing else -- the recovery secret above all -- is touched. */ + for (uint32_t i = 0; i < OTP_SIZE; i++) { + if (i >= OTP_ROLLBACK_OFFSET && i < OTP_ROLLBACK_OFFSET + 4) + continue; + ASSERT(sim_otp[i] == 0xA5); + } + for (uint32_t i = 0; i < OTP_RECOVERY_SECRET_SIZE; i++) + ASSERT(sim_otp[OTP_RECOVERY_SECRET_OFFSET + i] == 0xA5); +} + int main(void) { printf("=== eBootloader: Secure Boot Debug-Lock Policy Tests ===\n\n"); @@ -285,6 +321,7 @@ int main(void) run_test_the_same_image_boots_when_no_debug_lock_is_asked_for(); run_test_secure_boot_proceeds_when_the_debug_lock_succeeds(); run_test_the_ordinary_boot_path_is_unaffected_by_the_step_4_change(); + run_test_update_rollback_writes_only_the_counter_field(); printf("\n%d/%d tests passed\n", tests_passed, tests_run); return (tests_passed == tests_run) ? 0 : 1;