diff --git a/core/keystore.c b/core/keystore.c index bd74104..a86654d 100644 --- a/core/keystore.c +++ b/core/keystore.c @@ -16,9 +16,17 @@ #include /* Default development key — REPLACE with production key before deployment. - * This is the public half of a well-known test keypair. - * Production builds MUST set EBLDR_PRODUCTION_KEY at compile 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. + * + * 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. */ #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" 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, @@ -50,10 +58,22 @@ int eos_keystore_init(eos_keystore_t *ks) memset(ks, 0, sizeof(*ks)); - /* Try OTP first */ + /* Try OTP first. + * + * EOS_ERR_NOT_SUPPORTED means this board has no OTP at all, which is a + * legitimate configuration: the trust anchor is then the compiled-in key. + * Any other error means the board HAS an OTP and reading it failed, so the + * key material this device is provisioned with is unknown. Those two are + * not interchangeable, and treating them alike let a fault on the OTP bus + * silently swap the trust anchor for whatever was compiled in. */ int rc = eos_hal_otp_read(OTP_KEY_OFFSET_SLOT0, ks->slots[0].key, EOS_ED25519_PUB_KEY_SIZE); + bool otp_present = (rc != EOS_ERR_NOT_SUPPORTED); + + if (otp_present && rc != EOS_OK) + return EOS_ERR_KEY; + if (rc == EOS_OK) { /* Check if key is non-zero (provisioned) */ uint8_t zero[EOS_ED25519_PUB_KEY_SIZE] = {0}; @@ -66,21 +86,34 @@ int eos_keystore_init(eos_keystore_t *ks) rc = eos_hal_otp_read(OTP_KEY_OFFSET_SLOT1, ks->slots[1].key, EOS_ED25519_PUB_KEY_SIZE); + if (rc != EOS_OK && rc != EOS_ERR_NOT_SUPPORTED) + return EOS_ERR_KEY; if (rc == EOS_OK && safe_compare(ks->slots[1].key, zero, EOS_ED25519_PUB_KEY_SIZE) != 0) { ks->slots[1].valid = true; } - /* Check revocation status */ + /* Revocation status. + * + * If this read fails we cannot show that a key has NOT been revoked. + * Leaving the flags clear is the wrong default: revocation exists to + * retire a key whose private half is believed compromised, so a device + * that cannot read the revocation store must not keep using the keys it + * covers. Treat every OTP slot as revoked; eos_keystore_get_active_key() + * then reports EOS_ERR_KEY rather than handing back a key that may have + * been retired. */ uint8_t revoke_flags = 0; if (eos_hal_otp_read(OTP_REVOKE_OFFSET, &revoke_flags, 1) == EOS_OK) { if (revoke_flags & 0x01) ks->slots[0].revoked = true; if (revoke_flags & 0x02) ks->slots[1].revoked = true; + } else { + for (uint32_t i = 0; i < EOS_KEY_SLOTS; i++) + ks->slots[i].revoked = true; } } - /* Fall back to compiled-in key if OTP not available */ - if (!ks->slots[0].valid && !ks->slots[1].valid) { + /* Fall back to compiled-in key only when the board has no OTP to consult. */ + if (!ks->slots[0].valid && !ks->slots[1].valid && !otp_present) { #ifndef EBLDR_PRODUCTION_KEY memcpy(ks->slots[0].key, default_dev_key, EOS_ED25519_PUB_KEY_SIZE); #else @@ -186,11 +219,27 @@ int eos_keystore_revoke_slot(eos_keystore_t *ks, uint32_t slot) ks->slots[slot].revoked = true; - /* Try to persist revocation to OTP */ + /* Persist the revocation to OTP. + * + * The read must be checked before the OR: on failure revoke_flags stayed 0, + * so writing it back cleared every other slot's revocation bit -- revoking + * slot 1 un-revoked slot 0. And a write that fails leaves the revocation in + * RAM only, so it is gone at the next reset while this function reported + * EOS_OK and the caller believed the key was permanently retired. Both are + * reported now; the in-RAM revocation stands either way, so this boot + * still refuses the key. */ + int persist_rc = EOS_OK; uint8_t revoke_flags = 0; - eos_hal_otp_read(OTP_REVOKE_OFFSET, &revoke_flags, 1); - revoke_flags |= (1U << slot); - eos_hal_otp_write(OTP_REVOKE_OFFSET, &revoke_flags, 1); + int read_rc = eos_hal_otp_read(OTP_REVOKE_OFFSET, &revoke_flags, 1); + if (read_rc == EOS_OK) { + revoke_flags |= (uint8_t)(1U << slot); + persist_rc = eos_hal_otp_write(OTP_REVOKE_OFFSET, &revoke_flags, 1); + } else if (read_rc == EOS_ERR_NOT_SUPPORTED) { + /* No OTP on this board: nothing to persist to, and nothing to clobber. */ + persist_rc = EOS_OK; + } else { + persist_rc = read_rc; + } /* Update active slot */ ks->active_slot = EOS_KEY_SLOTS; @@ -201,7 +250,7 @@ int eos_keystore_revoke_slot(eos_keystore_t *ks, uint32_t slot) } } - return EOS_OK; + return persist_rc; } diff --git a/tests/unit/test_keystore.c b/tests/unit/test_keystore.c index ea9c384..7236bef 100644 --- a/tests/unit/test_keystore.c +++ b/tests/unit/test_keystore.c @@ -8,10 +8,73 @@ */ #include "eos_keystore.h" +#include "eos_hal.h" #include #include #include +/* ---- Simulated OTP ---- + * The tests above run with no board registered, so eos_hal_otp_read() reports + * EOS_ERR_NOT_SUPPORTED and the keystore uses the compiled-in key. The tests + * below register a board so the OTP paths are exercised. */ + +#define OTP_SIZE 0x200 +#define OTP_KEY0 0x100 +#define OTP_KEY1 0x120 +#define OTP_REVOKE 0x140 + +static uint8_t sim_otp[OTP_SIZE]; +static int otp_read_rc; /* forced result for reads of any offset */ +static int otp_revoke_read_rc; /* forced result for the revocation offset */ +static int otp_write_rc; +static int otp_write_calls; + +static int sim_otp_read(uint32_t offset, void *buf, size_t len) +{ + if (offset == OTP_REVOKE && otp_revoke_read_rc != EOS_OK) + return otp_revoke_read_rc; + if (otp_read_rc != EOS_OK) + return otp_read_rc; + if ((uint64_t)offset + len > OTP_SIZE) + return EOS_ERR_INVALID; + memcpy(buf, sim_otp + offset, len); + return EOS_OK; +} + +static int sim_otp_write(uint32_t offset, const void *buf, size_t len) +{ + otp_write_calls++; + if (otp_write_rc != EOS_OK) + return otp_write_rc; + if ((uint64_t)offset + len > OTP_SIZE) + return EOS_ERR_INVALID; + memcpy(sim_otp + offset, buf, len); + return EOS_OK; +} + +static const eos_board_ops_t sim_board = { + .otp_read = sim_otp_read, + .otp_write = sim_otp_write, +}; + +/* Provision both OTP key slots with distinguishable non-zero keys. */ +static void otp_reset(void) +{ + memset(sim_otp, 0, sizeof(sim_otp)); + memset(sim_otp + OTP_KEY0, 0xA1, 32); + memset(sim_otp + OTP_KEY1, 0xB2, 32); + otp_read_rc = EOS_OK; + otp_revoke_read_rc = EOS_OK; + otp_write_rc = EOS_OK; + otp_write_calls = 0; + eos_hal_init(&sim_board); +} + +static void otp_detach(void) +{ + eos_hal_init(NULL); +} + static int tests_run = 0; static int tests_passed = 0; @@ -107,6 +170,98 @@ TEST(test_keystore_security_version) ASSERT(rc == EOS_OK); } + +/* A device with OTP keys whose revocation store cannot be read must not keep + * using those keys. Revocation exists to retire a key believed compromised, so + * "I could not check" has to mean "do not use", not "not revoked". */ +TEST(test_unreadable_revocation_store_does_not_grant_keys) +{ + otp_reset(); + otp_revoke_read_rc = EOS_ERR_FLASH; + + eos_keystore_t ks; + ASSERT(eos_keystore_init(&ks) == EOS_OK); + + const uint8_t *key = NULL; + size_t key_len = 0; + ASSERT(eos_keystore_get_active_key(&ks, &key, &key_len) != EOS_OK); + + uint32_t count = 1; + ASSERT(eos_keystore_key_count(&ks, &count) == EOS_OK); + ASSERT(count == 0); + + otp_detach(); +} + +/* A readable revocation store still works normally: slot 0 revoked, slot 1 not. */ +TEST(test_revocation_flags_are_honoured) +{ + otp_reset(); + sim_otp[OTP_REVOKE] = 0x01; + + eos_keystore_t ks; + ASSERT(eos_keystore_init(&ks) == EOS_OK); + + const uint8_t *key = NULL; + size_t key_len = 0; + ASSERT(eos_keystore_get_active_key(&ks, &key, &key_len) == EOS_OK); + + /* Slot 0 is revoked, so the active key must be slot 1's. */ + uint8_t expect[32]; + memset(expect, 0xB2, sizeof(expect)); + ASSERT(memcmp(key, expect, 32) == 0); + + otp_detach(); +} + +/* A board that HAS an OTP but fails to read it has an unknown trust anchor. + * Falling back to the compiled-in key would let a fault on the OTP bus swap + * which key the device trusts. */ +TEST(test_failed_otp_read_does_not_fall_back_to_the_compiled_key) +{ + otp_reset(); + otp_read_rc = EOS_ERR_FLASH; + + eos_keystore_t ks; + ASSERT(eos_keystore_init(&ks) != EOS_OK); + + otp_detach(); +} + +/* Revoking must not clear another slot's revocation bit, and must report a + * failure to persist -- an unpersisted revocation is gone at the next reset. */ +TEST(test_revocation_is_persisted_without_clobbering_other_slots) +{ + otp_reset(); + sim_otp[OTP_REVOKE] = 0x01; /* slot 0 already revoked */ + + eos_keystore_t ks; + ASSERT(eos_keystore_init(&ks) == EOS_OK); + ASSERT(eos_keystore_revoke_slot(&ks, 1) == EOS_OK); + + ASSERT(otp_write_calls == 1); + ASSERT(sim_otp[OTP_REVOKE] == 0x03); /* both bits, not just slot 1 */ + + otp_detach(); +} + +TEST(test_revoke_reports_a_failed_persist) +{ + otp_reset(); + otp_write_rc = EOS_ERR_FLASH; + + eos_keystore_t ks; + ASSERT(eos_keystore_init(&ks) == EOS_OK); + ASSERT(eos_keystore_revoke_slot(&ks, 0) != EOS_OK); + + /* The revocation still applies for this boot even though it did not stick. */ + uint32_t count = 99; + ASSERT(eos_keystore_key_count(&ks, &count) == EOS_OK); + ASSERT(count == 1); /* slot 1 remains */ + + otp_detach(); +} + int main(void) { printf("=== eBootloader: Keystore Unit Tests ===\n\n"); @@ -115,8 +270,13 @@ int main(void) run_test_keystore_get_active_key(); run_test_keystore_null_args(); run_test_keystore_security_version(); + run_test_unreadable_revocation_store_does_not_grant_keys(); + run_test_revocation_flags_are_honoured(); + 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(); - tests_run = 4; + tests_run = 9; printf("\n%d/%d tests passed\n", tests_passed, tests_run); return (tests_passed == tests_run) ? 0 : 1; }