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

prepare-root: Add composefs.enabled=verity #3354

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ruihe774
Copy link
Contributor

@ruihe774 ruihe774 commented Dec 16, 2024

In the current implementation, a composefs deploy is either:

  • signed by key and verified by fs-verity, or
  • not signed and not verified.

This PR adds a new supported value verity to composefs.enabled, which:

  • requires the composefs to be verified by fs-verity;
  • does not require it to be signed.

The idea is that:

  • If we do not specify LCFS_MOUNT_FLAGS_REQUIRE_VERITY (the underlying of which is verity=require when mounting overlayfs), overlayfs does not verify file data even if fs-verity digests are present in the image.
  • It is difficult and troublesome to have the ostree commit signed in some circumstances. For example, when deploying with custom layering.
  • In some cases, the integrity of the composefs image is already protected by other mechanisms, for example EVM. It is not necessary to verify the image by a key, but it is still necessary to verify file data referenced by the image using fs-verity to protect file integrity from disk corruption.

Copy link

openshift-ci bot commented Dec 16, 2024

Hi @ruihe774. Thanks for your PR.

I'm waiting for a ostreedev member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@github-actions github-actions bot added the area/prepare-root Issue relates to ostree-prepare-root label Dec 16, 2024
Copy link
Member

@cgwalters cgwalters left a comment

Choose a reason for hiding this comment

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

ACK to the overall idea though the way I'd describe this is more "external signing". One thing I could imagine doing here is checking if a stamp file like /run/ostree/composefs-signed or so is present, then we skip our own signature checks and assume that an external systemd unit that ran before this code executed.

src/switchroot/ostree-prepare-root.c Outdated Show resolved Hide resolved
Copy link
Member

@cgwalters cgwalters left a comment

Choose a reason for hiding this comment

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

Thanks, looks sane. Can you also update man/ostree-prepare-root.xml?

@cgwalters
Copy link
Member

Also ref containers/composefs#251

@ruihe774
Copy link
Contributor Author

One thing I could imagine doing here is checking if a stamp file like /run/ostree/composefs-signed or so is present, then we skip our own signature checks and assume that an external systemd unit that ran before this code executed.

I think it's a bit of complicated and over-designed. If we do that, the "external" verification tool needs to explicitly cooperate with ostree and write the stamp file, or the end user needs to write a custom systemd service and add it to initramfs, which is troublesome.

@ruihe774
Copy link
Contributor Author

Can you also update man/ostree-prepare-root.xml?

Done. PTAL.

@ruihe774 ruihe774 changed the title [RFC] prepare-root: Add composefs.enabled=verity prepare-root: Add composefs.enabled=verity Dec 16, 2024
Copy link
Member

@cgwalters cgwalters left a comment

Choose a reason for hiding this comment

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

Looks good!

Our CI just fell over, I filed #3356 to get some things fixed and I think we need to have that resolved before merging more things. But I'll get this in after that, thanks again!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/prepare-root Issue relates to ostree-prepare-root needs-ok-to-test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants