composefs: Change how we do signatures
authorAlexander Larsson <alexl@redhat.com>
Thu, 8 Jun 2023 09:51:29 +0000 (11:51 +0200)
committerAlexander Larsson <alexl@redhat.com>
Sat, 10 Jun 2023 15:13:33 +0000 (17:13 +0200)
Currently we generate a signature for the actual composefs image, and
then we apply that when we enable fsverity on the composefs
image. However, there are some issues with this.

First of all, such a signed fs-verity image file can only be read if
the corresponding puiblic keyring is loaded into the fs-verity
keyring. In a typical secure setup we will have a per-commit key that
is loaded from the initrd. Additionally, the keyring is often sealed
to avoid loading more keys later.

This means you can only ever mount (or even look at) composefs images
from the current boot. While this is not a huge issue it is something
of a pain for example when debugging things.

Secondly, and more problematic, during a deploy we can't enable
fs-verity on the newly created composefs file, because and at that
point you need to pass in the signature. Unfortunately this will fail
if the matching public key is not in the keyring, which will fail for
similar reasons as the first issue.

The current workaround is to *not* enable fs-verity during deploy, but
write the signature to a file. Then the first time the particular
commit is booted we apply the signature to the iamge. This works
around issue two, but not issue one. But it causes us to do a lot of
writes and computation during the first boot as we need to write the
fs-verity merkle tree to disk. It would be much better and robust if
the merkle tree could be written during the deployment of the update
(i.e. before boot).

The new apporach is to always deploy an unsigned, but fs-verity
enabled composefs image. Then we create separate files that contain
the expected digest, and a signature of that file. On the first boot
we sign the digest file, and on further boots we can just verify
that it is signed before using it.

This fixes issue 1, since all deploys are always readable, and it
makes the workaround for issue 2 much less problematic, as we only
need to change a much smaller file on the first boot.

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.

src/libostree/ostree-repo-composefs.c
src/libostree/ostree-sysroot-deploy.c
src/switchroot/ostree-mount-util.h
src/switchroot/ostree-prepare-root.c

index 29a8bc6cb2751b809a3673daf7a2baa5b71538e7..330b0cf4958f51ece3035c75bfae3c5e3fc762e5 100644 (file)
@@ -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);
@@ -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));
index 2573fa64875cc93cb841bb5ff9871d7173464981..8920fd704de4506e27a4014d6f68757d3a13e87b 100644 (file)
@@ -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,
@@ -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);
@@ -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);
           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,
index 9f90dc01c7b72012969da1bfd5ed37124c8f82f4..683c546c25b590d512af07cc6527d82949b7638c 100644 (file)
@@ -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_ */
index 83f5897fe41f865fc652566f358f280212e5057f..162f1e983ab3dd885f8e044771e574e98acbe643 100644 (file)
@@ -96,6 +96,7 @@
 
 #ifdef HAVE_COMPOSEFS
 #include <libcomposefs/lcfs-mount.h>
+#include <libcomposefs/lcfs-writer.h>
 #endif
 
 #include "ostree-mount-util.h"
@@ -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);
+
+  /* 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)
 {
@@ -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;
@@ -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)
@@ -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