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 f7075c60a6602f5d47e269358985b477fbc7951b..2075d1b437aad45ac638c579db6334f5cde56aad 100644 (file)
--- 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 83b0ddcb9e153247d49ff487eb0c4d448362e65f..5a0b083b97dd0d8c21f0e60e0b2109b4e1c9debf 100644 (file)
--- 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 02435447d730d772f678936d4ef1aa0542f104fb..bed2776889ab839768c20f474c477ddbd7c277df 100644 (file)
--- 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 7e4c5063fa3492c65350045337d2cda3fdc4130c..24f6e23e231ca3c33346fb5d3379baf3e9296c99 100644 (file)
--- 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 7a248318ebf41f2646c39cd77cb5ad976b54f56f..bd758ea1c940a736289be21491c28bbc55ffd139 100644 (file)
--- 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 6fdd8abe32428152426f2c0f94fa80ed3ef97f35..037900b840c165419006c596209a37578f59d4ee 100644 (file)
--- 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 f35bfc6881da6b2593d6e1f4af0d6d1a775ff04a..6cf7dff20bbb5edd87d94015359113724c422dc8 100644 (file)
--- 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 1f3728d2293c085a35b8e902547f8f8af9a92f72..8b706a7f9a32febf5d98d6184707adbbca555fe7 100644 (file)
--- 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 742fcf1d2f0f35855adb7647ed1e70f3a344bc44..47d6defafd933428bb999b2993c151016bec80fa 100755 (executable)
--- 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" \