From fc0bfafe8a49f70755e7dcb05d7c7d70f0860049 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Sun, 10 Jan 2021 14:27:49 +0000 Subject: [PATCH] WIP: Use fsverity library, require keys and add signatures too At the time we added fsverity code to ostree, fsverity was just a CLI tool; since then it has gained a C shared library which wraps all the signature bits and the enablement `ioctl()` conveniently. This makes it much easier for us to support signatures, so do so. Note that at this time, because ostree doesn't define a mechanism to transport fsverity signatures across repositories, this is mostly only useful for locally-generated signatures. The idea however is this is a starting point from which we can build more support, including signature transport, remote keys, etc. In order to simplify things, drop support for "opportunistic" use of fsverity. In practice we expect people using this to set it up fully, or not at all. The "read only files" aspect *is* useful, but complicated the code too much relative to its benefit. Also drop support for using fsverity for `/boot` for now; in practice most things there are read by the bootloader, which doesn't know about fsverity. Instead those should be covered by e.g. Secure Boot. This ensures that we only have one high level API `_ostree_tmpf_fsverity()` that is invoked from the core commit path. xref https://lwn.net/Articles/842002 xref https://github.com/ostreedev/ostree/pull/1959 xref https://github.com/coreos/rpm-ostree/issues/1883 --- Makefile-libostree.am | 6 +- configure.ac | 27 +++- src/libostree/ostree-repo-private.h | 26 ++-- src/libostree/ostree-repo-verity.c | 176 +++++++++----------------- src/libostree/ostree-repo.c | 20 ++- src/libostree/ostree-sysroot-deploy.c | 11 -- tests/inst/Cargo.toml | 1 + tests/inst/src/fsverity.rs | 61 +++++++++ tests/inst/src/insttestmain.rs | 1 + tests/inst/src/test.rs | 18 +++ 10 files changed, 202 insertions(+), 145 deletions(-) create mode 100644 tests/inst/src/fsverity.rs diff --git a/Makefile-libostree.am b/Makefile-libostree.am index 7f9d7e671b..37318e7ff8 100644 --- a/Makefile-libostree.am +++ b/Makefile-libostree.am @@ -101,7 +101,6 @@ libostree_1_la_SOURCES = \ src/libostree/ostree-repo-libarchive.c \ src/libostree/ostree-repo-prune.c \ src/libostree/ostree-repo-refs.c \ - src/libostree/ostree-repo-verity.c \ src/libostree/ostree-repo-traverse.c \ src/libostree/ostree-repo-private.h \ src/libostree/ostree-repo-file.c \ @@ -163,6 +162,9 @@ else # if ENABLE_EXPERIMENTAL_API libostree_1_la_SOURCES += \ $(NULL) endif +if BUILDOPT_FSVERITY +libostree_1_la_SOURCES += src/libostree/ostree-repo-verity.c +endif if USE_AVAHI libostree_1_la_SOURCES += \ @@ -203,7 +205,7 @@ libostree_1_la_CFLAGS = $(AM_CFLAGS) -I$(srcdir)/bsdiff -I$(srcdir)/libglnx -I$( -fvisibility=hidden '-D_OSTREE_PUBLIC=__attribute__((visibility("default"))) extern' libostree_1_la_LDFLAGS = -version-number 1:0:0 -Bsymbolic-functions $(addprefix $(wl_versionscript_arg),$(symbol_files)) libostree_1_la_LIBADD = libotutil.la libglnx.la libbsdiff.la $(OT_INTERNAL_GIO_UNIX_LIBS) $(OT_INTERNAL_GPGME_LIBS) \ - $(OT_DEP_LZMA_LIBS) $(OT_DEP_ZLIB_LIBS) $(OT_DEP_CRYPTO_LIBS) + $(OT_DEP_LZMA_LIBS) $(OT_DEP_ZLIB_LIBS) $(OT_DEP_CRYPTO_LIBS) $(FSVERITY_LIBS) # Some change between rust-1.21.0-1.fc27 and rust-1.22.1-1.fc27.x86_64 if ENABLE_RUST libostree_1_la_LIBADD += -ldl diff --git a/configure.ac b/configure.ac index 8ffdf64b77..1a6ea9383f 100644 --- a/configure.ac +++ b/configure.ac @@ -252,14 +252,31 @@ AS_IF([test x$with_ed25519_libsodium != xno], [ ], with_ed25519_libsodium=no ) AM_CONDITIONAL(USE_LIBSODIUM, test "x$have_libsodium" = xyes) +AC_ARG_WITH(fsverity, + AS_HELP_STRING([--with-fsverity], [Support fsverity @<:@default=maybe@:>@]), + [], [with_fsverity=maybe]) +AS_IF([test x$with_fsverity != xno], [ + AC_CHECK_HEADERS([libfsverity.h]) + AM_CONDITIONAL(BUILDOPT_FSVERITY, [test x$ac_cv_header_libfsverity_h = xyes]) + AM_COND_IF([BUILDOPT_FSVERITY], + [ + AC_DEFINE([BUILDOPT_FSVERITY], 1, [Define if using fsverity]) + OSTREE_FEATURES="$OSTREE_FEATURES ex-fsverity" + buildopt_fsverity=yes + FSVERITY_LIBS="-lfsverity" + AC_SUBST(FSVERITY_LIBS) + ], + [ + AS_IF([test x$with_fsverity != xyes], [AC_MSG_ERROR([libfsverity requested but not found])]) + buildopt_fsverity=no + ] + ) +]) + LIBARCHIVE_DEPENDENCY="libarchive >= 2.8.0" # What's in RHEL7.2. FUSE_DEPENDENCY="fuse >= 2.9.2" -AC_CHECK_HEADERS([linux/fsverity.h]) -AS_IF([test x$ac_cv_header_linux_fsverity_h = xyes ], - [OSTREE_FEATURES="$OSTREE_FEATURES ex-fsverity"]) - # check for gtk-doc m4_ifdef([GTK_DOC_CHECK], [ GTK_DOC_CHECK([1.15], [--flavour no-tmpl]) @@ -632,7 +649,7 @@ echo " HTTP backend: $fetcher_backend \"ostree trivial-httpd\": $enable_trivial_httpd_cmdline SELinux: $with_selinux - fs-verity: $ac_cv_header_linux_fsverity_h + fs-verity: $buildopt_fsverity cryptographic checksums: $with_crypto systemd: $with_libsystemd libmount: $with_libmount diff --git a/src/libostree/ostree-repo-private.h b/src/libostree/ostree-repo-private.h index 6c01bc6bc4..4e0c46bdcc 100644 --- a/src/libostree/ostree-repo-private.h +++ b/src/libostree/ostree-repo-private.h @@ -162,8 +162,11 @@ struct OstreeRepo { GMutex txn_lock; OstreeRepoTxn txn; gboolean txn_locked; - _OstreeFeatureSupport fs_verity_wanted; - _OstreeFeatureSupport fs_verity_supported; + gboolean fs_verity_wanted; +#ifdef BUILDOPT_FSVERITY + char *fsverity_cert; + char *fsverity_key; +#endif GMutex cache_lock; guint dirmeta_cache_refcount; @@ -521,18 +524,21 @@ 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_repo_parse_fsverity_config (OstreeRepo *self, GError **error); - -gboolean -_ostree_tmpf_fsverity_core (GLnxTmpfile *tmpf, - _OstreeFeatureSupport fsverity_requested, - gboolean *supported, - GError **error); - +#define _OSTREE_FSVERITY_CONFIG_KEY "ex-fsverity" +#ifdef BUILDOPT_FSVERITY +gboolean +_ostree_repo_parse_fsverity_config (OstreeRepo *self, GError **error); gboolean _ostree_tmpf_fsverity (OstreeRepo *self, GLnxTmpfile *tmpf, GError **error); +#else +static inline gboolean +_ostree_repo_parse_fsverity_config (OstreeRepo *self, GError **error) { return TRUE;} +static inline gboolean +_ostree_tmpf_fsverity (OstreeRepo *self, GLnxTmpfile *tmpf, GError **error) { return TRUE; } +#endif + gboolean _ostree_repo_verify_bindings (const char *collection_id, diff --git a/src/libostree/ostree-repo-verity.c b/src/libostree/ostree-repo-verity.c index 92b026eeb4..ae4332d737 100644 --- a/src/libostree/ostree-repo-verity.c +++ b/src/libostree/ostree-repo-verity.c @@ -27,94 +27,46 @@ #include "ostree-repo-private.h" #include "otutil.h" #include "ot-fs-utils.h" -#ifdef HAVE_LINUX_FSVERITY_H -#include -#endif +#include gboolean _ostree_repo_parse_fsverity_config (OstreeRepo *self, GError **error) { - /* Currently experimental */ - static const char fsverity_key[] = "ex-fsverity"; - self->fs_verity_wanted = _OSTREE_FEATURE_NO; -#ifdef HAVE_LINUX_FSVERITY_H - self->fs_verity_supported = _OSTREE_FEATURE_MAYBE; -#else - self->fs_verity_supported = _OSTREE_FEATURE_NO; -#endif - gboolean fsverity_required = FALSE; - if (!ot_keyfile_get_boolean_with_default (self->config, fsverity_key, "required", - FALSE, &fsverity_required, error)) + struct stat stbuf; + self->fsverity_key = g_key_file_get_string (self->config, _OSTREE_FSVERITY_CONFIG_KEY, "key", error); + if (!self->fsverity_key) return FALSE; - if (fsverity_required) - { - self->fs_verity_wanted = _OSTREE_FEATURE_YES; - if (self->fs_verity_supported == _OSTREE_FEATURE_NO) - return glnx_throw (error, "fsverity required, but libostree compiled without support"); - } - else - { - gboolean fsverity_opportunistic = FALSE; - if (!ot_keyfile_get_boolean_with_default (self->config, fsverity_key, "opportunistic", - FALSE, &fsverity_opportunistic, error)) - return FALSE; - if (fsverity_opportunistic) - self->fs_verity_wanted = _OSTREE_FEATURE_MAYBE; - } - + if (!glnx_fstatat (self->repo_dir_fd, self->fsverity_cert, &stbuf, 0, error)) + return glnx_prefix_error (error, "Couldn't access fsverity key"); + /* Enforce not-world-readable for the same reason as ssh */ + if (stbuf.st_mode & S_IROTH) + return glnx_throw (error, "fsverity key must not be world-readable"); + + self->fsverity_cert = g_key_file_get_string (self->config, _OSTREE_FSVERITY_CONFIG_KEY, "cert", NULL); + if (!self->fsverity_cert) + self->fsverity_cert = g_strdup (self->fsverity_key); + else if (!glnx_fstatat (self->repo_dir_fd, self->fsverity_cert, &stbuf, 0, error)) + return glnx_prefix_error (error, "Couldn't access fsverity certificate"); return TRUE; } - -/* Wrapper around the fsverity ioctl, compressing the result to - * "success, unsupported or error". This is used for /boot where - * we enable verity if supported. - * */ -gboolean -_ostree_tmpf_fsverity_core (GLnxTmpfile *tmpf, - _OstreeFeatureSupport fsverity_requested, - gboolean *supported, - GError **error) +static int +ot_fsverity_read_callback (void *file, void *bufp, size_t count) { - /* Set this by default to simplify the code below */ - if (supported) - *supported = FALSE; - - if (fsverity_requested == _OSTREE_FEATURE_NO) - return TRUE; - -#ifdef HAVE_LINUX_FSVERITY_H - GLNX_AUTO_PREFIX_ERROR ("fsverity", error); - - /* 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; /* We don't currently expect use of in-kernel signature verification */ - arg.sig_ptr = 0; - - if (ioctl (tmpf->fd, FS_IOC_ENABLE_VERITY, &arg) < 0) + errno = 0; + int fd = GPOINTER_TO_INT (file); + guint8* buf = bufp; + while (count > 0) { - switch (errno) - { - case ENOTTY: - case EOPNOTSUPP: - return TRUE; - default: - return glnx_throw_errno_prefix (error, "ioctl(FS_IOC_ENABLE_VERITY)"); - } + ssize_t n = read (fd, buf, MIN (count, INT_MAX)); + if (n < 0) + return -errno; + if (n == 0) + return -EIO; + buf += n; + count -= n; } - - if (supported) - *supported = TRUE; -#endif - return TRUE; + return 0; } /* Enable verity on a file, respecting the "wanted" and "supported" states. @@ -127,48 +79,42 @@ _ostree_tmpf_fsverity (OstreeRepo *self, GLnxTmpfile *tmpf, GError **error) { -#ifdef HAVE_LINUX_FSVERITY_H - g_mutex_lock (&self->txn_lock); - _OstreeFeatureSupport fsverity_wanted = self->fs_verity_wanted; - _OstreeFeatureSupport fsverity_supported = self->fs_verity_supported; - g_mutex_unlock (&self->txn_lock); + GLNX_AUTO_PREFIX_ERROR ("fsverity", error); - switch (fsverity_wanted) - { - case _OSTREE_FEATURE_YES: - { - if (fsverity_supported == _OSTREE_FEATURE_NO) - return glnx_throw (error, "fsverity required but filesystem does not support it"); - } - break; - case _OSTREE_FEATURE_MAYBE: - break; - case _OSTREE_FEATURE_NO: - return TRUE; - } + if (!self->fs_verity_wanted) + return TRUE; - gboolean supported = FALSE; - if (!_ostree_tmpf_fsverity_core (tmpf, fsverity_wanted, &supported, error)) + struct libfsverity_signature_params sig_params = { + .keyfile = self->fsverity_key, + .certfile = self->fsverity_cert, + }; + + /* fs-verity requires a read-only file descriptor */ + if (!glnx_tmpfile_reopen_rdonly (tmpf, error)) return FALSE; - if (!supported) - { - if (G_UNLIKELY (fsverity_wanted == _OSTREE_FEATURE_YES)) - return glnx_throw (error, "fsverity required but filesystem does not support it"); + struct stat stbuf; + if (!glnx_fstat (tmpf->fd, &stbuf, error)) + return FALSE; + + struct libfsverity_merkle_tree_params tree_params = { + .version = 1, + .hash_algorithm = FS_VERITY_HASH_ALG_SHA256, + .file_size = stbuf.st_size, + .block_size = 4096, + /* TODO: salt? */ + }; + g_autofree struct libfsverity_digest *digest = NULL; + if (libfsverity_compute_digest (GINT_TO_POINTER (tmpf->fd), ot_fsverity_read_callback, &tree_params, &digest) < 0) + return glnx_throw (error, "failed to compute fsverity digest"); + + guint8 *sig = NULL; + size_t sig_size; + if (libfsverity_sign_digest (digest, &sig_params, &sig, &sig_size) < 0) + return glnx_throw (error, "failed to generate fsverity signature"); + + if (libfsverity_enable_with_sig (tmpf->fd, &tree_params, sig, sig_size) < 0) + return glnx_throw (error, "failed to enable fsverity for file"); - /* If we got here, we must be trying "opportunistic" use of fs-verity */ - g_assert_cmpint (fsverity_wanted, ==, _OSTREE_FEATURE_MAYBE); - g_mutex_lock (&self->txn_lock); - self->fs_verity_supported = _OSTREE_FEATURE_NO; - g_mutex_unlock (&self->txn_lock); - return TRUE; - } - - g_mutex_lock (&self->txn_lock); - self->fs_verity_supported = _OSTREE_FEATURE_YES; - g_mutex_unlock (&self->txn_lock); -#else - g_assert_cmpint (self->fs_verity_wanted, !=, _OSTREE_FEATURE_YES); -#endif return TRUE; } diff --git a/src/libostree/ostree-repo.c b/src/libostree/ostree-repo.c index 35b1a2b0c9..100c7568de 100644 --- a/src/libostree/ostree-repo.c +++ b/src/libostree/ostree-repo.c @@ -1049,6 +1049,11 @@ ostree_repo_finalize (GObject *object) g_free (self->collection_id); g_strfreev (self->repo_finders); +#ifdef BUILDOPT_FSVERITY + g_free (self->fsverity_cert); + g_free (self->fsverity_key); +#endif + g_clear_pointer (&self->remotes, g_hash_table_destroy); g_mutex_clear (&self->remotes_lock); @@ -3017,9 +3022,20 @@ reload_core_config (OstreeRepo *self, } } - if (!_ostree_repo_parse_fsverity_config (self, error)) + /* Currently experimental */ + if (!ot_keyfile_get_boolean_with_default (self->config, _OSTREE_FSVERITY_CONFIG_KEY, "required", + FALSE, &self->fs_verity_wanted, error)) return FALSE; - + if (self->fs_verity_wanted) + { + #ifndef BUILDOPT_FSVERITY + return glnx_throw (error, "fsverity required, but libostree compiled without support"); + #else + if (!_ostree_repo_parse_fsverity_config (self, error)) + return FALSE; + #endif + } + { g_clear_pointer (&self->collection_id, g_free); if (!ot_keyfile_get_value_with_default (self->config, "core", "collection-id", diff --git a/src/libostree/ostree-sysroot-deploy.c b/src/libostree/ostree-sysroot-deploy.c index 900efe4997..98a98111ff 100644 --- a/src/libostree/ostree-sysroot-deploy.c +++ b/src/libostree/ostree-sysroot-deploy.c @@ -161,17 +161,6 @@ install_into_boot (OstreeRepo *repo, if (fdatasync (tmp_dest.fd) < 0) return glnx_throw_errno_prefix (error, "fdatasync"); - /* Today we don't have a config flag to *require* verity on /boot, - * and at least for Fedora CoreOS we're not likely to do fsverity on - * /boot soon due to wanting to support mounting it from old Linux - * kernels. So change "required" to "maybe". - */ - _OstreeFeatureSupport boot_verity = _OSTREE_FEATURE_NO; - if (repo->fs_verity_wanted != _OSTREE_FEATURE_NO) - boot_verity = _OSTREE_FEATURE_MAYBE; - if (!_ostree_tmpf_fsverity_core (&tmp_dest, boot_verity, NULL, error)) - return FALSE; - if (!glnx_link_tmpfile_at (&tmp_dest, GLNX_LINK_TMPFILE_NOREPLACE, dest_dfd, dest_subpath, error)) return FALSE; diff --git a/tests/inst/Cargo.toml b/tests/inst/Cargo.toml index 31b43b4e70..53fbf99232 100644 --- a/tests/inst/Cargo.toml +++ b/tests/inst/Cargo.toml @@ -14,6 +14,7 @@ structopt = "0.3" serde = "1.0.111" serde_derive = "1.0.111" serde_json = "1.0" +serde_yaml = "0.8.14" sh-inline = "0.1.0" anyhow = "1.0" tempfile = "3.1.0" diff --git a/tests/inst/src/fsverity.rs b/tests/inst/src/fsverity.rs new file mode 100644 index 0000000000..2e1f268a67 --- /dev/null +++ b/tests/inst/src/fsverity.rs @@ -0,0 +1,61 @@ +//! Test fsverity + +use anyhow::Result; +use sh_inline::bash; +use std::os::unix::io::AsRawFd; + +use crate::test::*; + +#[itest] +fn test_fsverity() -> Result<()> { + if !check_ostree_feature("ex-fsverity")? { + return Ok(()); + } + + let mb = 1_000_000; + let disksize = 10 * mb; + // Create tempdir and sparse disk file in it + let td = tempfile::tempdir_in("/var/tmp")?; + let tmp_disk = tempfile::NamedTempFile::new_in(td.path())?; + nix::unistd::ftruncate(tmp_disk.as_file().as_raw_fd(), disksize)?; + let mnt = td.path().join("mnt"); + let mnt = mnt.as_path(); + std::fs::create_dir(&mnt)?; + // Create filesystem on it, loopback mount and create a repo + let repopath = td.path().join("repo"); + let repopath = repopath.as_path(); + bash!( + "mkfs.ext4 -O verity {tmp_disk} + mount -o loop {tmp_disk} {mnt} + echo foo > {mnt}/foo + echo bar > {mnt}/bar + ", + tmp_disk = tmp_disk.path(), + mnt = mnt + )?; + let cert_path = td.path().join("cert.der"); + let cert_path = cert_path.as_path(); + let key_path = td.path().join("key.pem"); + let key_path = key_path.as_path(); + bash!( + "openssl req -newkey rsa:4096 -nodes -keyout {key_path} -x509 -out cert.pem + openssl x509 -in cert.pem -out {cert_path} -outform der + chmod 0600 {key_path} + ", + key_path = key_path, + cert_path = cert_path + )?; + bash!( + "ostree --repo={repopath} init --mode=bare + ostree --repo={repopath} config set ex-fsverity.required=true + ostree --repo={repopath} config set ex-fsverity.key={key_path} + ostree --repo={repopath} config set ex-fsverity.cert={cert_path} + ostree --repo={repopath} commit -b testverity --tree=dir={mnt} + ", + repopath = repopath, + mnt = mnt, + key_path = key_path, + cert_path = cert_path + )?; + Ok(()) +} diff --git a/tests/inst/src/insttestmain.rs b/tests/inst/src/insttestmain.rs index 3fdc1be1a8..d35538aa93 100644 --- a/tests/inst/src/insttestmain.rs +++ b/tests/inst/src/insttestmain.rs @@ -2,6 +2,7 @@ use anyhow::{bail, Result}; use structopt::StructOpt; mod destructive; +mod fsverity; mod repobin; mod rpmostree; mod sysroot; diff --git a/tests/inst/src/test.rs b/tests/inst/src/test.rs index 9d8e156c47..044761a90f 100644 --- a/tests/inst/src/test.rs +++ b/tests/inst/src/test.rs @@ -8,6 +8,7 @@ use std::time; use anyhow::{bail, Context, Result}; use linkme::distributed_slice; use rand::Rng; +use serde_derive::Deserialize; pub use itest_macro::itest; pub use with_procspawn_tempdir::with_procspawn_tempdir; @@ -209,6 +210,23 @@ pub(crate) fn prepare_reboot>(mark: M) -> Result<()> { Ok(()) } +#[derive(Deserialize, Debug)] +struct OstreeVersion { + features: Vec, +} + +pub(crate) fn check_ostree_feature>(feature: S) -> Result { + let feature = feature.as_ref(); + let out = std::process::Command::new("ostree") + .arg("--version") + .output()?; + if !out.status.success() { + anyhow::bail!("{:?}", out); + } + let v: OstreeVersion = serde_yaml::from_reader(&*out.stdout)?; + Ok(v.features.iter().any(|f| f.as_str() == feature)) +} + // I put tests in your tests so you can test while you test #[cfg(test)] mod tests {