Skip to content

chore: One more pass at guest_disk simplification - #11394

Merged
frankdavid merged 2 commits into
masterfrom
frankdavid/encryption-simplifications-2
Sep 3, 2026
Merged

chore: One more pass at guest_disk simplification#11394
frankdavid merged 2 commits into
masterfrom
frankdavid/encryption-simplifications-2

Conversation

@frankdavid

Copy link
Copy Markdown
Contributor

Now that we only have maximum one keyslot+token, we can do a few more simplifications / optimizations.

  • Rename KeyslotMetadataKeyslotToken; add SINGLE_KEYSLOT_INDEX/SINGLE_TOKEN_INDEX constants replacing scattered 0 literals
  • Replace read_keyslot_metadata (Vec) with read_single_keyslot_token reading the one token from its fixed position
  • Make token writes deterministic: clear all token positions, then ReplaceToken(SINGLE_TOKEN_INDEX) instead of first-free AddToken
  • Drop the unused keyslot parameter of add_sev_metadata and the KeyslotToken::keyslot() method
  • format_crypt_device and activate_crypt_device no longer return the keyslot/device tuple
  • Collapse the duplicated Var/Store match arms in SevDiskEncryption into a shared header_location() helper
  • Both encryption backends own their metrics Registry
  • Tests assert the single-token invariant via new assert_single_metadata_token/assert_no_metadata_token helpers

Now that we only have maximum one keyslot+token, we can do a few more simplifications / optimizations.

- Rename `KeyslotMetadata` → `KeyslotToken`; add `SINGLE_KEYSLOT_INDEX`/`SINGLE_TOKEN_INDEX` constants replacing scattered `0` literals
- Replace `read_keyslot_metadata` (Vec) with `read_single_keyslot_token` reading the one token from its fixed position
- Make token writes deterministic: clear all token positions, then `ReplaceToken(SINGLE_TOKEN_INDEX)` instead of first-free `AddToken`
- Drop the unused `keyslot` parameter of `add_sev_metadata` and the `KeyslotToken::keyslot()` method
- `format_crypt_device` and `activate_crypt_device` no longer return the keyslot/device tuple
- Collapse the duplicated Var/Store match arms in `SevDiskEncryption` into a shared `header_location()` helper
- Both encryption backends own their metrics `Registry`
- Tests assert the single-token invariant via new `assert_single_metadata_token`/`assert_no_metadata_token` helpers
@frankdavid
frankdavid requested a review from a team as a code owner September 1, 2026 13:49
@github-actions github-actions Bot added the chore label Sep 1, 2026
@zeropath-ai

zeropath-ai Bot commented Sep 1, 2026

Copy link
Copy Markdown

No security or compliance issues detected. Reviewed everything up to 421fa44.

Security Overview
Detected Code Changes
Change Type Relevant files
Enhancement ► rs/ic_os/os_tools/guest_disk/src/crypt.rs
    Update keyslot/token metadata handling: introduce IC_KEY_TOKEN_TYPE, SINGLE_KEYSLOT_INDEX, SINGLE_TOKEN_INDEX; rename and adjust KeyslotMetadata to KeyslotToken; modify new_sev constructor; adjust activation, formatting, and token management to use single token/index; simplify read/add token flows; update read/replace token usage
Enhancement ► rs/ic_os/os_tools/guest_disk/src/generated_key.rs
    Adjust GeneratedKeyDiskEncryption to store metrics_registry as a value (not reference) and pass a reference where needed
Enhancement ► rs/ic_os/os_tools/guest_disk/src/main.rs
    Update GeneratedKeyDiskEncryption initialization to clone metrics_registry for ownership compatibility
Enhancement ► rs/ic_os/os_tools/guest_disk/src/sev.rs
    Use SINGLE_KEYSLOT_INDEX for operations; adjust imports to include SINGLE_KEYSLOT_INDEX; adjust header location logic via helper; simplify activate_crypt_device call usage with new header_location method; streamline SEV flow
Enhancement ► rs/ic_os/os_tools/guest_disk/src/tests.rs
    Update tests to reflect KeyslotToken and SINGLE_TOKEN_INDEX usage; replace read_keyslot_metadata with read_single_keyslot_token; adjust expectations for token presence; adapt test names and assertions accordingly

@frankdavid
frankdavid requested review from Bownairo and a balanced review from Copilot September 1, 2026 14:10

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Simplifies guest disk encryption around the single-keyslot and single-token model.

Changes:

  • Introduces fixed keyslot/token indices and deterministic token handling.
  • Streamlines crypt-device APIs and SEV header selection.
  • Updates registry ownership and invariant-focused tests.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Show a summary per file
File Description
crypt.rs Simplifies keyslot/token APIs and fixed-index handling.
sev.rs Consolidates header selection and updates rekeying.
generated_key.rs Owns its metrics registry.
main.rs Shares the metrics registry through cloning.
tests.rs Verifies single-token and no-token invariants.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@frankdavid
frankdavid enabled auto-merge September 3, 2026 13:03
@frankdavid
frankdavid added this pull request to the merge queue Sep 3, 2026
Merged via the queue into master with commit feec8df Sep 3, 2026
42 checks passed
@frankdavid
frankdavid deleted the frankdavid/encryption-simplifications-2 branch September 3, 2026 13:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants