WIP: fsverity

Using fs-verity is natural for OSTree because it's file-based, as opposed to block based (like dm-verity). This only covers files - not symlinks or directories. And we clearly need to have integrity for the deployment directories at least. So making this truly secure would need a lot more work. Nevertheless, I think it's time to start experimenting with it. Among other things, it does *finally* add an API that makes files immutable, which will help against some accidental damage.
ostreedev · Oct 31, 2019 · 957f05b · 957f05b
1 parent d554cee
commit 957f05b
Show file tree

Hide file tree

Showing 7 changed files with 162 additions and 36 deletions.
diff --git a/configure.ac b/configure.ac
@@ -246,6 +246,8 @@ LIBARCHIVE_DEPENDENCY="libarchive >= 2.8.0"
 # What's in RHEL7.2.
 FUSE_DEPENDENCY="fuse >= 2.9.2"
 
+AC_CHECK_HEADERS([linux/fsverity.h])
+
 # check for gtk-doc
 m4_ifdef([GTK_DOC_CHECK], [
 GTK_DOC_CHECK([1.15], [--flavour no-tmpl])
@@ -619,6 +621,7 @@ echo "
     HTTP backend:                                 $fetcher_backend
     \"ostree trivial-httpd\":                       $enable_trivial_httpd_cmdline
     SELinux:                                      $with_selinux
+    fs-verity:                                    $ac_cv_header_linux_fsverity_h
     cryptographic checksums:                      $with_crypto
     systemd:                                      $have_libsystemd
     libmount:                                     $with_libmount

diff --git a/libglnx b/libglnx
diff --git a/src/libostree/ostree-repo-commit.c b/src/libostree/ostree-repo-commit.c
@@ -32,6 +32,10 @@
 #include <glib/gprintf.h>
 #include <sys/ioctl.h>
 #include <linux/fs.h>
+#include <ext2fs/ext2_fs.h>
+#ifdef HAVE_LINUX_FSVERITY_H
+#include <linux/fsverity.h>
+#endif
 
 #include "otutil.h"
 #include "ostree.h"
@@ -168,6 +172,65 @@ ot_security_smack_reset_fd (int fd)
 #endif
 }
 
+gboolean
+_ostree_tmpf_fsverity (OstreeRepo *self,
+                       GLnxTmpfile *tmpf,
+                       GError    **error)
+{
+  GLNX_AUTO_PREFIX_ERROR ("fsverity", error);
+
+#ifdef HAVE_LINUX_FSVERITY_H
+  if (G_UNLIKELY(self->fs_verity == _OSTREE_FEATURE_NO &&
+                 self->fsverity_required))
+    return glnx_throw (error, "fsverity required but filesystem does not support it");
+
+  /* fs-verity requires a read-only file descriptor */
+  if (!glnx_tmpfile_reopen_rdonly (tmpf, error))
+    return FALSE;
+
+  struct fsverity_enable_arg arg = { 0, };
+
+  arg.version = 1;
+  arg.hash_algorithm =  FS_VERITY_HASH_ALG_SHA256;  /* TODO configurable */
+  arg.block_size = 4096; /* FIXME query */
+  arg.salt_size = 0; /* TODO store salt in ostree repo config */
+  arg.salt_ptr = 0;
+  arg.sig_size = 0; /* TODO use signatures?  Or maybe just tell people to do this after */
+  arg.sig_ptr = 0;
+
+  if (ioctl (tmpf->fd, FS_IOC_ENABLE_VERITY, &arg) < 0)
+    {
+      switch (errno)
+        {
+          case ENOTTY:
+          case EOPNOTSUPP:
+            if (!self->fsverity_required)
+              {
+                /* The default is "opportunistic" use of fs-verity for now. */
+                g_mutex_lock (&self->txn_lock);
+                self->fs_verity = _OSTREE_FEATURE_NO;
+                g_mutex_unlock (&self->txn_lock);
+                return TRUE;
+              }
+            else
+              {
+                return glnx_throw_errno_prefix (error, "(config required) ioctl(FS_IOC_ENABLE_VERITY)");
+              }
+          default:
+            return glnx_throw_errno_prefix (error, "ioctl(FS_IOC_ENABLE_VERITY)");
+        }
+    }
+
+  g_mutex_lock (&self->txn_lock);
+  self->fs_verity = _OSTREE_FEATURE_YES;
+  g_mutex_unlock (&self->txn_lock);
+#else
+  if (G_UNLIKELY(self->fsverity_required))
+    return glnx_throw (error, "fsverity required but OSTree is compiled without fsverity support");
+#endif
+  return TRUE;
+}
+
 /* Given an O_TMPFILE regular file, link it into place. */
 gboolean
 _ostree_repo_commit_tmpf_final (OstreeRepo        *self,
@@ -185,11 +248,14 @@ _ostree_repo_commit_tmpf_final (OstreeRepo        *self,
                                             cancellable, error))
     return FALSE;
 
+
+
   if (!glnx_link_tmpfile_at (tmpf, GLNX_LINK_TMPFILE_NOREPLACE_IGNORE_EXIST,
                              dest_dfd, tmpbuf, error))
     return FALSE;
   /* We're done with the fd */
   glnx_tmpfile_clear (tmpf);
+
   return TRUE;
 }
 

