core: gracefully skip unknown policy designators in RootImagePolicy et al (systemd#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 d452335

Author: bluca

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]);
 

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);
 

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

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);
 

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)

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 */

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));

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);
 

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" \