fix(keystore): fail closed when the OTP trust anchor cannot be read

[Kartikey1306]

The architectural problem

Every decision secure boot makes rests on one question: which public key does this device trust? core/keystore.c answers it. Three of its paths answer it with something weaker when hardware misbehaves, rather than refusing to answer.

This is the same shape as #38 (CRC32 failing open on an unreadable flash region), one layer up: the failure isn't in checking the image, it's in establishing what to check it against.

An unreadable revocation store revokes nothing

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;
}

On failure the flags stay clear and every key looks live. Revocation exists to retire a key whose private half is believed compromised, so "I could not check" has to mean "do not use" — not "not revoked". It now marks every OTP slot revoked, and eos_keystore_get_active_key() reports EOS_ERR_KEY instead of handing back a key that may have been retired.

A failed OTP read is treated as no OTP at all

Any non-EOS_OK result skipped the whole OTP block and fell through to the compiled-in key. But EOS_ERR_NOT_SUPPORTED (this board has no OTP) and EOS_ERR_FLASH (this board has one and reading it failed) are not interchangeable. The second means the trust anchor this device was provisioned with is unknown, and quietly substituting the compiled-in key lets a fault on the OTP bus choose which key the device trusts. Only the former falls back now.

Revoking a slot cleared the others, and reported success when it didn't stick

uint8_t revoke_flags = 0;
eos_hal_otp_read(OTP_REVOKE_OFFSET, &revoke_flags, 1);   /* result ignored */
revoke_flags |= (1U << slot);
eos_hal_otp_write(OTP_REVOKE_OFFSET, &revoke_flags, 1);  /* result ignored */

• If the read failed, revoke_flags stayed 0, and writing it back cleared every other slot's revocation bit — revoking slot 1 un-revoked slot 0.
• If the write failed, the revocation lived in RAM only and was gone at the next reset, while this returned EOS_OK and the caller believed the key was permanently retired.

Both are reported now. The in-RAM revocation stands either way, so the current boot still refuses the key.

Also: the default trust anchor is a published keypair

static const uint8_t default_dev_key[EOS_ED25519_PUB_KEY_SIZE] = {
    0xd7, 0x5a, 0x98, 0x01, 0x82, 0xb1, 0x0a, 0xb7, ...

That is the public half of TEST 1 in RFC 8032 §7.1. The matching private key is printed in the RFC, so anyone at all can produce a signature a bootloader trusting this key will accept. It is a reasonable default for bring-up and for these tests — and shipping it silently is the failure mode. A build without EBLDR_PRODUCTION_KEY now says so on every compile via #warning. There is no -Werror here, so this does not break a build.

I have deliberately not made it a hard error: that is a policy call for maintainers, and it would break the default board build.

Verification

Check	Result
ctest --no-tests=error	16/16 pass
pytest tests/	13 passed, 1 skipped

Five new tests in tests/unit/test_keystore.c, driven through a simulated OTP registered as a board:

• an unreadable revocation store yields no usable key
• readable revocation flags still select the non-revoked slot
• a failed OTP read does not fall back to the compiled-in key
• revoking slot 1 preserves slot 0's revocation bit (0x03, not 0x02)
• a revocation that could not be persisted is reported as a failure

Against the unpatched keystore.c the first of these fails on exactly the assertion it exists to make. The four pre-existing keystore tests are unchanged and still run against the compiled-key path, since no board is registered for them.

Stacking

Based on #58, which restores the build — master does not compile, so these tests cannot run without it. Rebase target once #58 or an equivalent (#55, #57) lands; the keystore changes themselves are independent of all of them.

🤖 Generated with Claude Code

[codecov-commenter]

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[srpatcha]

Verified — and the framing is the right one

"Which public key does this device trust?" is the question every other secure-boot decision rests on, and answering it with something weaker when hardware misbehaves is worse than refusing to answer. The parallel you draw to #38 — CRC32 failing open on an unreadable flash region — is exact: same failure mode, one layer up, and the layer that matters more.

The revocation case is the one I would not want left:

uint8_t revoke_flags = 0;
if (eos_hal_otp_read(OTP_REVOKE_OFFSET, &revoke_flags, 1) == EOS_OK) {
    ...
}

An OTP read that fails leaves revoke_flags at its initialiser, so an unreadable revocation store revokes nothing. That is precisely backwards: a device that cannot read which keys were revoked is a device that must assume the worst, not the best. And it is silent — no branch, no log, just a key that was revoked continuing to verify images.

Fail-closed is the only defensible default here. A bootloader that refuses to boot is recoverable; one that boots an image signed by a revoked key is not.

Verification

Merged onto origin/master + #58: 0 build errors, ctest 16/16.

Not verified against master alone — master does not build (eos_crc32 conflicting types), which #58 repairs.

One blocker outside this PR

eBoot has required_signatures: true on master, and your commits are unsigned, as are every other contributor's on this repo. That makes this PR — and #57, #58, #61, #62 — unmergeable regardless of review. eos, ebuild, eDB and most of the org do not have that setting; eBoot, eCAD and eNI do. I have flagged it to the maintainer as a policy decision: turn it off, or ask contributors to sign. Nothing for you to fix.

[Kartikey1306]

Rebased onto current master. The branch had gone stale in a way that mattered: it was still carrying #58's content, and since master picked that up (plus more) it would have reverted work that has landed since — tests/unit/test_image_abi.c, tests/unit/test_cmake_test_registration.py, and the better test_slot_manager.c and test_recovery.c now on master.

The PR is now the keystore change and nothing else: 2 files, +221/−12.

Re-verified on the rebased branch:

Check	Result
cmake --build (EBLDR_BUILD_TESTS=ON)	clean, 0 errors
ctest --no-tests=error	18/18 pass
eboot_test_keystore	9/9 pass

Nothing in the fix itself changed — core/keystore.c cherry-picked onto master without conflict, and I confirmed the fail-open paths are still present on master (no otp_present, and the revocation read still leaves revoke_flags at its initialiser on failure), so this is still needed.

Noted on the required_signatures: true blocker — thank you for chasing it as a policy question. My commits are unsigned and there is no signing key configured on this machine, so I cannot resolve it from my side without that decision being made first. Happy to re-sign and force-push the moment there is a direction.