diff --git a/src/libostree/ostree-repo-private.h b/src/libostree/ostree-repo-private.h
@@ -22,6 +22,7 @@
 #pragma once
 
 #include <sys/statvfs.h>
+#include "config.h"
 #include "otutil.h"
 #include "ostree-ref.h"
 #include "ostree-repo.h"
@@ -97,6 +98,12 @@ typedef struct {
   fsblkcnt_t max_blocks;
 } OstreeRepoTxn;
 
+typedef enum {
+  _OSTREE_FEATURE_NO,
+  _OSTREE_FEATURE_MAYBE,
+  _OSTREE_FEATURE_YES,
+} _OstreeFeatureSupport;
+
 /**
  * OstreeRepo:
  *
@@ -127,6 +134,7 @@ struct OstreeRepo {
   GMutex txn_lock;
   OstreeRepoTxn txn;
   gboolean txn_locked;
+  _OstreeFeatureSupport fs_verity;
 
   GMutex cache_lock;
   guint dirmeta_cache_refcount;
@@ -138,6 +146,7 @@ struct OstreeRepo {
   OstreeRepoSysrootKind sysroot_kind;
   GError *writable_error;
   gboolean in_transaction;
+  gboolean fsverity_required;
   gboolean disable_fsync;
   gboolean disable_xattrs;
   guint zlib_compression_level;
@@ -471,4 +480,9 @@ OstreeRepoAutoLock * _ostree_repo_auto_lock_push (OstreeRepo          *self,
 void          _ostree_repo_auto_lock_cleanup (OstreeRepoAutoLock *lock);
 G_DEFINE_AUTOPTR_CLEANUP_FUNC (OstreeRepoAutoLock, _ostree_repo_auto_lock_cleanup)
 
+gboolean
+_ostree_tmpf_fsverity (OstreeRepo *self,
+                       GLnxTmpfile *tmpf,
+                       GError    **error);
+
 G_END_DECLS
diff --git a/src/libostree/ostree-repo.c b/src/libostree/ostree-repo.c
@@ -31,6 +31,7 @@
 #include "libglnx.h"
 #include "otutil.h"
 #include <glnx-console.h>
+#include <linux/magic.h>
 
 #include "ostree-core-private.h"
 #include "ostree-sysroot-private.h"
@@ -47,6 +48,7 @@
 #include <glib/gstdio.h>
 #include <sys/file.h>
 #include <sys/statvfs.h>
+#include <sys/statfs.h>
 
 #define REPO_LOCK_DISABLED (-2)
 #define REPO_LOCK_BLOCKING (-1)
@@ -3033,6 +3035,10 @@ reload_core_config (OstreeRepo          *self,
       }
   }
 
+  if (!ot_keyfile_get_boolean_with_default (self->config, "fsverity", "required",
+                                            FALSE, &self->fsverity_required, error))
+    return FALSE;
+
   {
     g_clear_pointer (&self->collection_id, g_free);
     if (!ot_keyfile_get_value_with_default (self->config, "core", "collection-id",
@@ -3272,6 +3278,8 @@ ostree_repo_open (OstreeRepo    *self,
   self->device = stbuf.st_dev;
   self->inode = stbuf.st_ino;
 
+  self->fs_verity = _OSTREE_FEATURE_MAYBE;
+
   if (!glnx_opendirat (self->repo_dir_fd, "objects", TRUE,
                        &self->objects_dir_fd, error))
     return FALSE;

diff --git a/src/libostree/ostree-sysroot-deploy.c b/src/libostree/ostree-sysroot-deploy.c
@@ -41,6 +41,7 @@
 
 #include "otutil.h"
 #include "ostree.h"
+#include "ostree-repo-private.h"
 #include "ostree-sysroot-private.h"
 #include "ostree-sepolicy-private.h"
 #include "ostree-bootloader-zipl.h"
@@ -101,7 +102,8 @@ sysroot_flags_to_copy_flags (GLnxFileCopyFlags defaults,
  * hardlink if we're on the same partition.
  */
 static gboolean
-install_into_boot (OstreeSePolicy *sepolicy,
+install_into_boot (OstreeRepo *repo,
+                   OstreeSePolicy *sepolicy,
                    int         src_dfd,
                    const char *src_subpath,
                    int         dest_dfd,
@@ -110,32 +112,59 @@ install_into_boot (OstreeSePolicy *sepolicy,
                    GCancellable  *cancellable,
                    GError       **error)
 {
-  if (linkat (src_dfd, src_subpath, dest_dfd, dest_subpath, 0) != 0)
-    {
-      if (G_IN_SET (errno, EMLINK, EXDEV))
-        {
-          /* Be sure we relabel when copying the kernel, as in current
-           * e.g. Fedora it might be labeled module_object_t or usr_t,
-           * but policy may not allow other processes to read from that
-           * like kdump.
-           * See also https://github.com/fedora-selinux/selinux-policy/commit/747f4e6775d773ab74efae5aa37f3e5e7f0d4aca
-           * This means we also drop xattrs but...I doubt anyone uses
-           * non-SELinux xattrs for the kernel anyways aside from perhaps
-           * IMA but that's its own story.
-           */
-          g_auto(OstreeSepolicyFsCreatecon) fscreatecon = { 0, };
-          const char *boot_path = glnx_strjoina ("/boot/", glnx_basename (dest_subpath));
-          if (!_ostree_sepolicy_preparefscreatecon (&fscreatecon, sepolicy,
-                                                    boot_path, S_IFREG | 0644,
-                                                    error))
-            return FALSE;
-          return glnx_file_copy_at (src_dfd, src_subpath, NULL, dest_dfd, dest_subpath,
-                                    GLNX_FILE_COPY_NOXATTRS | GLNX_FILE_COPY_DATASYNC,
-                                    cancellable, error);
-        }
-      else
-        return glnx_throw_errno_prefix (error, "linkat(%s)", dest_subpath);
-    }
+  if (linkat (src_dfd, src_subpath, dest_dfd, dest_subpath, 0) == 0)
+    return TRUE; /* Note early return */
+  if (!G_IN_SET (errno, EMLINK, EXDEV))
+    return glnx_throw_errno_prefix (error, "linkat(%s)", dest_subpath);
+
+  /* Otherwise, copy */
+  struct stat src_stbuf;
+  if (!glnx_fstatat (src_dfd, src_subpath, &src_stbuf, AT_SYMLINK_NOFOLLOW, error))
+    return FALSE;
+
+  glnx_autofd int src_fd = -1;
+  if (!glnx_openat_rdonly (src_dfd, src_subpath, FALSE, &src_fd, error))
+    return FALSE;
+
+  /* Be sure we relabel when copying the kernel, as in current
+    * e.g. Fedora it might be labeled module_object_t or usr_t,
+    * but policy may not allow other processes to read from that
+    * like kdump.
+    * See also https://github.com/fedora-selinux/selinux-policy/commit/747f4e6775d773ab74efae5aa37f3e5e7f0d4aca
+    * This means we also drop xattrs but...I doubt anyone uses
+    * non-SELinux xattrs for the kernel anyways aside from perhaps
+    * IMA but that's its own story.
+    */
+  g_auto(OstreeSepolicyFsCreatecon) fscreatecon = { 0, };
+  const char *boot_path = glnx_strjoina ("/boot/", glnx_basename (dest_subpath));
+  if (!_ostree_sepolicy_preparefscreatecon (&fscreatecon, sepolicy,
+                                            boot_path, S_IFREG | 0644,
+                                            error))
+    return FALSE;
+
+  g_auto(GLnxTmpfile) tmp_dest = { 0, };
+  if (!glnx_open_tmpfile_linkable_at (dest_dfd, ".", O_WRONLY | O_CLOEXEC,
+                                      &tmp_dest, error))
+    return FALSE;
+
+  if (glnx_regfile_copy_bytes (src_fd, tmp_dest.fd, (off_t) -1) < 0)
+    return glnx_throw_errno_prefix (error, "regfile copy");
+
+  /* Kernel data should always be root-owned */
+  if (fchown (tmp_dest.fd, src_stbuf.st_uid, src_stbuf.st_gid) != 0)
+    return glnx_throw_errno_prefix (error, "fchown");
+
+  if (fchmod (tmp_dest.fd, src_stbuf.st_mode & 07777) != 0)
+    return glnx_throw_errno_prefix (error, "fchmod");
+
+  if (fdatasync (tmp_dest.fd) < 0)
+    return glnx_throw_errno_prefix (error, "fdatasync");
+
+  if (!_ostree_tmpf_fsverity (repo, &tmp_dest, error))
+    return FALSE;
+
+  if (!glnx_link_tmpfile_at (&tmp_dest, GLNX_LINK_TMPFILE_NOREPLACE, dest_dfd, dest_subpath, error))
+    return FALSE;
 
   return TRUE;
 }
@@ -1660,7 +1689,7 @@ install_deployment_kernel (OstreeSysroot   *sysroot,
     return FALSE;
   if (errno == ENOENT)
     {
-      if (!install_into_boot (sepolicy, kernel_layout->boot_dfd, kernel_layout->kernel_srcpath,
+      if (!install_into_boot (repo, sepolicy, kernel_layout->boot_dfd, kernel_layout->kernel_srcpath,
                               bootcsum_dfd, kernel_layout->kernel_namever,
                               sysroot->debug_flags,
                               cancellable, error))
@@ -1677,7 +1706,7 @@ install_deployment_kernel (OstreeSysroot   *sysroot,
         return FALSE;
       if (errno == ENOENT)
         {
-          if (!install_into_boot (sepolicy, kernel_layout->boot_dfd, kernel_layout->initramfs_srcpath,
+          if (!install_into_boot (repo, sepolicy, kernel_layout->boot_dfd, kernel_layout->initramfs_srcpath,
                                   bootcsum_dfd, kernel_layout->initramfs_namever,
                                   sysroot->debug_flags,
                                   cancellable, error))
@@ -1692,7 +1721,7 @@ install_deployment_kernel (OstreeSysroot   *sysroot,
         return FALSE;
       if (errno == ENOENT)
         {
-          if (!install_into_boot (sepolicy, kernel_layout->boot_dfd, kernel_layout->devicetree_srcpath,
+          if (!install_into_boot (repo, sepolicy, kernel_layout->boot_dfd, kernel_layout->devicetree_srcpath,
                                   bootcsum_dfd, kernel_layout->devicetree_namever,
                                   sysroot->debug_flags,
                                   cancellable, error))
@@ -1706,7 +1735,7 @@ install_deployment_kernel (OstreeSysroot   *sysroot,
         return FALSE;
       if (errno == ENOENT)
         {
-          if (!install_into_boot (sepolicy, kernel_layout->boot_dfd, kernel_layout->kernel_hmac_srcpath,
+          if (!install_into_boot (repo, sepolicy, kernel_layout->boot_dfd, kernel_layout->kernel_hmac_srcpath,
                                   bootcsum_dfd, kernel_layout->kernel_hmac_namever,
                                   sysroot->debug_flags,
                                   cancellable, error))

diff --git a/src/libotutil/ot-keyfile-utils.c b/src/libotutil/ot-keyfile-utils.c
@@ -27,6 +27,13 @@
 
 #include <string.h>
 
+static gboolean
+is_notfound (GError *error)
+{
+  return g_error_matches (error, G_KEY_FILE_ERROR, G_KEY_FILE_ERROR_KEY_NOT_FOUND)
+          || g_error_matches (error, G_KEY_FILE_ERROR, G_KEY_FILE_ERROR_GROUP_NOT_FOUND);
+}
+
 gboolean
 ot_keyfile_get_boolean_with_default (GKeyFile      *keyfile,
                                      const char    *section,
@@ -43,7 +50,7 @@ ot_keyfile_get_boolean_with_default (GKeyFile      *keyfile,
   gboolean ret_bool = g_key_file_get_boolean (keyfile, section, value, &temp_error);
   if (temp_error)
     {
-      if (g_error_matches (temp_error, G_KEY_FILE_ERROR, G_KEY_FILE_ERROR_KEY_NOT_FOUND))
+      if (is_notfound (temp_error))
         {
           g_clear_error (&temp_error);
           ret_bool = default_value;
@@ -75,7 +82,7 @@ ot_keyfile_get_value_with_default (GKeyFile      *keyfile,
   g_autofree char *ret_value = g_key_file_get_value (keyfile, section, value, &temp_error);
   if (temp_error)
     {
-      if (g_error_matches (temp_error, G_KEY_FILE_ERROR, G_KEY_FILE_ERROR_KEY_NOT_FOUND))
+      if (is_notfound (temp_error))
         {
           g_clear_error (&temp_error);
           g_assert (ret_value == NULL);
@@ -206,8 +213,7 @@ ot_keyfile_get_string_list_with_default (GKeyFile      *keyfile,
 
   if (temp_error)
     {
-      if (g_error_matches (temp_error, G_KEY_FILE_ERROR,
-                           G_KEY_FILE_ERROR_KEY_NOT_FOUND))
+      if (is_notfound (temp_error))
         {
           g_clear_error (&temp_error);
           ret_value = g_strdupv (default_value);