From 26abca71263d981b5f1db778d1945a99fc235e28 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Wed, 10 Jul 2024 17:21:57 -0400 Subject: [PATCH 1/3] keyfile-utils: Add API to parse tristate strings Prep for using this in multiple places. Add unit tests. --- src/libotutil/ot-keyfile-utils.c | 63 +++++++++++++++++++++++--------- src/libotutil/ot-keyfile-utils.h | 3 ++ tests/test-keyfile-utils.c | 37 +++++++++++++++++++ 3 files changed, 86 insertions(+), 17 deletions(-) diff --git a/src/libotutil/ot-keyfile-utils.c b/src/libotutil/ot-keyfile-utils.c index d6cd102ca0..12f4ac0b6d 100644 --- a/src/libotutil/ot-keyfile-utils.c +++ b/src/libotutil/ot-keyfile-utils.c @@ -60,6 +60,51 @@ ot_keyfile_get_boolean_with_default (GKeyFile *keyfile, const char *section, con return TRUE; } +// Keep this in sync with +// https://gitlab.gnome.org/GNOME/glib/-/blob/4a73fbda8be6f80f14b05983eb575c1eb1329c2c/glib/gkeyfile.c?page=5#L4585 +// Except for some reason at some point we added "yes" or "no" as possible values too... +gboolean +_ostree_parse_boolean (const char *s, gboolean *out_val, GError **error) +{ + g_assert (s); + g_assert (out_val); + + if (g_str_equal (s, "yes") || g_str_equal (s, "true") || g_str_equal (s, "1")) + { + *out_val = TRUE; + return TRUE; + } + else if (g_str_equal (s, "no") || g_str_equal (s, "false") || g_str_equal (s, "0")) + { + *out_val = FALSE; + return TRUE; + } + return glnx_throw (error, "Invalid boolean: %s", s); +} + +gboolean +_ostree_parse_tristate (const char *s, OtTristate *out_tri, GError **error) +{ + if (strcmp (s, "maybe") == 0) + { + *out_tri = OT_TRISTATE_MAYBE; + return TRUE; + } + + gboolean bool_value = FALSE; + // Discard the error here, just check if it's valid + if (_ostree_parse_boolean (s, &bool_value, NULL)) + { + if (bool_value) + *out_tri = OT_TRISTATE_YES; + else + *out_tri = OT_TRISTATE_NO; + return TRUE; + } + // If it's invalid, be clear a tristate was expected. + return glnx_throw (error, "Invalid tri-state value: %s", s); +} + gboolean ot_keyfile_get_tristate_with_default (GKeyFile *keyfile, const char *section, const char *value, OtTristate default_value, OtTristate *out_tri, GError **error) @@ -85,23 +130,7 @@ ot_keyfile_get_tristate_with_default (GKeyFile *keyfile, const char *section, co } ret_value = g_strstrip (ret_value); - - if (strcmp (ret_value, "yes") == 0 || strcmp (ret_value, "true") == 0 - || strcmp (ret_value, "1") == 0) - *out_tri = OT_TRISTATE_YES; - else if (strcmp (ret_value, "no") == 0 || strcmp (ret_value, "false") == 0 - || strcmp (ret_value, "0") == 0) - *out_tri = OT_TRISTATE_NO; - else if (strcmp (ret_value, "maybe") == 0) - *out_tri = OT_TRISTATE_MAYBE; - else - { - g_set_error (error, G_KEY_FILE_ERROR, G_KEY_FILE_ERROR_INVALID_VALUE, - "Invalid tri-state value: %s", ret_value); - return FALSE; - } - - return TRUE; + return _ostree_parse_tristate (ret_value, out_tri, error); } gboolean diff --git a/src/libotutil/ot-keyfile-utils.h b/src/libotutil/ot-keyfile-utils.h index eb97c8d7c6..1ed6747328 100644 --- a/src/libotutil/ot-keyfile-utils.h +++ b/src/libotutil/ot-keyfile-utils.h @@ -32,6 +32,9 @@ typedef enum G_BEGIN_DECLS +gboolean _ostree_parse_boolean (const char *s, gboolean *out_val, GError **error); +gboolean _ostree_parse_tristate (const char *s, OtTristate *out_tri, GError **error); + gboolean ot_keyfile_get_boolean_with_default (GKeyFile *keyfile, const char *section, const char *value, gboolean default_value, gboolean *out_bool, GError **error); diff --git a/tests/test-keyfile-utils.c b/tests/test-keyfile-utils.c index 198e71d256..748e029af0 100644 --- a/tests/test-keyfile-utils.c +++ b/tests/test-keyfile-utils.c @@ -173,6 +173,42 @@ fill_keyfile (GKeyFile *file) g_key_file_set_value (file, "section", "value_bar", "bar"); } +static void +test_parse_tristate (void) +{ + g_autoptr (GError) error = NULL; + + OtTristate t = OT_TRISTATE_NO; + // Verify maybe + (void)_ostree_parse_tristate ("maybe", &t, &error); + g_assert_no_error (error); + g_assert_cmpint (t, ==, OT_TRISTATE_MAYBE); + + // Alternate yes and no + (void)_ostree_parse_tristate ("yes", &t, &error); + g_assert_no_error (error); + g_assert_cmpint (t, ==, OT_TRISTATE_YES); + (void)_ostree_parse_tristate ("no", &t, &error); + g_assert_no_error (error); + g_assert_cmpint (t, ==, OT_TRISTATE_NO); + (void)_ostree_parse_tristate ("1", &t, &error); + g_assert_no_error (error); + g_assert_cmpint (t, ==, OT_TRISTATE_YES); + (void)_ostree_parse_tristate ("0", &t, &error); + g_assert_no_error (error); + g_assert_cmpint (t, ==, OT_TRISTATE_NO); + (void)_ostree_parse_tristate ("true", &t, &error); + g_assert_no_error (error); + g_assert_cmpint (t, ==, OT_TRISTATE_YES); + (void)_ostree_parse_tristate ("false", &t, &error); + g_assert_no_error (error); + g_assert_cmpint (t, ==, OT_TRISTATE_NO); + + // And an error case + (void)_ostree_parse_tristate ("foobar", &t, &error); + g_assert (error != NULL); +} + int main (int argc, char **argv) { @@ -186,6 +222,7 @@ main (int argc, char **argv) g_test_add_func ("/keyfile-utils/get-value-with-default-group-optional", test_get_value_with_default_group_optional); g_test_add_func ("/keyfile-utils/copy-group", test_copy_group); + g_test_add_func ("/keyfile-utils/parse-tristate", test_parse_tristate); ret = g_test_run (); From 65ff4041953d815e3e32adb28e8d85b7acb8367b Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Wed, 10 Jul 2024 17:27:11 -0400 Subject: [PATCH 2/3] prepare-root: Gather kernel cmdline early Prep for parsing the composefs config from the kernel cmdline. No functional changes intended. --- src/switchroot/ostree-prepare-root.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/switchroot/ostree-prepare-root.c b/src/switchroot/ostree-prepare-root.c index 2cb9f67ca9..c2bee87d5c 100644 --- a/src/switchroot/ostree-prepare-root.c +++ b/src/switchroot/ostree-prepare-root.c @@ -113,14 +113,11 @@ sysroot_is_configured_ro (const char *sysroot) } static char * -resolve_deploy_path (const char *root_mountpoint) +resolve_deploy_path (const char *kernel_cmdline, const char *root_mountpoint) { char destpath[PATH_MAX]; struct stat stbuf; char *deploy_path; - g_autofree char *kernel_cmdline = read_proc_cmdline (); - if (!kernel_cmdline) - errx (EXIT_FAILURE, "Failed to read kernel cmdline"); g_autoptr (GError) error = NULL; g_autofree char *ostree_target = NULL; @@ -268,6 +265,10 @@ main (int argc, char *argv[]) err (EXIT_FAILURE, "usage: ostree-prepare-root SYSROOT"); const char *root_arg = argv[1]; + g_autofree char *kernel_cmdline = read_proc_cmdline (); + if (!kernel_cmdline) + errx (EXIT_FAILURE, "Failed to read kernel cmdline"); + // Since several APIs want to operate in terms of file descriptors, let's // open the initramfs now. Currently this is just used for the config parser. glnx_autofd int initramfs_rootfs_fd = -1; @@ -308,7 +309,7 @@ main (int argc, char *argv[]) const char *root_mountpoint = realpath (root_arg, NULL); if (root_mountpoint == NULL) err (EXIT_FAILURE, "realpath(\"%s\")", root_arg); - g_autofree char *deploy_path = resolve_deploy_path (root_mountpoint); + g_autofree char *deploy_path = resolve_deploy_path (kernel_cmdline, root_mountpoint); const char *deploy_directory_name = glnx_basename (deploy_path); // Note that realpath() should have stripped any trailing `/` which shouldn't // be in the karg to start with, but we assert here to be sure we have a non-empty From e226c87614919d956f9ee298c4fd8121342e0c58 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Wed, 10 Jul 2024 16:38:48 -0400 Subject: [PATCH 3/3] prepare-root: Add `ostree.prepare-root.composefs` We have a use case for overriding the composefs state via the kernel commandline; see e.g. https://gitlab.com/fedora/bootc/tracker/-/issues/27 Signed-off-by: Colin Walters --- man/ostree-prepare-root.xml | 12 ++++++++++++ src/libostree/ostree-sysroot-deploy.c | 2 +- src/libotcore/otcore-prepare-root.c | 25 ++++++++++++++++++++++++- src/libotcore/otcore.h | 4 ++-- src/switchroot/ostree-prepare-root.c | 2 +- tests/inst/src/composefs.rs | 24 +++++++++++++++++++++++- 6 files changed, 63 insertions(+), 6 deletions(-) diff --git a/man/ostree-prepare-root.xml b/man/ostree-prepare-root.xml index 9117c340bd..8a682e7315 100644 --- a/man/ostree-prepare-root.xml +++ b/man/ostree-prepare-root.xml @@ -153,6 +153,18 @@ License along with this library. If not, see . commit must match the target composefs image. + + + The following kernel commandline parameters are also parsed: + + + + ostree.prepare-root.composefs + This accepts the same values as composefs.enabled above, and overrides the config file (if present). + For example, specifying ostree.prepare-root.composefs=0 will disable composefs, even if it is enabled by default in the initrd config. + + + diff --git a/src/libostree/ostree-sysroot-deploy.c b/src/libostree/ostree-sysroot-deploy.c index f7ca2dd4a3..4efaae6e86 100644 --- a/src/libostree/ostree-sysroot-deploy.c +++ b/src/libostree/ostree-sysroot-deploy.c @@ -655,7 +655,7 @@ checkout_deployment_tree (OstreeSysroot *sysroot, OstreeRepo *repo, OstreeDeploy // However, we don't load the keys here, because they may not exist, such // as in the initial deploy g_autoptr (ComposefsConfig) composefs_config - = otcore_load_composefs_config (prepare_root_config, FALSE, error); + = otcore_load_composefs_config ("", prepare_root_config, FALSE, error); if (!composefs_config) return glnx_prefix_error (error, "Reading composefs config"); diff --git a/src/libotcore/otcore-prepare-root.c b/src/libotcore/otcore-prepare-root.c index f76db04a33..e0a1641a8f 100644 --- a/src/libotcore/otcore-prepare-root.c +++ b/src/libotcore/otcore-prepare-root.c @@ -24,6 +24,8 @@ // in use, the ostree commit metadata will contain the composefs image digest, // which can be used to fully verify the target filesystem tree. #define BINDING_KEYPATH "/etc/ostree/initramfs-root-binding.key" +// The kernel argument to configure composefs +#define CMDLINE_KEY_COMPOSEFS "ostree.prepare-root.composefs" static bool proc_cmdline_has_key_starting_with (const char *cmdline, const char *key) @@ -161,8 +163,12 @@ otcore_free_composefs_config (ComposefsConfig *config) // Parse the [composefs] section of the prepare-root.conf. ComposefsConfig * -otcore_load_composefs_config (GKeyFile *config, gboolean load_keys, GError **error) +otcore_load_composefs_config (const char *cmdline, GKeyFile *config, gboolean load_keys, + GError **error) { + g_assert (cmdline); + g_assert (config); + GLNX_AUTO_PREFIX_ERROR ("Loading composefs config", error); g_autoptr (ComposefsConfig) ret = g_new0 (ComposefsConfig, 1); @@ -214,5 +220,22 @@ otcore_load_composefs_config (GKeyFile *config, gboolean load_keys, GError **err return glnx_null_throw (error, "public key file specified, but no public keys found"); } + g_autofree char *ostree_composefs = otcore_find_proc_cmdline_key (cmdline, CMDLINE_KEY_COMPOSEFS); + if (ostree_composefs) + { + if (g_strcmp0 (ostree_composefs, "signed") == 0) + { + ret->enabled = OT_TRISTATE_YES; + ret->is_signed = true; + } + else + { + // The other states force off signatures + ret->is_signed = false; + if (!_ostree_parse_tristate (ostree_composefs, &ret->enabled, error)) + return glnx_prefix_error (error, "handling karg " CMDLINE_KEY_COMPOSEFS), NULL; + } + } + return g_steal_pointer (&ret); } diff --git a/src/libotcore/otcore.h b/src/libotcore/otcore.h index fc6b81ca1a..6e1d510329 100644 --- a/src/libotcore/otcore.h +++ b/src/libotcore/otcore.h @@ -59,8 +59,8 @@ typedef struct void otcore_free_composefs_config (ComposefsConfig *config); G_DEFINE_AUTOPTR_CLEANUP_FUNC (ComposefsConfig, otcore_free_composefs_config) -ComposefsConfig *otcore_load_composefs_config (GKeyFile *config, gboolean load_keys, - GError **error); +ComposefsConfig *otcore_load_composefs_config (const char *cmdline, GKeyFile *config, + gboolean load_keys, GError **error); // Our directory with transient state (eventually /run/ostree-booted should be a link to // /run/ostree/booted) diff --git a/src/switchroot/ostree-prepare-root.c b/src/switchroot/ostree-prepare-root.c index c2bee87d5c..7d1b8ac822 100644 --- a/src/switchroot/ostree-prepare-root.c +++ b/src/switchroot/ostree-prepare-root.c @@ -290,7 +290,7 @@ main (int argc, char *argv[]) // We always parse the composefs config, because we want to detect and error // out if it's enabled, but not supported at compile time. g_autoptr (ComposefsConfig) composefs_config - = otcore_load_composefs_config (config, TRUE, &error); + = otcore_load_composefs_config (kernel_cmdline, config, TRUE, &error); if (!composefs_config) errx (EXIT_FAILURE, "%s", error->message); diff --git a/tests/inst/src/composefs.rs b/tests/inst/src/composefs.rs index fa6d3d374a..eddccd1d6e 100644 --- a/tests/inst/src/composefs.rs +++ b/tests/inst/src/composefs.rs @@ -131,6 +131,19 @@ fn verify_composefs_signed(sh: &xshell::Shell, metadata: &glib::VariantDict) -> Ok(()) } +fn verify_disable_composefs(sh: &xshell::Shell, metadata: &glib::VariantDict) -> Result<()> { + assert_eq!( + metadata + .lookup::("composefs") + .unwrap() + .unwrap_or_default(), + false + ); + let fstype = cmd!(sh, "findmnt -n -o FSTYPE /").read()?; + assert_ne!(fstype.as_str(), "overlay"); + Ok(()) +} + pub(crate) fn itest_composefs() -> Result<()> { let sh = &xshell::Shell::new()?; let mark = match crate::test::get_reboot_mark()? { @@ -165,7 +178,16 @@ pub(crate) fn itest_composefs() -> Result<()> { Err(reboot("2"))?; Ok(()) } - "2" => verify_composefs_signed(sh, &metadata), + "2" => { + verify_composefs_signed(sh, &metadata)?; + cmd!( + sh, + "rpm-ostree kargs --append=ostree.prepare-root.composefs=0" + ) + .run()?; + Err(reboot("3")) + } + "3" => verify_disable_composefs(sh, &metadata), o => anyhow::bail!("Unrecognized reboot mark {o}"), } }