fix(TcSecurityDeframer): a factory-fresh board cannot be provisioned (ship blocker)

[Mikefly123]

Summary

On hmac-to-storage as it stands, a board whose key store file does not exist can never be provisioned. It boots keyless (correct) and then refuses PROVISION_KEY with StoreUnreadable, permanently. That is total, unrecoverable command loss on any brand-new board, or any board whose /keys partition is erased or reformatted — including the recovery path you would reach for after a keystore corruption.

This PR targets hmac-to-storage, not main.

Failure mode

PROVISION_KEY (and ADD_KEY/REMOVE_KEY) gate on the key store re-read:

if (loadStatus != Os::File::OP_OK && loadStatus != Os::File::DOESNT_EXIST) {
    log_WARNING_HI_KeyProvisionFailed(StoreUnreadable);   // refuse
}

The chain:

1. loadKeyStore() → Utilities::FileHelper::readFromFile() → Os::File::open(..., OPEN_READ).
2. lib/fprime-zephyr/fprime-zephyr/Os/File.cpp:77-80, ZephyrFile::open:

   int return_code = fs_open(...);
   if (return_code != 0) { status = OTHER_ERROR; }

   It inspects no errno. Os::File::DOESNT_EXIST is unreachable on the Zephyr target. A missing file yields OTHER_ERROR.
3. OTHER_ERROR is neither OP_OK nor DOESNT_EXIST → StoreUnreadable → refused. Forever, since the only way to get a key on is the command being refused.

That File.cpp code is byte-identical at the pre-PR pin 5772b491 and this PR's pin 60d395ed, so the pin bump does not change it.

Regression origin

Commit b214dd88c26 ("fix: address PR #472 review…") added the StoreUnreadable gate. The original 69dcec747d6 gated only on activeKeyCount() != 0, which reads the in-memory m_keyStore — default-empty on a fresh boot — and therefore worked on a cold board.

Why CI and unit tests stayed green

• Hardware: every bench/CI board was provisioned under 69dcec747d6, and the littlefs keystore partition survives reflashing. The store file is never absent, so the cold path is never exercised. Those boards return NotEmpty, which the integration test (correctly, for that state) tolerates.
• Host unit tests: gtest runs on the POSIX Os layer, where open() does map ENOENT → DOESNT_EXIST. The buggy gate passes on the host. This is a pure host/target divergence: no component-level host test could have caught it.

Why the obvious fix is a security hole

The tempting fix is to map -ENOENT → DOESNT_EXIST in the Zephyr Os::File shim. That reintroduces exactly what b214dd88c26 closed.

PROVISION_KEY is bypass-allowlisted (Components/ProvesRouter/Bypasser.cpp) — unauthenticated and reachable over RF. So if a store we merely failed to read counted as "empty", an attacker who can induce a read failure installs their own key while a valid key still sits on flash: full hijack.

And -ENOENT cannot carry that weight. In Zephyr subsys/fs/fs.c, fs_open → fs_get_mnt_point returns -ENOENT when the mount point itself is not mounted (fs.c:118-119, 147-150) — the same code as a genuinely missing file. So "ENOENT ⇒ empty ⇒ provisionable" hands the satellite to anyone who can keep /keys from mounting.

The fix

Stop inferring anything from the read status; probe the filesystem for the two facts that actually matter:

Signal	How	Meaning
Is /keys really mounted?	Os::FileSystem::getFreeSpace("/keys") → fs_statvfs	a success means the mount resolved and the FS answered
Is the store file really absent?	Os::FileSystem::getSingleton()._getPathType(path) → fs_stat, keeping the real status	DOESNT_EXIST is a positive "not there"

(Os::FileSystem::getPathType() — the convenience static — is deliberately not used: it folds every error into NOT_EXIST, which would turn an I/O error into a false "absent" and re-open the hole.)

Admission policy, in Types.hpp as pure C++:

constexpr bool storeStateIsKnown(MountProbe mount, StoreProbe store) {
    return (store == StoreProbe::Present) || (store == StoreProbe::Absent && mount == MountProbe::Live);
}
constexpr bool storeIsProvisionable(MountProbe mount, StoreProbe store, uint8_t activeKeyCount) {
    return storeStateIsKnown(mount, store) && activeKeyCount == 0;
}

Note the asymmetry: Present needs no mount probe (a successful full read is proof the FS served the file); Absent requires one, because absence alone is ambiguous on Zephyr.

