From d7c25a206275fef951e41e70929f213058eb9104 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Fri, 22 Jul 2022 15:09:07 -0400 Subject: [PATCH 01/38] configure: post-release version bump --- configure.ac | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/configure.ac b/configure.ac index 964df128..56e0e040 100644 --- a/configure.ac +++ b/configure.ac @@ -1,10 +1,10 @@ AC_PREREQ([2.63]) dnl To perform a release, follow the instructions in `docs/CONTRIBUTING.md`. m4_define([year_version], [2022]) -m4_define([release_version], [5]) +m4_define([release_version], [6]) m4_define([package_version], [year_version.release_version]) AC_INIT([libostree], [package_version], [walters@verbum.org]) -is_release_build=yes +is_release_build=no AC_CONFIG_HEADER([config.h]) AC_CONFIG_MACRO_DIR([buildutil]) AC_CONFIG_AUX_DIR([build-aux]) From 55292e40079b1a1ae1ff469c31ebe9fca8a833e6 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Fri, 22 Jul 2022 15:19:43 -0400 Subject: [PATCH 02/38] rust-bindings: Fix `cargo fmt` --- rust-bindings/tests/sign/mod.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/rust-bindings/tests/sign/mod.rs b/rust-bindings/tests/sign/mod.rs index f3c817a3..300beedf 100644 --- a/rust-bindings/tests/sign/mod.rs +++ b/rust-bindings/tests/sign/mod.rs @@ -74,7 +74,9 @@ echo $ED25519SECRET > ed25519.secret let badsigs = [b"".as_slice()].to_variant(); let e = signer.data_verify(payload, &badsigs).err().unwrap(); - assert!(e.to_string().contains("Invalid signature length of 0 bytes")) + assert!(e + .to_string() + .contains("Invalid signature length of 0 bytes")) } #[test] From a83673f1a7a9c374415c9c1d17a1aa4bc75b03f0 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Mon, 1 Aug 2022 14:43:31 -0400 Subject: [PATCH 03/38] deny.toml: Add `Unicode-DFS-2016` This is used by the unicode crate now and is definitely a compatible FOSS license. --- deny.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/deny.toml b/deny.toml index e810c910..1a809ffc 100644 --- a/deny.toml +++ b/deny.toml @@ -1,7 +1,7 @@ [licenses] unlicensed = "deny" copyleft = "allow" -allow = ["Apache-2.0", "Apache-2.0 WITH LLVM-exception", "MIT", "BSD-3-Clause", "BSD-2-Clause", "Unlicense", "CC0-1.0"] +allow = ["Apache-2.0", "Apache-2.0 WITH LLVM-exception", "MIT", "BSD-3-Clause", "BSD-2-Clause", "Unlicense", "Unicode-DFS-2016"] private = { ignore = true } [bans] From edba4b33be10c05253bfa94895dfbc8477e44d76 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Wed, 3 Aug 2022 10:37:40 -0400 Subject: [PATCH 04/38] Remove unused `linux/fs.h` includes Prep for fixing conflicts introduced by newer glibc. cc https://github.com/ostreedev/ostree/issues/2685 --- src/libostree/ostree-repo-commit.c | 1 - src/ostree/ot-main.c | 1 - 2 files changed, 2 deletions(-) diff --git a/src/libostree/ostree-repo-commit.c b/src/libostree/ostree-repo-commit.c index afab3fdf..35b16c71 100644 --- a/src/libostree/ostree-repo-commit.c +++ b/src/libostree/ostree-repo-commit.c @@ -30,7 +30,6 @@ #include #include #include -#include #include #include "otutil.h" diff --git a/src/ostree/ot-main.c b/src/ostree/ot-main.c index b7b50d67..7a4405a5 100644 --- a/src/ostree/ot-main.c +++ b/src/ostree/ot-main.c @@ -28,7 +28,6 @@ #include #include #include -#include #include "ot-main.h" #include "ostree.h" From 0a908a180fcce98c2565b9fb34470e5953918260 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Wed, 3 Aug 2022 10:43:43 -0400 Subject: [PATCH 05/38] Move FIFREEZE/FITHAW ioctl invocations into linuxfsutil.c Should help avoid conflicts between glibc and linux headers. Closes: https://github.com/ostreedev/ostree/issues/2685 --- src/libostree/ostree-linuxfsutil.c | 24 ++++++++++++++++++++++-- src/libostree/ostree-linuxfsutil.h | 3 +++ src/libostree/ostree-sysroot-deploy.c | 7 +++---- 3 files changed, 28 insertions(+), 6 deletions(-) diff --git a/src/libostree/ostree-linuxfsutil.c b/src/libostree/ostree-linuxfsutil.c index 97f93604..aa389aaa 100644 --- a/src/libostree/ostree-linuxfsutil.c +++ b/src/libostree/ostree-linuxfsutil.c @@ -24,10 +24,12 @@ #include #include +// This should be the only file including linux/fs.h; see +// https://sourceware.org/glibc/wiki/Release/2.36#Usage_of_.3Clinux.2Fmount.h.3E_and_.3Csys.2Fmount.h.3E +// https://github.com/ostreedev/ostree/issues/2685 +#include #include -#include "otutil.h" - /** * _ostree_linuxfs_fd_alter_immutable_flag: * @fd: A file descriptor @@ -88,3 +90,21 @@ _ostree_linuxfs_fd_alter_immutable_flag (int fd, return TRUE; } + +/* Wrapper for FIFREEZE ioctl. + * This is split into a separate wrapped API for + * reasons around conflicts between glibc and linux/fs.h + * includes; see above. + */ +int +_ostree_linuxfs_filesystem_freeze (int fd) +{ + return TEMP_FAILURE_RETRY (ioctl (fd, FIFREEZE, 0)); +} + +/* Wrapper for FITHAW ioctl. See above. */ +int +_ostree_linuxfs_filesystem_thaw (int fd) +{ + return TEMP_FAILURE_RETRY (ioctl (fd, FITHAW, 0)); +} diff --git a/src/libostree/ostree-linuxfsutil.h b/src/libostree/ostree-linuxfsutil.h index 0654b6fc..9011c9d8 100644 --- a/src/libostree/ostree-linuxfsutil.h +++ b/src/libostree/ostree-linuxfsutil.h @@ -29,4 +29,7 @@ _ostree_linuxfs_fd_alter_immutable_flag (int fd, GCancellable *cancellable, GError **error); +int _ostree_linuxfs_filesystem_freeze (int fd); +int _ostree_linuxfs_filesystem_thaw (int fd); + G_END_DECLS diff --git a/src/libostree/ostree-sysroot-deploy.c b/src/libostree/ostree-sysroot-deploy.c index 2dc9f58b..d86de7dc 100644 --- a/src/libostree/ostree-sysroot-deploy.c +++ b/src/libostree/ostree-sysroot-deploy.c @@ -29,7 +29,6 @@ #include #include #include -#include #include #ifdef HAVE_LIBMOUNT @@ -1476,7 +1475,7 @@ fsfreeze_thaw_cycle (OstreeSysroot *self, * EOPNOTSUPP: If the filesystem doesn't support it */ int saved_errno = errno; - (void) TEMP_FAILURE_RETRY (ioctl (rootfs_dfd, FITHAW, 0)); + _ostree_linuxfs_filesystem_thaw (rootfs_dfd); errno = saved_errno; /* But if we got an error from poll, let's log it */ if (r < 0) @@ -1517,7 +1516,7 @@ fsfreeze_thaw_cycle (OstreeSysroot *self, return glnx_throw (error, "aborting due to test-fifreeze"); } /* Do a freeze/thaw cycle; TODO add a FIFREEZETHAW ioctl */ - if (ioctl (rootfs_dfd, FIFREEZE, 0) != 0) + if (_ostree_linuxfs_filesystem_freeze (rootfs_dfd) != 0) { /* Not supported, we're running in the unit tests (as non-root), or * the filesystem is already frozen (EBUSY). @@ -1539,7 +1538,7 @@ fsfreeze_thaw_cycle (OstreeSysroot *self, return glnx_throw_errno_prefix (error, "ioctl(FIFREEZE)"); } /* And finally thaw, then signal our completion to the watchdog */ - if (TEMP_FAILURE_RETRY (ioctl (rootfs_dfd, FITHAW, 0)) != 0) + if (_ostree_linuxfs_filesystem_thaw (rootfs_dfd) != 0) { /* Warn but don't error if the filesystem was already thawed */ if (errno == EINVAL) From 02b162347c63b4caf45cc64bade3780b506f262b Mon Sep 17 00:00:00 2001 From: Lukas Kalbertodt Date: Sat, 13 Aug 2022 16:14:23 +0200 Subject: [PATCH 06/38] Update to `libtest-mimic` 0.5.0 --- tests/inst/Cargo.toml | 2 +- tests/inst/src/insttestmain.rs | 28 ++++++---------------------- tests/inst/src/test.rs | 1 - 3 files changed, 7 insertions(+), 24 deletions(-) diff --git a/tests/inst/Cargo.toml b/tests/inst/Cargo.toml index e006dda1..cc3f712a 100644 --- a/tests/inst/Cargo.toml +++ b/tests/inst/Cargo.toml @@ -21,7 +21,7 @@ sh-inline = "0.2.0" anyhow = "1.0" tempfile = "3.1.0" ostree-ext = { version = "0.7.0" } -libtest-mimic = "0.3.0" +libtest-mimic = "0.5.0" twoway = "0.2.1" hyper = { version = "0.14", features = ["runtime", "http1", "http2", "tcp", "server"] } hyper-staticfile = "0.6.0" diff --git a/tests/inst/src/insttestmain.rs b/tests/inst/src/insttestmain.rs index 9d6d06b0..e8a7acb9 100644 --- a/tests/inst/src/insttestmain.rs +++ b/tests/inst/src/insttestmain.rs @@ -1,4 +1,5 @@ use anyhow::{bail, Result}; +use libtest_mimic::Trial; use structopt::StructOpt; mod destructive; @@ -49,26 +50,6 @@ enum NonDestructiveOpts { Args(Vec), } -fn libtest_from_test(t: &StaticTest) -> test::TestImpl { - libtest_mimic::Test { - name: t.0.into(), - kind: "".into(), - is_ignored: false, - is_bench: false, - data: t.1, - } -} - -fn run_test(test: &test::TestImpl) -> libtest_mimic::Outcome { - if let Err(e) = (test.data)() { - libtest_mimic::Outcome::Failed { - msg: Some(e.to_string()), - } - } else { - libtest_mimic::Outcome::Passed - } -} - fn main() -> Result<()> { // Ensure we're always in tempdir so we can rely on it globally. // We use /var/tmp to ensure we have storage space in the destructive @@ -100,8 +81,11 @@ fn main() -> Result<()> { // FIXME add method to parse subargs let NonDestructiveOpts::Args(iter) = subopt; let libtestargs = libtest_mimic::Arguments::from_iter(iter); - let tests: Vec<_> = TESTS.iter().map(libtest_from_test).collect(); - libtest_mimic::run_tests(&libtestargs, tests, run_test).exit(); + let tests: Vec<_> = TESTS + .iter() + .map(|(name, fun)| Trial::test(*name, move || fun().map_err(Into::into))) + .collect(); + libtest_mimic::run(&libtestargs, tests).exit(); } Opt::RunDestructive { name } => { if !std::path::Path::new(DESTRUCTIVE_TEST_STAMP).exists() { diff --git a/tests/inst/src/test.rs b/tests/inst/src/test.rs index c18dbd23..bca85977 100644 --- a/tests/inst/src/test.rs +++ b/tests/inst/src/test.rs @@ -18,7 +18,6 @@ use hyper_staticfile::Static; use tokio::runtime::Runtime; pub(crate) type TestFn = fn() -> Result<()>; -pub(crate) type TestImpl = libtest_mimic::Test; /// Run command and assert that its stderr contains pat pub(crate) fn cmd_fails_with>(mut c: C, pat: &str) -> Result<()> { From 93e47f88f486e5df1030066d0120b0db034ca6c8 Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Mon, 15 Aug 2022 13:54:35 -0400 Subject: [PATCH 07/38] lib/commit: Directly use FICLONE for payload link The idea of payload linking is to reflink between objects where possible. Instead of relying on `glnx_regfile_copy_bytes` to hit the `FICLONE` path, just call `FICLONE` directly. At that point in the code, we've already established that the source and dest repos are on the same filesystem and that it supports `FICLONE`. Related: https://gitlab.gnome.org/GNOME/libglnx/-/merge_requests/41 Related: https://github.com/ostreedev/ostree/pull/2684#issuecomment-1204068437 --- src/libostree/ostree-repo-commit.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/libostree/ostree-repo-commit.c b/src/libostree/ostree-repo-commit.c index 35b16c71..6cfdc32b 100644 --- a/src/libostree/ostree-repo-commit.c +++ b/src/libostree/ostree-repo-commit.c @@ -881,11 +881,8 @@ _try_clone_from_payload_link (OstreeRepo *self, else { /* This undoes all of the previous writes; we want to generate reflinked data. */ - if (ftruncate (tmpf->fd, 0) < 0) - return glnx_throw_errno_prefix (error, "ftruncate"); - - if (glnx_regfile_copy_bytes (fdf, tmpf->fd, -1) < 0) - return glnx_throw_errno_prefix (error, "regfile copy"); + if (ioctl (tmpf->fd, FICLONE, fdf) < 0) + return glnx_throw_errno_prefix (error, "FICLONE"); return TRUE; } From 84670a007061196249cde89e31b36b5b37478e61 Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Mon, 15 Aug 2022 17:50:11 -0400 Subject: [PATCH 08/38] tests/kolainst/staged-deploy: parse `rpm-ostree status --json` instead Don't parse `rpm-ostree status` output, it's not meant for that. Use `--json` output instead. While we're here, fix an obsolete reference to Ansible. Related: https://github.com/coreos/rpm-ostree/pull/3938 --- tests/kolainst/destructive/staged-deploy.sh | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/kolainst/destructive/staged-deploy.sh b/tests/kolainst/destructive/staged-deploy.sh index af31078a..c923605f 100755 --- a/tests/kolainst/destructive/staged-deploy.sh +++ b/tests/kolainst/destructive/staged-deploy.sh @@ -100,8 +100,7 @@ EOF newcommit=$(ostree rev-parse staged-deploy) ostree admin upgrade --stage >out.txt test -f /run/ostree/staged-deployment - # Debating bouncing back out to Ansible for this - firstdeploycommit=$(rpm-ostree status |grep 'Commit:' |head -1|sed -e 's,^ *Commit: *,,') + firstdeploycommit=$(rpm-ostree status --json | jq -r .deployments[0].checksum) assert_streq "${firstdeploycommit}" "${newcommit}" # Cleanup rpm-ostree cleanup -rp From 18c97d7563f929a90af34cfb36c7a42d5313ddf6 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue, 16 Aug 2022 07:28:21 +0000 Subject: [PATCH 09/38] build(deps): bump libglnx from `c59eb27` to `26375b5` Bumps libglnx from `c59eb27` to `26375b5`. --- updated-dependencies: - dependency-name: libglnx dependency-type: direct:production ... Signed-off-by: dependabot[bot] --- libglnx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libglnx b/libglnx index c59eb271..26375b53 160000 --- a/libglnx +++ b/libglnx @@ -1 +1 @@ -Subproject commit c59eb271b5127309c73117bbc871dcfd9fd159a6 +Subproject commit 26375b5308360764bc6604f292192b88a59ad3ce From 090f312e4075df4deb0e40dbead304eef05b5fdf Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Tue, 16 Aug 2022 17:11:30 -0400 Subject: [PATCH 10/38] cli/rev-parse: Port to new code style Prep for future changes. --- src/ostree/ot-builtin-rev-parse.c | 26 ++++++++------------------ 1 file changed, 8 insertions(+), 18 deletions(-) diff --git a/src/ostree/ot-builtin-rev-parse.c b/src/ostree/ot-builtin-rev-parse.c index c4a7ec94..521d2159 100644 --- a/src/ostree/ot-builtin-rev-parse.c +++ b/src/ostree/ot-builtin-rev-parse.c @@ -38,34 +38,24 @@ static GOptionEntry options[] = { gboolean ostree_builtin_rev_parse (int argc, char **argv, OstreeCommandInvocation *invocation, GCancellable *cancellable, GError **error) { - g_autoptr(GOptionContext) context = NULL; + g_autoptr(GOptionContext) context = g_option_context_new ("REV"); g_autoptr(OstreeRepo) repo = NULL; - gboolean ret = FALSE; - const char *rev = "master"; - int i; - g_autofree char *resolved_rev = NULL; - - context = g_option_context_new ("REV"); - if (!ostree_option_context_parse (context, options, &argc, &argv, invocation, &repo, cancellable, error)) - goto out; + return FALSE; if (argc < 2) { ot_util_usage_error (context, "REV must be specified", error); - goto out; + return FALSE; } - for (i = 1; i < argc; i++) + for (gint i = 1; i < argc; i++) { - rev = argv[i]; - g_free (resolved_rev); - resolved_rev = NULL; + const char *rev = argv[i]; + g_autofree char *resolved_rev = NULL; if (!ostree_repo_resolve_rev (repo, rev, FALSE, &resolved_rev, error)) - goto out; + return FALSE; g_print ("%s\n", resolved_rev); } - ret = TRUE; - out: - return ret; + return TRUE; } From 092421fabf3ae573b80370c924882cdcc6815af9 Mon Sep 17 00:00:00 2001 From: Georges Basile Stavracas Neto Date: Tue, 16 Aug 2022 19:54:29 -0300 Subject: [PATCH 11/38] lib/commit: Unref repo on success Commit 540e60c3 introduced _ostree_repo_auto_transaction_new(), a private constructor to OstreeRepoAutoTransaction, by factoring out some code from _ostree_repo_auto_transaction_start(). This factored code increased the refcount of the 'repo' variable. Subsequent commit 71304e854c made ostree_repo_prepare_transaction() use ths newly introduced constructor. However, in this function, the happy path assumed no ref was taken, and therefore did not unref it. Commit 71304e854c didn't add the corresponding unref either. This leaks a reference to OstreeRepo when calling ostree_repo_prepare_transaction(). Plug this leak by using g_clear_object() to clear the repo field of OstreeRepoAutoTransaction, instead of simply setting it to NULL. Closes https://github.com/flatpak/flatpak/issues/4928 --- src/libostree/ostree-repo-commit.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libostree/ostree-repo-commit.c b/src/libostree/ostree-repo-commit.c index 6cfdc32b..c22864cd 100644 --- a/src/libostree/ostree-repo-commit.c +++ b/src/libostree/ostree-repo-commit.c @@ -1745,7 +1745,7 @@ ostree_repo_prepare_transaction (OstreeRepo *self, return FALSE; /* Success: do not abort the transaction when returning. */ - txn->repo = NULL; (void) txn; + g_clear_object (&txn->repo); (void) txn; if (out_transaction_resume) *out_transaction_resume = ret_transaction_resume; From ad0354ac36aa2e7b0497f0e1636ee0d1a2d31033 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Tue, 16 Aug 2022 17:27:46 -0400 Subject: [PATCH 12/38] cli/rev-parse: Add `--single` option In the current "ostree native container" flow, we're inserting a commit object into the repo but with no refs. We have hacks in a few places to find the commit digest via e.g. `find repo/objects -name *.commit` but that's a horrible hack. Add `ostree rev-parse --single` which will print the single commit, and error out if there is not exactly one commit. Co-authored-by: Jonathan Lebon --- man/ostree-rev-parse.xml | 13 +++++++++++ src/ostree/ot-builtin-rev-parse.c | 36 +++++++++++++++++++++++++++++++ tests/basic-test.sh | 18 +++++++++++++++- 3 files changed, 66 insertions(+), 1 deletion(-) diff --git a/man/ostree-rev-parse.xml b/man/ostree-rev-parse.xml index 74c585fd..f39868a6 100644 --- a/man/ostree-rev-parse.xml +++ b/man/ostree-rev-parse.xml @@ -54,6 +54,19 @@ License along with this library. If not, see . + + Options + + + , + + + If the repository has exactly one commit, then print it; any other case will result in an error. + + + + + Description diff --git a/src/ostree/ot-builtin-rev-parse.c b/src/ostree/ot-builtin-rev-parse.c index 521d2159..95cb45ab 100644 --- a/src/ostree/ot-builtin-rev-parse.c +++ b/src/ostree/ot-builtin-rev-parse.c @@ -25,13 +25,17 @@ #include "ot-builtins.h" #include "ostree.h" #include "otutil.h" +#include /* ATTENTION: * Please remember to update the bash-completion script (bash/ostree) and * man page (man/ostree-rev-parse.xml) when changing the option list. */ +static gboolean opt_single; + static GOptionEntry options[] = { + { "single", 'S', 0, G_OPTION_ARG_NONE, &opt_single, "If the repository has exactly one commit, then print it; any other case will result in an error", NULL }, { NULL } }; @@ -43,11 +47,43 @@ ostree_builtin_rev_parse (int argc, char **argv, OstreeCommandInvocation *invoca if (!ostree_option_context_parse (context, options, &argc, &argv, invocation, &repo, cancellable, error)) return FALSE; + if (opt_single) + { + if (argc >= 2) + { + ot_util_usage_error (context, "Cannot specify arguments with --single", error); + return FALSE; + } + + g_autoptr(GHashTable) objects = NULL; + if (!ostree_repo_list_commit_objects_starting_with (repo, "", &objects, cancellable, error)) + return FALSE; + + GVariant *found = NULL; + GLNX_HASH_TABLE_FOREACH (objects, GVariant*, key) + { + if (found) + return glnx_throw (error, "Multiple commit objects found"); + found = key; + } + if (!found) + return glnx_throw (error, "No commit objects found"); + const char *checksum; + OstreeObjectType objtype; + ostree_object_name_deserialize (found, &checksum, &objtype); + g_assert (objtype == OSTREE_OBJECT_TYPE_COMMIT); + + g_print ("%s\n", checksum); + + return TRUE; /* Note early return */ + } + if (argc < 2) { ot_util_usage_error (context, "REV must be specified", error); return FALSE; } + for (gint i = 1; i < argc; i++) { const char *rev = argv[i]; diff --git a/tests/basic-test.sh b/tests/basic-test.sh index 04506c3d..67d57dda 100644 --- a/tests/basic-test.sh +++ b/tests/basic-test.sh @@ -19,7 +19,7 @@ set -euo pipefail -echo "1..$((87 + ${extra_basic_tests:-0}))" +echo "1..$((88 + ${extra_basic_tests:-0}))" CHECKOUT_U_ARG="" CHECKOUT_H_ARGS="-H" @@ -97,6 +97,22 @@ $OSTREE rev-parse 'test2^' $OSTREE rev-parse 'test2^^' 2>/dev/null && fatal "rev-parse test2^^ unexpectedly succeeded!" echo "ok rev-parse" +if $OSTREE rev-parse -S 2>err.txt; then + fatal "rev parse multiple" +fi +assert_file_has_content_literal err.txt 'Multiple commit objects found' +$CMD_PREFIX ostree --repo=repo-copy init --mode=archive +if $CMD_PREFIX ostree --repo=repo-copy rev-parse -S 2>err.txt; then + fatal "rev parse none" +fi +assert_file_has_content_literal err.txt 'No commit objects found' +rev=$($OSTREE rev-parse test2) +$CMD_PREFIX ostree --repo=repo-copy pull-local repo test2 +rev2=$($CMD_PREFIX ostree --repo=repo-copy rev-parse -S) +assert_streq "${rev}" "${rev2}" +echo "ok rev-parse -S" + + checksum=$($OSTREE rev-parse test2) partial=${checksum:0:6} echo "partial:" $partial From ff7d9a8a6dbeeb922847e42a2b1ed6f83b72615e Mon Sep 17 00:00:00 2001 From: Luca BRUNO Date: Fri, 19 Aug 2022 10:39:09 +0000 Subject: [PATCH 13/38] libostree: fix a typo in annotation This fixes a typo in the `allow-none` annotation on `ostree_sysroot_deployment_set_kargs_in_place` argument. --- src/libostree/ostree-sysroot-deploy.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libostree/ostree-sysroot-deploy.c b/src/libostree/ostree-sysroot-deploy.c index d86de7dc..1112dedf 100644 --- a/src/libostree/ostree-sysroot-deploy.c +++ b/src/libostree/ostree-sysroot-deploy.c @@ -3594,7 +3594,7 @@ ostree_sysroot_deployment_set_kargs (OstreeSysroot *self, * ostree_sysroot_deployment_set_kargs_in_place: * @self: Sysroot * @deployment: A deployment - * @kargs_str: (allow none): Replace @deployment's kernel arguments + * @kargs_str: (allow-none): Replace @deployment's kernel arguments * @cancellable: Cancellable * @error: Error * From b473cc7d6242cce992e955578612ec44b631fb37 Mon Sep 17 00:00:00 2001 From: Jon Oster Date: Fri, 19 Aug 2022 16:27:01 +0200 Subject: [PATCH 14/38] docs: Add aktualizr and TorizonCore to related projects Signed-off-by: Jon Oster --- README.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/README.md b/README.md index 6a51482b..d0232e7f 100644 --- a/README.md +++ b/README.md @@ -73,6 +73,8 @@ their host system as well as Flatpak. [Liri OS](https://liri.io/download/silverblue/) has the option to install their distribution using ostree. +[TorizonCore](https://developer.toradex.com/torizon/working-with-torizon/torizoncore-technical-overview/) is a Linux distribution for embedded systems that updates via OSTree images delivered via [Uptane](https://uptane.github.io/) and [aktualizr](https://github.com/uptane/aktualizr/). + ## Distribution build tools [meta-updater](https://github.com/advancedtelematic/meta-updater) is @@ -109,6 +111,8 @@ use the "libostree host system" aspects (e.g. bootloader management), just the "git-like hardlink dedup". For example, Flatpak supports a per-user OSTree repository. +[aktualizr](https://github.com/uptane/aktualizr/) is an [Uptane](https://uptane.github.io/)-conformant software update client library intended for use in automotive and other security-sensitive embedded devices. It uses OSTree to manage the OS of the host device by default. + ## Language bindings libostree is accessible via [GObject Introspection](https://gi.readthedocs.io/en/latest/); From 629d2f5d7e1c982bbd0d3aa5e0771ec689646773 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Mon, 22 Aug 2022 14:23:03 -0400 Subject: [PATCH 15/38] rust: Update to latest git In the future I may try to add CI that requires this to be sync'd... --- rust-bindings/src/auto/repo.rs | 12 ++++++------ rust-bindings/src/auto/sysroot.rs | 9 +++++++++ rust-bindings/src/auto/versions.txt | 2 +- rust-bindings/sys/src/auto/versions.txt | 2 +- rust-bindings/sys/src/lib.rs | 10 ++++++++++ 5 files changed, 27 insertions(+), 8 deletions(-) diff --git a/rust-bindings/src/auto/repo.rs b/rust-bindings/src/auto/repo.rs index 4fd2a29c..4bafe755 100644 --- a/rust-bindings/src/auto/repo.rs +++ b/rust-bindings/src/auto/repo.rs @@ -429,12 +429,12 @@ impl Repo { //} //#[doc(alias = "ostree_repo_list_commit_objects_starting_with")] - //pub fn list_commit_objects_starting_with>(&self, start: &str, out_commits: /*Unknown conversion*//*Unimplemented*/HashTable TypeId { ns_id: 2, id: 202 }/TypeId { ns_id: 2, id: 202 }, cancellable: Option<&P>) -> Result<(), glib::Error> { + //pub fn list_commit_objects_starting_with>(&self, start: &str, out_commits: /*Unknown conversion*//*Unimplemented*/HashTable TypeId { ns_id: 2, id: 203 }/TypeId { ns_id: 2, id: 203 }, cancellable: Option<&P>) -> Result<(), glib::Error> { // unsafe { TODO: call ffi:ostree_repo_list_commit_objects_starting_with() } //} //#[doc(alias = "ostree_repo_list_objects")] - //pub fn list_objects>(&self, flags: RepoListObjectsFlags, out_objects: /*Unknown conversion*//*Unimplemented*/HashTable TypeId { ns_id: 2, id: 202 }/TypeId { ns_id: 2, id: 202 }, cancellable: Option<&P>) -> Result<(), glib::Error> { + //pub fn list_objects>(&self, flags: RepoListObjectsFlags, out_objects: /*Unknown conversion*//*Unimplemented*/HashTable TypeId { ns_id: 2, id: 203 }/TypeId { ns_id: 2, id: 203 }, cancellable: Option<&P>) -> Result<(), glib::Error> { // unsafe { TODO: call ffi:ostree_repo_list_objects() } //} @@ -1027,7 +1027,7 @@ impl Repo { } //#[doc(alias = "ostree_repo_traverse_commit")] - //pub fn traverse_commit>(&self, commit_checksum: &str, maxdepth: i32, out_reachable: /*Unknown conversion*//*Unimplemented*/HashTable TypeId { ns_id: 2, id: 202 }/TypeId { ns_id: 2, id: 202 }, cancellable: Option<&P>) -> Result<(), glib::Error> { + //pub fn traverse_commit>(&self, commit_checksum: &str, maxdepth: i32, out_reachable: /*Unknown conversion*//*Unimplemented*/HashTable TypeId { ns_id: 2, id: 203 }/TypeId { ns_id: 2, id: 203 }, cancellable: Option<&P>) -> Result<(), glib::Error> { // unsafe { TODO: call ffi:ostree_repo_traverse_commit() } //} @@ -1053,7 +1053,7 @@ impl Repo { //#[cfg(any(feature = "v2018_6", feature = "dox"))] //#[cfg_attr(feature = "dox", doc(cfg(feature = "v2018_6")))] //#[doc(alias = "ostree_repo_traverse_reachable_refs")] - //pub fn traverse_reachable_refs>(&self, depth: u32, reachable: /*Unknown conversion*//*Unimplemented*/HashTable TypeId { ns_id: 2, id: 202 }/TypeId { ns_id: 2, id: 202 }, cancellable: Option<&P>) -> Result<(), glib::Error> { + //pub fn traverse_reachable_refs>(&self, depth: u32, reachable: /*Unknown conversion*//*Unimplemented*/HashTable TypeId { ns_id: 2, id: 203 }/TypeId { ns_id: 2, id: 203 }, cancellable: Option<&P>) -> Result<(), glib::Error> { // unsafe { TODO: call ffi:ostree_repo_traverse_reachable_refs() } //} @@ -1299,12 +1299,12 @@ impl Repo { //#[cfg(any(feature = "v2018_5", feature = "dox"))] //#[cfg_attr(feature = "dox", doc(cfg(feature = "v2018_5")))] //#[doc(alias = "ostree_repo_traverse_new_parents")] - //pub fn traverse_new_parents() -> /*Unknown conversion*//*Unimplemented*/HashTable TypeId { ns_id: 2, id: 202 }/TypeId { ns_id: 2, id: 202 } { + //pub fn traverse_new_parents() -> /*Unknown conversion*//*Unimplemented*/HashTable TypeId { ns_id: 2, id: 203 }/TypeId { ns_id: 2, id: 203 } { // unsafe { TODO: call ffi:ostree_repo_traverse_new_parents() } //} //#[doc(alias = "ostree_repo_traverse_new_reachable")] - //pub fn traverse_new_reachable() -> /*Unknown conversion*//*Unimplemented*/HashTable TypeId { ns_id: 2, id: 202 }/TypeId { ns_id: 2, id: 202 } { + //pub fn traverse_new_reachable() -> /*Unknown conversion*//*Unimplemented*/HashTable TypeId { ns_id: 2, id: 203 }/TypeId { ns_id: 2, id: 203 } { // unsafe { TODO: call ffi:ostree_repo_traverse_new_reachable() } //} diff --git a/rust-bindings/src/auto/sysroot.rs b/rust-bindings/src/auto/sysroot.rs index f10f630c..a88b3f66 100644 --- a/rust-bindings/src/auto/sysroot.rs +++ b/rust-bindings/src/auto/sysroot.rs @@ -109,6 +109,15 @@ impl Sysroot { } } + #[doc(alias = "ostree_sysroot_deployment_set_kargs_in_place")] + pub fn deployment_set_kargs_in_place>(&self, deployment: &Deployment, kargs_str: &str, cancellable: Option<&P>) -> Result<(), glib::Error> { + unsafe { + let mut error = ptr::null_mut(); + let _ = ffi::ostree_sysroot_deployment_set_kargs_in_place(self.to_glib_none().0, deployment.to_glib_none().0, kargs_str.to_glib_none().0, cancellable.map(|p| p.as_ref()).to_glib_none().0, &mut error); + if error.is_null() { Ok(()) } else { Err(from_glib_full(error)) } + } + } + #[doc(alias = "ostree_sysroot_deployment_set_mutable")] pub fn deployment_set_mutable>(&self, deployment: &Deployment, is_mutable: bool, cancellable: Option<&P>) -> Result<(), glib::Error> { unsafe { diff --git a/rust-bindings/src/auto/versions.txt b/rust-bindings/src/auto/versions.txt index 3c7edf9e..f22d532f 100644 --- a/rust-bindings/src/auto/versions.txt +++ b/rust-bindings/src/auto/versions.txt @@ -1,2 +1,2 @@ Generated by gir (https://github.com/gtk-rs/gir @ e8f82cf6) -from gir-files (@ ee5b3c76) +from gir-files (@ 1d478ced) diff --git a/rust-bindings/sys/src/auto/versions.txt b/rust-bindings/sys/src/auto/versions.txt index 47d43759..f22d532f 100644 --- a/rust-bindings/sys/src/auto/versions.txt +++ b/rust-bindings/sys/src/auto/versions.txt @@ -1,2 +1,2 @@ Generated by gir (https://github.com/gtk-rs/gir @ e8f82cf6) -from gir-files (@ 21901c2d) +from gir-files (@ 1d478ced) diff --git a/rust-bindings/sys/src/lib.rs b/rust-bindings/sys/src/lib.rs index dd29ee23..837c60b0 100644 --- a/rust-bindings/sys/src/lib.rs +++ b/rust-bindings/sys/src/lib.rs @@ -1210,6 +1210,9 @@ extern "C" { argv: *mut *mut c_char, prefixes: *mut *mut c_char, ); + #[cfg(any(feature = "v2022_5", feature = "dox"))] + #[cfg_attr(feature = "dox", doc(cfg(feature = "v2022_5")))] + pub fn ostree_kernel_args_append_if_missing(kargs: *mut OstreeKernelArgs, arg: *const c_char); #[cfg(any(feature = "v2019_3", feature = "dox"))] #[cfg_attr(feature = "dox", doc(cfg(feature = "v2019_3")))] pub fn ostree_kernel_args_append_proc_cmdline( @@ -3024,6 +3027,13 @@ extern "C" { cancellable: *mut gio::GCancellable, error: *mut *mut glib::GError, ) -> gboolean; + pub fn ostree_sysroot_deployment_set_kargs_in_place( + self_: *mut OstreeSysroot, + deployment: *mut OstreeDeployment, + kargs_str: *mut c_char, + cancellable: *mut gio::GCancellable, + error: *mut *mut glib::GError, + ) -> gboolean; pub fn ostree_sysroot_deployment_set_mutable( self_: *mut OstreeSysroot, deployment: *mut OstreeDeployment, From 21a8f3928426b6d324eb22eeba4704bdd3d1477b Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Mon, 22 Aug 2022 15:59:33 -0400 Subject: [PATCH 16/38] ci: Also drop seccomp on debian testing I didn't deep dive into debugging this but I'm pretty sure it's https://gitlab.gnome.org/GNOME/glib/-/issues/2580 which is us having an older Docker in the middle here. --- .github/workflows/tests.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 043202b5..ab3723ea 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -126,6 +126,7 @@ jobs: - name: Debian Testing image: debian:testing-slim + container-options: --security-opt seccomp=unconfined pre-checkout-setup: | apt-get update apt-get install -y git From e075c510578a468fd292d8e97c140013da867151 Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Tue, 23 Aug 2022 10:56:28 -0400 Subject: [PATCH 17/38] docs: Add section about staged deployments I was explaining staged deployments to someone today and was looking for a doc but we didn't have any. Fix that. --- docs/deployment.md | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/docs/deployment.md b/docs/deployment.md index 30323f8c..affad83d 100644 --- a/docs/deployment.md +++ b/docs/deployment.md @@ -85,6 +85,26 @@ At the moment, OSTree only supports upgrading a single refspec. However, in the future OSTree may support a syntax for composing layers of trees, for example. +### Staged deployments + +As mentioned above, when OSTree creates a new deployment, a 3-way merge is done +to update its `/etc`. Depending on the nature of the system, this can cause an +issue: if a user or program modifies the booted `/etc` *after* the pending +deployment is created but *before* rebooting, those modifications will be lost. +OSTree does not do a second `/etc` merge on reboot. + +To counter this, OSTree supports staged deployments. In this flow, deployments +are created using e.g. `ostree admin upgrade --stage` on the CLI. The new +deployment is still created when the command is invoked, but the 3-way `/etc` +merge is delayed until the system is rebooted or shut down. Additionally, +updating the bootloader is also delayed. This is done by the +`ostree-finalize-staged.service` systemd unit. + +The main disadvantage of this approach is that rebooting can take longer and the +failure mode can be confusing (the machine will reboot into the same +deployment). In systems where the workload is well-understood and not subject to +the `/etc` issue above, it may be better to not stage deployments. + ### The system /boot While OSTree parallel installs deployments cleanly inside the From c568073d1e6a5e602a6df29eaa5b7392e076f5d6 Mon Sep 17 00:00:00 2001 From: Sam James Date: Tue, 23 Aug 2022 23:37:06 +0100 Subject: [PATCH 18/38] buildutil/glibtests.m4: fix bashism configure scripts need to be runnable with a POSIX-compliant /bin/sh. On many (but not all!) systems, /bin/sh is provided by Bash, so errors like this aren't spotted. Notably Debian defaults to /bin/sh provided by dash which doesn't tolerate such bashisms as '=='. This retains compatibility with bash. Fixes configure warnings/errors like: ``` checking whether to build static libraries... no ./configure: 14795: test: unexpected operator ``` Signed-off-by: Sam James --- buildutil/glibtests.m4 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/buildutil/glibtests.m4 b/buildutil/glibtests.m4 index 108c8478..34dfcf67 100644 --- a/buildutil/glibtests.m4 +++ b/buildutil/glibtests.m4 @@ -25,7 +25,7 @@ AC_DEFUN([GLIB_TESTS], *) AC_MSG_ERROR([bad value ${enableval} for --enable-always-build-tests]) ;; esac]) AM_CONDITIONAL([ENABLE_ALWAYS_BUILD_TESTS], test "$ENABLE_ALWAYS_BUILD_TESTS" = "1") - if test "$ENABLE_INSTALLED_TESTS" == "1"; then + if test "$ENABLE_INSTALLED_TESTS" = "1"; then AC_SUBST(installed_test_metadir, [${datadir}/installed-tests/]AC_PACKAGE_NAME) AC_SUBST(installed_testdir, [${libexecdir}/installed-tests/]AC_PACKAGE_NAME) fi From b55054ec2494d5db320e2c858e6616a6f743973a Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Mon, 22 Aug 2022 14:10:16 -0400 Subject: [PATCH 19/38] rust: Bind `ostree_repo_list_commits_starting_with` I wanted to use this as a fallback for https://github.com/ostreedev/ostree-rs-ext/pull/351/commits/30dee81c22ad5cb90e77198d3ddbcc25d388afb5 --- rust-bindings/conf/ostree.toml | 5 ++++ rust-bindings/src/repo.rs | 48 +++++++++++++++++++++++++++++++++ rust-bindings/tests/repo/mod.rs | 27 +++++++++++++++++++ 3 files changed, 80 insertions(+) diff --git a/rust-bindings/conf/ostree.toml b/rust-bindings/conf/ostree.toml index 57c2a793..ec2b594d 100644 --- a/rust-bindings/conf/ostree.toml +++ b/rust-bindings/conf/ostree.toml @@ -173,6 +173,11 @@ concurrency = "send" pattern = "^(load_file)$" ignore = true + [[object.function]] + # [MANUAL] hash table with variants + pattern = "^(list_commit_objects_starting_with)$" + ignore = true + [[object]] name = "OSTree.RepoFinder" status = "generate" diff --git a/rust-bindings/src/repo.rs b/rust-bindings/src/repo.rs index 6aeccff5..1e3e90bc 100644 --- a/rust-bindings/src/repo.rs +++ b/rust-bindings/src/repo.rs @@ -556,4 +556,52 @@ impl Repo { // Safety: We know the variant type will match since we just passed it above Ok(crate::DirMetaParsed::from_variant(&v).unwrap()) } + + /// List all commit objects; an optional prefix filter may be applied. + #[doc(alias = "ostree_repo_list_commit_objects_starting_with")] + pub fn list_commit_objects_starting_with>( + &self, + prefix: Option<&str>, + cancellable: Option<&P>, + ) -> Result, glib::Error> { + use glib::ffi::gpointer; + let prefix = prefix.unwrap_or(""); + unsafe { + let repo = self.to_glib_none().0; + let mut commits = ptr::null_mut(); + let cancellable = cancellable.map(|p| p.as_ref()).to_glib_none().0; + let mut error = ptr::null_mut(); + let prefix = prefix.to_glib_none(); + let r = ffi::ostree_repo_list_commit_objects_starting_with( + repo, + prefix.0, + &mut commits, + cancellable, + &mut error, + ); + if !error.is_null() { + assert_eq!(r, 0); + return Err(from_glib_full(error)); + } + assert_ne!(r, 0); + let mut ret = HashSet::with_capacity(glib::ffi::g_hash_table_size(commits) as usize); + unsafe extern "C" fn visit_hash_table( + key: *mut libc::c_void, + _value: gpointer, + r: *mut libc::c_void, + ) -> glib::ffi::gboolean { + let key: glib::Variant = from_glib_none(key as *const glib::ffi::GVariant); + let checksum = crate::object_name_deserialize(&key).0; + let r = &mut *(r as *mut HashSet); + r.insert(checksum); + true.into() + } + glib::ffi::g_hash_table_foreach_remove( + commits, + Some(visit_hash_table), + &mut ret as *mut HashSet as *mut _, + ); + Ok(ret) + } + } } diff --git a/rust-bindings/tests/repo/mod.rs b/rust-bindings/tests/repo/mod.rs index 703dc735..0a94eb9f 100644 --- a/rust-bindings/tests/repo/mod.rs +++ b/rust-bindings/tests/repo/mod.rs @@ -26,6 +26,33 @@ fn should_commit_content_to_repo_and_list_refs_again() { assert_eq!(checksum, refs["test"]); } +#[test] +fn list_commits() { + let cancellable = gio::NONE_CANCELLABLE; + let test_repo = TestRepo::new(); + + for prefix in [None, Some("a"), Some("0abcde")] { + let commits = test_repo + .repo + .list_commit_objects_starting_with(prefix, cancellable) + .unwrap(); + assert_eq!(commits.len(), 0); + } + + let rev = test_repo.test_commit("testref"); + + for prefix in [None, Some(&rev[0..1]), Some(&rev[0..5])] { + let commits = test_repo + .repo + .list_commit_objects_starting_with(prefix, cancellable) + .unwrap() + .into_iter() + .collect::>(); + assert_eq!(commits.len(), 1); + assert_eq!(commits[0].as_str(), rev.as_str()); + } +} + #[test] #[cfg(feature = "cap-std-apis")] fn cap_std_commit() { From 37aa2ac2876ae7fb0f14689aa20394694d97c2dd Mon Sep 17 00:00:00 2001 From: Huijing Hei Date: Fri, 19 Aug 2022 11:31:46 +0800 Subject: [PATCH 20/38] Fix `ostree admin kargs edit-in-place` assertion when deployments are pending This is to support pending deployments instead of rasing assertion. For example: ``` $ sudo rpm-ostree kargs --append=foo=bar $ sudo ostree admin kargs edit-in-place --append-if-missing=foobar ``` After reboot we get both `foo=bar foobar`. Fix https://github.com/ostreedev/ostree/issues/2679 --- src/libostree/ostree-sysroot-deploy.c | 64 ++++++++++++++----- .../destructive/kargs-edit-in-place.sh | 20 ++++-- 2 files changed, 63 insertions(+), 21 deletions(-) diff --git a/src/libostree/ostree-sysroot-deploy.c b/src/libostree/ostree-sysroot-deploy.c index 1112dedf..7b2f1a6f 100644 --- a/src/libostree/ostree-sysroot-deploy.c +++ b/src/libostree/ostree-sysroot-deploy.c @@ -3614,25 +3614,55 @@ ostree_sysroot_deployment_set_kargs_in_place (OstreeSysroot *self, if (!_ostree_sysroot_ensure_writable (self, error)) return FALSE; - g_assert (!ostree_deployment_is_staged (deployment)); + // handle staged deployment + if (ostree_deployment_is_staged (deployment)) + { + /* Read the staged state from disk */ + glnx_autofd int fd = -1; + if (!glnx_openat_rdonly (AT_FDCWD, _OSTREE_SYSROOT_RUNSTATE_STAGED, TRUE, &fd, error)) + return FALSE; - OstreeBootconfigParser *new_bootconfig = ostree_deployment_get_bootconfig (deployment); - ostree_bootconfig_parser_set (new_bootconfig, "options", kargs_str); - - g_autofree char *bootconf_name = - g_strdup_printf ("ostree-%d-%s.conf", - self->deployments->len - ostree_deployment_get_index (deployment), - ostree_deployment_get_osname (deployment)); - - g_autofree char *bootconfdir = g_strdup_printf ("loader.%d/entries", self->bootversion); - glnx_autofd int bootconf_dfd = -1; - if (!glnx_opendirat (self->boot_fd, bootconfdir, TRUE, &bootconf_dfd, error)) - return FALSE; - - if (!ostree_bootconfig_parser_write_at (new_bootconfig, - bootconf_dfd, bootconf_name, + g_autoptr(GBytes) contents = ot_fd_readall_or_mmap (fd, 0, error); + if (!contents) + return FALSE; + g_autoptr(GVariant) staged_deployment_data = + g_variant_new_from_bytes ((GVariantType*)"a{sv}", contents, TRUE); + g_autoptr(GVariantDict) staged_deployment_dict = + g_variant_dict_new (staged_deployment_data); + + g_autoptr(OstreeKernelArgs) kargs = ostree_kernel_args_from_string (kargs_str); + g_auto(GStrv) kargs_strv = ostree_kernel_args_to_strv (kargs); + + g_variant_dict_insert (staged_deployment_dict, "kargs", "^a&s", kargs_strv); + g_autoptr(GVariant) new_staged_deployment_data = g_variant_dict_end (staged_deployment_dict); + + if (!glnx_file_replace_contents_at (fd, _OSTREE_SYSROOT_RUNSTATE_STAGED, + g_variant_get_data (new_staged_deployment_data), + g_variant_get_size (new_staged_deployment_data), + GLNX_FILE_REPLACE_NODATASYNC, cancellable, error)) - return FALSE; + return FALSE; + } + else + { + OstreeBootconfigParser *new_bootconfig = ostree_deployment_get_bootconfig (deployment); + ostree_bootconfig_parser_set (new_bootconfig, "options", kargs_str); + + g_autofree char *bootconf_name = + g_strdup_printf ("ostree-%d-%s.conf", + self->deployments->len - ostree_deployment_get_index (deployment), + ostree_deployment_get_osname (deployment)); + + g_autofree char *bootconfdir = g_strdup_printf ("loader.%d/entries", self->bootversion); + glnx_autofd int bootconf_dfd = -1; + if (!glnx_opendirat (self->boot_fd, bootconfdir, TRUE, &bootconf_dfd, error)) + return FALSE; + + if (!ostree_bootconfig_parser_write_at (new_bootconfig, + bootconf_dfd, bootconf_name, + cancellable, error)) + return FALSE; + } return TRUE; } diff --git a/tests/kolainst/destructive/kargs-edit-in-place.sh b/tests/kolainst/destructive/kargs-edit-in-place.sh index 6380ff33..66b78aa1 100755 --- a/tests/kolainst/destructive/kargs-edit-in-place.sh +++ b/tests/kolainst/destructive/kargs-edit-in-place.sh @@ -6,7 +6,19 @@ set -xeuo pipefail . ${KOLA_EXT_DATA}/libinsttest.sh -sudo ostree admin kargs edit-in-place --append-if-missing=testarg -assert_file_has_content /boot/loader/entries/ostree-* testarg - -echo "ok test `kargs edit-in-place --append-if-missing`" +case "${AUTOPKGTEST_REBOOT_MARK:-}" in +"") + sudo rpm-ostree kargs --append=somedummykarg=1 + sudo ostree admin kargs edit-in-place --append-if-missing=testarg + assert_file_has_content /boot/loader/entries/ostree-* testarg + /tmp/autopkgtest-reboot "2" + ;; +"2") + assert_file_has_content_literal /proc/cmdline somedummykarg=1 + assert_file_has_content_literal /proc/cmdline testarg + echo "ok test with stage: kargs edit-in-place --append-if-missing" + ;; +*) + fatal "Unexpected AUTOPKGTEST_REBOOT_MARK=${AUTOPKGTEST_REBOOT_MARK}" + ;; +esac From e30a3b6b17c89f55c33e7985d11ccae7eb173507 Mon Sep 17 00:00:00 2001 From: Dan Nicholson Date: Tue, 30 Aug 2022 08:38:36 -0600 Subject: [PATCH 21/38] main: Factor out sysroot loading It can be useful to parse the options and initialize the sysroot without actually loading it until later. Factor out the sysroot loading to a new `ostree_admin_sysroot_load` and add a new `OSTREE_ADMIN_BUILTIN_FLAG_NO_LOAD` flag to accommodate this. --- src/ostree/ot-main.c | 83 ++++++++++++++++++++++++++------------------ src/ostree/ot-main.h | 6 ++++ 2 files changed, 55 insertions(+), 34 deletions(-) diff --git a/src/ostree/ot-main.c b/src/ostree/ot-main.c index 7a4405a5..770962a4 100644 --- a/src/ostree/ot-main.c +++ b/src/ostree/ot-main.c @@ -579,41 +579,11 @@ on_sysroot_journal_msg (OstreeSysroot *sysroot, } gboolean -ostree_admin_option_context_parse (GOptionContext *context, - const GOptionEntry *main_entries, - int *argc, - char ***argv, - OstreeAdminBuiltinFlags flags, - OstreeCommandInvocation *invocation, - OstreeSysroot **out_sysroot, - GCancellable *cancellable, - GError **error) +ostree_admin_sysroot_load (OstreeSysroot *sysroot, + OstreeAdminBuiltinFlags flags, + GCancellable *cancellable, + GError **error) { - /* Entries are listed in --help output in the order added. We add the - * main entries ourselves so that we can add the --sysroot entry first. */ - - g_option_context_add_main_entries (context, global_admin_entries, NULL); - - if (!ostree_option_context_parse (context, main_entries, argc, argv, - invocation, NULL, cancellable, error)) - return FALSE; - - if (!opt_print_current_dir && (flags & OSTREE_ADMIN_BUILTIN_FLAG_NO_SYSROOT)) - { - g_assert_null (out_sysroot); - /* Early return if no sysroot is requested */ - return TRUE; - } - - g_autoptr(GFile) sysroot_path = NULL; - if (opt_sysroot != NULL) - sysroot_path = g_file_new_for_path (opt_sysroot); - - g_autoptr(OstreeSysroot) sysroot = ostree_sysroot_new (sysroot_path); - if (!ostree_sysroot_initialize (sysroot, error)) - return FALSE; - g_signal_connect (sysroot, "journal-msg", G_CALLBACK (on_sysroot_journal_msg), NULL); - if ((flags & OSTREE_ADMIN_BUILTIN_FLAG_UNLOCKED) == 0) { /* If we're requested to lock the sysroot, first check if we're operating @@ -657,6 +627,51 @@ ostree_admin_option_context_parse (GOptionContext *context, } } + return TRUE; +} + +gboolean +ostree_admin_option_context_parse (GOptionContext *context, + const GOptionEntry *main_entries, + int *argc, + char ***argv, + OstreeAdminBuiltinFlags flags, + OstreeCommandInvocation *invocation, + OstreeSysroot **out_sysroot, + GCancellable *cancellable, + GError **error) +{ + /* Entries are listed in --help output in the order added. We add the + * main entries ourselves so that we can add the --sysroot entry first. */ + + g_option_context_add_main_entries (context, global_admin_entries, NULL); + + if (!ostree_option_context_parse (context, main_entries, argc, argv, + invocation, NULL, cancellable, error)) + return FALSE; + + if (!opt_print_current_dir && (flags & OSTREE_ADMIN_BUILTIN_FLAG_NO_SYSROOT)) + { + g_assert_null (out_sysroot); + /* Early return if no sysroot is requested */ + return TRUE; + } + + g_autoptr(GFile) sysroot_path = NULL; + if (opt_sysroot != NULL) + sysroot_path = g_file_new_for_path (opt_sysroot); + + g_autoptr(OstreeSysroot) sysroot = ostree_sysroot_new (sysroot_path); + if (!ostree_sysroot_initialize (sysroot, error)) + return FALSE; + g_signal_connect (sysroot, "journal-msg", G_CALLBACK (on_sysroot_journal_msg), NULL); + + if (opt_print_current_dir || (flags & OSTREE_ADMIN_BUILTIN_FLAG_NO_LOAD) == 0) + { + if (!ostree_admin_sysroot_load (sysroot, flags, cancellable, error)) + return FALSE; + } + if (opt_print_current_dir) { g_autoptr(GPtrArray) deployments = NULL; diff --git a/src/ostree/ot-main.h b/src/ostree/ot-main.h index b369deb8..e296501a 100644 --- a/src/ostree/ot-main.h +++ b/src/ostree/ot-main.h @@ -36,6 +36,7 @@ typedef enum { OSTREE_ADMIN_BUILTIN_FLAG_SUPERUSER = (1 << 0), OSTREE_ADMIN_BUILTIN_FLAG_UNLOCKED = (1 << 1), OSTREE_ADMIN_BUILTIN_FLAG_NO_SYSROOT = (1 << 2), + OSTREE_ADMIN_BUILTIN_FLAG_NO_LOAD = (1 << 3), } OstreeAdminBuiltinFlags; @@ -91,6 +92,11 @@ gboolean ostree_admin_option_context_parse (GOptionContext *context, OstreeSysroot **out_sysroot, GCancellable *cancellable, GError **error); +gboolean ostree_admin_sysroot_load (OstreeSysroot *sysroot, + OstreeAdminBuiltinFlags flags, + GCancellable *cancellable, + GError **error); + gboolean ostree_ensure_repo_writable (OstreeRepo *repo, GError **error); void ostree_print_gpg_verify_result (OstreeGpgVerifyResult *result); From f3db79e7fa8d469e539b60ceb7e3d790747e530f Mon Sep 17 00:00:00 2001 From: Dan Nicholson Date: Wed, 16 Feb 2022 15:58:58 -0700 Subject: [PATCH 22/38] finalize-staged: Ensure /boot automount doesn't expire If `/boot` is an automount, then the unit will be stopped as soon as the automount expires. That's would defeat the purpose of using systemd to delay finalizing the deployment until shutdown. This is not uncommon as `systemd-gpt-auto-generator` will create an automount unit for `/boot` when it's the EFI System Partition and there's no fstab entry. To ensure that systemd doesn't stop the service early when the `/boot` automount expires, introduce a new unit that holds `/boot` open until it's sent `SIGTERM`. This uses a new `--hold` option for `finalize-staged` that loads but doesn't lock the sysroot. A separate unit is used since we want the process to remain active throughout the finalization run in `ExecStop`. That wouldn't work if it was specified in `ExecStart` in the same unit since it would be killed before the `ExecStop` action was run. Fixes: #2543 --- Makefile-boot.am | 2 + src/boot/ostree-finalize-staged-hold.service | 35 ++++++++ src/boot/ostree-finalize-staged.service | 5 ++ src/ostree/ot-admin-builtin-finalize-staged.c | 68 +++++++++++++-- tests/inst/src/destructive.rs | 7 +- tests/kolainst/destructive/boot-automount.sh | 83 +++++++++++++++++++ 6 files changed, 193 insertions(+), 7 deletions(-) create mode 100644 src/boot/ostree-finalize-staged-hold.service create mode 100755 tests/kolainst/destructive/boot-automount.sh diff --git a/Makefile-boot.am b/Makefile-boot.am index e42e5180..90f98048 100644 --- a/Makefile-boot.am +++ b/Makefile-boot.am @@ -41,6 +41,7 @@ systemdsystemunit_DATA = src/boot/ostree-prepare-root.service \ src/boot/ostree-boot-complete.service \ src/boot/ostree-finalize-staged.service \ src/boot/ostree-finalize-staged.path \ + src/boot/ostree-finalize-staged-hold.service \ $(NULL) systemdtmpfilesdir = $(prefix)/lib/tmpfiles.d dist_systemdtmpfiles_DATA = src/boot/ostree-tmpfiles.conf @@ -70,6 +71,7 @@ EXTRA_DIST += src/boot/dracut/module-setup.sh \ src/boot/ostree-finalize-staged.path \ src/boot/ostree-remount.service \ src/boot/ostree-finalize-staged.service \ + src/boot/ostree-finalize-staged-hold.service \ src/boot/grub2/grub2-15_ostree \ src/boot/grub2/ostree-grub-generator \ $(NULL) diff --git a/src/boot/ostree-finalize-staged-hold.service b/src/boot/ostree-finalize-staged-hold.service new file mode 100644 index 00000000..85b5d543 --- /dev/null +++ b/src/boot/ostree-finalize-staged-hold.service @@ -0,0 +1,35 @@ +# Copyright (C) 2018 Red Hat, Inc. +# Copyright (C) 2022 Endless OS Foundation LLC +# +# This library is free software; you can redistribute it and/or +# modify it under the terms of the GNU Lesser General Public +# License as published by the Free Software Foundation; either +# version 2 of the License, or (at your option) any later version. +# +# This library is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +# Lesser General Public License for more details. +# +# You should have received a copy of the GNU Lesser General Public +# License along with this library. If not, see . + +# See https://github.com/ostreedev/ostree/pull/2543 for background. +[Unit] +Description=Hold /boot Open for OSTree Finalize Staged Deployment +Documentation=man:ostree(1) +ConditionPathExists=/run/ostree-booted +DefaultDependencies=no + +RequiresMountsFor=/sysroot /boot +After=local-fs.target +Before=basic.target final.target + +[Service] +Type=exec + +# This is explicitly run in the root namespace to ensure an automounted +# /boot doesn't time out since autofs doesn't handle mount namespaces. +# +# https://bugzilla.redhat.com/show_bug.cgi?id=2056090 +ExecStart=+/usr/bin/ostree admin finalize-staged --hold diff --git a/src/boot/ostree-finalize-staged.service b/src/boot/ostree-finalize-staged.service index 2f28bbb7..63621ce1 100644 --- a/src/boot/ostree-finalize-staged.service +++ b/src/boot/ostree-finalize-staged.service @@ -29,6 +29,11 @@ Before=basic.target final.target After=systemd-journal-flush.service Conflicts=final.target +# Start the hold unit and ensure it stays active throughout this +# service. +Wants=ostree-finalize-staged-hold.service +After=ostree-finalize-staged-hold.service + [Service] Type=oneshot RemainAfterExit=yes diff --git a/src/ostree/ot-admin-builtin-finalize-staged.c b/src/ostree/ot-admin-builtin-finalize-staged.c index eedffdde..ed4f20d7 100644 --- a/src/ostree/ot-admin-builtin-finalize-staged.c +++ b/src/ostree/ot-admin-builtin-finalize-staged.c @@ -1,5 +1,6 @@ /* * Copyright (C) 2018 Red Hat, Inc. + * Copyright (C) 2022 Endless OS Foundation LLC * * SPDX-License-Identifier: LGPL-2.0+ * @@ -19,8 +20,9 @@ #include "config.h" -#include "config.h" - +#include +#include +#include #include #include "ot-main.h" @@ -32,10 +34,23 @@ #include "ostree-cmd-private.h" #include "ostree.h" +static gboolean opt_hold; + static GOptionEntry options[] = { + { "hold", 0, 0, G_OPTION_ARG_NONE, &opt_hold, "Hold /boot open during finalization", NULL }, { NULL } }; +static gboolean +sigterm_cb (gpointer user_data) +{ + gboolean *running = user_data; + g_print ("Received SIGTERM, exiting\n"); + *running = FALSE; + g_main_context_wakeup (NULL); + return G_SOURCE_REMOVE; +} + /* Called by ostree-finalize-staged.service, and in turn * invokes a cmdprivate function inside the shared library. */ @@ -50,13 +65,56 @@ ot_admin_builtin_finalize_staged (int argc, char **argv, OstreeCommandInvocation g_autoptr(GOptionContext) context = g_option_context_new (""); g_autoptr(OstreeSysroot) sysroot = NULL; + + /* First parse the args without loading the sysroot to see what options are + * set. */ if (!ostree_admin_option_context_parse (context, options, &argc, &argv, - OSTREE_ADMIN_BUILTIN_FLAG_SUPERUSER, + OSTREE_ADMIN_BUILTIN_FLAG_NO_LOAD, invocation, &sysroot, cancellable, error)) return FALSE; - if (!ostree_cmd__private__()->ostree_finalize_staged (sysroot, cancellable, error)) - return FALSE; + if (opt_hold) + { + /* Load the sysroot unlocked so that a separate namespace isn't + * created. */ + if (!ostree_admin_sysroot_load (sysroot, + OSTREE_ADMIN_BUILTIN_FLAG_SUPERUSER | OSTREE_ADMIN_BUILTIN_FLAG_UNLOCKED, + cancellable, error)) + return FALSE; + + /* In case it's an automount, open /boot so that the automount doesn't + * expire until before this process exits. If it did expire and got + * unmounted, the service would be stopped and the deployment would be + * finalized earlier than expected. + */ + int sysroot_fd = ostree_sysroot_get_fd (sysroot); + glnx_autofd int boot_fd = -1; + g_debug ("Opening /boot directory"); + if (!glnx_opendirat (sysroot_fd, "boot", TRUE, &boot_fd, error)) + return FALSE; + + /* We want to keep /boot open until the deployment is finalized during + * system shutdown, so block on SIGTERM under the assumption that it will + * be received when systemd stops the unit. + */ + gboolean running = TRUE; + g_unix_signal_add (SIGTERM, sigterm_cb, &running); + g_print ("Waiting for SIGTERM\n"); + while (running) + g_main_context_iteration (NULL, TRUE); + } + else + { + /* Load the sysroot with the normal flags and actually finalize the + * deployment. */ + if (!ostree_admin_sysroot_load (sysroot, + OSTREE_ADMIN_BUILTIN_FLAG_SUPERUSER, + cancellable, error)) + return FALSE; + + if (!ostree_cmd__private__()->ostree_finalize_staged (sysroot, cancellable, error)) + return FALSE; + } return TRUE; } diff --git a/tests/inst/src/destructive.rs b/tests/inst/src/destructive.rs index da188bcb..d0c9c9dc 100644 --- a/tests/inst/src/destructive.rs +++ b/tests/inst/src/destructive.rs @@ -463,6 +463,7 @@ fn impl_transaction_test>( " systemctl stop rpm-ostreed systemctl stop ostree-finalize-staged + systemctl stop ostree-finalize-staged-hold ostree reset testrepo:${testref} ${booted_commit} rpm-ostree cleanup -pbrm ", @@ -504,7 +505,8 @@ fn impl_transaction_test>( InterruptStrategy::Force(ForceInterruptStrategy::Kill9) => { bash!( "systemctl kill -s KILL rpm-ostreed || true - systemctl kill -s KILL ostree-finalize-staged || true" + systemctl kill -s KILL ostree-finalize-staged || true + systemctl kill -s KILL ostree-finalize-staged-hold || true" )?; live_strategy = Some(strategy); } @@ -528,7 +530,8 @@ fn impl_transaction_test>( InterruptStrategy::Polite(PoliteInterruptStrategy::Stop) => { bash!( "systemctl stop rpm-ostreed || true - systemctl stop ostree-finalize-staged || true" + systemctl stop ostree-finalize-staged || true + systemctl stop ostree-finalize-staged-hold || true" )?; live_strategy = Some(strategy); } diff --git a/tests/kolainst/destructive/boot-automount.sh b/tests/kolainst/destructive/boot-automount.sh new file mode 100755 index 00000000..d6d1732e --- /dev/null +++ b/tests/kolainst/destructive/boot-automount.sh @@ -0,0 +1,83 @@ +#!/bin/bash +# https://github.com/ostreedev/ostree/issues/2543 +set -xeuo pipefail + +. ${KOLA_EXT_DATA}/libinsttest.sh + +case "${AUTOPKGTEST_REBOOT_MARK:-}" in + "") + # Ensure boot is mount point + mountpoint /boot + + # Create an automount unit with an extremely short timeout + cat > /etc/systemd/system/boot.automount <<"EOF" +[Automount] +Where=/boot +TimeoutIdleSec=1 + +[Install] +WantedBy=local-fs.target +EOF + systemctl daemon-reload + systemctl enable boot.automount + + # Unmount /boot, start the automount unit, and ensure the units are + # in the correct states. + umount /boot + systemctl start boot.automount + boot_state=$(systemctl show -P ActiveState boot.mount) + boot_auto_state=$(systemctl show -P ActiveState boot.automount) + assert_streq "${boot_state}" inactive + assert_streq "${boot_auto_state}" active + + # Trigger a new staged deployment and check that the relevant units + # are enabled. + ostree admin deploy --stage --karg-append=somedummykarg=1 "${host_commit}" + rpm-ostree status --json + deployment_staged=$(rpmostree_query_json '.deployments[0].staged') + assert_streq "${deployment_staged}" true + test -f /run/ostree/staged-deployment + finalize_staged_state=$(systemctl show -P ActiveState ostree-finalize-staged.service) + finalize_staged_hold_state=$(systemctl show -P ActiveState ostree-finalize-staged-hold.service) + assert_streq "${finalize_staged_state}" active + assert_streq "${finalize_staged_hold_state}" active + + # Sleep 1 second to ensure the boot automount idle timeout has + # passed and then check that /boot is still mounted. + sleep 1 + boot_state=$(systemctl show -P ActiveState boot.mount) + assert_streq "${boot_state}" active + + /tmp/autopkgtest-reboot "2" + ;; + "2") + rpm-ostree status --json + deployment_staged=$(rpmostree_query_json '.deployments[0].staged') + assert_streq "${deployment_staged}" false + test ! -f /run/ostree/staged-deployment + assert_file_has_content_literal /proc/cmdline somedummykarg=1 + + # Check that the finalize and hold services succeeded in the + # previous boot. Dump them to the test log to help debugging. + prepare_tmpdir + journalctl -b -1 -o short-monotonic \ + -u ostree-finalize-staged.service \ + -u ostree-finalize-staged-hold.service \ + -u boot.mount \ + -u boot.automount \ + > logs.txt + cat logs.txt + assert_file_has_content logs.txt 'ostree-finalize-staged.service: \(Succeeded\|Deactivated successfully\)' + assert_file_has_content logs.txt 'ostree-finalize-staged-hold.service: \(Succeeded\|Deactivated successfully\)' + + # Check that the hold service remained active and kept /boot mounted until + # the finalize service completed. + finalize_stopped=$(journalctl -b -1 -o json -g Stopped -u ostree-finalize-staged.service | tail -n1 | jq -r .__MONOTONIC_TIMESTAMP) + hold_stopping=$(journalctl -b -1 -o json -g Stopping -u ostree-finalize-staged-hold.service | tail -n1 | jq -r .__MONOTONIC_TIMESTAMP) + hold_stopped=$(journalctl -b -1 -o json -g Stopped -u ostree-finalize-staged-hold.service | tail -n1 | jq -r .__MONOTONIC_TIMESTAMP) + boot_unmounting=$(journalctl -b -1 -o json -g Unmounting -u boot.mount | tail -n1 | jq -r .__MONOTONIC_TIMESTAMP) + test "${finalize_stopped}" -lt "${hold_stopping}" + test "${hold_stopped}" -lt "${boot_unmounting}" + ;; + *) fatal "Unexpected AUTOPKGTEST_REBOOT_MARK=${AUTOPKGTEST_REBOOT_MARK}" ;; +esac From 683e4eff08d668f5a5c0cef1f38797ebfff36624 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Tue, 30 Aug 2022 16:23:38 -0400 Subject: [PATCH 23/38] finalize-staged: Don't listen to `SIGTERM`, just let kernel exit us Followup from discussion in https://github.com/ostreedev/ostree/pull/2544#discussion_r958840936 This is more efficient; no need to have the kernel context switch us in at shutdown time just so we can turn around and call `exit()`. --- src/ostree/ot-admin-builtin-finalize-staged.c | 20 +++---------------- 1 file changed, 3 insertions(+), 17 deletions(-) diff --git a/src/ostree/ot-admin-builtin-finalize-staged.c b/src/ostree/ot-admin-builtin-finalize-staged.c index ed4f20d7..e408f447 100644 --- a/src/ostree/ot-admin-builtin-finalize-staged.c +++ b/src/ostree/ot-admin-builtin-finalize-staged.c @@ -41,16 +41,6 @@ static GOptionEntry options[] = { { NULL } }; -static gboolean -sigterm_cb (gpointer user_data) -{ - gboolean *running = user_data; - g_print ("Received SIGTERM, exiting\n"); - *running = FALSE; - g_main_context_wakeup (NULL); - return G_SOURCE_REMOVE; -} - /* Called by ostree-finalize-staged.service, and in turn * invokes a cmdprivate function inside the shared library. */ @@ -94,14 +84,10 @@ ot_admin_builtin_finalize_staged (int argc, char **argv, OstreeCommandInvocation return FALSE; /* We want to keep /boot open until the deployment is finalized during - * system shutdown, so block on SIGTERM under the assumption that it will - * be received when systemd stops the unit. + * system shutdown, so block until we get SIGTERM which systemd will send + * when the unit is stopped. */ - gboolean running = TRUE; - g_unix_signal_add (SIGTERM, sigterm_cb, &running); - g_print ("Waiting for SIGTERM\n"); - while (running) - g_main_context_iteration (NULL, TRUE); + pause (); } else { From 35d6ea88f65724a8fb960245647786916433fc93 Mon Sep 17 00:00:00 2001 From: git-bruh Date: Sat, 3 Sep 2022 12:50:00 +0530 Subject: [PATCH 24/38] ostree-fetcher-curl: check for HTTP2 support before trying to use it --- src/libostree/ostree-fetcher-curl.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/libostree/ostree-fetcher-curl.c b/src/libostree/ostree-fetcher-curl.c index 75038ecc..522eacfb 100644 --- a/src/libostree/ostree-fetcher-curl.c +++ b/src/libostree/ostree-fetcher-curl.c @@ -882,8 +882,10 @@ initiate_next_curl_request (FetcherRequest *req, if (!(self->config_flags & OSTREE_FETCHER_FLAGS_DISABLE_HTTP2)) { #if CURL_AT_LEAST_VERSION(7, 51, 0) - rc = curl_easy_setopt (req->easy, CURLOPT_HTTP_VERSION, CURL_HTTP_VERSION_2_0); - g_assert_cmpint (rc, ==, CURLM_OK); + if ((curl_version_info (CURLVERSION_NOW))->features & CURL_VERSION_HTTP2) { + rc = curl_easy_setopt (req->easy, CURLOPT_HTTP_VERSION, CURL_HTTP_VERSION_2_0); + g_assert_cmpint (rc, ==, CURLM_OK); + } #endif /* https://github.com/curl/curl/blob/curl-7_53_0/docs/examples/http2-download.c */ #if (CURLPIPE_MULTIPLEX > 0) From 49ce9b0289e42f92c0b9cc8bcd6f8420d482b77c Mon Sep 17 00:00:00 2001 From: Nikita Dubrovskii Date: Thu, 1 Sep 2022 16:34:48 +0200 Subject: [PATCH 25/38] s390x: ensure both 'root' and 'boot' luks keys exist --- src/libostree/ostree-bootloader-zipl.c | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/src/libostree/ostree-bootloader-zipl.c b/src/libostree/ostree-bootloader-zipl.c index 0ff350f9..f2c8063e 100644 --- a/src/libostree/ostree-bootloader-zipl.c +++ b/src/libostree/ostree-bootloader-zipl.c @@ -195,7 +195,8 @@ static gboolean _ostree_secure_execution_luks_key_exists (void) { return (access(SECURE_EXECUTION_LUKS_CONFIG, F_OK) == 0 && - (access(SECURE_EXECUTION_LUKS_ROOT_KEY, F_OK) == 0 || access(SECURE_EXECUTION_LUKS_BOOT_KEY, F_OK) == 0)); + access(SECURE_EXECUTION_LUKS_ROOT_KEY, F_OK) == 0 && + access(SECURE_EXECUTION_LUKS_BOOT_KEY, F_OK) == 0); } static gboolean @@ -245,23 +246,21 @@ _ostree_secure_execution_generate_sdboot (gchar *vmlinuz, g_autofree gchar *cmdline_filename = g_strdup_printf ("/proc/%d/fd/%d", self, cmdline.fd); // Copy initramfs to temp file and embed LUKS key and config into it + if (!_ostree_secure_execution_luks_key_exists ()) + return glnx_throw(error, "s390x SE: missing luks keys and config"); g_auto(GLnxTmpfile) ramdisk = { 0, }; - g_autofree gchar *ramdisk_filename = NULL; - if (_ostree_secure_execution_luks_key_exists ()) - { - if (!glnx_open_anonymous_tmpfile (O_RDWR | O_CLOEXEC, &ramdisk, error)) - return glnx_prefix_error(error, "s390x SE: creating new ramdisk"); - ramdisk_filename = g_strdup_printf ("/proc/%d/fd/%d", self, ramdisk.fd); - if (!_ostree_secure_execution_enable_luks (initramfs, ramdisk_filename, error)) - return FALSE; - } + if (!glnx_open_anonymous_tmpfile (O_RDWR | O_CLOEXEC, &ramdisk, error)) + return glnx_prefix_error(error, "s390x SE: creating new ramdisk"); + g_autofree gchar *ramdisk_filename = g_strdup_printf ("/proc/%d/fd/%d", self, ramdisk.fd); + if (!_ostree_secure_execution_enable_luks (initramfs, ramdisk_filename, error)) + return FALSE; g_autoptr(GPtrArray) argv = g_ptr_array_new (); g_ptr_array_add (argv, "genprotimg"); g_ptr_array_add (argv, "-i"); g_ptr_array_add (argv, vmlinuz); g_ptr_array_add (argv, "-r"); - g_ptr_array_add (argv, (ramdisk_filename == NULL) ? initramfs: ramdisk_filename); + g_ptr_array_add (argv, ramdisk_filename); g_ptr_array_add (argv, "-p"); g_ptr_array_add (argv, cmdline_filename); for (guint i = 0; i < keys->len; ++i) @@ -300,7 +299,7 @@ _ostree_secure_execution_call_zipl (GError **error) if (!g_spawn_check_exit_status (status, error)) return glnx_prefix_error(error, "s390x SE: `zipl` failed"); - ot_journal_print(LOG_INFO, "s390x SE: `sd-boot` zipled"); + ot_journal_print(LOG_INFO, "s390x SE: `sdboot` zipled"); return TRUE; } From 273089d9e43baab25d09732c639c22bda087bae9 Mon Sep 17 00:00:00 2001 From: Luca BRUNO Date: Mon, 5 Sep 2022 09:22:26 +0000 Subject: [PATCH 26/38] lib/bootloader: assert invariants This tweaks some invariants checks into full assertions, in order to avoid returning to the caller in case of known invalid states. --- src/libostree/ostree-bootconfig-parser.c | 2 +- src/libostree/ostree-bootloader.c | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/libostree/ostree-bootconfig-parser.c b/src/libostree/ostree-bootconfig-parser.c index e005fab9..08259ebf 100644 --- a/src/libostree/ostree-bootconfig-parser.c +++ b/src/libostree/ostree-bootconfig-parser.c @@ -73,7 +73,7 @@ ostree_bootconfig_parser_parse_at (OstreeBootconfigParser *self, GCancellable *cancellable, GError **error) { - g_return_val_if_fail (!self->parsed, FALSE); + g_assert (!self->parsed); g_autofree char *contents = glnx_file_get_contents_utf8_at (dfd, path, NULL, cancellable, error); if (!contents) diff --git a/src/libostree/ostree-bootloader.c b/src/libostree/ostree-bootloader.c index 785fd233..790bf502 100644 --- a/src/libostree/ostree-bootloader.c +++ b/src/libostree/ostree-bootloader.c @@ -31,7 +31,7 @@ _ostree_bootloader_query (OstreeBootloader *self, GCancellable *cancellable, GError **error) { - g_return_val_if_fail (OSTREE_IS_BOOTLOADER (self), FALSE); + g_assert (OSTREE_IS_BOOTLOADER (self)); return OSTREE_BOOTLOADER_GET_IFACE (self)->query (self, out_is_active, cancellable, error); } @@ -44,7 +44,7 @@ _ostree_bootloader_query (OstreeBootloader *self, const char * _ostree_bootloader_get_name (OstreeBootloader *self) { - g_return_val_if_fail (OSTREE_IS_BOOTLOADER (self), NULL); + g_assert (OSTREE_IS_BOOTLOADER (self)); return OSTREE_BOOTLOADER_GET_IFACE (self)->get_name (self); } @@ -56,7 +56,7 @@ _ostree_bootloader_write_config (OstreeBootloader *self, GCancellable *cancellable, GError **error) { - g_return_val_if_fail (OSTREE_IS_BOOTLOADER (self), FALSE); + g_assert (OSTREE_IS_BOOTLOADER (self)); return OSTREE_BOOTLOADER_GET_IFACE (self)->write_config (self, bootversion, new_deployments, @@ -69,7 +69,7 @@ _ostree_bootloader_post_bls_sync (OstreeBootloader *self, GCancellable *cancellable, GError **error) { - g_return_val_if_fail (OSTREE_IS_BOOTLOADER (self), FALSE); + g_assert (OSTREE_IS_BOOTLOADER (self)); if (OSTREE_BOOTLOADER_GET_IFACE (self)->post_bls_sync) return OSTREE_BOOTLOADER_GET_IFACE (self)->post_bls_sync (self, bootversion, cancellable, error); @@ -80,7 +80,7 @@ _ostree_bootloader_post_bls_sync (OstreeBootloader *self, gboolean _ostree_bootloader_is_atomic (OstreeBootloader *self) { - g_return_val_if_fail (OSTREE_IS_BOOTLOADER (self), FALSE); + g_assert (OSTREE_IS_BOOTLOADER (self)); if (OSTREE_BOOTLOADER_GET_IFACE (self)->is_atomic) return OSTREE_BOOTLOADER_GET_IFACE (self)->is_atomic (self); From 769ac686f158197103f42b01a1e3b3a97d84ebc7 Mon Sep 17 00:00:00 2001 From: Nikita Dubrovskii Date: Mon, 5 Sep 2022 12:08:35 +0200 Subject: [PATCH 27/38] s390x: simplify 's390x-se-luks-gencpio' script --- src/libostree/ostree-bootloader-zipl.c | 43 ++++++++++++++++---------- src/libostree/s390x-se-luks-gencpio | 18 ++++------- 2 files changed, 33 insertions(+), 28 deletions(-) diff --git a/src/libostree/ostree-bootloader-zipl.c b/src/libostree/ostree-bootloader-zipl.c index f2c8063e..fc0614c0 100644 --- a/src/libostree/ostree-bootloader-zipl.c +++ b/src/libostree/ostree-bootloader-zipl.c @@ -200,11 +200,28 @@ _ostree_secure_execution_luks_key_exists (void) } static gboolean -_ostree_secure_execution_enable_luks(const gchar *oldramfs, - const gchar *newramfs, - GError **error) +_ostree_secure_execution_generate_initrd (const gchar *initrd, + GLnxTmpfile *out_initrd, + gchar **out_initrdname, + GError **error) { - const char *const argv[] = {SECURE_EXECUTION_RAMDISK_TOOL, oldramfs, newramfs, NULL}; + if (!_ostree_secure_execution_luks_key_exists ()) + return glnx_throw (error, "s390x SE: missing luks keys and config"); + + + if (!glnx_open_anonymous_tmpfile (O_RDWR | O_CLOEXEC, out_initrd, error)) + return glnx_prefix_error (error, "s390x SE: opening new ramdisk"); + { + glnx_autofd int fd = -1; + glnx_openat_rdonly (AT_FDCWD, initrd, TRUE, &fd, error); + if (glnx_regfile_copy_bytes (fd, out_initrd->fd, (off_t) -1) < 0) + return glnx_throw_errno_prefix (error, "s390x SE: copying ramdisk"); + } + + g_autofree gchar *tmpdir = g_mkdtemp (g_strdup ("/var/tmp/se-initramfs-XXXXXX")); + + *out_initrdname = g_strdup_printf ("/proc/%d/fd/%d", getpid (), out_initrd->fd); + const char *const argv[] = {SECURE_EXECUTION_RAMDISK_TOOL, *out_initrdname, tmpdir, NULL}; g_autofree gchar *out = NULL; g_autofree gchar *err = NULL; int status = 0; @@ -219,7 +236,7 @@ _ostree_secure_execution_enable_luks(const gchar *oldramfs, return glnx_prefix_error(error, "s390x SE: `%s` failed", SECURE_EXECUTION_RAMDISK_TOOL); } - ot_journal_print(LOG_INFO, "s390x SE: luks key added to initrd"); + ot_journal_print(LOG_INFO, "s390x SE: luks keys added to initrd"); return TRUE; } @@ -235,24 +252,18 @@ _ostree_secure_execution_generate_sdboot (gchar *vmlinuz, ot_journal_print(LOG_INFO, "s390x SE: initrd: %s", initramfs); ot_journal_print(LOG_INFO, "s390x SE: kargs: %s", options); - pid_t self = getpid(); - // Store kernel options to temp file, so `genprotimg` can later embed it g_auto(GLnxTmpfile) cmdline = { 0, }; if (!glnx_open_anonymous_tmpfile (O_RDWR | O_CLOEXEC, &cmdline, error)) - return glnx_prefix_error(error, "s390x SE: opening cmdline file"); + return glnx_prefix_error (error, "s390x SE: opening cmdline file"); if (glnx_loop_write (cmdline.fd, options, strlen (options)) < 0) return glnx_throw_errno_prefix (error, "s390x SE: writting cmdline file"); - g_autofree gchar *cmdline_filename = g_strdup_printf ("/proc/%d/fd/%d", self, cmdline.fd); + g_autofree gchar *cmdline_filename = g_strdup_printf ("/proc/%d/fd/%d", getpid (), cmdline.fd); - // Copy initramfs to temp file and embed LUKS key and config into it - if (!_ostree_secure_execution_luks_key_exists ()) - return glnx_throw(error, "s390x SE: missing luks keys and config"); + // Copy initramfs to temp file and embed LUKS keys & config into it g_auto(GLnxTmpfile) ramdisk = { 0, }; - if (!glnx_open_anonymous_tmpfile (O_RDWR | O_CLOEXEC, &ramdisk, error)) - return glnx_prefix_error(error, "s390x SE: creating new ramdisk"); - g_autofree gchar *ramdisk_filename = g_strdup_printf ("/proc/%d/fd/%d", self, ramdisk.fd); - if (!_ostree_secure_execution_enable_luks (initramfs, ramdisk_filename, error)) + g_autofree gchar *ramdisk_filename = NULL; + if (!_ostree_secure_execution_generate_initrd (initramfs, &ramdisk, &ramdisk_filename, error)) return FALSE; g_autoptr(GPtrArray) argv = g_ptr_array_new (); diff --git a/src/libostree/s390x-se-luks-gencpio b/src/libostree/s390x-se-luks-gencpio index e821e2fe..4e5d7ad8 100755 --- a/src/libostree/s390x-se-luks-gencpio +++ b/src/libostree/s390x-se-luks-gencpio @@ -1,22 +1,16 @@ #!/bin/bash -# This script creates new initramdisk with LUKS config within +# This script appends LUKS keys and config to initrd set -euo pipefail -old_initrd=$1 -new_initrd=$2 -currdir=$PWD - -# Copying existing initramdisk -cp ${old_initrd} ${new_initrd} +initrd=$1 +tmpdir=$2 # Appending LUKS root keys and crypttab config to the end of initrd -workdir=$(mktemp -d -p /tmp se-initramfs-XXXXXX) -cd ${workdir} +cd ${tmpdir} mkdir -p etc/luks cp -f /etc/luks/* etc/luks/ cp -f /etc/crypttab etc/ -find . -mindepth 1 | cpio --quiet -H newc -o | gzip -9 -n >> ${new_initrd} +find . -mindepth 1 | cpio --quiet -H newc -o | gzip -9 -n >> ${initrd} # Cleanup -cd ${currdir} -rm -rf ${workdir} +rm -rf etc/ From 642efe924f71f67e16c2782d725968afb012c3b6 Mon Sep 17 00:00:00 2001 From: Luca BRUNO Date: Wed, 7 Sep 2022 12:33:37 +0000 Subject: [PATCH 28/38] lib/mtree: drop redundant name checks This drops several NULL checks against filename input argument. Those checks are both redundant (as filename validation already checks for that) and dangerous (as they return early without setting an error value). --- src/libostree/ostree-mutable-tree.c | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/libostree/ostree-mutable-tree.c b/src/libostree/ostree-mutable-tree.c index 58dd3c4c..60e3be2f 100644 --- a/src/libostree/ostree-mutable-tree.c +++ b/src/libostree/ostree-mutable-tree.c @@ -303,8 +303,6 @@ ostree_mutable_tree_replace_file (OstreeMutableTree *self, const char *checksum, GError **error) { - g_return_val_if_fail (name != NULL, FALSE); - if (!ot_util_filename_validate (name, error)) return FALSE; @@ -338,8 +336,6 @@ ostree_mutable_tree_remove (OstreeMutableTree *self, gboolean allow_noent, GError **error) { - g_return_val_if_fail (name != NULL, FALSE); - if (!ot_util_filename_validate (name, error)) return FALSE; @@ -374,8 +370,6 @@ ostree_mutable_tree_ensure_dir (OstreeMutableTree *self, OstreeMutableTree **out_subdir, GError **error) { - g_return_val_if_fail (name != NULL, FALSE); - if (!ot_util_filename_validate (name, error)) return FALSE; From c6a53ef6e8f9491633b0d494991f94c629791e1d Mon Sep 17 00:00:00 2001 From: Nikita Dubrovskii Date: Mon, 5 Sep 2022 14:54:03 +0200 Subject: [PATCH 29/38] s390x: use 'libarchive' to modify initrd in SE case --- Makefile-libostree.am | 6 +- src/libostree/ostree-bootloader-zipl.c | 100 ++++++++++++++++------ src/libostree/ostree-libarchive-private.h | 2 + src/libostree/s390x-se-luks-gencpio | 16 ---- 4 files changed, 75 insertions(+), 49 deletions(-) delete mode 100755 src/libostree/s390x-se-luks-gencpio diff --git a/Makefile-libostree.am b/Makefile-libostree.am index f93f712a..ea5e8ed5 100644 --- a/Makefile-libostree.am +++ b/Makefile-libostree.am @@ -284,12 +284,8 @@ EXTRA_DIST += src/libostree/README-gpg src/libostree/bupsplit.h \ src/libostree/ostree-enumtypes.c.template \ src/libostree/ostree-deployment-private.h \ src/libostree/ostree-repo-deprecated.h \ - src/libostree/ostree-version.h \ - src/libostree/s390x-se-luks-gencpio + src/libostree/ostree-version.h install-mkdir-remotes-d-hook: mkdir -p $(DESTDIR)$(sysconfdir)/ostree/remotes.d INSTALL_DATA_HOOKS += install-mkdir-remotes-d-hook - -# Secure Execution: script for creating new initramdisk with LUKS key and config -pkglibexec_SCRIPTS += src/libostree/s390x-se-luks-gencpio diff --git a/src/libostree/ostree-bootloader-zipl.c b/src/libostree/ostree-bootloader-zipl.c index fc0614c0..05a3b2ac 100644 --- a/src/libostree/ostree-bootloader-zipl.c +++ b/src/libostree/ostree-bootloader-zipl.c @@ -20,6 +20,7 @@ #include "ostree-sysroot-private.h" #include "ostree-bootloader-zipl.h" #include "ostree-deployment-private.h" +#include "ostree-libarchive-private.h" #include "otutil.h" #include #include @@ -34,7 +35,10 @@ #define SECURE_EXECUTION_LUKS_ROOT_KEY "/etc/luks/root" #define SECURE_EXECUTION_LUKS_BOOT_KEY "/etc/luks/boot" #define SECURE_EXECUTION_LUKS_CONFIG "/etc/crypttab" -#define SECURE_EXECUTION_RAMDISK_TOOL PKGLIBEXECDIR "/s390x-se-luks-gencpio" + +#if !(defined HAVE_LIBARCHIVE) && defined(__s390x__) +#error libarchive is required for s390x +#endif /* This is specific to zipl today, but in the future we could also * use it for the grub2-mkconfig case. @@ -199,16 +203,72 @@ _ostree_secure_execution_luks_key_exists (void) access(SECURE_EXECUTION_LUKS_BOOT_KEY, F_OK) == 0); } +static gboolean +_ostree_secure_execution_append_luks_keys (int initrd_fd, + GCancellable *cancellable, + GError **error) +{ +#ifdef HAVE_LIBARCHIVE + // appending cpio gzip archive with LUKS keys + g_autoptr(OtAutoArchiveWrite) a = archive_write_new (); + g_assert (a != NULL); + + if (archive_write_set_format_cpio_newc (a) != 0 || + archive_write_add_filter_gzip (a) != 0 || + archive_write_open_fd(a, initrd_fd) != 0) + return glnx_prefix_error (error, "s390x SE: initing cpio: %s", archive_error_string (a)); + + const char *files[] = {"/etc", "/etc/luks", SECURE_EXECUTION_LUKS_CONFIG, SECURE_EXECUTION_LUKS_BOOT_KEY, SECURE_EXECUTION_LUKS_ROOT_KEY}; + for (uint i = 0; i != G_N_ELEMENTS (files); ++i) + { + const char *path = files[i]; + struct stat st; + if (stat(path, &st) != 0) + glnx_throw_errno_prefix (error, "s390x SE: stat(%s) failed", path); + + g_autoptr(OtArchiveEntry) ae = archive_entry_new (); + g_assert (ae != NULL); + + archive_entry_copy_stat (ae, &st); + archive_entry_set_pathname (ae, path); + if (archive_write_header (a, ae) != 0) + glnx_prefix_error (error, "s390x SE: writing cpio header: %s", archive_error_string (a)); + + if (S_ISREG (st.st_mode)) + { + ot_journal_print(LOG_INFO, "s390x SE: appending %s to initrd", path); + glnx_autofd int fd = -1; + if (!glnx_openat_rdonly (AT_FDCWD, path, TRUE, &fd, error)) + return glnx_prefix_error (error, "s390x SE: opening %s", path); + g_autoptr(GBytes) data = glnx_fd_readall_bytes (fd, cancellable, error); + if (!data) + return glnx_prefix_error (error, "s390x SE: reading %s", path); + + gsize size = 0; + const char *ptr = (const char *) g_bytes_get_data (data, &size); + ssize_t written = archive_write_data (a, ptr, size); + if (written == -1) + return glnx_prefix_error (error, "s390x SE: writing cpio entry: %s", archive_error_string (a)); + if (written != size) + return glnx_prefix_error (error, "s390x SE: writing cpio entry %zd != %zu", written, size); + } + } + ot_journal_print(LOG_INFO, "s390x SE: luks keys added to initrd"); + return TRUE; + #else + return glnx_throw (error, "'libarchive' is required for s390x"); + #endif +} + static gboolean _ostree_secure_execution_generate_initrd (const gchar *initrd, GLnxTmpfile *out_initrd, - gchar **out_initrdname, + GCancellable *cancellable, GError **error) { if (!_ostree_secure_execution_luks_key_exists ()) return glnx_throw (error, "s390x SE: missing luks keys and config"); - if (!glnx_open_anonymous_tmpfile (O_RDWR | O_CLOEXEC, out_initrd, error)) return glnx_prefix_error (error, "s390x SE: opening new ramdisk"); { @@ -218,26 +278,7 @@ _ostree_secure_execution_generate_initrd (const gchar *initrd, return glnx_throw_errno_prefix (error, "s390x SE: copying ramdisk"); } - g_autofree gchar *tmpdir = g_mkdtemp (g_strdup ("/var/tmp/se-initramfs-XXXXXX")); - - *out_initrdname = g_strdup_printf ("/proc/%d/fd/%d", getpid (), out_initrd->fd); - const char *const argv[] = {SECURE_EXECUTION_RAMDISK_TOOL, *out_initrdname, tmpdir, NULL}; - g_autofree gchar *out = NULL; - g_autofree gchar *err = NULL; - int status = 0; - if (!g_spawn_sync (NULL, (char**)argv, NULL, G_SPAWN_SEARCH_PATH, - NULL, NULL, &out, &err, &status, error)) - return glnx_prefix_error(error, "s390x SE: spawning %s", SECURE_EXECUTION_RAMDISK_TOOL); - - if (!g_spawn_check_exit_status (status, error)) - { - g_printerr("s390x SE: `%s` stdout: %s\n", SECURE_EXECUTION_RAMDISK_TOOL, out); - g_printerr("s390x SE: `%s` stderr: %s\n", SECURE_EXECUTION_RAMDISK_TOOL, err); - return glnx_prefix_error(error, "s390x SE: `%s` failed", SECURE_EXECUTION_RAMDISK_TOOL); - } - - ot_journal_print(LOG_INFO, "s390x SE: luks keys added to initrd"); - return TRUE; + return _ostree_secure_execution_append_luks_keys (out_initrd->fd, cancellable, error); } static gboolean @@ -245,6 +286,7 @@ _ostree_secure_execution_generate_sdboot (gchar *vmlinuz, gchar *initramfs, gchar *options, GPtrArray *keys, + GCancellable *cancellable, GError **error) { g_assert (vmlinuz && initramfs && options && keys && keys->len); @@ -252,19 +294,21 @@ _ostree_secure_execution_generate_sdboot (gchar *vmlinuz, ot_journal_print(LOG_INFO, "s390x SE: initrd: %s", initramfs); ot_journal_print(LOG_INFO, "s390x SE: kargs: %s", options); + pid_t self = getpid (); + // Store kernel options to temp file, so `genprotimg` can later embed it g_auto(GLnxTmpfile) cmdline = { 0, }; if (!glnx_open_anonymous_tmpfile (O_RDWR | O_CLOEXEC, &cmdline, error)) return glnx_prefix_error (error, "s390x SE: opening cmdline file"); if (glnx_loop_write (cmdline.fd, options, strlen (options)) < 0) - return glnx_throw_errno_prefix (error, "s390x SE: writting cmdline file"); - g_autofree gchar *cmdline_filename = g_strdup_printf ("/proc/%d/fd/%d", getpid (), cmdline.fd); + return glnx_throw_errno_prefix (error, "s390x SE: writing cmdline file"); + g_autofree gchar *cmdline_filename = g_strdup_printf ("/proc/%d/fd/%d", self, cmdline.fd); // Copy initramfs to temp file and embed LUKS keys & config into it g_auto(GLnxTmpfile) ramdisk = { 0, }; - g_autofree gchar *ramdisk_filename = NULL; - if (!_ostree_secure_execution_generate_initrd (initramfs, &ramdisk, &ramdisk_filename, error)) + if (!_ostree_secure_execution_generate_initrd (initramfs, &ramdisk, cancellable, error)) return FALSE; + g_autofree gchar *ramdisk_filename = g_strdup_printf ("/proc/%d/fd/%d", self, ramdisk.fd); g_autoptr(GPtrArray) argv = g_ptr_array_new (); g_ptr_array_add (argv, "genprotimg"); @@ -328,7 +372,7 @@ _ostree_secure_execution_enable (OstreeBootloaderZipl *self, gboolean rc = _ostree_secure_execution_mount (error) && _ostree_secure_execution_get_bls_config (self, bootversion, &vmlinuz, &initramfs, &options, cancellable, error) && - _ostree_secure_execution_generate_sdboot (vmlinuz, initramfs, options, keys, error) && + _ostree_secure_execution_generate_sdboot (vmlinuz, initramfs, options, keys, cancellable, error) && _ostree_secure_execution_call_zipl (error) && _ostree_secure_execution_umount (error); diff --git a/src/libostree/ostree-libarchive-private.h b/src/libostree/ostree-libarchive-private.h index 6e6daddb..4eaeaedb 100644 --- a/src/libostree/ostree-libarchive-private.h +++ b/src/libostree/ostree-libarchive-private.h @@ -38,6 +38,8 @@ G_BEGIN_DECLS #ifdef HAVE_LIBARCHIVE typedef struct archive OtAutoArchiveWrite; G_DEFINE_AUTOPTR_CLEANUP_FUNC(OtAutoArchiveWrite, archive_write_free) +typedef struct archive_entry OtArchiveEntry; +G_DEFINE_AUTOPTR_CLEANUP_FUNC(OtArchiveEntry, archive_entry_free) typedef struct archive OtAutoArchiveRead; G_DEFINE_AUTOPTR_CLEANUP_FUNC(OtAutoArchiveRead, archive_read_free) diff --git a/src/libostree/s390x-se-luks-gencpio b/src/libostree/s390x-se-luks-gencpio deleted file mode 100755 index 4e5d7ad8..00000000 --- a/src/libostree/s390x-se-luks-gencpio +++ /dev/null @@ -1,16 +0,0 @@ -#!/bin/bash -# This script appends LUKS keys and config to initrd -set -euo pipefail - -initrd=$1 -tmpdir=$2 - -# Appending LUKS root keys and crypttab config to the end of initrd -cd ${tmpdir} -mkdir -p etc/luks -cp -f /etc/luks/* etc/luks/ -cp -f /etc/crypttab etc/ -find . -mindepth 1 | cpio --quiet -H newc -o | gzip -9 -n >> ${initrd} - -# Cleanup -rm -rf etc/ From 542b79cfeb3d6475f759ebd633cce91422d531ec Mon Sep 17 00:00:00 2001 From: Luca BRUNO Date: Thu, 8 Sep 2022 13:53:05 +0000 Subject: [PATCH 30/38] otutil: add error handling to variant builders This enhances a bunch of helpers related to GVariant building, in order to properly handle errors and avoid some potential cases of unexpected NULL results. --- .../ostree-repo-static-delta-compilation.c | 13 ++++++++----- src/libotutil/ot-variant-builder.c | 17 +++++++---------- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/src/libostree/ostree-repo-static-delta-compilation.c b/src/libostree/ostree-repo-static-delta-compilation.c index cf232c11..28b42139 100644 --- a/src/libostree/ostree-repo-static-delta-compilation.c +++ b/src/libostree/ostree-repo-static-delta-compilation.c @@ -1369,7 +1369,6 @@ ostree_repo_static_delta_generate (OstreeRepo *self, g_autoptr(GPtrArray) builder_parts = g_ptr_array_new_with_free_func ((GDestroyNotify)ostree_static_delta_part_builder_unref); g_autoptr(GPtrArray) builder_fallback_objects = g_ptr_array_new_with_free_func ((GDestroyNotify)g_variant_unref); g_auto(GLnxTmpfile) descriptor_tmpf = { 0, }; - g_autoptr(OtVariantBuilder) descriptor_builder = NULL; const char *opt_sign_name; const char **opt_key_ids; @@ -1456,7 +1455,10 @@ ostree_repo_static_delta_generate (OstreeRepo *self, &descriptor_tmpf, error)) return FALSE; - descriptor_builder = ot_variant_builder_new (G_VARIANT_TYPE (OSTREE_STATIC_DELTA_SUPERBLOCK_FORMAT), descriptor_tmpf.fd); + g_autoptr(OtVariantBuilder) descriptor_builder = + ot_variant_builder_new (G_VARIANT_TYPE (OSTREE_STATIC_DELTA_SUPERBLOCK_FORMAT), + descriptor_tmpf.fd); + g_assert (descriptor_builder != NULL); /* Open the metadata dict */ if (!ot_variant_builder_open (descriptor_builder, G_VARIANT_TYPE ("a{sv}"), error)) @@ -1603,7 +1605,6 @@ ostree_repo_static_delta_generate (OstreeRepo *self, const gchar *signature_key = NULL; g_autoptr(GVariantBuilder) signature_builder = NULL; g_auto(GLnxTmpfile) descriptor_sign_tmpf = { 0, }; - g_autoptr(OtVariantBuilder) descriptor_sign_builder = NULL; lseek (descriptor_tmpf.fd, 0, SEEK_SET); tmpdata = glnx_fd_readall_bytes (descriptor_tmpf.fd, cancellable, error); @@ -1640,8 +1641,10 @@ ostree_repo_static_delta_generate (OstreeRepo *self, &descriptor_sign_tmpf, error)) return FALSE; - descriptor_sign_builder = ot_variant_builder_new (G_VARIANT_TYPE (OSTREE_STATIC_DELTA_SIGNED_FORMAT), - descriptor_sign_tmpf.fd); + g_autoptr(OtVariantBuilder) descriptor_sign_builder = + ot_variant_builder_new (G_VARIANT_TYPE (OSTREE_STATIC_DELTA_SIGNED_FORMAT), + descriptor_sign_tmpf.fd); + g_assert (descriptor_sign_builder != NULL); if (!ot_variant_builder_add (descriptor_sign_builder, error, "t", GUINT64_TO_BE (OSTREE_STATIC_DELTA_SIGNED_MAGIC))) diff --git a/src/libotutil/ot-variant-builder.c b/src/libotutil/ot-variant-builder.c index 4d3dcbe5..955bf2ee 100644 --- a/src/libotutil/ot-variant-builder.c +++ b/src/libotutil/ot-variant-builder.c @@ -758,11 +758,9 @@ struct _OtVariantBuilder { static OtVariantBuilderInfo * ot_variant_builder_info_new (OtVariantBuilder *builder, const GVariantType *type) { - OtVariantBuilderInfo *info; + g_assert (g_variant_type_is_container (type)); - g_return_val_if_fail (g_variant_type_is_container (type), NULL); - - info = (OtVariantBuilderInfo *) g_slice_new0 (OtVariantBuilderInfo); + OtVariantBuilderInfo *info = (OtVariantBuilderInfo *) g_slice_new0 (OtVariantBuilderInfo); info->builder = builder; info->type = g_variant_type_copy (type); @@ -843,11 +841,9 @@ OtVariantBuilder * ot_variant_builder_new (const GVariantType *type, int fd) { - OtVariantBuilder *builder; + g_assert (g_variant_type_is_container (type)); - g_return_val_if_fail (g_variant_type_is_container (type), NULL); - - builder = (OtVariantBuilder *) g_slice_new0 (OtVariantBuilder); + OtVariantBuilder *builder = (OtVariantBuilder *) g_slice_new0 (OtVariantBuilder); builder->head = ot_variant_builder_info_new (builder, type); builder->ref_count = 1; @@ -1083,7 +1079,6 @@ ot_variant_builder_open (OtVariantBuilder *builder, GError **error) { OtVariantBuilderInfo *info = builder->head; - OtVariantBuilderInfo *new_info; g_assert (info->n_children < info->max_items); g_assert (!info->expected_type || @@ -1096,7 +1091,9 @@ ot_variant_builder_open (OtVariantBuilder *builder, if (!ot_variant_builder_pre_add (info, type, error)) return FALSE; - new_info = ot_variant_builder_info_new (builder, type); + OtVariantBuilderInfo *new_info = ot_variant_builder_info_new (builder, type); + g_assert (new_info != NULL); + new_info->parent = info; /* push the prev_item_type down into the subcontainer */ From ace973186c7f092a8638285b7a90fe30e080e7aa Mon Sep 17 00:00:00 2001 From: Luca BRUNO Date: Fri, 9 Sep 2022 13:20:43 +0000 Subject: [PATCH 31/38] lib/sign: convert invariant checks to assertions This converts several invariant checks to asserts. Most of the functions in this file were already using assertions, so this aligns the remaining few outliers to the rest. --- src/libostree/ostree-sign.c | 60 +++++++++++++++++++++++++------------ 1 file changed, 41 insertions(+), 19 deletions(-) diff --git a/src/libostree/ostree-sign.c b/src/libostree/ostree-sign.c index 16e5d0f1..587d7164 100644 --- a/src/libostree/ostree-sign.c +++ b/src/libostree/ostree-sign.c @@ -96,8 +96,11 @@ ostree_sign_default_init (OstreeSignInterface *iface) const gchar * ostree_sign_metadata_key (OstreeSign *self) { + g_assert (OSTREE_IS_SIGN (self)); + + if (OSTREE_SIGN_GET_IFACE (self)->metadata_key == NULL) + return NULL; - g_return_val_if_fail (OSTREE_SIGN_GET_IFACE (self)->metadata_key != NULL, NULL); return OSTREE_SIGN_GET_IFACE (self)->metadata_key (self); } @@ -116,8 +119,11 @@ ostree_sign_metadata_key (OstreeSign *self) const gchar * ostree_sign_metadata_format (OstreeSign *self) { + g_assert (OSTREE_IS_SIGN (self)); + + if (OSTREE_SIGN_GET_IFACE (self)->metadata_format == NULL) + return NULL; - g_return_val_if_fail (OSTREE_SIGN_GET_IFACE (self)->metadata_format != NULL, NULL); return OSTREE_SIGN_GET_IFACE (self)->metadata_format (self); } @@ -136,7 +142,8 @@ gboolean ostree_sign_clear_keys (OstreeSign *self, GError **error) { - g_return_val_if_fail (OSTREE_IS_SIGN (self), FALSE); + g_assert (OSTREE_IS_SIGN (self)); + if (OSTREE_SIGN_GET_IFACE (self)->clear_keys == NULL) return glnx_throw (error, "not implemented"); @@ -163,7 +170,8 @@ ostree_sign_set_sk (OstreeSign *self, GVariant *secret_key, GError **error) { - g_return_val_if_fail (OSTREE_IS_SIGN (self), FALSE); + g_assert (OSTREE_IS_SIGN (self)); + if (OSTREE_SIGN_GET_IFACE (self)->set_sk == NULL) return glnx_throw (error, "not implemented"); @@ -191,7 +199,8 @@ ostree_sign_set_pk (OstreeSign *self, GVariant *public_key, GError **error) { - g_return_val_if_fail (OSTREE_IS_SIGN (self), FALSE); + g_assert (OSTREE_IS_SIGN (self)); + if (OSTREE_SIGN_GET_IFACE (self)->set_pk == NULL) return glnx_throw (error, "not implemented"); @@ -219,7 +228,8 @@ ostree_sign_add_pk (OstreeSign *self, GVariant *public_key, GError **error) { - g_return_val_if_fail (OSTREE_IS_SIGN (self), FALSE); + g_assert (OSTREE_IS_SIGN (self)); + if (OSTREE_SIGN_GET_IFACE (self)->add_pk == NULL) return glnx_throw (error, "not implemented"); @@ -258,7 +268,8 @@ ostree_sign_load_pk (OstreeSign *self, GVariant *options, GError **error) { - g_return_val_if_fail (OSTREE_IS_SIGN (self), FALSE); + g_assert (OSTREE_IS_SIGN (self)); + if (OSTREE_SIGN_GET_IFACE (self)->load_pk == NULL) return glnx_throw (error, "not implemented"); @@ -290,8 +301,8 @@ ostree_sign_data (OstreeSign *self, GCancellable *cancellable, GError **error) { + g_assert (OSTREE_IS_SIGN (self)); - g_return_val_if_fail (OSTREE_IS_SIGN (self), FALSE); if (OSTREE_SIGN_GET_IFACE (self)->data == NULL) return glnx_throw (error, "not implemented"); @@ -324,7 +335,8 @@ ostree_sign_data_verify (OstreeSign *self, char **out_success_message, GError **error) { - g_return_val_if_fail (OSTREE_IS_SIGN (self), FALSE); + g_assert (OSTREE_IS_SIGN (self)); + if (OSTREE_SIGN_GET_IFACE (self)->data_verify == NULL) return glnx_throw (error, "not implemented"); @@ -337,9 +349,13 @@ ostree_sign_data_verify (OstreeSign *self, static GVariant * _sign_detached_metadata_append (OstreeSign *self, GVariant *existing_metadata, - GBytes *signature_bytes) + GBytes *signature_bytes, + GError **error) { - g_return_val_if_fail (signature_bytes != NULL, FALSE); + g_assert (OSTREE_IS_SIGN (self)); + + if (signature_bytes == NULL) + return glnx_null_throw (error, "Invalid NULL signature bytes"); GVariantDict metadata_dict; g_autoptr(GVariant) signature_data = NULL; @@ -395,7 +411,7 @@ ostree_sign_commit_verify (OstreeSign *self, GError **error) { - g_return_val_if_fail (OSTREE_IS_SIGN (self), FALSE); + g_assert (OSTREE_IS_SIGN (self)); g_autoptr(GVariant) commit_variant = NULL; /* Load the commit */ @@ -447,10 +463,12 @@ ostree_sign_commit_verify (OstreeSign *self, const gchar * ostree_sign_get_name (OstreeSign *self) { - g_return_val_if_fail (OSTREE_IS_SIGN (self), NULL); - g_return_val_if_fail (OSTREE_SIGN_GET_IFACE (self)->get_name != NULL, NULL); + g_assert (OSTREE_IS_SIGN (self)); - return OSTREE_SIGN_GET_IFACE (self)->get_name (self); + if (OSTREE_SIGN_GET_IFACE (self)->get_name == NULL) + return NULL; + + return OSTREE_SIGN_GET_IFACE (self)->get_name (self); } /** @@ -503,7 +521,9 @@ ostree_sign_commit (OstreeSign *self, return glnx_prefix_error (error, "Not able to sign the cobject"); new_metadata = - _sign_detached_metadata_append (self, old_metadata, signature); + _sign_detached_metadata_append (self, old_metadata, signature, error); + if (new_metadata == NULL) + return FALSE; if (!ostree_repo_write_commit_detached_metadata (repo, commit_checksum, @@ -603,8 +623,8 @@ ostree_sign_summary (OstreeSign *self, GCancellable *cancellable, GError **error) { - g_return_val_if_fail (OSTREE_IS_SIGN (self), FALSE); - g_return_val_if_fail (OSTREE_IS_REPO (repo), FALSE); + g_assert (OSTREE_IS_SIGN (self)); + g_assert (OSTREE_IS_REPO (repo)); g_autoptr(GVariant) normalized = NULL; g_autoptr(GBytes) summary_data = NULL; @@ -653,7 +673,9 @@ ostree_sign_summary (OstreeSign *self, g_autoptr(GVariant) old_metadata = g_steal_pointer (&metadata); metadata = - _sign_detached_metadata_append (self, old_metadata, signature); + _sign_detached_metadata_append (self, old_metadata, signature, error); + if (metadata == NULL) + return FALSE; } g_variant_iter_free (iter); From 565ad4a1b4bf0b81c85c39050f608480152444da Mon Sep 17 00:00:00 2001 From: Andrea Perotti Date: Sat, 17 Sep 2022 11:00:46 +0200 Subject: [PATCH 32/38] Fix recursive git archive reference Broken link for "recursive git archive" example --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index d0232e7f..63a12e99 100644 --- a/README.md +++ b/README.md @@ -138,7 +138,7 @@ However, in order to build from a git clone, you must update the submodules. If you're packaging OSTree and want a tarball, I recommend using a "recursive git archive" script. There are several available online; -[this code](https://github.com/ostreedev/ostree/blob/main/packaging/Makefile.dist-packaging#L11) +[this code](https://github.com/ostreedev/ostree/blob/main/ci/Makefile.dist-packaging#L18) in OSTree is an example. Once you have a git clone or recursive archive, building is the From 631e014bd6852c5116fe4d947c4e73d515676b66 Mon Sep 17 00:00:00 2001 From: Dan Nicholson Date: Sun, 18 Sep 2022 12:13:31 -0600 Subject: [PATCH 33/38] lib/pull: Fix max-metadata-size documentation The documented option is incorrect and has been since it's introduction in 2c55bc6997. --- src/libostree/ostree-repo-pull.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libostree/ostree-repo-pull.c b/src/libostree/ostree-repo-pull.c index 4819a40a..86b4358a 100644 --- a/src/libostree/ostree-repo-pull.c +++ b/src/libostree/ostree-repo-pull.c @@ -3680,7 +3680,7 @@ all_requested_refs_have_commit (GHashTable *requested_refs /* (element-type Ostr * * `override-commit-ids` (`as`): Array of specific commit IDs to fetch for refs * * `timestamp-check` (`b`): Verify commit timestamps are newer than current (when pulling via ref); Since: 2017.11 * * `timestamp-check-from-rev` (`s`): Verify that all fetched commit timestamps are newer than timestamp of given rev; Since: 2020.4 - * * `metadata-size-restriction` (`t`): Restrict metadata objects to a maximum number of bytes; 0 to disable. Since: 2018.9 + * * `max-metadata-size` (`t`): Restrict metadata objects to a maximum number of bytes; 0 to disable. Since: 2018.9 * * `dry-run` (`b`): Only print information on what will be downloaded (requires static deltas) * * `override-url` (`s`): Fetch objects from this URL if remote specifies no metalink in options * * `inherit-transaction` (`b`): Don't initiate, finish or abort a transaction, useful to do multiple pulls in one transaction. From 5412dc49ae83eb3f94be4fdd6ed6dcd4e5ca52e0 Mon Sep 17 00:00:00 2001 From: Luca BRUNO Date: Fri, 23 Sep 2022 13:02:00 +0000 Subject: [PATCH 34/38] lib/repo: properly initialize boolean variable This initializes a boolean variable that was previously left uninitialized. It was detected by a RHT internal static analyzer. --- src/libostree/ostree-repo.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libostree/ostree-repo.c b/src/libostree/ostree-repo.c index 90cde651..7bdb3c4e 100644 --- a/src/libostree/ostree-repo.c +++ b/src/libostree/ostree-repo.c @@ -3514,7 +3514,7 @@ reload_sysroot_config (OstreeRepo *self, * https://github.com/ostreedev/ostree/issues/1719 * https://github.com/ostreedev/ostree/issues/1801 */ - gboolean valid_bootloader; + gboolean valid_bootloader = FALSE; for (int i = 0; CFG_SYSROOT_BOOTLOADER_OPTS_STR[i]; i++) { if (g_str_equal (bootloader, CFG_SYSROOT_BOOTLOADER_OPTS_STR[i])) From a1018840f401207811e3aa615e999284f9036f93 Mon Sep 17 00:00:00 2001 From: Luca BRUNO Date: Mon, 26 Sep 2022 07:42:12 +0000 Subject: [PATCH 35/38] lib/sysroot-deploy: explicitly handle `g_variant_lookup` results This explicitly ignores the results of two optional variant lookups, in order to pacify a RHT internal static analyzer. --- src/libostree/ostree-sysroot-deploy.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libostree/ostree-sysroot-deploy.c b/src/libostree/ostree-sysroot-deploy.c index 7b2f1a6f..7cf2020f 100644 --- a/src/libostree/ostree-sysroot-deploy.c +++ b/src/libostree/ostree-sysroot-deploy.c @@ -2005,7 +2005,7 @@ install_deployment_kernel (OstreeSysroot *sysroot, &variant, NULL)) { metadata = g_variant_get_child_value (variant, 0); - g_variant_lookup (metadata, OSTREE_COMMIT_META_KEY_VERSION, "s", &deployment_version); + (void) g_variant_lookup (metadata, OSTREE_COMMIT_META_KEY_VERSION, "s", &deployment_version); } } @@ -2232,7 +2232,7 @@ get_deployment_ostree_version (OstreeRepo *repo, if (ostree_repo_load_variant (repo, OSTREE_OBJECT_TYPE_COMMIT, csum, &variant, NULL)) { g_autoptr(GVariant) metadata = g_variant_get_child_value (variant, 0); - g_variant_lookup (metadata, OSTREE_COMMIT_META_KEY_VERSION, "s", &version); + (void) g_variant_lookup (metadata, OSTREE_COMMIT_META_KEY_VERSION, "s", &version); } return g_steal_pointer (&version); From e234b630f85b97e48ecf45d5aaba9b1aa64e6b54 Mon Sep 17 00:00:00 2001 From: Miguel Angel Ajo Date: Mon, 19 Sep 2022 17:15:24 +0200 Subject: [PATCH 36/38] Support overlayfs whiteouts on checkout Introduces an intermediate format for overlayfs storage, where .wh-ostree. prefixed files will be converted into char 0:0 whiteout devices used by overlayfs to mark deletions across layers. The CI scripts now uses a volume for the scratch directories previously in /var/tmp otherwise we cannot create whiteout devices into an overlayfs mounted filesystem. Related-Issue: #2712 --- .github/workflows/tests.yml | 8 +- Makefile-tests.am | 1 + bash/ostree | 1 + man/ostree-checkout.xml | 11 ++ src/libostree/ostree-repo-checkout.c | 129 ++++++++++++++++++++- src/libostree/ostree-repo.h | 5 +- src/libostree/ostree-sysroot-deploy.c | 2 +- src/ostree/ot-builtin-checkout.c | 7 +- tests/archive-test.sh | 7 +- tests/basic-test.sh | 29 ++++- tests/kolainst/data-shared/libtest-core.sh | 7 ++ tests/libtest.sh | 52 ++++++++- tests/test-admin-deploy-whiteouts.sh | 42 +++++++ 13 files changed, 292 insertions(+), 9 deletions(-) create mode 100755 tests/test-admin-deploy-whiteouts.sh diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index ab3723ea..e3bc1726 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -172,8 +172,14 @@ jobs: # An empty string isn't valid, so a dummy --label option is always # added. options: --label ostree ${{ matrix.container-options }} + # make sure tests are performed on a non-overlayfs filesystem + volumes: + - tmp_dir:/test-tmp + env: + TEST_TMPDIR: /test-tmp steps: + - name: Pre-checkout setup run: ${{ matrix.pre-checkout-setup }} if: ${{ matrix.pre-checkout-setup }} @@ -187,7 +193,7 @@ jobs: run: ./ci/gh-install.sh ${{ matrix.extra-packages }} - name: Add non-root user - run: "useradd builder && chown -R -h builder: ." + run: "useradd builder && chown -R -h builder: . $TEST_TMPDIR" - name: Build and test run: runuser -u builder -- ./ci/gh-build.sh ${{ matrix.configure-options }} diff --git a/Makefile-tests.am b/Makefile-tests.am index c87893ee..1ad660bf 100644 --- a/Makefile-tests.am +++ b/Makefile-tests.am @@ -107,6 +107,7 @@ _installed_or_uninstalled_test_scripts = \ tests/test-admin-deploy-nomerge.sh \ tests/test-admin-deploy-none.sh \ tests/test-admin-deploy-bootid-gc.sh \ + tests/test-admin-deploy-whiteouts.sh \ tests/test-osupdate-dtb.sh \ tests/test-admin-instutil-set-kargs.sh \ tests/test-admin-upgrade-not-backwards.sh \ diff --git a/bash/ostree b/bash/ostree index 46363315..6f3b86ea 100644 --- a/bash/ostree +++ b/bash/ostree @@ -249,6 +249,7 @@ _ostree_checkout() { --union-identical --user-mode -U --whiteouts + --process-passthrough-whiteouts " local options_with_args=" diff --git a/man/ostree-checkout.xml b/man/ostree-checkout.xml index 4ed53a91..8f7d4f9b 100644 --- a/man/ostree-checkout.xml +++ b/man/ostree-checkout.xml @@ -114,6 +114,17 @@ License along with this library. If not, see . + + + + + Enable overlayfs whiteout extraction into 0:0 character devices. + Overlayfs whiteouts are encoded inside ostree as .ostree-wh.filename + and extracted as 0:0 character devices. This is useful to carry + container storage embedded into ostree. + + + diff --git a/src/libostree/ostree-repo-checkout.c b/src/libostree/ostree-repo-checkout.c index 663292a9..7c7d0cc7 100644 --- a/src/libostree/ostree-repo-checkout.c +++ b/src/libostree/ostree-repo-checkout.c @@ -35,6 +35,8 @@ #define WHITEOUT_PREFIX ".wh." #define OPAQUE_WHITEOUT_NAME ".wh..wh..opq" +#define OVERLAYFS_WHITEOUT_PREFIX ".ostree-wh." + /* Per-checkout call state/caching */ typedef struct { GString *path_buf; /* buffer for real path if filtering enabled */ @@ -582,6 +584,117 @@ checkout_file_hardlink (OstreeRepo *self, return TRUE; } +static gboolean +_checkout_overlayfs_whiteout_at_no_overwrite (OstreeRepoCheckoutAtOptions *options, + int destination_dfd, + const char *destination_name, + GFileInfo *file_info, + GVariant *xattrs, + gboolean *found_exant_file, + GCancellable *cancellable, + GError **error) +{ + if (found_exant_file != NULL) + *found_exant_file = FALSE; + guint32 file_mode = g_file_info_get_attribute_uint32 (file_info, "unix::mode"); + if (mknodat(destination_dfd, destination_name, (file_mode & ~S_IFMT) | S_IFCHR, (dev_t)0) < 0) + { + if (errno == EEXIST && found_exant_file != NULL) + { + *found_exant_file = TRUE; + return TRUE; + } + return glnx_throw_errno_prefix (error, "Creating whiteout char device"); + } + if (options->mode != OSTREE_REPO_CHECKOUT_MODE_USER) + { + if (xattrs != NULL && + !glnx_dfd_name_set_all_xattrs(destination_dfd, destination_name, xattrs, + cancellable, error)) + return glnx_throw_errno_prefix (error, "Setting xattrs for whiteout char device"); + + if (TEMP_FAILURE_RETRY(fchownat(destination_dfd, destination_name, + g_file_info_get_attribute_uint32 (file_info, "unix::uid"), + g_file_info_get_attribute_uint32 (file_info, "unix::gid"), + AT_SYMLINK_NOFOLLOW) < 0)) + return glnx_throw_errno_prefix (error, "fchownat"); + if (TEMP_FAILURE_RETRY (fchmodat (destination_dfd, destination_name, file_mode & ~S_IFMT, 0)) < 0) + return glnx_throw_errno_prefix (error, "fchmodat %s to 0%o", destination_name, file_mode & ~S_IFMT); + } + + return TRUE; +} + +static gboolean +_checkout_overlayfs_whiteout_at (OstreeRepo *repo, + OstreeRepoCheckoutAtOptions *options, + int destination_dfd, + const char *destination_name, + GFileInfo *file_info, + GVariant *xattrs, + GCancellable *cancellable, + GError **error) +{ + gboolean found_exant_file = FALSE; + if (!_checkout_overlayfs_whiteout_at_no_overwrite(options, destination_dfd, destination_name, + file_info, xattrs,&found_exant_file, + cancellable, error)) + return FALSE; + + if (!found_exant_file) + return TRUE; + + guint32 uid = g_file_info_get_attribute_uint32 (file_info, "unix::uid"); + guint32 gid = g_file_info_get_attribute_uint32 (file_info, "unix::gid"); + guint32 file_mode = g_file_info_get_attribute_uint32 (file_info, "unix::mode"); + + struct stat dest_stbuf; + + switch(options->overwrite_mode) + { + case OSTREE_REPO_CHECKOUT_OVERWRITE_NONE: + return FALSE; + case OSTREE_REPO_CHECKOUT_OVERWRITE_UNION_FILES: + if (!ot_ensure_unlinked_at (destination_dfd, destination_name, error)) + return FALSE; + return _checkout_overlayfs_whiteout_at_no_overwrite(options, destination_dfd, destination_name, + file_info, xattrs, NULL, cancellable, error); + case OSTREE_REPO_CHECKOUT_OVERWRITE_ADD_FILES: + return TRUE; + + case OSTREE_REPO_CHECKOUT_OVERWRITE_UNION_IDENTICAL: + if (!glnx_fstatat(destination_dfd, destination_name, &dest_stbuf, AT_SYMLINK_NOFOLLOW, + error)) + return FALSE; + if (!(repo->disable_xattrs || repo->mode == OSTREE_REPO_MODE_BARE_USER_ONLY)) + { + g_autoptr(GVariant) fs_xattrs; + if (!glnx_dfd_name_get_all_xattrs (destination_dfd, destination_name, + &fs_xattrs, cancellable, error)) + return FALSE; + if (!g_variant_equal(fs_xattrs, xattrs)) + return glnx_throw(error, "existing destination file %s xattrs don't match", + destination_name); + } + if (options->mode != OSTREE_REPO_CHECKOUT_MODE_USER) + { + if (gid != dest_stbuf.st_gid) + return glnx_throw(error, "existing destination file %s does not match gid %d", + destination_name, gid); + + if (uid != dest_stbuf.st_uid) + return glnx_throw(error, "existing destination file %s does not match uid %d", + destination_name, gid); + + if ((file_mode & ALLPERMS) != (dest_stbuf.st_mode & ALLPERMS)) + return glnx_throw(error, "existing destination file %s does not match mode %o", + destination_name, file_mode); + } + break; + } + return TRUE; +} + static gboolean checkout_one_file_at (OstreeRepo *repo, OstreeRepoCheckoutAtOptions *options, @@ -603,7 +716,8 @@ checkout_one_file_at (OstreeRepo *repo, /* FIXME - avoid the GFileInfo here */ g_autoptr(GFileInfo) source_info = NULL; - if (!ostree_repo_load_file (repo, checksum, NULL, &source_info, NULL, + g_autoptr(GVariant) source_xattrs = NULL; + if (!ostree_repo_load_file (repo, checksum, NULL, &source_info, &source_xattrs, cancellable, error)) return FALSE; @@ -623,6 +737,7 @@ checkout_one_file_at (OstreeRepo *repo, const gboolean is_unreadable = (!is_symlink && (source_mode & S_IRUSR) == 0); const gboolean is_whiteout = (!is_symlink && options->process_whiteouts && g_str_has_prefix (destination_name, WHITEOUT_PREFIX)); + const gboolean is_overlayfs_whiteout = (!is_symlink && g_str_has_prefix (destination_name, OVERLAYFS_WHITEOUT_PREFIX)); const gboolean is_reg_zerosized = (!is_symlink && g_file_info_get_size (source_info) == 0); const gboolean override_user_unreadable = (options->mode == OSTREE_REPO_CHECKOUT_MODE_USER && is_unreadable); @@ -643,6 +758,18 @@ checkout_one_file_at (OstreeRepo *repo, need_copy = FALSE; } + else if (is_overlayfs_whiteout && options->process_passthrough_whiteouts) + { + const char *name = destination_name + (sizeof (OVERLAYFS_WHITEOUT_PREFIX) - 1); + + if (!name[0]) + return glnx_throw (error, "Invalid empty overlayfs whiteout '%s'", name); + + g_assert (name[0] != '/'); /* Sanity */ + + return _checkout_overlayfs_whiteout_at(repo, options, destination_dfd, name, + source_info, source_xattrs, cancellable, error); + } else if (is_reg_zerosized || override_user_unreadable) { /* In https://github.com/ostreedev/ostree/commit/673cacd633f9d6b653cdea530657d3e780a41bbd we diff --git a/src/libostree/ostree-repo.h b/src/libostree/ostree-repo.h index b7ed3600..ce9b2507 100644 --- a/src/libostree/ostree-repo.h +++ b/src/libostree/ostree-repo.h @@ -989,8 +989,9 @@ typedef struct { gboolean force_copy; /* Since: 2017.6 */ gboolean bareuseronly_dirs; /* Since: 2017.7 */ gboolean force_copy_zerosized; /* Since: 2018.9 */ - gboolean unused_bools[4]; - /* 4 byte hole on 64 bit */ + gboolean process_passthrough_whiteouts; + gboolean unused_bools[3]; + /* 3 byte hole on 64 bit */ const char *subpath; diff --git a/src/libostree/ostree-sysroot-deploy.c b/src/libostree/ostree-sysroot-deploy.c index 7b2f1a6f..3ceb8ed7 100644 --- a/src/libostree/ostree-sysroot-deploy.c +++ b/src/libostree/ostree-sysroot-deploy.c @@ -641,7 +641,7 @@ checkout_deployment_tree (OstreeSysroot *sysroot, return FALSE; /* Generate hardlink farm, then opendir it */ - OstreeRepoCheckoutAtOptions checkout_opts = { 0, }; + OstreeRepoCheckoutAtOptions checkout_opts = { .process_passthrough_whiteouts = TRUE }; if (!ostree_repo_checkout_at (repo, &checkout_opts, osdeploy_dfd, checkout_target_name, csum, cancellable, error)) diff --git a/src/ostree/ot-builtin-checkout.c b/src/ostree/ot-builtin-checkout.c index d69c8b0b..bfa43885 100644 --- a/src/ostree/ot-builtin-checkout.c +++ b/src/ostree/ot-builtin-checkout.c @@ -37,6 +37,7 @@ static gboolean opt_union; static gboolean opt_union_add; static gboolean opt_union_identical; static gboolean opt_whiteouts; +static gboolean opt_process_passthrough_whiteouts; static gboolean opt_from_stdin; static char *opt_from_file; static gboolean opt_disable_fsync; @@ -77,6 +78,7 @@ static GOptionEntry options[] = { { "union-add", 0, 0, G_OPTION_ARG_NONE, &opt_union_add, "Keep existing files/directories, only add new", NULL }, { "union-identical", 0, 0, G_OPTION_ARG_NONE, &opt_union_identical, "When layering checkouts, error out if a file would be replaced with a different version, but add new files and directories", NULL }, { "whiteouts", 0, 0, G_OPTION_ARG_NONE, &opt_whiteouts, "Process 'whiteout' (Docker style) entries", NULL }, + { "process-passthrough-whiteouts", 0, 0, G_OPTION_ARG_NONE, &opt_process_passthrough_whiteouts, "Enable overlayfs whiteout extraction into char 0:0 devices", NULL }, { "allow-noent", 0, 0, G_OPTION_ARG_NONE, &opt_allow_noent, "Do nothing if specified path does not exist", NULL }, { "from-stdin", 0, 0, G_OPTION_ARG_NONE, &opt_from_stdin, "Process many checkouts from standard input", NULL }, { "from-file", 0, 0, G_OPTION_ARG_STRING, &opt_from_file, "Process many checkouts from input file", "FILE" }, @@ -129,7 +131,8 @@ process_one_checkout (OstreeRepo *repo, if (opt_disable_cache || opt_whiteouts || opt_require_hardlinks || opt_union_add || opt_force_copy || opt_force_copy_zerosized || opt_bareuseronly_dirs || opt_union_identical || - opt_skiplist_file || opt_selinux_policy || opt_selinux_prefix) + opt_skiplist_file || opt_selinux_policy || opt_selinux_prefix || + opt_process_passthrough_whiteouts) { OstreeRepoCheckoutAtOptions checkout_options = { 0, }; @@ -162,6 +165,8 @@ process_one_checkout (OstreeRepo *repo, } if (opt_whiteouts) checkout_options.process_whiteouts = TRUE; + if (opt_process_passthrough_whiteouts) + checkout_options.process_passthrough_whiteouts = TRUE; if (subpath) checkout_options.subpath = subpath; diff --git a/tests/archive-test.sh b/tests/archive-test.sh index b6d84979..6b45790e 100644 --- a/tests/archive-test.sh +++ b/tests/archive-test.sh @@ -71,6 +71,11 @@ mkdir -p test-overlays date > test-overlays/overlaid-file $OSTREE commit ${COMMIT_ARGS} -b test-base --base test2 --owner-uid 42 --owner-gid 42 test-overlays/ $OSTREE ls -R test-base > ls.txt -assert_streq "$(wc -l < ls.txt)" 14 +if can_create_whiteout_devices; then + assert_streq "$(wc -l < ls.txt)" 17 +else + assert_streq "$(wc -l < ls.txt)" 14 +fi + assert_streq "$(grep '42.*42' ls.txt | wc -l)" 2 echo "ok commit overlay base" diff --git a/tests/basic-test.sh b/tests/basic-test.sh index 67d57dda..f97f6fc3 100644 --- a/tests/basic-test.sh +++ b/tests/basic-test.sh @@ -19,7 +19,7 @@ set -euo pipefail -echo "1..$((88 + ${extra_basic_tests:-0}))" +echo "1..$((90 + ${extra_basic_tests:-0}))" CHECKOUT_U_ARG="" CHECKOUT_H_ARGS="-H" @@ -1203,3 +1203,30 @@ if test "$(id -u)" != "0"; then else echo "ok # SKIP not run when root" fi + +if ! skip_one_without_whiteouts_devices; then + cd ${test_tmpdir} + rm checkout-test2 -rf + $OSTREE checkout test2 checkout-test2 + + assert_not_has_file checkout-test2/whiteouts/whiteout + assert_not_has_file checkout-test2/whiteouts/whiteout2 + assert_has_file checkout-test2/whiteouts/.ostree-wh.whiteout + assert_has_file checkout-test2/whiteouts/.ostree-wh.whiteout2 + + echo "ok checkout: no whiteout passthrough by default" +fi + +if ! skip_one_without_whiteouts_devices; then + cd ${test_tmpdir} + rm checkout-test2 -rf + $OSTREE checkout --process-passthrough-whiteouts test2 checkout-test2 + + assert_not_has_file checkout-test2/whiteouts/.ostree-wh.whiteout + assert_not_has_file checkout-test2/whiteouts/.ostree-wh.whiteout2 + + assert_is_whiteout_device checkout-test2/whiteouts/whiteout + assert_is_whiteout_device checkout-test2/whiteouts/whiteout2 + + echo "ok checkout: whiteout with overlayfs passthrough processing" +fi diff --git a/tests/kolainst/data-shared/libtest-core.sh b/tests/kolainst/data-shared/libtest-core.sh index d10aac1c..3465fb9b 100644 --- a/tests/kolainst/data-shared/libtest-core.sh +++ b/tests/kolainst/data-shared/libtest-core.sh @@ -163,6 +163,13 @@ assert_file_has_mode () { fi } +assert_is_whiteout_device () { + device_details="$(stat -c '%F %t:%T' $1)" + if [ "$device_details" != "character special file 0:0" ]; then + fatal "File '$1' is not a whiteout character device 0:0" + fi +} + assert_symlink_has_content () { if ! test -L "$1"; then fatal "File '$1' is not a symbolic link" diff --git a/tests/libtest.sh b/tests/libtest.sh index 686f08dc..5830f210 100755 --- a/tests/libtest.sh +++ b/tests/libtest.sh @@ -148,6 +148,20 @@ if ! have_selinux_relabel; then fi echo done +# whiteout char 0:0 devices can be created as regular users, but +# cannot be created inside containers mounted via overlayfs +can_create_whiteout_devices() { + mknod -m 000 ${test_tmpdir}/.test-whiteout c 0 0 || return 1 + rm -f ${test_tmpdir}/.test-whiteout + return 0 +} + +echo -n checking for overlayfs whiteouts... +if ! can_create_whiteout_devices; then + export OSTREE_NO_WHITEOUTS=1 +fi +echo done + if test -n "${OT_TESTS_DEBUG:-}"; then set -x fi @@ -245,6 +259,15 @@ setup_test_repository () { ln -s nonexistent baz/alink mkdir baz/another/ echo x > baz/another/y + + # if we are running inside a container we cannot test + # the overlayfs whiteout marker passthrough + if ! test -n "${OSTREE_NO_WHITEOUTS:-}"; then + mkdir whiteouts + touch whiteouts/.ostree-wh.whiteout + touch whiteouts/.ostree-wh.whiteout2 + chmod 755 whiteouts/.ostree-wh.whiteout2 + fi umask "${oldumask}" cd ${test_tmpdir}/files @@ -406,7 +429,7 @@ setup_os_repository () { mkdir osdata cd osdata kver=3.6.0 - mkdir -p usr/bin ${bootdir} usr/lib/modules/${kver} usr/share usr/etc + mkdir -p usr/bin ${bootdir} usr/lib/modules/${kver} usr/share usr/etc usr/container/layers/abcd kernel_path=${bootdir}/vmlinuz initramfs_path=${bootdir}/initramfs.img # the HMAC file is only in /usr/lib/modules @@ -449,6 +472,17 @@ EOF mkdir -p usr/etc/testdirectory echo "a default daemon file" > usr/etc/testdirectory/test + # if we are running inside a container we cannot test + # the overlayfs whiteout marker passthrough + if ! test -n "${OSTREE_NO_WHITEOUTS:-}"; then + # overlayfs whiteout passhthrough marker files + touch usr/container/layers/abcd/.ostree-wh.whiteout + chmod 400 usr/container/layers/abcd/.ostree-wh.whiteout + + touch usr/container/layers/abcd/.ostree-wh.whiteout2 + chmod 777 usr/container/layers/abcd/.ostree-wh.whiteout2 + fi + ${CMD_PREFIX} ostree --repo=${test_tmpdir}/testos-repo commit ${bootable_flag} --add-metadata-string version=1.0.9 -b testos/buildmain/x86_64-runtime -s "Build" # Ensure these commits have distinct second timestamps @@ -588,6 +622,22 @@ skip_without_user_xattrs () { fi } +# Usage: if ! skip_one_without_whiteouts_devices; then ... more tests ...; fi +skip_one_without_whiteouts_devices() { + if ! can_create_whiteout_devices; then + echo "ok # SKIP - this test requires whiteout device support (test outside containers)" + return 0 + else + return 1 + fi +} + +skip_without_whiteouts_devices () { + if ! can_create_whiteout_devices; then + skip "this test requires whiteout device support (test outside containers)" + fi +} + _have_systemd_and_libmount='' have_systemd_and_libmount() { if test "${_have_systemd_and_libmount}" = ''; then diff --git a/tests/test-admin-deploy-whiteouts.sh b/tests/test-admin-deploy-whiteouts.sh new file mode 100755 index 00000000..66421949 --- /dev/null +++ b/tests/test-admin-deploy-whiteouts.sh @@ -0,0 +1,42 @@ +#!/bin/bash +# +# Copyright (C) 2022 Red Hat, Inc. +# +# SPDX-License-Identifier: LGPL-2.0+ +# +# This library is free software; you can redistribute it and/or +# modify it under the terms of the GNU Lesser General Public +# License as published by the Free Software Foundation; either +# version 2 of the License, or (at your option) any later version. +# +# This library is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +# Lesser General Public License for more details. +# +# You should have received a copy of the GNU Lesser General Public +# License along with this library. If not, see . + +set -euox pipefail + +. $(dirname $0)/libtest.sh + +skip_without_whiteouts_devices + +# Exports OSTREE_SYSROOT so --sysroot not needed. +setup_os_repository "archive" "syslinux" +${CMD_PREFIX} ostree --repo=sysroot/ostree/repo pull-local --remote=testos testos-repo testos/buildmain/x86_64-runtime + +echo "1..3" +${CMD_PREFIX} ostree admin deploy --os=testos --karg=root=LABEL=foo --karg=testkarg=1 testos:testos/buildmain/x86_64-runtime +origdeployment=$(${CMD_PREFIX} ostree admin --sysroot=sysroot --print-current-dir) + +assert_is_whiteout_device "${origdeployment}"/usr/container/layers/abcd/whiteout +echo "ok whiteout deployment" + +assert_not_has_file "${origdeployment}"/usr/container/layers/abcd/.ostree-wh.whiteout +echo "ok .ostree-wh.whiteout not created" + +assert_file_has_mode "${origdeployment}"/usr/container/layers/abcd/whiteout 400 +assert_file_has_mode "${origdeployment}"/usr/container/layers/abcd/whiteout2 777 +echo "ok whiteout permissions are preserved" From b83032d0d31e775bea721bd480494ac8d2e678a5 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Wed, 28 Sep 2022 08:30:18 -0400 Subject: [PATCH 37/38] README.md: Link otto Another project in the ostree/container space. --- README.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/README.md b/README.md index 63a12e99..7073cbda 100644 --- a/README.md +++ b/README.md @@ -87,6 +87,10 @@ which uses libostree. The [BuildStream](https://gitlab.com/BuildStream/buildstream) build and integration tool supports importing and exporting from libostree repos. +[fedora-iot/otto](https://github.com/fedora-iot/otto) is a tool that helps +ship ostree commits inside Docker/OCI containers and run a webserver +to serve the commits. + Fedora [coreos-assembler](https://github.com/coreos/coreos-assembler) is the build tool used to generate Fedora CoreOS derivatives. From eee649d902266e8d79465d34a2bf2f236e11dbd6 Mon Sep 17 00:00:00 2001 From: Luca BRUNO Date: Fri, 7 Oct 2022 07:54:09 +0000 Subject: [PATCH 38/38] Release 2022.6 --- configure.ac | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index 56e0e040..83ccbdbd 100644 --- a/configure.ac +++ b/configure.ac @@ -4,7 +4,7 @@ m4_define([year_version], [2022]) m4_define([release_version], [6]) m4_define([package_version], [year_version.release_version]) AC_INIT([libostree], [package_version], [walters@verbum.org]) -is_release_build=no +is_release_build=yes AC_CONFIG_HEADER([config.h]) AC_CONFIG_MACRO_DIR([buildutil]) AC_CONFIG_AUX_DIR([build-aux])