From 957f05b18823bfcabbdd88180e995c45d5640817 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Fri, 25 Oct 2019 22:07:44 +0000 Subject: [PATCH] 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. --- configure.ac | 3 + libglnx | 2 +- src/libostree/ostree-repo-commit.c | 66 +++++++++++++++++++ src/libostree/ostree-repo-private.h | 14 +++++ src/libostree/ostree-repo.c | 8 +++ src/libostree/ostree-sysroot-deploy.c | 91 ++++++++++++++++++--------- src/libotutil/ot-keyfile-utils.c | 14 +++-- 7 files changed, 162 insertions(+), 36 deletions(-) diff --git a/configure.ac b/configure.ac index 2f9579f5a0..28f1475ae8 100644 --- 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 index b1cb19b6b2..003663f578 160000 --- a/libglnx +++ b/libglnx @@ -1 +1 @@ -Subproject commit b1cb19b6b2d712b492e6376248f3010d18e59daa +Subproject commit 003663f5782adc47d79bee5de4c7d70e9ceb6813 diff --git a/src/libostree/ostree-repo-commit.c b/src/libostree/ostree-repo-commit.c index 8c5d94110d..b42bb47ea1 100644 --- a/src/libostree/ostree-repo-commit.c +++ b/src/libostree/ostree-repo-commit.c @@ -32,6 +32,10 @@ #include #include #include +#include +#ifdef HAVE_LINUX_FSVERITY_H +#include +#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 index b57ad799f7..ed4412d048 100644 --- a/src/libostree/ostree-repo-private.h +++ b/src/libostree/ostree-repo-private.h @@ -22,6 +22,7 @@ #pragma once #include +#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 index cff70d474e..64dda0cba9 100644 --- a/src/libostree/ostree-repo.c +++ b/src/libostree/ostree-repo.c @@ -31,6 +31,7 @@ #include "libglnx.h" #include "otutil.h" #include +#include #include "ostree-core-private.h" #include "ostree-sysroot-private.h" @@ -47,6 +48,7 @@ #include #include #include +#include #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 index a09c354b93..4e2c91bc8a 100644 --- 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 index 2050e969f5..3e028b203d 100644 --- a/src/libotutil/ot-keyfile-utils.c +++ b/src/libotutil/ot-keyfile-utils.c @@ -27,6 +27,13 @@ #include +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);