The security property is preserved and is now testable: an unreadable store is never provisionable, and an "absent" file on an unproven mount is never provisionable. ADD_KEY/REMOVE_KEY get the same predicate — their gates had the identical defect, so on target they too would refuse on a store that Zephyr reports as OTHER_ERROR.

Scope note: this is solved entirely inside proves-core-reference. Fixing ZephyrFile::open's errno handling upstream is still worth doing, but it is a separate fork PR + submodule pin bump, and on its own it would not be sufficient (see the mount-point ambiguity above).

Regression protection

A plain host gtest cannot catch this bug class, so the decision was extracted into a pure, injectable seam that consumes probe results rather than an Os status. There is no "missing file" status input left to get wrong.

test/unit-tests/test_TcSecurityDeframer_KeyStorePolicy.cpp (7 tests):

• cold board on a live mount is provisionable (the C1 regression)
• readable-but-empty store is provisionable
• a provisioned board is not (trust-on-first-use is one-shot)
• an unreadable store is never provisionable, at any mount state or key count
• an absent file without a live mount is never provisionable (the naive-ENOENT trap)
• exhaustive matrix over mount × store-probe × key-count, pinning the entire domain
• an explicit test documenting the host/target divergence: the same physical condition (file genuinely absent) produced DOESNT_EXIST on host and OTHER_ERROR on target, and the legacy status-based gate gave opposite answers for the same board state. Anyone reintroducing a raw-status gate trips this test's rationale.

test/int/provision_key_test.py:

• the NotEmpty tolerance is now documented as the only tolerated failure, with an explicit note that StoreUnreadable must never be accepted here — on a keyless board it is precisely the cold-lockout signature, and accepting it is how this shipped. (The assertion was already narrow; the reasoning was not written down.)
• added test_cold_provision_gap, a skipped test that names the coverage hole rather than faking it. A real cold test needs a genuinely blank /keys, and nothing in CI can produce one: every board is already provisioned, littlefs survives reflashing, and the existing fsFormat.FORMAT formats the FatFS /, not the littlefs /keys. Closing it needs new firmware capability (an authenticated keystore-erase command, or flashing a blank keystore image over SWD). I did not invent a mock cold board, since a mock would re-hide the very divergence at issue.

What I verified vs. did not

Verified:

• make test-unit: 9/9 test binaries pass, including the new test_TcSecurityDeframer_KeyStorePolicy (7/7). Full run:

  7/9 Test #7: test_TcSecurityDeframer_KeyStorePolicy ...   Passed    0.11 sec
  100% tests passed, 0 tests failed out of 9
  

• Full pre-commit gate passes on the commit (clang-format, cpplint, ruff, codespell, interrogate, SDD docs sync).
• The ZephyrFile::open claim and the fs_get_mnt_point -ENOENT behaviour were read directly from the pinned sources, and confirmed byte-identical between 5772b491 and 60d395ed.

NOT verified — please check these on the bench before merging:

• No firmware build. This work was done in a git worktree, which cannot resolve the lib/zephyr-workspace/zephyr submodule, so TcSecurityDeframer.cpp has not been compiled for the target (or at all — the host gtest suite only compiles the pure layer). The new code uses Os::FileSystem::getSingleton()._getPathType() and Os::FileSystem::getFreeSpace(); both are public and present at this pin, but a compile is the real check. lib/fprime / lib/fprime-extras were temporarily symlinked from a full checkout to run the host gtest suite; the symlinks are not part of the commit.
• No HWIL run. The end-to-end claim — that a board with a blank /keys now accepts PROVISION_KEY — is untested on hardware. The decisive test is: erase/reflash the keystore partition on a bench board, boot, and confirm PROVISION_KEY yields KeyProvisioned rather than KeyProvisionFailed(StoreUnreadable).
• fs_statvfs on a littlefs mount is assumed to succeed on the flight config. If littlefs is built without statvfs support and returns -ENOTSUP, probeMount() reports Unknown and provisioning stays refused — same symptom, different cause. Worth confirming on hardware in the same session as the point above.

Out of scope

Reported separately, deliberately not touched here: the missing no-format; on the littlefs fstab node, the absence of a keystore recovery command, the rotation reload bug, and seq-file locking.

🤖 Generated with Claude Code

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 63d7db4d-a509-457b-b318-a2ca031d9e15

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[nateinaction]

Thanks!