prepare-root: Add composefs.enabled=verity#3354
Conversation
|
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 Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions 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. |
cgwalters
left a comment
There was a problem hiding this comment.
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.
cgwalters
left a comment
There was a problem hiding this comment.
Thanks, looks sane. Can you also update man/ostree-prepare-root.xml?
|
Also ref composefs/composefs#251 |
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. |
Done. PTAL. |
bae950c to
5831fb5
Compare
|
/ok-to-test |
|
/override ci/prow/fcos-e2e |
|
@cgwalters: Overrode contexts on behalf of cgwalters: ci/prow/fcos-e2e DetailsIn response to this:
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. |
|
To help with upgrades, would a |
You mean setting option It only supports either no option (equivalent to https://www.kernel.org/doc/html/latest/filesystems/overlayfs.html#fs-verity-support BTW, To upgrade, you can simply enable fs-verity for all files under |
We have duplicate code to parse this between C and Rust unfortunately; update the Rust side to honor what landed in ostreedev/ostree#3354
We have duplicate code to parse this between C and Rust unfortunately; update the Rust side to honor what landed in ostreedev/ostree#3354 Signed-off-by: Colin Walters <walters@verbum.org>
We have duplicate code to parse this between C and Rust unfortunately; update the Rust side to honor what landed in ostreedev/ostree#3354 Signed-off-by: Colin Walters <walters@verbum.org>
|
I'm late to the party here but is it too late to change the option? Before this change, afaik, all options were boolean and the config file was effectively toml. This takes us into a somewhat more freeform direction.... probably ini-ish.... that might make things more difficult in the future if we want to add more advanced features. Something more like [composefs]
enable=true
verity=trueinstead? |
It's always been ini, we've parsed using GKeyFile; and especially booleans in ini/GKeyFile accept values like "yes/no" and "0/1" that aren't valid in TOML. You're right of course regardless that it'd definitely have been cleaner to use a separate boolean for I'm assuming you're asking this because you want to reimplement it in composefs-rs, but note there is already Rust code in bootc that's parsing this too, and I did have to adjust it for this in e.g. bootc-dev/bootc@06933ed |
In the current implementation, a composefs deploy is either:
This PR adds a new supported value
veritytocomposefs.enabled, which:The idea is that:
LCFS_MOUNT_FLAGS_REQUIRE_VERITY(the underlying of which isverity=requirewhen mounting overlayfs), overlayfs does not verify file data even if fs-verity digests are present in the image.