Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

storage: Enable confidential ephemeral volumes #230

Draft
wants to merge 3 commits into
base: msft-main
Choose a base branch
from

Conversation

sprt
Copy link
Collaborator

@sprt sprt commented Sep 11, 2024

Merge Checklist
  • Followed patch format from upstream recommendation: https://github.com/kata-containers/community/blob/main/CONTRIBUTING.md#patch-format
    • Included a single commit in a given PR - at least unless there are related commits and each makes sense as a change on its own.
  • Aware about the PR to be merged using "create a merge commit" rather than "squash and merge" (or similar)
  • genPolicy only: Ensured the tool still builds on Windows
  • The upstream/missing label (or upstream/not-needed) has been set on the PR.
Summary

Please see commit messages. Note that this approach follows the driver-shim interface design (with the new metadata driver options) that we agreed to propose upstream with Intel. When we sync our fork with upstream, the agent code will look much different, but those agent changes should be transparent to the end user.

Upstream proposal: confidential-containers/confidential-containers#247

Depends on:

Migration flows:

  • Of course if you want confidentiality at all you'll need to upgrade to these PRs.
  • If you only upgrade Kata but not your driver, the local volumes will still work, just not confidential.
  • If you only upgrade your driver but not Kata, you're broken because you need the Kata bits and the driver is now hardcoded to be confidential.
Test Methodology

Local testing

@sprt sprt added upstream/not-needed PRs that will not be upstreamed (e.g. internal) upstream/missing PRs that are yet to be upstreamed and removed upstream/not-needed PRs that will not be upstreamed (e.g. internal) labels Sep 11, 2024
src/agent/src/storage/mod.rs Outdated Show resolved Hide resolved
src/agent/src/storage/mod.rs Show resolved Hide resolved
src/agent/src/storage/mod.rs Outdated Show resolved Hide resolved
src/agent/src/storage/mod.rs Outdated Show resolved Hide resolved
src/agent/src/storage/mod.rs Outdated Show resolved Hide resolved
This enables confidential ephemeral storage by allowing the CSI driver
to pass options confidential=true and ephemeral=true.

When these options are provided, the agent will generate a random
encryption key and use cryptsetup to encrypt the host device, format it
as an ext4 filesystem, and expose it to the container.

Signed-off-by: Aurélien Bombo <[email protected]>
@sprt
Copy link
Collaborator Author

sprt commented Oct 17, 2024

New changes:

  • Addressed PR comments.
  • Deploying the luks-encrypt-storage script from upstream at runtime: upstream checks the integrity tags in a lazy way (--integrity-no-wipe), and we need to copy that as the eager version has a significant performance overhead (~100sec for a 10GB per my testing). It was much easier to call a script rather than to convert all those commands to Rust Command::new() calls.
    • This also means we are now fully aligned with upstream's encryption settings.

Next step: genpolicy changes.

@sprt
Copy link
Collaborator Author

sprt commented Oct 17, 2024

New changes: added genpolicy support.

@sprt sprt marked this pull request as ready for review October 17, 2024 20:39
@sprt sprt requested review from a team as code owners October 17, 2024 20:39
sprt added 2 commits October 17, 2024 20:40
This adds a new setting to genpolicy to support confidential ephemeral volumes.

Signed-off-by: Aurélien Bombo <[email protected]>
Using a symlink would create a cycle after calling this script again
when copying the final configuration at line 74 so we just use cp
instead.

Signed-off-by: Aurélien Bombo <[email protected]>
@Redent0r
Copy link

Redent0r commented Oct 18, 2024

Looks good to me from what I understand. My only feedback (possibly for a different PR) would be how to go about testing this. Would it be roughly something like:

  1. Create pod with ephemeral volume
  2. Write to it
  3. Content of encrypted storage should not be available from the host side
    ?

@sprt sprt marked this pull request as draft December 11, 2024 20:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
upstream/missing PRs that are yet to be upstreamed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants