Skip to content

soc: apple: sep: preserve unreadable and partial identity state - #234

Open
satchlj wants to merge 3 commits into
omacom:integration/aurorafrom
satchlj:satchlj/sep-store-open-errors
Open

satchlj wants to merge 3 commits into
omacom:integration/aurorafrom
satchlj:satchlj/sep-store-open-errors

Conversation

@satchlj

@satchlj satchlj commented Oct 26, 2026 •

Copy link
Copy Markdown

An unreadable or partial saved record must not become evidence that nothing is saved. Several readers currently make that substitution, and the callers then act on "absent": creating a replacement reference key, a second keybag, or a fresh enrollment context. Those actions orphan state held by the enclave.

Changes

  • Open errors. The regular-file C wrappers collapse every open error to NULL, and Rust turns NULL into ENOENT. They now return the error pointer, using the from_err_ptr convention open_host already follows.
  • Reference key. sks_machine_refkey creates a new key after any open failure. It now creates one only after a genuine ENOENT.
  • Identity index. IdentityIndex::load accepts short input as empty, caps oversized counts and returns a prefix of a truncated file. It now requires a complete header, a bounded count and the exact length the writer produces.
  • Refused keybag. keybag::read trusts STATE_REFUSED before checking the rest of the record. It now requires the exact size, checksum and zero fields that mark_refused writes. Valid committed records are still accepted.
  • Catacombs. catacomb::read returns Option and maps every failure to None, which the enrollment, restore, deletion and owner-probe paths read as "no saved catacomb". It now returns Result<Option<_>>, where Ok(None) means only that the file is missing. Callers use is_saved (present and valid) or is_absent (provably missing). read_stored propagates read errors, so restore reports Failed instead of NoStoredFile. A host-store read error is no longer treated as "not stored" either.

Behavior change: a damaged index, refusal record or catacomb now fails loudly instead of silently reading as empty. That is the intent, but such a device now needs manual repair instead of quietly re-enrolling over its saved state.

Storage formats, open flags, the enrollment protocol and xART policy are unchanged.

Validation

  • tools/aurora-sep/tests/test_store_open_errors.py: compiles the actual C wrapper bodies against a fake open_as_kernel, plus the unchanged shim.rs against a small kernel-API facade. Success and six errno values are preserved through C and Rust. Only successful handles are closed. A negative control restores the old NULL collapse and fails.
  • tools/aurora-sep/tests/test_identity_preservation.py: compiles the production reference-key, index, refused-keybag and catacomb readers against in-memory seams, in the style of test_sbio_exchange.py. Each test also restores the old behavior and confirms the test catches it. A caller test compiles the real restore_catacomb, read_stored and probe_owner_export: an unreadable catacomb or host store restores as Failed without contacting the enclave, and the owner probe stops before its enclave check when the owner or master file cannot be read. Mapping the error arm back to NoStoredFile, or letting is_absent accept a read error, fails it.
  • All nine pass on aarch64 Linux (rustc 1.93.1), under normal and optimized Python. git diff --check is clean.
  • A full make LLVM=1 kernel and modules build of this branch (arm64, Rust enabled, CONFIG_APPLE_SEP=m) succeeds, and apple-sep.ko links. drivers/soc/apple/ builds with no warnings at W=1. Clippy reports nothing new on changed lines.
  • No new-driver hardware test is claimed.

Notes for follow-up work (not changed here)

These came from observing one M3 enrollment that saved the owner and user catacombs but not the master. They are recorded to help whoever works on recovery next. No code here depends on them.

Partial-state recovery. After a completed enrollment, the host can be left with some catacombs saved and others missing. Today a missing catacomb at restore is NoStoredFile, and later enrollment can select a fresh context for that component. For the master, that discards the enclave-side state the saved owner and user catacombs depend on. A recovery design probably needs:

  • a way to tell "never saved" apart from "saved set is incomplete", for example a small commit record written after the last catacomb, or checking the set as a unit;
  • a refusal to select a fresh context while any catacomb of the set is present, unless the enclave proves it has zero identities (NoExistingCatacomb already models this for the owner probe);
  • a supported way to finish an incomplete save. It is unclear whether one exists once the enrollment session has closed. In the observed case the session was closed before persistence was attempted, so no in-session retry was possible;
  • a supported way to remove an orphaned keybag. No delete path for a keybag the host no longer references has been identified. Observing how macOS removes one would be the clean way to learn it.

Enclave acknowledgment handling. In the observed case the master component reported save-pending state (0x7) after the user catacomb was saved and confirmed, but SAVE_CATACOMB (0x6c) for the master was refused with status 0x6. The existing comment in save_all_components describes the same pattern on m2fix11. Status 0x6 has no name yet. Open questions:

  • Does 0x6 mean the save is out of order, the session is gone, or a precondition such as the OS volume UUID used for anti-replay is unavailable? On the observed boot the xART path reported /chosen/apfs-preboot-uuid unavailable. That correlation is not shown to be the cause.
  • Is a refused save retryable within the same session, and is the component's state unchanged after a refusal?
  • Should the driver read component states again and stop before saving or confirming any component once one save is refused, so the host never commits a partial set?

Until those are answered, treating any non-OK save status as a hard stop before further saves or confirmations, and reporting which components did persist, seems the safest default.

Signed-off-by: Satya Benson <satya@benson.earth>
@satchlj satchlj changed the title soc: apple: sep: preserve backing-store open errors soc: apple: sep: preserve unreadable and partial identity state Oct 26, 2026
Signed-off-by: Satya Benson <satya@benson.earth>
@satchlj
satchlj force-pushed the satchlj/sep-store-open-errors branch from 55b5b35 to 87d3222 Compare October 26, 2026 02:29
@satchlj
satchlj marked this pull request as ready for review October 26, 2026 03:18

@scottjones scottjones left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @satchlj, this is the right direction: every change makes a reader refuse rather than guess, and none of them adds a write. We checked the strict readers against the writers that exist today. IdentityIndex::save writes exactly 8 + count * 144 bytes. keybag::mark_refused writes a header-only record with zero length, UUID and secret length, and a CRC over the same preimage the new check computes. Neither writer's format has changed since the driver landed, so we don't see a valid record on an existing Mac that the new readers would reject.

We also ran the new tests and reverted each fix by hand to check they bite. Restoring the NULL collapse in store_shim.c, letting sks_machine_refkey create a key after any open error, dropping the CRC check on the refused-keybag record, and accepting a truncated index each fail a test.

One gap: nothing tests the caller side. With catacomb::is_absent changed back to treat a read error as absent (catacomb.rs, the new is_absent), or with read_stored's error arm in restore_catacomb mapped back to NoStoredFile (sbio.rs:654), both new test files still pass. restore_catacomb's error arm (sbio.rs:654) runs on every boot, so a test that an unreadable catacomb gives Failed, not NoStoredFile, is the one worth adding next to test_identity_preservation.py; the same for the opt-in probe_owner_export (sbio.rs:513).

Small nit in the description: the refused record is written by mark_refused, not write_intent. write_intent writes state 0, which read already treats as EEXIST.

What we ran: both new test files pass (8 tests, rustc 1.93.1). We built drivers/soc/apple/ with the release19 config (LLVM=1 W=1) on a merge of this branch and #180 (at 0dfa5ae), and it builds clean with no new warnings in the SEP files. #180's newest commit (21d2c81) now also touches keybag.rs, shim.rs and store_shim.c, so whichever lands second will need a look at those three files.

We'd be happy to approve once the caller test is in.

catacomb::read() mapped every failure to None, which the enrollment,
restore, deletion and owner-probe paths read as "no saved catacomb".
Return Result<Option<_>> so that only a missing file reads as absent,
add is_saved()/is_absent() for the callers, and make read_stored()
propagate read errors so restore reports Failed rather than
NoStoredFile. Test both callers that run on these paths:
restore_catacomb() must fail without contacting the enclave on an
unreadable catacomb or host store, and the owner probe must stop before
its enclave check when the owner or master file cannot be read. Drop a
source-text assertion made redundant by the native boundary test.

Signed-off-by: Satya Benson <satya@benson.earth>
@satchlj
satchlj force-pushed the satchlj/sep-store-open-errors branch from 87d3222 to accecc6 Compare October 27, 2026 03:03
@satchlj

satchlj commented Oct 27, 2026

Copy link
Copy Markdown
Author

Thanks @scottjones. The caller test is in (accecc6e2, folded into the catacomb commit).

test_callers_refuse_unreadable_catacombs in test_identity_preservation.py compiles the real restore_catacomb, read_stored, probe_owner_export, is_saved and is_absent against small seams:

  • An unreadable catacomb gives Failed, and nothing is sent to the enclave. An unreadable host-store copy also gives Failed. Only a genuinely missing file gives NoStoredFile, and a valid host-store copy still restores.
  • probe_owner_export reaches its enclave identity check when the owner file is missing and master/user are saved. It stops before that check when the owner file is unreadable (EIO or EACCES), or when the master file is unreadable.
  • Both mutations you named fail it: the error arm at sbio.rs:654 mapped back to NoStoredFile, and is_absent treating a read error as absent.

All nine tests pass (rustc 1.93.1, normal and python3 -O). The driver code is unchanged since your build, so that result still stands. I also fixed the description: the refused record comes from mark_refused, not write_intent.

On #180: if it lands first, I'll rebase this onto it and recheck keybag.rs, shim.rs and store_shim.c.

Written by Claude on @satchlj's behalf.

@scottjones scottjones left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, that covers it. We checked accecc6 against 87d3222: the only change is the new caller test in test_identity_preservation.py, so the driver code we built and reviewed is unchanged. The test compiles the real restore_catacomb, read_stored, probe_owner_export, is_saved and is_absent bodies out of sbio.rs and catacomb.rs, and it applies both mutations we named, with a guard that each mutation actually changed the code, so it can't pass vacuously. We read the test rather than running it this time. Approving.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants