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

composefs: Change how we do signatures #2879

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion composefs
Submodule composefs updated 47 files
+53 −22 .github/workflows/test.yaml
+3 −1 .gitignore
+14 −67 README.md
+0 −1 composefs.cil
+21 −0 configure.ac
+1 −1 hacking/clang-format/Makefile
+4 −0 hacking/installdeps.sh
+0 −6 kernel/.gitignore
+0 −339 kernel/COPYING
+0 −159 kernel/Documentation/filesystems/composefs.rst
+0 −5 kernel/Makefile
+0 −56 kernel/cfs-internals.h
+0 −730 kernel/cfs-reader.c
+0 −85 kernel/cfs-verity.h
+0 −830 kernel/cfs.c
+0 −172 kernel/cfs.h
+0 −2 libcomposefs/Makefile-lib.am
+0 −143 libcomposefs/lcfs-cfs.h
+6 −5 libcomposefs/lcfs-fsverity.c
+4 −1 libcomposefs/lcfs-fsverity.h
+45 −1 libcomposefs/lcfs-internal.h
+10 −163 libcomposefs/lcfs-mount.c
+2 −2 libcomposefs/lcfs-mount.h
+0 −471 libcomposefs/lcfs-writer-cfs.c
+15 −13 libcomposefs/lcfs-writer-erofs.c
+64 −52 libcomposefs/lcfs-writer.c
+9 −4 libcomposefs/lcfs-writer.h
+0 −5 tests/.gitignore
+0 −18 tests/README.md
+0 −12 tests/build_image.sh
+0 −240 tests/composefs.mpp.yml
+31 −0 tests/integration.sh
+0 −13 tests/runkernel.sh
+6 −9 tools/Makefile.am
+256 −22 tools/composefs-from-json.c
+0 −354 tools/dump.c
+23 −30 tools/mkcomposefs.c
+10 −4 tools/mountcomposefs.c
+1 −1 tools/read-file.c
+0 −1 tools/test-assets/config-with-hard-link.json.gz.sha256_composefs
+1 −1 tools/test-assets/config-with-hard-link.json.gz.sha256_erofs
+0 −1 tools/test-assets/config.json.gz.sha256_composefs
+1 −1 tools/test-assets/config.json.gz.sha256_erofs
+0 −1 tools/test-assets/cs9-x86_64-developer.json.gz.sha256_composefs
+0 −1 tools/test-assets/cs9-x86_64-minimal.json.gz.sha256_composefs
+0 −1 tools/test-assets/f36-x86_64-silverblue.json.gz.sha256_composefs
+2 −1 tools/test-checksums.sh
18 changes: 17 additions & 1 deletion src/libostree/ostree-repo-composefs.c
Original file line number Diff line number Diff line change
Expand Up @@ -575,6 +575,7 @@ ostree_repo_commit_add_composefs_sig (OstreeRepo *self, GVariantBuilder *builder
g_autofree char *certfile = NULL;
g_autofree char *keyfile = NULL;
g_autoptr (GBytes) sig = NULL;
guchar digest_digest[LCFS_DIGEST_SIZE];

certfile
= g_key_file_get_string (self->config, _OSTREE_INTEGRITY_SECTION, "composefs-certfile", NULL);
Expand All @@ -590,7 +591,22 @@ ostree_repo_commit_add_composefs_sig (OstreeRepo *self, GVariantBuilder *builder
if (keyfile == NULL)
return glnx_throw (error, "Error signing compoosefs: certfile specified but keyfile is not");

if (!_ostree_fsverity_sign (certfile, keyfile, fsverity_digest, &sig, cancellable, error))
/* We sign not the fs-verity of the image file itself, but rather we sign a file containing
* the fs-verity digest. This may seem weird, but disconnecting the signature from the
* actual image itself has two major advantages:
* * We can read/mount the image (non-verified) even without the public key in
* the keyring.
* * We can apply fs-verity to the image during deploy without the public key in
* the keyring.
*
* This is important because during an update we don't have the public key loaded until
* we boot into the new initrd.
*/

if (lcfs_compute_fsverity_from_data (digest_digest, fsverity_digest, LCFS_DIGEST_SIZE) < 0)
return glnx_throw_errno (error);

if (!_ostree_fsverity_sign (certfile, keyfile, digest_digest, &sig, cancellable, error))
return FALSE;

g_variant_builder_add (builder, "{sv}", "ostree.composefs-sig", ot_gvariant_new_ay_bytes (sig));
Expand Down
34 changes: 18 additions & 16 deletions src/libostree/ostree-sysroot-deploy.c
Original file line number Diff line number Diff line change
Expand Up @@ -649,7 +649,6 @@ checkout_deployment_tree (OstreeSysroot *sysroot, OstreeRepo *repo, OstreeDeploy
#ifdef HAVE_COMPOSEFS
if (repo->composefs_wanted != OT_TRISTATE_NO)
{
gboolean apply_composefs_signature;
g_autofree guchar *fsverity_digest = NULL;
g_auto (GLnxTmpfile) tmpf = {
0,
Expand All @@ -659,11 +658,6 @@ checkout_deployment_tree (OstreeSysroot *sysroot, OstreeRepo *repo, OstreeDeploy
if (!ostree_repo_load_commit (repo, revision, &commit_variant, NULL, error))
return FALSE;

if (!ot_keyfile_get_boolean_with_default (repo->config, _OSTREE_INTEGRITY_SECTION,
"composefs-apply-sig", TRUE,
&apply_composefs_signature, error))
return FALSE;

g_autoptr (GVariant) metadata = g_variant_get_child_value (commit_variant, 0);
g_autoptr (GVariant) metadata_composefs
= g_variant_lookup_value (metadata, "ostree.composefs", G_VARIANT_TYPE_BYTESTRING);
Expand Down Expand Up @@ -698,25 +692,33 @@ checkout_deployment_tree (OstreeSysroot *sysroot, OstreeRepo *repo, OstreeDeploy
if (!glnx_fchmod (tmpf.fd, 0644, error))
return FALSE;

if (metadata_composefs_sig && apply_composefs_signature)
{
/* We can't apply the signature during deploy, because the corresponding public key for
this commit is not loaded into the keyring. So, we delay fs-verity application to the
first boot. */
if (!_ostree_tmpf_fsverity (repo, &tmpf, NULL, error))
return FALSE;

if (metadata_composefs && metadata_composefs_sig)
{
g_autofree char *composefs_digest_path
= g_strdup_printf ("%s/.ostree.cfs.digest", checkout_target_name);
alexlarsson marked this conversation as resolved.
Show resolved Hide resolved
g_autofree char *composefs_sig_path
= g_strdup_printf ("%s/.ostree.cfs.sig", checkout_target_name);
g_autoptr (GBytes) digest = g_variant_get_data_as_bytes (metadata_composefs);
g_autoptr (GBytes) sig = g_variant_get_data_as_bytes (metadata_composefs_sig);

if (!glnx_file_replace_contents_at (osdeploy_dfd, composefs_digest_path,
g_bytes_get_data (digest, NULL),
g_bytes_get_size (digest), 0, cancellable, error))
return FALSE;

if (!glnx_file_replace_contents_at (osdeploy_dfd, composefs_sig_path,
g_bytes_get_data (sig, NULL), g_bytes_get_size (sig),
0, cancellable, error))
return FALSE;
}
else
{
if (!_ostree_tmpf_fsverity (repo, &tmpf, NULL, error))
return FALSE;

/* The signature should be applied as a fs-verity signature to the digest file. However
* we can't do that until boot, because we can't guarantee that the public key is
* loaded into the keyring until we boot the new initrd. So the signature is applied
* in ostree-prepare-root on first boot.
*/
}

if (!glnx_link_tmpfile_at (&tmpf, GLNX_LINK_TMPFILE_REPLACE, osdeploy_dfd, composefs_cfs_path,
Expand Down
15 changes: 15 additions & 0 deletions src/switchroot/ostree-mount-util.h
Original file line number Diff line number Diff line change
Expand Up @@ -190,4 +190,19 @@ fsverity_sign (int fd, unsigned char *signature, size_t signature_len)
#endif
}

static inline void
bin2hex (char *out_buf, const unsigned char *inbuf, size_t len)
{
static const char hexchars[] = "0123456789abcdef";
unsigned int i, j;

for (i = 0, j = 0; i < len; i++, j += 2)
{
unsigned char byte = inbuf[i];
out_buf[j] = hexchars[byte >> 4];
out_buf[j + 1] = hexchars[byte & 0xF];
}
out_buf[j] = '\0';
}

#endif /* __OSTREE_MOUNT_UTIL_H_ */
213 changes: 154 additions & 59 deletions src/switchroot/ostree-prepare-root.c
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@

#ifdef HAVE_COMPOSEFS
#include <libcomposefs/lcfs-mount.h>
#include <libcomposefs/lcfs-writer.h>
#endif

#include "ostree-mount-util.h"
Expand Down Expand Up @@ -183,6 +184,130 @@ resolve_deploy_path (const char *root_mountpoint)
return deploy_path;
}

#ifdef HAVE_COMPOSEFS
static void
apply_digest_signature (const char *digestfile, const char *sigfile)
{
unsigned char *signature;
size_t signature_len;
int digest_is_readonly;
int digest_fd;

signature = read_file (sigfile, &signature_len);
if (signature == NULL)
err (EXIT_FAILURE, "Missing signaure file %s", sigfile);

/* If we're read-only we temporarily make read-write bind mount to sign */
digest_is_readonly = path_is_on_readonly_fs (digestfile);
if (digest_is_readonly)
{
if (mount (digestfile, digestfile, NULL, MS_BIND | MS_SILENT, NULL) < 0)
err (EXIT_FAILURE, "failed to bind mount %s (for signing)", digestfile);
if (mount (digestfile, digestfile, NULL, MS_REMOUNT | MS_SILENT, NULL) < 0)
err (EXIT_FAILURE, "failed to remount %s read-write (for signing)", digestfile);
}

/* Ensure we re-open after any bindmounts */
digest_fd = open (digestfile, O_RDONLY | O_CLOEXEC);
if (digest_fd < 0)
err (EXIT_FAILURE, "failed to open %s", digestfile);

fsverity_sign (digest_fd, signature, signature_len);

close (digest_fd);

if (digest_is_readonly && umount2 (digestfile, MNT_DETACH) < 0)
err (EXIT_FAILURE, "failed to unmount %s (after signing)", digestfile);

free (signature);

#ifdef USE_LIBSYSTEMD
sd_journal_send ("MESSAGE=Applied fsverity signature %s to %s", sigfile, digestfile, NULL);
#endif
}

static void
ensure_digest_fd_is_signed (int digest_fd)
{
struct fsverity_read_metadata_arg read_metadata = { 0 };
char sig_data[1];
int res;

/* We verify there is a signature by reading one byte of it. */

read_metadata.metadata_type = FS_VERITY_METADATA_TYPE_SIGNATURE;
read_metadata.offset = 0;
read_metadata.length = sizeof (sig_data);
read_metadata.buf_ptr = (size_t)&sig_data;

res = ioctl (digest_fd, FS_IOC_READ_VERITY_METADATA, &read_metadata);
if (res == -1)
{
if (errno == ENODATA)
err (EXIT_FAILURE, "Digest file is unexpectedly not signed");
else
err (EXIT_FAILURE, "Failed to get signature from digest file");
}
}

static char *
read_signed_digest (const char *digestfile, const char *sigfile)
{
unsigned fd_flags;
int digest_fd;
unsigned char buf[LCFS_DIGEST_SIZE];
char *digest;
ssize_t bytes_read;

digest_fd = open (digestfile, O_RDONLY | O_CLOEXEC);
if (digest_fd < 0)
err (EXIT_FAILURE, "failed to open %s", digestfile);

/* Check if file is already fsverity */
if (ioctl (digest_fd, FS_IOC_GETFLAGS, &fd_flags) < 0)
err (EXIT_FAILURE, "failed to get fd flags for %s", digestfile);

/* If it is not, apply signature */
if ((fd_flags & FS_VERITY_FL) == 0)
{
close (digest_fd);

apply_digest_signature (digestfile, sigfile);

/* Reopen */
digest_fd = open (digestfile, O_RDONLY | O_CLOEXEC);
if (digest_fd < 0)
err (EXIT_FAILURE, "failed to reopen %s", digestfile);
}

/* By now we know its fs-verify enabled, also ensure it is signed
* with a key in the keyring */
ensure_digest_fd_is_signed (digest_fd);

Choose a reason for hiding this comment

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

Why aren't you just storing the signature in the contents of the digestfile and verifying it in userspace here?

Unfortunately I wasn't able to get in front of the train wreck that is CONFIG_FS_VERITY_BUILTIN_SIGNATURES fast enough, but in summary it should not be used, as it is not a good way to do signatures.

Copy link
Member

Choose a reason for hiding this comment

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

That's at the end of the commit message, right?

Long term I would like to avoid the first-boot writing totally, and
I've been chatting with David Howells (kernel keyring maintainer) and
he proposed adding a new keyring syscall that verifies a PKCS#7
signature from userspace directly. This would be exactly what
fs-verity does, except we wouldn't have to write the digest to disk
during boot, we would just read the digest file and the signature file
each boot and ask the kernel to verify it.

Or are you suggesting that it's simpler to just do full userspace verification here and not use the kernel keyring?

Choose a reason for hiding this comment

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

Yes, just doing full userspace verification would be much simpler.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for looking at this by the way! Conceptually I think this is a composefs-level issue and not an ostree issue so I've moved this to containers/composefs#151


/* Load the expected digest */
do
bytes_read = read (digest_fd, buf, LCFS_DIGEST_SIZE);
while (bytes_read == -1 && errno == EINTR);
if (bytes_read == -1)
err (EXIT_FAILURE, "Failed to read digest file");

if (bytes_read != LCFS_DIGEST_SIZE)
err (EXIT_FAILURE, "Digest file has wrong size");

digest = malloc (LCFS_DIGEST_SIZE * 2 + 1);
if (digest == NULL)
err (EXIT_FAILURE, "Out of memory");

bin2hex (digest, buf, LCFS_DIGEST_SIZE);

#ifdef USE_LIBSYSTEMD
sd_journal_send ("MESSAGE=Signed digest file found for root", NULL);
#endif

return digest;
}
#endif

static int
pivot_root (const char *new_root, const char *put_old)
{
Expand Down Expand Up @@ -315,57 +440,12 @@ main (int argc, char *argv[])
objdirs,
1,
};
int cfs_fd;
unsigned cfs_flags;

cfs_fd = open (OSTREE_COMPOSEFS_NAME, O_RDONLY | O_CLOEXEC);
if (cfs_fd < 0)
{
if (errno == ENOENT)
goto nocfs;

err (EXIT_FAILURE, "failed to open %s", OSTREE_COMPOSEFS_NAME);
}

/* Check if file is already fsverity */
if (ioctl (cfs_fd, FS_IOC_GETFLAGS, &cfs_flags) < 0)
err (EXIT_FAILURE, "failed to get %s flags", OSTREE_COMPOSEFS_NAME);

/* It is not, apply signature (if it exists) */
if ((cfs_flags & FS_VERITY_FL) == 0)
if (composefs_mode == OSTREE_COMPOSEFS_MODE_SIGNED)
{
const char signame[] = OSTREE_COMPOSEFS_NAME ".sig";
unsigned char *signature;
size_t signature_len;

signature = read_file (signame, &signature_len);
if (signature != NULL)
{
/* If we're read-only we temporarily make it read-write to sign the image */
if (!sysroot_currently_writable
&& mount (root_mountpoint, root_mountpoint, NULL, MS_REMOUNT | MS_SILENT, NULL)
< 0)
err (EXIT_FAILURE, "failed to remount rootfs writable (for signing)");

fsverity_sign (cfs_fd, signature, signature_len);
free (signature);

if (!sysroot_currently_writable
&& mount (root_mountpoint, root_mountpoint, NULL,
MS_REMOUNT | MS_RDONLY | MS_SILENT, NULL)
< 0)
err (EXIT_FAILURE, "failed to remount rootfs back read-only (after signing)");

#ifdef USE_LIBSYSTEMD
sd_journal_send ("MESSAGE=Applied fsverity signature %s", signame, NULL);
#endif
}
else
{
#ifdef USE_LIBSYSTEMD
sd_journal_send ("MESSAGE=No fsverity signature found for root", NULL);
#endif
}
composefs_digest
= read_signed_digest (OSTREE_COMPOSEFS_NAME ".digest", OSTREE_COMPOSEFS_NAME ".sig");
composefs_mode = OSTREE_COMPOSEFS_MODE_DIGEST;
}

cfs_options.flags = LCFS_MOUNT_FLAGS_READONLY;
Expand All @@ -374,17 +454,23 @@ main (int argc, char *argv[])
err (EXIT_FAILURE, "failed to assemble /boot/loader path");
cfs_options.image_mountdir = srcpath;

if (composefs_mode == OSTREE_COMPOSEFS_MODE_SIGNED)
{
cfs_options.flags |= LCFS_MOUNT_FLAGS_REQUIRE_SIGNATURE | LCFS_MOUNT_FLAGS_REQUIRE_VERITY;
}
else if (composefs_mode == OSTREE_COMPOSEFS_MODE_DIGEST)
if (composefs_mode == OSTREE_COMPOSEFS_MODE_DIGEST)
{
cfs_options.flags |= LCFS_MOUNT_FLAGS_REQUIRE_VERITY;
cfs_options.expected_digest = composefs_digest;
}

if (lcfs_mount_fd (cfs_fd, TMP_SYSROOT, &cfs_options) == 0)
#ifdef USE_LIBSYSTEMD
if (composefs_mode == OSTREE_COMPOSEFS_MODE_MAYBE)
sd_journal_send ("MESSAGE=Trying to mount composefs rootfs", NULL);
else if (composefs_mode == OSTREE_COMPOSEFS_MODE_DIGEST)
sd_journal_send ("MESSAGE=Mounting composefs rootfs with expected digest '%s'",
composefs_digest, NULL);
else
sd_journal_send ("MESSAGE=Mounting composefs rootfs", NULL);
#endif

if (lcfs_mount_image (OSTREE_COMPOSEFS_NAME, TMP_SYSROOT, &cfs_options) == 0)
{
int fd = open (_OSTREE_COMPOSEFS_ROOT_STAMP, O_WRONLY | O_CREAT | O_CLOEXEC, 0644);
if (fd < 0)
Expand All @@ -393,10 +479,19 @@ main (int argc, char *argv[])

using_composefs = 1;
}

close (cfs_fd);

nocfs:
else
{
#ifdef USE_LIBSYSTEMD
if (errno == ENOVERITY)
sd_journal_send ("MESSAGE=No verity in composefs image", NULL);
else if (errno == EWRONGVERITY)
sd_journal_send ("MESSAGE=Wrong verity digest in composefs image", NULL);
else if (errno == ENOSIGNATURE)
sd_journal_send ("MESSAGE=Missing signature in composefs image", NULL);
else
sd_journal_send ("MESSAGE=Mounting composefs image failed: %s", strerror (errno), NULL);
#endif
}
#else
err (EXIT_FAILURE, "Composefs not supported");
#endif
Expand Down