From 7c0afcdde22d3d94fd23bfd0e473c263aaf54e8a Mon Sep 17 00:00:00 2001 From: Luca Boccassi Date: Thu, 11 Dec 2025 05:38:26 +0000 Subject: [PATCH] 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 --- src/analyze/analyze-image-policy.c | 2 +- src/core/dbus-execute.c | 2 +- src/core/execute-serialize.c | 6 ++-- src/mountfsd/mountwork.c | 4 +-- src/shared/image-policy.c | 26 ++++++++++----- src/shared/image-policy.h | 4 +-- src/test/test-image-policy.c | 46 ++++++++++++++++----------- src/udev/udev-builtin-dissect_image.c | 2 +- test/units/TEST-50-DISSECT.dissect.sh | 6 ++++ 9 files changed, 62 insertions(+), 36 deletions(-) diff --git a/src/analyze/analyze-image-policy.c b/src/analyze/analyze-image-policy.c index f7075c60a6..2075d1b437 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 83b0ddcb9e..5a0b083b97 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 02435447d7..bed2776889 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 7e4c5063fa..24f6e23e23 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 7a248318eb..bd758ea1c9 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 6fdd8abe32..037900b840 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 f35bfc6881..6cf7dff20b 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 1f3728d229..8b706a7f9a 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 742fcf1d2f..47d6defafd 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" \