From: Luca Boccassi Date: Thu, 11 Dec 2025 05:38:26 +0000 (+0000) Subject: core: gracefully skip unknown policy designators in RootImagePolicy et al (#40060) X-Git-Tag: v259~37 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=7c0afcdde22d3d94fd23bfd0e473c263aaf54e8a;p=thirdparty%2Fsystemd.git core: gracefully skip unknown policy designators in RootImagePolicy et al (#40060) Usually we gracefully ignore unknown configuration parameters, so that service files can be written by upstreams and used across a variegated range of distributions with various versions of systemd, to avoid forcing users to the minimum common denominator and only adding settings that are supported by the oldest distro supported. Image policies do not behave like this, and any unknown partition or policy designator causes the whole unit to fail to parse and a hard error. Change it so that parsing RootImagePolicy and friends via unit file or D-Bus logs but otherwise ignores unknown specifiers, like other options do. This allows us to add new specifiers in the future, and users to adopt them immediately. Follow-up for d452335aa47fb1f1b11dc75bc462697431e64af3 --- diff --git a/src/analyze/analyze-image-policy.c b/src/analyze/analyze-image-policy.c index f7075c60a66..2075d1b437a 100644 --- a/src/analyze/analyze-image-policy.c +++ b/src/analyze/analyze-image-policy.c @@ -103,7 +103,7 @@ int verb_image_policy(int argc, char *argv[], void *userdata) { else if (streq(argv[i], "@host")) p = &image_policy_host; else { - r = image_policy_from_string(argv[i], &pbuf); + r = image_policy_from_string(argv[i], /* graceful= */ false, &pbuf); if (r < 0) return log_error_errno(r, "Failed to parse image policy '%s': %m", argv[i]); diff --git a/src/core/dbus-execute.c b/src/core/dbus-execute.c index 83b0ddcb9e1..5a0b083b97d 100644 --- a/src/core/dbus-execute.c +++ b/src/core/dbus-execute.c @@ -4308,7 +4308,7 @@ int bus_exec_context_set_transient_property( if (r < 0) return r; - r = image_policy_from_string(s, &p); + r = image_policy_from_string(s, /* graceful= */ true, &p); if (r < 0) return sd_bus_error_setf(reterr_error, SD_BUS_ERROR_INVALID_ARGS, "Failed to parse image policy string: %s", s); diff --git a/src/core/execute-serialize.c b/src/core/execute-serialize.c index 02435447d73..bed2776889a 100644 --- a/src/core/execute-serialize.c +++ b/src/core/execute-serialize.c @@ -3740,21 +3740,21 @@ static int exec_context_deserialize(ExecContext *c, FILE *f) { if (c->root_image_policy) return -EINVAL; /* duplicated */ - r = image_policy_from_string(val, &c->root_image_policy); + r = image_policy_from_string(val, /* graceful= */ true, &c->root_image_policy); if (r < 0) return r; } else if ((val = startswith(l, "exec-context-mount-image-policy="))) { if (c->mount_image_policy) return -EINVAL; /* duplicated */ - r = image_policy_from_string(val, &c->mount_image_policy); + r = image_policy_from_string(val, /* graceful= */ true, &c->mount_image_policy); if (r < 0) return r; } else if ((val = startswith(l, "exec-context-extension-image-policy="))) { if (c->extension_image_policy) return -EINVAL; /* duplicated */ - r = image_policy_from_string(val, &c->extension_image_policy); + r = image_policy_from_string(val, /* graceful= */ true, &c->extension_image_policy); if (r < 0) return r; } else diff --git a/src/mountfsd/mountwork.c b/src/mountfsd/mountwork.c index 7e4c5063fa3..24f6e23e231 100644 --- a/src/mountfsd/mountwork.c +++ b/src/mountfsd/mountwork.c @@ -76,7 +76,7 @@ static int json_dispatch_image_policy(const char *name, sd_json_variant *variant if (!sd_json_variant_is_string(variant)) return json_log(variant, flags, SYNTHETIC_ERRNO(EINVAL), "JSON field '%s' is not a string.", strna(name)); - r = image_policy_from_string(sd_json_variant_string(variant), &q); + r = image_policy_from_string(sd_json_variant_string(variant), /* graceful= */ false, &q); if (r < 0) return json_log(variant, flags, r, "JSON field '%s' is not a valid image policy.", strna(name)); @@ -244,7 +244,7 @@ static int determine_image_policy( e = secure_getenv(envvar); if (e) { - r = image_policy_from_string(e, &envvar_policy); + r = image_policy_from_string(e, /* graceful= */ false, &envvar_policy); if (r < 0) return log_error_errno(r, "Failed to parse image policy supplied via $%s: %m", envvar); diff --git a/src/shared/image-policy.c b/src/shared/image-policy.c index 7a248318ebf..bd758ea1c94 100644 --- a/src/shared/image-policy.c +++ b/src/shared/image-policy.c @@ -209,7 +209,7 @@ static PartitionPolicyFlags policy_flag_from_string_one(const char *s) { return _PARTITION_POLICY_FLAGS_INVALID; } -PartitionPolicyFlags partition_policy_flags_from_string(const char *s) { +PartitionPolicyFlags partition_policy_flags_from_string(const char *s, bool graceful) { PartitionPolicyFlags flags = 0; int r; @@ -229,8 +229,13 @@ PartitionPolicyFlags partition_policy_flags_from_string(const char *s) { break; ff = policy_flag_from_string_one(strstrip(f)); - if (ff < 0) + if (ff < 0) { + if (graceful) { + log_debug("Unknown partition policy flag: %s, ignoring", f); + continue; + } return -EBADRQC; /* recognizable error */ + } flags |= ff; } @@ -254,7 +259,7 @@ static ImagePolicy* image_policy_new(size_t n_policies) { return p; } -int image_policy_from_string(const char *s, ImagePolicy **ret) { +int image_policy_from_string(const char *s, bool graceful, ImagePolicy **ret) { _cleanup_free_ ImagePolicy *p = NULL; uint64_t dmask = 0; ImagePolicy *t; @@ -336,15 +341,20 @@ int image_policy_from_string(const char *s, ImagePolicy **ret) { default_specified = true; } else { designator = partition_designator_from_string(ds); - if (designator < 0) - return log_debug_errno(SYNTHETIC_ERRNO(EBADSLT), "Unknown partition designator: %s", ds); /* recognizable error */ + if (designator < 0) { + if (!graceful) + return log_debug_errno(SYNTHETIC_ERRNO(EBADSLT), "Unknown partition designator: %s", ds); /* recognizable error */ + + log_debug("Unknown partition designator: %s, ignoring", ds); + continue; + } if (dmask & (UINT64_C(1) << designator)) return log_debug_errno(SYNTHETIC_ERRNO(ENOTUNIQ), "Partition designator specified more than once: %s", ds); dmask |= UINT64_C(1) << designator; } fs = strstrip(f); - flags = partition_policy_flags_from_string(fs); + flags = partition_policy_flags_from_string(fs, graceful); if (flags == -EBADRQC) return log_debug_errno(flags, "Unknown partition policy flag: %s", fs); if (flags < 0) @@ -651,7 +661,7 @@ int config_parse_image_policy( return 0; } - r = image_policy_from_string(rvalue, &np); + r = image_policy_from_string(rvalue, /* graceful */ true, &np); if (r == -ENOTUNIQ) return log_syntax(unit, LOG_ERR, filename, line, r, "Duplicate rule in image policy, refusing: %s", rvalue); if (r == -EBADSLT) @@ -678,7 +688,7 @@ int parse_image_policy_argument(const char *s, ImagePolicy **policy) { * Hence, do not pass in uninitialized pointers. */ - r = image_policy_from_string(s, &np); + r = image_policy_from_string(s, /* graceful= */ false, &np); if (r == -ENOTUNIQ) return log_error_errno(r, "Duplicate rule in image policy: %s", s); if (r == -EBADSLT) diff --git a/src/shared/image-policy.h b/src/shared/image-policy.h index 6fdd8abe324..037900b840c 100644 --- a/src/shared/image-policy.h +++ b/src/shared/image-policy.h @@ -80,10 +80,10 @@ static inline size_t image_policy_n_entries(const ImagePolicy *policy) { PartitionPolicyFlags partition_policy_flags_extend(PartitionPolicyFlags flags); PartitionPolicyFlags partition_policy_flags_reduce(PartitionPolicyFlags flags); -PartitionPolicyFlags partition_policy_flags_from_string(const char *s); +PartitionPolicyFlags partition_policy_flags_from_string(const char *s, bool graceful); int partition_policy_flags_to_string(PartitionPolicyFlags flags, bool simplify, char **ret); -int image_policy_from_string(const char *s, ImagePolicy **ret); +int image_policy_from_string(const char *s, bool graceful, ImagePolicy **ret); int image_policy_to_string(const ImagePolicy *policy, bool simplify, char **ret); /* Recognizes three special policies by equivalence */ diff --git a/src/test/test-image-policy.c b/src/test/test-image-policy.c index f35bfc6881d..6cf7dff20bb 100644 --- a/src/test/test-image-policy.c +++ b/src/test/test-image-policy.c @@ -22,11 +22,11 @@ static void test_policy(const ImagePolicy *p, const char *name) { printf("%s\n", ansi_normal()); - assert_se(image_policy_from_string(as_string, &parsed) >= 0); + assert_se(image_policy_from_string(as_string, /* graceful= */ false, &parsed) >= 0); assert_se(image_policy_equal(p, parsed)); parsed = image_policy_free(parsed); - assert_se(image_policy_from_string(as_string_simplified, &parsed) >= 0); + assert_se(image_policy_from_string(as_string_simplified, /* graceful= */ false, &parsed) >= 0); assert_se(image_policy_equivalent(p, parsed)); parsed = image_policy_free(parsed); @@ -55,14 +55,14 @@ static void test_policy(const ImagePolicy *p, const char *name) { static void test_policy_string(const char *t) { _cleanup_free_ ImagePolicy *parsed = NULL; - assert_se(image_policy_from_string(t, &parsed) >= 0); + assert_se(image_policy_from_string(t, /* graceful= */ false, &parsed) >= 0); test_policy(parsed, t); } static void test_policy_equiv(const char *s, bool (*func)(const ImagePolicy *p)) { _cleanup_(image_policy_freep) ImagePolicy *p = NULL; - assert_se(image_policy_from_string(s, &p) >= 0); + assert_se(image_policy_from_string(s, /* graceful= */ false, &p) >= 0); assert_se(func(p)); assert_se(func == image_policy_equiv_ignore || !image_policy_equiv_ignore(p)); @@ -106,15 +106,25 @@ TEST_RET(test_image_policy_to_string) { test_policy_equiv("=unused+absent", image_policy_equiv_ignore); test_policy_equiv("root=ignore:=ignore", image_policy_equiv_ignore); - assert_se(image_policy_from_string("pfft", NULL) == -EINVAL); - assert_se(image_policy_from_string("öäüß", NULL) == -EINVAL); - assert_se(image_policy_from_string(":", NULL) == -EINVAL); - assert_se(image_policy_from_string("a=", NULL) == -EBADSLT); - assert_se(image_policy_from_string("=a", NULL) == -EBADRQC); - assert_se(image_policy_from_string("==", NULL) == -EBADRQC); - assert_se(image_policy_from_string("root=verity:root=encrypted", NULL) == -ENOTUNIQ); - assert_se(image_policy_from_string("root=grbl", NULL) == -EBADRQC); - assert_se(image_policy_from_string("wowza=grbl", NULL) == -EBADSLT); + assert_se(image_policy_from_string("pfft", /* graceful= */ false, NULL) == -EINVAL); + assert_se(image_policy_from_string("öäüß", /* graceful= */ false, NULL) == -EINVAL); + assert_se(image_policy_from_string(":", /* graceful= */ false, NULL) == -EINVAL); + assert_se(image_policy_from_string("a=", /* graceful= */ false, NULL) == -EBADSLT); + assert_se(image_policy_from_string("=a", /* graceful= */ false, NULL) == -EBADRQC); + assert_se(image_policy_from_string("==", /* graceful= */ false, NULL) == -EBADRQC); + assert_se(image_policy_from_string("root=verity:root=encrypted", /* graceful= */ false, NULL) == -ENOTUNIQ); + assert_se(image_policy_from_string("root=grbl", /* graceful= */ false, NULL) == -EBADRQC); + assert_se(image_policy_from_string("wowza=grbl", /* graceful= */ false, NULL) == -EBADSLT); + + assert_se(image_policy_from_string("pfft", /* graceful= */ true, NULL) == -EINVAL); + assert_se(image_policy_from_string("öäüß", /* graceful= */ true, NULL) == -EINVAL); + assert_se(image_policy_from_string(":", /* graceful= */ true, NULL) == -EINVAL); + assert_se(image_policy_from_string("a=", /* graceful= */ true, NULL) == 0); + assert_se(image_policy_from_string("=a", /* graceful= */ true, NULL) == 0); + assert_se(image_policy_from_string("==", /* graceful= */ true, NULL) == 0); + assert_se(image_policy_from_string("root=verity:root=encrypted", /* graceful= */ true, NULL) == -ENOTUNIQ); + assert_se(image_policy_from_string("root=grbl", /* graceful= */ true, NULL) == 0); + assert_se(image_policy_from_string("wowza=grbl", /* graceful= */ true, NULL) == 0); return 0; } @@ -131,9 +141,9 @@ TEST(extend) { static void test_policy_intersect_one(const char *a, const char *b, const char *c) { _cleanup_(image_policy_freep) ImagePolicy *x = NULL, *y = NULL, *z = NULL, *t = NULL; - assert_se(image_policy_from_string(a, &x) >= 0); - assert_se(image_policy_from_string(b, &y) >= 0); - assert_se(image_policy_from_string(c, &z) >= 0); + assert_se(image_policy_from_string(a, /* graceful= */ false, &x) >= 0); + assert_se(image_policy_from_string(b, /* graceful= */ false, &y) >= 0); + assert_se(image_policy_from_string(c, /* graceful= */ false, &z) >= 0); assert_se(image_policy_intersect(x, y, &t) >= 0); @@ -163,8 +173,8 @@ TEST(image_policy_intersect) { static void test_policy_ignore_designators_one(const char *a, const PartitionDesignator array[], size_t n, const char *b) { _cleanup_(image_policy_freep) ImagePolicy *x = NULL, *y = NULL, *t = NULL; - ASSERT_OK(image_policy_from_string(a, &x)); - ASSERT_OK(image_policy_from_string(b, &y)); + ASSERT_OK(image_policy_from_string(a, /* graceful= */ false, &x)); + ASSERT_OK(image_policy_from_string(b, /* graceful= */ false, &y)); _cleanup_free_ char *s1 = NULL, *s2 = NULL, *s3 = NULL; ASSERT_OK(image_policy_to_string(x, true, &s1)); diff --git a/src/udev/udev-builtin-dissect_image.c b/src/udev/udev-builtin-dissect_image.c index 1f3728d2293..8b706a7f9a3 100644 --- a/src/udev/udev-builtin-dissect_image.c +++ b/src/udev/udev-builtin-dissect_image.c @@ -34,7 +34,7 @@ static int acquire_image_policy(ImagePolicy **ret) { return 0; } - r = image_policy_from_string(value, ret); + r = image_policy_from_string(value, /* graceful= */ false, ret); if (r < 0) return log_error_errno(r, "Failed to parse image policy '%s': %m", value); diff --git a/test/units/TEST-50-DISSECT.dissect.sh b/test/units/TEST-50-DISSECT.dissect.sh index 742fcf1d2f0..47d6defafd9 100755 --- a/test/units/TEST-50-DISSECT.dissect.sh +++ b/test/units/TEST-50-DISSECT.dissect.sh @@ -237,6 +237,12 @@ systemd-run --wait -P \ -p RootImagePolicy='root=signed' \ -p MountAPIVFS=yes \ cat /usr/lib/os-release | grep -F "MARKER=1" >/dev/null +systemd-run --wait -P \ + -p RootImage="$MINIMAL_IMAGE.gpt" \ + -p RootHash="$MINIMAL_IMAGE_ROOTHASH" \ + -p RootImagePolicy='root=signed+lol:wut=wat+signed' \ + -p MountAPIVFS=yes \ + cat /usr/lib/os-release | grep -F "MARKER=1" >/dev/null (! systemd-run --wait -P \ -p RootImage="$MINIMAL_IMAGE.gpt" \ -p RootHash="$MINIMAL_IMAGE_ROOTHASH" \