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

repart: support creating CopyFile= filesystems with fs-verity files #35352

Open
allisonkarlitskaya opened this issue Nov 25, 2024 · 6 comments · May be fixed by #35401
Open

repart: support creating CopyFile= filesystems with fs-verity files #35352

allisonkarlitskaya opened this issue Nov 25, 2024 · 6 comments · May be fixed by #35401
Labels
repart RFE 🎁 Request for Enhancement, i.e. a feature request

Comments

@allisonkarlitskaya
Copy link
Contributor

allisonkarlitskaya commented Nov 25, 2024

Component

systemd-repart

Is your feature request related to a problem? Please describe

I'm creating operating system images containing fs-verity-enabled files. With tytso/e2fsprogs#203 I've verified that I can create an ext4 filesystem with fs-verity-enabled files created inside of it from the start, using mkfs.ext4 -O verity -d /path/to/files. Unfortunately that doesn't work with systemd-repart and CopyFiles= because the files are first copied to a temporary directory (like /var/tmp/.#repart7fca5bc0ddc4f765). It takes care to copy the file data, the xattrs, the permissions, and the access times, but doesn't check for fs-verity on the source and doesn't attempt to enable it on the destination.

Describe the solution you'd like

There are two options that would make me happy:

  1. Allow skipping the copy to a temporary directory. In case there are no subdirectories that need masking out, it would actually be easier to just use the data directly from the source (unless I'm missing something).
  2. If copying, check the origin file for fs-verity and enable it on the file in the temporary directory before calling the appropriate mkfs program.
  • this could be a simple check by calling FS_IOC_GETFLAGS and enabling fs-verity with the default options;r or
  • it could inspect the descriptor of the source file to try to do a "more accurate" job wrt. selection of hash algorithm, salt values, etc. I've never seen anything but the defaults used, though.

Bonus points: currently I have to say `SYSTEMD_REPART_MKFS_OPTIONS_EXT4='-O verity'. It would be cool if there was a config option for that, instead.

Describe alternatives you've considered

I've tried manually creating the filesystem image and using CopyBlocks= to copy it into the disk image, but this is pretty inconvenient. Aside from the obvious pain of the extra step, this argument (unlike CopyFiles) is not resolved relative to --root and demands to be given an absolute pathname. That makes it difficult to integrate into a build system that might be checked out in different places, without resorting to dynamically creating the repart.d/ files.

The systemd version you checked that didn't have the feature you are asking for

Fedora 41, systemd 256 (256.7-1.fc41) +PAM +AUDIT +SELINUX -APPARMOR +IMA +SMACK +SECCOMP -GCRYPT +GNUTLS +OPENSSL +ACL +BLKID +CURL +ELFUTILS +FIDO2 +IDN2 -IDN -IPTC +KMOD +LIBCRYPTSETUP +LIBCRYPTSETUP_PLUGINS +LIBFDISK +PCRE2 +PWQUALITY +P11KIT +QRENCODE +TPM2 +BZIP2 +LZ4 +XZ +ZLIB +ZSTD +BPF_FRAMEWORK +XKBCOMMON +UTMP +SYSVINIT +LIBARCHIVE

@allisonkarlitskaya allisonkarlitskaya added the RFE 🎁 Request for Enhancement, i.e. a feature request label Nov 25, 2024
@DaanDeMeyer
Copy link
Contributor

Completely on board with this, might just take me some time before I find the time to implement it

@allisonkarlitskaya
Copy link
Contributor Author

Completely on board with this, might just take me some time before I find the time to implement it

PR on the way.

allisonkarlitskaya added a commit to allisonkarlitskaya/systemd that referenced this issue Nov 28, 2024
When populating a filesytem with CopyFiles=, we first copy the files to
a temporary directory.  Make sure we use the (new) COPY_FS_VERITY flag
when doing that copy so that the `mkfs` that we invoke can see the files
with fs-verity enabled.

Closes systemd#35352

Signed-off-by: Allison Karlitskaya <allison.karlitskaya@redhat.com>
@allisonkarlitskaya
Copy link
Contributor Author

@DaanDeMeyer #35401 covers the "CopyFiles=" part of this issue but this part is outstanding:

Bonus points: currently I have to say `SYSTEMD_REPART_MKFS_OPTIONS_EXT4='-O verity'. It would be cool if there was a config option for that, instead.

Do you have any suggestions there? I see a few options:

  • just do that unconditionally
  • repart.d file option like FsVerity=yes or something which results in this extra flag for the ext4 case
  • dynamic detection of if there happen to be any fs-verity files that got copied in via CopyFiles= (but I consider this evil, and it's also difficult to implement)
  • any other ideas?

I'd be happy to do a patch for that one as well if you can give guidance about your preferred approach.

allisonkarlitskaya added a commit to allisonkarlitskaya/systemd that referenced this issue Nov 28, 2024
When populating a filesytem with CopyFiles=, we first copy the files to
a temporary directory.  Make sure we use the (new) COPY_FS_VERITY flag
when doing that copy so that the `mkfs` that we invoke can see the files
with fs-verity enabled.

Closes systemd#35352

Signed-off-by: Allison Karlitskaya <allison.karlitskaya@redhat.com>
allisonkarlitskaya added a commit to allisonkarlitskaya/systemd that referenced this issue Nov 28, 2024
When populating a filesytem with CopyFiles=, we first copy the files to
a temporary directory.  Make sure we use the (new) COPY_FS_VERITY flag
when doing that copy so that the `mkfs` that we invoke can see the files
with fs-verity enabled.

Closes systemd#35352

Signed-off-by: Allison Karlitskaya <allison.karlitskaya@redhat.com>
allisonkarlitskaya added a commit to allisonkarlitskaya/systemd that referenced this issue Nov 29, 2024
When populating a filesytem with CopyFiles=, we first copy the files to
a temporary directory.  Make sure we use the (new) COPY_FS_VERITY flag
when doing that copy so that the `mkfs` that we invoke can see the files
with fs-verity enabled.

Closes systemd#35352

Signed-off-by: Allison Karlitskaya <allison.karlitskaya@redhat.com>
@poettering
Copy link
Member

Do you have any suggestions there? I see a few options:

* just do that unconditionally

* `repart.d` file option like `FsVerity=yes` or something which results in this extra flag for the ext4 case

* dynamic detection of if there happen to be any fs-verity files that got copied in via `CopyFiles=` (but I consider this evil, and it's also difficult to implement)

* any other ideas?

I'd be happy to do a patch for that one as well if you can give guidance about your preferred approach.

As mentioned in the PR I think there should FsVerity= anyway, controlling whether to do verity for copied files. i figure this should then also result in "-O verity" if set to any other value than "no".

@allisonkarlitskaya
Copy link
Contributor Author

As mentioned in the PR I think there should FsVerity= anyway, controlling whether to do verity for copied files. i figure this should then also result in "-O verity" if set to any other value than "no".

First take: this would be difficult to implement: we'd have to perform the merkle tree calculation ourselves, in userspace.

But: it's not true. We could actually benefit from our copying the files to a temporary directory. We can ask the kernel to enable fs-verity on the files in that directory, run the mkfs program, and then delete it all when we're done. That way we'd still be able to copy the metadata from the running host system.

@allisonkarlitskaya
Copy link
Contributor Author

So: three modes, then?

  • no: no verity handling
  • always: enable verity, and set it for every copied regular file (I don't know of any usecase for this, but perhaps there is one)
  • yes: enable verity, and set it for the files that have it set in the CopyFrom= directory (the case I care about)

In the final two cases we'd have verity enabled on the filesystem level.

allisonkarlitskaya added a commit to allisonkarlitskaya/systemd that referenced this issue Dec 2, 2024
We currently pass the CopyFlags that we use to populate the temporary
directory in the form of a constant at each of the copy_tree_at() call
sites.  De-duplicate that and move it into the `CopyFilesLine` struct,
initializing it from the parser.

Add our first non-constant flag: `fsverity=`.  This can be set to `off`
(the default) or `copy`, in which case we copy the fs-verity state from
the source files.

This arrangement is amenable to the introduction of more flags to
`CopyFiles=` lines, if we want to add them in the future.

Update the `repart.d(5)` manpage.

Closes systemd#35352

Signed-off-by: Allison Karlitskaya <allison.karlitskaya@redhat.com>
allisonkarlitskaya added a commit to allisonkarlitskaya/systemd that referenced this issue Dec 3, 2024
We currently pass the CopyFlags that we use to populate the temporary
directory in the form of a constant at each of the copy_tree_at() call
sites.  De-duplicate that and move it into the `CopyFilesLine` struct,
initializing it from the parser.

Add our first non-constant flag: `fsverity=`.  This can be set to `off`
(the default) or `copy`, in which case we copy the fs-verity state from
the source files.

This arrangement is amenable to the introduction of more flags to
`CopyFiles=` lines, if we want to add them in the future.

Update the `repart.d(5)` manpage.

Closes systemd#35352

Signed-off-by: Allison Karlitskaya <allison.karlitskaya@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
repart RFE 🎁 Request for Enhancement, i.e. a feature request
Development

Successfully merging a pull request may close this issue.

3 participants