sysext: Fix config file support with --root=

Config files for --root= weren't picked up as expected because the
--root= flag got parsed after the config file.
Switch the order of config file and CLI flag parsing while letting the
CLI flags overwrite things set by the config files by tracking state
during parsing.

diff --git a/src/sysext/sysext.c b/src/sysext/sysext.c
index ab1f69f8c9b6bbde432bb8575a252706961cf014..d18f1389370082d0cde0b4423521da51e0519964 100644 (file)
--- a/src/sysext/sysext.c
+++ b/src/sysext/sysext.c
@@ -93,8 +93,10 @@ static bool arg_force = false;
 static bool arg_no_reload = false;
 static int arg_noexec = -1;
 static ImagePolicy *arg_image_policy = NULL;
+static bool arg_image_policy_set = false; /* Tracks initialization */
 static bool arg_varlink = false;
 static MutableMode arg_mutable = MUTABLE_NO;
+static bool arg_mutable_set = false; /* Tracks initialization */
 static const char *arg_overlayfs_mount_options = NULL;
 
 /* Is set to IMAGE_CONFEXT when systemd is called with the confext functionality instead of the default */
@@ -163,10 +165,13 @@ static int parse_mutable_mode(const char *p) {
 static DEFINE_CONFIG_PARSE_ENUM(config_parse_mutable_mode, mutable_mode, MutableMode);
 
 static int parse_config_file(ImageClass image_class) {
+        _cleanup_(image_policy_freep) ImagePolicy *config_image_policy = NULL;
+        MutableMode config_mutable = MUTABLE_NO;
         const char *section = image_class == IMAGE_SYSEXT ? "SysExt" : "ConfExt";
+        const char *sections = image_class == IMAGE_SYSEXT ? "SysExt\0" : "ConfExt\0";
         const ConfigTableItem items[] = {
-                { section, "Mutable",           config_parse_mutable_mode,      0,      &arg_mutable            },
-                { section, "ImagePolicy",       config_parse_image_policy,      0,      &arg_image_policy       },
+                { section, "Mutable",           config_parse_mutable_mode,      0,      &config_mutable            },
+                { section, "ImagePolicy",       config_parse_image_policy,      0,      &config_image_policy       },
                 {}
         };
         _cleanup_free_ char *config_file = NULL;
@@ -179,7 +184,7 @@ static int parse_config_file(ImageClass image_class) {
         r = config_parse_standard_file_with_dropins_full(
                         arg_root,
                         config_file,
-                        image_class == IMAGE_SYSEXT ? "SysExt\0" : "ConfExt\0",
+                        sections,
                         config_item_table_lookup, items,
                         CONFIG_PARSE_WARN,
                         /* userdata = */ NULL,
@@ -188,6 +193,17 @@ static int parse_config_file(ImageClass image_class) {
         if (r < 0)
                 return r;
 
+        /* Because this runs after parse_argv we only overwrite when things aren't set yet. */
+        if (!arg_mutable_set) {
+                arg_mutable = config_mutable;
+                arg_mutable_set = true;
+        }
+
+        if (!arg_image_policy_set) {
+                arg_image_policy = TAKE_PTR(config_image_policy);
+                arg_image_policy_set = true;
+        }
+
         return 0;
 }
 
@@ -2592,6 +2608,9 @@ static int parse_argv(int argc, char *argv[]) {
                         r = parse_image_policy_argument(optarg, &arg_image_policy);
                         if (r < 0)
                                 return r;
+                        /* When the CLI flag is given we initialize even if NULL
+                         * so that the config file entry won't overwrite it */
+                        arg_image_policy_set = true;
                         break;
 
                 case ARG_NOEXEC:
@@ -2618,6 +2637,7 @@ static int parse_argv(int argc, char *argv[]) {
                         if (r < 0)
                                 return log_error_errno(r, "Failed to parse argument to --mutable=: %s", optarg);
                         arg_mutable = r;
+                        arg_mutable_set = true;
                         break;
 
                 case '?':
@@ -2646,8 +2666,10 @@ static int parse_env(void) {
                 if (r < 0)
                         log_warning("Failed to parse %s environment variable value '%s'. Ignoring.",
                                     image_class_info[arg_image_class].mode_env, env_var);
-                else
+                else {
                         arg_mutable = r;
+                        arg_mutable_set = true;
+                }
         }
 
         env_var = secure_getenv(image_class_info[arg_image_class].opts_env);
@@ -2691,16 +2713,18 @@ static int run(int argc, char *argv[]) {
         if (r < 0)
                 return r;
 
-        /* Parse configuration file */
-        r = parse_config_file(arg_image_class);
-        if (r < 0)
-                log_warning_errno(r, "Failed to parse global config file, ignoring: %m");
-
         /* Parse command line */
         r = parse_argv(argc, argv);
         if (r <= 0)
                 return r;
 
+        /* Parse configuration file after argv because it needs --root=.
+         * The config entries will not overwrite values set already by
+         * env/argv because we track initialization. */
+        r = parse_config_file(arg_image_class);
+        if (r < 0)
+                log_warning_errno(r, "Failed to parse global config file, ignoring: %m");
+
         if (arg_varlink) {
                 _cleanup_(sd_varlink_server_unrefp) sd_varlink_server *varlink_server = NULL;
 

diff --git a/test/units/TEST-50-DISSECT.sysext.sh b/test/units/TEST-50-DISSECT.sysext.sh
index 3668e66e81bcc9256869d1c733730eaa5e067b07..c9ba89398594a1293f5c84e3d0936ca535d3d40b 100755 (executable)
--- a/test/units/TEST-50-DISSECT.sysext.sh
+++ b/test/units/TEST-50-DISSECT.sysext.sh
@@ -163,6 +163,24 @@ prepare_extension_image_with_matching_id_like() {
     prepend_trap "rm -rf ${ext_dir@Q}"
 }
 
+prepare_extension_image_raw() {
+    local root=${1:-}
+    local hierarchy=${2:?}
+    local ext_dir ext_release name
+
+    name="test-extension"
+    ext_dir="$root/var/lib/extensions/$name"
+    ext_release="$ext_dir/usr/lib/extension-release.d/extension-release.$name"
+    mkdir -p "${ext_release%/*}"
+    echo "ID=_any" >"$ext_release"
+    mkdir -p "$ext_dir/$hierarchy"
+    touch "$ext_dir$hierarchy/preexisting-file-in-extension-image"
+    mksquashfs "$ext_dir" "$ext_dir.raw" -all-root -noappend -quiet
+    rm -rf "$ext_dir"
+
+    prepend_trap "rm -rf ${ext_dir@Q}.raw"
+}
+
 prepare_extension_mutable_dir() {
     local dir=${1:?}
 
@@ -1170,6 +1188,57 @@ fi
 rm "${fake_root}/etc/initrd-release"
 )
 
+( init_trap
+: "Check config file support for --root="
+fake_root=${roots_dir:+"$roots_dir/config-file"}
+hierarchy=/opt
+extension_data_dir="$fake_root/var/lib/extensions.mutable$hierarchy"
+
+[[ "$FSTYPE" == "fuseblk" ]] && exit 0
+if [ "$roots_dir" = "" ]; then
+    echo >&2 "Skipping test when --root= is not used"
+    exit 0
+fi
+
+prepare_root "$fake_root" "$hierarchy"
+prepare_extension_image_raw "$fake_root" "$hierarchy"
+prepare_extension_mutable_dir "$extension_data_dir"
+prepare_read_only_hierarchy "$fake_root" "$hierarchy"
+
+mkdir -p "$fake_root/etc/systemd/"
+{ echo "[SysExt]" ; echo "Mutable=auto" ; } > "$fake_root/etc/systemd/sysext.conf"
+# Config file should be picked up with --root= set
+run_systemd_sysext "$fake_root" merge
+MNTOPT=$(findmnt "$fake_root$hierarchy" --first-only --direction backward --raw --noheadings -o VFS-OPTIONS | grep -o rw || true)
+if [ "$MNTOPT" != "rw" ]; then
+    echo >&2 "Merge did not pick up mutable setting from config file"
+    exit 1
+fi
+extension_verify_after_merge "$fake_root" "$hierarchy" -e -h -u
+run_systemd_sysext "$fake_root" unmerge
+
+# CLI arg should be able to overwrite config file
+run_systemd_sysext "$fake_root" merge --mutable=no
+MNTOPT=$(findmnt "$fake_root$hierarchy" --first-only --direction backward --raw --noheadings -o VFS-OPTIONS | grep -o ro || true)
+if [ "$MNTOPT" != "ro" ]; then
+    echo >&2 "Merge did not pick up CLI arg to overwrite mutable setting from config file"
+    exit 1
+fi
+extension_verify_after_merge "$fake_root" "$hierarchy" -e -h
+run_systemd_sysext "$fake_root" unmerge
+
+{ echo "[SysExt]" ; echo "ImagePolicy=root=signed+absent:usr=signed+absent" ; } > "$fake_root/etc/systemd/sysext.conf"
+# Config file should be picked up with --root= set
+if run_systemd_sysext "$fake_root" merge; then
+    echo >&2 "Merge did not fail with strict image policy in config file"
+    exit 1
+fi
+# CLI arg should be able to overwrite config file
+run_systemd_sysext "$fake_root" merge --image-policy="*"
+extension_verify_after_merge "$fake_root" "$hierarchy" -e -h
+run_systemd_sysext "$fake_root" unmerge
+)
+
 } # End of run_sysext_tests